Skip to content

Commit f24f9a9

Browse files
committed
fix(setup/mfa): thread requestId through MFA auto-skip so the OIDC ceremony resumes (fixes signin_failed)
1 parent 1775393 commit f24f9a9

3 files changed

Lines changed: 59 additions & 2 deletions

File tree

app/resources/mfa/mfa.service.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ export function offerableSetupRoutes(
227227

228228
export interface MfaSetupInput {
229229
loginName: string;
230+
requestId?: string;
230231
organization?: string;
231232
}
232233

@@ -272,7 +273,7 @@ export type MfaSetupResult = MfaSetupRedirect | MfaSetupData;
272273
export async function resolveMfaSetup(
273274
provider: AuthProvider,
274275
sessions: SessionEntry[],
275-
{ loginName, organization }: MfaSetupInput
276+
{ loginName, requestId, organization }: MfaSetupInput
276277
): Promise<MfaSetupResult> {
277278
const entry = byLoginName(sessions, loginName, organization);
278279
if (!entry) return { kind: 'redirect', target: '/login' };
@@ -310,6 +311,7 @@ export async function resolveMfaSetup(
310311
settings: refreshedSettings,
311312
loginName: session.user?.loginName ?? loginName,
312313
mfaInitSkippedAt: refreshedUser?.mfaInitSkippedAt,
314+
requestId,
313315
organization,
314316
});
315317

app/routes/setup/mfa.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
9595
// reads the cookie, mints the CSRF token, and wires the result to a redirect or rendered chooser.
9696
const provider = providerForRequest(request);
9797
const sessions = await readSessions(request);
98-
const result = await resolveMfaSetup(provider, sessions, { loginName, organization });
98+
const result = await resolveMfaSetup(provider, sessions, { loginName, requestId, organization });
9999
if (result.kind === 'redirect') return redirect(result.target);
100100

101101
const { csrfToken, headers } = await loaderCsrf(request);

cypress/component/resources/mfa/mfa.service.cy.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,61 @@ describe('resolveMfaSetup — auto-skip when no MFA methods are offerable', () =
132132
});
133133
});
134134

135+
// ── resolveMfaSetup: auto-skip threads requestId into redirect target ───────────────────────────
136+
//
137+
// Regression guard: when resolveMfaSetup auto-skips (no offerable MFA methods), the resulting
138+
// redirect target MUST carry the requestId so the OIDC/SAML ceremony can resume. Without it,
139+
// nextStepFromSession builds a target without the requestId query param → the handback to the
140+
// IdP is broken → staging returns /id/error?code=signin_failed after Google IdP login.
141+
//
142+
// REQUEST_ID_PATTERN in next-step only threads ids matching oidc_*/saml_*/device_* prefixes,
143+
// so we use 'oidc_test123' to exercise the actual threading path.
144+
145+
describe('resolveMfaSetup — auto-skip threads requestId into the redirect target', () => {
146+
const NO_MFA_CAPS = {
147+
passkey: false,
148+
u2f: false,
149+
totpOtp: false,
150+
emailOtp: false,
151+
smsOtp: false,
152+
externalIdp: false,
153+
ldap: false,
154+
saml: false,
155+
oidc: false,
156+
registration: false,
157+
};
158+
159+
const scenarioWithRequestId = {
160+
fn: 'resolveMfaSetup' as const,
161+
provider: 'fresh' as const,
162+
seed: {
163+
users: [{ id: 'u-nomfa2', loginName: 'nomfa2@test.example' }],
164+
authMethods: { 'u-nomfa2': ['password'] as string[] },
165+
capabilities: NO_MFA_CAPS,
166+
},
167+
liveSessions: [
168+
{
169+
id: 'sess-nomfa2',
170+
token: 'tok-nomfa2',
171+
user: { id: 'u-nomfa2', loginName: 'nomfa2@test.example' },
172+
},
173+
],
174+
request: {
175+
url: 'http://localhost/id/setup/mfa',
176+
sessions: [{ id: 'sess-nomfa2', token: 'tok-nomfa2', loginName: 'nomfa2@test.example' }],
177+
},
178+
mfaInput: { loginName: 'nomfa2@test.example', requestId: 'oidc_test123' },
179+
};
180+
181+
it('redirect target includes requestId=oidc_test123 so the OIDC ceremony can resume', () => {
182+
callService(scenarioWithRequestId).then((v) => {
183+
const o = v.outcome as { kind: string; target?: string };
184+
expect(o.kind, 'auto-skip: kind must be redirect').to.equal('redirect');
185+
expect(o.target ?? '', 'target must carry requestId').to.include('requestId=oidc_test123');
186+
});
187+
});
188+
});
189+
135190
// ── resolveMfaSetup: normal path (non-empty offerable set) still returns offerableKeys ──────────
136191
//
137192
// Regression guard: ensure the fix does not break the normal chooser path where MFA methods ARE

0 commit comments

Comments
 (0)