-
Notifications
You must be signed in to change notification settings - Fork 450
feat: Base MFA support #2480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Base MFA support #2480
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2480 +/- ##
==========================================
- Coverage 91.20% 90.97% -0.24%
==========================================
Files 39 40 +1
Lines 4698 4909 +211
Branches 981 1018 +37
==========================================
+ Hits 4285 4466 +181
- Misses 407 437 +30
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dd53018 to
248eca7
Compare
| // Handle MFA required error - return 403 with MFA context | ||
| // Note: session.mfa was already mutated by getTokenSet() before the error was thrown. | ||
| // JavaScript is single-threaded, so no race condition exists. | ||
| if (e instanceof MfaRequiredError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently in which all different flows/scenarios are we throwing MfaRequiredError, we should also update the PR description with this information ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scenarios:
- Token refresh with MFA token (normal flow)
- Token refresh without MFA token (edge case). If this happens, it will be handled and MfaRequiredError will be thrown with empty mfa token
mfa_required error)
mfa_required error)
This PR adds support for handling
mfa_requirederror natively and getting access tomfa_tokenandmfa_requirementswhen MFA step-up authentication is required.These parameters can be used to call MFA API methods for challenge and verify operations, which will be added in a later PR.
Changes
MfaRequiredErrorand other MFA related errors.Usage
When accessing a protected resource, catch the
MfaRequiredError. It automatically packages the encryptedmfa_tokenyou need.Testing
Flow tests:
Unit tests for util methods.