Skip to content

Commit 28806bb

Browse files
authored
feat(core,experience): additional 2fa suggestion (#7725)
* feat(phrases): add suggest_additional_mfa * feat(core): mfa suggestion * fix(core): respect mfa prompt * feat(experience): mfa suggestion * fix: fix api mfa suggestion test
1 parent ae49280 commit 28806bb

File tree

31 files changed

+518
-15
lines changed

31 files changed

+518
-15
lines changed

packages/core/src/routes/experience/classes/experience-interaction.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,10 @@ export default class ExperienceInteraction {
462462
return;
463463
}
464464

465-
// Verified
466-
await this.guardMfaVerificationStatus();
465+
// Verified, only SignIn requires MFA verification, for register, it does not make sense to verify MFA
466+
if (this.#interactionEvent === InteractionEvent.SignIn) {
467+
await this.guardMfaVerificationStatus();
468+
}
467469

468470
// Revalidate the new profile data if any
469471
await this.profile.validateAvailability();

packages/core/src/routes/experience/classes/mfa.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable max-lines */
12
import { type ToZodObject } from '@logto/connector-kit';
23
import {
34
type BindBackupCode,
@@ -26,13 +27,19 @@ import type Libraries from '#src/tenants/Libraries.js';
2627
import type Queries from '#src/tenants/Queries.js';
2728
import assertThat from '#src/utils/assert-that.js';
2829

30+
import { EnvSet } from '../../../env-set/index.js';
2931
import { type InteractionContext } from '../types.js';
3032

3133
import { getAllUserEnabledMfaVerifications } from './helpers.js';
3234
import { SignInExperienceValidator } from './libraries/sign-in-experience-validator.js';
3335

3436
export type MfaData = {
3537
mfaSkipped?: boolean;
38+
/**
39+
* Whether user skipped the optional suggestion to add another MFA factor during registration.
40+
* This flag lives only in the current interaction and should NOT be persisted to user profile.
41+
*/
42+
additionalBindingSuggestionSkipped?: boolean;
3643
totp?: BindTotp;
3744
webAuthn?: BindWebAuthn[];
3845
backupCode?: BindBackupCode;
@@ -47,6 +54,7 @@ export type SanitizedMfaData = {
4754

4855
export const mfaDataGuard = z.object({
4956
mfaSkipped: z.boolean().optional(),
57+
additionalBindingSuggestionSkipped: z.boolean().optional(),
5058
totp: bindTotpGuard.optional(),
5159
webAuthn: z.array(bindWebAuthnGuard).optional(),
5260
backupCode: bindBackupCodeGuard.optional(),
@@ -80,6 +88,7 @@ const isMfaSkipped = (logtoConfig: JsonObject): boolean => {
8088
export class Mfa {
8189
private readonly signInExperienceValidator: SignInExperienceValidator;
8290
#mfaSkipped?: boolean;
91+
#additionalBindingSuggestionSkipped?: boolean;
8392
#totp?: BindTotp;
8493
#webAuthn?: BindWebAuthn[];
8594
#backupCode?: BindBackupCode;
@@ -91,9 +100,10 @@ export class Mfa {
91100
private readonly interactionContext: InteractionContext
92101
) {
93102
this.signInExperienceValidator = new SignInExperienceValidator(libraries, queries);
94-
const { mfaSkipped, totp, webAuthn, backupCode } = data;
103+
const { mfaSkipped, additionalBindingSuggestionSkipped, totp, webAuthn, backupCode } = data;
95104

96105
this.#mfaSkipped = mfaSkipped;
106+
this.#additionalBindingSuggestionSkipped = additionalBindingSuggestionSkipped;
97107
this.#totp = totp;
98108
this.#webAuthn = webAuthn;
99109
this.#backupCode = backupCode;
@@ -103,6 +113,10 @@ export class Mfa {
103113
return this.#mfaSkipped;
104114
}
105115

116+
get additionalBindingSuggestionSkipped() {
117+
return this.#additionalBindingSuggestionSkipped;
118+
}
119+
106120
get bindMfaFactorsArray(): BindMfa[] {
107121
return [this.#totp, ...(this.#webAuthn ?? []), this.#backupCode].filter(Boolean);
108122
}
@@ -263,6 +277,14 @@ export class Mfa {
263277
this.#backupCode = verificationRecord.toBindMfa();
264278
}
265279

280+
/**
281+
* Mark the optional suggestion as skipped for this interaction.
282+
* No persistence to user account.
283+
*/
284+
skipAdditionalBindingSuggestion() {
285+
this.#additionalBindingSuggestionSkipped = true;
286+
}
287+
266288
/**
267289
* @throws {RequestError} with status 400 if the mfa factors are not enabled in the sign-in experience
268290
*/
@@ -271,6 +293,52 @@ export class Mfa {
271293
await this.checkMfaFactorsEnabledInSignInExperience(newBindMfaFactors);
272294
}
273295

296+
/**
297+
* Optionally suggest user to bind additional MFA factors during registration.
298+
* Encapsulates suggestion logic and throws a 422 with `session.mfa.suggest_additional_mfa`
299+
* when conditions are met.
300+
* The purpose is to suggest another MFA factor if the user has only one Email or Phone factor.
301+
*/
302+
async guardAdditionalBindingSuggestion(
303+
factorsInUser: MfaFactor[],
304+
availableFactors: MfaFactor[]
305+
) {
306+
// Only suggest during registration flow
307+
if (this.interactionContext.getInteractionEvent() !== InteractionEvent.Register) {
308+
return;
309+
}
310+
311+
// If no Email/Phone factor in use, then the user is not registered by Email/Phone
312+
if (
313+
!factorsInUser.includes(MfaFactor.EmailVerificationCode) &&
314+
!factorsInUser.includes(MfaFactor.PhoneVerificationCode)
315+
) {
316+
return;
317+
}
318+
319+
const additionalFactors = availableFactors.filter((factor) => !factorsInUser.includes(factor));
320+
321+
// Respect user's choice to skip suggestion for this interaction
322+
if (this.additionalBindingSuggestionSkipped) {
323+
return;
324+
}
325+
326+
// If user already bound an MFA in this interaction, don't suggest again
327+
if (this.bindMfaFactorsArray.length > 0) {
328+
return;
329+
}
330+
331+
// No available factors to suggest
332+
if (additionalFactors.length === 0) {
333+
return;
334+
}
335+
336+
throw new RequestError(
337+
{ code: 'session.mfa.suggest_additional_mfa', status: 422 },
338+
{ availableFactors: additionalFactors, skippable: true, suggestion: true }
339+
);
340+
}
341+
274342
/**
275343
* @throws {RequestError} with status 422 if the user has not bound the required MFA factors
276344
* @throws {RequestError} with status 422 if the user has not bound the backup code but enabled in the sign-in experience
@@ -333,6 +401,11 @@ export class Mfa {
333401
)
334402
);
335403

404+
if (EnvSet.values.isDevFeaturesEnabled) {
405+
// Optional suggestion: Let Mfa decide whether to suggest additional binding during registration
406+
await this.guardAdditionalBindingSuggestion(factorsInUser, availableFactors);
407+
}
408+
336409
// Assert backup code
337410
assertThat(
338411
!factors.includes(MfaFactor.BackupCode) || linkedFactors.includes(MfaFactor.BackupCode),
@@ -346,6 +419,7 @@ export class Mfa {
346419
get data(): MfaData {
347420
return {
348421
mfaSkipped: this.mfaSkipped,
422+
additionalBindingSuggestionSkipped: this.additionalBindingSuggestionSkipped,
349423
totp: this.#totp,
350424
webAuthn: this.#webAuthn,
351425
backupCode: this.#backupCode,
@@ -397,3 +471,4 @@ export class Mfa {
397471
].filter(Boolean);
398472
}
399473
}
474+
/* eslint-enable max-lines */

packages/core/src/routes/experience/profile-routes.openapi.json

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,31 @@
104104
}
105105
}
106106
},
107+
"/api/experience/profile/mfa/mfa-suggestion-skipped": {
108+
"post": {
109+
"operationId": "SkipMfaSuggestion",
110+
"summary": "Skip additional MFA suggestion",
111+
"tags": ["Dev feature"],
112+
"description": "Mark the optional additional MFA binding suggestion as skipped for the current interaction. When multiple MFA factors are enabled and only an email/phone factor is configured, a suggestion to add another factor may be shown; this endpoint records the choice to skip.",
113+
"responses": {
114+
"204": {
115+
"description": "The suggestion was successfully skipped."
116+
},
117+
"400": {
118+
"description": "Not supported for the current interaction event. The MFA profile API can only be used in the `SignIn` or `Register` interaction."
119+
},
120+
"403": {
121+
"description": "Some MFA factors have already been enabled for the user. The user must verify MFA before updating related settings."
122+
},
123+
"404": {
124+
"description": "The user has not been identified yet. The suggestion state must be associated with an identified user."
125+
},
126+
"422": {
127+
"description": "The suggestion is not skippable under current policy."
128+
}
129+
}
130+
}
131+
},
107132
"/api/experience/profile/mfa": {
108133
"post": {
109134
"operationId": "BindMfaVerification",

packages/core/src/routes/experience/profile-routes.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ function verifiedInteractionGuard<
4444
})
4545
);
4646

47-
await experienceInteraction.guardMfaVerificationStatus();
47+
if (experienceInteraction.interactionEvent === InteractionEvent.SignIn) {
48+
await experienceInteraction.guardMfaVerificationStatus();
49+
}
4850

4951
return next();
5052
};
@@ -184,6 +186,25 @@ export default function interactionProfileRoutes<T extends ExperienceInteraction
184186
}
185187
);
186188

189+
// Mark optional additional MFA binding suggestion as skipped in current interaction
190+
if (EnvSet.values.isDevFeaturesEnabled) {
191+
router.post(
192+
`${experienceRoutes.mfa}/mfa-suggestion-skipped`,
193+
koaGuard({ status: [204, 400, 403, 404, 422] }),
194+
verifiedInteractionGuard(),
195+
async (ctx, next) => {
196+
const { experienceInteraction } = ctx;
197+
198+
experienceInteraction.mfa.skipAdditionalBindingSuggestion();
199+
await experienceInteraction.save();
200+
201+
ctx.status = 204;
202+
203+
return next();
204+
}
205+
);
206+
}
207+
187208
router.post(
188209
`${experienceRoutes.mfa}`,
189210
koaGuard({

packages/experience/src/apis/experience/mfa.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ export const skipMfa = async () => {
6161
return submitInteraction();
6262
};
6363

64+
export const skipMfaSuggestion = async () => {
65+
await api.post(`${experienceApiRoutes.mfa}/mfa-suggestion-skipped`);
66+
return submitInteraction();
67+
};
68+
6469
export const bindMfa = async (
6570
type: MfaFactor,
6671
verificationId: string,

packages/experience/src/hooks/use-mfa-error-handler.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ const useMfaErrorHandler = ({ replace }: Options = {}) => {
107107
const factors = data?.availableFactors ?? [];
108108
const skippable = data?.skippable;
109109
const maskedIdentifiers = data?.maskedIdentifiers;
110+
const suggestion = data?.suggestion;
110111

111112
if (factors.length === 0) {
112113
setToast(error.message);
@@ -119,7 +120,12 @@ const useMfaErrorHandler = ({ replace }: Options = {}) => {
119120
? factors.filter((factor) => factor !== MfaFactor.WebAuthn)
120121
: factors;
121122

122-
await handleMfaRedirect(flow, { availableFactors, skippable, maskedIdentifiers });
123+
await handleMfaRedirect(flow, {
124+
availableFactors,
125+
skippable,
126+
maskedIdentifiers,
127+
suggestion,
128+
});
123129
};
124130
},
125131
[handleMfaRedirect, setToast]
@@ -129,6 +135,8 @@ const useMfaErrorHandler = ({ replace }: Options = {}) => {
129135
() => ({
130136
'user.missing_mfa': handleMfaError(UserMfaFlow.MfaBinding),
131137
'session.mfa.require_mfa_verification': handleMfaError(UserMfaFlow.MfaVerification),
138+
// Optional suggestion to add another MFA during registration
139+
'session.mfa.suggest_additional_mfa': handleMfaError(UserMfaFlow.MfaBinding),
132140
'session.mfa.backup_code_required': async () => startBackupCodeBinding(replace),
133141
}),
134142
[handleMfaError, replace, startBackupCodeBinding]
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { InteractionEvent } from '@logto/schemas';
2+
import { useCallback } from 'react';
3+
4+
import { skipMfaSuggestion } from '@/apis/experience';
5+
6+
import useApi from './use-api';
7+
import useErrorHandler from './use-error-handler';
8+
import useGlobalRedirectTo from './use-global-redirect-to';
9+
import useSubmitInteractionErrorHandler from './use-submit-interaction-error-handler';
10+
11+
/**
12+
* Skip handler for optional "add another MFA" suggestion during registration.
13+
* Only stores skip in current interaction; does not persist to user account.
14+
*/
15+
const useSkipOptionalMfa = () => {
16+
const asyncSkip = useApi(skipMfaSuggestion);
17+
const redirectTo = useGlobalRedirectTo();
18+
19+
const handleError = useErrorHandler();
20+
/**
21+
* Reuse pre-sign-in error handler for post-skip submission flow.
22+
*/
23+
const preSignInErrorHandler = useSubmitInteractionErrorHandler(InteractionEvent.SignIn, {
24+
replace: true,
25+
});
26+
27+
return useCallback(async () => {
28+
const [error, result] = await asyncSkip();
29+
if (error) {
30+
await handleError(error, preSignInErrorHandler);
31+
return;
32+
}
33+
34+
if (result) {
35+
await redirectTo(result.redirectTo);
36+
}
37+
}, [asyncSkip, handleError, preSignInErrorHandler, redirectTo]);
38+
};
39+
40+
export default useSkipOptionalMfa;

packages/experience/src/pages/MfaBinding/TotpBinding/index.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import UserInteractionContext from '@/Providers/UserInteractionContextProvider/U
99
import Divider from '@/components/Divider';
1010
import SwitchMfaFactorsLink from '@/components/SwitchMfaFactorsLink';
1111
import useSkipMfa from '@/hooks/use-skip-mfa';
12+
import useSkipOptionalMfa from '@/hooks/use-skip-optional-mfa';
1213
import ErrorPage from '@/pages/ErrorPage';
1314
import { UserMfaFlow } from '@/types';
1415
import { totpBindingStateGuard } from '@/types/guard';
@@ -24,17 +25,18 @@ const TotpBinding = () => {
2425
const verificationId = verificationIdsMap[VerificationType.TOTP];
2526

2627
const skipMfa = useSkipMfa();
28+
const skipOptionalMfa = useSkipOptionalMfa();
2729

2830
if (!totpBindingState || !verificationId) {
2931
return <ErrorPage title="error.invalid_session" />;
3032
}
3133

32-
const { availableFactors, skippable } = totpBindingState;
34+
const { availableFactors, skippable, suggestion } = totpBindingState;
3335

3436
return (
3537
<SecondaryPageLayout
3638
title="mfa.add_authenticator_app"
37-
onSkip={conditional(skippable && skipMfa)}
39+
onSkip={conditional(skippable && (suggestion ? skipOptionalMfa : skipMfa))}
3840
>
3941
<div className={styles.container}>
4042
<SecretSection {...totpBindingState} />

packages/experience/src/pages/MfaBinding/VerificationCodeMfaBinding/index.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { sendVerificationCode } from '@/apis/experience';
1111
import SwitchMfaFactorsLink from '@/components/SwitchMfaFactorsLink';
1212
import useErrorHandler from '@/hooks/use-error-handler';
1313
import useSkipMfa from '@/hooks/use-skip-mfa';
14+
import useSkipOptionalMfa from '@/hooks/use-skip-optional-mfa';
1415
import IdentifierProfileForm from '@/pages/Continue/IdentifierProfileForm';
1516
import ErrorPage from '@/pages/ErrorPage';
1617
import { UserMfaFlow } from '@/types';
@@ -39,6 +40,7 @@ const VerificationCodeMfaBinding = ({
3940
const [errorMessage, setErrorMessage] = useState<string>();
4041

4142
const skipMfa = useSkipMfa();
43+
const skipOptionalMfa = useSkipOptionalMfa();
4244
const handleError = useErrorHandler();
4345

4446
const clearErrorMessage = useCallback(() => {
@@ -84,13 +86,13 @@ const VerificationCodeMfaBinding = ({
8486
return <ErrorPage title="error.invalid_session" />;
8587
}
8688

87-
const { skippable, availableFactors } = mfaFlowState;
89+
const { skippable, availableFactors, suggestion } = mfaFlowState;
8890

8991
return (
9092
<SecondaryPageLayout
9193
title={titleKey}
9294
description={descriptionKey}
93-
onSkip={conditional(skippable && skipMfa)}
95+
onSkip={conditional(skippable && (suggestion ? skipOptionalMfa : skipMfa))}
9496
>
9597
<IdentifierProfileForm
9698
autoFocus

0 commit comments

Comments
 (0)