feat(mfa): mfa challenge expiration check

Author: pradel

packages/mfa/__tests__/accounts-mfa.ts

@@ -27,4 +27,21 @@ describe('AccountsMfa', () => {
       ).rejects.toThrowError('Invalid mfa token');
     });
   });
+
+  describe('isMfaChallengeValid', () => {
+    it('should return false if challenge is deactivated', () => {
+      const accountsMfa = new AccountsMfa({ factors: {} });
+      expect(accountsMfa.isMfaChallengeValid({ deactivated: true } as any)).toBe(false);
+    });
+
+    it('should return false if challenge is expired', () => {
+      const accountsMfa = new AccountsMfa({ factors: {} });
+      expect(accountsMfa.isMfaChallengeValid({ createdAt: 100 } as any)).toBe(false);
+    });
+
+    it('should return true if challenge is valid', () => {
+      const accountsMfa = new AccountsMfa({ factors: {} });
+      expect(accountsMfa.isMfaChallengeValid({ createdAt: Date.now() } as any)).toBe(true);
+    });
+  });
 });

packages/mfa/package.json

@@ -23,12 +23,14 @@
     "preset": "ts-jest"
   },
   "dependencies": {
+    "ms": "2.1.2",
     "tslib": "2.0.1"
   },
   "devDependencies": {
     "@accounts/server": "^0.29.0",
     "@accounts/types": "^0.29.0",
     "@types/jest": "26.0.9",
+    "@types/ms": "0.7.31",
     "@types/node": "14.0.27",
     "jest": "26.3.0",
     "rimraf": "3.0.2"

packages/mfa/src/accounts-mfa.ts

@@ -8,6 +8,7 @@ import {
   MfaChallenge,
 } from '@accounts/types';
 import { AccountsServer, AccountsJsError } from '@accounts/server';
+import ms from 'ms';
 import { ErrorMessages } from './types';
 import {
   errors,
@@ -27,6 +28,11 @@ export interface AccountsMfaOptions {
    * Factors used for mfa
    */
   factors: { [key: string]: AuthenticatorService | undefined };
+  /**
+   * Expressed in milliseconds or a string describing a time span zeit/ms.
+   * Default to '5m'.
+   */
+  expiresIn?: string | number;
 }
 
 interface AccountsMfaAuthenticateParams {
@@ -35,6 +41,7 @@ interface AccountsMfaAuthenticateParams {
 
 const defaultOptions = {
   errors,
+  expiresIn: '5m' as string | number,
 };
 
 export class AccountsMfa<CustomUser extends User = User> implements AuthenticationService {
@@ -46,7 +53,11 @@ export class AccountsMfa<CustomUser extends User = User> implements Authenticati
 
   constructor(options: AccountsMfaOptions) {
     this.options = { ...defaultOptions, ...options };
-    this.factors = options.factors;
+    if (typeof this.options.expiresIn === 'string') {
+      const milliseconds = ms(this.options.expiresIn);
+      this.options.expiresIn = milliseconds;
+    }
+    this.factors = this.options.factors;
   }
 
   public setStore(store: DatabaseInterface<CustomUser>) {
@@ -66,7 +77,7 @@ export class AccountsMfa<CustomUser extends User = User> implements Authenticati
   ): Promise<CustomUser | null> {
     const mfaToken = params.mfaToken;
     const mfaChallenge = mfaToken ? await this.db.findMfaChallengeByToken(mfaToken) : null;
-    if (!mfaChallenge || !mfaChallenge.authenticatorId) {
+    if (!mfaChallenge || !mfaChallenge.authenticatorId || !this.isMfaChallengeValid(mfaChallenge)) {
       throw new AccountsJsError(
         this.options.errors.invalidMfaToken,
         AuthenticateErrors.InvalidMfaToken
@@ -86,7 +97,6 @@ export class AccountsMfa<CustomUser extends User = User> implements Authenticati
         AuthenticateErrors.FactorNotFound
       );
     }
-    // TODO we need to implement some time checking for the mfaToken (eg: expire after X minutes, probably based on the authenticator configuration)
     if (!(await factor.authenticate(mfaChallenge, authenticator, params, infos))) {
       throw new AccountsJsError(
         this.options.errors.authenticationFailed(authenticator.type),
@@ -263,10 +273,15 @@ export class AccountsMfa<CustomUser extends User = User> implements Authenticati
   }
 
   public isMfaChallengeValid(mfaChallenge: MfaChallenge): boolean {
-    // TODO need to check that the challenge is not expired
     if (mfaChallenge.deactivated) {
       return false;
     }
+    const now = Date.now();
+    const expirationDate =
+      new Date(mfaChallenge.createdAt).getTime() + (this.options.expiresIn as number);
+    if (now > expirationDate) {
+      return false;
+    }
     return true;
   }
 }

packages/types/src/types/mfa/mfa-challenge.ts

@@ -27,6 +27,10 @@ export interface MfaChallenge {
    * If deactivated is true, contain the date when the challenge was used
    */
   deactivatedAt?: string;
+  /**
+   * Contain the date when the challenge was created, is used to check that the challenge is not expired
+   */
+  createdAt: string;
   /**
    * Custom properties
    */

yarn.lock

@@ -255,8 +255,10 @@ __metadata:
     "@accounts/server": ^0.29.0
     "@accounts/types": ^0.29.0
     "@types/jest": 26.0.9
+    "@types/ms": 0.7.31
     "@types/node": 14.0.27
     jest: 26.3.0
+    ms: 2.1.2
     rimraf: 3.0.2
     tslib: 2.0.1
   peerDependencies:
@@ -6097,6 +6099,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/ms@npm:0.7.31":
+  version: 0.7.31
+  resolution: "@types/ms@npm:0.7.31"
+  checksum: 7ff9798a408aaa371f3d0059f013d09b6eb471cd62019f35924301608f57279943b5d322f497f2ea9ae65477e2a47c26dfd8dd833a4352f883a8a86c051f7125
+  languageName: node
+  linkType: hard
+
 "@types/node-fetch@npm:2.5.7":
   version: 2.5.7
   resolution: "@types/node-fetch@npm:2.5.7"