Support rendering URLs inside sync barcodes #5944

CDRussell · 2025-04-21T17:00:41Z

Task/Issue URL: https://app.asana.com/1/137249556945/project/608920331025315/task/1209921184459923?focus=true

Description

Allows sync setup codes which appear in the QR barcode to be URL-based.

Note, this is disabled by default.
Note, this PR doesn’t handle scanning a URL-based barcode which will be in a higher branch on this stack.

Steps to test this PR

Get two devices (or one device + one emulator) as you’ll need to scan barcodes.

Feature disabled, everything works as normal

The feature is disabled by default, so just fresh install the app from this branch on both devices
For each of the following scenarios, make sure you can set up sync AND ensure that the “code to include” in logcat is NOT a URL:
- Scanning barcode when both devices signed out
- Scanning barcode when both devices signed in
- Scanning barcode when scanning device is the only one signed out
- Scanning barcode when scanning device is the only one signed in
- Manually entering codes for the same scenarios…

Feature enabled

Enable syncSetupBarcodeIsUrlBased feature flag on a device. Note, we expect the URL to be in the barcode but the other device won’t be able to scan it successfully (implemented in a higher branch on this stack) so don’t worry about that part.
Repeat the scenarios above,
- ensuring the barcode is a URL when showing the barcode (e.g., scan it from system camera app and inspect it). URL should match the spec from the tech design.
- ensuring when manually sharing the code it does NOT use a URL

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1210271896321082

CDRussell · 2025-04-21T17:00:56Z

Visual tweaks to the sync barcode screen #6114 : 2 dependent PRs (#6121 , #6137 )
Intercept sync setup URLs that are passed into our app #6008
Support scanning URL-based sync setup codes #5957
Support rendering URLs inside sync barcodes #5944 👈 (View in Graphite)
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/qrcode/SyncBarcodeUrl.kt

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncWithAnotherActivityViewModel.kt

cmonfortep

LGTM

I would like to discuss if possible the responsibility of SyncBarcodeDecorator, and how feasible do you see moving the logic inside SyncAccountRepository. If you are ok and prefer to do it later in the stack, I'm fine with that.

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncConnectViewModel.kt

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/qrcode/SyncBarcodeDecorator.kt

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncWithAnotherActivityViewModel.kt

CDRussell · 2025-05-06T09:09:07Z

how feasible do you see moving the logic inside SyncAccountRepository

Will take a look at this and ping back with thoughts

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncInternalSettingsViewModel.kt

sync/sync-impl/src/test/java/com/duckduckgo/sync/impl/ui/qrcode/SyncBarcodeUrlDecoratorTest.kt

cmonfortep

All good, thanks for the addressing the comments 👍

CDRussell · 2025-05-13T13:42:32Z

Merge activity

May 13, 9:42 AM EDT: A user started a stack merge that includes this pull request via Graphite.
May 13, 9:42 AM EDT: @CDRussell merged this pull request with Graphite.

Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1209921186465304?focus=true ### Description #5944 allows the barcodes to be rendered as URL-based; this PR is the other side of the coin in adding support for scanning URL-based sync setup barcodes. #### A note on feature flagging For feature flagging around URL-based barcodes, I’ve separated out the ability to render them vs the ability to scan them. I think this will help the logistics around rolling it out, but sense-check that for me. | Feature flag | Description | Default state | |--------|--------|--------| |`syncSetupBarcodeIsUrlBased` | Whether to render URL-based or plain barcodes | Off by default | | `canScanUrlBasedSyncSetupBarcodes` | Whether we can scan URL-based codes | On by default (it’s a kill-switch | ### Steps to test this PR You’ll need two devices, at least one of them being a real device so you can scan the barcode using its camera. #### Scanning a `connect` code - [x] Log out of sync on both devices - [x] Choose `Sync with another device` on both - [x] Scan the barcode **using the system camera** first, to verify you are dealing with a URL-based barcode - [x] Scan the barcode from inside sync settings; verify sync sets up correctly #### Scanning an `exchange` code - [x] Log out of sync on a physical device; stay logged in on the other - [x] Choose `Sync with another device` on both - [x] Using physical device, scan the barcode **using the system camera** first, to verify you are dealing with a URL-based barcode - [x] Using physical device, scan the barcode from inside sync settings; verify sync sets up correctly #### Scanning a plaintext `recovery` code - [x] Log out of sync on a physical device; stay logged in on the other - [x] On the logged-in device, disable `exchangeKeysToSyncWithAnotherDevice` - [x] Choose `Sync with another device` on both devices - [x] Using physical device, scan the barcode **using the system camera** first, and verify you are **not** dealing with a URL-based barcode; this should be just the b64-encoded recovery code (recovery codes aren’t allowed in URLs, so disabling `exchangeKeysToSyncWithAnotherDevice` means URL-based codes won’t show either) - [x] Using physical device, scan the barcode from inside sync settings; verify sync sets up correctly #### Scanning a URL-based `recovery` code This isn’t acceptable so we won’t generate URLs containing recovery codes, but let’s test what happens if one is scanned. [Barcode for testing what happens if you scan a recovery code inside a URL](https://app.asana.com/1/137249556945/task/1210110837732281?focus=true) - [x] Log out of sync on a physical device (don’t need the other device for this test) - [x] Choose `Sync with another device` on the physical device - [x] Using physical device, scan the barcode linked above from inside sync settings; verify sync is **not** set up and you see an error message. In the logs, you’ll see `Recovery code found inside a URL which is not acceptable`

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 6 times, most recently from c1e559b to b3a3796 Compare April 24, 2025 16:38

