fix(dpop): enable pool-server DPoP login; fix identity fetch for all DPoP flows - #3005
Conversation
…on failure Re-enables testECAJwtDPoP_ViaLoginPoolServer (was @ignore W-23864247, server bug now confirmed fixed). Fixes three test-assertion failures discovered while re-enabling and one SDK root cause: Test fixes: - L-marker: pool server routes through production infrastructure → expect L1 (FEATURE_LOGIN_SERVER_PRODUCTION) not L4 (FEATURE_LOGIN_SERVER_MY_DOMAIN) - A-marker: non-hybrid web server flow registers A1 (FEATURE_AUTH_TYPE_WEB_SERVER_NON_HYBRID) not A2; pass expectedAMarker explicitly instead of relying on the wrong default SDK fix — identity fetch retry (AuthenticationUtilities.kt): - Replace the bare fetchUserIdentity default with fetchUserIdentityWithRetry, which refreshes the token and retries once when the first identity call returns a null username. The pool server can return "Wrong_Org" on the initial call due to session-affinity routing; a token refresh yields a new access token and DPoP nonce that succeed at the identity endpoint. - This matches iOS SFIdentityCoordinator, which has always retried on 401/403 from the identity endpoint via SFSDKTokenRefreshCoordinator. Also: use raw idUrl (not idUrlWithInstance) in fetchUserIdentity so DPoP tokens are validated at the server that issued them, not the instance host substitution that previously caused Bad_OAuth_Token on pool-server logins.
For regular My Domain logins the DPoP nonce is keyed to the token-exchange host (e.g. authflowtesting.msdk.sdb38.salesforce.com). Using idUrlWithInstance (the original behavior) keeps the identity endpoint on the same host, so the nonce lookup hits and the DPoP proof is accepted. For pool-server logins idUrlWithInstance points to the production instance, which rejects pool-server-issued DPoP tokens with Bad_OAuth_Token. A new fallback in fetchUserIdentityWithRetry tries the raw idUrl (pool server host) before resorting to a full token refresh — matching the iOS SFIdentityCoordinator retry model. DPoPNonceCache gains a getLatest() fallback (mirrors iOS latest(forScope:)) used in callIdentityService when no per-host nonce is found, guarding edge cases where the identity endpoint host differs from the token-exchange host.
SDK 14.0 defaults useDPoP=true, so the OAuth URL built at app startup carries dpop_jkt. On the CA_OPAQUE all-defaults path loginAndValidate previously skipped Login Options entirely, so the stale URL was used and the server returned a DPoP-bound token even when the test intended Bearer. Add a needsDPoPUrlReset condition: when useDPoP=false AND knownAppConfig== CA_OPAQUE AND scopeSelection==EMPTY, open and dismiss Login Options so that loginDevMenuReload fires, LoginActivity.onResume regenerates the URL without dpop_jkt, and the login proceeds as Bearer as intended. Pre-existing issue on dev (introduced by 5d97417 alongside the useDPoP=true default). Caught while running LegacyLoginTests on the dpop-pool-server-test branch.
…ame getLatest to getAny Attempt 3 (token refresh + retry) is unreachable in practice: the access token is valid by construction immediately after login, so Attempt 1 or 2 always resolves the identity URL correctly. Keeping it was also unsafe under Refresh Token Rotation — the rotated refresh token from the refresh call was discarded, leaving the account with an already-consumed refresh token. Rename DPoPNonceCache.getLatest -> getAny to reflect that ConcurrentHashMap iteration order is undefined and the method returns an arbitrary cached nonce for the credential, not the most recently stored one. Strip iOS-parity references from inline comments; that context belongs in the PR description, not production code.
Investigated replacing the idUrlWithInstance-first / raw-idUrl-fallback order with a single upfront selection based on idUrl != idUrlWithInstance. The condition fires for both pool-server logins (where raw idUrl is correct) and classic-domain sandboxes where idUrl uses test.salesforce.com but the DPoP nonce is keyed to the My Domain / instance host — making idUrlWithInstance the right choice. A single-call selection would break those sandbox logins. Expanded the docstring to document this constraint explicitly.
…ction fail Two alternative single-call approaches were investigated and rejected: 1. idUrl != idUrlWithInstance upfront: fires for both pool-server logins (where raw idUrl is correct) AND classic-domain sandboxes (where idUrlWithInstance is correct). Breaks sandbox My Domain tests. 2. LoginServerManager.isPoolServer(loginServer): pool-server login portals do not serve identity endpoint requests, so routing there breaks pool-server identity fetches where idUrlWithInstance (the org My Domain in the same env) correctly accepts the token. Two-attempt strategy retained: try idUrlWithInstance first (succeeds for My Domain and same-env pool-server logins via getAny nonce fallback), fall back to raw idUrl only when the URLs differ (handles cross-env pool-server cases where the instance rejects the token).
Investigated matching iOS by always using raw idUrl (no idUrlWithInstance substitution). On iOS this works because pool-server login portals serve the identity endpoint. In the Android test environment (login.test1.pc-rnd.salesforce.com) the pool-server portal does not serve identity, so routing there fails while the org My Domain (idUrlWithInstance) accepts both the token and the nonce. Two-attempt strategy retained as the correct Android approach. iOS parity on the identity URL strategy is not achievable without changes to the test environment or confirmation that all production pool servers serve identity.
…planation Replaces the temporary raw-idUrl approach (which broke My Domain DPoP logins with Wrong_Org) with the correct two-attempt strategy, and updates the docstring to accurately describe the root cause based on observed server behavior. The identity endpoint URL selection works as follows: - Salesforce always returns the pool-server host in the `id` field of the token response, regardless of which server issued the token. - `idUrlWithInstance` (attempt 1) substitutes the issuing server's host; this is correct for My Domain logins (calling the pool server with a My Domain token returns Wrong_Org). - Raw `idUrl` (attempt 2, fallback when `idUrl != idUrlWithInstance`) is the correct URL for pool-server logins where `idUrlWithInstance` points to a production instance that returns Wrong_Org for pool-server-issued tokens.
| * so it is valid by construction. A refresh would also be unsafe under Refresh Token | ||
| * Rotation — consuming the fresh token and discarding the rotated replacement. | ||
| */ | ||
| private suspend fun fetchUserIdentityWithRetry( |
There was a problem hiding this comment.
@brandonpage — while investigating why iOS passes the My Domain DPoP test without idUrlWithInstance, I found that SFIdentityCoordinator.m handles identity 401/403 by triggering a full credential refresh and retrying. The reason that retry succeeds is subtle: the token refresh goes directly to My Domain (since credentials.domain = My Domain for a My Domain login), and the refresh token response sets credentials.identityUrl to the My Domain host — not the pool server host that was in the original login response. So the retried identity call lands on My Domain, which accepts its own token.
Android currently avoids that round trip with the two-attempt URL selection here. But the iOS approach is arguably more general: it does not need to predict the right URL upfront, it just corrects identityUrl as a side effect of the refresh. The downside is an extra network round trip on every pool-server DPoP login.
Worth aligning Android with iOS here, or is the two-attempt approach the right tradeoff?
There was a problem hiding this comment.
@wmathurin If the extra refresh is actually a more robust solution then I am not opposed to it. My concern is RTR. Since we haven't created the user yet we can refresh outside of our new synchronized code as long as the new TokenEndpointResponse is passed back and used instead of the one we got from code exchange.
There was a problem hiding this comment.
I don't think the way the refresh was done here was safe with RTR as you pointed out. Now I wonder if iOS has a latent RTR bug.
There was a problem hiding this comment.
I'm going to add a test through login pool server using an eca with dpop + rtr on Android and on iOS. And on iOS, it might surface an issue with the current approach. I'll create a story as well.
There was a problem hiding this comment.
Update: we dug deeper and resolved this cleanly.
The iOS credential-refresh approach works because the token refresh response sets credentials.identityUrl to the My Domain host (the id field in a refresh response points to My Domain, not the pool server). So the retry lands on My Domain with My Domain's own token — correct. But it's RTR-unsafe: burning the refresh token to fix a routing error on login is too expensive, and the rotated token would be discarded.
We also found that the root issue is a server bug: My Domain's /id/ endpoint returns Bad_OAuth_Token for pool-server-issued DPoP tokens instead of issuing a 401 use_dpop_nonce challenge the way data endpoints do. Bearer tokens are unaffected. Filed as W-23992239 against the Auth Protocols team.
On Android, we've since simplified to a direct isPoolServer check (latest commit 0a238ca1e): DPoP + pool-server login → idUrl; everything else → idUrlWithInstance. Single call, no retry, no speculative first attempt. Works because LoginServerManager.isPoolServer already covers all pool-server patterns including internal environments.
We're not aligning with the iOS credential-refresh pattern — the isPoolServer approach is cheaper, RTR-safe, and the server fix (W-23992239) should eventually make the whole workaround unnecessary on both platforms.
With the two-attempt URL strategy in fetchUserIdentityWithRetry, the successful identity call always lands on the host where a token-exchange nonce is already cached via the per-host get(). The getAny() fallback only fired on the failing attempt (wrong host), where it had no effect. Salesforce only issues DPoP-Nonce on token-endpoint responses, so the per-host cache is always populated from the correct token exchange before the identity call is made. getAny() is therefore redundant and removed.
…91713) Verifies that login via the pool server with DPoP + RTR works end-to-end and that the refresh token remains valid after login, confirming Android's two-attempt identity URL strategy never touches the refresh token.
…-23992239) Replace the two-attempt retry loop with a direct URL selection: - DPoP + pool-server login → use raw idUrl (pool server identity endpoint) - all other cases → use idUrlWithInstance (My Domain) My Domain rejects pool-server-issued DPoP tokens at the /id/ endpoint with Bad_OAuth_Token rather than issuing a nonce challenge (W-23992239 filed against Auth Protocols team). Bearer tokens are unaffected — My Domain accepts them regardless of which server issued them, so no DPoP guard is needed for Bearer. The two-attempt fallback loop was correct but opaque. The isPoolServer check makes the routing intent explicit and eliminates the speculative first attempt for pool-server DPoP logins. Validated: testECAJwtDPoP_ViaLoginPoolServer, testECAJwtDPoP_Hybrid, testECAJwtDPoP_ViaLoginPoolServer_Rtr all pass.
|
Observation: Android routing logic could potentially be simplified to match iOS
The current Android fix introduces an Since all environments have both iOS and Android clients, and iOS routes everything through the pool server without issues, it's worth asking: should we simplify Android to always use |
3 participants
Summary
Root cause (DPoP identity fetch)
Salesforce always returns the pool-server host (e.g. `login.test1.pc-rnd.salesforce.com`) in the `id` field of every token response, regardless of which server actually issued the token. `idUrlWithInstance` corrects this by replacing the `id` host with the issuing server's host.
For My Domain logins, the token was issued by My Domain. The correct identity URL is `idUrlWithInstance` (My Domain host).
For pool-server DPoP logins, `idUrlWithInstance` points to My Domain, which rejects the pool-server-issued DPoP token with `Bad_OAuth_Token` — it does not issue a `401 use_dpop_nonce` challenge the way data endpoints do. The correct URL is raw `idUrl` (pool-server host). Bearer tokens are unaffected: My Domain accepts pool-server-issued Bearer tokens at the identity endpoint regardless.
This server-side inconsistency is tracked in W-23992239 (Auth Protocols team).
Fix (DPoP identity fetch)
`fetchUserIdentityWithRetry` uses `LoginServerManager.isPoolServer` to select the URL upfront — a single call, no retry:
Why not the iOS approach? `SFIdentityCoordinator` on iOS handles identity 401/403 by triggering a full credential refresh and retrying. That works because the refresh response resets `credentials.identityUrl` to the My Domain host as a side effect. But it is RTR-unsafe — it consumes the refresh token unnecessarily to fix a routing error, and the rotated replacement is discarded. The `isPoolServer` selection avoids any token refresh entirely.
Why not a two-attempt retry? An earlier iteration tried `idUrlWithInstance` first and fell back to `idUrl`. The `isPoolServer` check makes the intent explicit, eliminates the speculative first attempt for pool-server DPoP logins, and is simpler to reason about.
Additional changes
Root cause (LegacyLoginTests Bearer baseline)
SDK 14.0 defaults `useDPoP=true`. The CA_OPAQUE all-defaults path skipped Login Options, so the OAuth URL built at app startup (with `dpop_jkt`) was used for Bearer tests. Opening and dismissing Login Options triggers `loginDevMenuReload`, causing `LoginActivity.onResume` to regenerate the URL with `useDPoP=false`.
Test results
All 14 AuthFlowTester UI test suites run on emulator (Pixel 9 Pro XL, API 36). All non-Beacon tests pass. Beacon tests (`BEACON_OPAQUE` / `BEACON_JWT`) fail pre-existing across all suites; the same failures are seen on iOS, pointing to an environment or server-side issue with the Beacon app configuration in the test sandbox rather than a code regression.
In `MultiUserLoginTests`, 2 non-Beacon tests (`testFirstDynamic_SecondStatic_DifferentApps`, `testMultiUser_revokeOtherUserRefreshToken`) showed instrumentation crashes when run as part of the full suite after preceding Beacon failures. Both pass when run in isolation.
Test plan