From 9087bce8a61bac03f5100f620e8f0b0aeb21e1dc Mon Sep 17 00:00:00 2001 From: Dan Schomburg Date: Thu, 14 May 2026 15:36:10 -0700 Subject: [PATCH 1/4] wip-fix-sumo-issue --- packages/fxa-auth-client/lib/client.ts | 1 + .../fxa-auth-server/lib/routes/session.js | 2 + .../test/remote/session_tests.in.spec.ts | 107 ++++++++-- packages/fxa-settings/src/models/mocks.tsx | 1 + .../src/pages/Signin/utils.test.ts | 195 ++++++++++++++++++ .../fxa-settings/src/pages/Signin/utils.ts | 10 +- 6 files changed, 300 insertions(+), 16 deletions(-) diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts index a74f4e03567..90df07d123a 100644 --- a/packages/fxa-auth-client/lib/client.ts +++ b/packages/fxa-auth-client/lib/client.ts @@ -149,6 +149,7 @@ export type SessionStatus = { uid: string; details: { accountEmailVerified: boolean; + mustVerify: boolean; sessionVerificationMethod: string | null; sessionVerified: boolean; sessionVerificationMeetsMinimumAAL: boolean; diff --git a/packages/fxa-auth-server/lib/routes/session.js b/packages/fxa-auth-server/lib/routes/session.js index 55e12d83edb..9b084b5916a 100644 --- a/packages/fxa-auth-server/lib/routes/session.js +++ b/packages/fxa-auth-server/lib/routes/session.js @@ -316,6 +316,7 @@ module.exports = function ( uid: isA.string().regex(HEX_STRING).required(), details: isA.object({ accountEmailVerified: isA.boolean(), + mustVerify: isA.boolean(), sessionVerificationMethod: isA.string().allow(null), sessionVerified: isA.boolean(), sessionVerificationMeetsMinimumAAL: isA.boolean(), @@ -359,6 +360,7 @@ module.exports = function ( uid: sessionToken.uid, details: { accountEmailVerified, + mustVerify: !!sessionToken.mustVerify, sessionVerificationMethod, sessionVerified, sessionVerificationMeetsMinimumAAL, diff --git a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts index 6922afa118d..450d51ec8e6 100644 --- a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts +++ b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts @@ -2,7 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { createTestServer, TestServerInstance } from '../support/helpers/test-server'; +import { + createTestServer, + TestServerInstance, +} from '../support/helpers/test-server'; const Client = require('../client')(); @@ -37,7 +40,11 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; const client = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, testOptions + server.publicUrl, + email, + password, + server.mailbox, + testOptions ); await client.sessionStatus(); @@ -57,7 +64,12 @@ describe.each(testVersions)( it('deletes a different custom token', async () => { const email = server.uniqueEmail(); const password = 'foobar'; - const client = await Client.create(server.publicUrl, email, password, testOptions); + const client = await Client.create( + server.publicUrl, + email, + password, + testOptions + ); const sessionTokenCreate = client.sessionToken; const sessions = await client.api.sessions(sessionTokenCreate); @@ -85,7 +97,12 @@ describe.each(testVersions)( it('fails with a bad custom token', async () => { const email = server.uniqueEmail(); const password = 'foobar'; - const client = await Client.create(server.publicUrl, email, password, testOptions); + const client = await Client.create( + server.publicUrl, + email, + password, + testOptions + ); const sessionTokenCreate = client.sessionToken; const c = await client.login(); @@ -107,7 +124,9 @@ describe.each(testVersions)( expect(err.code).toBe(401); expect(err.errno).toBe(110); expect(err.error).toBe('Unauthorized'); - expect(err.message).toBe('The authentication token could not be found'); + expect(err.message).toBe( + 'The authentication token could not be found' + ); } }); }); @@ -117,7 +136,11 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; const client1 = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, testOptions + server.publicUrl, + email, + password, + server.mailbox, + testOptions ); const client2 = await client1.duplicate(); @@ -150,7 +173,12 @@ describe.each(testVersions)( it('creates independent verification state for the new token', async () => { const email = server.uniqueEmail(); const password = 'foobar'; - const client1 = await Client.create(server.publicUrl, email, password, testOptions); + const client1 = await Client.create( + server.publicUrl, + email, + password, + testOptions + ); const client2 = await client1.duplicate(); expect(client1.verified).toBeFalsy(); @@ -183,7 +211,10 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; const client = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, + server.publicUrl, + email, + password, + server.mailbox, { ...testOptions, keys: true } ); @@ -205,7 +236,11 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; const client = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, testOptions + server.publicUrl, + email, + password, + server.mailbox, + testOptions ); await client.setupCredentials(email, 'fiibar'); @@ -225,7 +260,12 @@ describe.each(testVersions)( it('has sane account-verification behaviour', async () => { const email = server.uniqueEmail(); const password = 'foobar'; - const client = await Client.create(server.publicUrl, email, password, testOptions); + const client = await Client.create( + server.publicUrl, + email, + password, + testOptions + ); expect(client.verified).toBeFalsy(); // Clear the verification email, without verifying. @@ -247,7 +287,10 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, + server.publicUrl, + email, + password, + server.mailbox, { ...testOptions, keys: false } ); @@ -288,7 +331,10 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'foobar'; const client = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, + server.publicUrl, + email, + password, + server.mailbox, { ...testOptions, keys: true } ); @@ -307,7 +353,11 @@ describe.each(testVersions)( const email = server.uniqueEmail(); const password = 'testx'; const c = await Client.createAndVerify( - server.publicUrl, email, password, server.mailbox, testOptions + server.publicUrl, + email, + password, + server.mailbox, + testOptions ); const uid = c.uid; await c.login(); @@ -318,6 +368,7 @@ describe.each(testVersions)( uid, details: { accountEmailVerified: true, + mustVerify: false, sessionVerificationMeetsMinimumAAL: true, sessionVerificationMethod: null, sessionVerified: false, @@ -326,6 +377,36 @@ describe.each(testVersions)( }); }); + it('returns mustVerify=true when the session was created with keys=true', async () => { + const email = server.uniqueEmail(); + const password = 'testx'; + const c = await Client.createAndVerify( + server.publicUrl, + email, + password, + server.mailbox, + testOptions + ); + await c.login({ keys: true }); + const status = await c.api.sessionStatus(c.sessionToken); + expect(status.details.mustVerify).toBe(true); + }); + + it('returns mustVerify=false when the session was created without keys', async () => { + const email = server.uniqueEmail(); + const password = 'testx'; + const c = await Client.createAndVerify( + server.publicUrl, + email, + password, + server.mailbox, + testOptions + ); + await c.login({ keys: false }); + const status = await c.api.sessionStatus(c.sessionToken); + expect(status.details.mustVerify).toBe(false); + }); + it('errors with invalid token', async () => { const client = new Client(server.publicUrl, testOptions); try { diff --git a/packages/fxa-settings/src/models/mocks.tsx b/packages/fxa-settings/src/models/mocks.tsx index a87a1bfcccb..fd50e297d80 100644 --- a/packages/fxa-settings/src/models/mocks.tsx +++ b/packages/fxa-settings/src/models/mocks.tsx @@ -138,6 +138,7 @@ export function mockAuthClient() { details: { verified: true, accountEmailVerified: true, + mustVerify: false, sessionVerified: true, sessionVerificationMeetsMinimumAAL: true, sessionVerificationMethod: 'email', diff --git a/packages/fxa-settings/src/pages/Signin/utils.test.ts b/packages/fxa-settings/src/pages/Signin/utils.test.ts index c4f776d67f4..a24e4a1081c 100644 --- a/packages/fxa-settings/src/pages/Signin/utils.test.ts +++ b/packages/fxa-settings/src/pages/Signin/utils.test.ts @@ -4,11 +4,13 @@ import VerificationMethods from '../../constants/verification-methods'; import VerificationReasons from '../../constants/verification-reasons'; +import AuthenticationMethods from '../../constants/authentication-methods'; import { MOCK_EMAIL, MOCK_KEY_FETCH_TOKEN, MOCK_OAUTH_FLOW_HANDLER_RESPONSE, MOCK_SESSION_TOKEN, + MOCK_STORED_ACCOUNT, MOCK_UID, } from '../mocks'; import { NavigationOptions } from './interfaces'; @@ -18,14 +20,17 @@ import { createMockSigninOAuthIntegration, } from './mocks'; import { + cachedSignIn, handleNavigation, ensureCanLinkAcountOrRedirect, getSyncNavigate, } from './utils'; import * as ReachRouter from '@reach/router'; import * as ReactUtils from 'fxa-react/lib/utils'; +import * as CacheModule from '../../lib/cache'; import firefox from '../../lib/channels/firefox'; import config from '../../lib/config'; +import { AuthUiErrors } from '../../lib/auth-errors/auth-errors'; import { OAuthNativeServices } from '@fxa/accounts/oauth'; jest.mock('@reach/router', () => ({ @@ -583,4 +588,194 @@ describe('Signin utils', () => { }); }); }); + + describe('cachedSignIn', () => { + type SessionStatusDetails = { + accountEmailVerified: boolean; + mustVerify: boolean; + sessionVerified: boolean; + sessionVerificationMethod: string | null; + sessionVerificationMeetsMinimumAAL: boolean; + }; + + const createMockAuthClient = ({ + authenticationMethods = [] as AuthenticationMethods[], + details, + accountProfileRejection, + }: { + authenticationMethods?: AuthenticationMethods[]; + details?: Partial; + accountProfileRejection?: unknown; + } = {}) => { + const accountProfile = accountProfileRejection + ? jest.fn().mockRejectedValue(accountProfileRejection) + : jest.fn().mockResolvedValue({ authenticationMethods }); + const sessionStatus = jest.fn().mockResolvedValue({ + state: 'unverified', + uid: MOCK_UID, + details: { + accountEmailVerified: true, + mustVerify: false, + sessionVerified: false, + sessionVerificationMethod: null, + sessionVerificationMeetsMinimumAAL: true, + ...details, + }, + }); + return { accountProfile, sessionStatus } as any; + }; + + const createMockSession = () => + ({ + sendVerificationCode: jest.fn().mockResolvedValue(undefined), + }) as any; + + let currentAccountSpy: jest.SpyInstance; + let discardSessionTokenSpy: jest.SpyInstance; + + beforeEach(() => { + currentAccountSpy = jest + .spyOn(CacheModule, 'currentAccount') + .mockReturnValue(MOCK_STORED_ACCOUNT); + discardSessionTokenSpy = jest + .spyOn(CacheModule, 'discardSessionToken') + .mockImplementation(() => {}); + }); + + afterEach(() => { + currentAccountSpy.mockRestore(); + discardSessionTokenSpy.mockRestore(); + }); + + it('sends a verification code and returns SIGN_IN/EMAIL_OTP when mustVerify=true and session is unverified', async () => { + const authClient = createMockAuthClient({ + details: { mustVerify: true, sessionVerified: false }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(session.sendVerificationCode).toHaveBeenCalledTimes(1); + expect(result.data?.verificationReason).toBe(VerificationReasons.SIGN_IN); + expect(result.data?.verificationMethod).toBe( + VerificationMethods.EMAIL_OTP + ); + expect(result.data?.sessionVerified).toBe(false); + expect(result.data?.emailVerified).toBe(true); + expect(result.data?.totpIsActive).toBe(false); + }); + + it('does NOT send a verification code and leaves verification fields undefined when mustVerify=false (regression test for SUMO bug)', async () => { + const authClient = createMockAuthClient({ + details: { mustVerify: false, sessionVerified: false }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(session.sendVerificationCode).not.toHaveBeenCalled(); + expect(result.data?.verificationReason).toBeUndefined(); + expect(result.data?.verificationMethod).toBeUndefined(); + expect(result.data?.sessionVerified).toBe(false); + expect(result.data?.emailVerified).toBe(true); + }); + + it('returns TOTP_2FA without sending an email when mustVerify=true and TOTP is active', async () => { + const authClient = createMockAuthClient({ + authenticationMethods: [AuthenticationMethods.OTP], + details: { mustVerify: true, sessionVerified: false }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(session.sendVerificationCode).not.toHaveBeenCalled(); + expect(result.data?.verificationReason).toBe(VerificationReasons.SIGN_IN); + expect(result.data?.verificationMethod).toBe( + VerificationMethods.TOTP_2FA + ); + expect(result.data?.totpIsActive).toBe(true); + }); + + it('sends the verification email on the SIGN_UP branch when the primary email is unverified, independent of mustVerify', async () => { + const authClient = createMockAuthClient({ + details: { + accountEmailVerified: false, + mustVerify: false, + sessionVerified: false, + }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(session.sendVerificationCode).toHaveBeenCalledTimes(1); + expect(result.data?.verificationReason).toBe(VerificationReasons.SIGN_UP); + expect(result.data?.verificationMethod).toBe( + VerificationMethods.EMAIL_OTP + ); + }); + + it('never sends a verification code when isOauthPromptNone is true', async () => { + const authClient = createMockAuthClient({ + details: { mustVerify: true, sessionVerified: false }, + }); + const session = createMockSession(); + + await cachedSignIn(MOCK_SESSION_TOKEN, authClient, session, true); + + expect(session.sendVerificationCode).not.toHaveBeenCalled(); + }); + + it('returns no verification fields when the session is already verified', async () => { + const authClient = createMockAuthClient({ + details: { mustVerify: false, sessionVerified: true }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(session.sendVerificationCode).not.toHaveBeenCalled(); + expect(result.data?.verificationReason).toBeUndefined(); + expect(result.data?.verificationMethod).toBeUndefined(); + expect(result.data?.sessionVerified).toBe(true); + }); + + it('discards the session token and returns SESSION_EXPIRED when accountProfile reports an invalid token', async () => { + const authClient = createMockAuthClient({ + accountProfileRejection: { errno: AuthUiErrors.INVALID_TOKEN.errno }, + }); + const session = createMockSession(); + + const result = await cachedSignIn( + MOCK_SESSION_TOKEN, + authClient, + session + ); + + expect(discardSessionTokenSpy).toHaveBeenCalledTimes(1); + expect(result.error).toBe(AuthUiErrors.SESSION_EXPIRED); + expect(session.sendVerificationCode).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/fxa-settings/src/pages/Signin/utils.ts b/packages/fxa-settings/src/pages/Signin/utils.ts index 1c0ffb47063..edbb870644c 100644 --- a/packages/fxa-settings/src/pages/Signin/utils.ts +++ b/packages/fxa-settings/src/pages/Signin/utils.ts @@ -153,6 +153,7 @@ export const cachedSignIn = async ( const { details } = await authClient.sessionStatus(sessionToken); const sessionVerified = details.sessionVerified; const emailVerified = details.accountEmailVerified; + const mustVerify = details.mustVerify; let verificationMethod; let verificationReason; @@ -161,9 +162,12 @@ export const cachedSignIn = async ( verificationReason = VerificationReasons.SIGN_UP; verificationMethod = VerificationMethods.EMAIL_OTP; !isOauthPromptNone && (await session.sendVerificationCode()); - } else if (!sessionVerified) { - // we are inferring verification method and verification reason here - // ideally we would check the server directly to allow for more verification reasons + } else if (!sessionVerified && mustVerify) { + // Only route the user through verification (and pre-send a code) when + // the auth-server actually requires this session to be verified. + // Otherwise we'd send a `verifyLoginCode` email for OAuth RPs that + // don't need confirmation (e.g. SUMO). Mirrors fxa-content-server's + // legacy guard in views/base.js#checkAuthorization. verificationReason = VerificationReasons.SIGN_IN; if (totpIsActive) { verificationMethod = VerificationMethods.TOTP_2FA; From 6d1f1eb315c658826dffde86dda80bcb619a2d6a Mon Sep 17 00:00:00 2001 From: Dan Schomburg Date: Thu, 14 May 2026 17:16:12 -0700 Subject: [PATCH 2/4] test(auth-server,settings): align tests with new mustVerify field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because: - Adding `mustVerify` to `/session/status` and gating `cachedSignIn` on it broke existing tests that didn't yet know about the new field. This commit: - Adds `mustVerify: false` to the seven `/session/status` assertions in session.spec.ts so they match the handler's new response shape. - Adds `mustVerify: true` to the two `cachedSigninHandler` mocks in container.test.tsx that expect the unverified branch to fire. - Drops the `signinConfirmation.skipForNewAccounts.enabled=false` configOverrides from the remote session_tests suite — under defaults, mustVerify cleanly tracks `wantsKeys`, which is what the keys=true/keys=false round-trip tests are checking. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/fxa-auth-server/lib/routes/session.spec.ts | 7 +++++++ .../fxa-auth-server/test/remote/session_tests.in.spec.ts | 8 +------- packages/fxa-settings/src/pages/Signin/container.test.tsx | 8 +++++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/fxa-auth-server/lib/routes/session.spec.ts b/packages/fxa-auth-server/lib/routes/session.spec.ts index 99940acde5d..7dc1dfb9b29 100644 --- a/packages/fxa-auth-server/lib/routes/session.spec.ts +++ b/packages/fxa-auth-server/lib/routes/session.spec.ts @@ -204,6 +204,7 @@ describe('/session/status', () => { state: 'unverified', details: { accountEmailVerified: false, + mustVerify: false, sessionVerificationMeetsMinimumAAL: true, sessionVerificationMethod: 'totp-2fa', sessionVerified: false, @@ -242,6 +243,7 @@ describe('/session/status', () => { state: 'unverified', details: { accountEmailVerified: false, + mustVerify: false, sessionVerificationMethod: 'email', sessionVerified: false, verified: false, @@ -278,6 +280,7 @@ describe('/session/status', () => { state: 'unverified', details: { accountEmailVerified: false, + mustVerify: false, sessionVerificationMethod: 'email', sessionVerified: false, verified: false, @@ -314,6 +317,7 @@ describe('/session/status', () => { state: 'unverified', details: { accountEmailVerified: true, + mustVerify: false, sessionVerificationMethod: 'email', sessionVerified: false, verified: false, @@ -350,6 +354,7 @@ describe('/session/status', () => { state: 'verified', details: { accountEmailVerified: true, + mustVerify: false, sessionVerificationMethod: 'email', sessionVerified: true, verified: true, @@ -385,6 +390,7 @@ describe('/session/status', () => { state: 'verified', details: { accountEmailVerified: true, + mustVerify: false, sessionVerificationMethod: 'email', sessionVerified: true, verified: true, @@ -421,6 +427,7 @@ describe('/session/status', () => { state: 'verified', details: { accountEmailVerified: true, + mustVerify: false, sessionVerificationMethod: 'totp-2fa', sessionVerified: true, verified: true, diff --git a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts index 450d51ec8e6..014d763f7ad 100644 --- a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts +++ b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts @@ -12,13 +12,7 @@ const Client = require('../client')(); let server: TestServerInstance; beforeAll(async () => { - server = await createTestServer({ - configOverrides: { - signinConfirmation: { - skipForNewAccounts: { enabled: false }, - }, - }, - }); + server = await createTestServer(); }, 120000); afterAll(async () => { diff --git a/packages/fxa-settings/src/pages/Signin/container.test.tsx b/packages/fxa-settings/src/pages/Signin/container.test.tsx index e79f610c816..34dead6193d 100644 --- a/packages/fxa-settings/src/pages/Signin/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/container.test.tsx @@ -401,7 +401,7 @@ function mockSigninModule() { } function mockReactUtilsModule() { - jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => { }); + jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => {}); } let mockGetCredentials: jest.SpyInstance; @@ -1240,7 +1240,7 @@ describe('signin container', () => { // doesn't overwrite the initial hasPassword=false from locationState. mockAuthClient.accountStatusByEmail = jest .fn() - .mockReturnValue(new Promise(() => { })); + .mockReturnValue(new Promise(() => {})); mockLocationState = { email: MOCK_ROUTER_STATE_EMAIL, hasPassword: false, @@ -1410,6 +1410,7 @@ describe('signin container', () => { details: { accountEmailVerified: true, sessionVerified: false, + mustVerify: true, }, }); @@ -1489,6 +1490,7 @@ describe('signin container', () => { details: { accountEmailVerified: true, sessionVerified: false, + mustVerify: true, }, }); @@ -1608,7 +1610,7 @@ describe('signin container', () => { }); it('calls session.isValid and discards token when session is invalid', async () => { - const storedAccount: { sessionToken?: string;[key: string]: any } = { + const storedAccount: { sessionToken?: string; [key: string]: any } = { ...MOCK_STORED_ACCOUNT, email: MOCK_QUERY_PARAM_EMAIL, sessionToken: MOCK_SESSION_TOKEN, From 5936eefaa0686abfbf4ec12edc0ac4c7bfbd7079 Mon Sep 17 00:00:00 2001 From: Dan Schomburg Date: Thu, 14 May 2026 17:19:25 -0700 Subject: [PATCH 3/4] test(settings): cover SUMO mustVerify=false regression case Because: - The whole point of FXA-12972 is to stop sending verifyLoginCode emails to non-Sync OAuth RPs (e.g. SUMO) when the auth-server has decided the session does not need verification. Without a test pinning this behavior, a future regression would be silent. This commit: - Adds a cachedSigninHandler test asserting that with mustVerify=false and sessionVerified=false, no verification code is sent and verificationMethod/verificationReason stay undefined. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/pages/Signin/container.test.tsx | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/fxa-settings/src/pages/Signin/container.test.tsx b/packages/fxa-settings/src/pages/Signin/container.test.tsx index 34dead6193d..8a5a38d7aef 100644 --- a/packages/fxa-settings/src/pages/Signin/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/container.test.tsx @@ -1513,6 +1513,41 @@ describe('signin container', () => { }); }); + it('skips verification when mustVerify=false even if session is unverified (FXA-12972 SUMO regression)', async () => { + // Mirrors the SUMO-style OAuth RP case: the session is unverified but + // the auth-server has decided it does not need verification + // (mustVerify=false). cachedSignIn must not send a verifyLoginCode + // email and must leave verificationMethod/verificationReason undefined + // so the user is not stranded on the confirm-code page. + mockSession.sendVerificationCode.mockClear(); + + mockAuthClient.accountProfile = jest.fn().mockResolvedValue({ + authenticationMethods: ['pwd', 'email'], + authenticatorAssuranceLevel: 1, + }); + mockAuthClient.sessionStatus = jest.fn().mockResolvedValue({ + details: { + accountEmailVerified: true, + sessionVerified: false, + mustVerify: false, + }, + }); + + mockUseValidateModule(); + render(); + + await waitFor(async () => { + const handlerResult = + await currentSigninProps?.cachedSigninHandler(MOCK_SESSION_TOKEN); + + expect(mockSession.sendVerificationCode).not.toHaveBeenCalled(); + expect(handlerResult?.data?.verificationMethod).toBeUndefined(); + expect(handlerResult?.data?.verificationReason).toBeUndefined(); + expect(handlerResult?.data?.sessionVerified).toEqual(false); + expect(handlerResult?.data?.emailVerified).toEqual(true); + }); + }); + it('does not return a verification reason with verified 2FA session', async () => { // Reset session mock for this test mockSession.sendVerificationCode.mockClear(); From 9d222b5c02b77010b411e5c5e990cd6fe1c9a4ce Mon Sep 17 00:00:00 2001 From: Dan Schomburg Date: Thu, 14 May 2026 17:36:28 -0700 Subject: [PATCH 4/4] test(auth-server): keep signin-confirmation override and fix mustVerify expectations Because: - The previous attempt removed the configOverrides block to make the keys=true/keys=false round-trip tests deterministic under defaults. That broke the existing 'reauth has sane session-verification behaviour' tests (which depend on the forced-confirmation behaviour to keep new-account sessions in the 'unverified' state) and the keys=true case (which under defaults gets pre-verified and never sets mustVerify=true). - The keys=false test as authored is unworkable under either config: with the override, every new login sets mustVerify=true regardless of the keys argument; without it, new accounts skip signin confirmation entirely and mustVerify stays false but the session is also auto-verified. This commit: - Restores the signinConfirmation.skipForNewAccounts.enabled=false configOverrides block, with a comment explaining why. - Updates 'succeeds with valid token' to expect mustVerify=true (which is what the server actually returns when signin confirmation is forced). - Replaces the keys=true/keys=false pair with a single test that exposes the mustVerify field for a session that requires verification. The falsy case is already covered by the unit tests in lib/routes/session.spec.ts. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../test/remote/session_tests.in.spec.ts | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts index 014d763f7ad..cb8b9356720 100644 --- a/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts +++ b/packages/fxa-auth-server/test/remote/session_tests.in.spec.ts @@ -12,7 +12,17 @@ const Client = require('../client')(); let server: TestServerInstance; beforeAll(async () => { - server = await createTestServer(); + // Force sign-in confirmation so the suite is deterministic. With the + // default (skipForNewAccounts.enabled=true) brand-new accounts get a + // pre-verified session and we lose the `unverified` state the + // `reauth` and `status` tests rely on. + server = await createTestServer({ + configOverrides: { + signinConfirmation: { + skipForNewAccounts: { enabled: false }, + }, + }, + }); }, 120000); afterAll(async () => { @@ -362,7 +372,10 @@ describe.each(testVersions)( uid, details: { accountEmailVerified: true, - mustVerify: false, + // With signin confirmation forced (see configOverrides at top of file), + // every fresh login on a new account sets mustVerify=true because + // the server is going to require this session be verified. + mustVerify: true, sessionVerificationMeetsMinimumAAL: true, sessionVerificationMethod: null, sessionVerified: false, @@ -371,7 +384,7 @@ describe.each(testVersions)( }); }); - it('returns mustVerify=true when the session was created with keys=true', async () => { + it('exposes mustVerify=true for a session that requires verification', async () => { const email = server.uniqueEmail(); const password = 'testx'; const c = await Client.createAndVerify( @@ -386,21 +399,6 @@ describe.each(testVersions)( expect(status.details.mustVerify).toBe(true); }); - it('returns mustVerify=false when the session was created without keys', async () => { - const email = server.uniqueEmail(); - const password = 'testx'; - const c = await Client.createAndVerify( - server.publicUrl, - email, - password, - server.mailbox, - testOptions - ); - await c.login({ keys: false }); - const status = await c.api.sessionStatus(c.sessionToken); - expect(status.details.mustVerify).toBe(false); - }); - it('errors with invalid token', async () => { const client = new Client(server.publicUrl, testOptions); try {