Skip to content

Commit 3a20c62

Browse files
steebchenclaude
andcommitted
fix(auth): address SSO/SCIM review feedback
Follow-up to #2924. Implements the actionable review feedback from that PR: SCIM (apps/api/src/routes/scim.ts) - Scope GET/PUT/PATCH/DELETE /Users/:id and group membership to the token's org so a token can't read or mutate users outside its tenant. - Honor `active: false` on user create (staged/suspended assignments no longer get org membership). - Recompute roles when a user is reactivated via PUT/PATCH and when a group is renamed, so mapped admin/owner members aren't left as developer. - Guard against removing the org's last owner during deprovisioning. - Link the SSO provider account for pre-existing users, not just new ones. - Enforce displayName uniqueness on group rename (PUT/PATCH). - Push /Users and /Groups filtering + pagination into the database. SSO management (apps/api/src/routes/sso.ts) - Block plugin bypass: the raw Better Auth /auth/sso management endpoints are now rejected at the HTTP handler (they only require a session). - Prevent admins (non-owners) from creating owner role mappings. - Recompute affected members when a role mapping is added or removed. - Guard against a missing provider row after registration and clean up orphans. - Clear SCIM tokens' stale provider link when a connection is deleted. - Derive the SCIM token's provider link from the org's connection. - Audit SCIM revoke against the token id, not the org id. Auth enforcement (apps/api/src/auth/config.ts) - Gate SSO enforcement on SSO_ENABLED and on an active, enterprise org. - On a blocked non-SSO login, delete only that session, not all of the user's. UI (apps/ui) - Preserve the ?redirect= target and surface thrown errors on SSO sign-in. - Confirm before destructive SSO/SCIM actions; only toast on successful copy; show a loading state until the org plan resolves. Shared role-recompute logic extracted to apps/api/src/lib/sso-roles.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7faa91c commit 3a20c62

7 files changed

Lines changed: 637 additions & 209 deletions

File tree

