Skip to content

Commit 6863342

Browse files
committed
feat(core): add TOTP verification to MFA endpoints
1 parent 62cc7ec commit 6863342

File tree

5 files changed

+177
-48
lines changed

5 files changed

+177
-48
lines changed

packages/core/src/routes/account/index.openapi.json

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -284,22 +284,47 @@
284284
"post": {
285285
"operationId": "AddMfaVerification",
286286
"summary": "Add a MFA verification",
287-
"description": "Add a MFA verification to the user, a logto-verification-id in header is required for checking sensitive permissions. Only WebAuthn is supported for now, a new identifier verification record is required for the webauthn registration verification.",
287+
"description": "Add a MFA verification to the user, a logto-verification-id in header is required for checking sensitive permissions.",
288288
"requestBody": {
289289
"content": {
290290
"application/json": {
291291
"schema": {
292-
"properties": {
293-
"newIdentifierVerificationRecordId": {
294-
"description": "The identifier verification record ID for the new WebAuthn registration verification."
295-
},
296-
"type": {
297-
"description": "The type of the MFA verification."
292+
"oneOf": [
293+
{
294+
"type": "object",
295+
"properties": {
296+
"type": {
297+
"type": "string",
298+
"enum": ["WebAuthn"],
299+
"description": "The type of the MFA verification."
300+
},
301+
"newIdentifierVerificationRecordId": {
302+
"type": "string",
303+
"description": "The identifier verification record ID for the new WebAuthn registration verification."
304+
},
305+
"name": {
306+
"type": "string",
307+
"description": "The name of the MFA verification, if not provided, the name will be generated from user agent."
308+
}
309+
},
310+
"required": ["type", "newIdentifierVerificationRecordId"]
298311
},
299-
"name": {
300-
"description": "The name of the MFA verification, if not provided, the name will be generated from user agent."
312+
{
313+
"type": "object",
314+
"properties": {
315+
"type": {
316+
"type": "string",
317+
"enum": ["TOTP"],
318+
"description": "The type of the MFA verification, for TOTP, one user can only bind one TOTP factor."
319+
},
320+
"secret": {
321+
"type": "string",
322+
"description": "The TOTP secret for the MFA verification. Use the generate endpoint to create a secret, and verify the generated code with the user before binding to make sure the user has setup the secret in their authenticator app."
323+
}
324+
},
325+
"required": ["type", "secret"]
301326
}
302-
}
327+
]
303328
}
304329
}
305330
}

packages/core/src/routes/account/mfa-verifications.ts

Lines changed: 74 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import RequestError from '../../errors/RequestError/index.js';
1515
import { buildVerificationRecordByIdAndType } from '../../libraries/verification.js';
1616
import assertThat from '../../utils/assert-that.js';
1717
import { transpileUserMfaVerifications } from '../../utils/user.js';
18-
import { generateTotpSecret } from '../interaction/utils/totp-validation.js';
18+
import { generateTotpSecret, validateTotpSecret } from '../interaction/utils/totp-validation.js';
1919
import type { UserRouter, RouterInitArgs } from '../types.js';
2020

2121
import { accountApiPrefix } from './constants.js';
@@ -59,11 +59,17 @@ export default function mfaVerificationsRoutes<T extends UserRouter>(
5959
router.post(
6060
`${accountApiPrefix}/mfa-verifications`,
6161
koaGuard({
62-
body: z.object({
63-
type: z.literal(MfaFactor.WebAuthn),
64-
newIdentifierVerificationRecordId: z.string(),
65-
name: z.string().optional(),
66-
}),
62+
body: z.discriminatedUnion('type', [
63+
z.object({
64+
type: z.literal(MfaFactor.WebAuthn),
65+
newIdentifierVerificationRecordId: z.string(),
66+
name: z.string().optional(),
67+
}),
68+
z.object({
69+
type: z.literal(MfaFactor.TOTP),
70+
secret: z.string(),
71+
}),
72+
]),
6773
status: [204, 400, 401],
6874
}),
6975
async (ctx, next) => {
@@ -72,7 +78,6 @@ export default function mfaVerificationsRoutes<T extends UserRouter>(
7278
identityVerified,
7379
new RequestError({ code: 'verification_record.permission_denied', status: 401 })
7480
);
75-
const { newIdentifierVerificationRecordId, name } = ctx.guard.body;
7681
const { fields } = ctx.accountCenter;
7782
assertThat(
7883
fields.mfa === AccountCenterControlValue.Edit,
@@ -84,39 +89,72 @@ export default function mfaVerificationsRoutes<T extends UserRouter>(
8489
new RequestError({ code: 'auth.unauthorized', status: 401 })
8590
);
8691

87-
// Check new identifier
88-
const newVerificationRecord = await buildVerificationRecordByIdAndType({
89-
type: VerificationType.WebAuthn,
90-
id: newIdentifierVerificationRecordId,
91-
queries,
92-
libraries,
93-
});
94-
assertThat(newVerificationRecord.isVerified, 'verification_record.not_found');
95-
96-
const bindMfa = newVerificationRecord.toBindMfa();
92+
// Feature flag check, throw 500
93+
if (!EnvSet.values.isDevFeaturesEnabled && ctx.guard.body.type === MfaFactor.TOTP) {
94+
throw new Error('TOTP is not supported yet');
95+
}
9796

9897
const user = await findUserById(userId);
9998

100-
// Check sign in experience, if webauthn is enabled
99+
// Check sign in experience, if mfa factor is enabled
101100
const { mfa } = await findDefaultSignInExperience();
102-
assertThat(
103-
mfa.factors.includes(MfaFactor.WebAuthn),
104-
new RequestError({ code: 'session.mfa.mfa_factor_not_enabled', status: 400 })
105-
);
106-
107-
const updatedUser = await updateUserById(userId, {
108-
mfaVerifications: [
109-
...user.mfaVerifications,
110-
{
111-
...bindMfa,
112-
id: generateStandardId(),
113-
createdAt: new Date().toISOString(),
114-
name,
115-
},
116-
],
117-
});
118-
119-
ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser });
101+
assertThat(mfa.factors.includes(ctx.guard.body.type), 'session.mfa.mfa_factor_not_enabled');
102+
103+
if (ctx.guard.body.type === MfaFactor.TOTP) {
104+
// A user can only bind one TOTP factor
105+
assertThat(
106+
user.mfaVerifications.every(({ type }) => type !== MfaFactor.TOTP),
107+
new RequestError({
108+
code: 'user.totp_already_in_use',
109+
status: 422,
110+
})
111+
);
112+
113+
// Check secret
114+
assertThat(validateTotpSecret(ctx.guard.body.secret), 'user.totp_secret_invalid');
115+
116+
const updatedUser = await updateUserById(userId, {
117+
mfaVerifications: [
118+
...user.mfaVerifications,
119+
{
120+
id: generateStandardId(),
121+
createdAt: new Date().toISOString(),
122+
type: MfaFactor.TOTP,
123+
key: ctx.guard.body.secret,
124+
},
125+
],
126+
});
127+
128+
ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser });
129+
130+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
131+
} else if (ctx.guard.body.type === MfaFactor.WebAuthn) {
132+
const { newIdentifierVerificationRecordId, name } = ctx.guard.body;
133+
// Check new identifier
134+
const newVerificationRecord = await buildVerificationRecordByIdAndType({
135+
type: VerificationType.WebAuthn,
136+
id: newIdentifierVerificationRecordId,
137+
queries,
138+
libraries,
139+
});
140+
assertThat(newVerificationRecord.isVerified, 'verification_record.not_found');
141+
142+
const bindMfa = newVerificationRecord.toBindMfa();
143+
144+
const updatedUser = await updateUserById(userId, {
145+
mfaVerifications: [
146+
...user.mfaVerifications,
147+
{
148+
...bindMfa,
149+
id: generateStandardId(),
150+
createdAt: new Date().toISOString(),
151+
name,
152+
},
153+
],
154+
});
155+
156+
ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser });
157+
}
120158

121159
ctx.status = 204;
122160

packages/integration-tests/src/api/account-center.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const enableAllAccountCenterFields = async (api: KyInstance = authedAdmin
4040
profile: AccountCenterControlValue.Edit,
4141
social: AccountCenterControlValue.Edit,
4242
customData: AccountCenterControlValue.Edit,
43+
mfa: AccountCenterControlValue.Edit,
4344
},
4445
},
4546
api

packages/integration-tests/src/api/my-account.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type UserProfileResponse } from '@logto/schemas';
1+
import { type UserMfaVerificationResponse, type UserProfileResponse } from '@logto/schemas';
22
import { type KyInstance } from 'ky';
33

