Token Storage + UI #298
Merged
Token Storage + UI #298
+1,586
−665
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
mohssenfathi
requested review from
alanpaivaa,
sbondini,
renan-ssoares and
elanuber
alanpaivaa
reviewed
May 3, 2024
examples/UberSDK/UberSDKTests/UberCore/KeychainUtilityTests.swift
Outdated
Show resolved
Hide resolved
alanpaivaa
approved these changes
May 9, 2024
| } | ||
|
|
||
| private func login() { | ||
| let defaultContext = AuthContext( |
There was a problem hiding this comment.
NIT: could be moved to a constant at class level.
| ) | ||
| ) | ||
| let context = dataSource?.authContext(self) ?? defaultContext | ||
| UberAuth.login(context: context) { [weak self] result in |
There was a problem hiding this comment.
NIT: weak self likely not needed, since UberAuth is being referred statically
There was a problem hiding this comment.
Thanks. Will fix in the next PR.
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.
Description
This PR does two main things:
Changes
Supported platforms v13 -> v15
Bumped the minimum supported platform version to 15 to support the UIButton APIs. Platform adoption should be at a high enough level to support this. Also, this is in line with the 1P Uber app's min OS version.
AccessToken
A new AccessToken struct has be created to accompany the new Auth stack. The old one has been marked as DEPRECATED, but the logic should be similar.
Deprecated Classes
Legacy AccessToken and TokenManager have been marked as deprecated and call sites have been updated. There will be a later PR that migrates these call sites to the new classes.
Saving AccessToken
AuthorizationCodeAuthProvider now contains logic to save the AccessToken to the keychain after successfully exchanging authorization code for access token.
TokenManager + KeychainUtility
These are similar to the existing ones, with some improvements to the API. They now accept any objects conforming to the Codable protocol instead of NSCoding. Other logic should remain the same.
UberButton + LoginButton
The existing UberButton and LoginButton classes have been replaced with updated APIs that are now available. The UI and logic remains the same. Some resources including colors and images have been added to the bundle to support the new buttons.
Testing
Unit Tests
Unit tests have been added for new logic around token storage.
Manual Testing
The UberButton/LoginButton has been added to the sample app for manual testing purposes.