@W-24465686@: Download profile photo with OkHttp and a DPoP proof by wmathurin · Pull Request #3059 · forcedotcom/SalesforceMobileSDK-Android

wmathurin · 2026-10-09T04:30:20Z

Summary

Work item: W-24465686. Root cause: W-24433518.

UserAccount.downloadProfilePhoto() fetched the photo through the system DownloadManager with a hardcoded Authorization: Bearer <token> header and no DPoP proof. On the 266 server a proof-less request to the file domain with a DPoP-bound token gets the token's jti revoked (MISMATCH_JKT), so later REST calls fail with 401 INVALID_AUTH_HEADER.

Changes (all users, DPoP and Bearer):

Download with OkHttp on a background executor via HttpAccess.DEFAULT.okHttpClient, reusing the helpers behind fetchIsSalesforceIntegrationUser in AuthenticationUtilities.kt: attachAuthHeaders (scheme by token type plus proof with htm/htu/ath), reattachAuthOnRedirect (proof re-signed per Salesforce redirect hop), harvestNonce/isNonceChallenge (one retry). Token, token type and credentials identifier are captured when the download is requested.
Full-size photoUrl (/F) is kept. The body is streamed to a temp file next to the cached photo and renamed on a 200; a non-200 or failed download leaves the cache untouched. Failures are logged, non-fatal, and never trigger a token refresh.
Login (addAccount) still always downloads. The refresh paths (ClientManager, AuthenticatorService) now call the new downloadProfilePhotoIfMissing(), which returns early when a photo is cached.
Removed the DownloadManager enabled check/OEM workaround and the hardcoded Bearer constant.
Docs: docs/auth/token-lifecycle.md (new "Profile photo download" section), docs/auth/README.md.
AuthFlowTester: AuthFlowTest now wraps HttpAccess.DEFAULT for every test and fails in @After if any request carried Authorization: DPoP without a DPoP header.

Note on the diff

#3055 is merged and this branch is rebased on the latest dev, so the diff is the single W-24465686 commit.

Tests

./gradlew :libs:SalesforceSDK:connectedAndroidTest filtered to UserAccountDownloadProfilePhotoTest (rewritten; the old DownloadManager tests are gone) and AuthenticationUtilitiesIntegrationUserTest: 17/17 pass on the Pixel_9_Pro_XL emulator. New coverage: DPoP proof htm/htu/ath with Authorization: DPoP; Bearer with no DPoP header; redirect re-signs the proof for the new URL (via reattachAuthOnRedirect; a real redirect needs a network interceptor, which the canned-response HttpAccess used here cannot exercise); nonce challenge retries once and not more; failed download keeps the cached photo; refresh with a cached photo sends no request; refresh without one sends one; login download replaces a cached photo.
AuthFlowTester DPoP UI tests: results to follow in a comment.
DPoPLoginTests re-run with this change on the emulator: testMigrate_ECAJwtDPoP_To_ECAJwtDPoPRtr now passes. It failed before because the identity fetch at the end of the migration hit the intermittent 403 Wrong_Org (W-24433488), refreshed, and rotated the refresh token, which the test did not expect. That test fix (771a90fb6) shipped with @W-23075124@: Add community login UI tests #3055.

github-actions · 2026-10-09T04:30:53Z

	1 Warning
⚠️	Big PR, try to keep changes smaller if you can.

Generated by 🚫 Danger

github-actions · 2026-10-09T04:34:55Z

	1 Warning
⚠️	libs/SalesforceSDK/src/com/salesforce/androidsdk/auth/AuthenticatorService.java#L56 - Do not place Android context classes in static fields (static reference to `Authenticator` which has field `context` pointing to `Context`); this is a memory leak

Generated by 🚫 Danger

wmathurin · 2026-10-09T04:57:32Z

AuthFlowTester androidTest UI runs on the Pixel_9_Pro_XL emulator against this branch (each test run individually; the new DPoP-without-proof audit in AuthFlowTest was active for all of them):

Test	Result
DPoPLoginTests#testECAJwtDPoP_NoHybrid (3 runs)	pass, pass, pass
DPoPLoginTests#testECAJwtDPoPRtr_NoHybrid	pass
DPoPLoginTests#testMigrate_ECAJwtDPoP_AddMoreScopes	pass
DPoPLoginTests#testECAJwtDPoP_WithRestart	pass
DPoPLoginTests#testECAJwtDPoP_ViaLoginPoolServer	pass
CommunityLoginTests#testCommunityDPoP_NoHybrid	pass

