feat(EMS-1745): Account password reset - invalid link page/redirection (

#554) * feat(EMS-1745): account password reset - redirect to link invalid page if token is invalid * feat(EMS-1745): fix/update GQL schema
UK-Export-Finance · Jun 20, 2023 · aa6edb4 · aa6edb4
1 parent 5e6b728
commit aa6edb4
Show file tree

Hide file tree

Showing 18 changed files with 134 additions and 59 deletions.
diff --git a/e2e-tests/constants/routes/insurance/account.js b/e2e-tests/constants/routes/insurance/account.js
@@ -28,6 +28,7 @@ const PASSWORD_RESET = {
   ROOT: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}`,
   LINK_SENT: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/link-sent`,
   LINK_EXPIRED: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/link-expired`,
+  LINK_INVALID: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/invalid-link`,
   NEW_PASSWORD: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/new-password`,
   SUCCESS: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/success`,
 };

diff --git a/...e2e/journeys/insurance/account/password-reset/account-password-reset-link-invalid.spec.js b/...e2e/journeys/insurance/account/password-reset/account-password-reset-link-invalid.spec.js
@@ -0,0 +1,21 @@
+import { INSURANCE_ROUTES as ROUTES } from '../../../../../../constants/routes/insurance';
+
+const {
+  ACCOUNT: {
+    PASSWORD_RESET: { NEW_PASSWORD, LINK_INVALID },
+  },
+} = ROUTES;
+
+context('Insurance - Account - Password reset - new password page - Visit with an invalid token query param', () => {
+  const baseUrl = Cypress.config('baseUrl');
+  const newPasswordUrl = `${baseUrl}${NEW_PASSWORD}?token=invalid`;
+  const linkExpiredUrl = `${baseUrl}${LINK_INVALID}`;
+
+  before(() => {
+    cy.navigateToUrl(newPasswordUrl);
+  });
+
+  it(`should redirect to ${LINK_INVALID}`, () => {
+    cy.url().should('eq', linkExpiredUrl);
+  });
+});
diff --git a/...insurance/account/password-reset/link-expired/account-password-reset-link-invalid.spec.js b/...insurance/account/password-reset/link-expired/account-password-reset-link-invalid.spec.js
diff --git a/src/api/.keystone/config.js b/src/api/.keystone/config.js
@@ -1428,6 +1428,7 @@ var typeDefs = `
     success: Boolean!
     token: String
     expired: Boolean
+    invalid: Boolean
     accountId: String
   }
 
@@ -4296,22 +4297,29 @@ var verifyAccountPasswordResetToken = async (root, variables, context) => {
   try {
     const { token } = variables;
     const account = await get_account_by_field_default(context, PASSWORD_RESET_HASH, token);
-    if (!account) {
-      console.info("Unable to verify account password reset token - account does not exist");
-      return { success: false };
-    }
-    const now = /* @__PURE__ */ new Date();
-    const hasExpired = (0, import_date_fns10.isAfter)(now, account[PASSWORD_RESET_EXPIRY]);
-    if (hasExpired) {
-      console.info("Account password reset token has expired");
+    if (account) {
+      const now = /* @__PURE__ */ new Date();
+      const hasExpired = (0, import_date_fns10.isAfter)(now, account[PASSWORD_RESET_EXPIRY]);
+      if (hasExpired) {
+        console.info("Unable to verify account password reset token - token has expired");
+        return {
+          success: false,
+          expired: true,
+          accountId: account.id
+        };
+      }
+      console.info("Successfully verified account password reset token");
       return {
-        success: false,
-        expired: true,
-        accountId: account.id
+        success: true
       };
     }
-    return { success: true };
+    console.info(`Unable to verify account password reset token - no account found from the provided ${PASSWORD_RESET_HASH}`);
+    return {
+      success: false,
+      invalid: true
+    };
   } catch (err) {
+    console.error(err);
     throw new Error(`Verifying account password reset token ${err}`);
   }
 };

diff --git a/src/api/custom-resolvers/mutations/verify-account-email-address/index.ts b/src/api/custom-resolvers/mutations/verify-account-email-address/index.ts
@@ -66,6 +66,7 @@ const verifyAccountEmailAddress = async (root: any, variables: VerifyEmailAddres
     };
   } catch (err) {
     console.error(err);
+
     throw new Error(`Verifying account email address ${err}`);
   }
 };

diff --git a/src/api/custom-resolvers/queries/verify-account-password-reset-token/index.test.ts b/src/api/custom-resolvers/queries/verify-account-password-reset-token/index.test.ts
@@ -38,7 +38,7 @@ describe('custom-resolvers/verify-account-password-reset-token', () => {
   });
 
   describe(`when the account does not have ${PASSWORD_RESET_HASH}`, () => {
-    test('it should return success=false', async () => {
+    test('it should return success=false and invalid=true', async () => {
       // update the account so it does not have a PASSWORD_RESET_HASH
       await context.query.Account.updateOne({
         where: { id: account.id },
@@ -49,7 +49,10 @@ describe('custom-resolvers/verify-account-password-reset-token', () => {
 
       result = await verifyAccountPasswordResetToken({}, variables, context);
 
-      const expected = { success: false };
+      const expected = {
+        success: false,
+        invalid: true,
+      };
 
       expect(result).toEqual(expected);
     });
@@ -81,14 +84,37 @@ describe('custom-resolvers/verify-account-password-reset-token', () => {
     });
   });
 
+  describe(`when no account is found from the provided ${PASSWORD_RESET_HASH}`, () => {
+    test('it should return success=false and invalid=true', async () => {
+      // update the account so PASSWORD_RESET_HASH has a value
+      await context.query.Account.updateOne({
+        where: { id: account.id },
+        data: {
+          [PASSWORD_RESET_HASH]: mockAccount[PASSWORD_RESET_HASH],
+        },
+      });
+
+      variables.token = 'invalid';
+
+      result = await verifyAccountPasswordResetToken({}, variables, context);
+
+      const expected = { success: false, invalid: true };
+
+      expect(result).toEqual(expected);
+    });
+  });
+
   describe('when no account is found', () => {
     test('it should return success=false', async () => {
       // wipe accounts so an account will not be found.
       await accounts.deleteAll(context);
 
       result = await verifyAccountPasswordResetToken({}, variables, context);
 
-      const expected = { success: false };
+      const expected = {
+        success: false,
+        invalid: true,
+      };
 
       expect(result).toEqual(expected);
     });

diff --git a/src/api/custom-resolvers/queries/verify-account-password-reset-token/index.ts b/src/api/custom-resolvers/queries/verify-account-password-reset-token/index.ts
@@ -26,32 +26,41 @@ const verifyAccountPasswordResetToken = async (
   try {
     const { token } = variables;
 
-    // Get the account the token is associated with.
+    // get the account the token is associated with.
     const account = await getAccountByField(context, PASSWORD_RESET_HASH, token);
 
-    if (!account) {
-      console.info('Unable to verify account password reset token - account does not exist');
+    if (account) {
+      // check that the reset token period has not expired.
+      const now = new Date();
 
-      return { success: false };
-    }
+      const hasExpired = isAfter(now, account[PASSWORD_RESET_EXPIRY]);
 
-    // check that the reset token period has not expired.
-    const now = new Date();
+      if (hasExpired) {
+        console.info('Unable to verify account password reset token - token has expired');
 
-    const hasExpired = isAfter(now, account[PASSWORD_RESET_EXPIRY]);
+        return {
+          success: false,
+          expired: true,
+          accountId: account.id,
+        };
+      }
 
-    if (hasExpired) {
-      console.info('Account password reset token has expired');
+      console.info('Successfully verified account password reset token');
 
       return {
-        success: false,
-        expired: true,
-        accountId: account.id,
+        success: true,
       };
     }
 
-    return { success: true };
+    console.info(`Unable to verify account password reset token - no account found from the provided ${PASSWORD_RESET_HASH}`);
+
+    return {
+      success: false,
+      invalid: true,
+    };
   } catch (err) {
+    console.error(err);
+
     throw new Error(`Verifying account password reset token ${err}`);
   }
 };

diff --git a/src/api/custom-schema/type-defs.ts b/src/api/custom-schema/type-defs.ts
@@ -142,6 +142,7 @@ const typeDefs = `
     success: Boolean!
     token: String
     expired: Boolean
