W-24224593: Select request authentication by path

[wmathurin]

Summary

• Let SalesforceSDKManager synchronously choose ui_sid Bearer authentication by request path when a DPoP access token is present.
• Default the policy to select no paths; apps opt specific paths into ui_sid Bearer, and all other requests keep DPoP.
• Apply the policy consistently across REST, push, and DPoP nonce handling.
• Add coverage for policy, fallback, and authentication behavior and update DPoP documentation.

Test plan

• Focused SalesforceSDK unit tests: 109 passed.
• Additional authentication tests: 33 passed.
• assembleDebug: passed.

[JohnsonEricAtSalesforce]

Traced the header-selection path end to end (initial build, nonce retry, refresh replay) and couldn't find a case where a stale scheme choice survives a replay: the decision is recomputed fresh on every call, and the new synchronized on buildAuthenticatedRequest closes a race against the already-synchronized credential writers. Compatibility holds throughout via additive overloads and a new default interface method.

Compared against the merged iOS twin (PR 4175): the policy shape and nonce-retry guard match across platforms. Each platform's tracking item describes the replay coverage slightly differently ("refresh and replay" vs. "retries"), and that turns out to reflect a real architecture difference rather than a coverage gap — Android's per-user interceptor cache needs the new adoptCredentialMetadataIfSameAuthToken guard to avoid a stale snapshot rolling back a newer credential's metadata; iOS has no equivalent cache to reconcile.

One item below is worth addressing before merge; two more are small.

Unguarded policy callback risks a crash and lock contention

libs/SalesforceSDK/src/com/salesforce/androidsdk/auth/dpop/DPoPRequestDecorator.kt:86 invokes the app-supplied shouldUseUiSidBearerForPath policy with no try/catch. That call now happens inside RestClient.buildAuthenticatedRequest, which this PR changes to synchronized on the per-user cached interceptor's monitor. Two consequences follow from that combination: a policy that throws propagates uncaught out of buildAuthenticatedRequest — inconsistent with attachProof a few lines below in the same file, which already wraps its body in try/catch and logs failures; and a policy that blocks (touches disk, waits on another lock, etc.) stalls every other thread's request against that same cached interceptor while holding the lock, including the nonce-retry and refresh-replay paths that also need it. Since the policy is app-supplied and now runs under a lock shared across requests for a user, suggest wrapping the invocation in try/catch with a fallback to DPoP on failure (matching the existing "never silently drop auth" handling for a missing uiSid), and documenting in KDoc that the callback must be fast and non-blocking.

Silent no-op on credential mismatch (small)

libs/SalesforceSDK/src/com/salesforce/androidsdk/rest/RestClient.java:246, adoptCredentialMetadataIfSameAuthToken — silently no-ops (dropping a fresh, possibly-correct uiSid) whenever the candidate authToken doesn't exactly match the cached one, with no logging. This is the intended staleness guard, not a bug, but if it ever fires unexpectedly there's currently no way to tell from the logs. Suggest a debug-level log line on the mismatch branch.

Stale comment reference to a deleted method (small)

libs/test/SalesforceSDKTest/src/com/salesforce/androidsdk/rest/RestClientDPoPGateTests.kt, around the comment on test_givenNoTokenTypeButExistingKeyPair_whenIntercept_thenAttachesProof:

// Belt-and-suspenders: no tokenType, but a key pair exists for the credential
// → interceptor still attaches a DPoP proof (Authorization stays Bearer
// because setAuthHeader keys off exact tokenType match — this asymmetry
// is intentional and documented).

setAuthHeader was the private method this PR removes from RestClient.OAuthRefreshInterceptor (its behavior now lives in DPoPRequestDecorator.applyAuthHeaders). The comment predates this PR, but this PR's own deletion is what makes it definitively stale — the named method no longer exists anywhere in the codebase. The underlying claim (exact-tokenType-match asymmetry) still holds; only the method name needs updating.

---

Verdict: Request changes. The unguarded policy callback is the one item blocking approval — everything else here is optional.

This review was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.

[wmathurin]

Addressed the review in f4db199 (after merging current upstream/dev in e673e50): the app-supplied path policy is now exception-contained with a fail-safe DPoP fallback, its KDoc and auth documentation require fast/non-blocking behavior, credential-generation mismatch emits a generic debug log without credential values, and the stale test comment now names DPoPRequestDecorator.applyAuthHeaders. Added a regression test proving a throwing policy still produces DPoP authorization and proof headers. Validation: full :libs:SalesforceSDK:build plus assembleAndroidTest passed, and 100 focused emulator tests passed with 0 failures.

[JohnsonEricAtSalesforce]

Verified the fix commit (f4db19948) against the three items from the prior review round:

• Unguarded policy callback — DPoPRequestDecorator.kt now calls shouldUseUiSidBearerForPath through a private wrapper that catches Exception, logs a warning, and falls back to false (normal DPoP/Bearer path). KDoc on SalesforceSDKManager.shouldUseUiSidBearerForPath and docs/auth/token-lifecycle.md now state the callback must be fast and non-blocking, and document the fail-safe. A new test (applyAuthHeaders_dpopWithUiSidWhenPolicyThrows_fallsBackToDpopHeaders) confirms a throwing policy still produces DPoP authorization and proof headers.
• Silent no-op on credential mismatch — RestClient.java's adoptCredentialMetadataIfSameAuthToken now logs at debug level on the mismatch branch, with no credential values in the message.
• Stale comment — the test comment in RestClientDPoPGateTests.kt now correctly names DPoPRequestDecorator.applyAuthHeaders instead of the removed setAuthHeader.

All three read as complete and correct; no new issues introduced by the fix itself.

---

Verdict: Approve.

This review was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.