From 36190b9c4e9967dedd406f62fef576c17ad608b0 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Tue, 4 Mar 2025 09:21:46 +0000 Subject: [PATCH] Merge pull request #23763 from overleaf/td-bs5-user-activation Migrate user activation page to BS5 GitOrigin-RevId: 3f4c9173cf480aafc5d874bfe81bbc8503a2e5be --- .../app/src/UserActivateController.mjs | 121 ++++++++-------- .../user-activate/app/views/user/activate.pug | 129 +++++++++--------- .../unit/src/UserActivateControllerTests.mjs | 38 ++++-- 3 files changed, 153 insertions(+), 135 deletions(-) diff --git a/services/web/modules/user-activate/app/src/UserActivateController.mjs b/services/web/modules/user-activate/app/src/UserActivateController.mjs index 644fae42db..e8ce258ba7 100644 --- a/services/web/modules/user-activate/app/src/UserActivateController.mjs +++ b/services/web/modules/user-activate/app/src/UserActivateController.mjs @@ -3,68 +3,67 @@ import { fileURLToPath } from 'node:url' import UserGetter from '../../../../app/src/Features/User/UserGetter.js' import UserRegistrationHandler from '../../../../app/src/Features/User/UserRegistrationHandler.js' import ErrorController from '../../../../app/src/Features/Errors/ErrorController.js' +import { expressify } from '@overleaf/promise-utils' const __dirname = Path.dirname(fileURLToPath(import.meta.url)) -export default { - registerNewUser(req, res, next) { - res.render(Path.resolve(__dirname, '../views/user/register')) - }, - - register(req, res, next) { - const { email } = req.body - if (email == null || email === '') { - return res.sendStatus(422) // Unprocessable Entity - } - UserRegistrationHandler.registerNewUserAndSendActivationEmail( - email, - (error, user, setNewPasswordUrl) => { - if (error != null) { - return next(error) - } - res.json({ - email: user.email, - setNewPasswordUrl, - }) - } - ) - }, - - activateAccountPage(req, res, next) { - // An 'activation' is actually just a password reset on an account that - // was set with a random password originally. - if (req.query.user_id == null || req.query.token == null) { - return ErrorController.notFound(req, res) - } - - if (typeof req.query.user_id !== 'string') { - return ErrorController.forbidden(req, res) - } - - UserGetter.getUser( - req.query.user_id, - { email: 1, loginCount: 1 }, - (error, user) => { - if (error != null) { - return next(error) - } - if (!user) { - return ErrorController.notFound(req, res) - } - if (user.loginCount > 0) { - // Already seen this user, so account must be activate - // This lets users keep clicking the 'activate' link in their email - // as a way to log in which, if I know our users, they will. - res.redirect(`/login`) - } else { - req.session.doLoginAfterPasswordReset = true - res.render(Path.resolve(__dirname, '../views/user/activate'), { - title: 'activate_account', - email: user.email, - token: req.query.token, - }) - } - } - ) - }, +function registerNewUser(req, res, next) { + res.render(Path.resolve(__dirname, '../views/user/register')) +} + +async function register(req, res, next) { + const { email } = req.body + if (email == null || email === '') { + return res.sendStatus(422) // Unprocessable Entity + } + const { user, setNewPasswordUrl } = + await UserRegistrationHandler.promises.registerNewUserAndSendActivationEmail( + email + ) + res.json({ + email: user.email, + setNewPasswordUrl, + }) +} + +async function activateAccountPage(req, res, next) { + // An 'activation' is actually just a password reset on an account that + // was set with a random password originally. + if (req.query.user_id == null || req.query.token == null) { + return ErrorController.notFound(req, res) + } + + if (typeof req.query.user_id !== 'string') { + return ErrorController.forbidden(req, res) + } + + const user = await UserGetter.promises.getUser(req.query.user_id, { + email: 1, + loginCount: 1, + }) + + if (!user) { + return ErrorController.notFound(req, res) + } + + if (user.loginCount > 0) { + // Already seen this user, so account must be activated. + // This lets users keep clicking the 'activate' link in their email + // as a way to log in which, if I know our users, they will. + return res.redirect(`/login`) + } + + req.session.doLoginAfterPasswordReset = true + + res.render(Path.resolve(__dirname, '../views/user/activate'), { + title: 'activate_account', + email: user.email, + token: req.query.token, + }) +} + +export default { + registerNewUser, + register: expressify(register), + activateAccountPage: expressify(activateAccountPage), } diff --git a/services/web/modules/user-activate/app/views/user/activate.pug b/services/web/modules/user-activate/app/views/user/activate.pug index 6a268fd07e..20ccb4dd8e 100644 --- a/services/web/modules/user-activate/app/views/user/activate.pug +++ b/services/web/modules/user-activate/app/views/user/activate.pug @@ -1,67 +1,72 @@ -extends ../../../../../app/views/layout-marketing +extends ../../../../../app/views/layout-website-redesign-bootstrap-5 block content main.content.content-alt#main-content .container - .row - .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 - .alert.alert-success #{translate("nearly_activated")} - .row - .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 - .card - .page-header - h1 #{translate("please_set_a_password")} - form( - data-ol-async-form - name="activationForm", - action="/user/password/set", - method="POST", + div.col-lg-6.col-xl-4.m-auto + .notification-list + .notification.notification-type-success(aria-live="off" role="alert") + .notification-content-and-cta + .notification-icon + span.material-symbols(aria-hidden="true") + | check_circle + .notification-content + p + | #{translate("nearly_activated")} + + h1.h3 #{translate("please_set_a_password")} + + form( + data-ol-async-form + name="activationForm", + action="/user/password/set", + method="POST", + ) + +formMessages() + + +customFormMessage('token-expired', 'danger') + | #{translate("activation_token_expired")} + + +customFormMessage('invalid-password', 'danger') + | #{translate('invalid_password')} + + input(name='_csrf', type='hidden', value=csrfToken) + input( + type="hidden", + name="passwordResetToken", + value=token + ) + + .form-group + label(for='emailField') #{translate("email")} + input.form-control#emailField( + aria-label="email", + type='email', + name='email', + placeholder="email@example.com", + autocomplete="username" + value=email + required, + disabled ) - +formMessages() - - +customFormMessage('token-expired', 'danger') - | #{translate("activation_token_expired")} - - +customFormMessage('invalid-password', 'danger') - | #{translate('invalid_password')} - - input(name='_csrf', type='hidden', value=csrfToken) - input( - type="hidden", - name="passwordResetToken", - value=token - ) - - .form-group - label(for='email') #{translate("email")} - input.form-control( - aria-label="email", - type='email', - name='email', - placeholder="email@example.com", - autocomplete="username" - value=email - required, - readonly - ) - .form-group - label(for='password') #{translate("password")} - input.form-control#passwordField( - type='password', - name='password', - placeholder="********", - autocomplete="new-password", - autofocus, - required, - minlength=settings.passwordStrengthOptions.length.min - ) - .actions - button.btn.btn-primary( - type='submit', - data-ol-disabled-inflight - aria-label=translate('activate') - ) - span(data-ol-inflight="idle") - | #{translate('activate')} - span(hidden data-ol-inflight="pending") - | #{translate('activating')}… + .form-group + label(for='passwordField') #{translate("password")} + input.form-control#passwordField( + type='password', + name='password', + placeholder="********", + autocomplete="new-password", + autofocus, + required, + minlength=settings.passwordStrengthOptions.length.min + ) + .actions + button.btn.btn-primary( + type='submit', + data-ol-disabled-inflight + aria-label=translate('activate') + ) + span(data-ol-inflight="idle") + | #{translate('activate')} + span(hidden data-ol-inflight="pending") + | #{translate('activating')}… diff --git a/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.mjs b/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.mjs index 8e4f37e886..7c4382a720 100644 --- a/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.mjs +++ b/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.mjs @@ -17,15 +17,26 @@ describe('UserActivateController', function () { email: 'joe@example.com', } - this.UserGetter = { getUser: sinon.stub() } - this.UserRegistrationHandler = {} + this.UserGetter = { + promises: { + getUser: sinon.stub(), + }, + } + this.UserRegistrationHandler = { promises: {} } this.ErrorController = { notFound: sinon.stub() } + this.SplitTestHandler = { + promises: { + getAssignment: sinon.stub().resolves({ variant: 'default' }), + }, + } this.UserActivateController = await esmock(MODULE_PATH, { '../../../../../app/src/Features/User/UserGetter.js': this.UserGetter, '../../../../../app/src/Features/User/UserRegistrationHandler.js': this.UserRegistrationHandler, '../../../../../app/src/Features/Errors/ErrorController.js': this.ErrorController, + '../../../../../app/src/Features/SplitTests/SplitTestHandler': + this.SplitTestHandler, }) this.req = { body: {}, @@ -41,12 +52,12 @@ describe('UserActivateController', function () { describe('activateAccountPage', function () { beforeEach(function () { - this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user) + this.UserGetter.promises.getUser = sinon.stub().resolves(this.user) this.req.query.user_id = this.user_id this.req.query.token = this.token = 'mock-token-123' }) - it('should 404 without a user_id', function (done) { + it('should 404 without a user_id', async function (done) { delete this.req.query.user_id this.ErrorController.notFound = () => done() this.UserActivateController.activateAccountPage(this.req, this.res) @@ -59,7 +70,7 @@ describe('UserActivateController', function () { }) it('should 404 without a valid user_id', function (done) { - this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) + this.UserGetter.promises.getUser = sinon.stub().resolves(null) this.ErrorController.notFound = () => done() this.UserActivateController.activateAccountPage(this.req, this.res) }) @@ -73,7 +84,7 @@ describe('UserActivateController', function () { it('should redirect activated users to login', function (done) { this.user.loginCount = 1 this.res.redirect = url => { - this.UserGetter.getUser.calledWith(this.user_id).should.equal(true) + sinon.assert.calledWith(this.UserGetter.promises.getUser, this.user_id) url.should.equal('/login') return done() } @@ -93,17 +104,20 @@ describe('UserActivateController', function () { }) describe('register', function () { - beforeEach(function () { - this.UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon - .stub() - .callsArgWith(1, null, this.user, (this.url = 'mock/url')) + beforeEach(async function () { + this.UserRegistrationHandler.promises.registerNewUserAndSendActivationEmail = + sinon.stub().resolves({ + user: this.user, + setNewPasswordUrl: (this.url = 'mock/url'), + }) this.req.body.email = this.user.email = this.email = 'email@example.com' - this.UserActivateController.register(this.req, this.res) + await this.UserActivateController.register(this.req, this.res) }) it('should register the user and send them an email', function () { sinon.assert.calledWith( - this.UserRegistrationHandler.registerNewUserAndSendActivationEmail, + this.UserRegistrationHandler.promises + .registerNewUserAndSendActivationEmail, this.email ) })