+    invalid: Boolean
     accountId: String
   }
 

diff --git a/src/api/schema.graphql b/src/api/schema.graphql
@@ -2382,6 +2382,7 @@ type AccountPasswordResetTokenResponse {
   success: Boolean!
   token: String
   expired: Boolean
+  invalid: Boolean
   accountId: String
 }
 

diff --git a/src/api/test-helpers/accounts.ts b/src/api/test-helpers/accounts.ts
@@ -21,7 +21,8 @@ const create = async (context: Context, accountData?: Account) => {
 
     const account = (await context.query.Account.createOne({
       data: accountInput,
-      query: 'id email firstName lastName email salt hash verificationHash sessionExpiry otpExpiry reactivationHash reactivationExpiry isVerified isBlocked',
+      query:
+        'id email firstName lastName email salt hash verificationHash sessionExpiry otpExpiry reactivationHash reactivationExpiry isVerified isBlocked passwordResetHash passwordResetExpiry',
     })) as Account;
 
     return account;

diff --git a/src/api/types.ts b/src/api/types.ts
@@ -289,6 +289,7 @@ interface VerifyAccountPasswordResetTokenVariables {
 interface AccountPasswordResetTokenResponse extends SuccessResponse {
   token?: string;
   expired?: boolean;
+  invalid?: boolean;
   accountId?: string;
 }
 

diff --git a/src/ui/server/constants/routes/insurance/account.ts b/src/ui/server/constants/routes/insurance/account.ts
@@ -28,6 +28,7 @@ const PASSWORD_RESET = {
   ROOT: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}`,
   LINK_SENT: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/link-sent`,
   LINK_EXPIRED: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/link-expired`,
+  LINK_INVALID: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/invalid-link`,
   NEW_PASSWORD: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/new-password`,
   SUCCESS: `${INSURANCE_ROOT}${PASSWORD_RESET_ROOT}/success`,
 };

diff --git a/src/ui/server/controllers/insurance/account/password-reset/new-password/index.test.ts b/src/ui/server/controllers/insurance/account/password-reset/new-password/index.test.ts
@@ -16,7 +16,7 @@ const {
 const {
   INSURANCE: {
     ACCOUNT: {
-      PASSWORD_RESET: { ROOT: PASSWORD_RESET_ROOT, SUCCESS, LINK_EXPIRED },
+      PASSWORD_RESET: { ROOT: PASSWORD_RESET_ROOT, SUCCESS, LINK_EXPIRED, LINK_INVALID },
     },
     PROBLEM_WITH_SERVICE,
   },
@@ -99,10 +99,10 @@ describe('controllers/insurance/account/password-reset/new-password', () => {
         api.keystone.account.verifyPasswordResetToken = verifyAccountPasswordResetTokenSpy;
       });
 
-      it(`should redirect to ${LINK_EXPIRED}`, async () => {
+      it(`should redirect to ${LINK_INVALID}`, async () => {
         await get(req, res);
 
-        expect(res.redirect).toHaveBeenCalledWith(LINK_EXPIRED);
+        expect(res.redirect).toHaveBeenCalledWith(LINK_INVALID);
       });
     });
 
@@ -126,6 +126,25 @@ describe('controllers/insurance/account/password-reset/new-password', () => {
       });
     });
 
+    describe('when the api.keystone.account.verifyPasswordResetToken returns invalid=true', () => {
+      beforeEach(() => {
+        verifyAccountPasswordResetTokenSpy = jest.fn(() =>
+          Promise.resolve({
+            success: false,
+            invalid: true,
+          }),
+        );
+
+        api.keystone.account.verifyPasswordResetToken = verifyAccountPasswordResetTokenSpy;
+      });
+
+      it(`should redirect to ${LINK_INVALID}`, async () => {
+        await get(req, res);
+
+        expect(res.redirect).toHaveBeenCalledWith(LINK_INVALID);
+      });
+    });
+
     describe('api error handling', () => {
       describe('when the verify account password reset token API call fails', () => {
         beforeEach(() => {

diff --git a/src/ui/server/controllers/insurance/account/password-reset/new-password/index.ts b/src/ui/server/controllers/insurance/account/password-reset/new-password/index.ts
@@ -15,7 +15,7 @@ const {
 const {
   INSURANCE: {
     ACCOUNT: {
-      PASSWORD_RESET: { ROOT: PASSWORD_RESET_ROOT, SUCCESS, LINK_EXPIRED },
+      PASSWORD_RESET: { ROOT: PASSWORD_RESET_ROOT, SUCCESS, LINK_EXPIRED, LINK_INVALID },
     },
     PROBLEM_WITH_SERVICE,
   },
@@ -54,12 +54,12 @@ export const get = async (req: Request, res: Response) => {
 
     const response = await api.keystone.account.verifyPasswordResetToken(token);
 
-    if (response.expired) {
+    if (response.expired && response.accountId) {
       return res.redirect(`${LINK_EXPIRED}?id=${response.accountId}`);
     }
 
-    if (!response.success) {
-      return res.redirect(LINK_EXPIRED);
+    if (response.invalid || !response.success) {
+      return res.redirect(LINK_INVALID);
     }
 
     return res.render(TEMPLATE, {

diff --git a/src/ui/server/graphql/queries/account/verify-account-password-reset-token.ts b/src/ui/server/graphql/queries/account/verify-account-password-reset-token.ts
@@ -5,6 +5,7 @@ const verifyAccountPasswordResetToken = gql`
     verifyAccountPasswordResetToken(token: $token) {
       success
       expired
+      invalid
       accountId
     }
   }

diff --git a/src/ui/server/routes/insurance/account/index.test.ts b/src/ui/server/routes/insurance/account/index.test.ts
@@ -36,7 +36,7 @@ describe('routes/insurance/account', () => {
   });
 
   it('should setup all routes', () => {
-    expect(get).toHaveBeenCalledTimes(24);
+    expect(get).toHaveBeenCalledTimes(25);
     expect(post).toHaveBeenCalledTimes(9);
 
     expect(get).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.CREATE.YOUR_DETAILS, yourDetailsGet);
@@ -68,6 +68,8 @@ describe('routes/insurance/account', () => {
     expect(get).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_EXPIRED, passwordResetLinkExpiredGet);
     expect(post).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_EXPIRED, passwordResetLinkExpiredPost);
 
+    expect(get).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_INVALID, invalidLinkGet);
+
     expect(get).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.NEW_PASSWORD, newPasswordGet);
     expect(post).toHaveBeenCalledWith(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.NEW_PASSWORD, newPasswordPost);
 

