feat(core): initial implementation of user set mfa secrets / codes (#…

…6654) * feat(core): initial implementation of user set mfa secrets / codes * refactor(core): move totp secret validation to util * chore(core): fix openapi document * chore(core): fix unit tests --------- Co-authored-by: wangsijie <[email protected]>
logto-io · Oct 29, 2024 · 60a6d67 · 60a6d67
1 parent 7620479
commit 60a6d67
Show file tree

Hide file tree

Showing 7 changed files with 174 additions and 14 deletions.
diff --git a/packages/core/src/routes/admin-user/mfa-verification.openapi.json b/packages/core/src/routes/admin-user/mfa-verification.openapi.json
@@ -15,11 +15,39 @@
           "content": {
             "application/json": {
               "schema": {
-                "properties": {
-                  "type": {
-                    "description": "The type of MFA verification to create."
+                "oneOf": [
+                  {
+                    "type": "object",
+                    "properties": {
+                      "type": {
+                        "type": "string",
+                        "description": "The type of MFA verification to create."
+                      },
+                      "secret": {
+                        "type": "string",
+                        "description": "The secret for the MFA verification, if not provided, a new secret will be generated."
+                      }
+                    },
+                    "required": ["type"]
+                  },
+                  {
+                    "type": "object",
+                    "properties": {
+                      "type": {
+                        "type": "string",
+                        "description": "The type of MFA verification to create."
+                      },
+                      "codes": {
+                        "type": "array",
+                        "items": {
+                          "type": "string"
+                        },
+                        "description": "The backup codes for the MFA verification, if not provided, a new group of backup codes will be generated."
+                      }
+                    },
+                    "required": ["type"]
                   }
-                }
+                ]
               }
             }
           }

diff --git a/packages/core/src/routes/admin-user/mfa-verifications.test.ts b/packages/core/src/routes/admin-user/mfa-verifications.test.ts
@@ -59,6 +59,19 @@ const usersLibraries = {
   ),
 } satisfies Partial<Libraries['users']>;
 
+const codes = [
+  'd94c2f29ae',
+  '74fa801bb7',
+  '2cbcc9323c',
+  '87299f89aa',
+  '0d95df8598',
+  '78eedbf35d',
+  '0fa4c1fd19',
+  '7384b69eb5',
+  '7bf2481db7',
+  'f00febc9ae',
+];
+
 const adminUserRoutes = await pickDefault(import('./mfa-verifications.js'));
 
 describe('adminUserRoutes', () => {
@@ -103,6 +116,29 @@ describe('adminUserRoutes', () => {
         });
         expect(response.status).toEqual(422);
       });
+
+      it('should fail for malformed secret', async () => {
+        findUserById.mockResolvedValueOnce(mockUser);
+        const response = await userRequest.post(`/users/${mockUser.id}/mfa-verifications`).send({
+          type: MfaFactor.TOTP,
+          secret: 'invalid_secret',
+        });
+        expect(response.status).toEqual(422);
+      });
+
+      it('should return supplied secret', async () => {
+        findUserById.mockResolvedValueOnce(mockUser);
+        const response = await userRequest.post(`/users/${mockUser.id}/mfa-verifications`).send({
+          type: MfaFactor.TOTP,
+          secret: 'JBSWY3DPEHPK3PXP',
+        });
+        expect(response.body).toMatchObject({
+          type: MfaFactor.TOTP,
+          secret: 'JBSWY3DPEHPK3PXP',
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
+          secretQrCode: expect.stringContaining('data:image'),
+        });
+      });
     });
   });
 
@@ -142,6 +178,45 @@ describe('adminUserRoutes', () => {
       });
       expect(response.status).toEqual(422);
     });
