DBP: Integrate subscription account authentication to DBP #1995

jotaemepereira · 2023-12-20T18:29:49Z

Task

https://app.asana.com/0/1204006570077678/1206163442459077/f

Description

Given a user with a subscription, when the user signs in to his account, then we store the auth token to use as bearer on requests to the dbp API.
Given an authenticated user, when the user taps the PIR tab on the more options menu, then no waitlist related dialog appears.

Steps to test

Test everything works as expected

Create a sandbox test account on App Store Connect
Go to App Store -> Settings, and sign-in with that account on the Sandbox Account section
Change the baseUrl on EmailService and CaptchaService to point to staging (dbp-staging.duckduckgo.com)
Run the Privacy Pro target
Go to Debug -> Subscription -> Purchase Subscription from App Store
Chose the yearly or monthly subscription
The Personal Information Removal option should be available in the more options menu
Tap it, and start the process (creating profile, etc)
It is important to choose a profile with matches
When the scanning process finishes go to Debug -> PIR -> Operations -> Visible webviews -> Run opt-out operations
Then, check the email field on the web view is filled correctly

Test the default browser target

Run the DuckDuckGo Privacy Browser target
Tap on the PIR tab. It should prompt the waitlist dialog if not present there

jotaemepereira · 2023-12-20T18:31:41Z

DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

