Skip to content

Commit 9ae2381

Browse files
authored
fix(auth): validate IdP authority before SAML account linking (calcom#26005)
* fix(auth): validate IdP authority before SAML account linking Adds verification that SAML IdP is authoritative for the email domain before allowing account conversion * fix(auth): deny by default on missing SAML tenant + optimize membership query - Block account conversion when tenant is missing (deny by default) - Replace JOIN + ILIKE with two indexed lookups for O(1) performance * refactor(auth): apply data minimization to security logs
1 parent a378054 commit 9ae2381

7 files changed

Lines changed: 427 additions & 1 deletion

File tree

apps/web/app/(use-page-wrapper)/auth/error/page.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const ServerPage = async ({ searchParams }: PageProps) => {
4747
? "SAML (like Okta)"
4848
: "your original login method";
4949
return t("account_managed_by_identity_provider_error", { provider: providerName });
50+
} else if (error === "saml-idp-not-authoritative") {
51+
return t("saml_idp_not_authoritative_error");
5052
}
5153
return t("error_during_login") + (error ? ` Error code: ${error}` : "");
5254
};

apps/web/public/static/locales/en/common.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4168,6 +4168,7 @@
41684168
"timestamp": "Timestamp",
41694169
"json": "JSON",
41704170
"hubspot_ignore_guests": "Do not create new records for guests added to the booking",
4171+
"saml_idp_not_authoritative_error": "This SAML identity provider is not authorized to manage your account. Please sign in with your original login method.",
41714172
"audit_logs_organization_required": "You must be part of an organization to view audit logs.",
41724173
"audit_logs_booking_not_found_or_permission_denied": "Booking not found or you do not have permission to view its audit logs.",
41734174
"audit_logs_booking_has_no_owner": "Cannot verify permissions: booking has no associated user.",

packages/features/auth/lib/next-auth-options.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { teamMetadataSchema, userMetadata } from "@calcom/prisma/zod-utils";
4747
import { getOrgUsernameFromEmail } from "../signup/utils/getOrgUsernameFromEmail";
4848
import { ErrorCode } from "./ErrorCode";
4949
import { dub } from "./dub";
50+
import { validateSamlAccountConversion } from "./samlAccountLinking";
5051
import CalComAdapter from "./next-auth-custom-adapter";
5152
import { verifyPassword } from "./verifyPassword";
5253