diff --git a/src/ui/server/routes/insurance/account/index.ts b/src/ui/server/routes/insurance/account/index.ts
@@ -59,6 +59,8 @@ insuranceAccountRouter.get(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_SENT, pa
 insuranceAccountRouter.get(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_EXPIRED, passwordResetLinkExpiredGet);
 insuranceAccountRouter.post(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_EXPIRED, passwordResetLinkExpiredPost);
 
+insuranceAccountRouter.get(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.LINK_INVALID, invalidLinkGet);
+
 insuranceAccountRouter.get(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.NEW_PASSWORD, newPasswordGet);
 insuranceAccountRouter.post(INSURANCE_ROUTES.ACCOUNT.PASSWORD_RESET.NEW_PASSWORD, newPasswordPost);
 

diff --git a/src/ui/server/routes/insurance/index.test.ts b/src/ui/server/routes/insurance/index.test.ts
@@ -21,7 +21,7 @@ describe('routes/insurance', () => {
   });
 
   it('should setup all routes', () => {
-    expect(get).toHaveBeenCalledTimes(101);
+    expect(get).toHaveBeenCalledTimes(102);
     expect(post).toHaveBeenCalledTimes(93);
 
     expect(get).toHaveBeenCalledWith(INSURANCE_ROUTES.START, startGet);
-Original file line number
+Diff line change
@@ Expand Up @@
         };
       } catch (err) {
         console.error(err);
         throw new Error(`Verifying account email address ${err}`);
       }
     };
@@ Expand Down @@