Subscription keychain sharing for access token #690
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
miasma13
reviewed
Mar 1, 2024
| // This migration is to prevent breaking internal tests and should be removed before launch | ||
| do { | ||
| if let newAccessToken = try accessTokenStorage.getAccessToken() { | ||
| throw MigrationError.noMigrationNeeded |
There was a problem hiding this comment.
Not breaking functionality but this throw is caught down below and turned into throw MigrationError.migrationFailed
quanganhdo
force-pushed
the
graeme/subscription-keychain-sharing
branch
from
cb11e54 to
1eb0850
Compare
|
Rebased on main for latest changes. |
graeme
changed the title
miasma13
reviewed
Mar 6, 2024
| */ | ||
| enum AccountKeychainField: String, CaseIterable { | ||
| case accessToken = "subscription.account.accessToken" | ||
| case testString = "subscription.account.testString" |
There was a problem hiding this comment.
This is no longer needed ;)
miasma13
approved these changes
Mar 6, 2024
miasma13
reviewed
Mar 6, 2024
| @@ -34,15 +34,15 @@ public final class AppStoreRestoreFlow { | |||
| case subscriptionExpired(accountDetails: RestoredAccountDetails) | |||
| } | |||
|
|
|||
| public static func restoreAccountFromPastPurchase() async -> Result<Void, AppStoreRestoreFlow.Error> { | |||
| public static func restoreAccountFromPastPurchase(appGroup: String) async -> Result<Void, AppStoreRestoreFlow.Error> { | |||
There was a problem hiding this comment.
@quanganhdo Spotted this discrepancy, the param probably should be subscriptionAppGroup for consistency.
graeme
added a commit
to duckduckgo/macos-browser
that referenced
this pull request
Mar 6, 2024
Task/Issue URL: https://app.asana.com/0/414235014887631/1206564116141608/f iOS PR: duckduckgo/iOS#2538 BSK PR: duckduckgo/BrowserServicesKit#690 **Optional**: Tech Design URL: https://app.asana.com/0/481882893211075/1206591576458107/f CC: @quanganhdo @diegoreymendez **Description**: For the macOS: Recheck entitlements and display error messaging for cancelled subscriptions we need to make sure the VPN is no longer usable once their entitlement to use it are withdrawn. In [iOS: Integrate with Subscription](https://app.asana.com/0/0/1204886111391114), we’re already checking the entitlements intermittently and on rekey, disabling the VPN*. To authenticate the VPN's register requests (to receive a server + keypair for the tunnel configuration), the PacketTunnelProvider (central component of the VPN extension) needs an access token which is to be provided by the Subscription library built in O-1. This can be pulled accessed directly from the library and, in the iOS and macOS AppStore case, shared between processes (the app, the VPN agent, the VPN extension) using a keychain group. We would like the fetching and lifecycle to be managed exclusively by the Subscription library. We will need to share / delete the subscription access token between processes, so this task covers: - Adding a new subscriptions app group - Injecting this app group into the subscriptions library to be used to create keychain entries. **Steps to test this PR**: 1. Make sure subscriptions flow still works (don’t attempt to connect VPN yet) 2. (Remaining steps in DEBUG only) Open the console and start logging 3. Connect to Network Protection. 4. After the macOS menu item has appeared, check the console and ensure there is a log saying "VPN Agent found token" --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
samsymons
added a commit
that referenced
this pull request
Mar 6, 2024
* main: Subscription keychain sharing for access token (#690)
samsymons
added a commit
that referenced
this pull request
Mar 8, 2024
* main: Changes to 114.1.0-1 hotfix branch (#706) Navigation: Improve same-document navigation handling (#702) Add pixels for our main VPN funnels (#698) Remove Autoconsent subfeature (#697) #URL macro (#657) Handle expired entitlement in NetP (#692) Remove isSubscriptionEnabled check when attempting to delete NetP token (#701) Remove CGNAT range. (#691) Subscription keychain sharing for access token (#690) Integrate confirm entitlements endpoint (#700) fix bundle name in breakByRaisingSigInt (#699) Adds support for deleting all logins (#672) Bump C-S-S to 5.2.0 (#695) Update xcode-version to 15.2. (#688) Bump Tests/BrowserServicesKitTests/Resources/privacy-reference-tests (#687)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/414235014887631/1206564116141608/f
iOS PR: duckduckgo/iOS#2538
macOS PR: duckduckgo/macos-browser#2292
What kind of version bump will this require?: Major
Optional:
Tech Design URL: https://app.asana.com/0/481882893211075/1206591576458107/f
CC: @quanganhdo @diegoreymendez
Description:
For the macOS: Recheck entitlements and display error messaging for cancelled subscriptions we need to make sure the VPN is no longer usable once their entitlement to use it are withdrawn. In iOS: Integrate with Subscription, we’re already checking the entitlements intermittently and on rekey, disabling the VPN*.
To authenticate the VPN's register requests (to receive a server + keypair for the tunnel configuration), the PacketTunnelProvider (central component of the VPN extension) needs an access token which is to be provided by the Subscription library built in O-1. This can be pulled accessed directly from the library and, in the iOS and macOS AppStore case, shared between processes (the app, the VPN agent, the VPN extension) using a keychain group. We would like the fetching and lifecycle to be managed exclusively by the Subscription library.
We will need to share / delete the subscription access token between processes, so this task covers:
Steps to test this PR:
Generally make sure subscriptions flow still works. But to test with migration of token for existing internal users:
OS Testing:
—
Internal references:
Software Engineering Expectations
Technical Design Template