CDRussell mentioned this pull request Apr 24, 2025

Support scanning URL-based sync setup codes #5957

Merged

16 tasks

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 2 times, most recently from a1b61b8 to 4ff8eae Compare April 25, 2025 12:42

CDRussell commented Apr 25, 2025

View reviewed changes

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/qrcode/SyncBarcodeUrl.kt Show resolved Hide resolved

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 6 times, most recently from 1dc1ad0 to 2ada4fb Compare April 25, 2025 15:42

CDRussell changed the title ~~Support URLs inside sync barcodes~~ Support rendering URLs inside sync barcodes Apr 25, 2025

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 2 times, most recently from 1072477 to 444943d Compare April 25, 2025 16:18

CDRussell commented Apr 25, 2025

View reviewed changes

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncWithAnotherActivityViewModel.kt Outdated Show resolved Hide resolved

CDRussell marked this pull request as ready for review April 25, 2025 16:23

CDRussell assigned cmonfortep Apr 25, 2025

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 3 times, most recently from 860b063 to 61a59a3 Compare April 29, 2025 09:50

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch from 61a59a3 to 61eaf09 Compare May 5, 2025 12:25

CDRussell mentioned this pull request May 5, 2025

Intercept sync setup URLs that are passed into our app #6008

Merged

33 tasks

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 2 times, most recently from 68e00fa to 3e35d17 Compare May 5, 2025 12:37

cmonfortep approved these changes May 5, 2025

View reviewed changes

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 3 times, most recently from eb3b7ad to e832503 Compare May 7, 2025 13:13

CDRussell requested a review from cmonfortep May 7, 2025 13:15

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 3 times, most recently from 488f371 to 9d657cb Compare May 7, 2025 13:58

cmonfortep reviewed May 8, 2025

View reviewed changes

sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncInternalSettingsViewModel.kt Outdated Show resolved Hide resolved

cmonfortep reviewed May 8, 2025

View reviewed changes

sync/sync-impl/src/test/java/com/duckduckgo/sync/impl/ui/qrcode/SyncBarcodeUrlDecoratorTest.kt Outdated Show resolved Hide resolved

cmonfortep approved these changes May 8, 2025

View reviewed changes

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch 2 times, most recently from 65d96d9 to 0b84d73 Compare May 12, 2025 14:23

CDRussell added 2 commits May 13, 2025 11:35

Support URLs inside sync barcodes

01bd75e

Address PR feedback, namely, moving more logic inside SyncRepository

b3e468f

CDRussell force-pushed the feature/craig/sync_barcode_url_support branch from 0b84d73 to b3e468f Compare May 13, 2025 10:35

CDRussell merged commit a891cc9 into develop May 13, 2025
6 checks passed

CDRussell deleted the feature/craig/sync_barcode_url_support branch May 13, 2025 13:42

This was referenced May 21, 2025

Visual tweaks to the sync barcode screen #6114

Merged

Translations for sync barcode screen changes #6121

Merged

Pixel interactions on sync setup screens #6137

Merged

CDRussell mentioned this pull request May 29, 2025

Show recovery code after sync deep link flow if applicable #6161

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support rendering URLs inside sync barcodes #5944

Support rendering URLs inside sync barcodes #5944

Uh oh!

CDRussell commented Apr 21, 2025 •

edited

Loading

Uh oh!

CDRussell commented Apr 21, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cmonfortep left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CDRussell commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

cmonfortep left a comment

Uh oh!

CDRussell commented May 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Support rendering URLs inside sync barcodes #5944

Support rendering URLs inside sync barcodes #5944

Uh oh!

Conversation

CDRussell commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to test this PR

Feature disabled, everything works as normal

Feature enabled

Uh oh!

CDRussell commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmonfortep left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CDRussell commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

cmonfortep left a comment

Choose a reason for hiding this comment

Uh oh!

CDRussell commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

CDRussell commented Apr 21, 2025 •

edited

Loading

CDRussell commented Apr 21, 2025 •

edited

Loading

CDRussell commented May 13, 2025 •

edited

Loading