Fix use of non-current account in client manager and possible logout of current user. #2766
Conversation
| // produced may help developers better understand the state of their app. | ||
| SalesforceSDKManager.getInstance() | ||
| .logout(null, null, false, OAuth2.LogoutReason.REFRESH_TOKEN_EXPIRED); | ||
| .logout(matchingAccount, null, showLoginPage, OAuth2.LogoutReason.REFRESH_TOKEN_EXPIRED); |
There was a problem hiding this comment.
logout will default to the current user if null is passed 😬. That is intended in most cases, but really bad in this one.
| // Find the account for this client. | ||
| for (Account account : accounts) { | ||
| UserAccount user = userAccountManager.buildUserAccount(account); | ||
| if (user != null && lastNewAuthToken.equals(user.getAuthToken())) { | ||
| matchingAccount = account; | ||
| } | ||
| } |
There was a problem hiding this comment.
I wish there was a better way to do this. Perhaps we should associate each ClientManager to an account?
There was a problem hiding this comment.
But don't we have an account associated already with ClientManager: final Account acc = clientManager.getAccount();
There was a problem hiding this comment.
That returns the current account, which is (part of) the problem. If the user we are trying to get a new token for not the current user we will invalidate and refresh for the wrong account.
There was a problem hiding this comment.
Tying the ClientManager to account would make sense, but it's quite a change in behavior so probably not a 13.1 change. Maybe we could add a convenience method on UserAccountManager to find a matching account by auth token?
|
@brandonpage do you know how it was working in 12.1.1? |
It "worked" in
In this PR I am passing the correct account to |
2 participants
This bugfix is not a simple revert of a previous change since I believe the code changed in this PR has been working incorrectly for some time by using the wrong accounts token. We simply did not notice until the logout issue was added on top.