Skip to content

Commit 67c257d

Browse files
brandonpagebrandonpage
authored
Merge pull request #2774 from brandonpage/client-manager-logout
Improve getNewAuthToken.
2 parents 23874ff + 0dd8d02 commit 67c257d

File tree

2 files changed

+99
-56
lines changed

2 files changed

+99
-56
lines changed

‎libs/SalesforceSDK/src/com/salesforce/androidsdk/rest/ClientManager.java‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -379,22 +379,6 @@ public AccMgrAuthTokenProvider(ClientManager clientManager, String instanceUrl,
379379
@Override
380380
public String getNewAuthToken() {
381381
SalesforceSDKLogger.i(TAG, "Need new access token");
382-
UserAccountManager userAccountManager = SalesforceSDKManager.getInstance().getUserAccountManager();
383-
Account[] accounts = clientManager.getAccounts();
384-
Account matchingAccount = null;
385-
386-
// Find the account for this client.
387-
for (Account account : accounts) {
388-
UserAccount user = userAccountManager.buildUserAccount(account);
389-
if (user != null && lastNewAuthToken.equals(user.getAuthToken())) {
390-
matchingAccount = account;
391-
}
392-
}
393-
394-
// Fail early to ensure we don't logout the current user below by sending null.
395-
if (matchingAccount == null) {
396-
return null;
397-
}
398382

399383
// Wait if another thread is already fetching an access token
400384
synchronized (lock) {
@@ -408,10 +392,31 @@ public String getNewAuthToken() {
408392
}
409393
gettingAuthToken = true;
410394
}
395+
396+
// Only check for matching account inside synchronized thread that
397+
// is actually getting the new auth token.
398+
UserAccountManager userAccountManager = SalesforceSDKManager.getInstance().getUserAccountManager();
399+
Account[] accounts = clientManager.getAccounts();
400+
Account matchingAccount = null;
411401
String newAuthToken = null;
412402
String newInstanceUrl = null;
413-
try {
414403

404+
if (refreshToken != null) {
405+
for (Account account : accounts) {
406+
UserAccount user = userAccountManager.buildUserAccount(account);
407+
if (user != null && refreshToken.equals(user.getRefreshToken())) {
408+
matchingAccount = account;
409+
break;
410+
}
411+
}
412+
}
413+
414+
// Fail early to ensure we don't logout the current user below by sending null.
415+
if (matchingAccount == null) {
416+
return null;
417+
}
418+
419+
try {
415420
// Invalidate current auth token.
416421
clientManager.invalidateToken(lastNewAuthToken);
417422
final UserAccount userAccount = refreshStaleToken(matchingAccount);

‎libs/test/SalesforceSDKTest/src/com/salesforce/androidsdk/rest/ClientManagerMockTest.kt‎

Lines changed: 77 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ import org.junit.Assert
3232
import org.junit.Before
3333
import org.junit.Test
3434

35-
private const val OLD_TOKEN = "old-token"
36-
private const val REFRESHED_TOKEN = "refreshed-auth-token"
35+
private const val OLD_ACCESS_TOKEN = "old-token"
36+
private const val REFRESHED_ACCESS_TOKEN = "refreshed-auth-token"
37+
private const val REFRESH_TOKEN = "refresh-token"
3738

3839
@SmallTest
3940
class ClientManagerMockTest {
@@ -75,7 +76,7 @@ class ClientManagerMockTest {
7576

7677
val responseBody = """
7778
{
78-
"access_token": $REFRESHED_TOKEN,
79+
"access_token": $REFRESHED_ACCESS_TOKEN,
7980
"instance_url": "https://login.salesforce.com",
8081
"id": "https://login.salesforce.com/id/orgId/userId",
8182
"token_type": "Bearer",
@@ -108,7 +109,8 @@ class ClientManagerMockTest {
108109
val broadcastIntentSlot = slot<Intent>()
109110
val mockAccount = mockk<Account>(relaxed = true)
110111
val mockUser = mockk<UserAccount>(relaxed = true) {
111-
every { authToken } returns OLD_TOKEN
112+
every { authToken } returns OLD_ACCESS_TOKEN
113+
every { refreshToken } returns REFRESH_TOKEN
112114
every { loginServer } returns "https://login.salesforce.com"
113115
}
114116
val mockClientManager = mockk<ClientManager>(relaxed = true) {
@@ -120,22 +122,22 @@ class ClientManagerMockTest {
120122
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
121123
mockClientManager,
122124
"https://login.salesforce.com",
123-
OLD_TOKEN,
124-
"",
125+
OLD_ACCESS_TOKEN,
126+
REFRESH_TOKEN,
125127
)
126128

127129
val result = authTokenProvider.getNewAuthToken()
128-
Assert.assertEquals(REFRESHED_TOKEN, result)
130+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, result)
129131

130132
verify(exactly = 0) {
131133
mockSDKManager.logout(any(), any(), any(), any())
132134
}
133135
verify(exactly = 1) {
134-
mockClientManager.invalidateToken(OLD_TOKEN)
136+
mockClientManager.invalidateToken(OLD_ACCESS_TOKEN)
135137
mockUserAccountManager.updateAccount(mockAccount, capture(userSlot))
136138
mockAppContext.sendBroadcast(capture(broadcastIntentSlot))
137139
}
138-
Assert.assertEquals(REFRESHED_TOKEN, userSlot.captured.authToken)
140+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, userSlot.captured.authToken)
139141
Assert.assertEquals(ClientManager.ACCESS_TOKEN_REFRESH_INTENT, broadcastIntentSlot.captured.action)
140142
}
141143

@@ -145,7 +147,8 @@ class ClientManagerMockTest {
145147
val broadcastIntentSlot = slot<Intent>()
146148
val mockAccount = mockk<Account>(relaxed = true)
147149
val mockUser = mockk<UserAccount>(relaxed = true) {
148-
every { authToken } returns OLD_TOKEN
150+
every { authToken } returns OLD_ACCESS_TOKEN
151+
every { refreshToken } returns REFRESH_TOKEN
149152
every { loginServer } returns "https://login.salesforce.com"
150153
}
151154
val mockClientManager = mockk<ClientManager>(relaxed = true) {
@@ -157,22 +160,22 @@ class ClientManagerMockTest {
157160
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
158161
mockClientManager,
159162
"https://not.login.salesforce.com",
160-
OLD_TOKEN,
161-
"",
163+
OLD_ACCESS_TOKEN,
164+
REFRESH_TOKEN,
162165
)
163166

164167
val result = authTokenProvider.getNewAuthToken()
165-
Assert.assertEquals(REFRESHED_TOKEN, result)
168+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, result)
166169

167170
verify(exactly = 0) {
168171
mockSDKManager.logout(any(), any(), any(), any())
169172
}
170173
verify(exactly = 1) {
171-
mockClientManager.invalidateToken(OLD_TOKEN)
174+
mockClientManager.invalidateToken(OLD_ACCESS_TOKEN)
172175
mockUserAccountManager.updateAccount(mockAccount, capture(userSlot))
173176
mockAppContext.sendBroadcast(capture(broadcastIntentSlot))
174177
}
175-
Assert.assertEquals(REFRESHED_TOKEN, userSlot.captured.authToken)
178+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, userSlot.captured.authToken)
176179
Assert.assertEquals(ClientManager.INSTANCE_URL_UPDATE_INTENT, broadcastIntentSlot.captured.action)
177180
}
178181

@@ -184,8 +187,8 @@ class ClientManagerMockTest {
184187
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
185188
mockClientManager,
186189
"",
187-
OLD_TOKEN,
188-
"",
190+
OLD_ACCESS_TOKEN,
191+
REFRESH_TOKEN,
189192
)
190193

191194
Assert.assertNull(authTokenProvider.getNewAuthToken())
@@ -201,17 +204,45 @@ class ClientManagerMockTest {
201204
val mockAccount = mockk<Account>(relaxed = true)
202205
val mockUser = mockk<UserAccount>(relaxed = true) {
203206
every { authToken } returns "not-matching"
207+
every { refreshToken } returns "not-matching"
204208
}
205209
val mockClientManager = mockk<ClientManager>(relaxed = true) {
206-
every { accounts } returns emptyArray<Account>()
210+
every { accounts } returns arrayOf(mockAccount)
207211
}
208212
every { mockUserAccountManager.currentUser } returns mockUser
209213
every { mockUserAccountManager.buildUserAccount(mockAccount) } returns mockUser
210214
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
211215
mockClientManager,
212216
"",
213-
OLD_TOKEN,
217+
OLD_ACCESS_TOKEN,
218+
REFRESH_TOKEN,
219+
)
220+
221+
Assert.assertNull(authTokenProvider.getNewAuthToken())
222+
verify(exactly = 0) {
223+
mockSDKManager.logout(any(), any(), any(), any())
224+
mockClientManager.invalidateToken(any())
225+
mockAppContext.sendBroadcast(any())
226+
}
227+
}
228+
229+
@Test
230+
fun testGetNewAuthToken_NullAuthToken() {
231+
val mockAccount = mockk<Account>(relaxed = true)
232+
val mockUser = mockk<UserAccount>(relaxed = true) {
233+
every { authToken } returns "not-matching"
234+
every { refreshToken } returns "not-matching"
235+
}
236+
val mockClientManager = mockk<ClientManager>(relaxed = true) {
237+
every { accounts } returns arrayOf(mockAccount)
238+
}
239+
every { mockUserAccountManager.currentUser } returns mockUser
240+
every { mockUserAccountManager.buildUserAccount(mockAccount) } returns mockUser
241+
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
242+
mockClientManager,
214243
"",
244+
null,
245+
REFRESH_TOKEN,
215246
)
216247

217248
Assert.assertNull(authTokenProvider.getNewAuthToken())
@@ -229,11 +260,13 @@ class ClientManagerMockTest {
229260
val mockAccount = mockk<Account>(relaxed = true)
230261
val mockAccount2 = mockk<Account>(relaxed = true)
231262
val mockUser = mockk<UserAccount>(relaxed = true) {
232-
every { authToken } returns OLD_TOKEN
263+
every { authToken } returns OLD_ACCESS_TOKEN
264+
every { refreshToken } returns REFRESH_TOKEN
233265
every { loginServer } returns "https://login.salesforce.com"
234266
}
235267
val mockUser2 = mockk<UserAccount>(relaxed = true) {
236268
every { authToken } returns user2Token
269+
every { refreshToken } returns "user2Refresh"
237270
every { loginServer } returns "https://login.salesforce.com"
238271
}
239272
val mockClientManager = mockk<ClientManager>(relaxed = true) {
@@ -247,21 +280,21 @@ class ClientManagerMockTest {
247280
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
248281
mockClientManager,
249282
"https://login.salesforce.com",
250-
OLD_TOKEN,
251-
"",
283+
OLD_ACCESS_TOKEN,
284+
REFRESH_TOKEN,
252285
)
253286

254-
Assert.assertEquals(REFRESHED_TOKEN, authTokenProvider.getNewAuthToken())
287+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, authTokenProvider.getNewAuthToken())
255288
verify(exactly = 0) {
256289
mockClientManager.invalidateToken(user2Token)
257290
mockSDKManager.logout(any(), any(), any(), any())
258291
mockUserAccountManager.updateAccount(mockAccount2, any())
259292
}
260293
verify(exactly = 1) {
261-
mockClientManager.invalidateToken(OLD_TOKEN)
294+
mockClientManager.invalidateToken(OLD_ACCESS_TOKEN)
262295
mockUserAccountManager.updateAccount(mockAccount, capture(userSlot))
263296
}
264-
Assert.assertEquals(REFRESHED_TOKEN, userSlot.captured.authToken)
297+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, userSlot.captured.authToken)
265298
}
266299

267300
@Test
@@ -278,7 +311,8 @@ class ClientManagerMockTest {
278311
val broadcastIntentSlot = slot<Intent>()
279312
val mockAccount = mockk<Account>(relaxed = true)
280313
val mockUser = mockk<UserAccount>(relaxed = true) {
281-
every { authToken } returns OLD_TOKEN
314+
every { authToken } returns OLD_ACCESS_TOKEN
315+
every { refreshToken } returns REFRESH_TOKEN
282316
every { loginServer } returns "https://login.salesforce.com"
283317
}
284318

@@ -291,16 +325,16 @@ class ClientManagerMockTest {
291325
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
292326
clientManagerSpy,
293327
"https://login.salesforce.com",
294-
OLD_TOKEN,
295-
"",
328+
OLD_ACCESS_TOKEN,
329+
REFRESH_TOKEN,
296330
)
297331

298332
Assert.assertNull(authTokenProvider.getNewAuthToken())
299333
verify(exactly = 0) {
300334
mockUserAccountManager.updateAccount(any(), any())
301335
}
302336
verify(exactly = 1) {
303-
clientManagerSpy.invalidateToken(OLD_TOKEN)
337+
clientManagerSpy.invalidateToken(OLD_ACCESS_TOKEN)
304338
mockSDKManager.logout(capture(accountSlot), any(), any(), capture(reasonSlot))
305339
mockAppContext.sendBroadcast(capture(broadcastIntentSlot))
306340
}
@@ -321,11 +355,13 @@ class ClientManagerMockTest {
321355
val mockAccount = mockk<Account>(relaxed = true)
322356
val mockAccount2 = mockk<Account>(relaxed = true)
323357
val mockUser = mockk<UserAccount>(relaxed = true) {
324-
every { authToken } returns OLD_TOKEN
358+
every { authToken } returns OLD_ACCESS_TOKEN
359+
every { refreshToken } returns REFRESH_TOKEN
325360
every { loginServer } returns "https://login.salesforce.com"
326361
}
327362
val mockUser2 = mockk<UserAccount>(relaxed = true) {
328363
every { authToken } returns user2Token
364+
every { refreshToken } returns "user2Refresh"
329365
every { loginServer } returns "https://login.salesforce.com"
330366
}
331367
val mockClientManager = mockk<ClientManager>(relaxed = true) {
@@ -341,21 +377,21 @@ class ClientManagerMockTest {
341377
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
342378
mockClientManager,
343379
"https://login.salesforce.com",
344-
OLD_TOKEN,
345-
"",
380+
OLD_ACCESS_TOKEN,
381+
REFRESH_TOKEN,
346382
)
347383

348-
Assert.assertEquals(REFRESHED_TOKEN, authTokenProvider.getNewAuthToken())
384+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, authTokenProvider.getNewAuthToken())
349385
verify(exactly = 0) {
350386
mockClientManager.invalidateToken(user2Token)
351387
mockSDKManager.logout(any(), any(), any(), any())
352388
mockUserAccountManager.updateAccount(mockAccount2, any())
353389
}
354390
verify(exactly = 1) {
355-
mockClientManager.invalidateToken(OLD_TOKEN)
391+
mockClientManager.invalidateToken(OLD_ACCESS_TOKEN)
356392
mockUserAccountManager.updateAccount(mockAccount, capture(userSlot))
357393
}
358-
Assert.assertEquals(REFRESHED_TOKEN, userSlot.captured.authToken)
394+
Assert.assertEquals(REFRESHED_ACCESS_TOKEN, userSlot.captured.authToken)
359395
}
360396

361397
/*
@@ -379,11 +415,13 @@ class ClientManagerMockTest {
379415
val mockAccount = mockk<Account>(relaxed = true)
380416
val mockAccount2 = mockk<Account>(relaxed = true)
381417
val mockUser = mockk<UserAccount>(relaxed = true) {
382-
every { authToken } returns OLD_TOKEN
418+
every { authToken } returns OLD_ACCESS_TOKEN
419+
every { refreshToken } returns REFRESH_TOKEN
383420
every { loginServer } returns "https://login.salesforce.com"
384421
}
385422
val mockUser2 = mockk<UserAccount>(relaxed = true) {
386423
every { authToken } returns user2Token
424+
every { refreshToken } returns "user2Refresh"
387425
every { loginServer } returns "https://login.salesforce.com"
388426
}
389427
val mockClientManager = mockk<ClientManager>(relaxed = true) {
@@ -402,8 +440,8 @@ class ClientManagerMockTest {
402440
val authTokenProvider = ClientManager.AccMgrAuthTokenProvider(
403441
clientManagerSpy,
404442
"https://login.salesforce.com",
405-
OLD_TOKEN,
406-
"",
443+
OLD_ACCESS_TOKEN,
444+
REFRESH_TOKEN,
407445
)
408446

409447
Assert.assertNull(authTokenProvider.getNewAuthToken())
@@ -416,7 +454,7 @@ class ClientManagerMockTest {
416454
}
417455

418456
verify(exactly = 1) {
419-
clientManagerSpy.invalidateToken(OLD_TOKEN)
457+
clientManagerSpy.invalidateToken(OLD_ACCESS_TOKEN)
420458
mockSDKManager.logout(capture(accountSlot), any(), any(), capture(reasonSlot))
421459
mockAppContext.sendBroadcast(capture(broadcastIntentSlot))
422460
}

0 commit comments

Comments
 (0)