First pass at fixing RestClientCallback for getRestClient() #2315
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
gkotula
commented
May 23, 2022
| * Callback from either user account creation, or a call to getAuthToken, used | ||
| * by the Android account management components. | ||
| */ | ||
| private class AccMgrCallback implements AccountManagerCallback<Bundle> { |
Tests results for SalesforceSDKGenerated by 🚫 Danger |
wmathurin
approved these changes
May 23, 2022
|
Closing due to edge case discovered while testing that would leak the account change listener if the LoginActivity is finished without modifying the accounts (e.g. if the user clicks back and the Activity pops from the task stack) |
|
@gkotula I'd prefer to leave the login code as-is for now. There are many different flows that need to be accounted for. |
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.
Opening this for feedback on the solution for this issue. Mainly, I am unsure how this may affect complex login flows including IDP and user switching in all customer use-cases.
The issue: forcedotcom/SalesforceMobileSDK-ReactNative#247
Original PR that introduced the issue: #2203
The core problem is that launching the Login Activity from
Intentdoes not get the callback for when theRestClientis ready, but we cannot useAccountManager.addAccount(...)since users may not have permission to add through the Android framework'sAccountManager.Luckily,
AccountManagerhas an API for listening for changes to the accounts on the device, but care must be taken to properly remove the listener after the callback is invoked so as not to leak Activities.