-
Notifications
You must be signed in to change notification settings - Fork 46
Native auth: update auth method blocked error handling, Fixes AB#3395876 #2804
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
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 refactors the error detection mechanism for blocked authentication methods and verification contacts in Native Auth responses. The change migrates from an error-code-based approach (errorCodes checking for 550024) to a suberror-based approach (subError checking for "provider_blocked_by_rep"), aligning with updated API contract specifications.
Key changes:
- Added
subErrorfield toJITChallengeApiResponseto support provider blocking detection - Updated error detection logic from
error.isInvalidRequest() && errorCodes?.first().isBlockedChallengeTarget()toerror.isAccessDenied() && subError.isProviderBlocked() - Replaced the removed
isBlockedChallengeTarget()function with newisProviderBlocked()andisAccessDenied()utility functions
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ApiErrorResponseUtil.kt | Removed isBlockedChallengeTarget() function and added isAccessDenied() and isProviderBlocked() utility functions for new error detection |
| SignInChallengeApiResponse.kt | Updated imports and error detection logic to use isAccessDenied() and isProviderBlocked() instead of the removed isBlockedChallengeTarget() |
| JITChallengeApiResponse.kt | Added subError field, updated imports, and changed error detection logic for blocked verification contacts |
| NativeAuthResponseHandler.kt | Added subError = null to empty response construction for JITChallengeApiResponse |
| NativeAuthResponseHandlerTest.kt | Updated all JITChallengeApiResponse test constructors to include subError = null parameter |
Comments suppressed due to low confidence (1)
common4j/src/main/com/microsoft/identity/common/java/nativeauth/providers/responses/signin/SignInChallengeApiResponse.kt:87
- Missing test coverage for the new blocked auth method scenario. The PR introduces a new error detection path (
error.isAccessDenied() && subError.isProviderBlocked()) that triggersSignInChallengeApiResult.BlockedAuthMethod, but there are no tests validating this new behavior.
Recommendation: Add a test case in NativeAuthResponseHandlerTest.kt (similar to existing SignInChallengeApiResponse tests) that verifies:
- When
erroris "access_denied" - And
subErroris "provider_blocked_by_rep" - The result is correctly mapped to
SignInChallengeApiResult.BlockedAuthMethod
This ensures the new provider blocking logic works as intended for sign-in challenges.
error.isAccessDenied() && subError.isProviderBlocked() -> {
SignInChallengeApiResult.BlockedAuthMethod(
error = error.orEmpty(),
errorDescription = errorDescription.orEmpty(),
errorCodes = errorCodes.orEmpty(),
correlationId = correlationId
)
Update auth method blocked error handling. Use suberror instead of error codes
AB#3395876