From 0cf17478fed5fe0a65e17cfdd55b93758b694bf9 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:58:57 +0100 Subject: [PATCH] Merge pull request #17810 from overleaf/dp-compormised-password-prompt Add compromised password prompt GitOrigin-RevId: 7910a220943fcb3aa191da6d514d5bc3ae20f5a3 --- .../AuthenticationController.js | 17 +- .../Authentication/AuthenticationManager.js | 27 +-- .../app/src/Features/User/UserController.js | 8 +- .../src/Features/User/UserPagesController.js | 4 + services/web/app/src/router.js | 6 + services/web/app/views/layout-base.pug | 3 +- .../app/views/user/compromised_password.pug | 15 ++ .../web/frontend/extracted-translations.json | 4 + .../components/compromised-password-root.tsx | 46 +++++ .../js/pages/compromised-password.tsx | 12 ++ .../modules/overleaf-integration.less | 14 ++ services/web/locales/en.json | 3 + .../web/test/acceptance/src/CaptchaTests.js | 18 +- .../test/acceptance/src/UserHelperTests.js | 78 ++++---- .../AuthenticationManagerTests.js | 168 +++++++++++------- .../test/unit/src/User/UserControllerTests.js | 30 +++- 16 files changed, 327 insertions(+), 126 deletions(-) create mode 100644 services/web/app/views/user/compromised_password.pug create mode 100644 services/web/frontend/js/features/compromised-password/components/compromised-password-root.tsx create mode 100644 services/web/frontend/js/pages/compromised-password.tsx diff --git a/services/web/app/src/Features/Authentication/AuthenticationController.js b/services/web/app/src/Features/Authentication/AuthenticationController.js index 0b0158a90d..671a801afd 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationController.js +++ b/services/web/app/src/Features/Authentication/AuthenticationController.js @@ -178,6 +178,7 @@ const AuthenticationController = { const redir = AuthenticationController._getRedirectFromSession(req) || '/project' + _loginAsyncHandlers(req, user, anonymousAnalyticsId, isNewUser) const userId = user._id UserAuditLogHandler.addEntry( @@ -241,8 +242,10 @@ const AuthenticationController = { { email }, password, auditLog, - { skipHIBPCheck: fromKnownDevice }, - function (error, user) { + { + enforceHIBPCheck: !fromKnownDevice, + }, + function (error, user, isPasswordReused) { if (error != null) { if (error instanceof ParallelLoginError) { return done(null, false, { status: 429 }) @@ -278,6 +281,16 @@ const AuthenticationController = { status: 400, }) } else if (user) { + if ( + isPasswordReused && + AuthenticationController._getRedirectFromSession(req) == null + ) { + AuthenticationController.setRedirectInSession( + req, + '/compromised-password' + ) + } + // async actions done(null, user) } else { diff --git a/services/web/app/src/Features/Authentication/AuthenticationManager.js b/services/web/app/src/Features/Authentication/AuthenticationManager.js index 3d1b491f3a..33827f1673 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationManager.js +++ b/services/web/app/src/Features/Authentication/AuthenticationManager.js @@ -10,7 +10,10 @@ const { PasswordMustBeDifferentError, PasswordReusedError, } = require('./AuthenticationErrors') -const { callbackify } = require('util') +const { + callbackify, + callbackifyMultiResult, +} = require('@overleaf/promise-utils') const HaveIBeenPwned = require('./HaveIBeenPwned') const UserAuditLogHandler = require('../User/UserAuditLogHandler') const logger = require('@overleaf/logger') @@ -111,14 +114,14 @@ const AuthenticationManager = { return { user, match } }, - async authenticate(query, password, auditLog, { skipHIBPCheck = false }) { + async authenticate(query, password, auditLog, { enforceHIBPCheck = true }) { const { user, match } = await AuthenticationManager._checkUserPassword( query, password ) if (!user) { - return null + return { user: null } } const update = { $inc: { loginEpoch: 1 } } @@ -138,7 +141,7 @@ const AuthenticationManager = { if (!match) { if (!auditLog) { - return null + return { user: null } } else { try { await UserAuditLogHandler.promises.addEntry( @@ -154,16 +157,11 @@ const AuthenticationManager = { 'Error while adding AuditLog entry for failed-password-match' ) } - return null + return { user: null } } } await AuthenticationManager.checkRounds(user, user.hashedPassword, password) - if (skipHIBPCheck) { - HaveIBeenPwned.checkPasswordForReuseInBackground(password) - return user - } - let isPasswordReused try { isPasswordReused = @@ -172,11 +170,11 @@ const AuthenticationManager = { logger.err({ err }, 'cannot check password for re-use') } - if (isPasswordReused) { + if (isPasswordReused && enforceHIBPCheck) { throw new PasswordReusedError() } - return user + return { user, isPasswordReused } }, validateEmail(email) { @@ -471,7 +469,10 @@ module.exports = { validatePassword: AuthenticationManager.validatePassword, getMessageForInvalidPasswordError: AuthenticationManager.getMessageForInvalidPasswordError, - authenticate: callbackify(AuthenticationManager.authenticate), + authenticate: callbackifyMultiResult(AuthenticationManager.authenticate, [ + 'user', + 'isPasswordReused', + ]), setUserPassword: callbackify(AuthenticationManager.setUserPassword), checkRounds: callbackify(AuthenticationManager.checkRounds), hashPassword: callbackify(AuthenticationManager.hashPassword), diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index 12defe00cb..6d018da715 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -70,11 +70,11 @@ async function changePassword(req, res, next) { metrics.inc('user.password-change') const userId = SessionManager.getLoggedInUserId(req.session) - const user = await AuthenticationManager.promises.authenticate( + const { user } = await AuthenticationManager.promises.authenticate( { _id: userId }, req.body.currentPassword, null, - { skipHIBPCheck: true } + { enforceHIBPCheck: false } ) if (!user) { return HttpErrorHandler.badRequest( @@ -228,11 +228,11 @@ async function tryDeleteUser(req, res, next) { return res.sendStatus(403) } - const user = await AuthenticationManager.promises.authenticate( + const { user } = await AuthenticationManager.promises.authenticate( { _id: userId }, password, null, - { skipHIBPCheck: true } + { enforceHIBPCheck: false } ) if (!user) { logger.err({ userId }, 'auth failed during attempt to delete account') diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index ed2d5ce9d6..7d371c2bed 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -314,6 +314,10 @@ const UserPagesController = { ) }, + compromisedPasswordPage(_, res) { + res.render('user/compromised_password') + }, + _restructureThirdPartyIds(user) { // 3rd party identifiers are an array of objects // this turn them into a single object, which diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 8704a5c169..1da4608b03 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -224,6 +224,12 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { AuthenticationController.passportLogin ) + webRouter.get( + '/compromised-password', + AuthenticationController.requireLogin(), + UserPagesController.compromisedPasswordPage + ) + webRouter.get('/account-suspended', UserPagesController.accountSuspended) if (Settings.enableLegacyLogin) { diff --git a/services/web/app/views/layout-base.pug b/services/web/app/views/layout-base.pug index eeb79d09dc..2dde4d6eda 100644 --- a/services/web/app/views/layout-base.pug +++ b/services/web/app/views/layout-base.pug @@ -33,7 +33,8 @@ html( link(rel="preload", href=buildJsPath(currentLngCode + "-json.js"), as="script", nonce=scriptNonce) //- Scripts - include _google_analytics + if (typeof(suppressGoogleAnalytics) == "undefined") + include _google_analytics block meta meta(name="ol-csrfToken" content=csrfToken) diff --git a/services/web/app/views/user/compromised_password.pug b/services/web/app/views/user/compromised_password.pug new file mode 100644 index 0000000000..30353ede4b --- /dev/null +++ b/services/web/app/views/user/compromised_password.pug @@ -0,0 +1,15 @@ +extends ../layout-marketing + +block vars + - var suppressNavbar = true + - var suppressFooter = true + - var suppressGoogleAnalytics = true + +block entrypointVar + - entrypoint = 'pages/compromised-password' + +block append meta + +block content + main.content.content-alt + #compromised-password diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 611d34136c..df9a0d9ebd 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -190,6 +190,7 @@ "compile_terminated_by_user": "", "compiler": "", "compiling": "", + "compromised_password": "", "configure_sso": "", "confirm": "", "confirm_affiliation": "", @@ -480,6 +481,7 @@ "go_next_page": "", "go_page": "", "go_prev_page": "", + "go_to_account_settings": "", "go_to_code_location_in_pdf": "", "go_to_overleaf": "", "go_to_pdf_location_in_code": "", @@ -884,6 +886,7 @@ "plan_tooltip": "", "please_ask_the_project_owner_to_upgrade_to_track_changes": "", "please_change_primary_to_remove": "", + "please_change_your_password_by_going_to_your_account_settings": "", "please_check_your_inbox": "", "please_check_your_inbox_to_confirm": "", "please_compile_pdf_before_download": "", @@ -1566,6 +1569,7 @@ "your_git_access_tokens": "", "your_message_to_collaborators": "", "your_new_plan": "", + "your_password_was_detected": "", "your_plan": "", "your_plan_is_changing_at_term_end": "", "your_project_exceeded_compile_timeout_limit_on_free_plan": "", diff --git a/services/web/frontend/js/features/compromised-password/components/compromised-password-root.tsx b/services/web/frontend/js/features/compromised-password/components/compromised-password-root.tsx new file mode 100644 index 0000000000..8104042fc6 --- /dev/null +++ b/services/web/frontend/js/features/compromised-password/components/compromised-password-root.tsx @@ -0,0 +1,46 @@ +import useWaitForI18n from '@/shared/hooks/use-wait-for-i18n' +import { Button } from 'react-bootstrap' +import { Trans, useTranslation } from 'react-i18next' + +export function CompromisedPasswordCard() { + const { t } = useTranslation() + const { isReady } = useWaitForI18n() + + if (!isReady) { + return null + } + + return ( +
+
+

