fix: sso cross org link with valid invite (#621)
This commit is contained in:
parent
6769f5fa6c
commit
c1e8036323
9 changed files with 284 additions and 23 deletions
|
|
@ -69,7 +69,7 @@ describe("LoginPage", () => {
|
|||
|
||||
expect(
|
||||
await screen.findByText(
|
||||
"Your account exists but is not linked to this SSO provider. Sign in with username/password first, then enable auto linking in your provider settings or contact your administrator.",
|
||||
"SSO sign-in was blocked because this email already belongs to another user in this instance. Contact your administrator to resolve the account conflict.",
|
||||
),
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ export type LoginErrorCode = (typeof LOGIN_ERROR_CODES)[number];
|
|||
export function getLoginErrorDescription(errorCode: LoginErrorCode): string {
|
||||
switch (errorCode) {
|
||||
case "ACCOUNT_LINK_REQUIRED":
|
||||
return "Your account exists but is not linked to this SSO provider. Sign in with username/password first, then enable auto linking in your provider settings or contact your administrator.";
|
||||
return "SSO sign-in was blocked because this email already belongs to another user in this instance. Contact your administrator to resolve the account conflict.";
|
||||
case "EMAIL_NOT_VERIFIED":
|
||||
return "Your identity provider did not mark your email as verified.";
|
||||
case "INVITE_REQUIRED":
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
type MiddlewareContext,
|
||||
type MiddlewareOptions,
|
||||
} from "better-auth";
|
||||
import { APIError } from "better-auth/api";
|
||||
import { drizzleAdapter } from "better-auth/adapters/drizzle";
|
||||
import { admin, twoFactor, username, organization, testUtils } from "better-auth/plugins";
|
||||
import { createAuthMiddleware } from "better-auth/api";
|
||||
|
|
@ -58,6 +59,20 @@ export const auth = betterAuth({
|
|||
provider: "sqlite",
|
||||
}),
|
||||
databaseHooks: {
|
||||
account: {
|
||||
create: {
|
||||
before: async (account, ctx) => {
|
||||
if (ssoIntegration.isSsoCallback(ctx)) {
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(account.userId, account.providerId);
|
||||
if (!allowed) {
|
||||
throw new APIError("FORBIDDEN", {
|
||||
message: "SSO account linking is not permitted for users outside this organization",
|
||||
});
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
},
|
||||
user: {
|
||||
delete: {
|
||||
before: async (user) => {
|
||||
|
|
|
|||
32
app/server/modules/sso/__tests__/sso.errors.test.ts
Normal file
32
app/server/modules/sso/__tests__/sso.errors.test.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import { describe, expect, test } from "bun:test";
|
||||
import { mapAuthErrorToCode } from "../sso.errors";
|
||||
|
||||
describe("mapAuthErrorToCode", () => {
|
||||
test("maps account-not-linked errors to ACCOUNT_LINK_REQUIRED", () => {
|
||||
expect(mapAuthErrorToCode(encodeURIComponent("account not linked"))).toBe("ACCOUNT_LINK_REQUIRED");
|
||||
});
|
||||
|
||||
test("maps unable-to-link-account errors to ACCOUNT_LINK_REQUIRED", () => {
|
||||
expect(mapAuthErrorToCode(encodeURIComponent("unable to link account"))).toBe("ACCOUNT_LINK_REQUIRED");
|
||||
});
|
||||
|
||||
test("maps security account-linking denial to ACCOUNT_LINK_REQUIRED", () => {
|
||||
expect(
|
||||
mapAuthErrorToCode(
|
||||
encodeURIComponent("SSO account linking is not permitted for users outside this organization"),
|
||||
),
|
||||
).toBe("ACCOUNT_LINK_REQUIRED");
|
||||
});
|
||||
|
||||
test("maps invite-required errors to INVITE_REQUIRED", () => {
|
||||
expect(
|
||||
mapAuthErrorToCode(
|
||||
encodeURIComponent("Access denied. You must be invited to this organization before you can sign in with SSO."),
|
||||
),
|
||||
).toBe("INVITE_REQUIRED");
|
||||
});
|
||||
|
||||
test("maps unknown errors to SSO_LOGIN_FAILED", () => {
|
||||
expect(mapAuthErrorToCode(encodeURIComponent("some random error"))).toBe("SSO_LOGIN_FAILED");
|
||||
});
|
||||
});
|
||||
|
|
@ -38,6 +38,22 @@ async function createUser(email: string, username: string) {
|
|||
return userId;
|
||||
}
|
||||
|
||||
async function createUserWithCredentialAccount(email: string, username: string) {
|
||||
const userId = await createUser(email, username);
|
||||
|
||||
await db.insert(account).values({
|
||||
id: randomId(),
|
||||
accountId: username,
|
||||
providerId: "credential",
|
||||
userId,
|
||||
password: "test-password-hash",
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
});
|
||||
|
||||
return userId;
|
||||
}
|
||||
|
||||
describe("ssoIntegration.resolveOrgMembership", () => {
|
||||
beforeEach(async () => {
|
||||
await db.delete(member);
|
||||
|
|
@ -329,3 +345,141 @@ describe("ssoIntegration.resolveOrgMembership", () => {
|
|||
expect(updatedInvitation.status).toBe("accepted");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ssoIntegration.canLinkSsoAccount", () => {
|
||||
beforeEach(async () => {
|
||||
await db.delete(member);
|
||||
await db.delete(account);
|
||||
await db.delete(invitation);
|
||||
await db.delete(ssoProvider);
|
||||
await db.delete(organization);
|
||||
await db.delete(usersTable);
|
||||
});
|
||||
|
||||
async function setupOrgWithProvider(providerId: string, autoLinkMatchingEmails = false) {
|
||||
const orgId = randomId();
|
||||
const ownerId = await createUser(`owner-${randomSlug("u")}@example.com`, randomSlug("owner"));
|
||||
await db.insert(organization).values({ id: orgId, name: "Acme", slug: randomSlug("acme"), createdAt: new Date() });
|
||||
await db.insert(ssoProvider).values({
|
||||
id: randomId(),
|
||||
providerId,
|
||||
organizationId: orgId,
|
||||
userId: ownerId,
|
||||
issuer: "https://issuer.example.com",
|
||||
domain: "example.com",
|
||||
autoLinkMatchingEmails,
|
||||
});
|
||||
return { orgId, ownerId };
|
||||
}
|
||||
|
||||
const autoLinkingStates = [false, true] as const;
|
||||
|
||||
test("allows linking for a user who is already a member of the org", async () => {
|
||||
const { orgId } = await setupOrgWithProvider("oidc-acme");
|
||||
const userId = await createUserWithCredentialAccount("alice@example.com", randomSlug("alice"));
|
||||
await db.insert(member).values({
|
||||
id: randomId(),
|
||||
userId,
|
||||
organizationId: orgId,
|
||||
role: "member",
|
||||
createdAt: new Date(),
|
||||
});
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(userId, "oidc-acme");
|
||||
|
||||
expect(allowed).toBe(true);
|
||||
});
|
||||
|
||||
for (const autoLinkingEnabled of autoLinkingStates) {
|
||||
test(`allows linking for a new SSO user with a valid pending invitation (auto-linking ${autoLinkingEnabled ? "enabled" : "disabled"})`, async () => {
|
||||
const providerId = `oidc-acme-${autoLinkingEnabled ? "on" : "off"}`;
|
||||
const { orgId, ownerId } = await setupOrgWithProvider(providerId, autoLinkingEnabled);
|
||||
const userId = await createUser("alice@example.com", randomSlug("alice"));
|
||||
await db.insert(invitation).values({
|
||||
id: randomId(),
|
||||
organizationId: orgId,
|
||||
email: "alice@example.com",
|
||||
role: "member",
|
||||
status: "pending",
|
||||
expiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
createdAt: new Date(),
|
||||
inviterId: ownerId,
|
||||
});
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(userId, providerId);
|
||||
|
||||
expect(allowed).toBe(true);
|
||||
});
|
||||
|
||||
test(`blocks linking for an existing user account even with a pending invitation (auto-linking ${autoLinkingEnabled ? "enabled" : "disabled"})`, async () => {
|
||||
const providerId = `oidc-acme-${autoLinkingEnabled ? "on" : "off"}`;
|
||||
const { orgId, ownerId } = await setupOrgWithProvider(providerId, autoLinkingEnabled);
|
||||
const userId = await createUserWithCredentialAccount("alice@example.com", randomSlug("alice"));
|
||||
await db.insert(invitation).values({
|
||||
id: randomId(),
|
||||
organizationId: orgId,
|
||||
email: "alice@example.com",
|
||||
role: "member",
|
||||
status: "pending",
|
||||
expiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
createdAt: new Date(),
|
||||
inviterId: ownerId,
|
||||
});
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(userId, providerId);
|
||||
|
||||
expect(allowed).toBe(false);
|
||||
});
|
||||
}
|
||||
|
||||
test("blocks linking for a user with no membership and no invitation in the org", async () => {
|
||||
await setupOrgWithProvider("oidc-acme");
|
||||
const aliceId = await createUserWithCredentialAccount("alice@example.com", randomSlug("alice"));
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(aliceId, "oidc-acme");
|
||||
|
||||
expect(allowed).toBe(false);
|
||||
});
|
||||
|
||||
test("blocks linking when the user only has an invitation to a different org", async () => {
|
||||
const { ownerId } = await setupOrgWithProvider("oidc-acme");
|
||||
const otherOrgId = randomId();
|
||||
await db
|
||||
.insert(organization)
|
||||
.values({ id: otherOrgId, name: "Other", slug: randomSlug("other"), createdAt: new Date() });
|
||||
const aliceId = await createUserWithCredentialAccount("alice@example.com", randomSlug("alice"));
|
||||
await db.insert(invitation).values({
|
||||
id: randomId(),
|
||||
organizationId: otherOrgId,
|
||||
email: "alice@example.com",
|
||||
role: "member",
|
||||
status: "pending",
|
||||
expiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
createdAt: new Date(),
|
||||
inviterId: ownerId,
|
||||
});
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(aliceId, "oidc-acme");
|
||||
|
||||
expect(allowed).toBe(false);
|
||||
});
|
||||
|
||||
test("blocks linking when the user's invitation to the org has expired", async () => {
|
||||
const { orgId, ownerId } = await setupOrgWithProvider("oidc-acme");
|
||||
const aliceId = await createUserWithCredentialAccount("alice@example.com", randomSlug("alice"));
|
||||
await db.insert(invitation).values({
|
||||
id: randomId(),
|
||||
organizationId: orgId,
|
||||
email: "alice@example.com",
|
||||
role: "member",
|
||||
status: "pending",
|
||||
expiresAt: new Date(Date.now() - 60 * 60 * 1000), // expired
|
||||
createdAt: new Date(),
|
||||
inviterId: ownerId,
|
||||
});
|
||||
|
||||
const allowed = await ssoIntegration.canLinkSsoAccount(aliceId, "oidc-acme");
|
||||
|
||||
expect(allowed).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -76,17 +76,25 @@ describe("resolveTrustedProvidersForRequest", () => {
|
|||
expect(await resolveTrustedProvidersForRequest(createRequest("/sso/callback/missing-provider"))).toEqual([]);
|
||||
});
|
||||
|
||||
test("returns auto-link-enabled providers from the callback provider organization", async () => {
|
||||
const { organizationId, userId } = await createSsoProviderRecord("pocket-id", true);
|
||||
test.each([
|
||||
{ callbackAutoLinking: true, sameOrgAutoLinking: true, expected: ["acme-saml", "pocket-id"] },
|
||||
{ callbackAutoLinking: false, sameOrgAutoLinking: true, expected: ["acme-saml"] },
|
||||
{ callbackAutoLinking: true, sameOrgAutoLinking: false, expected: ["pocket-id"] },
|
||||
{ callbackAutoLinking: false, sameOrgAutoLinking: false, expected: [] },
|
||||
])(
|
||||
"returns trusted providers matrix (callback: $callbackAutoLinking, same-org: $sameOrgAutoLinking)",
|
||||
async ({ callbackAutoLinking, sameOrgAutoLinking, expected }) => {
|
||||
const { organizationId, userId } = await createSsoProviderRecord("pocket-id", callbackAutoLinking);
|
||||
|
||||
await createSsoProviderRecord("acme-saml", true, { organizationId, userId });
|
||||
await createSsoProviderRecord("acme-disabled", false, { organizationId, userId });
|
||||
await createSsoProviderRecord("other-org-provider", true);
|
||||
await createSsoProviderRecord("acme-saml", sameOrgAutoLinking, { organizationId, userId });
|
||||
await createSsoProviderRecord("acme-disabled", false, { organizationId, userId });
|
||||
await createSsoProviderRecord("other-org-provider", true);
|
||||
|
||||
const trustedProviders = await resolveTrustedProvidersForRequest(createRequest("/sso/callback/pocket-id"));
|
||||
const trustedProviders = await resolveTrustedProvidersForRequest(createRequest("/sso/callback/pocket-id"));
|
||||
|
||||
expect([...trustedProviders].sort()).toEqual(["acme-saml", "pocket-id"]);
|
||||
});
|
||||
expect([...trustedProviders].sort()).toEqual([...expected]);
|
||||
},
|
||||
);
|
||||
|
||||
test("supports /sso/saml2/callback/:providerId paths", async () => {
|
||||
await createSsoProviderRecord("saml-provider", true);
|
||||
|
|
@ -112,7 +120,7 @@ describe("resolveTrustedProvidersForRequest", () => {
|
|||
]);
|
||||
});
|
||||
|
||||
test("removes providers from the result when auto-linking is disabled", async () => {
|
||||
test("updates trusted providers immediately when callback provider auto-linking is toggled", async () => {
|
||||
await createSsoProviderRecord("pocket-id", true);
|
||||
|
||||
expect(await resolveTrustedProvidersForRequest(createRequest("/sso/callback/pocket-id"))).toEqual(["pocket-id"]);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,12 @@ const INVITE_REQUIRED_ERRORS = new Set([
|
|||
"unable to create session",
|
||||
]);
|
||||
|
||||
const ACCOUNT_LINK_REQUIRED_ERRORS = new Set([
|
||||
"account not linked",
|
||||
"unable to link account",
|
||||
"SSO account linking is not permitted for users outside this organization",
|
||||
]);
|
||||
|
||||
export function mapAuthErrorToCode(error: string): LoginErrorCode {
|
||||
let decoded: string;
|
||||
|
||||
|
|
@ -15,7 +21,7 @@ export function mapAuthErrorToCode(error: string): LoginErrorCode {
|
|||
return "SSO_LOGIN_FAILED";
|
||||
}
|
||||
|
||||
if (decoded === "account not linked") {
|
||||
if (ACCOUNT_LINK_REQUIRED_ERRORS.has(decoded)) {
|
||||
return "ACCOUNT_LINK_REQUIRED";
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ async function resolveOrgMembership(userId: string, ctx: GenericEndpointContext
|
|||
throw new APIError("FORBIDDEN", { message: "SSO sign-in is invite-only for this organization" });
|
||||
}
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
db.transaction((tx) => {
|
||||
tx.insert(member)
|
||||
.values({
|
||||
id: Bun.randomUUIDv7(),
|
||||
|
|
@ -82,6 +82,39 @@ async function onUserCreate(
|
|||
user.hasDownloadedResticPassword = true;
|
||||
}
|
||||
|
||||
async function canLinkSsoAccount(userId: string, providerId: string): Promise<boolean> {
|
||||
const ssoProviderRecord = await ssoService.getSsoProviderById(providerId);
|
||||
if (!ssoProviderRecord) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const existingMembership = await findMembershipWithOrganization(userId, ssoProviderRecord.organizationId);
|
||||
if (existingMembership) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const existingAccount = await db.query.account.findFirst({
|
||||
where: { userId },
|
||||
columns: { id: true },
|
||||
});
|
||||
|
||||
if (existingAccount) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const user = await db.query.usersTable.findFirst({ where: { id: userId } });
|
||||
if (!user) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const pendingInvitation = await ssoService.getPendingInvitation(
|
||||
ssoProviderRecord.organizationId,
|
||||
normalizeEmail(user.email),
|
||||
);
|
||||
|
||||
return !!pendingInvitation;
|
||||
}
|
||||
|
||||
async function resolveOrgMembershipOrThrow(userId: string, ctx: GenericEndpointContext | null) {
|
||||
const membership = await resolveOrgMembership(userId, ctx);
|
||||
if (!membership) {
|
||||
|
|
@ -122,5 +155,7 @@ export const ssoIntegration = {
|
|||
|
||||
resolveOrgMembership,
|
||||
|
||||
canLinkSsoAccount,
|
||||
|
||||
resolveTrustedProviders: resolveTrustedProvidersForRequest,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ const autoLinkUninvitedLocalUsername = "sso-link-guard";
|
|||
const inviteOnlyMessage =
|
||||
"Access is invite-only. Ask an organization admin to send you an invitation before signing in with SSO.";
|
||||
const accountLinkRequiredMessage =
|
||||
"Your account exists but is not linked to this SSO provider. Sign in with username/password first, then enable auto linking in your provider settings or contact your administrator.";
|
||||
"SSO sign-in was blocked because this email already belongs to another user in this instance. Contact your administrator to resolve the account conflict.";
|
||||
|
||||
type OrgMembersResponse = {
|
||||
members: {
|
||||
|
|
@ -226,18 +226,27 @@ async function withOidcLoginAttempt(
|
|||
}
|
||||
}
|
||||
|
||||
function isLoginPath(url: string): boolean {
|
||||
const pathname = new URL(url).pathname;
|
||||
return pathname === "/login" || pathname === "/login/error";
|
||||
}
|
||||
|
||||
function isSsoCallbackPath(url: string): boolean {
|
||||
return new URL(url).pathname.startsWith("/api/auth/sso/callback/");
|
||||
}
|
||||
|
||||
async function expectInviteOnlyLoginError(page: Page) {
|
||||
await expect
|
||||
.poll(
|
||||
() => {
|
||||
const url = page.url();
|
||||
return /\/login(\/error)?/.test(url) || /\/api\/auth\/sso\/callback\//.test(url);
|
||||
return isLoginPath(url) || isSsoCallbackPath(url);
|
||||
},
|
||||
{ timeout: 30000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
if (/\/login(\/error)?/.test(page.url())) {
|
||||
if (isLoginPath(page.url())) {
|
||||
await waitForAppReady(page);
|
||||
await expect(page.getByText(inviteOnlyMessage)).toBeVisible();
|
||||
return;
|
||||
|
|
@ -251,19 +260,23 @@ async function expectAccountLinkRequiredLoginError(page: Page) {
|
|||
.poll(
|
||||
() => {
|
||||
const url = page.url();
|
||||
return /\/login(\/error)?/.test(url) || /\/api\/auth\/sso\/callback\//.test(url);
|
||||
return isLoginPath(url) || isSsoCallbackPath(url);
|
||||
},
|
||||
{ timeout: 30000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
if (/\/login(\/error)?/.test(page.url())) {
|
||||
if (isLoginPath(page.url())) {
|
||||
await waitForAppReady(page);
|
||||
await expect(page.getByText(accountLinkRequiredMessage)).toBeVisible();
|
||||
return;
|
||||
}
|
||||
|
||||
await expect(page.getByText(/account exists but is not linked/i)).toBeVisible();
|
||||
await expect(
|
||||
page.getByText(
|
||||
/(account not linked|unable to link account|already belongs to another user|outside this organization)/i,
|
||||
),
|
||||
).toBeVisible();
|
||||
}
|
||||
|
||||
test("uninvited OIDC users are blocked", async ({ page, browser }) => {
|
||||
|
|
@ -321,7 +334,7 @@ test("auto-link policy enforces invitation and controls account linking", async
|
|||
await setProviderAutoLinking(page, providerIds.autoLinkNoInvite, true);
|
||||
|
||||
await withOidcLoginAttempt(browser, providerIds.autoLinkNoInvite, autoLinkUninvitedLocalEmail, async (ssoPage) => {
|
||||
await expectInviteOnlyLoginError(ssoPage);
|
||||
await expectAccountLinkRequiredLoginError(ssoPage);
|
||||
});
|
||||
|
||||
await registerOidcProvider(page, providerIds.autoLink);
|
||||
|
|
@ -336,8 +349,6 @@ test("auto-link policy enforces invitation and controls account linking", async
|
|||
await setProviderAutoLinking(page, providerIds.autoLink, true);
|
||||
|
||||
await withOidcLoginAttempt(browser, providerIds.autoLink, autoLinkTargetEmail, async (ssoPage) => {
|
||||
await ssoPage.waitForURL(/\/volumes/, { timeout: 30000 });
|
||||
await waitForAppReady(ssoPage);
|
||||
await expect(ssoPage).toHaveURL(/\/volumes/);
|
||||
await expectAccountLinkRequiredLoginError(ssoPage);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue