fix(core): stabilize adaptive MFA hook delivery by darcyYe · Pull Request #8448 · logto-io/logto

darcyYe · 2026-03-09T09:02:16Z

Summary

dispatch PostSignInAdaptiveMfaTriggered from the experience interaction hook middleware finally block so adaptive MFA challenge submits still emit the webhook when /experience/submit returns session.mfa.require_mfa_verification
only enqueue the adaptive MFA interaction hook while the current interaction still requires MFA verification, so the same sign-in flow does not deliver duplicate adaptive MFA webhooks after the MFA challenge succeeds
add regression unit and integration coverage for the failed-submit adaptive MFA path and verify PostSignIn remains success-only in experience MFA challenge flows

Testing

Integration tests

Checklist

.changeset
unit tests
integration tests
necessary TSDoc comments

github-actions · 2026-03-09T09:02:31Z

COMPARE TO `master`

Total Size Diff ⚠️ 📈 +23.97 KB

Diff by File

Name	Diff
packages/core/src/routes/experience/classes/experience-interaction.adaptive-mfa.test.ts	📈 +5.65 KB
packages/core/src/routes/experience/classes/experience-interaction.ts	📈 +1023 Bytes
packages/core/src/routes/experience/middleware/interaction-hook-dispatch.ts	📈 +930 Bytes
packages/core/src/routes/experience/middleware/koa-experience-interaction-hooks.test.ts	📈 +5.88 KB
packages/core/src/routes/experience/middleware/koa-experience-interaction-hooks.ts	📈 +1.54 KB
packages/core/src/routes/experience/types.ts	📈 +177 Bytes
packages/integration-tests/src/tests/api/hook/hook.trigger.experience.adaptive-mfa.test.ts	📈 +4.18 KB
packages/integration-tests/src/tests/api/hook/hook.trigger.experience.test.ts	📈 +2.63 KB
packages/integration-tests/src/tests/api/hook/utils.test.ts	📈 +1.15 KB
packages/integration-tests/src/tests/api/hook/utils.ts	📈 +882 Bytes

Copilot

Pull request overview

Stabilizes delivery semantics for the adaptive MFA interaction hook so the webhook is still emitted when an MFA challenge causes /experience/submit to fail with session.mfa.require_mfa_verification, while preventing duplicate adaptive MFA hook deliveries across submit retries within the same interaction.

Changes:

Dispatch interaction hooks from the experience interaction-hook middleware finally block (keeping data hooks success-only).
Only enqueue PostSignInAdaptiveMfaTriggered when MFA is required and not yet verified for the current interaction.
Add unit + integration regressions covering the failed-submit adaptive MFA path and ensuring PostSignIn remains success-only across MFA challenge flows.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/integration-tests/src/tests/api/hook/utils.ts	Adds polling helpers for webhook log fetching; updates negative/positive assertions behavior.
packages/integration-tests/src/tests/api/hook/hook.trigger.experience.test.ts	Adds integration regression ensuring `PostSignIn` is emitted only after MFA challenge completion.
packages/integration-tests/src/tests/api/hook/hook.trigger.experience.adaptive-mfa.test.ts	Adds integration regression for failed-submit adaptive MFA hook emission + no duplicates + `PostSignIn` success-only.
packages/core/src/routes/experience/middleware/koa-experience-interaction-hooks.ts	Moves interaction hook dispatch into `finally`; keeps data hook dispatch success-only.
packages/core/src/routes/experience/middleware/koa-experience-interaction-hooks.test.ts	Adds unit coverage verifying interaction hooks are dispatched on error when results were assigned.
packages/core/src/routes/experience/classes/experience-interaction.ts	Prevents adaptive MFA hook enqueueing once MFA is already verified in the current interaction.
packages/core/src/routes/experience/classes/experience-interaction.adaptive-mfa.test.ts	Adds unit regression ensuring no adaptive MFA hook is assigned when MFA is already satisfied.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

packages/core/src/routes/experience/classes/experience-interaction.adaptive-mfa.test.ts

Copilot · 2026-03-10T02:58:31Z

packages/integration-tests/src/tests/api/hook/utils.ts

+  const logEntry = assertions.toBeUndefined
+    ? await getLatestHookLogAfterDelay(hookId, event)
+    : await waitForLatestHookLog(hookId, event);



assertHookLogResult uses only a single 100ms delay before asserting toBeUndefined. Since webhook delivery is async, this can produce false passes if the hook arrives slightly later (especially on slower CI), making negative assertions flaky. Consider polling for a bounded window (similar to waitForLatestHookLog) and only then asserting undefined, or otherwise ensuring the test observes a sufficient “no log” period.

packages/core/src/routes/experience/classes/experience-interaction.adaptive-mfa.test.ts

simeng-li · 2026-03-10T06:03:19Z

packages/core/src/routes/experience/middleware/koa-experience-interaction-hooks.ts

      if (interactionHookContext.interactionHookResults.length > 0) {
+        // Interaction hooks are queued when the corresponding interaction event is known to
+        // have happened. `PostSignInAdaptiveMfaTriggered` is assigned before the MFA
+        // challenge error is thrown, so dispatch must happen in `finally`.
+        // If future interaction hooks need mixed semantics, split them by dispatch policy
+        // (for example, success-only vs finally) instead of sending every interaction hook
+        // on failed submits.
        // Hooks should not crash the app
        void trySafe(
          triggerInteractionHooks(getConsoleLogFromContext(ctx), interactionHookContext)
        );
      }


This is a breaking change? triggerInteractionHooks includes PostSignIn/PostRegister events, which should only triggered if the interaction submit succesfully.

Fixed.

I split the experience interaction hook dispatch into two paths:

success-only interaction hooks are only released after next() completes successfully

