-
Notifications
You must be signed in to change notification settings - Fork 138
Native auth: Update Email OTP MFA to Match EC Implementation, Fixes AB#3351233 #2379
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
Conversation
…ed. No getAuthMethod methods
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.
Pull Request Overview
This PR updates the Native Auth MFA (Multi-Factor Authentication) implementation to align with the latest EC (External Credential) flow. The key changes remove the intermediate authentication method selection step and require developers to always provide an authentication method when requesting challenges.
- Removes the
getAuthMethods()API and related selection logic from MFA flows - Updates
requestChallenge()methods to require anAuthMethodparameter instead of making it optional - Modifies
SignInResult.MFARequiredto include authentication methods directly from the initial MFA response
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| NativeAuthPublicClientApplicationKotlinTest.kt | Updated test cases to use auth methods from MFA result and removed obsolete selection flow tests |
| NativeAuthPublicClientApplicationJavaTest.java | Simplified MFA test scenarios and removed callback classes for deprecated getAuthMethods |
| SignInMFATest.kt | Updated E2E tests to pass auth method parameter when requesting challenges |
| SignInStates.kt | Added authMethods parameter to SignInResult.MFARequired constructor |
| MFAStates.kt | Removed getAuthMethods() functionality and made authMethod parameter required in requestChallenge() |
| SignInResult.kt | Added authMethods property to MFARequired result class |
| MFAResult.kt | Removed SelectionRequired result type and related interfaces |
| MFAErrors.kt | Removed MFAGetAuthMethodsError class |
| NativeAuthPublicClientApplication.kt | Updated to pass auth methods when creating MFA required states |
| CommandParametersAdapter.java | Consolidated MFA challenge command parameters and removed separate methods for default/selected challenges |
Comments suppressed due to low confidence (5)
msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationKotlinTest.kt:1
- The import for
MFAGetAuthMethodsErroron line 53 is removed butMFARequestChallengeErroron line 54 is also removed. However, the code still usesMFARequestChallengeErrorin several places but the import is missing, which will cause compilation errors.
// Copyright (c) Microsoft Corporation.
msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java:1
- The import for
MFAGetAuthMethodsResultis removed but this class is still referenced in theGetAuthMethodsTestCallbackclass definition on line 3319, which will cause a compilation error.
// Copyright (c) Microsoft Corporation.
msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java:3344
- The test callback classes starting from
ResetPasswordStartTestCallbackonwards appear to have incomplete class definitions with only method signatures and no complete implementations, which will cause compilation errors.
}
msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/MFAStates.kt:1
- The import for
GetAuthMethodsCommandResultis removed but may still be needed if this class is referenced elsewhere in the file or in related functionality.
// Copyright (c) Microsoft Corporation.
msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java:1
- Three imports are removed (lines 64-66) but the new import
MFAChallengeAuthMethodCommandParameterson line 67 uses a different class name that may not exist, potentially causing compilation errors.
// Copyright (c) Microsoft Corporation.
| * @throws ClientException | ||
| */ | ||
| public static MFASelectedDefaultChallengeCommandParameters createMFASelectedChallengeCommandParameters( | ||
| public static MFAChallengeAuthMethodCommandParameters createMFASelectedChallengeCommandParameters( |
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.
should this method name be changed as well?
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.
Yes, let me change it
...c/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java
Show resolved
Hide resolved
# Conflicts: # common
…B#3351233 (#2760) This PR updates the SDK to match the latest flow from EC. In this new flow, the developer must always supply an auth Method to the /oauth2/v2.0/challenge endpoint which means once the .mfaRequired error is received from token endpoint, the /oauth2/v2.0/introspect endpoint needs to be called to retrieve the methods which are automatically returned to the external developer. Furthermore, whenever calling the /token endpoint is called with an MFA Email OTP code, the grant type should be mfa_oob Fixes [AB#3351233](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/3351233) MSAL PR: AzureAD/microsoft-authentication-library-for-android#2379 --------- Co-authored-by: Mustafa Mizrak <[email protected]>
This PR updates the SDK to match the latest flow from EC.
In this new flow, the developer must always supply an auth Method to the /oauth2/v2.0/challenge endpoint which means once the .mfaRequired error is received from token endpoint, the /oauth2/v2.0/introspect endpoint needs to be called to retrieve the methods which are automatically returned to the external developer.
Furthermore, whenever calling the /token endpoint is called with an MFA Email OTP code, the grant type should be mfa_oob
Fixes AB#3351233
MSAL Common PR: AzureAD/microsoft-authentication-library-common-for-android#2760