diff --git a/packages/functional-tests/tests/settings/changeEmail.spec.ts b/packages/functional-tests/tests/settings/changeEmail.spec.ts index c7c71cd3345..800ea1d8789 100644 --- a/packages/functional-tests/tests/settings/changeEmail.spec.ts +++ b/packages/functional-tests/tests/settings/changeEmail.spec.ts @@ -216,21 +216,6 @@ test.describe('severity-1 #smoke', () => { await settings.secondaryEmail.deleteButton.click(); await expect(settings.alertBar).toHaveText(/successfully deleted/); - - await settings.secondaryEmail.addButton.click(); - await secondaryEmail.emailTextbox.fill(newEmail); - await secondaryEmail.submit(); - - // skip verification - await settings.goto(); - - await expect(settings.secondaryEmail.unverifiedText).toHaveText( - 'unconfirmed' - ); - - await settings.secondaryEmail.deleteButton.click(); - - await expect(settings.alertBar).toHaveText(/successfully deleted/); }); }); }); @@ -274,7 +259,7 @@ async function setNewPassword( oldPassword: string, newPassword: string, target: BaseTarget, - email: string, + email: string ): Promise { await settings.password.changeButton.click(); diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts index 0b91165fe9c..6c9ce9bfed3 100644 --- a/packages/fxa-auth-server/config/index.ts +++ b/packages/fxa-auth-server/config/index.ts @@ -504,8 +504,7 @@ const convictConf = convict({ subscriptionSupportUrl: { doc: 'url to Mozilla subscription support page', format: String, - default: - 'https://support.mozilla.org/products', + default: 'https://support.mozilla.org/products', }, redirectDomain: { doc: 'Domain that mail urls are allowed to redirect to', @@ -1749,6 +1748,12 @@ const convictConf = convict({ format: 'duration', env: 'SECONDARY_EMAIL_MIN_UNVERIFIED_ACCOUNT_TIME', }, + pendingTtlSeconds: { + doc: 'TTL in seconds for pending secondary email reservations (Redis)', + format: 'nat', + default: 3600, + env: 'SECONDARY_EMAIL_PENDING_TTL_SECONDS', + }, }, signinCodeSize: { doc: 'signinCode size in bytes', diff --git a/packages/fxa-auth-server/lib/error.js b/packages/fxa-auth-server/lib/error.js index 65a4e64802c..a833ef09aa2 100644 --- a/packages/fxa-auth-server/lib/error.js +++ b/packages/fxa-auth-server/lib/error.js @@ -583,8 +583,7 @@ AppError.recoveryCodesAlreadyExist = () => { code: 400, error: 'Bad Request', errno: ERRNO.RECOVERY_CODES_ALREADY_EXISTS, - message: - 'Recovery codes or a verified TOTP token already exist', + message: 'Recovery codes or a verified TOTP token already exist', }); }; @@ -810,6 +809,15 @@ AppError.emailExists = () => { }); }; +AppError.emailInUseByAnotherAccount = () => { + return new AppError({ + code: 400, + error: 'Bad Request', + errno: ERRNO.EMAIL_IN_USE_BY_ANOTHER_ACCOUNT, + message: 'This email is already in use by another account.', + }); +}; + AppError.cannotDeletePrimaryEmail = () => { return new AppError({ code: 400, diff --git a/packages/fxa-auth-server/lib/routes/emails.js b/packages/fxa-auth-server/lib/routes/emails.js index bf3a926f529..8c1d32697b6 100644 --- a/packages/fxa-auth-server/lib/routes/emails.js +++ b/packages/fxa-auth-server/lib/routes/emails.js @@ -90,6 +90,7 @@ module.exports = ( zendeskClient, /** @type import('../payments/stripe').StripeHelper */ stripeHelper, + authServerCacheRedis, statsd ) => { const REMINDER_PATTERN = new RegExp( @@ -98,22 +99,31 @@ module.exports = ( const otpOptions = config.otp; const otpUtils = require('./utils/otp').default(db, statsd); + const SECONDARY_EMAIL_PENDING_TTL = config.secondaryEmail.pendingTtlSeconds; + + function toRedisSecondaryEmailReservationKey(normalizedEmail) { + return `additional_email:reservation:${normalizedEmail}`; + } const handlers = { recoveryEmailPost: async (request) => { log.begin('Account.RecoveryEmailCreate', request); const sessionToken = request.auth.credentials; + + // error early if the account or session are not verified + if (!sessionToken.emailVerified) { + throw error.unverifiedAccount(); + } + + if (sessionToken.tokenVerificationId) { + throw error.unverifiedSession(); + } + const uid = sessionToken.uid; const primaryEmail = sessionToken.email; const { email } = request.payload; - const emailData = { - email: email, - normalizedEmail: normalizeEmail(email), - isVerified: false, - isPrimary: false, - uid: uid, - }; + const normalizedEmail = normalizeEmail(email); await customs.checkAuthenticated( request, @@ -123,83 +133,140 @@ module.exports = ( ); const account = await db.account(uid); - const secondaryEmails = account.emails.filter( - (email) => !email.isPrimary - ); + const secondaryEmails = account.emails.filter((e) => !e.isPrimary); // This is compared against all secondary email - // records, both verified and unverified + // records in db, both verified and unverified if (secondaryEmails.length >= MAX_SECONDARY_EMAILS) { throw error.maxSecondaryEmailsReached(); } - if (emailsMatch(sessionToken.email, email)) { + const normalizedPrimary = normalizeEmail(primaryEmail); + if (normalizedEmail === normalizedPrimary) { + // attempting to add the account's existing primary as secondary throw error.yourPrimaryEmailExists(); } - - if ( - account.emails.map((accountEmail) => accountEmail.email).includes(email) - ) { + const existingSecondary = new Set( + account.emails + .filter((e) => !e.isPrimary) + .map((e) => normalizeEmail(e.email)) + ); + if (existingSecondary.has(normalizedEmail)) { + // attempting to add an email that already exists on this account (as secondary) throw error.alreadyOwnsEmail(); } - if (!sessionToken.emailVerified) { - throw error.unverifiedAccount(); - } + // Verifies if existing holds on the requested email can be released + await clearOtherSecondaryEmailClaims(normalizedEmail); - if (sessionToken.tokenVerificationId) { - throw error.unverifiedSession(); + // Reserve email globally to prevent cross-account races for secondary email setup + // Note: In a race condition between new account creation and secondary email setup, + // the new account will take precedence and be able to claim the email. + const key = toRedisSecondaryEmailReservationKey(normalizedEmail); + const rawRecord = await authServerCacheRedis.get(key); + let parsedRecord; + if (rawRecord) { + try { + parsedRecord = JSON.parse(rawRecord); + } catch (err) { + await authServerCacheRedis.del(key); + parsedRecord = undefined; + } + } + const uidStr = Buffer.isBuffer(uid) + ? uid.toString('base64') + : String(uid); + if (parsedRecord && parsedRecord.uid !== uidStr) { + // the email is already in use by another account (in-progress secondary email setup) + // we should abort early and return a conflict error + throw error.emailInUseByAnotherAccount(); + } + const secret = parsedRecord?.secret ?? (await random.hex(16)); + const value = JSON.stringify({ uid: uidStr, secret }); + // create new reservation or refresh existing reservation with updated TTL + const setResult = await authServerCacheRedis.set( + key, + value, + 'EX', + SECONDARY_EMAIL_PENDING_TTL, + parsedRecord ? 'XX' : 'NX' + ); + if (setResult !== 'OK') { + throw error.emailInUseByAnotherAccount(); } - - await deleteAccountIfUnverified(); - - const hex = await random.hex(16); - emailData.emailCode = hex; - - await db.createEmail(uid, emailData); const geoData = request.app.geo; try { - await mailer.sendVerifySecondaryCodeEmail([emailData], sessionToken, { - code: otpUtils.generateOtpCode(hex, otpOptions), - deviceId: sessionToken.deviceId, - acceptLanguage: request.app.acceptLanguage, - email: emailData.email, - primaryEmail, - location: geoData.location, - timeZone: geoData.timeZone, - uaBrowser: sessionToken.uaBrowser, - uaBrowserVersion: sessionToken.uaBrowserVersion, - uaOS: sessionToken.uaOS, - uaOSVersion: sessionToken.uaOSVersion, + await mailer.sendVerifySecondaryCodeEmail( + [ + { + email, + normalizedEmail, + isVerified: false, + isPrimary: false, + uid, + }, + ], + sessionToken, + { + code: otpUtils.generateOtpCode(secret, otpOptions), + deviceId: sessionToken.deviceId, + acceptLanguage: request.app.acceptLanguage, + email, + primaryEmail, + location: geoData.location, + timeZone: geoData.timeZone, + uaBrowser: sessionToken.uaBrowser, + uaBrowserVersion: sessionToken.uaBrowserVersion, + uaOS: sessionToken.uaOS, + uaOSVersion: sessionToken.uaOSVersion, + uid, + } + ); + } catch (err) { + log.error('secondary_email.sendVerifySecondaryCodeEmail.error', { + err: err, uid, + normalizedEmail, }); - } catch (err) { - log.error('mailer.sendVerifySecondaryCodeEmail', { err: err }); - await db.deleteEmail(emailData.uid, emailData.normalizedEmail); + await authServerCacheRedis.del( + toRedisSecondaryEmailReservationKey(normalizedEmail) + ); throw emailUtils.sendError(err, true); } - await recordSecurityEvent('account.secondary_email_added', { - db, - request, - account, - }); - return {}; - async function deleteAccountIfUnverified() { + async function clearOtherSecondaryEmailClaims(normalizedEmail) { try { - const secondaryEmailRecord = await db.getSecondaryEmail(email); - if (secondaryEmailRecord.isPrimary) { - if (secondaryEmailRecord.isVerified) { - throw error.verifiedPrimaryEmailAlreadyExists(); - } + const secondaryEmailRecord = + await db.getSecondaryEmail(normalizedEmail); + + const msSinceCreated = Date.now() - secondaryEmailRecord.createdAt; + const minUnverifiedAccountTime = + config.secondaryEmail.minUnverifiedAccountTime; + const exceedsMinUnverifiedAccountTime = + msSinceCreated >= minUnverifiedAccountTime; + + if ( + secondaryEmailRecord.isPrimary && + secondaryEmailRecord.isVerified + ) { + throw error.verifiedPrimaryEmailAlreadyExists(); + } + + // Can't add an email that is already verified + if ( + !secondaryEmailRecord.isPrimary && + secondaryEmailRecord.isVerified + ) { + throw error.emailExists(); + } - const msSinceCreated = Date.now() - secondaryEmailRecord.createdAt; - const minUnverifiedAccountTime = - config.secondaryEmail.minUnverifiedAccountTime; - const exceedsMinUnverifiedAccountTime = - msSinceCreated >= minUnverifiedAccountTime; + // check if unverified account can be deleted to free up the email + if ( + secondaryEmailRecord.isPrimary && + !secondaryEmailRecord.isVerified + ) { if ( exceedsMinUnverifiedAccountTime && !(await stripeHelper.hasActiveSubscription( @@ -218,9 +285,9 @@ module.exports = ( } } - // Only delete secondary email if it is unverified and does not belong - // to the current user. + // Check if unverified secondary email can be deleted to free up the email if ( + !secondaryEmailRecord.isPrimary && !secondaryEmailRecord.isVerified && !butil.buffersAreEqual(secondaryEmailRecord.uid, uid) ) { @@ -242,6 +309,7 @@ module.exports = ( const sessionToken = request.auth.credentials; const { email, code } = request.payload; + const normalizedEmail = normalizeEmail(email); await customs.checkAuthenticated( request, @@ -251,49 +319,178 @@ module.exports = ( ); const { uid } = sessionToken; - const account = await db.account(uid); - const emails = await db.accountEmails(uid); + const [account, accountEmails] = await Promise.all([ + db.account(uid), + db.accountEmails(uid), + ]); - // Get the secondary email code - const matchedEmail = emails.find((userEmail) => - emailsMatch(userEmail.normalizedEmail, email) + const accountEmailRecord = accountEmails.find( + (e) => e.normalizedEmail === normalizedEmail ); - if (!matchedEmail) { - throw error.invalidVerificationCode(); + // If email is already saved and verified for this account -> silent success + if (accountEmailRecord?.isVerified) { + return {}; } - const secret = matchedEmail.emailCode; - const { valid: isValid } = otpUtils.verifyOtpCode( - code, - secret, - otpOptions, - 'recovery_email.secondary.verify_code' + // Enforce cap at verify-time + const verifiedSecondaries = accountEmails.filter( + (e) => !e.isPrimary && e.isVerified ); + if (verifiedSecondaries.length >= MAX_SECONDARY_EMAILS) { + throw error.maxSecondaryEmailsReached(); + } - if (!isValid) { - throw error.invalidVerificationCode(); + // Verify that the email is not already verified by another account. + try { + const emailRecord = await db.getSecondaryEmail(normalizedEmail); + if ( + emailRecord && + emailRecord.isVerified && + !butil.buffersAreEqual(emailRecord.uid, uid) + ) { + try { + await authServerCacheRedis.del( + toRedisSecondaryEmailReservationKey(normalizedEmail) + ); + } catch {} + throw error.emailInUseByAnotherAccount(); + } + } catch (err) { + if (err && err.errno !== error.ERRNO.SECONDARY_EMAIL_UNKNOWN) { + throw err; + } } - // User is attempting to verify a secondary email that has already been verified. - // Silently succeed and don't send post verification email. - if (matchedEmail.isVerified) { - log.info('account.verifyEmail.secondary.already-verified', { - uid, - }); - return {}; + // Verify against Redis reservation + const key = toRedisSecondaryEmailReservationKey(normalizedEmail); + const rawRecord = await authServerCacheRedis.get(key); + let parsedRecord; + if (rawRecord) { + try { + parsedRecord = JSON.parse(rawRecord); + } catch { + await authServerCacheRedis.del(key); + } } - await db.verifyEmail(account, matchedEmail.emailCode); - log.info('account.verifyEmail.secondary.confirmed', { - uid, - }); + const uidStr = Buffer.isBuffer(uid) + ? uid.toString('base64') + : String(uid); + if (parsedRecord && parsedRecord.uid !== uidStr) { + // Another account is in the process of setting up this email as secondary + throw error.emailInUseByAnotherAccount(); + } + let secret = parsedRecord?.secret; + if (secret) { + const { valid } = otpUtils.verifyOtpCode( + code, + secret, + otpOptions, + 'recovery_email.secondary.verify_code' + ); + if (!valid) { + throw error.invalidVerificationCode(); + } + // If a legacy unverified record already exists for this account/email, mark it verified + if (accountEmailRecord?.emailCode && !accountEmailRecord.isVerified) { + try { + await db.verifyEmail(account, accountEmailRecord.emailCode); + } catch (e) { + log.error('secondary_email.verify.legacy_record.verify_error', { + uid, + normalizedEmail, + error: e?.message, + }); + throw e; + } + } else { + try { + await db.createEmail(uid, { + email, + normalizedEmail, + emailCode: await random.hex(16), + isVerified: true, + verifiedAt: Date.now(), + }); + } catch (e) { + log.error('secondary_email.verify.dbCreate.error', { + uid, + normalizedEmail, + error: e?.message, + errno: e?.errno, + code: e?.code, + stack: e?.stack, + }); + try { + await authServerCacheRedis.del(key); + secret = undefined; + } catch {} + throw e; + } + } + await recordSecurityEvent('account.secondary_email_added', { + db, + request, + account, + }); + try { + await authServerCacheRedis.del(key); + } catch {} + } + // Fallback to legacy unverified record if present + // Only needed until "old" unverified secondary email records are cleaned up + // See FXA-10083 for more details + if (!secret) { + // Recheck for the email record in the account + const emailRecord = accountEmails.find( + (e) => e.normalizedEmail === normalizedEmail + ); + if (!emailRecord || !emailRecord.emailCode) { + // no trace of the requested email in the account + throw error.unknownSecondaryEmail(); + } + if (emailRecord.isVerified) { + // already verified for this account -> silent success + return {}; + } + // there is a legacy unverified record for this email -> verify it + const { valid } = otpUtils.verifyOtpCode( + code, + emailRecord.emailCode, + otpOptions, + 'recovery_email.secondary.verify_code' + ); + if (!valid) { + throw error.invalidVerificationCode(); + } + try { + await db.verifyEmail(account, emailRecord.emailCode); + log.info('account.verifyEmail.secondary.confirmed', { uid }); + } catch (e) { + log.error('secondary_email.verify.dbVerify.error', { + uid, + normalizedEmail, + error: e?.message, + }); + throw e; + } + } - await mailer.sendPostVerifySecondaryEmail([], account, { - acceptLanguage: request.app.acceptLanguage, - secondaryEmail: matchedEmail.email, - uid, - }); + try { + await mailer.sendPostVerifySecondaryEmail([], account, { + acceptLanguage: request.app.acceptLanguage, + secondaryEmail: email, + uid, + }); + } catch (e) { + log.error('secondary_email.sendPostVerifySecondaryEmail.error', { + uid, + normalizedEmail, + error: e?.message, + }); + // don't throw on email send error + } return {}; }, @@ -665,6 +862,14 @@ module.exports = ( // Any matching code verifies the account await signupUtils.verifyAccount(request, account, request.payload); + // best-effort cleanup: clear any secondary reservation for this primary email + try { + const normalized = normalizeEmail(account.email); + await authServerCacheRedis.del( + toRedisSecondaryEmailReservationKey(normalized) + ); + } catch (_) {} + return {}; async function accountAndTokenVerification( @@ -1057,10 +1262,14 @@ module.exports = ( response: {}, }, handler: async function (request) { + // Note that this "resend" flow is a legacy flow + // for secondary emails stored as unconfirmed records in the db + // This route only uses the legacy records, not the redis reservations + // TODO: Remove this flow once we have cleaned out the old unconfirmed records + // See FXA-10083 for more details log.begin('Account.RecoveryEmailSecondaryResend', request); const sessionToken = request.auth.credentials; - const geoData = request.app.geo; const { email } = request.payload; await customs.checkAuthenticated( @@ -1081,41 +1290,57 @@ module.exports = ( } = sessionToken; const account = await db.account(uid); - const emails = await db.accountEmails(uid); + const normalized = normalizeEmail(email); - // Get the secondary email code - const foundEmail = emails.find((userEmail) => - emailsMatch(userEmail.normalizedEmail, email) - ); - - // This user is attempting to verify a secondary email that doesn't belong to the account. - if (!foundEmail) { + // check if there is an unconfirmed record for this email + let existingRecord; + try { + existingRecord = await db.getSecondaryEmail(normalized); + } catch (e) { + if (e && e.errno === error.ERRNO.SECONDARY_EMAIL_UNKNOWN) { + // maintain legacy errno for resend-to-unowned + throw error.cannotResendEmailCodeToUnownedEmail(); + } + throw e; + } + if (!existingRecord) { throw error.cannotResendEmailCodeToUnownedEmail(); } + if (existingRecord?.isVerified) { + throw error.alreadyOwnsEmail(); + } - const secret = foundEmail.emailCode; - - const code = otpUtils.generateOtpCode(secret, otpOptions); - - const mailerOpts = { - code, - deviceId, - location: geoData.location, - timeZone: geoData.timeZone, - timestamp: Date.now(), - acceptLanguage: request.app.acceptLanguage, - uaBrowser, - uaBrowserVersion, - uaOS, - uaOSVersion, - uaDeviceType, - uid, - }; + const code = otpUtils.generateOtpCode( + existingRecord.emailCode, + otpOptions + ); + const geoData = request.app.geo; await mailer.sendVerifySecondaryCodeEmail( - [foundEmail], + [ + { + email, + normalizedEmail: normalized, + isVerified: false, + isPrimary: false, + uid, + }, + ], account, - mailerOpts + { + code, + deviceId, + location: geoData.location, + timeZone: geoData.timeZone, + timestamp: Date.now(), + acceptLanguage: request.app.acceptLanguage, + uaBrowser, + uaBrowserVersion, + uaOS, + uaOSVersion, + uaDeviceType, + uid, + } ); return {}; diff --git a/packages/fxa-auth-server/lib/routes/index.js b/packages/fxa-auth-server/lib/routes/index.js index c4056521e89..67f389d4f81 100644 --- a/packages/fxa-auth-server/lib/routes/index.js +++ b/packages/fxa-auth-server/lib/routes/index.js @@ -118,6 +118,7 @@ module.exports = function ( signupUtils, zendeskClient, stripeHelper, + authServerCacheRedis, statsd ); const password = require('./password')( diff --git a/packages/fxa-auth-server/test/local/routes/emails.js b/packages/fxa-auth-server/test/local/routes/emails.js index 37399adfd9d..28de244839f 100644 --- a/packages/fxa-auth-server/test/local/routes/emails.js +++ b/packages/fxa-auth-server/test/local/routes/emails.js @@ -192,7 +192,14 @@ const makeRoutes = function (options = {}, requireMocks) { verificationReminders, glean ); - return proxyquire('../../../lib/routes/emails', requireMocks || {})( + + const authServerCacheRedis = options.authServerCacheRedis || { + get: sinon.stub(), + set: sinon.stub().resolves('OK'), + del: sinon.stub().resolves(1), + }; + + const routes = proxyquire('../../../lib/routes/emails', requireMocks || {})( log, db, mailer, @@ -204,8 +211,11 @@ const makeRoutes = function (options = {}, requireMocks) { signupUtils, undefined, options.stripeHelper, + authServerCacheRedis, statsd ); + routes.__redis = authServerCacheRedis; + return routes; }; function runTest(route, request, assertions) { @@ -1175,15 +1185,32 @@ describe('/recovery_email', () => { mockMailer.sendPostChangePrimaryEmail.resetHistory(); }); - it('should create email on account', () => { + it('should store reservation in redis and send verification email', () => { route = getRoute(accountRoutes, '/recovery_email'); return runTest(route, mockRequest, (response) => { assert.ok(response); - assert.equal(mockDB.createEmail.callCount, 1, 'call db.createEmail'); + // grab the injected redis and route key helper + const injectedRedis = accountRoutes.__redis; + // Reconstruct expected key format locally to avoid importing the route factory + const toRedisSecondaryEmailReservationKey = (email) => + `additional_email:reservation:${normalizeEmail(email)}`; + + assert.equal(injectedRedis.set.callCount, 1, 'call redis.set'); + const args = injectedRedis.set.args[0]; + assert.equal( + args[0], + toRedisSecondaryEmailReservationKey(TEST_EMAIL_ADDITIONAL) + ); + // uid string in route is stringified; use string here + assert.include(args[1], '"uid"'); + assert.include(args[1], '"secret"'); + assert.equal(args[2], 'EX'); + assert.equal(args[4], 'NX'); + assert.equal( mockMailer.sendVerifySecondaryCodeEmail.callCount, 1, - 'call db.sendVerifySecondaryCodeEmail' + 'call mailer.sendVerifySecondaryCodeEmail' ); assert.equal( mockMailer.sendVerifySecondaryCodeEmail.args[0][2].deviceId, @@ -1194,15 +1221,12 @@ describe('/recovery_email', () => { mockRequest.auth.credentials.uid ); - sinon.assert.calledWith( - mockAccountEventManager.recordSecurityEvent, - sinon.match.defined, - sinon.match({ - name: 'account.secondary_email_added', - ipAddr: '63.245.221.32', - uid: uid, - tokenId: undefined, - }) + // TODO: Should we create a new security event for the reservation? + // Should we actually record the add event here even if not added to db, and 'confirm' event later? + assert.equal( + mockAccountEventManager.recordSecurityEvent.callCount, + 0, + 'no security event recorded' ); }); }); @@ -1232,7 +1256,7 @@ describe('/recovery_email', () => { assert.equal( err.errno, 139, - 'add secondary email that is same as your primary' + 'Can not add secondary email that is same as your primary' ) ); }); @@ -1260,7 +1284,12 @@ describe('/recovery_email', () => { assert.fail( 'Should have failed when adding an email that already belongs to the account' ), - (err) => assert.equal(err.errno, 189, 'already exists on your account') + (err) => + assert.equal( + err.errno, + 189, + 'This email already exists on your account' + ) ); }); @@ -1300,6 +1329,12 @@ describe('/recovery_email', () => { }); route = getRoute(accountRoutes, '/recovery_email'); mockRequest.payload.email = TEST_EMAIL_ADDITIONAL; + // grab the injected redis and route key helper + const injectedRedis = accountRoutes.__redis; + // Reconstruct expected key format locally to avoid importing the route factory + const toRedisSecondaryEmailReservationKey = (email) => + `additional_email:reservation:${normalizeEmail(email)}`; + return runTest(route, mockRequest, (response) => { assert.ok(response); assert.equal( @@ -1307,13 +1342,17 @@ describe('/recovery_email', () => { 1, 'call db.deleteAccount' ); - assert.equal(mockDB.createEmail.callCount, 1, 'call db.createEmail'); - const args = mockDB.createEmail.getCall(0).args; + assert.equal(injectedRedis.set.callCount, 1, 'call redis.set'); + const args = injectedRedis.set.args[0]; assert.equal( - args[1].email, - TEST_EMAIL_ADDITIONAL, - 'call db.createEmail with correct email' + args[0], + toRedisSecondaryEmailReservationKey(TEST_EMAIL_ADDITIONAL) ); + // uid string in route is stringified; use string here + assert.include(args[1], '"uid"'); + assert.include(args[1], '"secret"'); + assert.equal(args[2], 'EX'); + assert.equal(args[4], 'NX'); assert.equal( mockMailer.sendVerifySecondaryCodeEmail.callCount, 1, @@ -1371,8 +1410,9 @@ describe('/recovery_email', () => { ); }); - it('deletes secondary email if there was an error sending verification email', () => { + it('clears reservation if there was an error sending verification email', () => { route = getRoute(accountRoutes, '/recovery_email'); + const injectedRedis = accountRoutes.__redis; mockMailer.sendVerifySecondaryCodeEmail = sinon.spy(() => { return Promise.reject(new Error('failed to send')); }); @@ -1382,33 +1422,17 @@ describe('/recovery_email', () => { }).catch((err) => { assert.equal(err.errno, 151, 'failed to send email error'); assert.equal(err.output.payload.code, 422); - assert.equal(mockDB.createEmail.callCount, 1, 'call db.createEmail'); - assert.equal(mockDB.deleteEmail.callCount, 1, 'call db.deleteEmail'); assert.equal( - mockDB.deleteEmail.args[0][0], - mockRequest.auth.credentials.uid, - 'correct uid passed' - ); - assert.equal( - mockDB.deleteEmail.args[0][1], - TEST_EMAIL_ADDITIONAL, - 'correct email passed' - ); - assert.equal( - mockMailer.sendVerifySecondaryCodeEmail.callCount, + injectedRedis.del.callCount, 1, - 'call db.sendVerifySecondaryCodeEmail' + 'call authServerCacheRedis.del' ); + const args = injectedRedis.del.args[0]; + const toRedisSecondaryEmailReservationKey = (email) => + `additional_email:reservation:${normalizeEmail(email)}`; assert.equal( - mockMailer.sendVerifySecondaryCodeEmail.args[0][2].deviceId, - mockRequest.auth.credentials.deviceId - ); - assert.equal( - mockMailer.sendVerifySecondaryCodeEmail.args[0][2].uid, - mockRequest.auth.credentials.uid - ); - mockMailer.sendVerifySecondaryCodeEmail = sinon.spy(() => - Promise.resolve() + args[0], + toRedisSecondaryEmailReservationKey(TEST_EMAIL_ADDITIONAL) ); }); }); @@ -1712,8 +1736,8 @@ describe('/recovery_email', () => { mockRequest.payload.email = 'notcorrectemail@a.com'; await assert.failsAsync(runTest(route, mockRequest), { - errno: 105, - message: 'Invalid confirmation code', + errno: 143, + message: 'Unknown email', }); }); @@ -1732,11 +1756,24 @@ describe('/recovery_email', () => { it('should resend otp code', async () => { route = getRoute(accountRoutes, '/recovery_email/secondary/resend_code'); + // For the legacy resend flow, ensure there is an unconfirmed DB record + mockDB.getSecondaryEmail = sinon.spy(() => { + return Promise.resolve({ + isVerified: false, + emailCode: dbData.secondEmailCode, + uid: mockRequest.auth.credentials.uid, + }); + }); + + // Ensure mailer resolves in this test (it may have been stubbed to reject in prior tests) + mockMailer.sendVerifySecondaryCodeEmail = sinon.spy(() => + Promise.resolve() + ); + const response = await runTest(route, mockRequest); assert.ok(response); assert.calledOnce(mockDB.account); - assert.calledOnce(mockDB.accountEmails); assert.calledOnce(mockMailer.sendVerifySecondaryCodeEmail); const expectedCode = otpUtils.generateOtpCode( diff --git a/packages/fxa-auth-server/test/mail_helper.js b/packages/fxa-auth-server/test/mail_helper.js index 05277452e92..82239df7ed7 100644 --- a/packages/fxa-auth-server/test/mail_helper.js +++ b/packages/fxa-auth-server/test/mail_helper.js @@ -86,7 +86,8 @@ module.exports = (printLogs) => { const sc = mail.headers['x-signin-verify-code']; const rpc = mail.headers['x-password-forgot-otp']; const mfa = mail.headers['x-account-change-verify-code']; - const template = mail.headers['x-template-name']; + const template = + mail.headers['x-template-name'] || mail.headers['X-Template-Name']; // Workaround because the email service wraps this header in `< >`. // See: https://github.com/mozilla/fxa-content-server/pull/6470#issuecomment-415224438 @@ -95,6 +96,11 @@ module.exports = (printLogs) => { if (vsc) { console.log('\x1B[34mSignin code', vsc, '\x1B[39m'); + } else if ( + vc && + /verifySecondaryCode/i.test(String(template || '')) + ) { + console.log('\x1B[36mSecondary email verify code:', vc, '\x1B[39m'); } else if (vc) { console.log('\x1B[32m', link || vc, '\x1B[39m'); } else if (sc) { diff --git a/packages/fxa-auth-server/test/remote/recovery_email_change_email.js b/packages/fxa-auth-server/test/remote/recovery_email_change_email.js index 7c8df626a05..964179bc8c5 100644 --- a/packages/fxa-auth-server/test/remote/recovery_email_change_email.js +++ b/packages/fxa-auth-server/test/remote/recovery_email_change_email.js @@ -7,546 +7,596 @@ const { assert } = require('chai'); const TestServer = require('../test_server'); const Client = require('../client')(); +const { setupAccountDatabase } = require('@fxa/shared/db/mysql/account'); +const cfg = require('../../config').default.getProperties(); +const { email: emailHelper } = require('fxa-shared'); +const crypto = require('crypto'); let config, server, client, email, secondEmail; const password = 'allyourbasearebelongtous', newPassword = 'newpassword'; -[ {version:""}, {version:"V2"}].forEach((testOptions) => { +[{ version: '' }, { version: 'V2' }].forEach((testOptions) => { + describe(`#integration${testOptions.version} - remote change email`, function () { + this.timeout(60000); -describe(`#integration${testOptions.version} - remote change email`, function () { - this.timeout(60000); - - before(async () => { - config = require('../../config').default.getProperties(); - config.securityHistory.ipProfiling = {}; - server = await TestServer.start(config); - }); + before(async () => { + config = require('../../config').default.getProperties(); + config.securityHistory.ipProfiling = {}; + server = await TestServer.start(config); + }); - after(async () => { - await TestServer.stop(server); - }); + after(async () => { + await TestServer.stop(server); + }); - beforeEach(() => { - email = server.uniqueEmail(); - secondEmail = server.uniqueEmail('@notrestmail.com'); - - return Client.createAndVerify( - config.publicUrl, - email, - password, - server.mailbox, - testOptions - ) - .then((x) => { - client = x; - assert.ok(client.authAt, 'authAt was set'); - }) - .then(() => { - return client.emailStatus(); - }) - .then((status) => { - assert.equal(status.verified, true, 'account is verified'); - return client.createEmail(secondEmail); - }) - .then((res) => { - assert.ok(res, 'ok response'); - return server.mailbox.waitForEmail(secondEmail); - }) - .then((emailData) => { - const templateName = emailData['headers']['x-template-name']; - const emailCode = emailData['headers']['x-verify-code']; - assert.equal(templateName, 'verifySecondaryCode'); - assert.ok(emailCode, 'emailCode set'); - return client.verifySecondaryEmailWithCode(emailCode, secondEmail); - }) - .then((res) => { - assert.ok(res, 'ok response'); - return client.accountEmails(); - }) - .then((res) => { - assert.equal(res.length, 2, 'returns number of emails'); - assert.equal(res[1].email, secondEmail, 'returns correct email'); - assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[1].verified, true, 'returns correct verified'); - return server.mailbox.waitForEmail(email); - }); - }); + beforeEach(() => { + email = server.uniqueEmail(); + secondEmail = server.uniqueEmail('@notrestmail.com'); - describe('should change primary email', () => { - it('fails to change email to an that is not owned by user', () => { - const userEmail2 = server.uniqueEmail(); - const anotherEmail = server.uniqueEmail(); return Client.createAndVerify( config.publicUrl, - userEmail2, + email, password, server.mailbox, testOptions ) - .then((client2) => { - return client2.createEmail(anotherEmail); + .then((x) => { + client = x; + assert.ok(client.authAt, 'authAt was set'); }) .then(() => { - return client.setPrimaryEmail(anotherEmail).then(() => { - assert.fail( - 'Should not have set email that belongs to another account' - ); - }); + return client.emailStatus(); }) - .catch((err) => { - assert.equal(err.errno, 148, 'returns correct errno'); - assert.equal(err.code, 400, 'returns correct error code'); - }); - }); - - it('fails to change email to unverified email', () => { - const someEmail = server.uniqueEmail(); - return client - .createEmail(someEmail) - .then(() => { - return client.setPrimaryEmail(someEmail).then(() => { - assert.fail('Should not have set email to an unverified email'); - }); + .then((status) => { + assert.equal(status.verified, true, 'account is verified'); + return client.createEmail(secondEmail); + }) + .then((res) => { + assert.ok(res, 'ok response'); + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + const templateName = emailData['headers']['x-template-name']; + const emailCode = emailData['headers']['x-verify-code']; + assert.equal(templateName, 'verifySecondaryCode'); + assert.ok(emailCode, 'emailCode set'); + return client.verifySecondaryEmailWithCode(emailCode, secondEmail); }) - .catch((err) => { - assert.equal(err.errno, 147, 'returns correct errno'); - assert.equal(err.code, 400, 'returns correct error code'); - }); - }); - - it('can change primary email', () => { - return client - .setPrimaryEmail(secondEmail) .then((res) => { assert.ok(res, 'ok response'); return client.accountEmails(); }) .then((res) => { assert.equal(res.length, 2, 'returns number of emails'); - assert.equal(res[0].email, secondEmail, 'returns correct email'); - assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); - assert.equal(res[0].verified, true, 'returns correct verified'); - assert.equal(res[1].email, email, 'returns correct email'); + assert.equal(res[1].email, secondEmail, 'returns correct email'); assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); assert.equal(res[1].verified, true, 'returns correct verified'); - - return server.mailbox.waitForEmail(secondEmail); - }) - .then((emailData) => { - assert.equal(emailData.headers['to'], secondEmail, 'to email set'); - assert.equal(emailData.headers['cc'], email, 'cc emails set'); - assert.equal( - emailData.headers['x-template-name'], - 'postChangePrimary' - ); + return server.mailbox.waitForEmail(email); }); }); - it('can login', () => { - return client - .setPrimaryEmail(secondEmail) - .then((res) => { - assert.ok(res, 'ok response'); - - if (testOptions.version === 'V2') { - // Note for V2 we can login with new primary email. The password is not encrypted with - // the original email, so this now works! - return Client.login( - config.publicUrl, - secondEmail, - password, - testOptions - ); - } else { - // Verify account can login with new primary email - return Client.login( - config.publicUrl, - secondEmail, - password, - testOptions - ).then(() => { + describe('should change primary email', () => { + it('fails to change email to an that is not owned by user', () => { + const userEmail2 = server.uniqueEmail(); + const anotherEmail = server.uniqueEmail(); + return Client.createAndVerify( + config.publicUrl, + userEmail2, + password, + server.mailbox, + testOptions + ) + .then((client2) => { + return client2 + .createEmail(anotherEmail) + .then(() => server.mailbox.waitForEmail(anotherEmail)) + .then((emailData) => { + const code = emailData.headers['x-verify-code']; + assert.ok(code, 'email code set'); + return client2.verifySecondaryEmailWithCode(code, anotherEmail); + }); + }) + .then(() => { + return client.setPrimaryEmail(anotherEmail).then(() => { assert.fail( - new Error( - 'Should have returned correct email for user to login' - ) + 'Should not have set email that belongs to another account' ); }); - } - }) - .catch((err) => { - // Login should fail for this user and return the normalizedEmail used when - // the account was created. We then attempt to re-login with this email and pass - // the original email used to login - assert.equal(err.code, 400, 'correct error code'); - assert.equal(err.errno, 120, 'correct errno code'); - assert.equal(err.email, email, 'correct hashed email returned'); - - return Client.login(config.publicUrl, err.email, password, { - originalLoginEmail: secondEmail, - ...testOptions, + }) + .catch((err) => { + assert.equal(err.errno, 148, 'returns correct errno'); + assert.equal(err.code, 400, 'returns correct error code'); }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); + }); - it('can change password', () => { - return client - .setPrimaryEmail(secondEmail) - .then((res) => { - assert.ok(res, 'ok response'); - return Client.login(config.publicUrl, email, password, { - originalLoginEmail: secondEmail, - ...testOptions, + it('fails to change email to unverified email', () => { + const someEmail = server.uniqueEmail(); + return client + .createEmail(someEmail) + .then(() => { + return client.setPrimaryEmail(someEmail).then(() => { + assert.fail('Should not have set email to an unverified email'); + }); + }) + .catch((err) => { + // we expect the email to be unknown if the email has not been verified + // the email is only stored in the database if it has been verified + // until then, it is only reserved in Redis and can't be set as primary + assert.equal(err.errno, 143, 'returns correct errno'); + assert.equal(err.code, 400, 'returns correct error code'); }); - }) - .then((res) => { - client = res; - return client.changePassword( - newPassword, - undefined, - client.sessionToken - ); - }) - .then((res) => { - assert.ok(res, 'ok response'); - return Client.login(config.publicUrl, email, newPassword, { - originalLoginEmail: secondEmail, - ...testOptions, + }); + + it('fails to to change primary email to an unverified email stored in database (legacy)', async () => { + const someEmail = server.uniqueEmail(); + // Pre-seed the DB with an unverified secondary email record for this uid + const db = await setupAccountDatabase(cfg.database.mysql.auth); + try { + await db + .insertInto('emails') + .values({ + email: someEmail, + normalizedEmail: emailHelper.helpers.normalizeEmail(someEmail), + uid: Buffer.from(client.uid, 'hex'), + emailCode: Buffer.from( + crypto.randomBytes(16).toString('hex'), + 'hex' + ), + isVerified: 0, + isPrimary: 0, + createdAt: Date.now(), + }) + .execute(); + } finally { + await db.destroy(); + } + + return client + .setPrimaryEmail(someEmail) + .then(() => { + assert.fail('Should not have set email to an unverified email'); + }) + .catch((err) => { + assert.equal(err.errno, 147, 'returns correct errno'); + assert.equal(err.code, 400, 'returns correct error code'); }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); + }); - it('can reset password', () => { - return client - .setPrimaryEmail(secondEmail) - .then((res) => { - assert.ok(res, 'ok response'); - return server.mailbox.waitForEmail(secondEmail); - }) - .then((emailData) => { - assert.equal(emailData.headers['to'], secondEmail, 'to email set'); - assert.equal(emailData.headers['cc'], email, 'cc emails set'); - assert.equal( - emailData.headers['x-template-name'], - 'postChangePrimary' - ); - - client.email = secondEmail; - return client.forgotPassword(); - }) - .then(() => { - return server.mailbox.waitForCode(secondEmail); - }) - .then((code) => { - assert.ok(code, 'code is set'); - return resetPassword(client, code, newPassword, undefined, { - emailToHashWith: email, + it('can change primary email', () => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + return client.accountEmails(); + }) + .then((res) => { + assert.equal(res.length, 2, 'returns number of emails'); + assert.equal(res[0].email, secondEmail, 'returns correct email'); + assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); + assert.equal(res[0].verified, true, 'returns correct verified'); + assert.equal(res[1].email, email, 'returns correct email'); + assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); + assert.equal(res[1].verified, true, 'returns correct verified'); + + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + assert.equal(emailData.headers['to'], secondEmail, 'to email set'); + assert.equal(emailData.headers['cc'], email, 'cc emails set'); + assert.equal( + emailData.headers['x-template-name'], + 'postChangePrimary' + ); }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }) - .then(() => { - if (testOptions.version === 'V2') { - return Client.upgradeCredentials( - config.publicUrl, - email, + }); + + it('can login', () => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + + if (testOptions.version === 'V2') { + // Note for V2 we can login with new primary email. The password is not encrypted with + // the original email, so this now works! + return Client.login( + config.publicUrl, + secondEmail, + password, + testOptions + ); + } else { + // Verify account can login with new primary email + return Client.login( + config.publicUrl, + secondEmail, + password, + testOptions + ).then(() => { + assert.fail( + new Error( + 'Should have returned correct email for user to login' + ) + ); + }); + } + }) + .catch((err) => { + // Login should fail for this user and return the normalizedEmail used when + // the account was created. We then attempt to re-login with this email and pass + // the original email used to login + assert.equal(err.code, 400, 'correct error code'); + assert.equal(err.errno, 120, 'correct errno code'); + assert.equal(err.email, email, 'correct hashed email returned'); + + return Client.login(config.publicUrl, err.email, password, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); + }); + }); + + it('can change password', () => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + return Client.login(config.publicUrl, email, password, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + client = res; + return client.changePassword( newPassword, - { - originalLoginEmail: secondEmail, - version: '', - keys: true, - } + undefined, + client.sessionToken ); - } - }) - .then(() => { - return Client.login(config.publicUrl, email, newPassword, { - originalLoginEmail: secondEmail, - ...testOptions, + }) + .then((res) => { + assert.ok(res, 'ok response'); + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); + }); - it('can delete account', () => { - return client - .setPrimaryEmail(secondEmail) - .then((res) => { - assert.ok(res, 'ok response'); - return client.destroyAccount(); - }) - .then(() => { - return Client.login(config.publicUrl, email, newPassword, { - originalLoginEmail: secondEmail, - ...testOptions, + it('can reset password', () => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + return server.mailbox.waitForEmail(secondEmail); }) - .then(() => { - assert.fail( - 'Should not have been able to login after deleting account' + .then((emailData) => { + assert.equal(emailData.headers['to'], secondEmail, 'to email set'); + assert.equal(emailData.headers['cc'], email, 'cc emails set'); + assert.equal( + emailData.headers['x-template-name'], + 'postChangePrimary' + ); + + client.email = secondEmail; + return client.forgotPassword(); + }) + .then(() => { + return server.mailbox.waitForCode(secondEmail); + }) + .then((code) => { + assert.ok(code, 'code is set'); + return resetPassword(client, code, newPassword, undefined, { + emailToHashWith: email, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); + }) + .then(() => { + if (testOptions.version === 'V2') { + return Client.upgradeCredentials( + config.publicUrl, + email, + newPassword, + { + originalLoginEmail: secondEmail, + version: '', + keys: true, + } ); - }) - .catch((err) => { - assert.equal(err.errno, 102, 'unknown account error code'); - assert.equal(err.email, secondEmail, 'returns correct email'); + } + }) + .then(() => { + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, }); - }); - }); - }); + }) + .then((res) => { + assert.ok(res, 'ok response'); + }); + }); - it('change primary email with multiple accounts', async () => { - /** - * Below tests the following scenario: - * - * User A with Email A (primary) and Email A1 (secondary) - * User B with Email B (primary) and Email B1 (secondary) - * - * with changing primary emails etc transform to ==> - * - * User A with Email B (primary) - * User B with Email A (primary) - * - * and can successfully login - */ - let emailData, emailCode; - const password2 = 'asdf'; - const client1Email = server.uniqueEmail(); - const client1SecondEmail = server.uniqueEmail(); - const client2Email = server.uniqueEmail(); - const client2SecondEmail = server.uniqueEmail(); - - const client1 = await Client.createAndVerify( - config.publicUrl, - client1Email, - password, - server.mailbox, - testOptions - ); - - // Create a second client - const client2 = await Client.createAndVerify( - config.publicUrl, - client2Email, - password2, - server.mailbox, - testOptions - ); - - // Update client1's email and verify - await client1.createEmail(client1SecondEmail); - emailData = await server.mailbox.waitForEmail(client1SecondEmail); - emailCode = emailData['headers']['x-verify-code']; - await client1.verifySecondaryEmailWithCode(emailCode, client1SecondEmail); - - // Update client2 - await client2.createEmail(client2SecondEmail); - emailData = await server.mailbox.waitForEmail(client2SecondEmail); - emailCode = emailData['headers']['x-verify-code']; - await client2.verifySecondaryEmailWithCode(emailCode, client2SecondEmail); - - await client1.setPrimaryEmail(client1SecondEmail); - await client1.deleteEmail(client1Email); - - await client2.setPrimaryEmail(client2SecondEmail); - await client2.deleteEmail(client2Email); - - await client1.createEmail(client2Email); - emailData = await server.mailbox.waitForEmail(client2Email); - emailCode = emailData[2]['headers']['x-verify-code']; - await client1.verifySecondaryEmailWithCode(emailCode, client2Email); - await client1.setPrimaryEmail(client2Email); - await client1.deleteEmail(client1SecondEmail); - - await client2.createEmail(client1Email); - emailData = await server.mailbox.waitForEmail(client1Email); - emailCode = emailData[2]['headers']['x-verify-code']; - await client2.verifySecondaryEmailWithCode(emailCode, client1Email); - await client2.setPrimaryEmail(client1Email); - await client2.deleteEmail(client2SecondEmail); - - const res = await Client.login(config.publicUrl, client1Email, password, { - originalLoginEmail: client2Email, - ...testOptions, + it('can delete account', () => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + return client.destroyAccount(); + }) + .then(() => { + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, + }) + .then(() => { + assert.fail( + 'Should not have been able to login after deleting account' + ); + }) + .catch((err) => { + assert.equal(err.errno, 102, 'unknown account error code'); + assert.equal(err.email, secondEmail, 'returns correct email'); + }); + }); + }); }); - assert.ok(res, 'ok response'); - }); + it('change primary email with multiple accounts', async () => { + /** + * Below tests the following scenario: + * + * User A with Email A (primary) and Email A1 (secondary) + * User B with Email B (primary) and Email B1 (secondary) + * + * with changing primary emails etc transform to ==> + * + * User A with Email B (primary) + * User B with Email A (primary) + * + * and can successfully login + */ + let emailData, emailCode; + const password2 = 'asdf'; + const client1Email = server.uniqueEmail(); + const client1SecondEmail = server.uniqueEmail(); + const client2Email = server.uniqueEmail(); + const client2SecondEmail = server.uniqueEmail(); + + const client1 = await Client.createAndVerify( + config.publicUrl, + client1Email, + password, + server.mailbox, + testOptions + ); - describe('change primary email, deletes old primary', () => { - beforeEach(() => { - return client - .setPrimaryEmail(secondEmail) - .then((res) => { - assert.ok(res, 'ok response'); - return server.mailbox.waitForEmail(secondEmail); - }) - .then((emailData) => { - assert.equal(emailData.headers['to'], secondEmail, 'to email set'); - assert.equal(emailData.headers['cc'], email, 'cc emails set'); - assert.equal( - emailData.headers['x-template-name'], - 'postChangePrimary' - ); - return client.deleteEmail(email); - }) - .then((res) => { - assert.ok(res, 'ok response'); - return client.accountEmails(); - }) - .then((res) => { - assert.equal(res.length, 1, 'returns number of emails'); - assert.equal(res[0].email, secondEmail, 'returns correct email'); - assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); - assert.equal(res[0].verified, true, 'returns correct verified'); + // Create a second client + const client2 = await Client.createAndVerify( + config.publicUrl, + client2Email, + password2, + server.mailbox, + testOptions + ); + + // Update client1's email and verify + await client1.createEmail(client1SecondEmail); + emailData = await server.mailbox.waitForEmail(client1SecondEmail); + emailCode = emailData['headers']['x-verify-code']; + await client1.verifySecondaryEmailWithCode(emailCode, client1SecondEmail); + + // Update client2 + await client2.createEmail(client2SecondEmail); + emailData = await server.mailbox.waitForEmail(client2SecondEmail); + emailCode = emailData['headers']['x-verify-code']; + await client2.verifySecondaryEmailWithCode(emailCode, client2SecondEmail); + + await client1.setPrimaryEmail(client1SecondEmail); + await client1.deleteEmail(client1Email); + + await client2.setPrimaryEmail(client2SecondEmail); + await client2.deleteEmail(client2Email); + + await client1.createEmail(client2Email); + emailData = await server.mailbox.waitForEmail(client2Email); + emailCode = emailData[2]['headers']['x-verify-code']; + await client1.verifySecondaryEmailWithCode(emailCode, client2Email); + await client1.setPrimaryEmail(client2Email); + await client1.deleteEmail(client1SecondEmail); + + await client2.createEmail(client1Email); + emailData = await server.mailbox.waitForEmail(client1Email); + emailCode = emailData[2]['headers']['x-verify-code']; + await client2.verifySecondaryEmailWithCode(emailCode, client1Email); + await client2.setPrimaryEmail(client1Email); + await client2.deleteEmail(client2SecondEmail); + + const res = await Client.login(config.publicUrl, client1Email, password, { + originalLoginEmail: client2Email, + ...testOptions, + }); - // Primary account is notified that secondary email has been removed - return server.mailbox.waitForEmail(secondEmail); - }) - .then((emailData) => { - const templateName = emailData['headers']['x-template-name']; - assert.equal(templateName, 'postRemoveSecondary'); - }); + assert.ok(res, 'ok response'); }); - it('can login', () => { - if (testOptions.version === 'V2') { - // Note that with V2 logins, you can actually use the secondary email to login. This is - // due to the fact the salt is now independent of the original email. + describe('change primary email, deletes old primary', () => { + beforeEach(() => { + return client + .setPrimaryEmail(secondEmail) + .then((res) => { + assert.ok(res, 'ok response'); + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + assert.equal(emailData.headers['to'], secondEmail, 'to email set'); + assert.equal(emailData.headers['cc'], email, 'cc emails set'); + assert.equal( + emailData.headers['x-template-name'], + 'postChangePrimary' + ); + return client.deleteEmail(email); + }) + .then((res) => { + assert.ok(res, 'ok response'); + return client.accountEmails(); + }) + .then((res) => { + assert.equal(res.length, 1, 'returns number of emails'); + assert.equal(res[0].email, secondEmail, 'returns correct email'); + assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); + assert.equal(res[0].verified, true, 'returns correct verified'); + + // Primary account is notified that secondary email has been removed + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + const templateName = emailData['headers']['x-template-name']; + assert.equal(templateName, 'postRemoveSecondary'); + }); + }); + + it('can login', () => { + if (testOptions.version === 'V2') { + // Note that with V2 logins, you can actually use the secondary email to login. This is + // due to the fact the salt is now independent of the original email. + return Client.login( + config.publicUrl, + secondEmail, + password, + testOptions + ).then((res) => { + assert.exists(res.sessionToken); + }); + } + + // Verify account can still login with new primary email return Client.login( config.publicUrl, secondEmail, password, testOptions - ).then((res) => { - assert.exists(res.sessionToken); - }); - } - - // Verify account can still login with new primary email - return Client.login(config.publicUrl, secondEmail, password, testOptions) - .then(() => { - assert.fail( - new Error('Should have returned correct email for user to login') - ); - }) - .catch((err) => { - // Login should fail for this user and return the normalizedEmail used when - // the account was created. We then attempt to re-login with this email and pass - // the original email used to login - assert.equal(err.code, 400, 'correct error code'); - assert.equal(err.errno, 120, 'correct errno code'); - assert.equal(err.email, email, 'correct hashed email returned'); - - return Client.login(config.publicUrl, err.email, password, { - originalLoginEmail: secondEmail, - ...testOptions, - }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); - - it('can change password', () => { - return Client.login(config.publicUrl, email, password, { - originalLoginEmail: secondEmail, - ...testOptions, - }) - .then((res) => { - client = res; - return client.changePassword( - newPassword, - undefined, - client.sessionToken - ); - }) - .then((res) => { - assert.ok(res, 'ok response'); - return Client.login(config.publicUrl, email, newPassword, { - originalLoginEmail: secondEmail, - ...testOptions, - }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); - - it('can reset password', () => { - client.email = secondEmail; - return client - .forgotPassword() - .then(() => { - return server.mailbox.waitForCode(secondEmail); - }) - .then((code) => { - assert.ok(code, 'code is set'); - return resetPassword(client, code, newPassword, undefined, { - emailToHashWith: email, - }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }) - .then(() => { - if (testOptions.version === 'V2') { - return Client.upgradeCredentials( - config.publicUrl, - email, - newPassword, - { - originalLoginEmail: secondEmail, - version: '', - keys: true, - } + ) + .then(() => { + assert.fail( + new Error('Should have returned correct email for user to login') ); - } - }) - .then(() => { - return Client.login(config.publicUrl, email, newPassword, { - originalLoginEmail: secondEmail, - ...testOptions, + }) + .catch((err) => { + // Login should fail for this user and return the normalizedEmail used when + // the account was created. We then attempt to re-login with this email and pass + // the original email used to login + assert.equal(err.code, 400, 'correct error code'); + assert.equal(err.errno, 120, 'correct errno code'); + assert.equal(err.email, email, 'correct hashed email returned'); + + return Client.login(config.publicUrl, err.email, password, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); }); - }) - .then((res) => { - assert.ok(res, 'ok response'); - }); - }); + }); - it('can delete account', () => { - return client.destroyAccount().then(() => { - return Client.login(config.publicUrl, email, newPassword, { + it('can change password', () => { + return Client.login(config.publicUrl, email, password, { originalLoginEmail: secondEmail, ...testOptions, }) - .then(() => { - assert.fail( - 'Should not have been able to login after deleting account' + .then((res) => { + client = res; + return client.changePassword( + newPassword, + undefined, + client.sessionToken ); }) - .catch((err) => { - assert.equal(err.errno, 102, 'unknown account error code'); - assert.equal(err.email, secondEmail, 'returns correct email'); + .then((res) => { + assert.ok(res, 'ok response'); + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); }); }); - }); - }); - + it('can reset password', () => { + client.email = secondEmail; + return client + .forgotPassword() + .then(() => { + return server.mailbox.waitForCode(secondEmail); + }) + .then((code) => { + assert.ok(code, 'code is set'); + return resetPassword(client, code, newPassword, undefined, { + emailToHashWith: email, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); + }) + .then(() => { + if (testOptions.version === 'V2') { + return Client.upgradeCredentials( + config.publicUrl, + email, + newPassword, + { + originalLoginEmail: secondEmail, + version: '', + keys: true, + } + ); + } + }) + .then(() => { + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, + }); + }) + .then((res) => { + assert.ok(res, 'ok response'); + }); + }); - function resetPassword(client, code, newPassword, headers, options) { - return client.verifyPasswordResetCode(code, headers, options).then(() => { - return client.resetPassword(newPassword, {}, options); + it('can delete account', () => { + return client.destroyAccount().then(() => { + return Client.login(config.publicUrl, email, newPassword, { + originalLoginEmail: secondEmail, + ...testOptions, + }) + .then(() => { + assert.fail( + 'Should not have been able to login after deleting account' + ); + }) + .catch((err) => { + assert.equal(err.errno, 102, 'unknown account error code'); + assert.equal(err.email, secondEmail, 'returns correct email'); + }); + }); + }); }); - } -}); + function resetPassword(client, code, newPassword, headers, options) { + return client.verifyPasswordResetCode(code, headers, options).then(() => { + return client.resetPassword(newPassword, {}, options); + }); + } + }); }); diff --git a/packages/fxa-auth-server/test/remote/recovery_email_emails.js b/packages/fxa-auth-server/test/remote/recovery_email_emails.js index 841ad884779..6d5f2b3d3df 100644 --- a/packages/fxa-auth-server/test/remote/recovery_email_emails.js +++ b/packages/fxa-auth-server/test/remote/recovery_email_emails.js @@ -8,6 +8,10 @@ const { assert } = require('chai'); const TestServer = require('../test_server'); const Client = require('../client')(); const ERRNO = require('../../lib/error').ERRNO; +const { setupAccountDatabase } = require('@fxa/shared/db/mysql/account'); +const cfg = require('../../config').default.getProperties(); +const { email: emailHelper } = require('fxa-shared'); +const crypto = require('crypto'); let config, server, client, email; const password = 'allyourbasearebelongtous'; @@ -52,7 +56,6 @@ const password = 'allyourbasearebelongtous'; describe('should create and get additional email', () => { it('can create', () => { const secondEmail = server.uniqueEmail(); - const thirdEmail = server.uniqueEmail(); return client .accountEmails() .then((res) => { @@ -67,40 +70,118 @@ const password = 'allyourbasearebelongtous'; return client.accountEmails(); }) .then((res) => { - assert.equal(res.length, 2, 'returns number of emails'); - assert.equal(res[1].email, secondEmail, 'returns correct email'); - assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[1].verified, false, 'returns correct verified'); - return client.createEmail(thirdEmail); + // the email is not verified, so it should not be returned yet + assert.equal(res.length, 1, 'returns number of emails'); + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + const templateName = emailData['headers']['x-template-name']; + const emailCode = emailData['headers']['x-verify-code']; + assert.equal(templateName, 'verifySecondaryCode'); + assert.ok(emailCode, 'emailCode set'); + return client.verifySecondaryEmailWithCode(emailCode, secondEmail); }) .then((res) => { assert.ok(res, 'ok response'); return client.accountEmails(); }) .then((res) => { - assert.equal(res.length, 3, 'returns number of emails'); - assert.equal(res[2].email, thirdEmail, 'returns correct email'); - assert.equal(res[2].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[2].verified, false, 'returns correct verified'); + assert.equal(res.length, 2, 'returns number of emails'); + assert.equal(res[1].email, secondEmail, 'returns correct email'); + assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); + assert.equal(res[1].verified, true, 'returns correct verified'); }) .catch((err) => { assert.fail(err); }); }); - it('can create email if email is unverified on another account', () => { + it('can create account with an email that is an unverified secondary email on another account', async () => { let client2; - const clientEmail = server.uniqueEmail(); const secondEmail = server.uniqueEmail(); + // create an unverified secondary email on the first account by seeding the db + const db = await setupAccountDatabase(cfg.database.mysql.auth); + try { + await db + .insertInto('emails') + .values({ + email: secondEmail, + normalizedEmail: emailHelper.helpers.normalizeEmail(secondEmail), + uid: Buffer.from(client.uid, 'hex'), + emailCode: Buffer.from( + crypto.randomBytes(16).toString('hex'), + 'hex' + ), + isVerified: 0, + isPrimary: 0, + createdAt: Date.now(), + }) + .execute(); + } finally { + await db.destroy(); + } + return client - .createEmail(secondEmail) + .accountEmails() .then((res) => { - assert.ok(res, 'ok response'); - return server.mailbox.waitForEmail(secondEmail); + assert.equal(res.length, 2, 'returns number of emails'); + assert.equal(res[1].email, secondEmail, 'returns correct email'); + assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); + assert.equal(res[1].verified, false, 'returns correct verified'); + return Client.createAndVerify( + config.publicUrl, + secondEmail, + password, + server.mailbox, + testOptions + ).catch(assert.fail); }) - .then(() => { + .then((x) => { + client2 = x; + assert.equal( + client2.email, + secondEmail, + 'account created with first account unverified secondary email' + ); return client.accountEmails(); }) + .then((res) => { + // Secondary email on first account should have been deleted + assert.equal(res.length, 1, 'returns number of emails'); + assert.equal(res[0].email, client.email, 'returns correct email'); + assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); + assert.equal(res[0].verified, true, 'returns correct verified'); + }); + }); + + it('can transfer an unverified secondary email from one account to another', async () => { + let client2; + const clientEmail = server.uniqueEmail(); + const secondEmail = server.uniqueEmail(); + // create an unverified secondary email on the first account by seeding the db + const db = await setupAccountDatabase(cfg.database.mysql.auth); + try { + await db + .insertInto('emails') + .values({ + email: secondEmail, + normalizedEmail: emailHelper.helpers.normalizeEmail(secondEmail), + uid: Buffer.from(client.uid, 'hex'), + emailCode: Buffer.from( + crypto.randomBytes(16).toString('hex'), + 'hex' + ), + isVerified: 0, + isPrimary: 0, + createdAt: Date.now(), + }) + .execute(); + } finally { + await db.destroy(); + } + + return client + .accountEmails() .then((res) => { assert.equal(res.length, 2, 'returns number of emails'); assert.equal(res[1].email, secondEmail, 'returns correct email'); @@ -123,6 +204,16 @@ const password = 'allyourbasearebelongtous'; ); return client2.createEmail(secondEmail); }) + .then(() => { + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + const templateName = emailData['headers']['x-template-name']; + const emailCode = emailData['headers']['x-verify-code']; + assert.equal(templateName, 'verifySecondaryCode'); + assert.ok(emailCode, 'emailCode set'); + return client2.verifySecondaryEmailWithCode(emailCode, secondEmail); + }) .then((res) => { assert.ok(res, 'ok response'); return client.accountEmails(); @@ -140,7 +231,7 @@ const password = 'allyourbasearebelongtous'; assert.equal(res.length, 2, 'returns number of emails'); assert.equal(res[1].email, secondEmail, 'returns correct email'); assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[1].verified, false, 'returns correct verified'); + assert.equal(res[1].verified, true, 'returns correct verified'); }); }); @@ -163,6 +254,16 @@ const password = 'allyourbasearebelongtous'; const secondEmail = server.uniqueEmail(); return client .createEmail(secondEmail) + .then(() => { + return server.mailbox.waitForEmail(secondEmail); + }) + .then((emailData) => { + const templateName = emailData['headers']['x-template-name']; + const emailCode = emailData['headers']['x-verify-code']; + assert.equal(templateName, 'verifySecondaryCode'); + assert.ok(emailCode, 'emailCode set'); + return client.verifySecondaryEmailWithCode(emailCode, secondEmail); + }) .then((res) => { assert.ok(res, 'ok response'); return client.createEmail(secondEmail); @@ -451,9 +552,43 @@ const password = 'allyourbasearebelongtous'; .then((emailData) => { const templateName = emailData['headers']['x-template-name']; assert.equal(templateName, 'postVerifySecondary'); - - // Create a third email but don't verify it. This should not appear in the cc-list - return client.createEmail(thirdEmail); + }) + .then(async () => { + // Create a third email but don't verify it (legacy unverified email) + // This should not appear in the cc-list + const db = await setupAccountDatabase(cfg.database.mysql.auth); + try { + await db + .insertInto('emails') + .values({ + email: thirdEmail, + normalizedEmail: + emailHelper.helpers.normalizeEmail(thirdEmail), + uid: Buffer.from(client.uid, 'hex'), + emailCode: Buffer.from( + crypto.randomBytes(16).toString('hex'), + 'hex' + ), + isVerified: 0, + isPrimary: 0, + createdAt: Date.now(), + }) + .execute(); + } finally { + await db.destroy(); + } + }) + .then(() => { + return client.accountEmails(); + }) + .then((res) => { + assert.equal(res.length, 3, 'returns number of emails'); + assert.equal(res[2].email, thirdEmail, 'returns correct email'); + assert.equal(res[2].isPrimary, false, 'returns correct isPrimary'); + assert.equal(res[2].verified, false, 'returns correct verified'); + }) + .catch((err) => { + assert.fail(err); }); }); @@ -554,7 +689,6 @@ const password = 'allyourbasearebelongtous'; it('receives password reset notification', () => { return client - .forgotPassword() .then(() => { return server.mailbox.waitForEmail(email); @@ -605,7 +739,7 @@ const password = 'allyourbasearebelongtous'; return client.accountEmails(); }) .then((res) => { - // Emails maintain there verification status through the password reset + // Email addresses maintain their verification status after password reset assert.equal(res.length, 3, 'returns number of emails'); assert.equal(res[1].email, secondEmail, 'returns correct email'); assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); @@ -794,11 +928,10 @@ const password = 'allyourbasearebelongtous'; }); }); - describe('should handle account creation', () => { + describe('verified secondary email', () => { let secondEmail; - let emailCode; - beforeEach(() => { + beforeEach(async () => { secondEmail = server.uniqueEmail(); return client .createEmail(secondEmail) @@ -807,13 +940,10 @@ const password = 'allyourbasearebelongtous'; return server.mailbox.waitForEmail(secondEmail); }) .then((emailData) => { - emailCode = emailData['headers']['x-verify-code']; - }); - }); - - it('fails to create account using verified secondary email', () => { - return client - .verifySecondaryEmailWithCode(emailCode, secondEmail) + const emailCode = emailData['headers']['x-verify-code']; + assert.ok(emailCode, 'emailCode set'); + return client.verifySecondaryEmailWithCode(emailCode, secondEmail); + }) .then((res) => { assert.ok(res, 'ok response'); return client.accountEmails(); @@ -823,55 +953,20 @@ const password = 'allyourbasearebelongtous'; assert.equal(res[1].email, secondEmail, 'returns correct email'); assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); assert.equal(res[1].verified, true, 'returns correct verified'); - return server.mailbox.waitForEmail(email); - }) - .then(() => { - return Client.create( - config.publicUrl, - secondEmail, - password, - testOptions - ) - .then(assert.fail) - .catch((err) => { - assert.equal(err.errno, 144, 'return correct errno'); - assert.equal(err.code, 400, 'return correct code'); - }); }); }); - it('should create account with unverified secondary email and delete unverified secondary email', () => { - let client2; - return client - .accountEmails() - .then((res) => { - assert.equal(res.length, 2, 'returns number of emails'); - assert.equal(res[1].email, secondEmail, 'returns correct email'); - assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[1].verified, false, 'returns correct verified'); - return Client.createAndVerify( - config.publicUrl, - secondEmail, - password, - server.mailbox, - testOptions - ).catch(assert.fail); - }) - .then((x) => { - client2 = x; - assert.equal( - client2.email, - secondEmail, - 'account created with secondary email address' - ); - return client.accountEmails(); - }) - .then((res) => { - // Secondary email on first account should have been deleted - assert.equal(res.length, 1, 'returns number of emails'); - assert.equal(res[0].email, client.email, 'returns correct email'); - assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); - assert.equal(res[0].verified, true, 'returns correct verified'); + it('cannot be used to create a new account', () => { + return Client.create( + config.publicUrl, + secondEmail, + password, + testOptions + ) + .then(assert.fail) + .catch((err) => { + assert.equal(err.errno, 144, 'return correct errno'); + assert.equal(err.code, 400, 'return correct code'); }); }); }); @@ -880,15 +975,8 @@ const password = 'allyourbasearebelongtous'; let secondEmail; beforeEach(async () => { secondEmail = server.uniqueEmail(); - let res = await client.createEmail(secondEmail, 'email-otp'); - + const res = await client.createEmail(secondEmail); assert.ok(res, 'ok response'); - res = await client.accountEmails(); - - assert.equal(res.length, 2, 'returns number of emails'); - assert.equal(res[1].email, secondEmail, 'returns correct email'); - assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); - assert.equal(res[1].verified, false, 'returns correct verified'); }); it('can verify using a code', async () => { @@ -915,11 +1003,6 @@ const password = 'allyourbasearebelongtous'; }); it('does not verify on random email code', async () => { - const emailData = await server.mailbox.waitForEmail(secondEmail); - const templateName = emailData['headers']['x-template-name']; - const emailCode = emailData['headers']['x-verify-code']; - assert.equal(templateName, 'verifySecondaryCode'); - assert.ok(emailCode, 'emailCode set'); let failed = false; try { await client.verifySecondaryEmailWithCode('123123', secondEmail); @@ -933,29 +1016,88 @@ const password = 'allyourbasearebelongtous'; assert.fail('should have failed'); } }); + }); + + // These tests cover legacy secondary emails that are stored in the database as unverified records + // These tests will no longer apply once we have cleaned out the old unverified records + // See FXA-10083 for more details + describe('(legacy) unverified secondary email', async () => { + let secondEmail; + beforeEach(async () => { + secondEmail = server.uniqueEmail(); + const db = await setupAccountDatabase(cfg.database.mysql.auth); + const emailCode = Buffer.from( + crypto.randomBytes(16).toString('hex'), + 'hex' + ); + try { + await db + .insertInto('emails') + .values({ + email: secondEmail, + normalizedEmail: emailHelper.helpers.normalizeEmail(secondEmail), + uid: Buffer.from(client.uid, 'hex'), + emailCode, + isVerified: 0, + isPrimary: 0, + createdAt: Date.now(), + }) + .execute(); + } finally { + await db.destroy(); + } + }); + + it('is deleted from the initial account if the email is verified on another account', async () => { + let client2; + return client + .accountEmails() + .then((res) => { + assert.equal(res.length, 2, 'returns number of emails'); + assert.equal(res[1].email, secondEmail, 'returns correct email'); + assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); + assert.equal(res[1].verified, false, 'returns correct verified'); + return Client.createAndVerify( + config.publicUrl, + secondEmail, + password, + server.mailbox, + testOptions + ).catch(assert.fail); + }) + .then((x) => { + client2 = x; + assert.equal( + client2.email, + secondEmail, + 'account created with secondary email address' + ); + return client.accountEmails(); + }) + .then((res) => { + // Secondary email on first account should have been deleted + assert.equal(res.length, 1, 'returns number of emails'); + assert.equal(res[0].email, client.email, 'returns correct email'); + assert.equal(res[0].isPrimary, true, 'returns correct isPrimary'); + assert.equal(res[0].verified, true, 'returns correct verified'); + }); + }); it('can resend verify email code', async () => { - let emailData = await server.mailbox.waitForEmail(secondEmail); - let templateName = emailData['headers']['x-template-name']; - const emailCode = emailData['headers']['x-verify-code']; - assert.equal(templateName, 'verifySecondaryCode'); - assert.ok(emailCode, 'emailCode set'); - assert.equal(emailCode.length, 6); - client.options.email = secondEmail; - let res = await client.resendVerifySecondaryEmailWithCode(secondEmail); + let res = await client.resendVerifySecondaryEmailWithCode( + secondEmail, + 'email-otp' + ); assert.ok(res, 'ok response'); - emailData = await server.mailbox.waitForEmail(secondEmail); + const emailData = await server.mailbox.waitForEmail(secondEmail); - templateName = emailData['headers']['x-template-name']; + const templateName = emailData['headers']['x-template-name']; const resendEmailCode = emailData['headers']['x-verify-code']; assert.equal(templateName, 'verifySecondaryCode'); - assert.equal(resendEmailCode, emailCode, 'emailCode matches'); - res = await client.verifySecondaryEmailWithCode(emailCode, secondEmail); - - assert.ok(res, 'ok response'); + assert.equal(resendEmailCode.length, 6, 'emailCode length is 6'); + await client.verifySecondaryEmailWithCode(resendEmailCode, secondEmail); res = await client.accountEmails(); - assert.equal(res.length, 2, 'returns number of emails'); assert.equal(res[1].email, secondEmail, 'returns correct email'); assert.equal(res[1].isPrimary, false, 'returns correct isPrimary'); diff --git a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx index d5f9e29b432..beb8f895a7a 100644 --- a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx +++ b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx @@ -3,8 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import React, { ChangeEvent, useCallback, useRef, useState } from 'react'; -import { Localized, useLocalization } from '@fluent/react'; import { RouteComponentProps } from '@reach/router'; +import { FtlMsg } from 'fxa-react/lib/utils'; import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery'; import { logViewEvent, usePageViewEvent } from '../../../lib/metrics'; import { SETTINGS_PATH } from '../../../constants'; @@ -12,13 +12,13 @@ import InputText from '../../InputText'; import FlowContainer from '../FlowContainer'; import { isEmailMask, isEmailValid } from 'fxa-shared/email/helpers'; import { useAccount, useAlertBar } from 'fxa-settings/src/models'; -import { AuthUiErrorNos } from 'fxa-settings/src/lib/auth-errors/auth-errors'; -import { getErrorFtlId } from '../../../lib/error-utils'; import { MfaGuard } from '../MfaGuard'; import { useErrorHandler } from 'react-error-boundary'; import VerifiedSessionGuard from '../VerifiedSessionGuard'; import { isInvalidJwtError } from '../../../lib/mfa-guard-utils'; import { MfaReason } from '../../../lib/types'; +import { useFtlMsgResolver } from '../../../models/hooks'; +import { getLocalizedErrorMessage } from '../../../lib/error-utils'; export const PageSecondaryEmailAdd = (_: RouteComponentProps) => { usePageViewEvent('settings.emails'); @@ -27,11 +27,10 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => { const [errorText, setErrorText] = useState(); const [email, setEmail] = useState(); const inputRefDOM = useRef(null); - const { l10n } = useLocalization(); + const ftlMsgResolver = useFtlMsgResolver(); - const subtitleText = l10n.getString( + const subtitleText = ftlMsgResolver.getMsg( 'add-secondary-email-step-1', - null, 'Step 1 of 2' ); const navigateWithQuery = useNavigateWithQuery(); @@ -53,24 +52,29 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => { return; } if (e.errno) { - const errorText = l10n.getString( - getErrorFtlId(e), - { retryAfter: e.retryAfterLocalized }, - AuthUiErrorNos[e.errno].message + const localizedErrorMessage = getLocalizedErrorMessage( + ftlMsgResolver, + e ); - setErrorText(errorText); + setErrorText(localizedErrorMessage); } else { alertBar.error( - l10n.getString( + ftlMsgResolver.getMsg( 'add-secondary-email-error-2', - null, 'There was a problem creating this email' ) ); } } }, - [account, navigateWithQuery, setErrorText, alertBar, l10n, errorHandler] + [ + account, + navigateWithQuery, + setErrorText, + alertBar, + errorHandler, + ftlMsgResolver, + ] ); const checkEmail = useCallback( @@ -83,20 +87,19 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => { setErrorText(''); if (isEmailMask(email)) { - const errorText = l10n.getString( + const errorText = ftlMsgResolver.getMsg( 'add-secondary-email-mask', - null, 'Email masks can’t be used as a secondary email' ); setErrorText(errorText); setSaveBtnDisabled(true); } }, - [setSaveBtnDisabled, setErrorText, l10n] + [setSaveBtnDisabled, setErrorText, ftlMsgResolver] ); return ( - +
{ }} >
- @@ -120,11 +123,11 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => { inputRefDOM={inputRefDOM} {...{ errorText }} /> - +
- + - - + + - +
- + ); }; diff --git a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx index 321445c1c1f..e39eb11e897 100644 --- a/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx +++ b/packages/fxa-settings/src/components/Settings/PageSecondaryEmailVerify/index.tsx @@ -12,7 +12,10 @@ import { useAccount, useAlertBar } from '../../../models'; import InputText from '../../InputText'; import FlowContainer from '../FlowContainer'; import { useForm } from 'react-hook-form'; -import { AuthUiErrors } from 'fxa-settings/src/lib/auth-errors/auth-errors'; +import { + AuthUiErrors, + AuthUiErrorNos, +} from 'fxa-settings/src/lib/auth-errors/auth-errors'; import { getErrorFtlId } from '../../../lib/error-utils'; import { MfaGuard } from '../MfaGuard'; import { useErrorHandler } from 'react-error-boundary'; @@ -75,11 +78,10 @@ export const PageSecondaryEmailVerify = ({ location }: RouteComponentProps) => { return; } if (e.errno) { - const errorText = l10n.getString( - getErrorFtlId(e), - null, - AuthUiErrors.INVALID_VERIFICATION_CODE.message - ); + const fallback = AuthUiErrorNos[e.errno] + ? AuthUiErrorNos[e.errno].message + : AuthUiErrors.INVALID_VERIFICATION_CODE.message; + const errorText = l10n.getString(getErrorFtlId(e), null, fallback); setErrorText(errorText); } else { alertBar.error( diff --git a/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.test.tsx b/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.test.tsx index a458d6c45fe..01e32952d3f 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.test.tsx @@ -30,7 +30,7 @@ jest.mock('../../../lib/cache', () => ({ const account = { emails: [mockEmail(), mockEmail('johndope2@example.com', false, false)], - resendEmailCode: jest.fn().mockResolvedValue(true), + resendSecondaryEmailCode: jest.fn().mockResolvedValue(true), makeEmailPrimaryWithJwt: jest.fn().mockResolvedValue(true), deleteSecondaryEmail: jest.fn().mockResolvedValue(true), refresh: jest.fn(), @@ -237,7 +237,7 @@ describe('UnitRowSecondaryEmail', () => { ]; const account = { emails, - resendEmailCode: jest.fn().mockResolvedValue(true), + resendSecondaryEmailCode: jest.fn().mockResolvedValue(true), } as unknown as Account; const { history } = renderWithRouter( @@ -262,7 +262,7 @@ describe('UnitRowSecondaryEmail', () => { ]; const account = { emails, - resendEmailCode: jest.fn().mockRejectedValue(new Error()), + resendSecondaryEmailCode: jest.fn().mockRejectedValue(new Error()), } as unknown as Account; const context = mockAppContext({ account }); const settingsContext = mockSettingsContext(); diff --git a/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx b/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx index 8ac1b636b91..b17b4a4b3f0 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx @@ -48,10 +48,10 @@ export const UnitRowSecondaryEmail = () => { .map((email) => email.verified) .lastIndexOf(true); - const resendEmailCode = useCallback( + const resendSecondaryEmailCode = useCallback( async (email: string) => { try { - await account.resendEmailCode(email); + await account.resendSecondaryEmailCode(email); navigateWithQuery(`${SETTINGS_PATH}/emails/verify`, { state: { email }, }); @@ -264,7 +264,7 @@ export const UnitRowSecondaryEmail = () => { className="link-blue" data-testid="secondary-email-resend-code-button" onClick={() => { - resendEmailCode(email); + resendSecondaryEmailCode(email); }} /> ), @@ -276,7 +276,7 @@ export const UnitRowSecondaryEmail = () => { className="link-blue" data-testid="secondary-email-resend-code-button" onClick={() => { - resendEmailCode(email); + resendSecondaryEmailCode(email); }} > Resend confirmation code diff --git a/packages/fxa-settings/src/lib/auth-errors/auth-errors.ts b/packages/fxa-settings/src/lib/auth-errors/auth-errors.ts index 30f6a09be96..fa2ea7c139b 100644 --- a/packages/fxa-settings/src/lib/auth-errors/auth-errors.ts +++ b/packages/fxa-settings/src/lib/auth-errors/auth-errors.ts @@ -195,7 +195,8 @@ const ERRORS = { RESEND_EMAIL_CODE_TO_UNOWNED_EMAIL: { errno: 150, message: - 'Can not change primary email to an email that does not belong to this account', + 'Can not resend email code to an email that does not belong to this account', + version: 2, }, FAILED_TO_SEND_EMAIL: { errno: 151, @@ -323,6 +324,14 @@ const ERRORS = { message: 'This phone number has been registered with too many accounts. Please try a different number.', }, + TOTP_SECRET_DOES_NOT_EXIST: { + errno: 220, + message: 'TOTP secret does not exist', + }, + EMAIL_IN_USE_BY_ANOTHER_ACCOUNT: { + errno: 221, + message: 'This email is already in use by another account.', + }, SERVICE_UNAVAILABLE: { errno: 998, message: 'System unavailable, try again soon', diff --git a/packages/fxa-settings/src/models/Account.ts b/packages/fxa-settings/src/models/Account.ts index b0024059fe3..9108ff2a0f5 100644 --- a/packages/fxa-settings/src/models/Account.ts +++ b/packages/fxa-settings/src/models/Account.ts @@ -1106,22 +1106,6 @@ export class Account implements AccountData { await this.withLoadingStatus( this.authClient.recoveryEmailCreate(jwt, email) ); - const cache = this.apolloClient.cache; - cache.modify({ - id: cache.identify({ __typename: 'Account' }), - fields: { - emails(existingEmails) { - return [ - ...existingEmails, - { - email, - isPrimary: false, - verified: false, - }, - ]; - }, - }, - }); } async verifySecondaryEmail(email: string, code: string) { @@ -1129,17 +1113,7 @@ export class Account implements AccountData { await this.withLoadingStatus( this.authClient.recoveryEmailSecondaryVerifyCode(jwt, email, code) ); - const cache = this.apolloClient.cache; - cache.modify({ - id: cache.identify({ __typename: 'Account' }), - fields: { - emails(existingEmails) { - return existingEmails.map((x: Email) => - x.email === email ? { ...x, verified: true } : { ...x } - ); - }, - }, - }); + await this.refresh('account'); } async disableTwoStepAuth() { @@ -1298,7 +1272,7 @@ export class Account implements AccountData { firefox.profileChanged({ uid: this.uid }); } - async resendEmailCode(email: string) { + async resendSecondaryEmailCode(email: string) { return this.withLoadingStatus( this.authClient.recoveryEmailSecondaryResendCode(sessionToken()!, email) ); diff --git a/packages/fxa-shared/lib/errors.ts b/packages/fxa-shared/lib/errors.ts index db2cecbb8f1..6ca4d9e8de6 100644 --- a/packages/fxa-shared/lib/errors.ts +++ b/packages/fxa-shared/lib/errors.ts @@ -141,6 +141,8 @@ export const AUTH_SERVER_ERRNOS = { TOTP_SECRET_DOES_NOT_EXIST: 220, + EMAIL_IN_USE_BY_ANOTHER_ACCOUNT: 221, + INTERNAL_VALIDATION_ERROR: 998, UNEXPECTED_ERROR: 999, };