Commit 7f3442d
authored
Sync exchange v2: Fix password deriation mismatch (#9014)
Task/Issue URL: https://app.asana.com/1/137249556945/task/1216049977533939?focus=true
Tech Design URL (if applicable):
### Description
Fixes a password derivation mismatch in the native 3party → ddg upgrade flow (joinAccountFromThirdPartyRecoveryCode).
When minting the new DDG credential on `POST /access-credentials/ddg`, `credentialHashedPassword` now uses `newDdgKeys.passwordHash` (libsodium/Argon2, same as signup) instead of an `HKDF-derived` hash from the new primary key. That keeps server-side stored hashes aligned with what `performLogin` sends via `prepareForLogin`.
### Steps to test this PR
> [!NOTE]
> - Fresh install `internal` build from this branch
> - You'll need two emulators/devices to test
> - When I say `Sync screen` I mean `Settings -> Sync & Backup`
**Native to native pairing**
- [x] **Both devices:**: Go to `Sync screen` and choose `Sync With Another Device`
- [x] Scan one barcode from the other (or copy/paste code between them)
- [ ] **Both devices:** On confirmation dialog that appears, choose `Sync Now`
- [x] **Both devices:** Verify both devices appear in the `Synced Devices` list (might take a few seconds)
**Adding in 3rd party browser**
- [x] On a 3rd party desktop browser (e.g., Firefox/Chrome), visit https://duck.ai/
- [x] Tap `Settings & More` (bottom left) then `Settings` and tap `Turn On Sync & Backup` -> `Sync With Another Device`
- [x] On Android device/emulator, visit `Sync screen` and tap `Sync With Another Device`
- [x] From Android, scan the 3p browser's barcode (copy / paste between them)
- [x] When prompted on Android, choose `Sync Now`
- [x] Verify you now have three devices in your device list; the two native Android ones and your 3rd party browser.
> [!IMPORTANT]
> Log out of sync on all devices (e.g., `Sync screen` -> `Turn Off Sync & Backup...`
**3rd party code first**
- [x] Confirm you are not signed in to sync with the native Android, nor on 3rd party browser
- [x] **In 3rd party browser:** Tap `Settings & More`, `Settings` and tap `Turn On Sync & Backup` -> `Sync With Another Device`
- [x] **In Android:** `Sync screen` then `Sync with Another Device`
- [x] Use Android app to scan 3rd party browser's code (or copy from 3p to Android app)
- [x] **In Android:** When you see user confirmation tap `Sync Now`
- [x] **In Android:** Verify you see two devices in your list; the current Android device and the 3rd party browser
- [x] **In Android:** Tap `Sync with Another Device`
- [x] **In the other Android:** Tap `Sync with Another Device` too
- [ ] Pair these together (one scans the other's code or copy paste), and tap `Sync Now` on both devices when prompted
- [x] Verify you now have 3 devices all signed into sync
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **High Risk**
> Changes authentication hashing and the post-upgrade login/commit path for joining sync accounts; a regression could block 3party upgrades or leave credentials in a bad state until TTL expiry.
>
> **Overview**
> Fixes a **password derivation mismatch** in the native **3party → ddg upgrade** flow (`joinAccountFromThirdPartyRecoveryCode`).
>
> When minting the new DDG credential on `POST /access-credentials/ddg`, **`credentialHashedPassword` now uses `newDdgKeys.passwordHash`** (libsodium/Argon2, same as signup) instead of an HKDF-derived hash from the new primary key. That keeps server-side stored hashes aligned with what **`performLogin`** sends via `prepareForLogin`.
>
> **Step 7** no longer uses the removed **`performDdgLoginForUpgrade`** plus a manual **`SyncStore`** write. It calls **`performLogin`** (with retries), so committing the credential, persisting the unrestricted ddg token, decrypting **`protected_encryption_key`**, setting **`credentialId`**, scoped password handling, and post-login sync all follow the normal login path.
>
> Interface/docs now describe **three** network steps, with the third unscoped `/sync/login` going through **`performLogin`**. Tests mock **`prepareForLogin`**, decrypt, and return **`protected_encryption_key`** on the post-upgrade login response.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 994d6c4. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 92a6600 commit 7f3442d
2 files changed
Lines changed: 51 additions & 96 deletions
File tree
- sync/sync-impl/src
- main/java/com/duckduckgo/sync/impl
- test/java/com/duckduckgo/sync/impl
Lines changed: 30 additions & 94 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
112 | 112 | | |
113 | 113 | | |
114 | 114 | | |
115 | | - | |
| 115 | + | |
116 | 116 | | |
117 | 117 | | |
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
122 | 125 | | |
123 | 126 | | |
124 | | - | |
125 | | - | |
| 127 | + | |
| 128 | + | |
126 | 129 | | |
127 | 130 | | |
128 | 131 | | |
| |||
750 | 753 | | |
751 | 754 | | |
752 | 755 | | |
753 | | - | |
754 | | - | |
755 | | - | |
756 | | - | |
757 | | - | |
758 | | - | |
759 | | - | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
760 | 759 | | |
761 | 760 | | |
762 | | - | |
| 761 | + | |
763 | 762 | | |
764 | 763 | | |
765 | 764 | | |
| |||
843 | 842 | | |
844 | 843 | | |
845 | 844 | | |
846 | | - | |
847 | | - | |
848 | | - | |
849 | | - | |
850 | | - | |
851 | | - | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
852 | 850 | | |
853 | 851 | | |
854 | 852 | | |
855 | 853 | | |
856 | 854 | | |
857 | | - | |
858 | | - | |
859 | | - | |
860 | | - | |
861 | | - | |
862 | | - | |
863 | | - | |
864 | | - | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
865 | 860 | | |
866 | | - | |
| 861 | + | |
| 862 | + | |
867 | 863 | | |
868 | | - | |
| 864 | + | |
869 | 865 | | |
870 | 866 | | |
871 | | - | |
872 | | - | |
873 | | - | |
874 | | - | |
875 | | - | |
876 | | - | |
877 | | - | |
878 | | - | |
879 | | - | |
880 | | - | |
881 | | - | |
882 | | - | |
883 | | - | |
884 | | - | |
885 | | - | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
886 | 874 | | |
887 | 875 | | |
888 | | - | |
889 | | - | |
890 | | - | |
891 | | - | |
892 | 876 | | |
893 | 877 | | |
894 | 878 | | |
| |||
1230 | 1214 | | |
1231 | 1215 | | |
1232 | 1216 | | |
1233 | | - | |
1234 | | - | |
1235 | | - | |
1236 | | - | |
1237 | | - | |
1238 | | - | |
1239 | | - | |
1240 | | - | |
1241 | | - | |
1242 | | - | |
1243 | | - | |
1244 | | - | |
1245 | | - | |
1246 | | - | |
1247 | | - | |
1248 | | - | |
1249 | | - | |
1250 | | - | |
1251 | | - | |
1252 | | - | |
1253 | | - | |
1254 | | - | |
1255 | | - | |
1256 | | - | |
1257 | | - | |
1258 | | - | |
1259 | | - | |
1260 | | - | |
1261 | | - | |
1262 | | - | |
1263 | | - | |
1264 | | - | |
1265 | | - | |
1266 | | - | |
1267 | | - | |
1268 | | - | |
1269 | | - | |
1270 | | - | |
1271 | | - | |
1272 | | - | |
1273 | | - | |
1274 | | - | |
1275 | | - | |
1276 | | - | |
1277 | | - | |
1278 | | - | |
1279 | | - | |
1280 | | - | |
1281 | 1217 | | |
1282 | 1218 | | |
1283 | 1219 | | |
| |||
Lines changed: 21 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1324 | 1324 | | |
1325 | 1325 | | |
1326 | 1326 | | |
| 1327 | + | |
| 1328 | + | |
| 1329 | + | |
| 1330 | + | |
| 1331 | + | |
| 1332 | + | |
| 1333 | + | |
| 1334 | + | |
| 1335 | + | |
| 1336 | + | |
| 1337 | + | |
| 1338 | + | |
| 1339 | + | |
| 1340 | + | |
1327 | 1341 | | |
1328 | 1342 | | |
1329 | 1343 | | |
| |||
1524 | 1538 | | |
1525 | 1539 | | |
1526 | 1540 | | |
1527 | | - | |
| 1541 | + | |
1528 | 1542 | | |
1529 | 1543 | | |
1530 | 1544 | | |
| |||
1551 | 1565 | | |
1552 | 1566 | | |
1553 | 1567 | | |
| 1568 | + | |
| 1569 | + | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
1554 | 1573 | | |
1555 | 1574 | | |
1556 | 1575 | | |
| |||
1567 | 1586 | | |
1568 | 1587 | | |
1569 | 1588 | | |
1570 | | - | |
| 1589 | + | |
1571 | 1590 | | |
1572 | 1591 | | |
1573 | 1592 | | |
| |||
0 commit comments