fix(W-23836447): allow dpop_jkt on login pool servers

CLAUDE.md

@@ -67,11 +67,35 @@ See [README.md](README.md) for basic setup. Commands below are for contributors
 ./gradlew :libs:SmartStore:build
 ./gradlew :libs:MobileSync:build
 
-# Run tests for a specific library (runs on Firebase Test Lab in CI)
+# Run instrumented tests for a specific library on a connected emulator/device
+# Note: SalesforceSDKTest sources live in libs/test/SalesforceSDKTest but are wired into
+# :libs:SalesforceSDK's androidTest source set via build.gradle.kts setRoot().
+# Run them via :libs:SalesforceSDK:connectedAndroidTest, NOT a separate project.
 ./gradlew :libs:SalesforceSDK:connectedAndroidTest
 ./gradlew :libs:SmartStore:connectedAndroidTest
 ./gradlew :libs:MobileSync:connectedAndroidTest
 
+# Run a single test class on the emulator
+./gradlew :libs:SalesforceSDK:connectedAndroidTest \
+  -Pandroid.testInstrumentationRunnerArguments.class=com.salesforce.androidsdk.auth.LoginViewModelTest
+
+# Run a single test method on the emulator
+./gradlew :libs:SalesforceSDK:connectedAndroidTest \
+  -Pandroid.testInstrumentationRunnerArguments.class=com.salesforce.androidsdk.auth.LoginViewModelTest#generateAuthorizationUrl_WhenUseDPoP_AndPoolServer_AddsDpopJktToUrl
+
+# Run AuthFlowTester UI tests on emulator (requires ui_test_config.json in shared/test/ with valid org credentials)
+# The emulator CAN reach internal test environments when the machine has VPN/network access.
+# First verify emulator is running: adb devices
+./gradlew :native:NativeSampleApps:AuthFlowTester:connectedAndroidTest \
+  -Pandroid.testInstrumentationRunnerArguments.class=com.salesforce.samples.authflowtester.DPoPLoginTests
+
+# Run a single AuthFlowTester UI test method
+./gradlew :native:NativeSampleApps:AuthFlowTester:connectedAndroidTest \
+  -Pandroid.testInstrumentationRunnerArguments.class=com.salesforce.samples.authflowtester.DPoPLoginTests#testECAJwtDPoP_ViaLoginPoolServer
+
+# Run all AuthFlowTester UI tests
+./gradlew :native:NativeSampleApps:AuthFlowTester:connectedAndroidTest
+
 # Run lint checks
 ./gradlew :libs:SalesforceSDK:lint
 ./gradlew :libs:SmartStore:lint

libs/SalesforceSDK/src/com/salesforce/androidsdk/auth/OAuth2.java

@@ -776,7 +776,6 @@ public static TokenEndpointResponse makeTokenEndpointRequest(HttpAccess httpAcce
                 DPoPNonceCache.INSTANCE.store(credentialsIdentifier, tokenHost, responseNonce);
             }
         }