44
const verificationRecordIdHeader = 'logto-verification-id';
@@ -77,3 +77,16 @@ export const getUserInfo = async (api: KyInstance) =>
7777

7878
export const generateTotpSecret = async (api: KyInstance) =>
7979
api.post('api/my-account/mfa-verifications/totp-secret/generate').json<{ secret: string }>();
80+
81+
export const getMfaVerifications = async (api: KyInstance) =>
82+
api.get('api/my-account/mfa-verifications').json<UserMfaVerificationResponse>();
83+
84+
export const addMfaVerification = async (
85+
api: KyInstance,
86+
verificationRecordId: string,
87+
body: Record<string, unknown>
88+
) =>
89+
api.post('api/my-account/mfa-verifications', {
90+
json: body,
91+
headers: { [verificationRecordIdHeader]: verificationRecordId },
92+
});

packages/integration-tests/src/tests/api/account/mfa.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ import { UserScope } from '@logto/core-kit';
22
import { MfaFactor } from '@logto/schemas';
33

44
import { enableAllAccountCenterFields } from '#src/api/account-center.js';
5-
import { generateTotpSecret } from '#src/api/my-account.js';
65
import {
6+
addMfaVerification,
7+
generateTotpSecret,
8+
getMfaVerifications,
9+
} from '#src/api/my-account.js';
10+
import {
11+
createVerificationRecordByPassword,
712
createWebAuthnRegistrationOptions,
813
verifyWebAuthnRegistration,
914
} from '#src/api/verification-record.js';
@@ -46,6 +51,53 @@ describe('my-account (mfa)', () => {
4651
});
4752
});
4853

