From f19b603b337e0d82ad8c7b6299928ec21a07b577 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Thu, 17 Jul 2025 12:53:55 +0200 Subject: [PATCH] [web] In tests, update email confirmation to use the 6-digits flow (#27069) * Rename `sendExistingSecondaryEmailConfirmationCode` to `sendExistingEmailConfirmationCode` * Deduplicate error handling into `throwIfErrorResponse` * Update `userHelper.confirmEmail` to use the 6-digits flow GitOrigin-RevId: 91bdf7b185407b58520ca6b2aa1a7c71bdd23bc8 --- .../src/Features/User/UserEmailsController.js | 6 +- services/web/app/src/router.mjs | 2 +- .../acceptance/src/helpers/UserHelper.mjs | 115 ++++++------------ .../src/User/UserEmailsControllerTests.js | 10 +- 4 files changed, 48 insertions(+), 85 deletions(-) diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js index e4ad7423af..2f9be5427e 100644 --- a/services/web/app/src/Features/User/UserEmailsController.js +++ b/services/web/app/src/Features/User/UserEmailsController.js @@ -141,7 +141,7 @@ async function sendReconfirmation(req, res) { res.sendStatus(204) } -async function sendExistingSecondaryEmailConfirmationCode(req, res) { +async function sendExistingEmailConfirmationCode(req, res) { const userId = SessionManager.getLoggedInUserId(req.session) const email = EmailHelper.parseEmail(req.body.email) if (!email) { @@ -715,8 +715,8 @@ const UserEmailsController = { sendReconfirmation: expressify(sendReconfirmation), - sendExistingSecondaryEmailConfirmationCode: expressify( - sendExistingSecondaryEmailConfirmationCode + sendExistingEmailConfirmationCode: expressify( + sendExistingEmailConfirmationCode ), addSecondaryEmailPage: expressify(addSecondaryEmailPage), diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 7851a4a66f..e727fa7bc5 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -345,7 +345,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { '/user/emails/send-confirmation-code', AuthenticationController.requireLogin(), RateLimiterMiddleware.rateLimit(rateLimiters.sendConfirmation), - UserEmailsController.sendExistingSecondaryEmailConfirmationCode + UserEmailsController.sendExistingEmailConfirmationCode ) webRouter.post( diff --git a/services/web/test/acceptance/src/helpers/UserHelper.mjs b/services/web/test/acceptance/src/helpers/UserHelper.mjs index cfeafed47c..951dccaf2b 100644 --- a/services/web/test/acceptance/src/helpers/UserHelper.mjs +++ b/services/web/test/acceptance/src/helpers/UserHelper.mjs @@ -7,7 +7,6 @@ import UserGetter from '../../../../app/src/Features/User/UserGetter.js' import UserUpdater from '../../../../app/src/Features/User/UserUpdater.js' import moment from 'moment' import fetch from 'node-fetch' -import { db } from '../../../../app/src/infrastructure/mongodb.js' import mongodb from 'mongodb-legacy' import { UserAuditLogEntry } from '../../../../app/src/models/UserAuditLogEntry.js' @@ -19,11 +18,20 @@ import { RateLimiter } from '../../../../app/src/infrastructure/RateLimiter.js' const { ObjectId } = mongodb const rateLimiters = { - resendConfirmation: new RateLimiter('resend-confirmation'), + sendConfirmation: new RateLimiter('send-confirmation'), } let globalUserNum = Settings.test.counterInit +const throwIfErrorResponse = async response => { + if (response.status < 200 || response.status >= 300) { + const body = await response.text() + throw new Error( + `request failed: status=${response.status} body=${JSON.stringify(body)}` + ) + } +} + class UserHelper { /** * Create UserHelper @@ -129,13 +137,7 @@ class UserHelper { // get csrf token from api and store const response = await this.fetch('/dev/csrf') const body = await response.text() - if (response.status !== 200) { - throw new Error( - `get csrf token failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } + await throwIfErrorResponse(response) this._csrfToken = body } @@ -145,14 +147,7 @@ class UserHelper { async getSession() { const response = await this.fetch('/dev/session') const body = await response.text() - - if (response.status !== 200) { - throw new Error( - `get session failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } + await throwIfErrorResponse(response) return JSON.parse(body) } @@ -160,24 +155,22 @@ class UserHelper { const response = await this.fetch( `/dev/split_test/get_assignment?splitTestName=${splitTestName}` ) + await throwIfErrorResponse(response) const body = await response.text() - - if (response.status !== 200) { - throw new Error( - `get split test assignment failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } return JSON.parse(body) } - async getEmailConfirmationCode() { + /** + * + * @param {'pendingExistingEmail'|'pendingUserRegistration'}sessionKey + * @return {Promise<*>} + */ + async getEmailConfirmationCode(sessionKey) { const session = await this.getSession() - const code = session.pendingUserRegistration?.confirmCode + const code = session[sessionKey]?.confirmCode if (!code) { - throw new Error('No confirmation code found in session') + throw new Error(`No confirmation code found in session (${sessionKey})`) } return code } @@ -383,14 +376,8 @@ class UserHelper { body: JSON.stringify(userData), ...options, }) + await throwIfErrorResponse(response) const body = await response.json() - if (response.status !== 200) { - throw new Error( - `register failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } if (body.message && body.message.type === 'error') { throw new Error(`register api error: ${body.message.text}`) } @@ -400,7 +387,9 @@ class UserHelper { ) } - const code = await userHelper.getEmailConfirmationCode() + const code = await userHelper.getEmailConfirmationCode( + 'pendingUserRegistration' + ) const confirmationResponse = await userHelper.fetch( '/registration/confirm-email', @@ -446,14 +435,7 @@ class UserHelper { method: 'POST', body: new URLSearchParams([['email', email]]), }) - const body = await response.text() - if (response.status !== 204) { - throw new Error( - `add email failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } + await throwIfErrorResponse(response) } async addEmailAndConfirm(userId, email) { @@ -519,40 +501,21 @@ class UserHelper { async confirmEmail(userId, email) { // clear ratelimiting on resend confirmation endpoint - await rateLimiters.resendConfirmation.delete(userId) - // UserHelper.createUser does not create a confirmation token - let response = await this.fetch('/user/emails/resend_confirmation', { + await rateLimiters.sendConfirmation.delete(userId) + const requestConfirmationCode = await this.fetch( + '/user/emails/send-confirmation-code', + { + method: 'POST', + body: new URLSearchParams({ email }), + } + ) + await throwIfErrorResponse(requestConfirmationCode) + const code = await this.getEmailConfirmationCode('pendingExistingEmail') + const requestConfirmCode = await this.fetch('/user/emails/confirm-code', { method: 'POST', - body: new URLSearchParams([['email', email]]), + body: new URLSearchParams({ code }), }) - if (response.status !== 200) { - const body = await response.text() - throw new Error( - `resend confirmation failed: status=${ - response.status - } body=${JSON.stringify(body)}` - ) - } - const tokenData = await db.tokens - .find({ - use: 'email_confirmation', - 'data.user_id': userId.toString(), - 'data.email': email, - usedAt: { $exists: false }, - }) - .next() - response = await this.fetch('/user/emails/confirm', { - method: 'POST', - body: new URLSearchParams([['token', tokenData.token]]), - }) - if (response.status !== 200) { - const body = await response.text() - throw new Error( - `confirm email failed: status=${response.status} body=${JSON.stringify( - body - )}` - ) - } + await throwIfErrorResponse(requestConfirmCode) } } diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js index b89df8821a..2cd7ad491f 100644 --- a/services/web/test/unit/src/User/UserEmailsControllerTests.js +++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js @@ -905,7 +905,7 @@ describe('UserEmailsController', function () { }) }) - describe('sendExistingSecondaryEmailConfirmationCode', function () { + describe('sendExistingEmailConfirmationCode', function () { beforeEach(function () { this.email = 'existing-email@example.com' this.req.body.email = this.email @@ -923,7 +923,7 @@ describe('UserEmailsController', function () { }) it('should send confirmation code for existing email', async function () { - await this.UserEmailsController.sendExistingSecondaryEmailConfirmationCode( + await this.UserEmailsController.sendExistingEmailConfirmationCode( this.req, { sendStatus: code => { @@ -944,7 +944,7 @@ describe('UserEmailsController', function () { this.UserEmailsConfirmationHandler.promises.sendConfirmationCode.resolves( { confirmCode, confirmCodeExpiresTimestamp } ) - await this.UserEmailsController.sendExistingSecondaryEmailConfirmationCode( + await this.UserEmailsController.sendExistingEmailConfirmationCode( this.req, { sendStatus: sinon.stub() } ) @@ -958,7 +958,7 @@ describe('UserEmailsController', function () { it('should handle invalid email', async function () { this.EmailHelper.parseEmail.returns(null) - await this.UserEmailsController.sendExistingSecondaryEmailConfirmationCode( + await this.UserEmailsController.sendExistingEmailConfirmationCode( this.req, { sendStatus: code => { @@ -975,7 +975,7 @@ describe('UserEmailsController', function () { this.UserGetter.promises.getUserByAnyEmail.resolves({ _id: 'another-user-id', }) - await this.UserEmailsController.sendExistingSecondaryEmailConfirmationCode( + await this.UserEmailsController.sendExistingEmailConfirmationCode( this.req, { sendStatus: code => {