-
         // Nonce challenge: server requires a nonce. Retry once with the harvested nonce.
         if (attachDPoP && isNonceChallenge(response)) {
             response.close();

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginViewModel.kt

@@ -713,17 +713,25 @@ open class LoginViewModel(
     // endregion
 
     /**
-     * Adds `dpop_jkt` to [params] when DPoP is enabled and [server] is a my-domain server.
-     * Pool servers (login.salesforce.com, test.salesforce.com, welcome.salesforce.com) do not
-     * support DPoP code binding and reject the parameter.
+     * Adds `dpop_jkt` to [params] when DPoP is enabled.
+     * welcome.salesforce.com/discovery is never passed here — discovery resolves a my-domain
+     * server before /authorize is called.
+     *
+     * If [pendingCredentialsIdentifier] is already set (meaning dpop_jkt was already committed
+     * for this login flow, e.g. via a pool-server redirect), the existing key pair is reused so
+     * that the auth code's dpop_jkt binding and the token-exchange proof use the same key.
+     * Only generates a new key pair when starting a fresh login flow.
      */
     private fun addDpopJktIfNeeded(
         server: String,
         sdkManager: SalesforceSDKManager,
         params: MutableMap<String, String>,
     ) {
-        val isMyDomainServer = !LoginServerManager.isPoolServer(server)
-        if (!sdkManager.useDPoP || !isMyDomainServer) {
+        // Welcome Discovery is a pre-authentication host, not a resource server — never attach
+        // dpop_jkt there. Belt-and-suspenders guard: generateAuthorizationUrl is not called for
+        // the discovery URL today (reloadWebView short-circuits it), but this prevents a stale
+        // thumbprint from leaking if the call graph changes in the future.
+        if (!sdkManager.useDPoP || LoginServerManager.WELCOME_LOGIN_URL == server) {
             // Clear any stale dpop_jkt and its key from a previous server-picker entry.
             params.remove("dpop_jkt")
             pendingCredentialsIdentifier?.let {
@@ -733,16 +741,12 @@ open class LoginViewModel(
             return
         }
         runCatching {
-            // Delete any orphaned key from a prior server-picker navigation before generating a new one.
-            pendingCredentialsIdentifier?.let {
-                DPoPKeyManager.deleteKeyPair(DPoPKeyManager.aliasForCredentialsIdentifier(it))
-            }
-            val credId = java.util.UUID.randomUUID().toString()
+            val credId = pendingCredentialsIdentifier
+                ?: java.util.UUID.randomUUID().toString().also { pendingCredentialsIdentifier = it }
             val alias = DPoPKeyManager.aliasForCredentialsIdentifier(credId)
             val keyPair = DPoPKeyManager.generateOrLoadKeyPair(alias)
             val thumbprint = DPoPProofBuilder.jwkThumbprint(keyPair.public as ECPublicKey)
             params["dpop_jkt"] = thumbprint
-            pendingCredentialsIdentifier = credId
         }.onFailure { t ->
             android.util.Log.w(TAG, "Failed to compute dpop_jkt for /authorize; proceeding without it", t)
         }

libs/test/SalesforceSDKTest/src/com/salesforce/androidsdk/auth/LoginViewModelTest.kt

@@ -493,6 +493,76 @@ class LoginViewModelTest {
         }
     }
 
+    @Test
+    fun generateAuthorizationUrl_WhenUseDPoP_AndPoolServer_AddsDpopJktToUrl() = runBlocking {
+        // dpop_jkt must be sent for pool servers when useDPoP=true.
+        val sdkManagerMock = mockk<SalesforceSDKManager>(relaxed = true)
+        every { sdkManagerMock.isDebugBuild } returns false
+        every { sdkManagerMock.useHybridAuthentication } returns false
+        every { sdkManagerMock.isBrowserLoginEnabled } returns false
+        every { sdkManagerMock.appConfigForLoginHost } returns { _ -> null }
+        every { sdkManagerMock.debugOverrideAppConfig } returns null
+        every { sdkManagerMock.useDPoP } returns true
+
+        viewModel.generateAuthorizationUrl("https://login.salesforce.com", sdkManagerMock)
+        val url = viewModel.loginUrl.value ?: ""
+        assert(url.contains("dpop_jkt=")) {
+            "Expected dpop_jkt in authorization URL for pool server when useDPoP=true, got: $url"
+        }
+        val thumbprint = url.toUri().getQueryParameter("dpop_jkt") ?: ""
+        assert(thumbprint.matches(Regex("[A-Za-z0-9_-]{43}"))) {
+            "dpop_jkt must be 43-char base64url RFC 7638 thumbprint, got: '$thumbprint'"
+        }
+    }
+
+    /**
+     * Regression guard for W-23836447: pool server login calls generateAuthorizationUrl multiple
+     * times (pool → my-domain redirect).  The dpop_jkt must remain stable across all calls so
+     * that the auth code's dpop_jkt binding and the subsequent token-exchange DPoP proof use the
+     * same key.
+     */
+    @Test
+    fun test_givenDPoPEnabled_whenGenerateAuthorizationUrlCalledTwice_thenDpopJktIsStable() = runBlocking {
+        val sdkManagerMock = mockk<SalesforceSDKManager>(relaxed = true)
+        every { sdkManagerMock.isDebugBuild } returns false
+        every { sdkManagerMock.useHybridAuthentication } returns false
+        every { sdkManagerMock.isBrowserLoginEnabled } returns false
+        every { sdkManagerMock.appConfigForLoginHost } returns { _ -> null }
+        every { sdkManagerMock.debugOverrideAppConfig } returns null
+        every { sdkManagerMock.useDPoP } returns true
+
+        // First call — simulates pool server issuing the initial /authorize redirect.
+        viewModel.generateAuthorizationUrl("https://login.salesforce.com", sdkManagerMock)
+        val firstUrl = viewModel.loginUrl.value ?: ""
+        val firstThumbprint = firstUrl.toUri().getQueryParameter("dpop_jkt") ?: ""
+        val firstCredId = viewModel.pendingCredentialsIdentifier
+
+        assert(firstThumbprint.isNotEmpty()) {
+            "Expected dpop_jkt after first generateAuthorizationUrl call, got empty"
+        }
+        assertNotNull("pendingCredentialsIdentifier must be set after first call", firstCredId)
+
+        // Second call — simulates the pool server redirecting to my-domain /authorize.
+        viewModel.generateAuthorizationUrl("https://myorg.my.salesforce.com", sdkManagerMock)
+        val secondUrl = viewModel.loginUrl.value ?: ""
+        val secondThumbprint = secondUrl.toUri().getQueryParameter("dpop_jkt") ?: ""
+        val secondCredId = viewModel.pendingCredentialsIdentifier
+
+        assert(secondThumbprint.isNotEmpty()) {
+            "Expected dpop_jkt after second generateAuthorizationUrl call, got empty"
+        }
+        assertEquals(
+            "dpop_jkt must be stable across pool-server redirects (same key must be reused)",
+            firstThumbprint,
+            secondThumbprint,
+        )
+        assertEquals(
+            "pendingCredentialsIdentifier must be the same across pool-server redirects",
+            firstCredId,
+            secondCredId,
+        )
+    }
+
     // endregion
 
     // region frontDoorBridgeUrl Tests

native/NativeSampleApps/AuthFlowTester/README.md

@@ -60,9 +60,11 @@ All DPoP tests live here — basic login, RTR, multi-user, migration, and restar
 | `testECAJwtDPoPRtr_Hybrid` | ECA JWT DPoP RTR | Yes | DPoP + refresh token rotation |
 | `testECAJwtDPoPRtr_NoHybrid` | ECA JWT DPoP RTR | No | DPoP + refresh token rotation |
 | `testECAJwtDPoP_MultiUser_UniqueTokens` | ECA JWT DPoP | — | Two users; unique tokens; independent revoke+refresh per user |
+| `testECAJwtDPoP_And_NonDPoP_MultiUser_FlagOff_IndependentProofs` | ECA JWT DPoP + ECA JWT | — | DPoP and non-DPoP users coexist; toggling DPoP off for second user does not affect first |
 | `testMigrate_ECAJwtDPoP_AddMoreScopes` | ECA JWT DPoP | — | Scope upgrade; DPoP binding preserved |
 | `testMigrate_ECAJwtDPoP_To_ECAJwtDPoPRtr` | ECA JWT DPoP → ECA JWT DPoP RTR | — | Migrate from DPoP to DPoP+RTR |
 | `testECAJwtDPoP_WithRestart` | ECA JWT DPoP | — | DPoP EC key pair survives process restart (AndroidKeyStore) |
+| `testECAJwtDPoP_ViaLoginPoolServer` | ECA JWT DPoP | — | `@Ignore` (W-23864247 — pool login server rejects valid `dpop_jkt` token exchange) |
 | `testLoginForAdmin_DPoP` | ECA JWT DPoP | — | Login for Admins hand-off to Custom Tab works with DPoP |
 
 #### RTRLoginTests

native/NativeSampleApps/AuthFlowTester/src/androidTest/java/com/salesforce/samples/authflowtester/DPoPLoginTests.kt

@@ -143,14 +143,16 @@ class DPoPLoginTests : AuthFlowTest() {
 
         // Switch to user A (DPoP-bound). Refresh + REST GET must attach a DPoP proof —
         // gated by credential state, not the global flag.
-        switchToUserAndValidateUser(user, isDpop = true)
+        // ECA_JWT_DPOP issues JWT tokens, so isJwt=true is required to expect JT in the UA.
+        switchToUserAndValidateUser(user, isDpop = true, isJwt = true)
         app.validateOAuthValues(knownAppConfig = ECA_JWT_DPOP, scopeSelection = ScopeSelection.EMPTY)
-        assertRevokeAndRefreshWorks(isRtr = false, isDpop = true, isMultiUser = true)
+        assertRevokeAndRefreshWorks(isRtr = false, isDpop = true, isMultiUser = true, isJwt = true)
 
         // Switch to user B (Bearer). Refresh + REST GET must NOT attach DPoP anywhere.
-        switchToUserAndValidateUser(otherUser, isDpop = false)
+        // ECA_JWT issues JWT tokens, so isJwt=true is required to expect JT (not OT) in the UA.
+        switchToUserAndValidateUser(otherUser, isDpop = false, isJwt = true)
         app.validateOAuthValues(knownAppConfig = ECA_JWT, scopeSelection = ScopeSelection.EMPTY)
-        assertRevokeAndRefreshWorks(isRtr = false, isDpop = false, isMultiUser = true)
+        assertRevokeAndRefreshWorks(isRtr = false, isDpop = false, isMultiUser = true, isJwt = true)
     }
 
     // endregion
@@ -221,6 +223,31 @@ class DPoPLoginTests : AuthFlowTest() {
 
     // endregion
 
+    // endregion
+
+    // region DPoP Pool Server Tests
+
+    // Login via the pool server (login.test1.pc-rnd.salesforce.com) with DPoP enabled
+    // and verify dpop_jkt was accepted and DPoP binding holds after a revoke+refresh.
+    //
+    // Skipped: server-side bug W-23864247 — the pool login server returns
+    // invalid_dpop_proof on the authorization-code token exchange even though the
+    // DPoP proof is cryptographically valid and the JWK thumbprint exactly matches
+    // the dpop_jkt sent in /authorize.  Re-enable when the server fix is confirmed.
+    @Ignore("W-23864247: pool login server rejects valid dpop_jkt token exchange")
+    @Test
+    fun testECAJwtDPoP_ViaLoginPoolServer() {
+        loginAndValidate(
+            knownAppConfig = ECA_JWT_DPOP,
+            useHybridAuthToken = false,
+            useDPoP = true,
+            useLoginPoolHost = true,
+        )
+        assertRevokeAndRefreshWorks(isRtr = false, isDpop = true, isJwt = true)
+    }
+
+    // endregion
+
     // region DPoP Login for Admins Tests
 
     // Login for Admins with DPoP ECA; verifies the admin Custom Tab hand-off works with DPoP.

native/NativeSampleApps/AuthFlowTester/src/androidTest/java/com/salesforce/samples/authflowtester/ECALoginTests.kt

@@ -33,6 +33,7 @@ import com.salesforce.samples.authflowtester.testUtility.KnownAppConfig.ECA_JWT
 import com.salesforce.samples.authflowtester.testUtility.KnownAppConfig.ECA_OPAQUE
 import com.salesforce.samples.authflowtester.testUtility.ScopeSelection.ALL
 import com.salesforce.samples.authflowtester.testUtility.ScopeSelection.SUBSET
+import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith
 
@@ -83,4 +84,18 @@ fun testECAJwt_SubsetScopes_NotHybrid() {
 fun testECAJwt_AllScopes() {
         loginAndValidate(knownAppConfig = ECA_JWT, scopeSelection = ALL)
     }
+
+    // region ECA Pool Server Tests
+
+    // Login via the pool server without DPoP and verify the session is valid.
+    //
+    // Skipped: loginPoolHost not yet provisioned in CI ui_test_config.json.
+    // Re-enable once the key is added and the CI environment can reach the pool server.
+    @Ignore("loginPoolHost not provisioned in CI config — add the key and re-enable")
+    @Test
+    fun testECAJwt_ViaLoginPoolServer() {
+        loginAndValidate(knownAppConfig = ECA_JWT, useLoginPoolHost = true)
+    }
+
+    // endregion
 }

native/NativeSampleApps/AuthFlowTester/src/androidTest/java/com/salesforce/samples/authflowtester/NegativeLoginTests.kt

@@ -102,10 +102,13 @@ class NegativeLoginTests : AuthFlowTest() {
         loginPage.openLoginOptions()
         loginOptions.setOverrideBootConfig(ECA_OPAQUE, EMPTY)
 
-        // Saving Login Options re-launches the Custom Tab. Back out of it (and dismiss the
-        // resulting server picker) so navigateBackToApp only has to walk the remaining
-        // LoginActivity -> AccountSwitcher -> AuthFlowTester stack.
+        // Saving Login Options re-launches the Custom Tab. Back out of it, then exit the flow via
+        // the picker's login-exit back button: since W-23731759 the login-server picker is a
+        // non-dismissable modal sheet that swallows device back presses, so it must be dismissed
+        // through its own back button (which finishes LoginActivity) before navigateBackToApp can
+        // walk the remaining AccountSwitcher -> AuthFlowTester stack.
         loginPage.backOutToLoginActivity()
+        loginPage.exitServerPickerIfShowing()
         navigateBackToApp()
 
         // The existing user must remain the only authenticated account.
@@ -179,11 +182,11 @@ class NegativeLoginTests : AuthFlowTest() {
         private const val INVALID_SCOPE = "invalid_scope_for_negative_tests"
 
         // Maximum number of back-presses to walk from a saved-but-unused
-        // dynamic config back to the AuthFlowTester main screen.
-        // LoginOptions has been dismissed by Save, so worst-case stack is
-        // LoginActivity -> AccountSwitcher -> AuthFlowTester (2 presses);
-        // an extra press accommodates devices that are slow to dismiss
-        // dialogs or transitions.
+        // dynamic config back to the AuthFlowTester main screen. The login
+        // picker has already been exited via its back button (which finishes
+        // LoginActivity), so worst-case stack is AccountSwitcher ->
+        // AuthFlowTester (2 presses); an extra press accommodates devices that
+        // are slow to dismiss dialogs or transitions.
         private const val BACK_PRESS_LIMIT = 4
         private const val PER_BACK_PRESS_TIMEOUT_MS = 3_000L
         private const val POLL_INTERVAL_MS = 250L

native/NativeSampleApps/AuthFlowTester/src/androidTest/java/com/salesforce/samples/authflowtester/RefreshTokenMigrationTests.kt

@@ -227,6 +227,7 @@ class RefreshTokenMigrationTests: AuthFlowTest() {
         forceAdvancedAuthentication: Boolean,
         useWelcomeDiscovery: Boolean,
         isMultiUser: Boolean,
+        useLoginPoolHost: Boolean,
     ) {
         super.loginAndValidate(
             knownAppConfig = knownAppConfig,
@@ -239,6 +240,7 @@ class RefreshTokenMigrationTests: AuthFlowTest() {
             knownUserConfig = user,
             useWelcomeDiscovery = useWelcomeDiscovery,
             isMultiUser = isMultiUser,
+            useLoginPoolHost = useLoginPoolHost,
         )
     }
 }