@@ -315,6 +316,10 @@ if (isSAMLLoginEnabled) {
315316
lastName?: string;
316317
email?: string;
317318
locale?: string;
319+
requested?: {
320+
tenant?: string;
321+
product?: string;
322+
};
318323
}) => {
319324
log.debug("BoxyHQ:profile", safeStringify({ profile }));
320325
const userRepo = new UserRepository(prisma);
@@ -329,6 +334,8 @@ if (isSAMLLoginEnabled) {
329334
name: `${profile.firstName || ""} ${profile.lastName || ""}`.trim(),
330335
email_verified: true,
331336
locale: profile.locale,
337+
// Pass SAML tenant for domain authority checks in signIn callback
338+
samlTenant: profile.requested?.tenant,
332339
...(user ? { profile: user.allProfiles[0] } : {}),
333340
};
334341
},
@@ -939,6 +946,15 @@ export const getOptions = ({
939946
existingUserWithEmail.emailVerified &&
940947
existingUserWithEmail.identityProvider !== IdentityProvider.CAL
941948
) {
949+
// Verify SAML IdP is authoritative before auto-merge
950+
if (idP === IdentityProvider.SAML) {
951+
const samlTenant = (user as { samlTenant?: string }).samlTenant;
952+
const validation = await validateSamlAccountConversion(samlTenant, user.email, "SelfHosted→SAML");
953+
if (!validation.allowed) {
954+
return validation.errorUrl;
955+
}
956+
}
957+
942958
if (existingUserWithEmail.twoFactorEnabled) {
943959
return loginWithTotp(existingUserWithEmail.email);
944960
} else {
@@ -952,6 +968,15 @@ export const getOptions = ({
952968
!existingUserWithEmail.emailVerified &&
953969
!existingUserWithEmail.username
954970
) {
971+
// Verify SAML IdP is authoritative before claiming invited user
972+
if (idP === IdentityProvider.SAML) {
973+
const samlTenant = (user as { samlTenant?: string }).samlTenant;
974+
const validation = await validateSamlAccountConversion(samlTenant, user.email, "Invite→SAML");
975+
if (!validation.allowed) {
976+
return validation.errorUrl;
977+
}
978+
}
979+
955980
await prisma.user.update({
956981
where: {
957982
email: existingUserWithEmail.email,
@@ -981,6 +1006,15 @@ export const getOptions = ({
9811006
existingUserWithEmail.identityProvider === IdentityProvider.CAL &&
9821007
(idP === IdentityProvider.GOOGLE || idP === IdentityProvider.SAML)
9831008
) {
1009+
// Verify SAML IdP is authoritative before converting account
1010+
if (idP === IdentityProvider.SAML) {
1011+
const samlTenant = (user as { samlTenant?: string }).samlTenant;
1012+
const validation = await validateSamlAccountConversion(samlTenant, user.email, "CAL→SAML");
1013+
if (!validation.allowed) {
1014+
return validation.errorUrl;
1015+
}
1016+
}
1017+
9841018
await prisma.user.update({
9851019
where: { email: existingUserWithEmail.email },
9861020
// also update email to the IdP email
@@ -1002,6 +1036,13 @@ export const getOptions = ({
10021036
existingUserWithEmail.identityProvider === IdentityProvider.GOOGLE &&
10031037
idP === IdentityProvider.SAML
10041038
) {
1039+
// Verify SAML IdP is authoritative before converting account
1040+
const samlTenant = (user as { samlTenant?: string }).samlTenant;
1041+
const validation = await validateSamlAccountConversion(samlTenant, user.email, "Google→SAML");
1042+
if (!validation.allowed) {
1043+
return validation.errorUrl;
1044+
}
1045+
10051046
await prisma.user.update({
10061047
where: { email: existingUserWithEmail.email },
10071048
// also update email to the IdP email
Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
import type { PrismaClient } from "@prisma/client";
2+
import { describe, it, expect, vi, beforeEach, type MockInstance } from "vitest";
3+
4+
import { MembershipRepository } from "@calcom/features/membership/repositories/MembershipRepository";
5+
import { OrganizationSettingsRepository } from "@calcom/features/organizations/repositories/OrganizationSettingsRepository";
6+
7+
import {
8+
SamlAccountLinkingService,
9+
getTeamIdFromSamlTenant,
10+
validateSamlAccountConversion,
11+
} from "./samlAccountLinking";
12+
13+
vi.mock("@calcom/lib/logger", () => ({
14+
default: {
15+
getSubLogger: () => ({
16+
warn: vi.fn(),
17+
error: vi.fn(),
18+
debug: vi.fn(),
19+
}),
20+
},
21+
}));
22+
23+
vi.mock("@calcom/prisma", () => ({
24+
prisma: {},
25+
}));
26+
27+
const mockPrismaClient = {} as PrismaClient;
28+
29+
let hasAcceptedMembershipSpy: MockInstance;
30+
let getVerifiedDomainsSpy: MockInstance;
31+
32+
function setupMocks(config: { verifiedDomains?: string[]; hasMembership?: boolean }) {
33+
hasAcceptedMembershipSpy = vi
34+
.spyOn(MembershipRepository.prototype, "hasAcceptedMembershipByEmail")
35+
.mockResolvedValue(config.hasMembership ?? false);
36+
37+
getVerifiedDomainsSpy = vi
38+
.spyOn(OrganizationSettingsRepository.prototype, "getVerifiedDomains")
39+
.mockResolvedValue(config.verifiedDomains ?? []);
40+
}
41+
42+
describe("getTeamIdFromSamlTenant", () => {
43+
it("extracts team ID from valid tenant string", () => {
44+
expect(getTeamIdFromSamlTenant("team-123")).toBe(123);
45+
expect(getTeamIdFromSamlTenant("team-1")).toBe(1);
46+
expect(getTeamIdFromSamlTenant("team-999999")).toBe(999999);
47+
});
48+
49+
it("returns null for invalid tenant formats", () => {
50+
expect(getTeamIdFromSamlTenant("")).toBeNull();
51+
expect(getTeamIdFromSamlTenant("invalid")).toBeNull();
52+
expect(getTeamIdFromSamlTenant("org-123")).toBeNull();
53+
expect(getTeamIdFromSamlTenant("team-")).toBeNull();
54+
expect(getTeamIdFromSamlTenant("team-abc")).toBeNull();
55+
});
56+
57+
it("returns null for tenant with non-numeric ID", () => {
58+
expect(getTeamIdFromSamlTenant("team-abc123")).toBeNull();
59+
});
60+
61+
it("truncates decimal values (parseInt behavior)", () => {
62+
expect(getTeamIdFromSamlTenant("team-12.5")).toBe(12);
63+
});
64+
});
65+
66+
describe("SamlAccountLinkingService.isSamlIdpAuthoritativeForEmail", () => {
67+
let service: SamlAccountLinkingService;
68+
const ORG_TEAM_ID = 123;
69+
70+
beforeEach(() => {
71+
vi.restoreAllMocks();
72+
});
73+
74+
it("returns authoritative when email domain matches org verified domain", async () => {
75+
setupMocks({ verifiedDomains: ["acme.com"], hasMembership: false });
76+
service = new SamlAccountLinkingService(mockPrismaClient);
77+
78+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@acme.com");
79+
80+
expect(result).toEqual({ authoritative: true, reason: "domain_verified" });
81+
expect(hasAcceptedMembershipSpy).not.toHaveBeenCalled();
82+
});
83+
84+
it("performs case-insensitive domain matching", async () => {
85+
setupMocks({ verifiedDomains: ["acme.com"] });
86+
service = new SamlAccountLinkingService(mockPrismaClient);
87+
88+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@ACME.COM");
89+
90+
expect(result).toEqual({ authoritative: true, reason: "domain_verified" });
91+
});
92+
93+
it("matches subdomains of verified domain", async () => {
94+
setupMocks({ verifiedDomains: ["acme.com"] });
95+
service = new SamlAccountLinkingService(mockPrismaClient);
96+
97+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@sales.acme.com");
98+
99+
expect(result).toEqual({ authoritative: true, reason: "domain_verified" });
100+
});
101+
102+
it("returns not authoritative when domain doesn't match and user is not member", async () => {
103+
setupMocks({ verifiedDomains: ["acme.com"], hasMembership: false });
104+
service = new SamlAccountLinkingService(mockPrismaClient);
105+
106+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@different.com");
107+
108+
expect(result).toEqual({ authoritative: false, reason: "domain_mismatch" });
109+
});
110+
111+
it("returns authoritative for existing org member with different domain", async () => {
112+
setupMocks({ verifiedDomains: ["acme.com"], hasMembership: true });
113+
service = new SamlAccountLinkingService(mockPrismaClient);
114+
115+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@personal-email.com");
116+
117+
expect(result).toEqual({ authoritative: true, reason: "existing_member" });
118+
});
119+
120+
it("skips membership check when domain matches", async () => {
121+
setupMocks({ verifiedDomains: ["acme.com"], hasMembership: true });
122+
service = new SamlAccountLinkingService(mockPrismaClient);
123+
124+
await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@acme.com");
125+
126+
expect(hasAcceptedMembershipSpy).not.toHaveBeenCalled();
127+
});
128+
129+
it("returns not authoritative for invalid email", async () => {
130+
setupMocks({});
131+
service = new SamlAccountLinkingService(mockPrismaClient);
132+
133+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "invalid-email");
134+
135+
expect(result).toEqual({ authoritative: false, reason: "invalid_email" });
136+
expect(getVerifiedDomainsSpy).not.toHaveBeenCalled();
137+
});
138+
139+
it("returns not authoritative for empty email", async () => {
140+
setupMocks({});
141+
service = new SamlAccountLinkingService(mockPrismaClient);
142+
143+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "");
144+
145+
expect(result).toEqual({ authoritative: false, reason: "invalid_email" });
146+
});
147+
148+
it("falls back to membership when org has no verified domains", async () => {
149+
setupMocks({ verifiedDomains: [], hasMembership: true });
150+
service = new SamlAccountLinkingService(mockPrismaClient);
151+
152+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "user@any-domain.com");
153+
154+
expect(result).toEqual({ authoritative: true, reason: "existing_member" });
155+
});
156+
157+
it("rejects when org has no verified domains and user is not member", async () => {
158+
setupMocks({ verifiedDomains: [], hasMembership: false });
159+
service = new SamlAccountLinkingService(mockPrismaClient);
160+
161+
const result = await service.isSamlIdpAuthoritativeForEmail(ORG_TEAM_ID, "attacker@evil.com");
162+
163+
expect(result).toEqual({ authoritative: false, reason: "domain_mismatch" });
164+
});
165+
});
166+
167+
describe("validateSamlAccountConversion", () => {
168+
beforeEach(() => {
169+
vi.restoreAllMocks();
170+
setupMocks({ verifiedDomains: [], hasMembership: false });
171+
});
172+
173+
it("blocks when no SAML tenant provided (deny by default)", async () => {
174+
const result = await validateSamlAccountConversion(undefined, "user@example.com", "CAL→SAML");
175+
expect(result).toEqual({
176+
allowed: false,
177+
errorUrl: "/auth/error?error=saml-idp-not-authoritative",
178+
});
179+
});
180+
181+
it("allows when tenant is not org-based", async () => {
182+
const result = await validateSamlAccountConversion("Cal.com", "user@example.com", "CAL→SAML");
183+
expect(result).toEqual({ allowed: true });
184+
});
185+
186+
it("blocks when IdP is not authoritative", async () => {
187+
const result = await validateSamlAccountConversion("team-123", "attacker@evil.com", "CAL→SAML");
188+
189+
expect(result).toEqual({
190+
allowed: false,
191+
errorUrl: "/auth/error?error=saml-idp-not-authoritative",
192+
});
193+
});
194+
195+
it("allows when IdP is authoritative", async () => {
196+
setupMocks({ verifiedDomains: ["acme.com"] });
197+
198+
const result = await validateSamlAccountConversion("team-123", "user@acme.com", "CAL→SAML");
199+
200+
expect(result).toEqual({ allowed: true });
201+
});
202+
});
203+
204+
describe("Security: SAML Account Takeover Prevention", () => {
205+
beforeEach(() => {
206+
vi.restoreAllMocks();
207+
});
208+
209+
it("blocks takeover when attacker org asserts victim's email", async () => {
210+
setupMocks({ verifiedDomains: ["attacker-org.com"], hasMembership: false });
211+
212+
const result = await validateSamlAccountConversion("team-999", "victim@gmail.com", "Google→SAML");
213+
214+
expect(result.allowed).toBe(false);
215+
});
216+
217+
it("allows SSO when org owns the email domain", async () => {
218+
setupMocks({ verifiedDomains: ["acme.com"] });
219+
220+
const result = await validateSamlAccountConversion("team-100", "employee@acme.com", "CAL→SAML");
221+
222+
expect(result).toEqual({ allowed: true });
223+
});
224+
225+
it("allows SSO for existing members with personal email", async () => {
226+
setupMocks({ verifiedDomains: ["acme.com"], hasMembership: true });
227+
228+
const service = new SamlAccountLinkingService(mockPrismaClient);
229+
const result = await service.isSamlIdpAuthoritativeForEmail(100, "contractor@personal-email.com");
230+
231+
expect(result.authoritative).toBe(true);
232+
expect(result.reason).toBe("existing_member");
233+
});
234+
235+
it("blocks invite takeover when attacker org claims invited user", async () => {
236+
// Org A invites victim@gmail.com, attacker in Org B tries to claim via SAML
237+
setupMocks({ verifiedDomains: ["attacker-org.com"], hasMembership: false });
238+
239+
const result = await validateSamlAccountConversion("team-999", "victim@gmail.com", "Invite→SAML");
240+
241+
expect(result.allowed).toBe(false);
242+
});
243+
});

0 commit comments

Comments
 (0)