brandonpage · 2026-10-10T00:53:37Z

+	private static final ExecutorService PHOTO_EXECUTOR = Executors.newSingleThreadExecutor(runnable -> {
+		final Thread thread = new Thread(runnable, "UserAccount-profile-photo");
+		thread.setDaemon(true);
+		return thread;
+	});


Consider implementing the download function as a Kotlin extension of UserAccount so you can use a Coroutine instead of Thread/Executor.

Done. The download is now a Kotlin extension, UserAccount.launchProfilePhotoDownload(onlyIfMissing) in UserAccountExtension.kt, launched on CoroutineScope(IO). A Mutex keeps downloads serialized (what the single-thread executor did). The Java UserAccount no longer has an executor.

brandonpage · 2026-10-10T00:56:04Z

+
+	@VisibleForTesting
+	@Nullable
+	Future<?> downloadProfilePhotoAsync(boolean onlyIfMissing) {


Same suggestion as above, in Kotlin we have access to non-blocking suspension functions like Job.join().

Done — it returns a Job now and the tests join() it. downloadProfilePhoto() / downloadProfilePhotoIfMissing() stay on UserAccount as thin void delegates so the public API and the Java callers (ClientManager, AuthenticatorService) are unchanged.

The photo was fetched through DownloadManager with a hardcoded Bearer header and no DPoP proof, which makes the server revoke a DPoP-bound token (W-24433518). Use OkHttp on a background thread with the same auth helpers as fetchIsSalesforceIntegrationUser (scheme by token type, proof re-signed per Salesforce redirect hop, one nonce retry). Stream to a temp file and rename on success so a failed download keeps the cache. Refresh paths skip the download when a photo is already cached. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

…o download; drop docs section

github-actions · 2026-10-10T05:49:15Z

+ * @return The download job, or null if there is nothing to download
+ */
+internal fun UserAccount.launchProfilePhotoDownload(onlyIfMissing: Boolean): Job? {
+    val file = profilePhotoFile


⚠️ This method should only be accessed from tests or within private scope

github-actions · 2026-10-10T05:49:16Z

+    val credentialsId = credentialsIdentifier
+    return CoroutineScope(IO).launch {
+        profilePhotoMutex.withLock {
+            downloadProfilePhotoToFile(url, file, token, type, credentialsId)


⚠️ This method should only be accessed from tests or within private scope

…(review follow-ups from forcedotcom#3055)

wmathurin · 2026-10-12T01:59:18Z

Test status update on c0805d5a5: all the UI tests pass locally.

The failures in CI run 38059709531 (ui-tests-pr) are:

AdvancedAuthBeaconLoginTests#testBeaconJwt_DefaultScopes: the Custom Tab login page did not advance from the username step to the password step. This is the page interaction in ChromeCustomTabPageObject, which this PR does not touch.
CommunityLoginTests#testCommunityUpgradeToDPoP_InPlace and #testCommunityDPoP_NoHybrid: both report a bare FAILED with no message or stack trace, 13s and 26s in. The shard's retry-merge step also printed could not validate complete results, so the Firebase result for this shard may be incomplete.

These look like CI flakiness rather than regressions from this change. I'll re-run the failed jobs.

JohnsonEricAtSalesforce

@brandonpage, could you weigh in on these proposed fixes before merge?

Requested Changes

AuthenticationUtilities.kt:474–487: If proof signing fails, attachProof catches the exception after Authorization: DPoP is set, and this method still sends the proof-less request. Please fail the photo download before newCall when a DPoP request lacks a proof, and test that failure path.
AuthenticationUtilities.kt:500–508: isSuccessful accepts 204, which can replace a cached photo with an empty file that later refreshes treat as valid. Please require HTTP 200 and test cache preservation on 204.

Open Questions

UserAccountDownloadProfilePhotoTest.kt:174–194: The redirect test mocks the interceptor chain, while the UI audit sees only the initial OkHttp request. Would a real redirect test checking the second request's proof be useful before merge?

The focused photo tests passed locally (10/10). CI's DPoP login tests passed; the red UI job has a Custom Tab failure and an incomplete community test result.

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

JohnsonEricAtSalesforce · 2026-10-14T23:56:24Z

Following up on my review, @brandonpage: if photo proof signing fails, the current code can still send Authorization: DPoP without a proof. On servers with the affected mismatch behavior, that can revoke the token's jti and make later REST calls fail with 401. A 204 photo response can replace a valid cached photo with an empty file; later refreshes then skip the download because the file exists. The proposed guards prevent these outcomes, without adding an immediate retry.

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

brandonpage · 2026-10-15T00:31:57Z

@JohnsonEricAtSalesforce

AuthenticationUtilities.kt:474–487: If proof signing fails, attachProof catches the exception after Authorization: DPoP is set, and this method still sends the proof-less request. Please fail the photo download before newCall when a DPoP request lacks a proof, and test that failure path.

Does this actually make much difference? If DPoP signing fails the login will fail. Defensively, yeah it is always good to avoid an operation like this when we know it will not succeed but I don't see it as being a huge blocker.

AuthenticationUtilities.kt:500–508: isSuccessful accepts 204, which can replace a cached photo with an empty file that later refreshes treat as valid. Please require HTTP 200 and test cache preservation on 204.

Good catch, this should be added.

UserAccountDownloadProfilePhotoTest.kt:174–194: The redirect test mocks the interceptor chain, while the UI audit sees only the initial OkHttp request. Would a real redirect test checking the second request's proof be useful before merge?

I think it is worth adding.

JohnsonEricAtSalesforce · 2026-10-15T19:34:17Z

@brandonpage, I agree with your severity point: a persistent signing failure during login would likely stop login before the photo request. I also need to narrow my earlier revocation claim. The GUS reproduction used Bearer with a DPoP-bound token; this path uses Authorization: DPoP without a proof. We have not established that it causes revocation, and the related Auth work item is marked Fixed (deployment unverified).

The photo-specific issue remains: attachProof swallows a later signing failure, so an initial request or nonce retry can go out without a proof, or a redirect can carry the previous URL's proof. Under the fixed server behavior, those requests would be rejected, leaving the photo absent or stale and adding avoidable failed calls.

I pushed three commits to this PR: the 204 cache guard, a real redirect test, and a photo-specific signing guard. The signing guard stops the initial request or nonce retry if proof creation fails and clears the old proof before signing a redirect. The focused photo tests pass 15/15, including the new failure cases; the affected integration-user tests pass 7/7. This prevents those malformed requests and avoidable server rejections, though the photo can still be absent or stale until a later successful download. Does this resolve your concern, or do you see a reason to handle the failure differently?

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

wmathurin mentioned this pull request Oct 9, 2026

@W-23075124@: Add community login UI tests #3055

Merged

brandonpage self-requested a review October 10, 2026 00:38

brandonpage reviewed Oct 10, 2026

View reviewed changes

wmathurin force-pushed the android-profile-photo-dpop-proof branch from 62ef8b8 to 196f814 Compare October 10, 2026 05:38

wmathurin marked this pull request as ready for review October 10, 2026 05:38

@W-24465686@: Use a coroutine (Kotlin extension) for the profile phot…

dd2c4de

…o download; drop docs section

github-actions Bot reviewed Oct 10, 2026

View reviewed changes

wmathurin added 2 commits October 9, 2026 22:57

@W-24465686@: Scroll to Login Options toggles before tapping them

e6b6199

@W-24465686@: Rename identity summary helper and fix refresh comment …

c0805d5

…(review follow-ups from forcedotcom#3055)

brandonpage approved these changes Oct 14, 2026

View reviewed changes

JohnsonEricAtSalesforce reviewed Oct 14, 2026

View reviewed changes

JohnsonEricAtSalesforce added 3 commits October 15, 2026 10:43

Preserve cached profile photo on non-200 responses

072e858

Test DPoP proof after a real photo redirect

6dabed0

Stop photo requests when DPoP signing fails

797ee20

wmathurin merged commit 1c0c128 into forcedotcom:dev Oct 19, 2026
5 of 6 checks passed

Conversation

wmathurin commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Note on the diff

Tests

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wmathurin commented Oct 9, 2026

Uh oh!

Uh oh!

Uh oh!

brandonpage Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

wmathurin Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

brandonpage Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

wmathurin Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wmathurin commented Oct 12, 2026

Uh oh!

JohnsonEricAtSalesforce left a comment

Choose a reason for hiding this comment

Requested Changes

Open Questions

Uh oh!

JohnsonEricAtSalesforce commented Oct 14, 2026

Uh oh!

brandonpage commented Oct 15, 2026

Uh oh!

JohnsonEricAtSalesforce commented Oct 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wmathurin commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot Oct 10, 2026 •

edited

Loading

github-actions Bot Oct 10, 2026 •

edited

Loading