From c4419d91d1ebf83e7849f3fc73b7febb8bc37b06 Mon Sep 17 00:00:00 2001 From: mark-cab <144702012+mark-cab@users.noreply.github.com> Date: Thu, 1 Feb 2024 16:06:44 +0000 Subject: [PATCH] mfa password reset required journey (#1293) --- .../common/state-machine/state-machine.ts | 46 +--- .../reset-password-controller.ts | 102 ++++---- .../tests/reset-password-controller.test.ts | 45 ++++ ...word-forced-2fa-before-integration.test.ts | 227 ++++++++++++++++++ 4 files changed, 328 insertions(+), 92 deletions(-) create mode 100644 src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts diff --git a/src/components/common/state-machine/state-machine.ts b/src/components/common/state-machine/state-machine.ts index ebe32498c..b3c6ae39c 100644 --- a/src/components/common/state-machine/state-machine.ts +++ b/src/components/common/state-machine/state-machine.ts @@ -588,51 +588,7 @@ const authStateMachine = createMachine( ], }, }, - [PATH_NAMES.RESET_PASSWORD_REQUIRED]: { - on: { - [USER_JOURNEY_EVENTS.PASSWORD_CREATED]: [ - { - target: [PATH_NAMES.GET_SECURITY_CODES], - cond: "isAccountPartCreated", - }, - { - target: [PATH_NAMES.ENTER_AUTHENTICATOR_APP_CODE], - cond: "requiresMFAAuthAppCode", - }, - { target: [PATH_NAMES.ENTER_MFA], cond: "requiresTwoFactorAuth" }, - { - target: [PATH_NAMES.UPDATED_TERMS_AND_CONDITIONS], - cond: "isLatestTermsAndConditionsAccepted", - }, - { - target: [PATH_NAMES.SHARE_INFO], - cond: "isConsentRequired", - }, - { target: [PATH_NAMES.AUTH_CODE] }, - ], - }, - [PATH_NAMES.CREATE_ACCOUNT_ENTER_PHONE_NUMBER]: { - on: { - [USER_JOURNEY_EVENTS.VERIFY_PHONE_NUMBER]: [ - PATH_NAMES.CHECK_YOUR_PHONE, - ], - }, - meta: { - optionalPaths: [ - PATH_NAMES.SECURITY_CODE_WAIT, - PATH_NAMES.SECURITY_CODE_INVALID, - PATH_NAMES.SECURITY_CODE_REQUEST_EXCEEDED, - ], - }, - }, - meta: { - optionalPaths: [ - PATH_NAMES.ENTER_EMAIL_SIGN_IN, - PATH_NAMES.ACCOUNT_LOCKED, - PATH_NAMES.SIGN_IN_OR_CREATE, - ], - }, - }, + [PATH_NAMES.RESET_PASSWORD_REQUIRED]: {}, [PATH_NAMES.PROVE_IDENTITY]: { on: { [USER_JOURNEY_EVENTS.PROVE_IDENTITY_INIT]: [ diff --git a/src/components/reset-password/reset-password-controller.ts b/src/components/reset-password/reset-password-controller.ts index 273333675..6129f2fe7 100644 --- a/src/components/reset-password/reset-password-controller.ts +++ b/src/components/reset-password/reset-password-controller.ts @@ -79,59 +79,67 @@ export function resetPasswordPost( ); } } - - const loginResponse = await loginService.loginUser( - sessionId, - email, - newPassword, - clientSessionId, - req.ip, - persistentSessionId - ); - - if (!loginResponse.success) { - throw new BadRequestError( - loginResponse.data.message, - loginResponse.data.code - ); - } - - req.session.user.redactedPhoneNumber = - loginResponse.data.redactedPhoneNumber; - req.session.user.isConsentRequired = loginResponse.data.consentRequired; - req.session.user.isLatestTermsAndConditionsAccepted = - loginResponse.data.latestTermsAndConditionsAccepted; - req.session.user.isAccountPartCreated = - !loginResponse.data.mfaMethodVerified; - if (req.session.user.isPasswordChangeRequired) { - req.session.user.isPasswordChangeRequired = false; - } - - if ( - !support2FABeforePasswordReset() && - loginResponse.data.mfaMethodVerified && - loginResponse.data.mfaMethodType === MFA_METHOD_TYPE.SMS - ) { - const mfaResponse = await mfaCodeService.sendMfaCode( + let mfaMethodType; + let isMfaMethodVerified; + if (support2FABeforePasswordReset && req.session.user.isAuthenticated) { + mfaMethodType = req.session.user.accountRecoveryVerifiedMfaType; + isMfaMethodVerified = !req.session.user.isAccountPartCreated; + } else { + const loginResponse = await loginService.loginUser( sessionId, - clientSessionId, email, + newPassword, + clientSessionId, req.ip, - persistentSessionId, - false, - xss(req.cookies.lng as string) + persistentSessionId ); - if (!mfaResponse.success) { - const path = getErrorPathByCode(mfaResponse.data.code); - if (path) { - return res.redirect(path); - } + if (!loginResponse.success) { throw new BadRequestError( - mfaResponse.data.message, - mfaResponse.data.code + loginResponse.data.message, + loginResponse.data.code + ); + } + + req.session.user.redactedPhoneNumber = + loginResponse.data.redactedPhoneNumber; + req.session.user.isConsentRequired = loginResponse.data.consentRequired; + req.session.user.isLatestTermsAndConditionsAccepted = + loginResponse.data.latestTermsAndConditionsAccepted; + req.session.user.isAccountPartCreated = + !loginResponse.data.mfaMethodVerified; + if (req.session.user.isPasswordChangeRequired) { + req.session.user.isPasswordChangeRequired = false; + } + + if ( + !support2FABeforePasswordReset() && + loginResponse.data.mfaMethodVerified && + loginResponse.data.mfaMethodType === MFA_METHOD_TYPE.SMS + ) { + const mfaResponse = await mfaCodeService.sendMfaCode( + sessionId, + clientSessionId, + email, + req.ip, + persistentSessionId, + false, + xss(req.cookies.lng as string) ); + + if (!mfaResponse.success) { + const path = getErrorPathByCode(mfaResponse.data.code); + if (path) { + return res.redirect(path); + } + throw new BadRequestError( + mfaResponse.data.message, + mfaResponse.data.code + ); + } } + mfaMethodType = loginResponse.data.mfaMethodType; + isMfaMethodVerified = loginResponse.data.mfaMethodVerified; } return res.redirect( @@ -144,8 +152,8 @@ export function resetPasswordPost( requiresTwoFactorAuth: !support2FABeforePasswordReset(), isLatestTermsAndConditionsAccepted: req.session.user.isLatestTermsAndConditionsAccepted, - mfaMethodType: loginResponse.data.mfaMethodType, - isMfaMethodVerified: loginResponse.data.mfaMethodVerified, + mfaMethodType: mfaMethodType, + isMfaMethodVerified: isMfaMethodVerified, support2FABeforePasswordReset: support2FABeforePasswordReset(), }, res.locals.sessionId diff --git a/src/components/reset-password/tests/reset-password-controller.test.ts b/src/components/reset-password/tests/reset-password-controller.test.ts index 46736f9ac..e2e7c8046 100644 --- a/src/components/reset-password/tests/reset-password-controller.test.ts +++ b/src/components/reset-password/tests/reset-password-controller.test.ts @@ -270,5 +270,50 @@ describe("reset password controller (in 6 digit code flow)", () => { expect(res.redirect).to.have.calledWith(PATH_NAMES.ENTER_MFA); }); + + it("should not request 2fa and not login user when user already logged in", async () => { + process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; + const fakeResetService: ResetPasswordServiceInterface = { + updatePassword: sinon.fake.returns({ success: true }), + } as unknown as ResetPasswordServiceInterface; + const fakeLoginService: EnterPasswordServiceInterface = { + loginUser: sinon.fake.returns({ + success: true, + data: { + redactedPhoneNumber: "1234", + consentRequired: false, + latestTermsAndConditionsAccepted: true, + mfaMethodVerified: true, + mfaRequired: false, + mfaMethodType: MFA_METHOD_TYPE.SMS, + passwordChangeRequired: params.passwordChangeRequired, + }, + }), + } as unknown as EnterPasswordServiceInterface; + fakeLoginService.loginUser; + const fakeMfAService: MfaServiceInterface = { + sendMfaCode: sinon.fake.returns({ success: true }), + } as unknown as MfaServiceInterface; + + req.session.user = { + email: "joe.bloggs@test.com", + isAuthenticated: true, + isAccountPartCreated: false, + accountRecoveryVerifiedMfaType: MFA_METHOD_TYPE.SMS, + }; + req.body.password = "Password1"; + + await resetPasswordPost( + fakeResetService, + fakeLoginService, + fakeMfAService + )(req as Request, res as Response); + + expect(fakeResetService.updatePassword).to.have.been.calledOnce; + expect(fakeLoginService.loginUser).to.not.have.been.called; + expect(fakeMfAService.sendMfaCode).to.not.have.been.called; + + expect(res.redirect).to.have.calledWith(PATH_NAMES.AUTH_CODE); + }); }); }); diff --git a/src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts b/src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts new file mode 100644 index 000000000..e710d067b --- /dev/null +++ b/src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts @@ -0,0 +1,227 @@ +import request from "supertest"; +import { describe } from "mocha"; +import { expect, sinon } from "../../../../test/utils/test-utils"; +import nock = require("nock"); +import * as cheerio from "cheerio"; +import { MFA_METHOD_TYPE, PATH_NAMES } from "../../../app.constants"; +import decache from "decache"; + +describe("Integration::reset password (in 2FA Before Reset Password flow)", () => { + let token: string | string[]; + let cookies: string; + let app: any; + let baseApi: string; + + const ENDPOINT = "/reset-password"; + + before(async () => { + process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; + decache("../../../app"); + decache("../../../middleware/session-middleware"); + const sessionMiddleware = require("../../../middleware/session-middleware"); + + sinon + .stub(sessionMiddleware, "validateSessionMiddleware") + .callsFake(function (req: any, res: any, next: any): void { + res.locals.sessionId = "tDy103saszhcxbQq0-mjdzU854"; + req.session.user = { + email: "test@test.com", + phoneNumber: "7867", + journey: { + nextPath: PATH_NAMES.RESET_PASSWORD, + }, + isAuthenticated: true, + isAccountPartCreated: false, + accountRecoveryVerifiedMfaType: MFA_METHOD_TYPE.SMS, + }; + + next(); + }); + app = await require("../../../app").createApp(); + baseApi = process.env.FRONTEND_API_BASE_URL; + + request(app) + .get(ENDPOINT) + .end((err, res) => { + const $ = cheerio.load(res.text); + token = $("[name=_csrf]").val(); + cookies = res.headers["set-cookie"]; + }); + }); + + beforeEach(() => { + nock.cleanAll(); + }); + + after(() => { + sinon.restore(); + app = undefined; + }); + + it("should return reset password page", (done) => { + request(app).get(ENDPOINT).expect(200, done); + }); + + it("should return error when csrf not present", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .send({ + password: "password", + }) + .expect(500, done); + }); + + it("should return validation error when password not entered", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "", + "confirm-password": "", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains("Enter your password"); + }) + .expect(400, done); + }); + + it("should return validation error when passwords don't match", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "sadsadasd33da", + "confirm-password": "sdnnsad99d", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#confirm-password-error").text()).to.contains( + "Enter the same password in both fields" + ); + }) + .expect(400, done); + }); + + it("should return validation error when password less than 8 characters", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "dad", + "confirm-password": "", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains( + "Your password must be at least 8 characters long and must include letters and numbers" + ); + }) + .expect(400, done); + }); + + it("should return validation error when password is amongst most common passwords", (done) => { + nock(baseApi).post("/reset-password").once().reply(400, { code: 1040 }); + + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "password123", + "confirm-password": "password123", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains( + "Enter a stronger password. Do not use very common passwords, such as ‘password’ or a sequence of numbers." + ); + }) + .expect(400, done); + }); + + it("should return error when new password is the same as existing password", (done) => { + nock(baseApi).post("/reset-password").once().reply(400, { code: 1024 }); + + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "p@ssw0rd-123", + "confirm-password": "p@ssw0rd-123", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains( + "You are already using that password. Enter a different password" + ); + }) + .expect(400, done); + }); + + it("should return validation error when no numbers present in password", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "testpassword", + "confirm-password": "testpassword", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains( + "Your password must be at least 8 characters long and must include letters and numbers" + ); + }) + .expect(400, done); + }); + + it("should return validation error when password all numeric", (done) => { + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "222222222222222", + "confirm-password": "222222222222222", + }) + .expect(function (res) { + const $ = cheerio.load(res.text); + expect($("#password-error").text()).to.contains( + "Your password must be at least 8 characters long and must include letters and numbers" + ); + }) + .expect(400, done); + }); + + it("should redirect to /auth-code when valid password entered", (done) => { + nock(baseApi).post("/reset-password").once().reply(204); + nock(baseApi).post("/login").once().reply(200); + nock(baseApi).post("/mfa").once().reply(204); + + request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "Testpassword1", + "confirm-password": "Testpassword1", + }) + .expect("Location", PATH_NAMES.AUTH_CODE) + .expect(302, done); + }); +});