Sync exchange v2: Fix password deriation mismatch

Author: CDRussell

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/SyncAccountRepository.kt

@@ -112,17 +112,20 @@ interface SyncAccountRepository {
     /**
      * Joins this device to an existing account using a 3party recovery code, executing the
      * "Native joining a 3party account" upgrade flow per the Unified Algorithm (Asana
-     * 1214739740392701). Two network calls:
+     * 1214739740392701). Three network calls:
      *
      *   1. POST /sync/login with scope=ai_chats — authenticates against the existing 3party
      *      credential and returns a short-lived token + the protected keys to re-wrap.
      *   2. POST /access-credentials/ddg — mints a fresh DDG credential on the account, attached
      *      alongside the existing 3party entry (which gets decorated with encrypted_3party_credential
      *      so future ddg-side logins can re-derive SP).
+     *   3. POST /sync/login (unscoped) — commits the new credential (without this inside the 5-min
+     *      TTL the BE auto-removes it), exchanges the ai_chats-only token for the unrestricted
+     *      ddg-scoped token, and atomically writes the SyncStore via [performLogin].
      *
      * On success the device ends in a normal Native signed-in state: full primaryKey / secretKey
-     * populated, credentialId=ddg, scopedPassword populated with SP. SyncStore is written
-     * atomically only after both network calls succeed; observers never see an intermediate state.
+     * populated, credentialId=ddg, scopedPassword populated with SP. SyncStore is written only
+     * after all three network calls succeed; observers never see an intermediate state.
      */
     fun joinAccountFromThirdPartyRecoveryCode(pastedCode: String): Result<Boolean>
 
@@ -750,16 +753,12 @@ class AppSyncAccountRepository @Inject constructor(
         val hashedPasswordForReauth = kotlin.runCatching { syncJweCrypto.hkdfDeriveBase64Url(spStandardB64, hkdfSalt, "Password", 32) }
             .getOrElse { return Error(reason = "Upgrade: failed to derive 3party hashed_password: ${it.message}") }
 
-        // credentialHashedPassword for the new DDG credential — HKDF(MP, salt=user_id, info="Password", 32).
-        // The server stores twice_hash(this) and validates future /login submissions against it.
-        // Pinned against the TD's test1 vector in SyncJweCryptoTdVectorsTest.
-        val credentialHashedPassword = kotlin.runCatching {
-            syncJweCrypto.hkdfDeriveBase64Url(newDdgKeys.primaryKey, hkdfSalt, "Password", 32)
-        }.getOrElse { return Error(reason = "Upgrade: failed to derive new DDG credential_hashed_password: ${it.message}") }
-
+        // credentialHashedPassword for the new DDG credential — libsodium/Argon2, the same derivation
+        // performCreateAccount uses for standard signup. Keeping every ddg credential libsodium-hashed
+        // means the standard performLogin path works for both signup-created and upgrade-created
         val request = CreateAccessCredentialRequest(
             hashedPassword = hashedPasswordForReauth,
-            credentialHashedPassword = credentialHashedPassword,
+            credentialHashedPassword = newDdgKeys.passwordHash,
             protectedEncryptionKey = newDdgKeys.protectedSecretKey,
             encrypted3partyCredential = encryptedThreePartyCredential,
             keys = rewrappedKeysList.ifEmpty { null },
@@ -843,52 +842,37 @@ class AppSyncAccountRepository @Inject constructor(
             return postResult.copy(reason = "JoinFrom3party: ${postResult.reason}")
         }
 
-        // Step 7a — Flow 4 native login with the new ddg credential. This is the BE-side commit
-        // for the credential just POSTed: without a login inside the 5-minute TTL the server
-        // auto-removes the credential. It also yields the unrestricted ddg-scoped token that
-        // device-management endpoints need (the 3party login token at this point is ai_chats-only).
-        val ddgLoginResponse = when (
-            val ddgLogin = performDdgLoginForUpgrade(
+        // Step 7 — Native login as the new ddg credential. Required: without a login inside the
+        // 5-minute TTL the BE auto-removes the credential, and the 3party token from Step 2 is
+        // ai_chats-scoped so it can't drive device-management endpoints.
+        val ddgLoginResult = retryingOnTransientError {
+            performLogin(
                 userId = parsed.userId,
                 deviceId = deviceId,
                 deviceName = deviceName,
                 primaryKey = upgradePackage.newDdgKeys.primaryKey,
             )
-        ) {
-            is Error -> {
-                if (ddgLogin.code == API_CODE.INVALID_LOGIN_CREDENTIALS.code) {
-                    logcat(ERROR) {
-                        "Sync-ScopedToken: ddg login after upgrade returned 401 — credential 5-minute TTL likely expired"
-                    }
-                } else {
-                    logcat(ERROR) { "Sync-ScopedToken: ddg login after upgrade failed: ${ddgLogin.reason}" }
+        }
+        if (ddgLoginResult is Error) {
+            if (ddgLoginResult.code == API_CODE.INVALID_LOGIN_CREDENTIALS.code) {
+                logcat(ERROR) {
+                    "Sync-ScopedToken: ddg login after upgrade returned 401 — credential 5-minute TTL likely expired"
                 }
-                return ddgLogin.copy(reason = "JoinFrom3party: post-upgrade ddg login failed: ${ddgLogin.reason}")
+            } else {
+                logcat(ERROR) { "Sync-ScopedToken: ddg login after upgrade failed: ${ddgLoginResult.reason}" }
             }
-            is Success -> ddgLogin.data
+            return ddgLoginResult.copy(reason = "JoinFrom3party: post-upgrade ddg login failed: ${ddgLoginResult.reason}")
         }
 
-        // Step 7b — atomic SyncStore commit. Everything above has either failed (returning Error
-        // without mutating SyncStore) or succeeded. External observers see the device as
-        // pre-upgrade until this block runs.
-        val spStandardB64 = kotlin.runCatching { base64UrlStringToStandardBase64(parsed.secret) }
-            .getOrElse { return Error(reason = "JoinFrom3party: failed to decode SP: ${it.message}") }
-        syncStore.storeCredentials(
-            userId = parsed.userId,
-            deviceId = deviceId,
-            deviceName = deviceName,
-            primaryKey = upgradePackage.newDdgKeys.primaryKey,
-            secretKey = upgradePackage.newDdgKeys.secretKey,
-            token = ddgLoginResponse.token,
-        )
-        syncStore.credentialId = CREDENTIAL_ID_DDG
-        syncStore.scopedPassword = ScopedPassword(spStandardB64)
+        // Defensive: performLogin will also set scopedPassword if the BE echoes back the new
+        // credential's encrypted_3party_credential, but we have the SP in hand from the pasted
+        // recovery code. Writing it locally guarantees scopedPassword is populated independent
+        // of the server response shape.
+        kotlin.runCatching { base64UrlStringToStandardBase64(parsed.secret) }
+            .onSuccess { syncStore.scopedPassword = ScopedPassword(it) }
+            .onFailure { logcat(ERROR) { "Sync-ScopedToken: failed to write local SP after upgrade: ${it.message}" } }
 
         logcat { "Sync-ScopedToken: 3party→ddg upgrade complete; account joined as ddg" }
-
-        appCoroutineScope.launch(dispatcherProvider.io()) {
-            syncEngine.triggerSync(ACCOUNT_LOGIN)
-        }
         return Success(true)
     }
 
@@ -1230,54 +1214,6 @@ class AppSyncAccountRepository @Inject constructor(
         )
     }
 
-    /**
-     * This login acts as the BE-side *commit* for the newly minted credential —
-     * without it, the server removes the credential after a 5-minute TTL. It is also what
-     * yields an unrestricted ddg-scoped token; the 3party token from [performThirdPartyLogin]
-     * is `scope=ai_chats` and cannot drive device-management endpoints.
-     *
-     * Returns the [LoginResponse] for the caller to commit alongside the new local key
-     * material.
-     */
-    private fun performDdgLoginForUpgrade(
-        userId: String,
-        deviceId: String,
-        deviceName: String,
-        primaryKey: String,
-    ): Result<LoginResponse> {
-        // HKDF-derived hashed_password matching the `credentialHashedPassword` we POSTed in
-        // [buildThirdPartyUpgradePackage] — the upgrade-created ddg credential was registered with
-        // the v2 cross-platform algorithm (Encryption Algorithms TD: HKDF(MP, salt=user_id,
-        // info="Password", 32)), NOT v1's, using nativeLib.prepareForLogin(...) would 401
-        val hkdfSalt = userId.toByteArray(Charsets.UTF_8)
-        val hashedPassword = kotlin.runCatching {
-            syncJweCrypto.hkdfDeriveBase64Url(primaryKey, hkdfSalt, "Password", 32)
-        }.getOrElse { return Error(reason = "Upgrade ddg login: derive hashed_password failed: ${it.message}") }
-
-        val deviceType = syncDeviceIds.deviceType()
-        val encryptedDeviceName = kotlin.runCatching {
-            nativeLib.encryptData(deviceName, primaryKey).also {
-                it.checkResult("Upgrade ddg login: encrypt device name failed")
-            }.encryptedData
-        }.getOrElse { return it.asErrorResult() }
-        val encryptedDeviceType = kotlin.runCatching {
-            nativeLib.encryptData(deviceType.deviceFactor, primaryKey).also {
-                it.checkResult("Upgrade ddg login: encrypt device type failed")
-            }.encryptedData
-        }.getOrElse { return it.asErrorResult() }
-
-        return retryingOnTransientError {
-            syncApi.login(
-                userID = userId,
-                hashedPassword = hashedPassword,
-                deviceId = deviceId,
-                deviceName = encryptedDeviceName,
-                deviceType = encryptedDeviceType,
-                scope = null,
-            )
-        }
-    }
-
     private fun performLogin(
         userId: String,
         deviceId: String,

sync/sync-impl/src/test/java/com/duckduckgo/sync/impl/AppSyncAccountRepositoryTest.kt

@@ -1324,6 +1324,20 @@ class AppSyncAccountRepositoryTest {
         )
     }
 
+    @Test
+    fun whenJoinAccountFromThirdPartySucceedsThenScopedPasswordIsWrittenFromLocalRecoveryCode() {
+        // Defensive: even if the BE response omits encrypted_3party_credential (so performLogin's
+        // server-decoded SP path is a no-op), the join should still leave scopedPassword populated
+        // from the locally-parsed recovery code.
+        val pastedCode = prepareForJoinAccountFromThirdPartyRecoveryCode()
+
+        val result = syncRepo.joinAccountFromThirdPartyRecoveryCode(pastedCode)
+
+        assertEquals(Success(true), result)
+        val expectedSp = base64UrlStringToStandardBase64("rUzlGqLLlbonAC_zIeh1nrCmuDsDAn6UooUUDz-6x3o")
+        verify(syncStore).scopedPassword = ScopedPassword(expectedSp)
+    }
+
     @Test
     fun whenJoinAccountFromThirdPartyThenStep7aPostUpgradeLoginUsesUnrestrictedScope() {
         val pastedCode = prepareForJoinAccountFromThirdPartyRecoveryCode()
@@ -1524,7 +1538,7 @@ class AppSyncAccountRepositoryTest {
     private val step7aLoginSuccess = Success(
         LoginResponse(
             token = "ddg_token_step7a",
-            protected_encryption_key = null,
+            protected_encryption_key = protectedEncryptionKey,
             devices = emptyList(),
             accessCredentials = null,
             keys = null,
@@ -1551,6 +1565,11 @@ class AppSyncAccountRepositoryTest {
         whenever(syncJweCrypto.jweEncryptSymmetric(any(), any(), any())).thenReturn("encrypted_3party_credential")
         whenever(nativeLib.generateAccountKeys(userId = eq(userId), password = anyString())).thenReturn(accountKeys)
 
+        // Step 7's performLogin uses libsodium derivations on the new ddg primaryKey, then decrypts
+        // the protected_encryption_key the server echoes back to recover the secretKey.
+        whenever(nativeLib.prepareForLogin(primaryKey = primaryKey)).thenReturn(validLoginKeys)
+        whenever(nativeLib.decrypt(encryptedData = protectedEncryptionKey, secretKey = stretchedPrimaryKey)).thenReturn(decryptedSecretKey)
+
         // Step 2 response; optionally includes ddg to drive the "already upgraded" path.
         val accessCredentialsInResponse = buildList {
             add(AccessCredentialEntry(id = "3party", scope = "ai_chats"))
@@ -1567,7 +1586,7 @@ class AppSyncAccountRepositoryTest {
         val step7aResponse = step7aResult ?: Success(
             LoginResponse(
                 token = "ddg_token_step7a",
-                protected_encryption_key = null,
+                protected_encryption_key = protectedEncryptionKey,
                 devices = emptyList(),
                 accessCredentials = null,
                 keys = null,