+ {t('compromised_password')} +

+

+ , + ]} + /> +

+

+ ]} + /> +

+
+ + +
+ ) +} diff --git a/services/web/frontend/js/pages/compromised-password.tsx b/services/web/frontend/js/pages/compromised-password.tsx new file mode 100644 index 0000000000..b18502780c --- /dev/null +++ b/services/web/frontend/js/pages/compromised-password.tsx @@ -0,0 +1,12 @@ +import '../marketing' + +import ReactDOM from 'react-dom' +import { CompromisedPasswordCard } from '../features/compromised-password/components/compromised-password-root' + +const compromisedPasswordContainer = document.getElementById( + 'compromised-password' +) + +if (compromisedPasswordContainer) { + ReactDOM.render(, compromisedPasswordContainer) +} diff --git a/services/web/frontend/stylesheets/modules/overleaf-integration.less b/services/web/frontend/stylesheets/modules/overleaf-integration.less index f995ac11cb..ca534b2725 100644 --- a/services/web/frontend/stylesheets/modules/overleaf-integration.less +++ b/services/web/frontend/stylesheets/modules/overleaf-integration.less @@ -154,3 +154,17 @@ margin-bottom: 100px; } } + +.compromised-password { + max-width: 400px; + padding: 24px; + margin: 0 auto; + background: @white; + display: flex; + flex-direction: column; + gap: 16px; + + .compromised-password-header { + margin-top: 0; + } +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 062f52ca91..87a76e2ff0 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -284,6 +284,7 @@ "compiling": "Compiling", "complete": "Complete", "compliance": "Compliance", + "compromised_password": "Compromised Password", "configure_sso": "Configure SSO", "configured": "Configured", "confirm": "Confirm", @@ -1319,6 +1320,7 @@ "plans_and_pricing": "Plans and Pricing", "please_ask_the_project_owner_to_upgrade_to_track_changes": "Please ask the project owner to upgrade to use track changes", "please_change_primary_to_remove": "Please change your primary email in order to remove", + "please_change_your_password_by_going_to_your_account_settings": "Please change your password by going to your <0>Account Settings.", "please_check_your_inbox": "Please check your inbox", "please_check_your_inbox_to_confirm": "Please check your email inbox to confirm your <0>__institutionName__ affiliation.", "please_compile_pdf_before_download": "Please compile your project before downloading the PDF", @@ -2208,6 +2210,7 @@ "your_message_to_collaborators": "Send a message to your collaborators", "your_new_plan": "Your new plan", "your_password_has_been_successfully_changed": "Your password has been successfully changed", + "your_password_was_detected": "Your password was detected on a <0>public list of known compromised passwords. This makes your account vulnerable.", "your_plan": "Your plan", "your_plan_is_changing_at_term_end": "Your plan is changing to <0>__pendingPlanName__ at the end of the current billing period.", "your_project_exceeded_compile_timeout_limit_on_free_plan": "Your project exceeded the compile timeout limit on our free plan.", diff --git a/services/web/test/acceptance/src/CaptchaTests.js b/services/web/test/acceptance/src/CaptchaTests.js index 0deb554e7e..a7282aa9fc 100644 --- a/services/web/test/acceptance/src/CaptchaTests.js +++ b/services/web/test/acceptance/src/CaptchaTests.js @@ -57,6 +57,14 @@ describe('Captcha', function () { expect(body).to.deep.equal({ redir: '/project' }) } + function expectSuccessfulLoginWithRedirectToCompromisedPasswordPage( + response, + body + ) { + expect(response.statusCode).to.equal(200) + expect(body).to.deep.equal({ redir: '/compromised-password' }) + } + function expectBadLogin(response, body) { expect(response.statusCode).to.equal(401) expect(body).to.deep.equal({ @@ -226,12 +234,18 @@ describe('Captcha', function () { }) it('should be able to skip HIBP check with deviceHistory and valid captcha', async function () { const { response, body } = await loginWithCaptcha('valid') - expectSuccessfulLogin(response, body) + expectSuccessfulLoginWithRedirectToCompromisedPasswordPage( + response, + body + ) }) it('should be able to skip HIBP check with deviceHistory and skipped captcha', async function () { const { response, body } = await loginWithCaptcha('') - expectSuccessfulLogin(response, body) + expectSuccessfulLoginWithRedirectToCompromisedPasswordPage( + response, + body + ) }) it('should not be able to skip HIBP check without deviceHistory', async function () { diff --git a/services/web/test/acceptance/src/UserHelperTests.js b/services/web/test/acceptance/src/UserHelperTests.js index cb5037044e..930db68227 100644 --- a/services/web/test/acceptance/src/UserHelperTests.js +++ b/services/web/test/acceptance/src/UserHelperTests.js @@ -16,12 +16,13 @@ describe('UserHelper', function () { it('should create new user with default username and password', async function () { const userHelper = await UserHelper.createUser() userHelper.user.email.should.equal(userHelper.getDefaultEmail()) - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - userHelper.getDefaultPassword(), - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + userHelper.getDefaultPassword(), + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) @@ -32,12 +33,13 @@ describe('UserHelper', function () { email: 'foo@test.com', }) userHelper.user.email.should.equal('foo@test.com') - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - userHelper.getDefaultPassword(), - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + userHelper.getDefaultPassword(), + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) @@ -48,12 +50,13 @@ describe('UserHelper', function () { password: 'foofoofoo', }) userHelper.user.email.should.equal(userHelper.getDefaultEmail()) - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - 'foofoofoo', - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + 'foofoofoo', + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) @@ -128,12 +131,13 @@ describe('UserHelper', function () { it('should create new user with default username and password', async function () { const userHelper = await UserHelper.registerUser() userHelper.user.email.should.equal(userHelper.getDefaultEmail()) - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - userHelper.getDefaultPassword(), - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + userHelper.getDefaultPassword(), + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) @@ -144,12 +148,13 @@ describe('UserHelper', function () { email: 'foo2@test.com', }) userHelper.user.email.should.equal('foo2@test.com') - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - userHelper.getDefaultPassword(), - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + userHelper.getDefaultPassword(), + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) @@ -160,12 +165,13 @@ describe('UserHelper', function () { password: 'foofoofoo', }) userHelper.user.email.should.equal(userHelper.getDefaultEmail()) - const authedUser = await AuthenticationManager.promises.authenticate( - { _id: userHelper.user._id }, - 'foofoofoo', - null, - { skipHIBPCheck: true } - ) + const { user: authedUser } = + await AuthenticationManager.promises.authenticate( + { _id: userHelper.user._id }, + 'foofoofoo', + null, + { enforceHIBPCheck: false } + ) expect(authedUser).to.not.be.null }) }) diff --git a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js index 866882d3e5..72fc14b98b 100644 --- a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js +++ b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js @@ -81,12 +81,13 @@ describe('AuthenticationManager', function () { describe('when the hashed password matches', function () { beforeEach(async function () { this.unencryptedPassword = 'testpassword' - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should look up the correct user in the database', function () { @@ -119,12 +120,13 @@ describe('AuthenticationManager', function () { describe('when the encrypted passwords do not match', function () { beforeEach(async function () { - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - 'notthecorrectpassword', - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + 'notthecorrectpassword', + null, + { enforceHIBPCheck: false } + )) }) it('should persist the login failure and bump epoch', function () { @@ -159,7 +161,7 @@ describe('AuthenticationManager', function () { { email: this.email }, 'testpassword', null, - { skipHIBPCheck: true } + { enforceHIBPCheck: false } ) ).to.be.rejectedWith(AuthenticationErrors.ParallelLoginError) }) @@ -177,7 +179,7 @@ describe('AuthenticationManager', function () { { email: this.email }, 'notthecorrectpassword', null, - { skipHIBPCheck: true } + { enforceHIBPCheck: false } ) ).to.be.rejectedWith(AuthenticationErrors.ParallelLoginError) }) @@ -249,12 +251,13 @@ describe('AuthenticationManager', function () { this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf' this.bcrypt.compare = sinon.stub().resolves(true) this.bcrypt.getRounds = sinon.stub().returns(4) - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should look up the correct user in the database', function () { @@ -280,22 +283,62 @@ describe('AuthenticationManager', function () { }) describe('HIBP', function () { - it('should not check HIBP if not requested', function () { - this.HaveIBeenPwned.promises.checkPasswordForReuse.should.not.have - .been.called + it('should enforce HIBP if requested', async function () { + this.HaveIBeenPwned.promises.checkPasswordForReuse.resolves(true) + + await expect( + this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: true } + ) + ).to.be.rejectedWith(AuthenticationErrors.PasswordReusedError) }) - it('should check HIBP if requested', async function () { - await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: false } - ) + it('should check but not enforce HIBP if not requested', async function () { + this.HaveIBeenPwned.promises.checkPasswordForReuse.resolves(true) + + const { user } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + ) this.HaveIBeenPwned.promises.checkPasswordForReuse.should.have.been.calledWith( this.unencryptedPassword ) + expect(user).to.equal(this.user) + }) + + it('should report password reused when check not enforced', async function () { + this.HaveIBeenPwned.promises.checkPasswordForReuse.resolves(true) + + const { isPasswordReused } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + ) + + expect(isPasswordReused).to.equal(true) + }) + + it('should report password not reused when check not enforced', async function () { + this.HaveIBeenPwned.promises.checkPasswordForReuse.resolves(false) + + const { isPasswordReused } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + ) + + expect(isPasswordReused).to.equal(false) }) }) }) @@ -304,12 +347,13 @@ describe('AuthenticationManager', function () { beforeEach(async function () { this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf' this.bcrypt.compare = sinon.stub().resolves(false) - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should not return the user', function () { @@ -323,12 +367,13 @@ describe('AuthenticationManager', function () { this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf' this.bcrypt.compare = sinon.stub().resolves(false) this.auditLog = { ipAddress: 'ip', info: { method: 'foo' } } - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - this.auditLog, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + this.auditLog, + { enforceHIBPCheck: false } + )) }) it('should not return the user, but add entry to audit log', function () { @@ -354,12 +399,13 @@ describe('AuthenticationManager', function () { this.AuthenticationManager.promises._setUserPasswordInMongo = sinon .stub() .resolves() - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should look up the correct user in the database', function () { @@ -400,12 +446,13 @@ describe('AuthenticationManager', function () { this.AuthenticationManager.promises.setUserPassword = sinon .stub() .resolves() - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencryptedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencryptedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should not check the number of rounds', function () { @@ -433,12 +480,13 @@ describe('AuthenticationManager', function () { this.User.findOne = sinon .stub() .returns({ exec: sinon.stub().resolves(null) }) - this.result = await this.AuthenticationManager.promises.authenticate( - { email: this.email }, - this.unencrpytedPassword, - null, - { skipHIBPCheck: true } - ) + ;({ user: this.result } = + await this.AuthenticationManager.promises.authenticate( + { email: this.email }, + this.unencrpytedPassword, + null, + { enforceHIBPCheck: false } + )) }) it('should not return a user', function () { diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index df6961450c..5c0d9f5e3d 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -178,7 +178,9 @@ describe('UserController', function () { this.SessionManager.getLoggedInUserId = sinon .stub() .returns(this.user._id) - this.AuthenticationManager.promises.authenticate.resolves(this.user) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) }) it('should send 200', function (done) { @@ -246,7 +248,9 @@ describe('UserController', function () { describe('when authenticate does not produce a user', function () { beforeEach(function () { - this.AuthenticationManager.promises.authenticate.resolves(null) + this.AuthenticationManager.promises.authenticate.resolves({ + user: null, + }) }) it('should return 403', function (done) { @@ -696,7 +700,9 @@ describe('UserController', function () { describe('changePassword', function () { describe('success', function () { beforeEach(function () { - this.AuthenticationManager.promises.authenticate.resolves(this.user) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) this.AuthenticationManager.promises.setUserPassword.resolves() this.req.body = { newPassword1: 'newpass', @@ -759,7 +765,7 @@ describe('UserController', function () { describe('errors', function () { it('should check the old password is the current one at the moment', function (done) { - this.AuthenticationManager.promises.authenticate.resolves() + this.AuthenticationManager.promises.authenticate.resolves({}) this.req.body = { currentPassword: 'oldpasshere' } this.HttpErrorHandler.badRequest.callsFake(() => { expect(this.HttpErrorHandler.badRequest).to.have.been.calledWith( @@ -780,7 +786,9 @@ describe('UserController', function () { }) it('it should not set the new password if they do not match', function (done) { - this.AuthenticationManager.promises.authenticate.resolves({}) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) this.req.body = { newPassword1: '1', newPassword2: '2', @@ -813,7 +821,9 @@ describe('UserController', function () { message ) this.AuthenticationManager.promises.setUserPassword.rejects(err) - this.AuthenticationManager.promises.authenticate.resolves({}) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) this.req.body = { newPassword1: 'newpass', newPassword2: 'newpass', @@ -831,7 +841,9 @@ describe('UserController', function () { describe('UserAuditLogHandler error', function () { it('should return error and not update password', function (done) { this.UserAuditLogHandler.promises.addEntry.rejects(new Error('oops')) - this.AuthenticationManager.promises.authenticate.resolves(this.user) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) this.AuthenticationManager.promises.setUserPassword.resolves() this.req.body = { newPassword1: 'newpass', @@ -851,7 +863,9 @@ describe('UserController', function () { describe('EmailHandler error', function () { const anError = new Error('oops') beforeEach(function () { - this.AuthenticationManager.promises.authenticate.resolves(this.user) + this.AuthenticationManager.promises.authenticate.resolves({ + user: this.user, + }) this.AuthenticationManager.promises.setUserPassword.resolves() this.req.body = { newPassword1: 'newpass',