-
Notifications
You must be signed in to change notification settings - Fork 364
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
[SDK-2750] Expose mfa_token from the mfa_required error when getting new tokens #789
Conversation
@@ -133,16 +133,20 @@ export async function getJSON<T>( | |||
} | |||
|
|||
const { | |||
json: { error, error_description, ...success }, | |||
json: { error, error_description, ...data }, |
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.
Renamed this to data, because it contains the mfa_token and it was a bit weird to use success
in case of an error
.
@@ -30,7 +30,7 @@ export class AuthenticationError extends GenericError { | |||
public state: string, | |||
public appState: any = null | |||
) { | |||
super(error, error_description); | |||
super(error, error_description) /* istanbul ignore next */; |
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.
Added a few istanbul ignores, see gotwarlost/istanbul#690
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.
LGTM
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.
LG, just left a couple things around exports
When mfa is enabled, the
/token
endpoint can return amfa_required
error. In the case that it does, it also included amfa_token
property, which we are currently ignoring.This PR introduces a
MfaRequiredError
class that is used to unwrap the above information in case of amfa_required
error, exposing themfa_token
to the users.