From 84456c5913671cb1ecaf85052d0f1ec3d15910ee Mon Sep 17 00:00:00 2001 From: BeckaL Date: Thu, 26 Sep 2024 16:15:35 +0100 Subject: [PATCH 01/13] BAU: Start removing 2fa before password reset feature flag logic from enter password controller This: * Moves 2fa before password reset tests from the enter-password-support-2fa-before-password-reset-integration.test.ts to enter-password-integration.test.ts * Deletes enter-password-support-2fa-before-password-reset-integration.test.ts (the behaviour is going to exist in all cases, so we no longer need a feature-flag specific integration test * Hard-codes the value we pass in to the state machine to true. Eventually this can be removed entirely from the state machine and all calls, but doing this as an interim step to avoid to many changes at once --- .../enter-password-controller.ts | 8 +- .../tests/enter-password-integration.test.ts | 47 +++- ...-before-password-reset-integration.test.ts | 208 ------------------ 3 files changed, 48 insertions(+), 215 deletions(-) delete mode 100644 src/components/enter-password/tests/enter-password-support-2fa-before-password-reset-integration.test.ts diff --git a/src/components/enter-password/enter-password-controller.ts b/src/components/enter-password/enter-password-controller.ts index 9b371bf42..fc2bfedb8 100644 --- a/src/components/enter-password/enter-password-controller.ts +++ b/src/components/enter-password/enter-password-controller.ts @@ -19,11 +19,7 @@ import { JOURNEY_TYPE, MFA_METHOD_TYPE, PATH_NAMES } from "../../app.constants"; import xss from "xss"; import { EnterEmailServiceInterface } from "../enter-email/types"; import { enterEmailService } from "../enter-email/enter-email-service"; -import { - support2FABeforePasswordReset, - support2hrLockout, - supportAccountInterventions, -} from "../../config"; +import { support2hrLockout, supportAccountInterventions } from "../../config"; import { getJourneyTypeFromUserSession } from "../common/journey/journey"; import { accountInterventionService } from "../account-intervention/account-intervention-service"; import { AccountInterventionsInterface } from "../account-intervention/types"; @@ -256,7 +252,7 @@ export function enterPasswordPost( mfaMethodType: userLogin.data.mfaMethodType, isMfaMethodVerified: userLogin.data.mfaMethodVerified, isPasswordChangeRequired: isPasswordChangeRequired, - support2FABeforePasswordReset: support2FABeforePasswordReset(), + support2FABeforePasswordReset: true, }, sessionId ) diff --git a/src/components/enter-password/tests/enter-password-integration.test.ts b/src/components/enter-password/tests/enter-password-integration.test.ts index aa6c30faa..0fa6cac68 100644 --- a/src/components/enter-password/tests/enter-password-integration.test.ts +++ b/src/components/enter-password/tests/enter-password-integration.test.ts @@ -6,6 +6,10 @@ import * as cheerio from "cheerio"; import decache from "decache"; import { API_ENDPOINTS, PATH_NAMES } from "../../../app.constants"; import { ERROR_CODES } from "../../common/constants"; +import { + noInterventions, + setupAccountInterventionsResponse, +} from "../../../../test/helpers/account-interventions-helpers"; describe("Integration::enter password", () => { let token: string | string[]; @@ -38,7 +42,6 @@ describe("Integration::enter password", () => { }); process.env.SUPPORT_REAUTHENTICATION = "0"; - app = await require("../../../app").createApp(); baseApi = process.env.FRONTEND_API_BASE_URL; @@ -127,6 +130,48 @@ describe("Integration::enter password", () => { .expect(302); }); + it("should redirect to /reset-password-2fa-sms when password is correct and user's MFA is set to SMS when 2FA is not required", async () => { + nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).once().reply(200, { + mfaRequired: false, + mfaMethodType: "SMS", + passwordChangeRequired: true, + }); + + setupAccountInterventionsResponse(baseApi, noInterventions); + + await request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "password", + }) + .expect("Location", PATH_NAMES.RESET_PASSWORD_REQUIRED) + .expect(302); + }); + + it("should redirect to /reset-password-2fa-sms when password is correct and user's MFA is set to SMS when 2FA is required", async () => { + nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).once().reply(200, { + mfaRequired: true, + mfaMethodType: "SMS", + passwordChangeRequired: true, + }); + + setupAccountInterventionsResponse(baseApi, noInterventions); + + await request(app) + .post(ENDPOINT) + .type("form") + .set("Cookie", cookies) + .send({ + _csrf: token, + password: "password", + }) + .expect("Location", PATH_NAMES.RESET_PASSWORD_2FA_SMS) + .expect(302); + }); + it("should redirect to /account-locked from sign-in flow when incorrect password entered 5 times", async () => { nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).times(6).reply(400, { code: ERROR_CODES.INVALID_PASSWORD_MAX_ATTEMPTS_REACHED, diff --git a/src/components/enter-password/tests/enter-password-support-2fa-before-password-reset-integration.test.ts b/src/components/enter-password/tests/enter-password-support-2fa-before-password-reset-integration.test.ts deleted file mode 100644 index 1e4eb119a..000000000 --- a/src/components/enter-password/tests/enter-password-support-2fa-before-password-reset-integration.test.ts +++ /dev/null @@ -1,208 +0,0 @@ -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 decache from "decache"; -import { - API_ENDPOINTS, - HTTP_STATUS_CODES, - PATH_NAMES, -} from "../../../app.constants"; -import { ERROR_CODES } from "../../common/constants"; -import { AxiosResponse } from "axios"; -import { createApiResponse } from "../../../utils/http"; -import { CheckReauthServiceInterface } from "../../check-reauth-users/types"; -import { DefaultApiResponse } from "../../../types"; -import { - noInterventions, - setupAccountInterventionsResponse, -} from "../../../../test/helpers/account-interventions-helpers"; - -describe("Integration::enter password", () => { - let token: string | string[]; - let cookies: string; - let app: any; - let baseApi: string; - - const ENDPOINT = "/enter-password"; - - before(async () => { - decache("../../../app"); - decache("../../../middleware/session-middleware"); - const sessionMiddleware = require("../../../middleware/session-middleware"); - const checkReauthUsersService = require("../../check-reauth-users/check-reauth-users-service"); - - sinon - .stub(sessionMiddleware, "validateSessionMiddleware") - .callsFake(function (req: any, res: any, next: any): void { - res.locals.sessionId = "tDy103saszhcxbQq0-mjdzU854"; - res.locals.clientSessionId = "gdsfsfdsgsdgsd-mjdzU854"; - res.locals.persistentSessionId = "dips-123456-abc"; - - req.session.user = { - email: "test@test.com", - journey: { - nextPath: PATH_NAMES.ENTER_PASSWORD, - }, - enterEmailMfaType: "SMS", - isPasswordChangeRequired: true, - }; - - next(); - }); - - sinon - .stub(checkReauthUsersService, "checkReauthUsersService") - .callsFake((): CheckReauthServiceInterface => { - async function checkReauthUsers() { - const fakeAxiosResponse: AxiosResponse = { - status: HTTP_STATUS_CODES.OK, - } as AxiosResponse; - - return createApiResponse(fakeAxiosResponse); - } - - return { checkReauthUsers }; - }); - - app = await require("../../../app").createApp(); - baseApi = process.env.FRONTEND_API_BASE_URL; - - await request(app) - .get(ENDPOINT) - .then((res) => { - const $ = cheerio.load(res.text); - token = $("[name=_csrf]").val(); - cookies = res.headers["set-cookie"]; - }); - }); - - beforeEach(() => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; - process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; - nock.cleanAll(); - }); - - after(() => { - sinon.restore(); - app = undefined; - delete process.env.SUPPORT_ACCOUNT_INTERVENTIONS; - delete process.env.SUPPORT_2FA_B4_PASSWORD_RESET; - }); - - it("should return enter password page", async () => { - await request(app).get(ENDPOINT).expect(200); - }); - - it("should return enter password page when support reauthentication flag is on and check reauth users api call is successfull", async () => { - process.env.SUPPORT_REAUTHENTICATION = "1"; - await request(app).get(ENDPOINT).expect(200); - }); - - it("should return error when csrf not present", async () => { - await request(app) - .post(ENDPOINT) - .type("form") - .send({ - password: "password", - }) - .expect(403); - }); - - it("should return validation error when password not entered", async () => { - await request(app) - .post(ENDPOINT) - .type("form") - .set("Cookie", cookies) - .send({ - _csrf: token, - password: "", - }) - .expect(function (res) { - const $ = cheerio.load(res.text); - expect($("#password-error").text()).to.contains("Enter your password"); - }) - .expect(400); - }); - - it("should return validation error when password is incorrect", async () => { - nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).once().reply(401); - process.env.SUPPORT_2HR_LOCKOUT = "0"; - - await request(app) - .post(ENDPOINT) - .type("form") - .set("Cookie", cookies) - .send({ - _csrf: token, - password: "pasasd", - }) - .expect(function (res) { - const $ = cheerio.load(res.text); - expect($("#password-error").text()).to.contains( - "Enter the correct password" - ); - }) - .expect(400); - }); - - it("should redirect to /reset-password-2fa-sms when password is correct and user's MFA is set to SMS when 2FA is not required", async () => { - nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).once().reply(200, { - mfaRequired: false, - mfaMethodType: "SMS", - passwordChangeRequired: true, - }); - - setupAccountInterventionsResponse(baseApi, noInterventions); - - await request(app) - .post(ENDPOINT) - .type("form") - .set("Cookie", cookies) - .send({ - _csrf: token, - password: "password", - }) - .expect("Location", PATH_NAMES.RESET_PASSWORD_REQUIRED) - .expect(302); - }); - - it("should redirect to /reset-password-2fa-sms when password is correct and user's MFA is set to SMS when 2FA is required", async () => { - nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).once().reply(200, { - mfaRequired: true, - mfaMethodType: "SMS", - passwordChangeRequired: true, - }); - - setupAccountInterventionsResponse(baseApi, noInterventions); - - await request(app) - .post(ENDPOINT) - .type("form") - .set("Cookie", cookies) - .send({ - _csrf: token, - password: "password", - }) - .expect("Location", PATH_NAMES.RESET_PASSWORD_2FA_SMS) - .expect(302); - }); - - it("should redirect to /account-locked during sign in flow when incorrect password entered 5 times", async () => { - nock(baseApi).post(API_ENDPOINTS.LOG_IN_USER).times(6).reply(400, { - code: ERROR_CODES.INVALID_PASSWORD_MAX_ATTEMPTS_REACHED, - }); - - await request(app) - .post(ENDPOINT) - .type("form") - .set("Cookie", cookies) - .send({ - _csrf: token, - password: "password", - }) - .expect("Location", PATH_NAMES.ACCOUNT_LOCKED) - .expect(302); - }); -}); From 3de281bf388e7c63ebc9897fddbbc036116a1f92 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Thu, 26 Sep 2024 16:18:44 +0100 Subject: [PATCH 02/13] BAU: Hard-code support2FABeforePasswordReset to true in verify code controller Eventually we'll remove from the state machine, but this is so that we can go incrementally without making too many changes at once --- src/components/common/verify-code/verify-code-controller.ts | 3 +-- .../tests/reset-password-check-email-integration.test.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/common/verify-code/verify-code-controller.ts b/src/components/common/verify-code/verify-code-controller.ts index cb379f0b0..55f62ffd7 100644 --- a/src/components/common/verify-code/verify-code-controller.ts +++ b/src/components/common/verify-code/verify-code-controller.ts @@ -18,7 +18,6 @@ import { PATH_NAMES, } from "../../../app.constants"; import { - support2FABeforePasswordReset, supportAccountInterventions, supportReauthentication, } from "../../../config"; @@ -158,7 +157,7 @@ export function verifyCodePost( isIdentityRequired: req.session.user.isIdentityRequired, isLatestTermsAndConditionsAccepted: req.session.user.isLatestTermsAndConditionsAccepted, - support2FABeforePasswordReset: support2FABeforePasswordReset(), + support2FABeforePasswordReset: true, mfaMethodType: req.session.user.enterEmailMfaType, isPasswordChangeRequired: req.session.user.isPasswordChangeRequired, isOnForcedPasswordResetJourney: diff --git a/src/components/reset-password-check-email/tests/reset-password-check-email-integration.test.ts b/src/components/reset-password-check-email/tests/reset-password-check-email-integration.test.ts index e84f913a1..d6a57d133 100644 --- a/src/components/reset-password-check-email/tests/reset-password-check-email-integration.test.ts +++ b/src/components/reset-password-check-email/tests/reset-password-check-email-integration.test.ts @@ -23,7 +23,6 @@ describe("Integration::reset password check email ", () => { decache("../../../middleware/session-middleware"); const sessionMiddleware = require("../../../middleware/session-middleware"); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "0"; sinon @@ -161,7 +160,7 @@ describe("Integration::reset password check email ", () => { _csrf: token, code: "123456", }) - .expect("Location", PATH_NAMES.RESET_PASSWORD) + .expect("Location", PATH_NAMES.RESET_PASSWORD_2FA_SMS) .expect(302); }); }); From db53daf0e423770c6ffcadec7c196f79349d26aa Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 11:23:10 +0100 Subject: [PATCH 03/13] BAU: Start removing 2fa before password reset feature flag logic from reset password controller This: * Adjusts existing controller and integration tests to reflect the fact that we always now assume 2fa has been done by the point of resetting the password * Deletes src/components/reset-password/tests/reset-password-2fa-before-integration.test.ts (the behaviour is going to exist in all cases, so we no longer need a feature-flag specific integration test * Hard-codes the value we pass in to the state machine to true. Eventually this can be removed entirely from the state machine and all calls, but doing this as an interim step to avoid to many changes at once --- .../reset-password-controller.ts | 4 +- ...et-password-2fa-before-integration.test.ts | 277 ------------------ .../tests/reset-password-controller.test.ts | 101 +------ ...word-forced-2fa-before-integration.test.ts | 1 - .../tests/reset-password-integration.test.ts | 9 +- 5 files changed, 8 insertions(+), 384 deletions(-) delete mode 100644 src/components/reset-password/tests/reset-password-2fa-before-integration.test.ts diff --git a/src/components/reset-password/reset-password-controller.ts b/src/components/reset-password/reset-password-controller.ts index 6caeb974d..99c08c1cb 100644 --- a/src/components/reset-password/reset-password-controller.ts +++ b/src/components/reset-password/reset-password-controller.ts @@ -155,12 +155,12 @@ export function resetPasswordPost( USER_JOURNEY_EVENTS.PASSWORD_CREATED, { isIdentityRequired: req.session.user.isIdentityRequired, - requiresTwoFactorAuth: !support2FABeforePasswordReset(), + requiresTwoFactorAuth: false, isLatestTermsAndConditionsAccepted: req.session.user.isLatestTermsAndConditionsAccepted, mfaMethodType: loginResponse.data.mfaMethodType, isMfaMethodVerified: loginResponse.data.mfaMethodVerified, - support2FABeforePasswordReset: support2FABeforePasswordReset(), + support2FABeforePasswordReset: true, }, res.locals.sessionId ) diff --git a/src/components/reset-password/tests/reset-password-2fa-before-integration.test.ts b/src/components/reset-password/tests/reset-password-2fa-before-integration.test.ts deleted file mode 100644 index 417a436b8..000000000 --- a/src/components/reset-password/tests/reset-password-2fa-before-integration.test.ts +++ /dev/null @@ -1,277 +0,0 @@ -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 { PATH_NAMES } from "../../../app.constants"; -import decache from "decache"; -import { - noInterventions, - setupAccountInterventionsResponse, -} from "../../../../test/helpers/account-interventions-helpers"; - -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, - }, - }; - - next(); - }); - app = await require("../../../app").createApp(); - baseApi = process.env.FRONTEND_API_BASE_URL; - process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; - setupAccountInterventionsResponse(baseApi, noInterventions); - - await request(app) - .get(ENDPOINT) - .then((res) => { - const $ = cheerio.load(res.text); - token = $("[name=_csrf]").val(); - cookies = res.headers["set-cookie"]; - }); - }); - - beforeEach(() => { - process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; - nock.cleanAll(); - }); - - after(() => { - sinon.restore(); - app = undefined; - delete process.env.SUPPORT_ACCOUNT_INTERVENTIONS; - }); - - it("should return reset password page", async () => { - setupAccountInterventionsResponse(baseApi, noInterventions); - await request(app).get(ENDPOINT).expect(200); - }); - - it("should return the blocked screen when someone has a blocked intervention", async () => { - setupAccountInterventionsResponse(baseApi, { - blocked: true, - passwordResetRequired: false, - temporarilySuspended: false, - reproveIdentity: false, - }); - - await request(app) - .get(ENDPOINT) - .expect(function (res) { - expect(res.headers.location).to.eq("/unavailable-permanent"); - }) - .expect(302); - }); - - it("should return the suspended screen when someone has a suspended intervention", async () => { - setupAccountInterventionsResponse(baseApi, { - blocked: false, - passwordResetRequired: false, - temporarilySuspended: true, - reproveIdentity: false, - }); - - await request(app) - .get(ENDPOINT) - .expect(function (res) { - expect(res.headers.location).to.eq("/unavailable-temporary"); - }) - .expect(302); - }); - - it("should return reset password page when someone has a reset password intervention", async () => { - setupAccountInterventionsResponse(baseApi, { - blocked: false, - passwordResetRequired: true, - temporarilySuspended: false, - reproveIdentity: false, - }); - - await request(app).get(ENDPOINT).expect(200); - }); - - it("should return error when csrf not present", async () => { - await request(app) - .post(ENDPOINT) - .type("form") - .send({ - password: "password", - }) - .expect(403); - }); - - it("should return validation error when password not entered", async () => { - await 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); - }); - - it("should return validation error when passwords don't match", async () => { - await 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); - }); - - it("should return validation error when password less than 8 characters", async () => { - await 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); - }); - - it("should return validation error when password is amongst most common passwords", async () => { - nock(baseApi).post("/reset-password").once().reply(400, { code: 1040 }); - - await 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); - }); - - it("should return error when new password is the same as existing password", async () => { - nock(baseApi).post("/reset-password").once().reply(400, { code: 1024 }); - - await 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); - }); - - it("should return validation error when no numbers present in password", async () => { - await 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); - }); - - it("should return validation error when password all numeric", async () => { - await 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); - }); - - it("should redirect to /auth-code when valid password entered", async () => { - nock(baseApi).post("/reset-password").once().reply(204); - nock(baseApi).post("/login").once().reply(200); - nock(baseApi).post("/mfa").once().reply(204); - - await 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); - }); -}); 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 e995ee28e..7fa7b4731 100644 --- a/src/components/reset-password/tests/reset-password-controller.test.ts +++ b/src/components/reset-password/tests/reset-password-controller.test.ts @@ -44,7 +44,6 @@ describe("reset password controller (in 6 digit code flow)", () => { beforeEach(() => { req = createMockRequest(PATH_NAMES.RESET_PASSWORD); res = mockResponse(); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; }); afterEach(() => { @@ -91,7 +90,7 @@ describe("reset password controller (in 6 digit code flow)", () => { " passwordChangeRequired: " + params.passwordChangeRequired, () => { - it("should redirect to /enter-code when password updated and phone number verified", async () => { + it("should redirect to /auth-code if request is success", async () => { const fakeResetService: ResetPasswordServiceInterface = { updatePassword: sinon.fake.returns({ success: true }), } as unknown as ResetPasswordServiceInterface; @@ -113,71 +112,15 @@ describe("reset password controller (in 6 digit code flow)", () => { sendMfaCode: sinon.fake.returns({ success: true }), } as unknown as MfaServiceInterface; - req.session.user = { - email: "joe.bloggs@test.com", - isPasswordChangeRequired: params.passwordChangeRequired, - }; - 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.have.been.calledOnce; - expect(fakeMfAService.sendMfaCode).to.have.been.calledOnce; - - if ( - params.supportPasswordResetRequired === "1" && - params.passwordChangeRequired - ) { - expect(req.session.user.isPasswordChangeRequired).to.be.eq(false); - } - - expect(req.session.user.passwordResetTime).not.to.be.undefined; - - expect(res.redirect).to.have.calledWith(PATH_NAMES.ENTER_MFA); + expect(res.redirect).to.have.calledWith(PATH_NAMES.AUTH_CODE); }); - it( - "should redirect to /auth-code if support2FABeforePasswordReset " + - "flag is set to true", - async () => { - 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", - latestTermsAndConditionsAccepted: true, - mfaMethodVerified: true, - mfaMethodType: MFA_METHOD_TYPE.SMS, - mfaRequired: true, - passwordChangeRequired: params.passwordChangeRequired, - }, - }), - } as unknown as EnterPasswordServiceInterface; - fakeLoginService.loginUser; - const fakeMfAService: MfaServiceInterface = { - sendMfaCode: sinon.fake.returns({ success: true }), - } as unknown as MfaServiceInterface; - - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; - - await resetPasswordPost( - fakeResetService, - fakeLoginService, - fakeMfAService - )(req as Request, res as Response); - - expect(res.redirect).to.have.calledWith(PATH_NAMES.AUTH_CODE); - } - ); - it("should redirect to /get-security-codes when password updated and mfa method not verified", async () => { const fakeResetService: ResetPasswordServiceInterface = { updatePassword: sinon.fake.returns({ success: true }), @@ -221,46 +164,6 @@ describe("reset password controller (in 6 digit code flow)", () => { } ); - it("should request 2fa when password updated even for non 2fa service", async () => { - 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", - 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", - }; - 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.have.been.calledOnce; - expect(fakeMfAService.sendMfaCode).to.have.been.calledOnce; - - expect(res.redirect).to.have.calledWith(PATH_NAMES.ENTER_MFA); - }); - it("should not set the passwordResetTime flag on the session if update password is not a success", async () => { const fakeResetService: ResetPasswordServiceInterface = { updatePassword: sinon.fake.returns({ 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 index 18ab4e2b5..9d10caf8e 100644 --- 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 @@ -19,7 +19,6 @@ describe("Integration::reset password (in 2FA Before Reset Password flow)", () = 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"); diff --git a/src/components/reset-password/tests/reset-password-integration.test.ts b/src/components/reset-password/tests/reset-password-integration.test.ts index 8bff58312..9eead7742 100644 --- a/src/components/reset-password/tests/reset-password-integration.test.ts +++ b/src/components/reset-password/tests/reset-password-integration.test.ts @@ -19,7 +19,6 @@ describe("Integration::reset password (in 6 digit code flow)", () => { const ENDPOINT = "/reset-password"; before(async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; decache("../../../app"); @@ -257,12 +256,12 @@ describe("Integration::reset password (in 6 digit code flow)", () => { .expect(400, done); }); - it("should redirect to MFA step when valid password entered", (done) => { + it("should redirect to /auth-code when valid password entered", async () => { nock(baseApi).post("/reset-password").once().reply(204); nock(baseApi).post("/login").once().reply(200); nock(baseApi).post("/mfa").once().reply(204); - request(app) + await request(app) .post(ENDPOINT) .type("form") .set("Cookie", cookies) @@ -271,7 +270,7 @@ describe("Integration::reset password (in 6 digit code flow)", () => { password: "Testpassword1", "confirm-password": "Testpassword1", }) - .expect("Location", PATH_NAMES.ENTER_MFA) - .expect(302, done); + .expect("Location", PATH_NAMES.AUTH_CODE) + .expect(302); }); }); From 185b11947ed8dfcb797246ba18c5b238af026584 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 11:29:56 +0100 Subject: [PATCH 04/13] BAU: Rename a test and make it test the right endpoint This: * renames this integration test to reflect the fact that we always do 2fa before password reset * amends the endpoint that we're hitting in these tests so it does actually test what it says it is testing (currently the tests are a duplicate of the existing integration test file --- ....test.ts => reset-password-required-integration.test.ts} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename src/components/reset-password/tests/{reset-password-forced-2fa-before-integration.test.ts => reset-password-required-integration.test.ts} (97%) diff --git a/src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts b/src/components/reset-password/tests/reset-password-required-integration.test.ts similarity index 97% rename from src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts rename to src/components/reset-password/tests/reset-password-required-integration.test.ts index 9d10caf8e..d0e6b7550 100644 --- a/src/components/reset-password/tests/reset-password-forced-2fa-before-integration.test.ts +++ b/src/components/reset-password/tests/reset-password-required-integration.test.ts @@ -10,13 +10,13 @@ import { setupAccountInterventionsResponse, } from "../../../../test/helpers/account-interventions-helpers"; -describe("Integration::reset password (in 2FA Before Reset Password flow)", () => { +describe("Integration::reset password required", () => { let token: string | string[]; let cookies: string; let app: any; let baseApi: string; - const ENDPOINT = "/reset-password"; + const ENDPOINT = PATH_NAMES.RESET_PASSWORD_REQUIRED; before(async () => { decache("../../../app"); @@ -31,7 +31,7 @@ describe("Integration::reset password (in 2FA Before Reset Password flow)", () = email: "test@test.com", phoneNumber: "7867", journey: { - nextPath: PATH_NAMES.RESET_PASSWORD, + nextPath: ENDPOINT, }, isAuthenticated: true, isAccountPartCreated: false, From 2384b5ec32c7e153b7c439fc2b7b8a8c99b0a2e6 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 11:34:37 +0100 Subject: [PATCH 05/13] BAU: Remove send mfa logic in reset password controller This was only done in the case of support2FABeforePasswordReset being false. Given the feature flag is now switched on everywhere, it was never being called and we can remove this code block since we will have always done the 2fa step by this point --- .../reset-password-controller.ts | 41 +------------------ .../tests/reset-password-controller.test.ts | 38 ++++++----------- 2 files changed, 14 insertions(+), 65 deletions(-) diff --git a/src/components/reset-password/reset-password-controller.ts b/src/components/reset-password/reset-password-controller.ts index 99c08c1cb..525476981 100644 --- a/src/components/reset-password/reset-password-controller.ts +++ b/src/components/reset-password/reset-password-controller.ts @@ -6,20 +6,11 @@ import { formatValidationError, renderBadRequest, } from "../../utils/validation"; -import { - ERROR_CODES, - getErrorPathByCode, - getNextPathAndUpdateJourney, -} from "../common/constants"; +import { ERROR_CODES, getNextPathAndUpdateJourney } from "../common/constants"; import { USER_JOURNEY_EVENTS } from "../common/state-machine/state-machine"; import { BadRequestError } from "../../utils/error"; import { EnterPasswordServiceInterface } from "../enter-password/types"; import { enterPasswordService } from "../enter-password/enter-password-service"; -import { MfaServiceInterface } from "../common/mfa/types"; -import { mfaService } from "../common/mfa/mfa-service"; -import { MFA_METHOD_TYPE } from "../../app.constants"; -import xss from "xss"; -import { support2FABeforePasswordReset } from "../../config"; const resetPasswordTemplate = "reset-password/index.njk"; @@ -47,8 +38,7 @@ export function resetPasswordRequiredGet(req: Request, res: Response): void { export function resetPasswordPost( resetService: ResetPasswordServiceInterface = resetPasswordService(), - loginService: EnterPasswordServiceInterface = enterPasswordService(), - mfaCodeService: MfaServiceInterface = mfaService() + loginService: EnterPasswordServiceInterface = enterPasswordService() ): ExpressRouteFunc { return async function (req: Request, res: Response) { const { email, withinForcedPasswordResetJourney } = req.session.user; @@ -121,33 +111,6 @@ export function resetPasswordPost( 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, - persistentSessionId, - false, - xss(req.cookies.lng as string), - req - ); - - if (!mfaResponse.success) { - const path = getErrorPathByCode(mfaResponse.data.code); - if (path) { - return res.redirect(path); - } - throw new BadRequestError( - mfaResponse.data.message, - mfaResponse.data.code - ); - } - } - return res.redirect( await getNextPathAndUpdateJourney( req, 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 7fa7b4731..e1af57bec 100644 --- a/src/components/reset-password/tests/reset-password-controller.test.ts +++ b/src/components/reset-password/tests/reset-password-controller.test.ts @@ -14,7 +14,6 @@ import { ResetPasswordServiceInterface } from "../types"; import { MFA_METHOD_TYPE, PATH_NAMES } from "../../../app.constants"; import { mockResponse, RequestOutput, ResponseOutput } from "mock-req-res"; import { EnterPasswordServiceInterface } from "../../enter-password/types"; -import { MfaServiceInterface } from "../../common/mfa/types"; import { ERROR_CODES } from "../../common/constants"; import { createMockRequest } from "../../../../test/helpers/mock-request-helper"; @@ -108,15 +107,11 @@ describe("reset password controller (in 6 digit code flow)", () => { }), } as unknown as EnterPasswordServiceInterface; fakeLoginService.loginUser; - const fakeMfAService: MfaServiceInterface = { - sendMfaCode: sinon.fake.returns({ success: true }), - } as unknown as MfaServiceInterface; - await resetPasswordPost( - fakeResetService, - fakeLoginService, - fakeMfAService - )(req as Request, res as Response); + await resetPasswordPost(fakeResetService, fakeLoginService)( + req as Request, + res as Response + ); expect(res.redirect).to.have.calledWith(PATH_NAMES.AUTH_CODE); }); @@ -138,24 +133,19 @@ describe("reset password controller (in 6 digit code flow)", () => { }), } 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", }; req.body.password = "Password1"; - await resetPasswordPost( - fakeResetService, - fakeLoginService, - fakeMfAService - )(req as Request, res as Response); + await resetPasswordPost(fakeResetService, fakeLoginService)( + req as Request, + res as Response + ); expect(fakeResetService.updatePassword).to.have.been.calledOnce; expect(fakeLoginService.loginUser).to.have.been.calledOnce; - expect(fakeMfAService.sendMfaCode).to.not.have.been.called; expect(res.redirect).to.have.calledWith( PATH_NAMES.GET_SECURITY_CODES @@ -185,20 +175,16 @@ describe("reset password controller (in 6 digit code flow)", () => { }), } 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", }; req.body.password = "Password1"; - await resetPasswordPost( - fakeResetService, - fakeLoginService, - fakeMfAService - )(req as Request, res as Response); + await resetPasswordPost(fakeResetService, fakeLoginService)( + req as Request, + res as Response + ); expect(fakeResetService.updatePassword).to.have.been.calledOnce; expect(fakeLoginService.loginUser).not.to.have.been.called; From a2319b6eaf2b39ce177eb5ce872ff5fe600f30cf Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 11:57:17 +0100 Subject: [PATCH 06/13] BAU: Hardcode support 2fa before password reset flag to true We can remove this from the template logic later (trying to avoid doing too many changes at once), for now we can just set to true always --- .../reset-password-check-email-controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/reset-password-check-email/reset-password-check-email-controller.ts b/src/components/reset-password-check-email/reset-password-check-email-controller.ts index 2f5fb35a7..e20be14b3 100644 --- a/src/components/reset-password-check-email/reset-password-check-email-controller.ts +++ b/src/components/reset-password-check-email/reset-password-check-email-controller.ts @@ -8,7 +8,7 @@ import { VerifyCodeInterface } from "../common/verify-code/types"; import { codeService } from "../common/verify-code/verify-code-service"; import { verifyCodePost } from "../common/verify-code/verify-code-controller"; import { NOTIFICATION_TYPE } from "../../app.constants"; -import { support2FABeforePasswordReset, support2hrLockout } from "../../config"; +import { support2hrLockout } from "../../config"; import { AccountInterventionsInterface } from "../account-intervention/types"; import { accountInterventionService } from "../account-intervention/account-intervention-service"; import { isLocked } from "../../utils/lock-helper"; @@ -83,7 +83,7 @@ export function resetPasswordCheckEmailGet( }; if (!requestCode || result.success) { - const support2FABeforePasswordResetFlag = support2FABeforePasswordReset(); + const support2FABeforePasswordResetFlag = true; const isForcedPasswordResetJourney = req.session.user.withinForcedPasswordResetJourney || false; return res.render(TEMPLATE_NAME, { From dde3e75343da0afd30e970cb2ab4cf74c9b29e68 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:02:59 +0100 Subject: [PATCH 07/13] BAU: Remove conditional support2faBeforePasswordReset logic from app.ts We are going to remove the feature flag logic since we've now switched it on in all cases, so we don't have to conditionally use these two routers now --- src/app.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/app.ts b/src/app.ts index bdbbcadbc..48a0497be 100644 --- a/src/app.ts +++ b/src/app.ts @@ -20,7 +20,6 @@ import { getNodeEnv, getSessionExpiry, getSessionSecret, - support2FABeforePasswordReset, supportAccountInterventions, supportAccountRecovery, supportAuthorizeController, @@ -128,10 +127,8 @@ function registerRoutes(app: express.Application) { app.use(signedOutRouter); app.use(updatedTermsConditionsRouter); app.use(resetPasswordRouter); - if (support2FABeforePasswordReset()) { - app.use(resetPassword2FARouter); - app.use(resetPassword2FAAuthAppRouter); - } + app.use(resetPassword2FARouter); + app.use(resetPassword2FAAuthAppRouter); app.use(upliftJourneyRouter); app.use(contactUsRouter); app.use(healthcheckRouter); From 01c7041e1cc7f7fab48db79d284066b279b3ff01 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:05:45 +0100 Subject: [PATCH 08/13] BAU: Remove unused config method We've now either removed all conditional logic based on this feature flag (since it's now always true) or hard-coded this value to true in various places where we haven't wanted to change too many things at once. Therefore this config can be removed as it's no longer used --- .../auth-code/tests/auth-code-service.test.ts | 25 +------------------ src/config.ts | 4 --- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/src/components/auth-code/tests/auth-code-service.test.ts b/src/components/auth-code/tests/auth-code-service.test.ts index 12be938b5..6f4ddaed2 100644 --- a/src/components/auth-code/tests/auth-code-service.test.ts +++ b/src/components/auth-code/tests/auth-code-service.test.ts @@ -7,10 +7,7 @@ import { SinonStub } from "sinon"; import { API_ENDPOINTS, PATH_NAMES } from "../../../app.constants"; import { AuthCodeServiceInterface } from "../types"; import { Http } from "../../../utils/http"; -import { - support2FABeforePasswordReset, - support2hrLockout, -} from "../../../config"; +import { support2hrLockout } from "../../../config"; import { createMockRequest } from "../../../../test/helpers/mock-request-helper"; describe("authentication auth code service", () => { @@ -211,26 +208,6 @@ describe("authentication auth code service", () => { }); }); - describe("support2FABeforePasswordReset() with the support 2FA before password reset feature flag on", () => { - it("should return true when SUPPORT_2FA_B4_PASSWORD_RESET is set to '1'", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; - - expect(support2FABeforePasswordReset()).to.be.true; - }); - - it("should return false when SUPPORT_2FA_B4_PASSWORD_RESET is set to '0'", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; - - expect(support2FABeforePasswordReset()).to.be.false; - }); - - it("should return false when SUPPORT_2FA_B4_PASSWORD_RESET is undefined", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = undefined; - - expect(support2FABeforePasswordReset()).to.be.false; - }); - }); - describe("support2hrLockout() with the support 2hr lockout for password and code lockouts", () => { it("should return true when SUPPORT_2HR_LOCKOUT is set to '1'", async () => { process.env.SUPPORT_2HR_LOCKOUT = "1"; diff --git a/src/config.ts b/src/config.ts index a4d95ca39..922f446d4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -122,10 +122,6 @@ export function getPasswordResetCodeEnteredWrongBlockDurationInMinutes(): number ); } -export function support2FABeforePasswordReset(): boolean { - return process.env.SUPPORT_2FA_B4_PASSWORD_RESET === "1"; -} - export function support2hrLockout(): boolean { return process.env.SUPPORT_2HR_LOCKOUT === "1"; } From 84301dc790005a5f90c3b4d7699ef095774ef311 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:09:37 +0100 Subject: [PATCH 09/13] BAU: Remove all terraform variables related to support_2fa_b4_password_reset feature flag The application has now been updated to no longer rely on this feature flag (all logic based on the feature flag being off has either been removed, or we now hard-code variables in the app set from this to true (further logic to be removed subsequently) so we no longer need this feature flag --- ci/terraform/authdev1.tfvars | 1 - ci/terraform/authdev2.tfvars | 1 - ci/terraform/build.tfvars | 1 - ci/terraform/dev.tfvars | 1 - ci/terraform/ecs.tf | 4 ---- ci/terraform/integration.tfvars | 1 - ci/terraform/production.tfvars | 1 - ci/terraform/sandpit.tfvars | 1 - ci/terraform/staging.tfvars | 1 - ci/terraform/variables.tf | 6 ------ 10 files changed, 18 deletions(-) diff --git a/ci/terraform/authdev1.tfvars b/ci/terraform/authdev1.tfvars index 555d33537..ac97e144e 100644 --- a/ci/terraform/authdev1.tfvars +++ b/ci/terraform/authdev1.tfvars @@ -14,7 +14,6 @@ support_account_recovery = "1" support_authorize_controller = "1" support_account_interventions = "1" support_reauthentication = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" password_reset_code_entered_wrong_blocked_minutes = "1" account_recovery_code_entered_wrong_blocked_minutes = "1" diff --git a/ci/terraform/authdev2.tfvars b/ci/terraform/authdev2.tfvars index 5c73b54d5..56b7adab2 100644 --- a/ci/terraform/authdev2.tfvars +++ b/ci/terraform/authdev2.tfvars @@ -18,7 +18,6 @@ support_account_recovery = "1" support_authorize_controller = "1" support_account_interventions = "1" support_reauthentication = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" password_reset_code_entered_wrong_blocked_minutes = "1" account_recovery_code_entered_wrong_blocked_minutes = "1" diff --git a/ci/terraform/build.tfvars b/ci/terraform/build.tfvars index f2834e0c9..0b604111e 100644 --- a/ci/terraform/build.tfvars +++ b/ci/terraform/build.tfvars @@ -15,7 +15,6 @@ support_account_recovery = "1" support_authorize_controller = "1" support_account_interventions = "1" support_reauthentication = "0" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" password_reset_code_entered_wrong_blocked_minutes = "1" account_recovery_code_entered_wrong_blocked_minutes = "1" diff --git a/ci/terraform/dev.tfvars b/ci/terraform/dev.tfvars index 2d6e134b9..9b37803b0 100644 --- a/ci/terraform/dev.tfvars +++ b/ci/terraform/dev.tfvars @@ -20,7 +20,6 @@ support_account_recovery = "1" support_authorize_controller = "1" support_account_interventions = "1" support_reauthentication = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" password_reset_code_entered_wrong_blocked_minutes = "1" account_recovery_code_entered_wrong_blocked_minutes = "1" diff --git a/ci/terraform/ecs.tf b/ci/terraform/ecs.tf index f34596623..5cc13c494 100644 --- a/ci/terraform/ecs.tf +++ b/ci/terraform/ecs.tf @@ -136,10 +136,6 @@ locals { name = "REDUCED_CODE_BLOCK_DURATION_MINUTES" value = var.reduced_code_block_duration_minutes }, - { - name = "SUPPORT_2FA_B4_PASSWORD_RESET" - value = var.support_2fa_b4_password_reset - }, { name = "SUPPORT_ACCOUNT_INTERVENTIONS" value = var.support_account_interventions diff --git a/ci/terraform/integration.tfvars b/ci/terraform/integration.tfvars index 333ef3a63..8cab862c0 100644 --- a/ci/terraform/integration.tfvars +++ b/ci/terraform/integration.tfvars @@ -14,7 +14,6 @@ frontend_task_definition_memory = 1024 support_account_recovery = "1" support_account_interventions = "1" support_authorize_controller = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" support_reauthentication = "1" language_toggle_enabled = "1" diff --git a/ci/terraform/production.tfvars b/ci/terraform/production.tfvars index f77d0cc45..d348f5d12 100644 --- a/ci/terraform/production.tfvars +++ b/ci/terraform/production.tfvars @@ -17,7 +17,6 @@ ecs_desired_count = 4 support_account_recovery = "1" support_account_interventions = "1" support_authorize_controller = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" code_request_blocked_minutes = "120" account_recovery_code_entered_wrong_blocked_minutes = "120" diff --git a/ci/terraform/sandpit.tfvars b/ci/terraform/sandpit.tfvars index 42f10bca1..42820aadd 100644 --- a/ci/terraform/sandpit.tfvars +++ b/ci/terraform/sandpit.tfvars @@ -15,7 +15,6 @@ support_account_recovery = "1" support_authorize_controller = "1" support_account_interventions = "1" support_reauthentication = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" password_reset_code_entered_wrong_blocked_minutes = "1" account_recovery_code_entered_wrong_blocked_minutes = "1" diff --git a/ci/terraform/staging.tfvars b/ci/terraform/staging.tfvars index 612a410a0..883dd6cff 100644 --- a/ci/terraform/staging.tfvars +++ b/ci/terraform/staging.tfvars @@ -16,7 +16,6 @@ ecs_desired_count = 4 support_account_recovery = "1" support_account_interventions = "1" support_authorize_controller = "1" -support_2fa_b4_password_reset = "1" support_2hr_lockout = "1" code_request_blocked_minutes = "120" account_recovery_code_entered_wrong_blocked_minutes = "120" diff --git a/ci/terraform/variables.tf b/ci/terraform/variables.tf index 848566ba9..a80156747 100644 --- a/ci/terraform/variables.tf +++ b/ci/terraform/variables.tf @@ -236,12 +236,6 @@ variable "orch_to_auth_audience" { variable "dynatrace_secret_arn" { } -variable "support_2fa_b4_password_reset" { - description = "When true enables 2FA before password reset" - type = string - default = "0" -} - variable "support_2hr_lockout" { description = "When true enables 2hr lockout" type = string From 87e5a6aa5eb691e1cde3cb3929cb294e7a87179f Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:20:58 +0100 Subject: [PATCH 10/13] BAU: Remove support2FABeforePasswordReset logic from the state machine and the places where the state machine is called We now want support 2fa before password reset in all cases, so we can remove some conditional logic in the state machine and remove this flag from the calls to the state machine --- src/components/common/state-machine/state-machine.ts | 11 ++--------- .../common/verify-code/verify-code-controller.ts | 1 - .../enter-password/enter-password-controller.ts | 1 - .../reset-password/reset-password-controller.ts | 1 - 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/components/common/state-machine/state-machine.ts b/src/components/common/state-machine/state-machine.ts index 608fc95f4..1bb0ef0ba 100644 --- a/src/components/common/state-machine/state-machine.ts +++ b/src/components/common/state-machine/state-machine.ts @@ -75,7 +75,6 @@ const authStateMachine = createMachine( isMfaMethodVerified: true, isPasswordChangeRequired: false, isAccountRecoveryJourney: false, - support2FABeforePasswordReset: false, isReauthenticationRequired: false, requiresResetPasswordMFASmsCode: false, requiresResetPasswordMFAAuthAppCode: false, @@ -760,26 +759,20 @@ const authStateMachine = createMachine( context.requiresTwoFactorAuth === true, requiresResetPasswordMFAAuthAppCode: (context) => context.mfaMethodType === MFA_METHOD_TYPE.AUTH_APP && - context.isOnForcedPasswordResetJourney !== true && - context.support2FABeforePasswordReset === true, + context.isOnForcedPasswordResetJourney !== true, requiresResetPasswordMFASmsCode: (context) => context.mfaMethodType === MFA_METHOD_TYPE.SMS && - context.isOnForcedPasswordResetJourney !== true && - context.support2FABeforePasswordReset === true, + context.isOnForcedPasswordResetJourney !== true, isPasswordChangeRequired: (context) => context.isPasswordChangeRequired, is2FASMSPasswordChangeRequired: (context) => context.isPasswordChangeRequired === true && context.mfaMethodType === MFA_METHOD_TYPE.SMS && - context.support2FABeforePasswordReset === true && context.requiresTwoFactorAuth === true, is2FAAuthAppPasswordChangeRequired: (context) => context.isPasswordChangeRequired === true && context.mfaMethodType === MFA_METHOD_TYPE.AUTH_APP && - context.support2FABeforePasswordReset === true && context.requiresTwoFactorAuth === true, isAccountRecoveryJourney: (context) => context.isAccountRecoveryJourney, - support2FABeforePasswordReset: (context) => - context.support2FABeforePasswordReset, }, } ); diff --git a/src/components/common/verify-code/verify-code-controller.ts b/src/components/common/verify-code/verify-code-controller.ts index 55f62ffd7..afc861628 100644 --- a/src/components/common/verify-code/verify-code-controller.ts +++ b/src/components/common/verify-code/verify-code-controller.ts @@ -157,7 +157,6 @@ export function verifyCodePost( isIdentityRequired: req.session.user.isIdentityRequired, isLatestTermsAndConditionsAccepted: req.session.user.isLatestTermsAndConditionsAccepted, - support2FABeforePasswordReset: true, mfaMethodType: req.session.user.enterEmailMfaType, isPasswordChangeRequired: req.session.user.isPasswordChangeRequired, isOnForcedPasswordResetJourney: diff --git a/src/components/enter-password/enter-password-controller.ts b/src/components/enter-password/enter-password-controller.ts index fc2bfedb8..237446e2d 100644 --- a/src/components/enter-password/enter-password-controller.ts +++ b/src/components/enter-password/enter-password-controller.ts @@ -252,7 +252,6 @@ export function enterPasswordPost( mfaMethodType: userLogin.data.mfaMethodType, isMfaMethodVerified: userLogin.data.mfaMethodVerified, isPasswordChangeRequired: isPasswordChangeRequired, - support2FABeforePasswordReset: true, }, sessionId ) diff --git a/src/components/reset-password/reset-password-controller.ts b/src/components/reset-password/reset-password-controller.ts index 525476981..c9b727fa0 100644 --- a/src/components/reset-password/reset-password-controller.ts +++ b/src/components/reset-password/reset-password-controller.ts @@ -123,7 +123,6 @@ export function resetPasswordPost( req.session.user.isLatestTermsAndConditionsAccepted, mfaMethodType: loginResponse.data.mfaMethodType, isMfaMethodVerified: loginResponse.data.mfaMethodVerified, - support2FABeforePasswordReset: true, }, res.locals.sessionId ) From a474ffae4904f3fcce669bd708e6b7207a665afc Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:25:36 +0100 Subject: [PATCH 11/13] BAU: Remove conditional logic from template based on support2FABeforePasswordResetFlag We're always going to do 2fa before password reset, so we can remove this condition and also remove the flag which gets passed in to the template --- src/components/reset-password-check-email/index.njk | 2 +- .../reset-password-check-email-controller.ts | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/reset-password-check-email/index.njk b/src/components/reset-password-check-email/index.njk index b7a61750e..bdc46a73a 100644 --- a/src/components/reset-password-check-email/index.njk +++ b/src/components/reset-password-check-email/index.njk @@ -13,7 +13,7 @@

{{'pages.resetPasswordCheckEmail.header' | translate}}

- {% if support2FABeforePasswordResetFlag and not isForcedPasswordResetJourney %} + {% if not isForcedPasswordResetJourney %}

{{ diff --git a/src/components/reset-password-check-email/reset-password-check-email-controller.ts b/src/components/reset-password-check-email/reset-password-check-email-controller.ts index e20be14b3..3f58ad1f5 100644 --- a/src/components/reset-password-check-email/reset-password-check-email-controller.ts +++ b/src/components/reset-password-check-email/reset-password-check-email-controller.ts @@ -83,11 +83,9 @@ export function resetPasswordCheckEmailGet( }; if (!requestCode || result.success) { - const support2FABeforePasswordResetFlag = true; const isForcedPasswordResetJourney = req.session.user.withinForcedPasswordResetJourney || false; return res.render(TEMPLATE_NAME, { - support2FABeforePasswordResetFlag, isForcedPasswordResetJourney, email, currentPath: req.originalUrl, From d37c7e869ed83ae2789c3bea1a4977a9afe70478 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:32:18 +0100 Subject: [PATCH 12/13] BAU: Remove all setting of SUPPORT_2FA_B4_PASSWORD_RESET variable from tests The app has now been updated to always do 2fa before password reset, so these env variables no longer control anything and can be removed --- .../tests/reset-password-2fa-auth-app-controller.test.ts | 5 +---- .../reset-password-2fa-auth-app-integration.test.ts | 2 -- .../tests/reset-password-2fa-sms-controller.test.ts | 3 --- .../tests/reset-password-2fa-sms-integration.test.ts | 2 -- ...password-check-email-auth-app-2fa-integration.test.ts | 1 - .../tests/reset-password-check-email-controller.test.ts | 9 ++------- 6 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-controller.test.ts b/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-controller.test.ts index fc4c46afb..6264ec651 100644 --- a/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-controller.test.ts +++ b/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-controller.test.ts @@ -23,7 +23,6 @@ describe("reset password 2fa auth app controller", () => { email: commonVariables.email, }; res = mockResponse(); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; }); afterEach(() => { @@ -41,8 +40,7 @@ describe("reset password 2fa auth app controller", () => { }); describe("resetPassword2FAAuthAppPost", () => { - it("should redirect to reset-password if code entered is correct and feature flag is turned on", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; + it("should redirect to reset-password if code entered is correct", async () => { const fakeService: VerifyMfaCodeInterface = { verifyMfaCode: sinon.fake.returns({ success: true, @@ -60,7 +58,6 @@ describe("reset password 2fa auth app controller", () => { }); it("should render check email page with errors if incorrect code entered", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; const fakeService: VerifyMfaCodeInterface = { verifyMfaCode: sinon.fake.returns({ success: false, diff --git a/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-integration.test.ts b/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-integration.test.ts index f26819e55..eaf254719 100644 --- a/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-integration.test.ts +++ b/src/components/reset-password-2fa-auth-app/tests/reset-password-2fa-auth-app-integration.test.ts @@ -22,8 +22,6 @@ describe("Integration::2fa auth app (in reset password flow)", () => { decache("../../../middleware/session-middleware"); const sessionMiddleware = require("../../../middleware/session-middleware"); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; - sinon .stub(sessionMiddleware, "validateSessionMiddleware") .callsFake(function (req: any, res: any, next: any): void { diff --git a/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-controller.test.ts b/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-controller.test.ts index 02cb7c5cd..7c7b7f96c 100644 --- a/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-controller.test.ts +++ b/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-controller.test.ts @@ -20,7 +20,6 @@ describe("reset password 2fa auth app controller", () => { let res: ResponseOutput; beforeEach(() => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; req = createMockRequest(PATH_NAMES.RESET_PASSWORD_2FA_SMS); res = mockResponse(); }); @@ -56,7 +55,6 @@ describe("reset password 2fa auth app controller", () => { }), } as unknown as MfaServiceInterface; - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; const date = new Date(); const futureDate = new Date( date.setDate(date.getDate() + 6) @@ -123,7 +121,6 @@ describe("reset password 2fa auth app controller", () => { }); it("should render check email page with errors if incorrect code entered", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; const fakeService: VerifyCodeInterface = { verifyCode: sinon.fake.returns({ success: false, diff --git a/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-integration.test.ts b/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-integration.test.ts index 1de679e58..950386bfe 100644 --- a/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-integration.test.ts +++ b/src/components/reset-password-2fa-sms/tests/reset-password-2fa-sms-integration.test.ts @@ -22,8 +22,6 @@ describe("Integration::2fa sms (in reset password flow)", () => { decache("../../../middleware/session-middleware"); const sessionMiddleware = require("../../../middleware/session-middleware"); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; - sinon .stub(sessionMiddleware, "validateSessionMiddleware") .callsFake(function (req: any, res: any, next: any): void { diff --git a/src/components/reset-password-check-email/tests/reset-password-check-email-auth-app-2fa-integration.test.ts b/src/components/reset-password-check-email/tests/reset-password-check-email-auth-app-2fa-integration.test.ts index 3c0b439aa..a8d52618c 100644 --- a/src/components/reset-password-check-email/tests/reset-password-check-email-auth-app-2fa-integration.test.ts +++ b/src/components/reset-password-check-email/tests/reset-password-check-email-auth-app-2fa-integration.test.ts @@ -26,7 +26,6 @@ describe("Integration::reset password check email ", () => { decache("../../../middleware/session-middleware"); const sessionMiddleware = require("../../../middleware/session-middleware"); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; sinon diff --git a/src/components/reset-password-check-email/tests/reset-password-check-email-controller.test.ts b/src/components/reset-password-check-email/tests/reset-password-check-email-controller.test.ts index 01f6f5228..d070ee2fc 100644 --- a/src/components/reset-password-check-email/tests/reset-password-check-email-controller.test.ts +++ b/src/components/reset-password-check-email/tests/reset-password-check-email-controller.test.ts @@ -27,7 +27,6 @@ describe("reset password check email controller", () => { beforeEach(() => { req = createMockRequest(PATH_NAMES.RESET_PASSWORD_CHECK_EMAIL); res = mockResponse(); - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "0"; process.env.SUPPORT_ACCOUNT_INTERVENTIONS = "1"; req.session.user = { email: "joe.bloggs@test.com", @@ -82,8 +81,7 @@ describe("reset password check email controller", () => { expect(res.redirect).to.have.calledWith("/reset-password"); }); - it("should redirect to check_phone if code entered is correct and feature flag is turned on", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; + it("should redirect to check_phone if code entered is correct", async () => { const fakeService = fakeVerifyCodeServiceHelper(true); const fakeInterventionsService = accountInterventionsFakeHelper(noInterventions); @@ -98,8 +96,7 @@ describe("reset password check email controller", () => { ); }); - it("should redirect to check_auth_app if code entered is correct and feature flag is turned on", async () => { - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; + it("should redirect to check_auth_app if code entered is correct", async () => { const fakeService = fakeVerifyCodeServiceHelper(true); const fakeInterventionsService = accountInterventionsFakeHelper(noInterventions); @@ -131,7 +128,6 @@ describe("reset password check email controller", () => { it("should redirect to /reset-password without calling the account interventions service when session.user.withinForcedPasswordResetJourney === true and enterEmailMfaType == SMS", async () => { req.session.user.withinForcedPasswordResetJourney = true; - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; req.session.user.enterEmailMfaType = "SMS"; const fakeInterventionsService = @@ -148,7 +144,6 @@ describe("reset password check email controller", () => { it("should redirect to /reset-password without calling the account interventions service when session.user.withinForcedPasswordResetJourney === true and enterEmailMfaType == AUTH_APP", async () => { req.session.user.withinForcedPasswordResetJourney = true; - process.env.SUPPORT_2FA_B4_PASSWORD_RESET = "1"; req.session.user.enterEmailMfaType = "AUTH_APP"; const fakeInterventionsService = From df712b3038d038af73a07c804fd4b9d44f864849 Mon Sep 17 00:00:00 2001 From: BeckaL Date: Fri, 27 Sep 2024 12:48:49 +0100 Subject: [PATCH 13/13] BAU: Remove SUPPORT_2FA_B4_PASSWORD_RESET from cloudformation template and script to create env file --- cloudformation/deploy/template.yaml | 2 -- scripts/_create_env_file.py | 1 - 2 files changed, 3 deletions(-) diff --git a/cloudformation/deploy/template.yaml b/cloudformation/deploy/template.yaml index fed80c398..9a105b037 100644 --- a/cloudformation/deploy/template.yaml +++ b/cloudformation/deploy/template.yaml @@ -554,8 +554,6 @@ Resources: Value: 1 - Name: REDUCED_CODE_BLOCK_DURATION_MINUTES Value: 0.5 - - Name: SUPPORT_2FA_B4_PASSWORD_RESET - Value: 1 - Name: SUPPORT_ACCOUNT_INTERVENTIONS Value: 1 - Name: SUPPORT_REAUTHENTICATION diff --git a/scripts/_create_env_file.py b/scripts/_create_env_file.py index c25670f3a..0edc5b16f 100755 --- a/scripts/_create_env_file.py +++ b/scripts/_create_env_file.py @@ -88,7 +88,6 @@ class EnvFileSection(TypedDict): "SUPPORT_ACCOUNT_RECOVERY": 1, "SUPPORT_AUTHORIZE_CONTROLLER": 1, "SUPPORT_ACCOUNT_INTERVENTIONS": 1, - "SUPPORT_2FA_B4_PASSWORD_RESET": 1, "SUPPORT_REAUTHENTICATION": 1, "SUPPORT_2HR_LOCKOUT": 1, "SUPPORT_CHECK_EMAIL_FRAUD": 1,