Skip to content

Commit cd9e7af

Browse files
committed
fix(webapp): only lock the email field when an IdP really owns the address
The check treated any org with SSO switched on as owning every member's email. It now requires SSO to be enforced, a live connection, and the member's domain to be one the org has verified - which is what the enforcement dialog promises, so contractors on other domains keep their own address instead of being sent to an admin who can't help them. An unreachable SSO service is now its own answer rather than being folded into "an IdP owns this". The save is still refused, but the page says it couldn't check instead of asserting something it doesn't know. A definite answer from any org also wins over an org that couldn't be read, so one unreachable org neither masks a real claim nor blocks the write alone.
1 parent 47fe9c6 commit cd9e7af

3 files changed

Lines changed: 159 additions & 21 deletions

File tree

apps/webapp/app/routes/account._index/route.tsx

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ import { useFeatures } from "~/hooks/useFeatures";
6565
import { useHasAdminAccess, useUser } from "~/hooks/useUser";
6666
import { updateUserEmail, updateUserMarketingEmails, updateUserName } from "~/models/user.server";
6767
import { profileUpdateRateLimiter } from "~/services/profileUpdateRateLimiter.server";
68-
import { isSsoManagedUser } from "~/services/ssoManagedIdentity.server";
68+
import { type EmailOwnership, getEmailOwnership } from "~/services/ssoManagedIdentity.server";
6969
import {
7070
updateContrastPreference,
7171
updateIconContrastPreference,
@@ -209,7 +209,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
209209
user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false }));
210210

211211
// Picks the modal only; the action re-checks before writing.
212-
const isSsoManaged = await isSsoManagedUser(user.id);
212+
const emailOwnership = await getEmailOwnership(user);
213213

214214
// Null when the user has no project yet; the row hides itself.
215215
let sidebarContext: {
@@ -228,7 +228,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
228228
};
229229
} catch {}
230230

231-
return json({ showThemeSwitcher, sidebarContext, isSsoManaged });
231+
return json({ showThemeSwitcher, sidebarContext, emailOwnership });
232232
}
233233

