-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Universal auth v1 and v2 NetworkProtection #1207
Universal auth v1 and v2 NetworkProtection #1207
Conversation
} | ||
} | ||
|
||
private func loadTokenContainer(from options: StartupOptions) async throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is AuthV2 counterpart of loadAuthToken
and it's unused.
# Conflicts: # Sources/Subscription/API/SubscriptionEndpointServiceV2.swift # Sources/Subscription/Flows/AppStore/AppStorePurchaseFlowV2.swift # Sources/Subscription/Managers/SubscriptionManagerV2.swift # Sources/Subscription/Storage/V2/SubscriptionTokenKeychainStorageV2.swift # Sources/Subscription/SubscriptionFeatureV2.swift
@@ -55,7 +55,8 @@ final class NetworkProtectionKeychainStore { | |||
|
|||
// MARK: - Keychain Interaction | |||
|
|||
func readData(named name: String) throws -> Data? { | |||
public func readData(named name: String) throws -> Data? { | |||
Logger.networkProtectionKeyManagement.debug("Reading key \(name, privacy: .public) from keychain") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, the new log entries are handy 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, I added logs that I found useful during my VPN debug sessions
...kProtection/Subscription/NetworkProtectionKeychainTokenStore+SubscriptionTokenHandling.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues with this from my side, worked as expected and the changes look reasonable. Thanks!
Task/Issue URL: https://app.asana.com/0/72649045549333/1209279739041319/f
iOS PR: duckduckgo/iOS#3914
macOS PR: duckduckgo/macos-browser#3817
What kind of version bump will this require?: Major
Description:
This PR introduces a few changes and some unused code in network protection allowing the future adoption of Auth V2.
All code related to Auth V2 is not used; instead, Auth V1 is maintained.
Steps to test this PR:
Internal references:
Software Engineering Expectations
Technical Design Template