Repository navigation
[ECUK In-App 3DS] [App] Refactor MultifactorAuthenticationContextProvider - fix React rules violations and race conditions risks #81197
Description
Background
The MultifactorAuthenticationContextProvider component (src/components/MultifactorAuthentication/Context/Main.tsx) manages the entire Multifactor Authentication (MFA) flow using a useEffect that runs an async process function whenever state changes.
Current Implementation
The component has a complex flow orchestration pattern:
executeScenario()is called to start the MFA flow- A
useEffectwatches for state changes and callsprocess() process()is an async function that:- Reads current state
- Determines the next step via if-statements
- Performs async operations (API calls, biometric prompts)
- Updates state to trigger the next iteration
useEffect(() => {
if (!state.scenario) {
return;
}
process();
// We intentionally omit `process` and `state` from dependencies.
// Including them would cause infinite re-renders since `process` is recreated on every state change.
// Instead, we list only the specific state fields that should trigger a re-run of the MFA flow.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
state.error,
state.scenario,
state.softPromptApproved,
state.validateCode,
state.registrationChallenge,
state.authorizationChallenge,
state.isRegistrationComplete,
state.isAuthorizationComplete,
state.isFlowComplete,
]);Current Implementation - Strengths
Before diving into the problems, it's worth noting what the current implementation does well:
1. Readable Flow Structure
The process() function provides a clear, top-to-bottom view of the entire MFA flow. Each step is protected by explicit conditions (if-statements), making it easy to understand:
- What the flow looks like end-to-end
- What conditions must be met before each step executes
- How the flow progresses from one step to the next
2. Why It Currently Works
The current implementation functions correctly because of two key invariants:
-
Dependency array contains only flow-controlled state: The
useEffectdependencies are narrowed down to state fields that only change at well-defined points in the flow - either byprocess()steps (e.g.,registrationChallenge,isRegistrationComplete) or by expected user interactions that signal flow continuation (e.g.,validateCode,softPromptApproved). These fields never change at unexpected times. -
State changes occur only at step completion: Every execution path through
process()dispatches state updates only at the very end of the step, after all async operations have completed. No state mutations occur mid-step while async operations are in flight.
- Adding state fields to the dependency array that can change at unexpected times (not at step boundaries or expected user input points)
- Dispatching state updates mid-step before async operations complete
This fragility is one of the main motivations for refactoring.
Problems
1. Violates React Rules (exhaustive-deps)
The useEffect explicitly disables the react-hooks/exhaustive-deps rule and omits process and state from dependencies.
2. Race Condition Risk
If the invariants above are violated (e.g., state changes at unexpected times), multiple process() invocations can run concurrently, causing two steps to execute at the same time.
Real example: An optimistic update when registering a key to the backend triggered process() immediately. This started the authorization challenge request, but the allowCredentials response didn't include the newly registered key because it hadn't been saved to the database yet from the previous request. The fix required reducing the useEffect dependencies to avoid reacting to the optimistic update.
Proposal
We need approach that:
- Follows React rules - no eslint-disable comments for hooks
- Prevents race conditions - proper handling of async operations
- Is easy to read - the flow should be obvious from the code structure
Potential Solutions to Explore
-
React composition - Use React composition patterns so each step can define its own flow and error handling. This approach assumes drastically reducing the global state stored in context. It also has potential to reduce the number of rerenders. Proposed by Rory Costello.
-
Explicit state machine - Define named states (
IDLE,AWAITING_VALIDATE_CODE,REGISTERING, etc.) with explicit transitions to eliminate race condition risk.
Acceptance Criteria
- No eslint-disable comments for
react-hooks/exhaustive-deps - Race conditions are properly handled
- The authentication flow logic is easy to follow
- All existing MFA scenarios continue to work correctly
- Unit tests cover the flow orchestration logic
Related Files
src/components/MultifactorAuthentication/Context/Main.tsx- Main context providersrc/components/MultifactorAuthentication/Context/State.tsx- State managementsrc/components/MultifactorAuthentication/Context/useNativeBiometrics.ts- Biometrics hooksrc/libs/actions/MultifactorAuthentication/- Backend API calls
Activity
samranahm commentedon Feb 2, 2026
Solution already mentioned in OP, I am available to take this one
[-]Refactor MultifactorAuthenticationContextProvider - fix React rules violations and race conditions risks[/-][+][ECUK In-App 3DS] [App] Refactor MultifactorAuthenticationContextProvider - fix React rules violations and race conditions risks[/+]rafecolton commentedon Feb 2, 2026
@samranahm thanks for the offer, but we will keep this with the project team
melvin-bot commentedon Mar 5, 2026
This issue has not been updated in over 15 days. @rafecolton, @chuckdries, @dariusz-biela, @roryabraham, @JakubKorytko, @DylanDylann eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
JakubKorytko commentedon Mar 6, 2026
It's not a near-term priority, but it's a cool add-on. Perhaps we should degrade it from a high priority in project rather than close it.
57 remaining items
jakubstec commentedon Aug 21, 2026
Hey, quick update:
dariusz-biela commentedon Aug 28, 2026
Jakub was out of office for 3 days this week. However, Chuck and I have completed the review for the current PR(software-mansion-labs#360). This is the project's largest slice since it covers authorization. I expect to merge it on Monday once Jakub makes his final updates.
Our next slice will focus on executing backend commands (such as "Approve transaction"), which will make the core flow fully functional. After that, only UX improvements, error handling, and logging will remain.
jakubstec commentedon Sep 8, 2026
Hey, currently working on a PR draft for a slice that handles all the scenarios. I expect to give it to Dariusz tomorrow for an initial review. As he said above, we'll have a core flow fully working!
jakubstec commentedon Sep 11, 2026
Hey, quick update - I've prepared a draft PR with the next slice software-mansion-labs#385, @dariusz-biela will review it at the beginning of the next week!
jakubstec commentedon Sep 14, 2026
Hey @rafecolton, just wanted to let you know that I’ll be OoO from Sep 18 to Oct 2.
We were discussing how to handle the development of the next slices while I’m away, and our preference would be to pause the work until I’m back, since onboarding someone new just for that period could take quite a bit of time and effort. Of course, let us know if you have any concerns with that approach.
Dariusz should finish his review tomorrow, and I’ll address all the comments as soon as possible so we can move forward with the current slice, which is the last one covering the core behavior.
After that, only UX improvements, error handling, and logging will remain. I’ll focus on those this week and share an update with you before I leave.
What do you think?
rafecolton commentedon Sep 14, 2026
I think that will be fine, thank you for letting me know. Enjoy your OOO!
melvin-bot commentedon Sep 21, 2026
This issue has not been updated in over 15 days. @rafecolton, @chuckdries, @dariusz-biela, @roryabraham, @JakubKorytko, @DylanDylann, @jakubkalinski0 eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
jakubstec commentedon Oct 5, 2026
Hey! I'm back from OoO and I've addressed all of Dariusz's comments. @chuckdries, the PR is ready for your review. software-mansion-labs#385
Also, sorry for not posting the update I promised before I left. I was busy finishing other work before OoO and didn't get to the remaining slices. I'll post an update on them soon.
jakubstec commentedon Oct 9, 2026
Weekly update!
jakubstec commentedon Oct 19, 2026
Weekly update: