From 71067bd3170d486a8aa16b99b98215d9ca21e37c Mon Sep 17 00:00:00 2001 From: GTVJ Date: Fri, 17 May 2024 18:43:31 +0100 Subject: [PATCH] AUT-1432: Fix secret key display in error state and tests This fixes a bug reported by QA where authenticator app secret keys were not displayed following a validation error. The fix ensures the secret will display both: - where the code format is invalid and therefore fails the validation rules as set in setup-authenticator-app-validation.ts - where the code is incorrect, as demonstrated by the verifyMfaCode service not returning a success The related integration tests have also been updated to prevent passing where the code is not included (the previous non-empty test would always pass because the text included whitespace and line breaks) --- .../setup-authenticator-app-controller.ts | 4 +++- .../setup-authenticator-app-validation.ts | 9 +++++++-- .../tests/setup-authenticator-app-integration.test.ts | 9 +++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/components/setup-authenticator-app/setup-authenticator-app-controller.ts b/src/components/setup-authenticator-app/setup-authenticator-app-controller.ts index b23e07d69..19f42037a 100644 --- a/src/components/setup-authenticator-app/setup-authenticator-app-controller.ts +++ b/src/components/setup-authenticator-app/setup-authenticator-app-controller.ts @@ -102,7 +102,9 @@ export function setupAuthenticatorAppPost( ); return renderBadRequest(res, req, TEMPLATE, error, { qrCode: req.session.user.authAppQrCodeUrl, - secretKey: req.session.user.authAppSecret, + secretKeyFragmentArray: splitSecretKeyIntoFragments( + req.session.user.authAppSecret + ), }); } diff --git a/src/components/setup-authenticator-app/setup-authenticator-app-validation.ts b/src/components/setup-authenticator-app/setup-authenticator-app-validation.ts index 903cc1919..9d0114230 100644 --- a/src/components/setup-authenticator-app/setup-authenticator-app-validation.ts +++ b/src/components/setup-authenticator-app/setup-authenticator-app-validation.ts @@ -2,7 +2,10 @@ import { body } from "express-validator"; import { validateBodyMiddleware } from "../../middleware/form-validation-middleware"; import { ValidationChainFunc } from "../../types"; import { Request } from "express"; -import { containsNumbersOnly } from "../../utils/strings"; +import { + containsNumbersOnly, + splitSecretKeyIntoFragments, +} from "../../utils/strings"; export function validateSetupAuthAppRequest(): ValidationChainFunc { return [ @@ -47,6 +50,8 @@ const postValidationLocals = function locals( ): Record { return { qrCode: req.session.user.authAppQrCodeUrl, - secretKey: req.session.user.authAppSecret, + secretKeyFragmentArray: splitSecretKeyIntoFragments( + req.session.user.authAppSecret + ), }; }; diff --git a/src/components/setup-authenticator-app/tests/setup-authenticator-app-integration.test.ts b/src/components/setup-authenticator-app/tests/setup-authenticator-app-integration.test.ts index c9c73b159..dd2db11e1 100644 --- a/src/components/setup-authenticator-app/tests/setup-authenticator-app-integration.test.ts +++ b/src/components/setup-authenticator-app/tests/setup-authenticator-app-integration.test.ts @@ -15,6 +15,7 @@ describe("Integration::setup-authenticator-app", () => { let cookies: string; let app: any; let baseApi: string; + const AUTH_APP_SECRET: string = "MJRGA2KMETI7BEVNT33MOITMEQQUJMAQ"; before(async () => { decache("../../../app"); @@ -33,7 +34,7 @@ describe("Integration::setup-authenticator-app", () => { journey: { nextPath: PATH_NAMES.CREATE_ACCOUNT_SETUP_AUTHENTICATOR_APP, }, - authAppSecret: "secret", + authAppSecret: AUTH_APP_SECRET, }; next(); @@ -89,7 +90,7 @@ describe("Integration::setup-authenticator-app", () => { expect($("#code-error").text()).to.contains( "Enter the security code shown in your authenticator app" ); - expect($("#secret-key").text()).to.not.be.empty; + expect($("#secret-key").text()).to.contain(AUTH_APP_SECRET); }) .expect(400, done); }); @@ -108,7 +109,7 @@ describe("Integration::setup-authenticator-app", () => { expect($("#code-error").text()).to.contains( "Enter the security code using only 6 digits" ); - expect($("#secret-key").text()).to.not.be.empty; + expect($("#secret-key").text()).to.contain(AUTH_APP_SECRET); }) .expect(400, done); }); @@ -127,7 +128,7 @@ describe("Integration::setup-authenticator-app", () => { expect($("#code-error").text()).to.contains( "Enter the security code using only 6 digits" ); - expect($("#secret-key").text()).to.not.be.empty; + expect($("#secret-key").text()).to.contain(AUTH_APP_SECRET); }) .expect(400, done); });