[minor][engg]: Add error code handling for 50142 error returned by ESTS#1692
[minor][engg]: Add error code handling for 50142 error returned by ESTS#1692fidelianawar wants to merge 12 commits intodevfrom
Conversation
…ps://github.com/AzureAD/microsoft-authentication-library-common-for-objc into maagubuzo/ufic/introduce_ufic_into_api_contract
ero "exit merge editor" | git merge --continue 2>/dev/null || git log --oneline -3 t branch e branch 'dev' into maagubuzo/ufic/introduce_ufic_into_api_contract
…tion-library-common-for-objc into fidelianawar/sts_error_mapping
![azure-pipelines[bot]](https://avatars.githubusercontent.com/in/9426?s=60&v=4){kind=small}
| @@ -367,6 +367,9 @@ typedef NS_ENUM(NSInteger, MSIDErrorCode) | |||
|
|
|||
There was a problem hiding this comment.
This pull request does not update changelog.txt.
Please consider if this change would be noticeable to a partner or user and either update changelog.txt or resolve this conversation.
There was a problem hiding this comment.
Pull request overview
Adds handling for ESTS STS error code 50142 (SecureChangePasswordDueToConditionalAccess) by mapping an invalid_request OAuth error with that STS code to a dedicated MSIDErrorCode, and wiring token response parsing to use STS error codes when deriving oauthErrorCode.
Changes:
- Introduces
MSIDErrorCodeForOAuthErrorWithSTSErrorCodes(...)and uses it inMSIDTokenResponse.oauthErrorCode. - Adds a new
MSIDErrorCode(MSIDErrorServerInvalidRequestResetPasswordRequired) and includes it in domain/code mappings + string conversion. - Adds unit tests covering the new STS error-code mapping behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| IdentityCore/tests/MSIDErrorTests.m | Adds tests validating the STS error code → MSIDErrorCode mapping behavior. |
| IdentityCore/src/oauth2/MSIDTokenResponse.m | Switches oauthErrorCode derivation to consider stsErrorCodes. |
| IdentityCore/src/oauth2/MSIDOauth2Factory.m | Adds MSIDError.h import (enables direct access to MSID error helpers/constants). |
| IdentityCore/src/MSIDError.m | Implements STS error-code-aware OAuth error mapping and registers the new error code. |
| IdentityCore/src/MSIDError.h | Declares the new error code + helper function for STS error-code mapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Error thrown when oauth error = MSIDServerInvalidRequest and error_code = 50142 (SecureChangePasswordDueToConditionalAccess) | ||
| MSIDErrorServerInvalidRequestResetPasswordRequired = -50142, |
There was a problem hiding this comment.
MSIDErrorCode values are documented as grouped by domain/range (e.g., Server errors are 514xx in MSIDOAuthErrorDomain). Adding MSIDErrorServerInvalidRequestResetPasswordRequired = -50142 breaks that established numbering scheme and places an OAuth/server error code outside the 514xx range, which can confuse diagnostics and any code that relies on these ranges. Consider assigning a -514xx value (e.g., next available after -51417) and locating this enum entry in the existing “Server errors (514xx)” section alongside the other OAuth server errors.
| // Error thrown when oauth error = MSIDServerInvalidRequest and error_code = 50142 (SecureChangePasswordDueToConditionalAccess) | ||
| MSIDErrorServerInvalidRequestResetPasswordRequired = -50142, |
There was a problem hiding this comment.
The enum comment says “oauth error = MSIDServerInvalidRequest”, but the OAuth error value being checked is the string "invalid_request" (which maps to MSIDErrorServerInvalidRequest). Updating this comment would avoid confusion about whether this refers to an OAuth error string vs an MSIDErrorCode value.
| if (oauthError && [oauthError caseInsensitiveCompare:@"invalid_request"] == NSOrderedSame | ||
| && [stsErrorCodes containsObject:@50142]) | ||
| { |
There was a problem hiding this comment.
@50142 is a hard-coded STS error code. To make this mapping easier to audit and extend (and to keep the meaning discoverable), consider introducing a named constant (e.g., MSIDSTSErrorCodeSecureChangePasswordDueToConditionalAccess) and using that in the containsObject: check (and in tests).
2 participants
PR Title Format
Required Format:
[Keyword1] [Keyword2]: Descriptionmajor,minor, orpatch(case-insensitive)feature,bugfix,engg, ortests(case-insensitive)Examples:
[MAJOR] [Feature]: new API[minor] [bugfix]: fix crash[PATCH][tests]:add coverageProposed changes
Describe what this PR is trying to do.
Type of change
Risk
Additional information