[Android] fetchIsSalesforceIntegrationUser omits DPoP proof on /userinfo, causing Bad_OAuth_Token login failure with DPoP-bound tokens - #3044
Open
Conversation
…itted DPoP proof on /userinfo causing Bad_OAuth_Token login failure)
Generated by 🚫 Danger |
… prefer-static-imports convention for new test code)
…new test (Follow prefer-static-imports convention, missed on first pass)
…ch the direct-import convention used at 7 of 9 repo call sites)
…field (Complete prefer-static-imports pass, drop dead cursor field)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fetchIsSalesforceIntegrationUser()called/services/oauth2/userinfousing the 2-argOAuth2.addAuthorizationHeader()overload, which always attaches a plainBearerheader andnever a DPoP proof. For orgs using DPoP-bound tokens, this made the integration-user check fail
with
Bad_OAuth_Token, breaking login for any app withshouldBlockSalesforceIntegrationUser = true.This brings the check to parity with the DPoP-aware sibling,
OAuth2.callIdentityService():DPoPRequestDecorator.attachProofwhen the token is DPoP-bound(shared with the initial request and the redirect-reattachment path via a new
attachAuthHeadershelper).callIdentityService).reattachAuthOnRedirectfunction.IOException,same as before this change) — but that throw was only being caught at 2 of 4 call sites. The
other two (
LoginActivity's User-Agent-flow branch andNativeLoginManager.suspendFinishAuthFlow)had no exception handling, so a transient network/parse error during this check would have
crashed the login coroutine instead of surfacing a graceful error. Added matching
try/catchguards at both sites (
LoginActivity.kt,NativeLoginManager.kt), consistent with the existingguarded call sites (
LoginViewModel.doCodeExchange,SPAuthCodeHelper.loginWithAuthCode).iOS
iOS already goes through
SFSDKDPoPRequestDecorator applyAuthHeaders:for the equivalent checkin
SFUserAccountManager.m/SFIdentityCoordinator.m— no iOS gap, no companion PR needed.Test plan
AuthenticationUtilitiesIntegrationUserTest(new, 7 tests) — DPoP-bound attaches proof,Bearer never attaches proof, nonce-challenge retries once, non-JSON error body →
IOExceptionnotJSONException, malformed 200 body →IOException, redirect to aSalesforce host reattaches Authorization+DPoP, no redirect attaches nothing.
NativeLoginManagerTest#nativeLoginManager_login_convertsOnAuthFlowCompleteExceptionToUnknownError(new) — confirms the new catch guard converts a thrown exception to
NativeLoginResult.UnknownError.LoginActivityTest#authWebViewClient_shouldOverrideUrlLoading_convertsOnAuthFlowCompleteExceptionToOnAuthFlowError(new) — confirms the new catch guard routes to
onAuthFlowErrorinstead of crashing.AuthenticationUtilitiesTest42/42,OAuth2DPoPTest10/10,NativeLoginManagerTest24/24,LoginActivityTest56/56,LoginActivityScenarioTest14/14.test_credentials.json/ui_test_config.jsonavailable in this environment). Relying on theunit-test coverage above; deferring to CI/reviewer for E2E confirmation.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.