Skip to content

Commit f432227

Browse files
committed
fix(sso): keep Vercel install across SSO handoff; harden SSO callback
- vercel.onboarding: already-authenticated users whose domain requires SSO were redirected to /login/sso, which bounces authed users home and dropped the single-use Vercel code. Destroy the session first via authenticator.logout so /login/sso accepts them and the resume URL survives the round-trip (addresses PR #3911 review). - auth.sso.callback: sanitize redirectTo at the exit point (IdP-initiated values come from relay-state and never passed the host sanitizer) and attribute the referral source, matching the other auth callbacks. - ssoAuth: extract private early-return helpers (resolveSsoUserId, attachSsoIdentityBestEffort, provisionJitMembershipBestEffort, runPostAuthentication) and fail closed if the user can't be confirmed post-resolution instead of minting a session.
1 parent af75bc4 commit f432227

3 files changed

Lines changed: 129 additions & 83 deletions

File tree

apps/webapp/app/routes/auth.sso.callback.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { logger } from "~/services/logger.server";
99
import { ssoController } from "~/services/sso.server";
1010
import { commitAuthenticatedSession } from "~/services/sessionDuration.server";
1111
import { commitSession, getUserSession } from "~/services/sessionStorage.server";
12+
import { trackAndClearReferralSource } from "~/services/referralSource.server";
13+
import { sanitizeRedirectPath } from "~/utils";
1214

1315
// Resolve the SSO completion for either the SP-initiated (state present)
1416
// or IdP-initiated (no state) flow. Throws a redirect to the error page
@@ -44,7 +46,15 @@ export async function loader({ request }: LoaderFunctionArgs) {
4446
}
4547
const state = url.searchParams.get("state");
4648

47-
const { profile, redirectTo, flow } = await resolveSsoCompletion(code, state);
49+
const { profile, redirectTo: rawRedirectTo, flow } = await resolveSsoCompletion(code, state);
50+
// Sanitize at the exit point regardless of flow. SP-initiated values were
51+
// sanitized on the way in (auth.sso.ts) and signed into the state token, but
52+
// IdP-initiated `redirectTo` originates from the IdP's relay-state and never
53+
// passed through the host — without this an IdP admin could craft an open
54+
// redirect. Mirrors every other auth callback. A rejected value falls back
55+
// to "/". The Vercel resume URL (`/vercel/onboarding?...`) is navigable and
56+
// survives.
57+
const redirectTo = sanitizeRedirectPath(rawRedirectTo);
4858

4959
// `throwOnError` makes the SSO strategy's verify-callback failures
5060
// (resolveSsoIdentity errors, DB failures in findOrCreateSsoUser,
@@ -106,5 +116,9 @@ export async function loader({ request }: LoaderFunctionArgs) {
106116
headers.append("Set-Cookie", await commitAuthenticatedSession(session, auth.userId));
107117
headers.append("Set-Cookie", await setLastAuthMethodHeader("sso"));
108118

119+
// Attribute the referral source on the final session creation, like the
120+
// other non-MFA auth callbacks. The MFA path defers this to `completeLogin`.
121+
await trackAndClearReferralSource(request, auth.userId, headers);
122+
109123
return redirect(redirectTo, { headers });
110124
}

apps/webapp/app/routes/vercel.onboarding.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { Label } from "~/components/primitives/Label";
1717
import { Select, SelectItem } from "~/components/primitives/Select";
1818
import { ButtonSpinner } from "~/components/primitives/Spinner";
1919
import { prisma } from "~/db.server";
20+
import { authenticator } from "~/services/auth.server";
2021
import { logger } from "~/services/logger.server";
2122
import { requireUserId } from "~/services/session.server";
2223
import { ssoRedirectForEmail } from "~/services/ssoAutoDiscovery.server";
@@ -221,7 +222,17 @@ export async function action({ request }: ActionFunctionArgs) {
221222
resumeUrl
222223
);
223224
if (ssoRedirect) {
224-
return redirect(ssoRedirect);
225+
// The user is already authenticated via a non-SSO method, so a plain
226+
// redirect to `/login/sso` would be bounced straight home by that
227+
// route's already-authenticated guard — silently dropping the install.
228+
// Destroy the current session first (mirroring the OAuth callbacks,
229+
// which never commit a session when SSO is required) so `/login/sso`
230+
// accepts them. `authenticator.logout` redirects to `ssoRedirect`
231+
// verbatim — unlike the `/logout` route it doesn't run the redirect
232+
// sanitizer, which would otherwise reject the non-navigable
233+
// `/login/sso` target. The resume URL rides along as `redirectTo` and
234+
// survives the SSO round-trip.
235+
return authenticator.logout(request, { redirectTo: ssoRedirect });
225236
}
226237
}
227238

