diff --git a/services/web/app/src/Features/Authentication/AuthenticationManager.js b/services/web/app/src/Features/Authentication/AuthenticationManager.js index 7dd4d2d62e..7e730c2484 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationManager.js +++ b/services/web/app/src/Features/Authentication/AuthenticationManager.js @@ -402,6 +402,48 @@ const AuthenticationManager = { } } }, + + getMessageForInvalidPasswordError(error, req) { + const errorCode = error?.info?.code + const message = { + type: 'error', + } + switch (errorCode) { + case 'not_set': + message.key = 'password-not-set' + message.text = req.i18n.translate('invalid_password_not_set') + break + case 'invalid_character': + message.key = 'password-invalid-character' + message.text = req.i18n.translate('invalid_password_invalid_character') + break + case 'contains_email': + message.key = 'password-contains-email' + message.text = req.i18n.translate('invalid_password_contains_email') + break + case 'too_similar': + message.key = 'password-too-similar' + message.text = req.i18n.translate('invalid_password_too_similar') + break + case 'too_short': + message.key = 'password-too-short' + message.text = req.i18n.translate('invalid_password_too_short', { + minLength: Settings.passwordStrengthOptions?.length?.min || 8, + }) + break + case 'too_long': + message.key = 'password-too-long' + message.text = req.i18n.translate('invalid_password_too_long', { + maxLength: Settings.passwordStrengthOptions?.length?.max || 72, + }) + break + default: + logger.error({ err: error }, 'Unknown password validation error code') + message.text = req.i18n.translate('invalid_password') + break + } + return message + }, } AuthenticationManager.promises = { diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.js b/services/web/app/src/Features/PasswordReset/PasswordResetController.js index 4b79967b6e..805fd3600a 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.js +++ b/services/web/app/src/Features/PasswordReset/PasswordResetController.js @@ -22,23 +22,11 @@ async function setNewUserPassword(req, res, next) { const err = AuthenticationManager.validatePassword(password, email) if (err) { - if (err?.info?.code === 'contains_email') { - return res.status(400).json({ - message: { - text: req.i18n.translate('invalid_password_contains_email'), - }, - }) - } else if (err?.info?.code === 'too_similar') { - return res.status(400).json({ - message: { - text: req.i18n.translate('invalid_password_too_similar'), - }, - }) - } else { - return res.status(400).json({ - message: { text: err.message }, - }) - } + const message = AuthenticationManager.getMessageForInvalidPasswordError( + err, + req + ) + return res.status(400).json({ message }) } passwordResetToken = passwordResetToken.trim() diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index bd545b0529..1c41b47a6a 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -96,21 +96,11 @@ async function changePassword(req, res, next) { ) } catch (error) { if (error.name === 'InvalidPasswordError') { - if (error?.info?.code === 'contains_email') { - return HttpErrorHandler.badRequest( - req, - res, - req.i18n.translate('invalid_password_contains_email') - ) - } else if (error?.info?.code === 'too_similar') { - return HttpErrorHandler.badRequest( - req, - res, - req.i18n.translate('invalid_password_too_similar') - ) - } else { - return HttpErrorHandler.badRequest(req, res, error.message) - } + const message = AuthenticationManager.getMessageForInvalidPasswordError( + error, + req + ) + return res.status(400).json({ message }) } else if (error.name === 'PasswordMustBeDifferentError') { return HttpErrorHandler.badRequest( req, diff --git a/services/web/app/views/user/setPassword.pug b/services/web/app/views/user/setPassword.pug index 38f0abf1e3..469c681386 100644 --- a/services/web/app/views/user/setPassword.pug +++ b/services/web/app/views/user/setPassword.pug @@ -24,6 +24,14 @@ block content p(data-ol-hide-on-error-message="token-expired") #{translate("create_a_new_password_for_your_account")}. +formMessages() + +customFormMessage('password-contains-email', 'danger') + | #{translate('invalid_password_contains_email')}. + | #{translate('use_a_different_password')} + + +customFormMessage('password-too-similar', 'danger') + | #{translate('invalid_password_too_similar')}. + | #{translate('use_a_different_password')} + +customFormMessage('token-expired', 'danger') | #{translate('password_reset_token_expired')} br diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 5b5496619c..fe85a1caf6 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -410,6 +410,8 @@ "invalid_email": "", "invalid_file_name": "", "invalid_filename": "", + "invalid_password_contains_email": "", + "invalid_password_too_similar": "", "invalid_request": "", "invite_more_collabs": "", "invite_not_accepted": "", diff --git a/services/web/frontend/js/features/settings/components/password-section.tsx b/services/web/frontend/js/features/settings/components/password-section.tsx index 70620d13b0..a2ad1740a1 100644 --- a/services/web/frontend/js/features/settings/components/password-section.tsx +++ b/services/web/frontend/js/features/settings/components/password-section.tsx @@ -164,6 +164,16 @@ function PasswordForm() { /> . {t('use_a_different_password')} + ) : getErrorMessageKey(error) === 'password-contains-email' ? ( + <> + {t('invalid_password_contains_email')}.{' '} + {t('use_a_different_password')} + + ) : getErrorMessageKey(error) === 'password-too-similar' ? ( + <> + {t('invalid_password_too_similar')}.{' '} + {t('use_a_different_password')} + ) : ( getUserFacingMessage(error) )} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 010c0d0925..cff72d1dca 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -751,7 +751,7 @@ "invalid_password_not_set": "Password is required", "invalid_password_too_long": "Maximum password length __maxLength__ exceeded", "invalid_password_too_short": "Password too short, minimum __minLength__", - "invalid_password_too_similar": "Password is too similar to email address", + "invalid_password_too_similar": "Password is too similar to parts of email address", "invalid_request": "Invalid Request. Please correct the data and try again.", "invalid_zip_file": "Invalid zip file", "invite_more_collabs": "Invite more collaborators", diff --git a/services/web/test/acceptance/src/PasswordResetTests.js b/services/web/test/acceptance/src/PasswordResetTests.js index 8f401257bb..18555488d2 100644 --- a/services/web/test/acceptance/src/PasswordResetTests.js +++ b/services/web/test/acceptance/src/PasswordResetTests.js @@ -200,7 +200,11 @@ describe('PasswordReset', function () { expect(response.status).to.equal(400) const body = await response.json() expect(body).to.deep.equal({ - message: { text: 'Password cannot contain parts of email address' }, + message: { + type: 'error', + key: 'password-contains-email', + text: 'Password cannot contain parts of email address', + }, }) }) @@ -223,7 +227,11 @@ describe('PasswordReset', function () { expect(response.status).to.equal(400) const body = await response.json() expect(body).to.deep.equal({ - message: { text: 'Password is too similar to email address' }, + message: { + type: 'error', + key: 'password-too-similar', + text: 'Password is too similar to parts of email address', + }, }) }) diff --git a/services/web/test/acceptance/src/PasswordUpdateTests.js b/services/web/test/acceptance/src/PasswordUpdateTests.js index 67df502b9a..727c43f036 100644 --- a/services/web/test/acceptance/src/PasswordUpdateTests.js +++ b/services/web/test/acceptance/src/PasswordUpdateTests.js @@ -133,7 +133,43 @@ describe('PasswordUpdate', function () { }) it('should return error message', async function () { const body = await response.json() - expect(body.message).to.equal('password is too short') + expect(body.message).to.deep.equal({ + type: 'error', + key: 'password-too-short', + text: 'Password too short, minimum 8', + }) + }) + it('should not update audit log', async function () { + const auditLog = userHelper.getAuditLogWithoutNoise() + expect(auditLog).to.deep.equal([]) + }) + }) + describe('new password contains part of email', function () { + beforeEach(async function () { + response = await userHelper.fetch('/user/password/update', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + }, + body: JSON.stringify({ + currentPassword: password, + newPassword1: 'somecooluser123', + newPassword2: 'somecooluser123', + }), + }) + userHelper = await UserHelper.getUser({ email }) + }) + it('should return 400', async function () { + expect(response.status).to.equal(400) + }) + it('should return error message', async function () { + const body = await response.json() + expect(body.message).to.deep.equal({ + key: 'password-contains-email', + type: 'error', + text: 'Password cannot contain parts of email address', + }) }) it('should not update audit log', async function () { const auditLog = userHelper.getAuditLogWithoutNoise() @@ -161,9 +197,11 @@ describe('PasswordUpdate', function () { }) it('should return error message', async function () { const body = await response.json() - expect(body.message).to.equal( - 'Password is too similar to email address' - ) + expect(body.message).to.deep.equal({ + key: 'password-too-similar', + type: 'error', + text: 'Password is too similar to parts of email address', + }) }) it('should not update audit log', async function () { const auditLog = userHelper.getAuditLogWithoutNoise() diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index 01652fdce9..df108a8778 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -61,6 +61,9 @@ describe('UserController', function () { authenticate: sinon.stub(), setUserPassword: sinon.stub(), }, + getMessageForInvalidPasswordError: sinon + .stub() + .returns({ type: 'error', key: 'some-key' }), } this.UserUpdater = { changeEmailAddress: sinon.stub(), @@ -771,18 +774,21 @@ describe('UserController', function () { // .returns({ message: 'validation-error' }) const err = new Error('bad') err.name = 'InvalidPasswordError' + const message = { + type: 'error', + key: 'some-message-key', + } + this.AuthenticationManager.getMessageForInvalidPasswordError.returns( + message + ) this.AuthenticationManager.promises.setUserPassword.rejects(err) this.AuthenticationManager.promises.authenticate.resolves({}) this.req.body = { newPassword1: 'newpass', newPassword2: 'newpass', } - this.HttpErrorHandler.badRequest.callsFake(() => { - expect(this.HttpErrorHandler.badRequest).to.have.been.calledWith( - this.req, - this.res, - err.message - ) + this.res.json.callsFake(result => { + expect(result.message).to.deep.equal(message) this.AuthenticationManager.promises.setUserPassword.callCount.should.equal( 1 )