+    @objc private func handleAccountDidSignIn() {
+        guard let token = accountManager.token else {
+            Pixel.fire(.dataBrokerProtectionSubscriptionErrorWhenFetchingToken)
+            assertionFailure("[DBP Subscription] AccountManager signed in but token could not be retrieved")


@samsymons I know this is unlikely, but I wonder if I should add a safeguard somewhere else for this to happen.

For example, fetching the token again before using it (if no token is present). What are your thoughts?

I think in this case we're good since we're reacting to a sign in event. But, we can talk with Michał about it to be sure – I am of the opinion that we should only need to access the token, and the underlying subscription logic should ensure that we always get the latest.

DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

samsymons · 2023-12-21T01:21:41Z

DuckDuckGo/Application/AppDelegate.swift

@@ -73,6 +73,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel
    private var emailCancellables = Set<AnyCancellable>()
    let bookmarksManager = LocalBookmarkManager.shared

+#if DBP && SUBSCRIPTION


👍 I've been adding some of these checks myself as a part of the remote messaging change. Once we enable these features in the App Store target, we should be good to remove the flags.

samsymons · 2023-12-21T02:01:28Z

DuckDuckGo/Statistics/PixelEvent.swift

+        case .dataBrokerProtectionSubscriptionErrorWhenFetchingToken:
+            return "m_mac_dbp_subscription_error_when_fetching_token"


Note that this will need privacy triage, but we can probably just include it in a larger Privacy Pro integration privacy triage later, since this pixel isn't going out to any users (even internally) just yet.

Makes sense. I will leave a note on Asana so we do not forget.

samsymons · 2023-12-21T02:02:35Z

DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

+
+    @objc private func handleAccountDidSignIn() {
+        guard let token = accountManager.token else {
+            Pixel.fire(.dataBrokerProtectionSubscriptionErrorWhenFetchingToken)


I'd consider renaming this pixel to something very specific, or adding an error parameter to the pixel that includes something like noAccountManagerTokenFound. Then we can add other error types in the future, if they become relevant.

Agree. I will rename it to dataBrokerProtectionErrorWhenFetchingSubscriptionAuthTokenAfterSignIn

samsymons

LGTM, worked as expected. I added a couple non-blocking comments here and there. Thanks!

# By Dominik Kapusta (41) and others # Via Dominik Kapusta (9) and others * main: (138 commits) Make sure when we set custom config url, we don't expect etag in return (#1994) Add PixelKit source parameter (#1989) Fix internal user toggling (#2000) Show alert and display warning icon in Sync Settings when data syncing is disabled (#1996) DBP: Integrate subscription account authentication to DBP (#1995) Improve bookmarks html reader (#1986) Add Sync feature flags (#1992) Add daily stats pixel (#1993) Do not reload DBP tab when switching to it (#1942) Fix: external application requests via redirect URLs shows wrong origin. (#1900) Update clean-app.sh to work on macOS Sonoma and include NetP containers (#1988) Fix: "SwiftLintPlugin" must be enabled before it can be used (#1987) Prevent VPN server list persistence failures (#1985) add test can remove data (#1980) Remove VPN upgrade card (#1983) Fix low-res VPN warning asset (#1984) DBP: Fix unreliable date tests (#1981) Add search retention pixel for NetP (#1964) Sabrina/sync e2e tests (#1959) swiftlint build plugin (#1318) ... # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/Application/AppDelegate.swift

# By Dominik Kapusta (2) and others # Via GitHub * main: Make sure when we set custom config url, we don't expect etag in return (#1994) Add PixelKit source parameter (#1989) Fix internal user toggling (#2000) Show alert and display warning icon in Sync Settings when data syncing is disabled (#1996) DBP: Integrate subscription account authentication to DBP (#1995) Improve bookmarks html reader (#1986) Add Sync feature flags (#1992) Add daily stats pixel (#1993) Do not reload DBP tab when switching to it (#1942) Fix: external application requests via redirect URLs shows wrong origin. (#1900) # Conflicts: # DuckDuckGo/Statistics/PixelEvent.swift

jotaemepereira added 4 commits December 20, 2023 10:37

Save token when user signs in

53e4afe

Show PIR tab when on subscription target

e0f90c7

Add handler to project

136c77c

Fire pixel if we fail to retrieve subscription token

cda7456

jotaemepereira requested a review from samsymons December 20, 2023 18:30

jotaemepereira commented Dec 20, 2023

View reviewed changes

jotaemepereira changed the title ~~Juan/dbp/subscription auth~~ DBP: Integrate subscription account authentication to DBP Dec 20, 2023

jotaemepereira commented Dec 20, 2023

View reviewed changes

DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift Show resolved Hide resolved

samsymons reviewed Dec 21, 2023

View reviewed changes

samsymons approved these changes Dec 21, 2023

View reviewed changes

Address feedback

49fd1bd

jotaemepereira merged commit 187b319 into main Dec 21, 2023
14 checks passed

jotaemepereira deleted the juan/dbp/subscription-auth branch December 21, 2023 13:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DBP: Integrate subscription account authentication to DBP #1995

DBP: Integrate subscription account authentication to DBP #1995

jotaemepereira commented Dec 20, 2023 •

edited

Loading

jotaemepereira Dec 20, 2023

samsymons Dec 21, 2023

samsymons Dec 21, 2023

samsymons Dec 21, 2023

jotaemepereira Dec 21, 2023

samsymons Dec 21, 2023

jotaemepereira Dec 21, 2023 •

edited

Loading

samsymons left a comment

		case .dataBrokerProtectionSubscriptionErrorWhenFetchingToken:
		return "m_mac_dbp_subscription_error_when_fetching_token"

DBP: Integrate subscription account authentication to DBP #1995

DBP: Integrate subscription account authentication to DBP #1995

Conversation

jotaemepereira commented Dec 20, 2023 • edited Loading

Task

Description

Steps to test

Test everything works as expected

Test the default browser target

jotaemepereira Dec 20, 2023

Choose a reason for hiding this comment

samsymons Dec 21, 2023

Choose a reason for hiding this comment

samsymons Dec 21, 2023

Choose a reason for hiding this comment

samsymons Dec 21, 2023

Choose a reason for hiding this comment

jotaemepereira Dec 21, 2023

Choose a reason for hiding this comment

samsymons Dec 21, 2023

Choose a reason for hiding this comment

jotaemepereira Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

samsymons left a comment

Choose a reason for hiding this comment

jotaemepereira commented Dec 20, 2023 •

edited

Loading

jotaemepereira Dec 21, 2023 •

edited

Loading