PostSignInAdaptiveMfaTriggered is still released from finally, so it is emitted when MFA challenge submission throws session.mfa.require_mfa_verification

This keeps the adaptive MFA webhook behavior while preventing default interaction hooks like PostSignIn from being sent on failed submits.

I also extracted the dispatch helpers into interaction-hook-dispatch.ts to keep the middleware logic focused, and added middleware coverage for both cases:

failed submit only releases the adaptive MFA hook

successful submit releases the success-only hooks separately from the release-anyway hook

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

Adaptive MFA assigns the interaction hook result before MFA verification challenge errors are thrown, but the experience interaction middleware only dispatched interaction hooks after successful requests. That made PostSignInAdaptiveMfaTriggered disappear when the first submit returned session.mfa.require_mfa_verification. Dispatch interaction hooks in the middleware finally block so assigned interaction events are preserved for adaptive MFA challenge flows. Add unit and integration coverage for the failed submit path.

Run the repository lint fix pass after the adaptive MFA hook change. This picks up the import ordering change in the new integration test so the branch stays clean before push.

Document why data hooks remain success-only while interaction hooks dispatch from the experience middleware finally block. Note that future interaction hooks may need explicit success-only vs finally dispatch policies if their semantics diverge from adaptive MFA.

Dispatching interaction hooks from the experience middleware finally block made adaptive MFA webhooks visible on failed submit requests, but it also exposed that the same sign-in flow could enqueue PostSignInAdaptiveMfaTriggered again after the MFA challenge succeeded. Only emit the adaptive MFA interaction hook while the current interaction still requires MFA verification, and add regression coverage to keep PostSignIn success-only in MFA challenge flows.

CI failed in the integration-tests lint job because the new hook regression coverage introduced import-order and prettier violations. Only fix the lint errors in the new hook regression tests so the behavioral changes stay unchanged.

github-actions bot added bugfix size/m labels Mar 9, 2026

github-actions bot assigned darcyYe Mar 9, 2026

github-actions bot added size/m and removed size/m labels Mar 9, 2026

darcyYe changed the title ~~fix(core): dispatch adaptive MFA hook for failed submit~~ fix(core): stabilize adaptive MFA hook delivery Mar 9, 2026

github-actions bot added size/l and removed size/m size/l labels Mar 9, 2026

darcyYe force-pushed the yemq-log-13043-adaptive-mfa-hook-on-failed-submit branch from bd1e666 to 6563093 Compare March 9, 2026 15:19

github-actions bot added size/l and removed size/l labels Mar 9, 2026

darcyYe force-pushed the yemq-log-13043-adaptive-mfa-hook-on-failed-submit branch from 6563093 to 6c10345 Compare March 9, 2026 15:51

github-actions bot added size/l and removed size/l labels Mar 9, 2026

darcyYe marked this pull request as ready for review March 10, 2026 02:54

darcyYe requested a review from simeng-li as a code owner March 10, 2026 02:54

Copilot AI review requested due to automatic review settings March 10, 2026 02:54

darcyYe requested review from gao-sun and wangsijie as code owners March 10, 2026 02:54

Copilot started reviewing on behalf of darcyYe March 10, 2026 02:54 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

simeng-li reviewed Mar 10, 2026

View reviewed changes

packages/core/src/routes/experience/classes/experience-interaction.ts Show resolved Hide resolved

darcyYe force-pushed the yemq-log-13043-adaptive-mfa-hook-on-failed-submit branch from 6c10345 to b92100b Compare March 10, 2026 09:44

github-actions bot added size/l and removed size/l labels Mar 10, 2026

github-actions bot added size/xl and removed size/xl labels Mar 10, 2026

darcyYe force-pushed the yemq-log-13043-adaptive-mfa-hook-on-failed-submit branch from 09e1f0b to 36393c0 Compare March 10, 2026 13:09

github-actions bot added size/xl and removed size/xl labels Mar 10, 2026

darcyYe added 11 commits March 12, 2026 12:07

test: apply lint fixes for adaptive MFA hook coverage

0433de5

Run the repository lint fix pass after the adaptive MFA hook change. This picks up the import ordering change in the new integration test so the branch stays clean before push.

test: fix hook regression lint errors

a1df9c2

CI failed in the integration-tests lint job because the new hook regression coverage introduced import-order and prettier violations. Only fix the lint errors in the new hook regression tests so the behavioral changes stay unchanged.

chore: stabilize hook regression checks

6008395

fix(core): address adaptive mfa hook review feedback

bb234e3

fix(core): split experience interaction hook dispatch

05f06ca

test: fix jest timer typing

6020baa

fix(core): persist adaptive MFA hook across retries

aeed458

fix(core): remove adaptive MFA hook interaction state

cf11f1e

darcyYe force-pushed the yemq-log-13043-adaptive-mfa-hook-on-failed-submit branch from 67eac9f to cf11f1e Compare March 12, 2026 04:08

github-actions bot added size/xl and removed size/xl labels Mar 12, 2026

fix(core): dedupe adaptive MFA hook per sign-in flow

4b1efb8

github-actions bot added size/xl and removed size/xl labels Mar 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): stabilize adaptive MFA hook delivery#8448

fix(core): stabilize adaptive MFA hook delivery#8448
darcyYe wants to merge 12 commits intomasterfrom
yemq-log-13043-adaptive-mfa-hook-on-failed-submit

darcyYe commented Mar 9, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Uh oh!

simeng-li Mar 10, 2026

Uh oh!

darcyYe Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

darcyYe commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Checklist

Uh oh!

github-actions bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

COMPARE TO master

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

simeng-li Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

darcyYe Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

darcyYe commented Mar 9, 2026 •

edited

Loading

github-actions bot commented Mar 9, 2026 •

edited

Loading

COMPARE TO `master`