apps/webapp/app/services/ssoAuth.server.ts

Lines changed: 102 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -54,93 +54,114 @@ class SsoStrategy extends Strategy<AuthUser, SsoVerifyParams> {
5454
}
5555
}
5656

57-
export function addSsoStrategy(authenticator: Authenticator<AuthUser>) {
58-
authenticator.use(
59-
new SsoStrategy(async ({ profile, flow }) => {
60-
const decision = await ssoController.resolveSsoIdentity({ profile });
61-
if (decision.isErr()) {
62-
// Surfaces "feature_disabled" in OSS deployments. The callback
63-
// route's error path translates this into a generic
64-
// sign-in-failed user-facing message.
65-
throw new Error(`SSO resolve failed: ${decision.error}`);
66-
}
57+
// Resolve the host User for a verified SSO profile, creating one on the
58+
// `create_new_user` decision. Throws on an errored decision — this surfaces
59+
// "feature_disabled" in OSS deployments, which the callback route's error
60+
// path translates into a generic sign-in-failed user-facing message.
61+
async function resolveSsoUserId(
62+
profile: SsoProfile
63+
): Promise<{ userId: string; isNewUser: boolean }> {
64+
const decision = await ssoController.resolveSsoIdentity({ profile });
65+
if (decision.isErr()) {
66+
throw new Error(`SSO resolve failed: ${decision.error}`);
67+
}
68+
69+
if (decision.value.kind === "create_new_user") {
70+
const created = await findOrCreateSsoUser({
71+
authenticationMethod: "SSO",
72+
email: profile.email,
73+
firstName: profile.firstName,
74+
lastName: profile.lastName,
75+
});
76+
return { userId: created.user.id, isNewUser: created.isNewUser };
77+
}
6778

68-
const value = decision.value;
79+
return { userId: decision.value.userId, isNewUser: false };
80+
}
6981

70-
let userId: string;
71-
let isNewUser = false;
82+
// Best-effort: attaching the IdP identity row is an optimisation for the
83+
// next login (it lets resolveSsoIdentity take the existing_user_by_idp fast
84+
// path instead of falling back to linked_by_email). The user is already
85+
// authenticated by this point, so we log and continue rather than failing
86+
// the sign-in; a later successful login will write the row.
87+
async function attachSsoIdentityBestEffort(
88+
userId: string,
89+
profile: SsoProfile,
90+
flow: SsoFlow
91+
): Promise<void> {
92+
const attach = await ssoController.attachSsoIdentity({ userId, profile });
93+
if (attach.isErr()) {
94+
logger.warn("SSO attachSsoIdentity failed", { reason: attach.error, userId, flow });
95+
}
96+
}
7297

73-
if (value.kind === "create_new_user") {
74-
const created = await findOrCreateSsoUser({
75-
authenticationMethod: "SSO",
76-
email: profile.email,
77-
firstName: profile.firstName,
78-
lastName: profile.lastName,
79-
});
80-
userId = created.user.id;
81-
isNewUser = created.isNewUser;
82-
} else {
83-
userId = value.userId;
84-
}
98+
// Best-effort JIT org provisioning. Like attachSsoIdentity above, a failure
99+
// must not block an otherwise-valid sign-in: the user simply isn't
100+
// provisioned this time and a later login retries. "feature_disabled" is the
101+
// expected OSS-fallback result, so it's swallowed silently.
102+
async function provisionJitMembershipBestEffort(
103+
userId: string,
104+
profile: SsoProfile,
105+
flow: SsoFlow
106+
): Promise<void> {
107+
const jit = await ssoController.evaluateJit({ userId, idpOrgId: profile.idpOrgId });
108+
if (jit.isErr()) {
109+
if (jit.error !== "feature_disabled") {
110+
logger.warn("SSO evaluateJit failed", { reason: jit.error, userId, flow });
111+
}
112+
return;
113+
}
85114

86-
// Best-effort: attaching the IdP identity row is an optimisation
87-
// for the next login (it lets resolveSsoIdentity take the
88-
// existing_user_by_idp fast path instead of falling back to
89-
// linked_by_email). The user is already authenticated by this
90-
// point, so we log and continue rather than failing the sign-in;
91-
// a later successful login will write the row.
92-
const attach = await ssoController.attachSsoIdentity({ userId, profile });
93-
if (attach.isErr()) {
94-
logger.warn("SSO attachSsoIdentity failed", {
95-
reason: attach.error,
96-
userId,
97-
flow,
98-
});
99-
}
115+
if (!jit.value.shouldProvision) return;
100116

101-
const jit = await ssoController.evaluateJit({
102-
userId,
103-
idpOrgId: profile.idpOrgId,
104-
});
105-
if (jit.isOk() && jit.value.shouldProvision) {
106-
const [provisionError, result] = await tryCatch(
107-
ensureOrgMember({
108-
userId,
109-
organizationId: jit.value.organizationId,
110-
roleId: jit.value.roleId,
111-
source: "sso_jit",
112-
})
113-
);
114-
if (provisionError) {
115-
// Best-effort, like attachSsoIdentity above: a provisioning failure
116-
// (e.g. the RBAC role couldn't be applied, so ensureOrgMember rolled
117-
// back the membership) must not block an otherwise-valid sign-in.
118-
// The user simply isn't provisioned this time; a later login retries.
119-
logger.warn("SSO JIT provisioning failed", {
120-
reason:
121-
provisionError instanceof Error ? provisionError.message : String(provisionError),
122-
userId,
123-
organizationId: jit.value.organizationId,
124-
flow,
125-
});
126-
} else if (!result.created) {
127-
logger.info("SSO JIT skipped — membership already exists", {
128-
userId,
129-
organizationId: jit.value.organizationId,
130-
});
131-
}
132-
} else if (jit.isErr() && jit.error !== "feature_disabled") {
133-
logger.warn("SSO evaluateJit failed", { reason: jit.error, userId, flow });
134-
}
117+
const [provisionError, result] = await tryCatch(
118+
ensureOrgMember({
119+
userId,
120+
organizationId: jit.value.organizationId,
121+
roleId: jit.value.roleId,
122+
source: "sso_jit",
123+
})
124+
);
125+
if (provisionError) {
126+
// e.g. the RBAC role couldn't be applied, so ensureOrgMember rolled back
127+
// the membership.
128+
logger.warn("SSO JIT provisioning failed", {
129+
reason: provisionError instanceof Error ? provisionError.message : String(provisionError),
130+
userId,
131+
organizationId: jit.value.organizationId,
132+
flow,
133+
});
134+
return;
135+
}
136+
137+
if (!result.created) {
138+
logger.info("SSO JIT skipped — membership already exists", {
139+
userId,
140+
organizationId: jit.value.organizationId,
141+
});
142+
}
143+
}
144+
145+
async function runPostAuthentication(userId: string, isNewUser: boolean): Promise<void> {
146+
const user = await prisma.user.findFirst({ where: { id: userId } });
147+
if (!user) {
148+
// The user was just resolved or created above, so a null here means it
149+
// was hard-deleted mid-flow (or a DB inconsistency). Fail closed — throw
150+
// rather than skipping postAuthentication and still returning a valid
151+
// AuthUser, which would mint a session for a user we can't confirm.
152+
throw new Error(`SSO user not found after resolution: ${userId}`);
153+
}
154+
await postAuthentication({ user, isNewUser, loginMethod: "SSO" });
155+
}
156+
157+
export function addSsoStrategy(authenticator: Authenticator<AuthUser>) {
158+
authenticator.use(
159+
new SsoStrategy(async ({ profile, flow }) => {
160+
const { userId, isNewUser } = await resolveSsoUserId(profile);
135161

136-
const user = await prisma.user.findFirst({ where: { id: userId } });
137-
if (user) {
138-
await postAuthentication({
139-
user,
140-
isNewUser,
141-
loginMethod: "SSO",
142-
});
143-
}
162+
await attachSsoIdentityBestEffort(userId, profile, flow);
163+
await provisionJitMembershipBestEffort(userId, profile, flow);
164+
await runPostAuthentication(userId, isNewUser);
144165

145166
// Carry the SSO marker on the returned AuthUser so the session is
146167
// self-describing — `revalidateSsoSession()` keys off `AuthUser.sso`,

0 commit comments

Comments
 (0)