Subscription oauth api v2 #3580

federicocappelli · 2024-11-21T15:12:07Z

Task/Issue URL: https://app.asana.com/0/1205842942115003/1207991044706236/f
Tech Design URL: https://app.asana.com/0/1205842942115003/1207991044706232/f
CC: @miasma13

iOS PR: duckduckgo/iOS#3480
BSK PR: duckduckgo/BrowserServicesKit#1033

Description:

This PR introduces the use of OAuth V2 authentication in Privacy Pro Subscription.
The code changes are comprehensive due to the paradigm changes between the old access token lifecycle and the new JWT lifecycle.
The Subscription UI and UX should be unchanged.

Steps to test this PR:

Test all Privacy Pro Subscription features and UX, more details here

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

# Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

github-actions · 2024-11-26T12:30:44Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against f3a5d24

# Conflicts: # DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift

# Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/Application/AppDelegate.swift # DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift # DuckDuckGo/Subscription/SubscriptionManager+StandardConfiguration.swift # DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift # DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewModel.swift # LocalPackages/SubscriptionUI/Sources/SubscriptionUI/DebugMenu/SubscriptionDebugMenu.swift # LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionModel.swift # UnitTests/Freemium/DBP/Experiment/FreemiumDBPPixelExperimentManagingTests.swift # UnitTests/Freemium/DBP/FreemiumDBPFeatureTests.swift # UnitTests/Menus/MoreOptionsMenuTests.swift # UnitTests/Subscription/SubscriptionAppStoreRestorerTests.swift # UnitTests/Subscription/SubscriptionPagesUseSubscriptionFeatureForStripeTests.swift # UnitTests/Subscription/SubscriptionPagesUseSubscriptionFeatureTests.swift # UnitTests/UnifiedFeedbackForm/UnifiedFeedbackFormViewModelTests.swift

diegoreymendez · 2025-01-14T16:29:33Z

...workProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift

@@ -436,16 +417,60 @@ final class MacPacketTunnelProvider: PacketTunnelProvider {
        case .staging:
            subscriptionEnvironment.serviceEnvironment = .staging
        }
+        subscriptionEnvironment.purchasePlatform = .stripe // we don't care about the purchasePlatform


I've no idea why we don't care about the purchase platform. Could we document what this means, maybe providing a link with reference or a clearer explanation?

Comment added: The SysExt doesn't care about the purchase platform because the only operations executed here are about the Auth token. No purchase or platforms-related operations are performed.

diegoreymendez · 2025-01-14T16:31:05Z

...workProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift

+                                                          serviceName: Self.tokenServiceName,
+                                                          keychainType: Bundle.keychainType)
+        let authClient = DefaultOAuthClient(tokensStorage: tokenStorage,
+                                            legacyTokenStorage: nil, // Note: The VPN SysExt will stop at the first transition from auth v1 to v2, is up to the user to re-enable it


Can you explain this to me in a bit more detail? It's not clear to me what the "transition" is... is it the first time they manually start the VPN?

I've confirmed with Thomas that it is actually possible to exchange a token v1 multiple times so this comment is not relevant anymore, I've updated the code so the SysExt can now perform a full migration from auth v1 to v2 and never stop

diegoreymendez

Still looking but added several comments.

THISISDINOSAUR

Testing and PIR seems to work now. I've just left one minor comment on the code

THISISDINOSAUR · 2025-01-14T18:13:27Z

DuckDuckGo/DBP/DataBrokerProtectionAppEvents.swift

@@ -52,6 +53,7 @@ struct DataBrokerProtectionAppEvents {
            // In this case, let's disable the agent and delete any left-over data because there's nothing for it to do
            if let profileQueriesCount = try? DataBrokerProtectionManager.shared.dataManager.profileQueriesCount(),
               profileQueriesCount > 0 {
+                Logger.dataBrokerProtection.log("Found \(profileQueriesCount) profile queries in DB. Disabling agent.")


I think this comment is wrong (confused by the comment above no longer being accurate)
If there are profile queries, we don't disable it, we want the agent to be running. We restart it rather than disable it (to help with the flakeyness of login items)

Thanks, modified with "Restarting agent"

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/Common/Extensions/WKWebViewExtension.swift # LocalPackages/NewTabPage/Sources/NewTabPage/Configuration/NewTabPageConfigurationClient.swift # LocalPackages/NewTabPage/Sources/NewTabPage/Favorites/NewTabPageFavoritesClient.swift

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

github-actions · 2025-01-29T00:16:56Z

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

github-actions · 2025-02-06T00:17:25Z

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

github-actions · 2025-02-14T00:17:11Z

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

github-actions · 2025-02-21T00:17:39Z

This PR has been closed after 14 days of inactivity. Feel free to reopen it if you plan to continue working on it or have further discussions.

federicocappelli added 3 commits November 19, 2024 11:18

Initial auth v2 adoption

88549ff

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

9b2e7ba

# Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

DI done

4fa2b0a

federicocappelli added includes BSK changes DO NOT MERGE labels Nov 21, 2024

token v1/v2 exchange fixed, token providing improved

0d333af

federicocappelli changed the title ~~Fcappelli/subscription oauth api v2~~ Subscription oauth api v2 Nov 22, 2024

federicocappelli added 8 commits November 22, 2024 14:16

new log

b79e8eb

tokencontainer codable

18b8fe6

vpn token propagation

e6cc664

more logs

00c4cbc

keychain type unification

4bcc286

keychain fix

875e880

init improvements

92014b4

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

8dc4078

# Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

federicocappelli added 14 commits November 26, 2024 13:51

codablehelper

09eee50

it works!

ffb9ecd

fixing unit tests

dcccf44

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

6ddece0

# Conflicts: # DuckDuckGo/UnifiedFeedbackForm/UnifiedFeedbackFormViewController.swift

unit tests fixes

62ddb64

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

975d8ac

# Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

unit tests fixed

a121c06

BSK > branch

d3e657c

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

5ccd25b

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

lint

08bf3f0

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

1ef216e

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

unit tests improvments

b5a3f43

varius

6dc2e1f

diegoreymendez reviewed Jan 14, 2025

View reviewed changes

federicocappelli added 3 commits January 14, 2025 16:32

pixels added

d0ca705

cleanup

39b30fc

settings subscription status logic improved

de72420

THISISDINOSAUR reviewed Jan 14, 2025

View reviewed changes

samsymons removed their request for review January 15, 2025 05:36

federicocappelli added 12 commits January 15, 2025 10:43

legacy token support implemented in VPN, migration improved

c29989b

pr comments addressed

ca8f251

bsk updated

f737f2d

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

4d2d595

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift

Subscription stuff renamed

7284019

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

e5ec1bd

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

e98529a

# Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

BSK updated

bbde6d6

Merge branch 'main' into fcappelli/subscription_oauth_api_v2

f11debc

BSK update

f3a5d24

Tunnel controller now refreshes the token after passing it to the VPN

445fe07

github-actions bot added the stale label Jan 29, 2025

federicocappelli removed the stale label Jan 29, 2025

github-actions bot added the stale label Feb 6, 2025

federicocappelli removed the stale label Feb 6, 2025

github-actions bot added the stale label Feb 14, 2025

github-actions bot closed this Feb 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subscription oauth api v2 #3580

Subscription oauth api v2 #3580

federicocappelli commented Nov 21, 2024 •

edited by diegoreymendez

Loading

github-actions bot commented Nov 26, 2024 •

edited

Loading

diegoreymendez Jan 14, 2025

federicocappelli Jan 15, 2025

diegoreymendez Jan 14, 2025

federicocappelli Jan 15, 2025

diegoreymendez left a comment

THISISDINOSAUR left a comment

THISISDINOSAUR Jan 14, 2025

federicocappelli Jan 15, 2025

github-actions bot commented Jan 29, 2025

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 14, 2025

github-actions bot commented Feb 21, 2025

Subscription oauth api v2 #3580

Subscription oauth api v2 #3580

Conversation

federicocappelli commented Nov 21, 2024 • edited by diegoreymendez Loading

Internal references:

github-actions bot commented Nov 26, 2024 • edited Loading

diegoreymendez Jan 14, 2025

Choose a reason for hiding this comment

federicocappelli Jan 15, 2025

Choose a reason for hiding this comment

diegoreymendez Jan 14, 2025

Choose a reason for hiding this comment

federicocappelli Jan 15, 2025

Choose a reason for hiding this comment

diegoreymendez left a comment

Choose a reason for hiding this comment

THISISDINOSAUR left a comment

Choose a reason for hiding this comment

THISISDINOSAUR Jan 14, 2025

Choose a reason for hiding this comment

federicocappelli Jan 15, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 29, 2025

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 14, 2025

github-actions bot commented Feb 21, 2025

federicocappelli commented Nov 21, 2024 •

edited by diegoreymendez

Loading

github-actions bot commented Nov 26, 2024 •

edited

Loading