apps/api/src/auth/config.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,44 @@ const isHosted = process.env.HOSTED === "true";
3333
async function isSSOEnforcedForEmail(
3434
email: string | null | undefined,
3535
): Promise<boolean> {
36+
// Keep enforcement tied to the same flag that surfaces the SSO sign-in entry
37+
// point: with SSO_ENABLED off there's no way to sign in via SSO, so blocking
38+
// password/social/passkey would just lock the domain out entirely.
39+
if (process.env.SSO_ENABLED !== "true") {
40+
return false;
41+
}
42+
3643
const domain = email?.trim().toLowerCase().split("@")[1];
3744
if (!domain) {
3845
return false;
3946
}
4047
const providers = await db.query.ssoProvider.findMany({
4148
where: { enforced: { eq: true } },
42-
columns: { domain: true },
49+
columns: { domain: true, organizationId: true },
50+
});
51+
const matching = providers.filter(
52+
(provider) =>
53+
provider.organizationId &&
54+
provider.domain
55+
.split(",")
56+
.map((d) => d.trim().toLowerCase())
57+
.includes(domain),
58+
);
59+
if (!matching.length) {
60+
return false;
61+
}
62+
63+
// Only an active, enterprise org can enforce SSO. A soft-deleted org can no
64+
// longer be managed to turn enforcement off, so a stale enforced row must not
65+
// keep blocking a domain's users forever.
66+
const orgs = await db.query.organization.findMany({
67+
where: {
68+
id: { in: [...new Set(matching.map((p) => p.organizationId as string))] },
69+
},
70+
columns: { status: true, plan: true },
4371
});
44-
return providers.some((provider) =>
45-
provider.domain
46-
.split(",")
47-
.map((d) => d.trim().toLowerCase())
48-
.includes(domain),
72+
return orgs.some(
73+
(org) => org.status !== "deleted" && org.plan === "enterprise",
4974
);
5075
}
5176

@@ -1060,9 +1085,12 @@ The LLM Gateway Team`.trim();
10601085
!ctx.path.startsWith("/sso/") &&
10611086
(await isSSOEnforcedForEmail(dbUser?.email))
10621087
) {
1088+
// Delete only the session this blocked attempt just created — not
1089+
// every session for the user — so a rejected social/passkey login
1090+
// doesn't also sign them out of valid SSO sessions elsewhere.
10631091
await db
10641092
.delete(tables.session)
1065-
.where(eq(tables.session.userId, userId));
1093+
.where(eq(tables.session.id, newSession.session.id));
10661094
return ssoRequiredResponse();
10671095
}
10681096

apps/api/src/auth/handler.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,25 @@ import type { ServerTypes } from "@/vars.js";
77
// Create a Hono app for auth routes
88
export const authHandler = new OpenAPIHono<ServerTypes>();
99

10+
// Better Auth's sso() plugin mounts its own provider-management endpoints under
11+
// the /auth/* catch-all. They only require a session (any authenticated user),
12+
// so exposing them would let a user bypass the enterprise/admin-gated /sso
13+
// wrapper and register a provider — claiming any email domain — with no
14+
// organization scoping. Block direct HTTP access to them here; our server-side
15+
// registration calls `apiAuth.api.registerSSOProvider` directly and never goes
16+
// through this handler, so it is unaffected. The SAML sign-in/callback/metadata
17+
// endpoints (/auth/sso/saml2/*, /auth/sso/callback, /auth/sign-in/sso) are left
18+
// reachable because the login flow needs them.
19+
const BLOCKED_SSO_PLUGIN_PATHS = new Set([
20+
"/auth/sso/register",
21+
"/auth/sso/update-provider",
22+
"/auth/sso/delete-provider",
23+
"/auth/sso/get-provider",
24+
"/auth/sso/providers",
25+
"/auth/sso/request-domain-verification",
26+
"/auth/sso/verify-domain",
27+
]);
28+
1029
authHandler.use("*", async (c, next) => {
1130
const session = await apiAuth.api.getSession({ headers: c.req.raw.headers });
1231

@@ -22,5 +41,8 @@ authHandler.use("*", async (c, next) => {
2241
});
2342

2443
authHandler.on(["POST", "GET"], "/auth/*", (c) => {
44+
if (BLOCKED_SSO_PLUGIN_PATHS.has(new URL(c.req.url).pathname)) {
45+
return c.json({ error: "not_found", message: "Not found" }, 404);
46+
}
2547
return apiAuth.handler(c.req.raw);
2648
});

apps/api/src/lib/sso-roles.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { db, eq, tables } from "@llmgateway/db";
2+
3+
export type OrgRole = "owner" | "admin" | "developer";
4+
5+
export interface RoleChange {
6+
old: OrgRole;
7+
new: OrgRole;
8+
}
9+
10+
const ROLE_RANK: Record<OrgRole, number> = {
11+
developer: 1,
12+
admin: 2,
13+
owner: 3,
14+
};
15+
16+
// Recompute an org member's role from their SCIM group memberships and the
17+
// org's group->role mappings. The highest-precedence mapped role wins; the
18+
// default is `developer`. Owners are never auto-demoted — owner is only ever
19+
// assigned manually (or by an explicit owner mapping), so an admin who set up
20+
// SSO can't be locked out by a group that maps to a lower role.
21+
//
22+
// Returns the {old,new} role change when it updated the membership, or null
23+
// when nothing changed, so callers (e.g. SCIM) can audit the transition.
24+
export async function recomputeUserRole(
25+
userId: string,
26+
organizationId: string,
27+
): Promise<RoleChange | null> {
28+
const membership = await db.query.userOrganization.findFirst({
29+
where: {
30+
userId: { eq: userId },
31+
organizationId: { eq: organizationId },
32+
},
33+
columns: { id: true, role: true },
34+
});
35+
if (!membership) {
36+
return null;
37+
}
38+
39+
const groupMemberships = await db.query.scimGroupMember.findMany({
40+
where: { userId: { eq: userId } },
41+
columns: { scimGroupId: true },
42+
});
43+
const groupIds = groupMemberships.map((m) => m.scimGroupId);
44+
45+
let mappedRole: OrgRole = "developer";
46+
if (groupIds.length) {
47+
const groups = await db.query.scimGroup.findMany({
48+
where: {
49+
id: { in: groupIds },
50+
organizationId: { eq: organizationId },
51+
},
52+
columns: { displayName: true },
53+
});
54+
const names = groups.map((g) => g.displayName);
55+
if (names.length) {
56+
const mappings = await db.query.ssoRoleMapping.findMany({
57+
where: {
58+
organizationId: { eq: organizationId },
59+
groupName: { in: names },
60+
},
61+
columns: { role: true },
62+
});
63+
for (const mapping of mappings) {
64+
if (ROLE_RANK[mapping.role] > ROLE_RANK[mappedRole]) {
65+
mappedRole = mapping.role;
66+
}
67+
}
68+
}
69+
}
70+
71+
if (membership.role === "owner" && ROLE_RANK[mappedRole] < ROLE_RANK.owner) {
72+
return null;
73+
}
74+
if (membership.role !== mappedRole) {
75+
await db
76+
.update(tables.userOrganization)
77+
.set({ role: mappedRole })
78+
.where(eq(tables.userOrganization.id, membership.id));
79+
return { old: membership.role, new: mappedRole };
80+
}
81+
return null;
82+
}
83+
84+
// Recompute the role of every member of the SCIM group(s) with `groupName` in
85+
// this org. Used when a role mapping is added or removed after the IdP has
86+
// already pushed the group and its members, so existing members pick up (or
87+
// lose) the mapped role without waiting for a later SCIM membership event.
88+
export async function recomputeRoleForGroupName(
89+
organizationId: string,
90+
groupName: string,
91+
) {
92+
const groups = await db.query.scimGroup.findMany({
93+
where: {
94+
organizationId: { eq: organizationId },
95+
displayName: { eq: groupName },
96+
},
97+
columns: { id: true },
98+
});
99+
if (!groups.length) {
100+
return;
101+
}
102+
103+
const members = await db.query.scimGroupMember.findMany({
104+
where: { scimGroupId: { in: groups.map((g) => g.id) } },
105+
columns: { userId: true },
106+
});
107+
const userIds = [...new Set(members.map((m) => m.userId))];
108+
for (const userId of userIds) {
109+
await recomputeUserRole(userId, organizationId);
110+
}
111+
}

0 commit comments

Comments
 (0)