234234
export const action: ActionFunction = async ({ request }) => {
@@ -333,12 +333,20 @@ export const action: ActionFunction = async ({ request }) => {
333333
if (rateLimited) return rateLimited;
334334

335335
// Re-checked: the loader only picked the modal.
336-
if (await isSsoManagedUser(userId)) {
336+
const user = await requireUser(request);
337+
const ownership = await getEmailOwnership(user);
338+
if (ownership === "idp") {
337339
return profileUpdateError(
338340
"Your email address is managed by your organization's identity provider.",
339341
403
340342
);
341343
}
344+
if (ownership === "unknown") {
345+
return profileUpdateError(
346+
"We couldn't check your single sign-on settings just now. Please try again shortly.",
347+
503
348+
);
349+
}
342350

343351
const submission = EmailSchema.safeParse({ email: formData.get("email") });
344352
if (!submission.success) {
@@ -486,7 +494,7 @@ function EditNameButton() {
486494
);
487495
}
488496

489-
function EditEmailButton({ isSsoManaged }: { isSsoManaged: boolean }) {
497+
function EditEmailButton({ ownership }: { ownership: EmailOwnership }) {
490498
const user = useUser();
491499
const [isOpen, setIsOpen] = useState(false);
492500
const { fetcher, error, setError, isSubmitting } = useProfileFieldUpdate({
@@ -517,12 +525,17 @@ function EditEmailButton({ isSsoManaged }: { isSsoManaged: boolean }) {
517525
<DialogHeader>
518526
<DialogTitle>Email address</DialogTitle>
519527
</DialogHeader>
520-
{isSsoManaged ? (
528+
{ownership === "idp" ? (
521529
<Paragraph variant="small" className="pt-2">
522530
Your organization uses single sign-on, so your email address is managed by your
523531
identity provider rather than here. To change it, ask an organization admin to update
524532
it for you.
525533
</Paragraph>
534+
) : ownership === "unknown" ? (
535+
<Paragraph variant="small" className="pt-2">
536+
We couldn't check your organization's single sign-on settings just now, so this can't
537+
be edited yet. Please try again shortly.
538+
</Paragraph>
526539
) : (
527540
<fetcher.Form method="post">
528541
<input type="hidden" name="action" value="update-email" />
@@ -742,7 +755,7 @@ function CustomizeSidebarButton({
742755

743756
export default function Page() {
744757
const user = useUser();
745-
const { showThemeSwitcher, sidebarContext, isSsoManaged } = useLoaderData<typeof loader>();
758+
const { showThemeSwitcher, sidebarContext, emailOwnership } = useLoaderData<typeof loader>();
746759
const themeFetcher = useFetcher();
747760
const contrastFetcher = useFetcher();
748761
const iconContrastFetcher = useFetcher();
@@ -860,7 +873,7 @@ export default function Page() {
860873
<Paragraph variant="small" className="min-w-0 break-all text-right">
861874
{user.email}
862875
</Paragraph>
863-
<EditEmailButton isSsoManaged={isSsoManaged} />
876+
<EditEmailButton ownership={emailOwnership} />
864877
</div>
865878
</div>
866879
</div>
Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,83 @@
1+
import type { OrgSsoStatus } from "@trigger.dev/plugins";
12
import { prisma } from "~/db.server";
23
import { logger } from "~/services/logger.server";
34
import { ssoController } from "~/services/sso.server";
45

56
/**
6-
* Whether an identity provider owns this user's identity rather than the user.
7-
* Any org they belong to counts. Fails closed: if the plugin can't answer, treat
8-
* the account as IdP-managed rather than allowing an unverifiable write.
7+
* Who owns a user's email address.
8+
*
9+
* - `user` - theirs to change.
10+
* - `idp` - an identity provider asserts it, so changing it here would break
11+
* their next login.
12+
* - `unknown` - SSO couldn't be reached. Refuse the write, but don't claim an IdP
13+
* owns it.
914
*/
10-
export async function isSsoManagedUser(userId: string): Promise<boolean> {
15+
export type EmailOwnership = "user" | "idp" | "unknown";
16+
17+
/**
18+
* An org owns a member's email only when SSO is enforced, a connection is live,
19+
* and the member's domain is one the org has verified. Enforcement alone isn't
20+
* enough: members on other domains (contractors) keep their own sign-in, so
21+
* their address is still theirs.
22+
*/
23+
export function idpOwnsEmailDomain(status: OrgSsoStatus, emailDomain: string): boolean {
24+
if (!status.enforced) return false;
25+
if (!status.connections.some((connection) => connection.state === "active")) return false;
26+
return status.domains.some(
27+
(domain) => domain.verified && domain.domain.toLowerCase() === emailDomain
28+
);
29+
}
30+
31+
function domainOf(email: string): string | undefined {
32+
const domain = email.toLowerCase().trim().split("@")[1];
33+
return domain || undefined;
34+
}
35+
36+
export async function getEmailOwnership(user: {
37+
id: string;
38+
email: string;
39+
}): Promise<EmailOwnership> {
1140
if (!(await ssoController.isUsingPlugin())) {
12-
return false;
41+
return "user";
42+
}
43+
44+
const emailDomain = domainOf(user.email);
45+
if (!emailDomain) {
46+
return "user";
1347
}
1448

1549
const memberships = await prisma.orgMember.findMany({
16-
where: { userId, organization: { deletedAt: null } },
50+
where: { userId: user.id, organization: { deletedAt: null } },
1751
select: { organizationId: true },
1852
});
1953

2054
if (memberships.length === 0) {
21-
return false;
55+
return "user";
2256
}
2357

2458
const statuses = await Promise.all(
2559
memberships.map((membership) => ssoController.getStatus(membership.organizationId))
2660
);
2761

62+
// A definite answer from any org wins over an org we couldn't read, so one
63+
// unreachable org doesn't mask a real IdP claim - or block a write on its own.
64+
let unreadable = false;
65+
2866
for (const [index, status] of statuses.entries()) {
2967
if (status.isErr()) {
30-
logger.warn("SSO status lookup failed; treating the account as IdP-managed", {
31-
userId,
68+
unreadable = true;
69+
logger.warn("SSO status lookup failed; can't establish email ownership", {
70+
userId: user.id,
3271
organizationId: memberships[index].organizationId,
3372
reason: status.error,
3473
});
35-
return true;
74+
continue;
3675
}
3776

38-
if (status.value.hasIdpOrg && status.value.connections.some((c) => c.state === "active")) {
39-
return true;
77+
if (idpOwnsEmailDomain(status.value, emailDomain)) {
78+
return "idp";
4079
}
4180
}
4281

43-
return false;
82+
return unreadable ? "unknown" : "user";
4483
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { OrgSsoStatus } from "@trigger.dev/plugins";
2+
import { describe, expect, it } from "vitest";
3+
import { idpOwnsEmailDomain } from "~/services/ssoManagedIdentity.server";
4+
5+
function status(overrides: Partial<OrgSsoStatus> = {}): OrgSsoStatus {
6+
return {
7+
hasIdpOrg: true,
8+
enforced: true,
9+
jitProvisioningEnabled: false,
10+
jitDefaultRoleId: null,
11+
idpOrgId: "idp_123",
12+
primaryConnectionId: "conn_123",
13+
domains: [
14+
{ domain: "acme.com", verified: true, state: "verified", verificationFailedReason: null },
15+
],
16+
connections: [{ id: "conn_123", name: "Okta", connectionType: "OktaSAML", state: "active" }],
17+
...overrides,
18+
};
19+
}
20+
21+
describe("idpOwnsEmailDomain", () => {
22+
it("claims a member on a verified domain of an enforcing org", () => {
23+
expect(idpOwnsEmailDomain(status(), "acme.com")).toBe(true);
24+
});
25+
26+
it("leaves a contractor on another domain alone", () => {
27+
expect(idpOwnsEmailDomain(status(), "freelance.io")).toBe(false);
28+
});
29+
30+
it("leaves everyone alone until SSO is enforced", () => {
31+
expect(idpOwnsEmailDomain(status({ enforced: false }), "acme.com")).toBe(false);
32+
});
33+
34+
it("ignores a domain that hasn't been verified", () => {
35+
expect(
36+
idpOwnsEmailDomain(
37+
status({
38+
domains: [
39+
{
40+
domain: "acme.com",
41+
verified: false,
42+
state: "pending",
43+
verificationFailedReason: null,
44+
},
45+
],
46+
}),
47+
"acme.com"
48+
)
49+
).toBe(false);
50+
});
51+
52+
it("ignores an org with no live connection", () => {
53+
expect(
54+
idpOwnsEmailDomain(
55+
status({
56+
connections: [
57+
{ id: "conn_123", name: "Okta", connectionType: "OktaSAML", state: "inactive" },
58+
],
59+
}),
60+
"acme.com"
61+
)
62+
).toBe(false);
63+
});
64+
65+
it("matches domains case-insensitively", () => {
66+
expect(
67+
idpOwnsEmailDomain(
68+
status({
69+
domains: [
70+
{
71+
domain: "ACME.com",
72+
verified: true,
73+
state: "verified",
74+
verificationFailedReason: null,
75+
},
76+
],
77+
}),
78+
"acme.com"
79+
)
80+
).toBe(true);
81+
});
82+
83+
it("does not treat a subdomain as the verified domain", () => {
84+
expect(idpOwnsEmailDomain(status(), "mail.acme.com")).toBe(false);
85+
});
86+
});

0 commit comments

Comments
 (0)