54+
devFeatureTest.describe('POST /my-account/mfa-verifications', () => {
55+
devFeatureTest.it('should be able to add totp verification', async () => {
56+
await enableAllAccountCenterFields();
57+
58+
const { user, username, password } = await createDefaultTenantUserWithPassword();
59+
const api = await signInAndGetUserApi(username, password, {
60+
scopes: [UserScope.Profile, UserScope.Identities],
61+
});
62+
const { secret } = await generateTotpSecret(api);
63+
const verificationRecordId = await createVerificationRecordByPassword(api, password);
64+
65+
await addMfaVerification(api, verificationRecordId, {
66+
type: MfaFactor.TOTP,
67+
secret,
68+
});
69+
const mfaVerifications = await getMfaVerifications(api);
70+
71+
expect(mfaVerifications).toHaveLength(1);
72+
expect(mfaVerifications[0]?.type).toBe(MfaFactor.TOTP);
73+
74+
await deleteDefaultTenantUser(user.id);
75+
});
76+
77+
devFeatureTest.it('should fail if totp secret is invalid', async () => {
78+
await enableAllAccountCenterFields();
79+
80+
const { user, username, password } = await createDefaultTenantUserWithPassword();
81+
const api = await signInAndGetUserApi(username, password, {
82+
scopes: [UserScope.Profile, UserScope.Identities],
83+
});
84+
const verificationRecordId = await createVerificationRecordByPassword(api, password);
85+
86+
await expectRejects(
87+
addMfaVerification(api, verificationRecordId, {
88+
type: MfaFactor.TOTP,
89+
secret: 'invalid-totp-secret',
90+
}),
91+
{
92+
code: 'user.totp_secret_invalid',
93+
status: 400,
94+
}
95+
);
96+
97+
await deleteDefaultTenantUser(user.id);
98+
});
99+
});
100+
49101
describe('POST /my-account/mfa-verifications/web-authn/registration', () => {
50102
it('should be able to get webauthn registration options', async () => {
51103
const { user, username, password } = await createDefaultTenantUserWithPassword();

0 commit comments

Comments
 (0)