+
+    it('should fail for wrong length', async () => {
+      findUserById.mockResolvedValueOnce({
+        ...mockUser,
+        mfaVerifications: [mockUserTotpMfaVerification],
+      });
+      const response = await userRequest.post(`/users/${mockUser.id}/mfa-verifications`).send({
+        type: MfaFactor.BackupCode,
+        codes: ['wrong-code'],
+      });
+      expect(response.status).toEqual(422);
+    });
+
+    it('should fail for wrong characters', async () => {
+      findUserById.mockResolvedValueOnce({
+        ...mockUser,
+        mfaVerifications: [mockUserTotpMfaVerification],
+      });
+      const response = await userRequest.post(`/users/${mockUser.id}/mfa-verifications`).send({
+        type: MfaFactor.BackupCode,
+        codes: [...codes, '0fa4c1xd19'],
+      });
+      expect(response.status).toEqual(422);
+    });
+
+    it('should return the supplied codes', async () => {
+      findUserById.mockResolvedValueOnce({
+        ...mockUser,
+        mfaVerifications: [mockUserTotpMfaVerification],
+      });
+      const response = await userRequest.post(`/users/${mockUser.id}/mfa-verifications`).send({
+        type: MfaFactor.BackupCode,
+        codes,
+      });
+      expect(response.body).toMatchObject({
+        type: MfaFactor.BackupCode,
+        codes,
+      });
+    });
   });
 
   it('DELETE /users/:userId/mfa-verifications/:verificationId', async () => {

diff --git a/packages/core/src/routes/admin-user/mfa-verifications.ts b/packages/core/src/routes/admin-user/mfa-verifications.ts
@@ -9,8 +9,11 @@ import koaGuard from '#src/middleware/koa-guard.js';
 import assertThat from '#src/utils/assert-that.js';
 import { transpileUserMfaVerifications } from '#src/utils/user.js';
 
-import { generateBackupCodes } from '../interaction/utils/backup-code-validation.js';
-import { generateTotpSecret } from '../interaction/utils/totp-validation.js';
+import {
+  generateBackupCodes,
+  validateBackupCodes,
+} from '../interaction/utils/backup-code-validation.js';
+import { generateTotpSecret, validateTotpSecret } from '../interaction/utils/totp-validation.js';
 import type { ManagementApiRouter, RouterInitArgs } from '../types.js';
 
 export default function adminUserMfaVerificationsRoutes<T extends ManagementApiRouter>(
@@ -47,9 +50,16 @@ export default function adminUserMfaVerificationsRoutes<T extends ManagementApiR
     '/users/:userId/mfa-verifications',
     koaGuard({
       params: object({ userId: string() }),
-      body: z.object({
-        type: z.literal(MfaFactor.TOTP).or(z.literal(MfaFactor.BackupCode)),
-      }),
+      body: z.discriminatedUnion('type', [
+        z.object({
+          type: z.literal(MfaFactor.TOTP),
+          secret: z.string().optional(),
+        }),
+        z.object({
+          type: z.literal(MfaFactor.BackupCode),
+          codes: z.string().array().optional(),
+        }),
+      ]),
       response: z.discriminatedUnion('type', [
         z.object({
           type: z.literal(MfaFactor.TOTP),
@@ -79,7 +89,17 @@ export default function adminUserMfaVerificationsRoutes<T extends ManagementApiR
           })
         );
 
-        const secret = generateTotpSecret();
+        if (ctx.guard.body.secret) {
+          assertThat(
+            validateTotpSecret(ctx.guard.body.secret),
+            new RequestError({
+              code: 'user.totp_secret_invalid',
+              status: 422,
+            })
+          );
+        }
+
+        const secret = ctx.guard.body.secret ?? generateTotpSecret();
         const service = ctx.URL.hostname;
         const user = getUserDisplayName({ username, primaryEmail, primaryPhone, name });
         const keyUri = authenticator.keyuri(user ?? 'Unnamed User', service, secret);
@@ -111,7 +131,16 @@ export default function adminUserMfaVerificationsRoutes<T extends ManagementApiR
           status: 422,
         })
       );
-      const codes = generateBackupCodes();
+      if (ctx.guard.body.codes) {
+        assertThat(
+          validateBackupCodes(ctx.guard.body.codes),
+          new RequestError({
+            code: 'user.wrong_backup_code_format',
+            status: 422,
+          })
+        );
+      }
+      const codes = ctx.guard.body.codes ?? generateBackupCodes();
       await addUserMfaVerification(id, { type: MfaFactor.BackupCode, codes });
       ctx.body = { codes, type: MfaFactor.BackupCode };
       return next();

diff --git a/packages/core/src/routes/interaction/utils/backup-code-validation.ts b/packages/core/src/routes/interaction/utils/backup-code-validation.ts
@@ -8,6 +8,13 @@ const alphabet = '0123456789abcdef';
  * The code is a 10-digit string of letters from 1 to f.
  */
 export const generateBackupCodes = () => {
-  const codes = Array.from({ length: backupCodeCount }, () => customAlphabet(alphabet, 10)());
-  return codes;
+  return Array.from({ length: backupCodeCount }, () => customAlphabet(alphabet, 10)());
+};
+
+/**
+ * Validates a group of backup codes.
+ * @param codes
+ */
+export const validateBackupCodes = (codes: string[]) => {
+  return codes.length === backupCodeCount && codes.every((code) => /^[\da-f]{10}$/i.test(code));
 };
diff --git a/packages/core/src/routes/interaction/utils/totp-validation.test.ts b/packages/core/src/routes/interaction/utils/totp-validation.test.ts
@@ -1,13 +1,25 @@
 const { jest } = import.meta;
 
-const { generateTotpSecret, validateTotpToken } = await import('./totp-validation.js');
+const { generateTotpSecret, validateTotpToken, validateTotpSecret } = await import(
+  './totp-validation.js'
+);
 
 describe('generateTotpSecret', () => {
   it('should generate a secret', () => {
     expect(typeof generateTotpSecret()).toBe('string');
   });
 });
 
+describe('validateTotpSecret', () => {
+  it('should return true on valid secret', () => {
+    expect(validateTotpSecret(generateTotpSecret())).toBe(true);
+  });
+
+  it('should return false on invalid secret', () => {
+    expect(validateTotpSecret('invalid')).toBe(false);
+  });
+});
+
 describe('validateTotpToken', () => {
   beforeAll(() => {
     jest.useFakeTimers();

diff --git a/packages/core/src/routes/interaction/utils/totp-validation.ts b/packages/core/src/routes/interaction/utils/totp-validation.ts
@@ -12,6 +12,13 @@ authenticator.options = { window: 1 };
 
 export const generateTotpSecret = () => authenticator.generateSecret();
 
+export const validateTotpSecret = (secret: string) => {
+  const base32Regex =
+    /^(?:[2-7A-Z]{8})*(?:[2-7A-Z]{2}={6}|[2-7A-Z]{4}={4}|[2-7A-Z]{5}={3}|[2-7A-Z]{7}=)?$/;
+
+  return secret.length >= 16 && secret.length <= 32 && base32Regex.test(secret);
+};
+
 export const validateTotpToken = (secret: string, token: string) => {
   return authenticator.check(token, secret);
 };
diff --git a/packages/phrases/src/locales/en/errors/user.ts b/packages/phrases/src/locales/en/errors/user.ts
@@ -35,6 +35,8 @@ const user = {
   password_algorithm_required: 'Password algorithm is required.',
   password_and_digest: 'You cannot set both plain text password and password digest.',
   personal_access_token_name_exists: 'Personal access token name already exists.',
+  totp_secret_invalid: 'Invalid TOTP secret supplied.',
+  wrong_backup_code_format: 'Backup code format is invalid.',
 };
 
 export default Object.freeze(user);