diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts index 6422784dee4..6faa33ebbaa 100644 --- a/packages/fxa-auth-server/config/index.ts +++ b/packages/fxa-auth-server/config/index.ts @@ -1687,14 +1687,8 @@ const convictConf = convict({ default: /.+@mozilla\.com$/, env: 'SIGNIN_CONFIRMATION_FORCE_EMAIL_REGEX', }, - skipForEmailAddresses: { - doc: 'Comma separated list of email addresses that will always skip any non TOTP sign-in confirmation', - format: Array, - default: [], - env: 'SIGNIN_CONFIRMATION_SKIP_FOR_EMAIL_ADDRESS', - }, skipForEmailRegex: { - doc: 'Regex pattern for email addresses that will always skip any non-TOTP sign-in confirmation. Checked in addition to skipForEmailAddresses.', + doc: 'Regex pattern for email addresses that will always skip any non-TOTP sign-in confirmation.', format: RegExp, default: /^$/, env: 'SIGNIN_CONFIRMATION_SKIP_FOR_EMAIL_REGEX', diff --git a/packages/fxa-auth-server/lib/routes/account.spec.ts b/packages/fxa-auth-server/lib/routes/account.spec.ts index 0e47d43283c..c6f5560c53d 100644 --- a/packages/fxa-auth-server/lib/routes/account.spec.ts +++ b/packages/fxa-auth-server/lib/routes/account.spec.ts @@ -28,16 +28,12 @@ const { const { AppStoreSubscriptions, } = require('../payments/iap/apple-app-store/subscriptions'); -const { - deleteAccountIfUnverified, -} = require('./utils/account'); +const { deleteAccountIfUnverified } = require('./utils/account'); const { AppConfig, AuthLogger } = require('../types'); const defaultConfig = require('../../config').default.getProperties(); const { ProfileClient } = require('@fxa/profile/client'); const { RelyingPartyConfigurationManager } = require('@fxa/shared/cms'); -const { - OAuthClientInfoServiceName, -} = require('../senders/oauth_client_info'); +const { OAuthClientInfoServiceName } = require('../senders/oauth_client_info'); const { FxaMailer } = require('../senders/fxa-mailer'); const { RecoveryPhoneService } = require('@fxa/accounts/recovery-phone'); @@ -185,7 +181,16 @@ const makeRoutes = function (options: any = {}, requireMocks: any = {}) { const signinUtils = options.signinUtils || - require('./utils/signin')(log, config, customs, db, mailer, cadReminders, glean, statsd); + require('./utils/signin')( + log, + config, + customs, + db, + mailer, + cadReminders, + glean, + statsd + ); if (options.checkPassword) { signinUtils.checkPassword = options.checkPassword; } @@ -205,7 +210,9 @@ const makeRoutes = function (options: any = {}, requireMocks: any = {}) { verificationReminders, glean ); - const pushbox = options.pushbox || { deleteAccount: jest.fn().mockResolvedValue(undefined) }; + const pushbox = options.pushbox || { + deleteAccount: jest.fn().mockResolvedValue(undefined), + }; const oauthDb = { removeTokensAndCodes: () => {}, removePublicAndCanGrantTokens: () => {}, @@ -395,9 +402,7 @@ describe('/account/reset', () => { }); it('called mailer.sendPasswordResetAccountRecoveryEmail correctly', () => { - expect( - fxaMailer.sendPasswordResetAccountRecoveryEmail.callCount - ).toBe(1); + expect(fxaMailer.sendPasswordResetAccountRecoveryEmail.callCount).toBe(1); const args = fxaMailer.sendPasswordResetAccountRecoveryEmail.args[0]; expect(args[0].to).toBe(TEST_EMAIL); }); @@ -782,7 +787,11 @@ describe('/account/create', () => { glean.registration.confirmationEmailSent.reset(); }); - function setup(extraConfig?: any, mockRequestOptsCb?: any, makeRoutesOptions: any = {}) { + function setup( + extraConfig?: any, + mockRequestOptsCb?: any, + makeRoutesOptions: any = {} + ) { const config = { securityHistory: { enabled: true, @@ -2302,10 +2311,7 @@ describe('/account/login', () => { beforeEach(() => { Container.set(AppConfig, config); Container.set(AuthLogger, mockLog); - Container.set( - AccountEventsManager, - new AccountEventsManager() - ); + Container.set(AccountEventsManager, new AccountEventsManager()); Container.set(CapabilityService, jest.fn().mockResolvedValue(undefined)); Container.set(OAuthClientInfoServiceName, mockOAuthClientInfo); Container.set(FxaMailer, mockFxaMailer); @@ -2614,9 +2620,9 @@ describe('/account/login', () => { expect(mockFxaMailer.sendVerifyLoginEmail.callCount).toBe(1); expect(mockMetricsContext.setFlowCompleteSignal.callCount).toBe(1); - expect( - mockMetricsContext.setFlowCompleteSignal.args[0][0] - ).toEqual('account.confirmed'); + expect(mockMetricsContext.setFlowCompleteSignal.args[0][0]).toEqual( + 'account.confirmed' + ); expect(response.verified).toBeFalsy(); expect(response.verificationMethod).toBe('email'); @@ -2869,7 +2875,11 @@ describe('/account/login', () => { }); describe('skip for new accounts', () => { - function setupSkipNewAccounts(enabled: any, accountCreatedSince: any, makeRoutesOptions: any = {}) { + function setupSkipNewAccounts( + enabled: any, + accountCreatedSince: any, + makeRoutesOptions: any = {} + ) { config.signinConfirmation.skipForNewAccounts = { enabled: enabled, maxAge: 5, @@ -2938,9 +2948,7 @@ describe('/account/login', () => { expect(sendVerifyLoginEmailArgs.location.country).toBe( 'United States' ); - expect(sendVerifyLoginEmailArgs.timeZone).toBe( - 'America/Los_Angeles' - ); + expect(sendVerifyLoginEmailArgs.timeZone).toBe('America/Los_Angeles'); }); }); @@ -2990,18 +2998,18 @@ describe('/account/login', () => { expect( mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].deviceId ).toBe(mockRequest.payload.metricsContext.deviceId); - expect( - mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].flowId - ).toBe(mockRequest.payload.metricsContext.flowId); + expect(mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].flowId).toBe( + mockRequest.payload.metricsContext.flowId + ); expect( mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].flowBeginTime ).toBe(mockRequest.payload.metricsContext.flowBeginTime); - expect( - mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].sync - ).toBe(true); - expect( - mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].uid - ).toBe(uid); + expect(mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].sync).toBe( + true + ); + expect(mockFxaMailer.sendNewDeviceLoginEmail.args[0][0].uid).toBe( + uid + ); expect(response.emailVerified).toBeTruthy(); }); }); @@ -3072,86 +3080,14 @@ describe('/account/login', () => { ); }); }); - }); - - describe('skip for emails', () => { - function setupSkipForEmails(email: string) { - config.securityHistory.ipProfiling.allowedRecency = 0; - config.signinConfirmation.skipForNewAccounts = { enabled: false }; - config.signinConfirmation.skipForEmailAddresses = [ - 'skip@confirmation.com', - 'other@email.com', - ]; - - // Reset the spy to avoid leaking state between tests - mockDB.verifiedLoginSecurityEvents = sinon.spy(() => - Promise.resolve([]) - ); - - mockRequest.payload.email = email; - - mockDB.accountRecord = () => { - return Promise.resolve({ - authSalt: hexString(32), - data: hexString(32), - email, - emailVerified: true, - primaryEmail: { - normalizedEmail: normalizeEmail(email), - email, - isVerified: true, - isPrimary: true, - }, - kA: hexString(32), - lastAuthAt: () => Date.now(), - uid, - wrapWrapKb: hexString(32), - }); - }; - - const innerAccountRoutes = makeRoutes({ - checkPassword: () => Promise.resolve(true), - config, - customs: mockCustoms, - db: mockDB, - log: mockLog, - mailer: mockMailer, - push: mockPush, - }); - - route = getRoute(innerAccountRoutes, '/account/login'); - } afterEach(() => { - // Restore config to default to avoid leaking into subsequent tests + config.signinConfirmation.skipForNewAccounts = undefined; config.securityHistory.ipProfiling.allowedRecency = defaultConfig.securityHistory.ipProfiling.allowedRecency; - }); - - it('should not skip sign-in confirmation for specified email', () => { - setupSkipForEmails('not@skip.com'); - - return runTest(route, mockRequest, (response: any) => { - expect(mockDB.createSessionToken.callCount).toBe(1); - const tokenData = mockDB.createSessionToken.getCall(0).args[0]; - expect(tokenData.tokenVerificationId).toBeTruthy(); - expect(mockFxaMailer.sendVerifyLoginEmail.callCount).toBe(1); - expect(mockFxaMailer.sendNewDeviceLoginEmail.callCount).toBe(0); - expect(response.verified).toBeFalsy(); - }); - }); - - it('should skip sign-in confirmation for specified email', () => { - setupSkipForEmails('skip@confirmation.com'); - - return runTest(route, mockRequest, (response: any) => { - expect(mockDB.createSessionToken.callCount).toBe(1); - const tokenData = mockDB.createSessionToken.getCall(0).args[0]; - expect(tokenData.tokenVerificationId).toBeFalsy(); - expect(mockMailer.sendVerifyLoginEmail.callCount).toBe(0); - expect(mockFxaMailer.sendNewDeviceLoginEmail.callCount).toBe(1); - expect(response.emailVerified).toBeTruthy(); - }); + mockDB.verifiedLoginSecurityEvents = sinon.spy(() => + Promise.resolve([]) + ); }); }); @@ -3176,7 +3112,6 @@ describe('/account/login', () => { function setupSkipForEmailRegex(email: string, regex: RegExp) { config.securityHistory.ipProfiling.allowedRecency = 0; config.signinConfirmation.skipForNewAccounts = { enabled: false }; - config.signinConfirmation.skipForEmailAddresses = []; config.signinConfirmation.skipForEmailRegex = regex; mockDB.verifiedLoginSecurityEvents = sinon.spy(() => @@ -3216,11 +3151,18 @@ describe('/account/login', () => { route = getRoute(innerAccountRoutes, '/account/login'); } - + beforeEach(() => { + // one test is checking the statsd, and this is included in it. + // We set it here, and reset in the afterEach to avoid having the + // config state leak to other tests if these fail + mockRequest.app.clientIdTag = 'test-client-id'; + statsd.increment.resetHistory(); + }); afterEach(() => { config.securityHistory.ipProfiling.allowedRecency = defaultConfig.securityHistory.ipProfiling.allowedRecency; config.signinConfirmation.skipForEmailRegex = /^$/; + mockRequest.app.clientIdTag = undefined; }); it('should skip sign-in confirmation for email matching regex', () => { @@ -3244,6 +3186,19 @@ describe('/account/login', () => { expect(response.verified).toBeFalsy(); }); }); + + it('should increment statsd metric for emailAlways', () => { + setupSkipForEmailRegex('qa-test@example.com', /.+@example\.com$/); + + return runTest(route, mockRequest, () => { + sinon.assert.calledWith( + statsd.increment, + 'account.signin.confirm.bypass.emailAlways', + { clientId: 'test-client-id' } + ); + mockRequest.app.clientIdTag = undefined; + }); + }); }); describe('skip for known device', () => { @@ -3386,17 +3341,21 @@ describe('/account/login', () => { }, }; - return runTest(route, requestWithDifferentUserAgent, (response: any) => { - expect(mockDB.createSessionToken.callCount).toBe(1); - const tokenData = mockDB.createSessionToken.getCall(0).args[0]; - expect(tokenData.mustVerify).toBeTruthy(); - expect(response.verified).toBeFalsy(); - - sinon.assert.calledWith( - statsd.increment, - 'account.signin.confirm.device.notfound' - ); - }); + return runTest( + route, + requestWithDifferentUserAgent, + (response: any) => { + expect(mockDB.createSessionToken.callCount).toBe(1); + const tokenData = mockDB.createSessionToken.getCall(0).args[0]; + expect(tokenData.mustVerify).toBeTruthy(); + expect(response.verified).toBeFalsy(); + + sinon.assert.calledWith( + statsd.increment, + 'account.signin.confirm.device.notfound' + ); + } + ); }); it('should not skip verification when in report-only mode', () => { @@ -3635,7 +3594,9 @@ describe('/account/login', () => { it('invalid code', async () => { mockDB.consumeUnblockCode = () => Promise.reject(error.invalidUnblockCode()); - await expect(runTest(route, mockRequestWithUnblockCode)).rejects.toMatchObject({ + await expect( + runTest(route, mockRequestWithUnblockCode) + ).rejects.toMatchObject({ errno: error.ERRNO.INVALID_UNBLOCK_CODE, output: { statusCode: 400 }, }); @@ -3653,7 +3614,9 @@ describe('/account/login', () => { createdAt: Date.now() - (config.signinUnblock.codeLifetime + 5000), }); - await expect(runTest(route, mockRequestWithUnblockCode)).rejects.toMatchObject({ + await expect( + runTest(route, mockRequestWithUnblockCode) + ).rejects.toMatchObject({ errno: error.ERRNO.INVALID_UNBLOCK_CODE, output: { statusCode: 400 }, }); @@ -3668,7 +3631,9 @@ describe('/account/login', () => { it('unknown account', async () => { mockDB.accountRecord = () => Promise.reject(error.unknownAccount()); mockDB.emailRecord = () => Promise.reject(error.unknownAccount()); - await expect(runTest(route, mockRequestWithUnblockCode)).rejects.toMatchObject({ + await expect( + runTest(route, mockRequestWithUnblockCode) + ).rejects.toMatchObject({ errno: error.ERRNO.REQUEST_BLOCKED, output: { statusCode: 400 }, }); @@ -3685,12 +3650,8 @@ describe('/account/login', () => { expect(mockLog.flowEvent.args[1][0].event).toBe( 'account.login.confirmedUnblockCode' ); - expect(mockLog.flowEvent.args[2][0].event).toBe( - 'account.login' - ); - expect(mockLog.flowEvent.args[3][0].event).toBe( - 'flow.complete' - ); + expect(mockLog.flowEvent.args[2][0].event).toBe('account.login'); + expect(mockLog.flowEvent.args[3][0].event).toBe('flow.complete'); }); }); }); @@ -3746,7 +3707,9 @@ describe('/account/login', () => { }); }); return runTest(route, mockRequest).then( - () => { throw new Error('should have thrown'); }, + () => { + throw new Error('should have thrown'); + }, (err: any) => { expect(mockDB.accountRecord.callCount).toBe(1); expect(err.errno).toBe(142); @@ -3763,7 +3726,9 @@ describe('/account/login', () => { }); mockRequest.payload.verificationMethod = 'totp-2fa'; return runTest(route, mockRequest).then( - () => { throw new Error('should have thrown'); }, + () => { + throw new Error('should have thrown'); + }, (err: any) => { expect(mockDB.totpToken.callCount).toBe(1); expect(err.errno).toBe(160); @@ -3839,8 +3804,12 @@ describe('/account/login', () => { expect(emailMessage.cmsRpFromName).toBe('Testo Inc.'); expect(emailMessage.entrypoint).toBe('testo'); expect(emailMessage.logoUrl).toBe('http://img.exmpl.gg/logo.svg'); - expect(emailMessage.subject).toBe(rpCmsConfig.NewDeviceLoginEmail.subject); - expect(emailMessage.headline).toBe(rpCmsConfig.NewDeviceLoginEmail.headline); + expect(emailMessage.subject).toBe( + rpCmsConfig.NewDeviceLoginEmail.subject + ); + expect(emailMessage.headline).toBe( + rpCmsConfig.NewDeviceLoginEmail.headline + ); expect(emailMessage.description).toBe( rpCmsConfig.NewDeviceLoginEmail.description ); @@ -3905,7 +3874,9 @@ describe('/account/keys', () => { mockRequest.auth.credentials.tokenVerified = false; return runTest(route, mockRequest) .then( - () => { throw new Error('should have thrown'); }, + () => { + throw new Error('should have thrown'); + }, (response: any) => { expect(response.errno).toBe(104); expect(response.message).toBe('Unconfirmed account'); @@ -3922,7 +3893,12 @@ describe('/account/destroy', () => { const tokenVerified = true; const uid = uuid.v4({}, Buffer.alloc(16)).toString('hex'); - let mockDB: any, mockLog: any, mockRequest: any, mockPush: any, mockPushbox: any, mockCustoms: any; + let mockDB: any, + mockLog: any, + mockRequest: any, + mockPush: any, + mockPushbox: any, + mockCustoms: any; beforeEach(async () => { mockDB = { @@ -3990,9 +3966,13 @@ describe('/account/destroy', () => { customerId: 'customer123', reason: ReasonForDeletion.UserRequested, }); - sinon.assert.calledOnceWithExactly(glean.account.deleteComplete, mockRequest, { - uid, - }); + sinon.assert.calledOnceWithExactly( + glean.account.deleteComplete, + mockRequest, + { + uid, + } + ); sinon.assert.calledOnceWithExactly( mockLog.info, 'accountDeleted.ByRequest', @@ -4005,7 +3985,9 @@ describe('/account/destroy', () => { const route = buildRoute(); // Here we act like there's an error when calling accountDeleteManager.quickDelete(...) - mockAccountQuickDelete = jest.fn().mockRejectedValue(new Error('quickDelete failed')); + mockAccountQuickDelete = jest + .fn() + .mockRejectedValue(new Error('quickDelete failed')); return runTest(route, mockRequest, () => { sinon.assert.calledOnceWithExactly(mockDB.accountRecord, email); @@ -4014,9 +3996,13 @@ describe('/account/destroy', () => { customerId: 'customer123', reason: ReasonForDeletion.UserRequested, }); - sinon.assert.calledOnceWithExactly(glean.account.deleteComplete, mockRequest, { - uid, - }); + sinon.assert.calledOnceWithExactly( + glean.account.deleteComplete, + mockRequest, + { + uid, + } + ); }); }); @@ -4043,9 +4029,13 @@ describe('/account/destroy', () => { customerId: 'customer123', reason: ReasonForDeletion.UserRequested, }); - sinon.assert.calledOnceWithExactly(glean.account.deleteComplete, mockRequest, { - uid, - }); + sinon.assert.calledOnceWithExactly( + glean.account.deleteComplete, + mockRequest, + { + uid, + } + ); }); }); @@ -4149,7 +4139,9 @@ describe('/account', () => { mockStripeHelper.subscriptionsToResponse = sinon.spy( async (subscriptions: any) => mockWebSubscriptionsResponse ); - mockStripeHelper.removeFirestoreCustomer = jest.fn().mockResolvedValue(undefined); + mockStripeHelper.removeFirestoreCustomer = jest + .fn() + .mockResolvedValue(undefined); Container.set(CapabilityService, jest.fn()); }); @@ -4343,7 +4335,9 @@ describe('/account', () => { }); it('should return an empty list when no active Google Play or web subscriptions are found', () => { - mockPlaySubscriptions.getSubscriptions = sinon.spy(async (uid: any) => []); + mockPlaySubscriptions.getSubscriptions = sinon.spy( + async (uid: any) => [] + ); return runTest( buildRoute(subscriptionsEnabled, playSubscriptionsEnabled), @@ -4434,9 +4428,9 @@ describe('/account', () => { 'getSubscriptions', ]); Container.set(AppStoreSubscriptions, mockAppStoreSubscriptions); - mockAppStoreSubscriptions.getSubscriptions = sinon.spy(async (uid: any) => [ - mockAppendedAppStoreSubscriptionPurchase, - ]); + mockAppStoreSubscriptions.getSubscriptions = sinon.spy( + async (uid: any) => [mockAppendedAppStoreSubscriptionPurchase] + ); }); it('should return formatted Apple App Store subscriptions when App Store subscriptions are enabled', () => { @@ -4487,7 +4481,9 @@ describe('/account', () => { }); it('should return an empty list when no active Apple App Store or web subscriptions are found', () => { - mockAppStoreSubscriptions.getSubscriptions = sinon.spy(async (uid: any) => []); + mockAppStoreSubscriptions.getSubscriptions = sinon.spy( + async (uid: any) => [] + ); return runTest( buildRoute(subscriptionsEnabled, false, appStoreSubscriptionsEnabled), @@ -4531,15 +4527,27 @@ describe('/account', () => { describe('expanded account data fields', () => { it('should return account metadata and 2FA status', () => { return runTest(buildRoute(), request, (result: any) => { - expect(Object.prototype.hasOwnProperty.call(result, 'createdAt')).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'createdAt') + ).toBeTruthy(); expect( Object.prototype.hasOwnProperty.call(result, 'passwordCreatedAt') ).toBeTruthy(); - expect(Object.prototype.hasOwnProperty.call(result, 'hasPassword')).toBeTruthy(); - expect(Object.prototype.hasOwnProperty.call(result, 'emails')).toBeTruthy(); - expect(Object.prototype.hasOwnProperty.call(result, 'totp')).toBeTruthy(); - expect(Object.prototype.hasOwnProperty.call(result, 'backupCodes')).toBeTruthy(); - expect(Object.prototype.hasOwnProperty.call(result, 'recoveryKey')).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'hasPassword') + ).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'emails') + ).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'totp') + ).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'backupCodes') + ).toBeTruthy(); + expect( + Object.prototype.hasOwnProperty.call(result, 'recoveryKey') + ).toBeTruthy(); expect( Object.prototype.hasOwnProperty.call(result, 'recoveryPhone') ).toBeTruthy(); @@ -4593,7 +4601,9 @@ describe('/account', () => { allowedRegions: ['US'], }); Container.set(RecoveryPhoneService, { - hasConfirmed: jest.fn().mockResolvedValue({ exists: false, phoneNumber: null }), + hasConfirmed: jest + .fn() + .mockResolvedValue({ exists: false, phoneNumber: null }), available: jest.fn().mockResolvedValue(false), }); return runTest(route, request, (result: any) => { @@ -4607,7 +4617,9 @@ describe('/account', () => { allowedRegions: ['US'], }); Container.set(RecoveryPhoneService, { - hasConfirmed: jest.fn().mockResolvedValue({ exists: false, phoneNumber: null }), + hasConfirmed: jest + .fn() + .mockResolvedValue({ exists: false, phoneNumber: null }), available: jest.fn().mockRejectedValue(new Error('service error')), }); return runTest(route, request, (result: any) => { @@ -4626,7 +4638,9 @@ describe('/account', () => { allowedRegions: ['US'], }); Container.set(RecoveryPhoneService, { - hasConfirmed: jest.fn().mockResolvedValue({ exists: false, phoneNumber: null }), + hasConfirmed: jest + .fn() + .mockResolvedValue({ exists: false, phoneNumber: null }), available: jest.fn().mockResolvedValue(false), }); return runTest(route, noGeoRequest, (result: any) => { diff --git a/packages/fxa-auth-server/lib/routes/account.ts b/packages/fxa-auth-server/lib/routes/account.ts index 5503493ca18..199e9b1cae7 100644 --- a/packages/fxa-auth-server/lib/routes/account.ts +++ b/packages/fxa-auth-server/lib/routes/account.ts @@ -85,7 +85,6 @@ export class AccountHandler { private otpUtils: OtpUtils; private otpOptions: ConfigType['otp']; - private skipConfirmationForEmailAddresses: string[]; private skipConfirmationForEmailRegex: RegExp; private capabilityService: CapabilityService; private accountEventsManager: AccountEventsManager; @@ -115,8 +114,6 @@ export class AccountHandler { private authServerCacheRedis: Redis ) { this.otpUtils = require('./utils/otp').default(db, statsd); - this.skipConfirmationForEmailAddresses = config.signinConfirmation - .skipForEmailAddresses as string[]; this.skipConfirmationForEmailRegex = config.signinConfirmation.skipForEmailRegex; @@ -1262,9 +1259,6 @@ export class AccountHandler { // to guarantee the login experience. const lowerCaseEmail = account.primaryEmail.normalizedEmail.toLowerCase(); const alwaysSkip = - this.skipConfirmationForEmailAddresses?.includes(lowerCaseEmail) || - // use both as a backward compatibility and eventually remove - // the array of emails in favor of just a regex which is more flexible this.skipConfirmationForEmailRegex?.test(lowerCaseEmail); if (alwaysSkip) { this.log.info('account.signin.confirm.bypass.emailAlways', { diff --git a/packages/fxa-auth-server/test/local/routes/account.js b/packages/fxa-auth-server/test/local/routes/account.js index 35435e6aace..cf092b17a55 100644 --- a/packages/fxa-auth-server/test/local/routes/account.js +++ b/packages/fxa-auth-server/test/local/routes/account.js @@ -781,7 +781,6 @@ describe('deleteAccountIfUnverified', () => { const mockConfig = {}; mockConfig.oauth = {}; mockConfig.signinConfirmation = {}; - mockConfig.signinConfirmation.skipForEmailAddresses = []; const emailRecord = { isPrimary: true, isVerified: false, @@ -3780,116 +3779,14 @@ describe('/account/login', () => { ); }); }); - }); - - describe('skip for emails', () => { - function setup(email) { - config.securityHistory.ipProfiling.allowedRecency = 0; - config.signinConfirmation.skipForNewAccounts = { enabled: false }; - config.signinConfirmation.skipForEmailAddresses = [ - 'skip@confirmation.com', - 'other@email.com', - ]; - - // Reset the spy to avoid leaking state between tests - // Previously, "should not skip sign-in confirmation for specified email" test - // was failing intermittently because the spy was not reset between tests. - mockDB.verifiedLoginSecurityEvents = sinon.spy(() => - Promise.resolve([]) - ); - - mockRequest.payload.email = email; - - mockDB.accountRecord = () => { - return Promise.resolve({ - authSalt: hexString(32), - data: hexString(32), - email, - emailVerified: true, - primaryEmail: { - normalizedEmail: normalizeEmail(email), - email, - isVerified: true, - isPrimary: true, - }, - kA: hexString(32), - lastAuthAt: () => Date.now(), - uid, - wrapWrapKb: hexString(32), - }); - }; - - const accountRoutes = makeRoutes({ - checkPassword: () => Promise.resolve(true), - config, - customs: mockCustoms, - db: mockDB, - log: mockLog, - mailer: mockMailer, - push: mockPush, - }); - - route = getRoute(accountRoutes, '/account/login'); - } afterEach(() => { - // Restore config to default to avoid leaking into subsequent tests + config.signinConfirmation.skipForNewAccounts = undefined; config.securityHistory.ipProfiling.allowedRecency = defaultConfig.securityHistory.ipProfiling.allowedRecency; - }); - - it('should not skip sign-in confirmation for specified email', () => { - setup('not@skip.com'); - - return runTest(route, mockRequest, (response) => { - assert.equal( - mockDB.createSessionToken.callCount, - 1, - 'db.createSessionToken was called' - ); - const tokenData = mockDB.createSessionToken.getCall(0).args[0]; - assert.ok( - tokenData.tokenVerificationId, - 'sessionToken was created unverified' - ); - assert.equal( - mockFxaMailer.sendVerifyLoginEmail.callCount, - 1, - 'mailer.sendVerifyLoginEmail was called' - ); - assert.equal(mockFxaMailer.sendNewDeviceLoginEmail.callCount, 0); - assert.ok( - !response.verified, - 'response indicates account is unverified' - ); - }); - }); - - it('should skip sign-in confirmation for specified email', () => { - setup('skip@confirmation.com'); - - return runTest(route, mockRequest, (response) => { - assert.equal( - mockDB.createSessionToken.callCount, - 1, - 'db.createSessionToken was called' - ); - const tokenData = mockDB.createSessionToken.getCall(0).args[0]; - assert.ok( - !tokenData.tokenVerificationId, - 'sessionToken was created verified' - ); - assert.equal( - mockMailer.sendVerifyLoginEmail.callCount, - 0, - 'mailer.sendVerifyLoginEmail was not called' - ); - assert.equal(mockFxaMailer.sendNewDeviceLoginEmail.callCount, 1); - assert.ok( - response.emailVerified, - 'response indicates account is verified' - ); - }); + mockDB.verifiedLoginSecurityEvents = sinon.spy(() => + Promise.resolve([]) + ); }); });