Skip to content

Commit bacff6e

Browse files
rosalyntanleotianlizhan
rosalyntan
authored and
leotianlizhan
committed
Partial fix for expired ID token issue (#9253)
* Brainstorm fix for expired ID token issue. * Only retry once, in case of clock skew, and fix unit tests. * Fix FIRAuthTests. * Modify check to be for if the token will expire within 5 minutes. * Update changelog.
1 parent 8e609c8 commit bacff6e

File tree

4 files changed

+153
-35
lines changed

4 files changed

+153
-35
lines changed

‎FirebaseAuth/CHANGELOG.md

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,51 @@
11
# v8.12.0
22
- [added] Added documentation note and error logging to `getStoredUser(forAccessGroup:)` regarding tvOS keychain sharing issues. (#8878)
3+
- [fixed] Partial fix for expired ID token issue. (#6521)
34

45
# v8.11.0
56
- [changed] Added a `X-Firebase-GMPID` header to network requests. (#9046)
67
- [fixed] Added multi-tenancy support to generic OAuth providers. (#7990)
7-
- [fixed] macOS Extension access to Shared Keychain by adding kSecUseDataProtectionKeychain recommended key (#6876)
8+
- [fixed] macOS Extension access to Shared Keychain by adding `kSecUseDataProtectionKeychain` recommended key. (#6876)
89

9-
# 8.9.0
10+
# v8.9.0
1011
- [changed] Improved error logging. (#8704)
1112
- [added] Added MFA support for email link sign-in. (#8705)
1213

13-
# 8.8.0
14+
# v8.8.0
1415
- [fixed] Fall back to reCAPTCHA for phone auth app verification if the push notification is not received before the timeout. (#8653)
1516

16-
# 8.6.0
17+
# v8.6.0
1718
- [fixed] Annotated platform-level availability using `API_UNAVAILABLE` instead of conditionally compiling certain methods with `#if` directives. (#8451)
1819

19-
# 8.5.0
20+
# v8.5.0
2021
- [fixed] Fixed an analyze issue introduced in Xcode 12.5. (#8411)
2122

22-
# 8.2.0
23+
# v8.2.0
2324
- [fixed] Fixed analyze issues introduced in Xcode 12.5. (#8210)
2425
- [fixed] Fixed a bug in the link with email link, Game Center, and phone auth flows. (#8196)
2526

26-
# 8.0.0
27+
# v8.0.0
2728
- [fixed] Fixed a crash that occurred when assigning auth settings. (#7670)
2829

29-
# 7.8.0
30+
# v7.8.0
3031
- [fixed] Fixed auth state sharing during first app launch. (#7472)
3132

32-
# 7.6.0
33+
# v7.6.0
3334
- [fixed] Auth emulator now works across the local network. (#7350)
3435
- [fixed] Fixed incorrect import for watchOS (#7425)
3536

36-
# 7.4.0
37+
# v7.4.0
3738
- [fixed] Check if the reverse client ID is configured as a custom URL scheme before setting it as the callback scheme. (#7211)
3839
- [added] Add ability to sync auth state across devices. (#6924)
3940
- [fixed] Add multi-tenancy support for email link sign-in. (#7246)
4041

41-
# 7.3.0
42+
# v7.3.0
4243
- [fixed] Catalyst browser issue with `verifyPhoneNumber` API. (#7049)
4344

44-
# 7.1.0
45+
# v7.1.0
4546
- [fixed] Fixed completion handler issue in `application(_:didReceiveRemoteNotification:fetchCompletionHandler:)` method. (#6863)
4647

47-
# 7.0.0
48+
# v7.0.0
4849
- [removed] Remove deprecated APIs `dataForKey`,`fetchProvidersForEmail:completion`, `signInAndRetrieveDataWithCredential:completion`, `reauthenticateAndRetrieveDataWithCredential:completion`, `linkAndRetrieveDataWithCredential:completion`. (#6607)
4950
- [added] Add support for the auth emulator. (#6624)
5051
- [changed] The global variables `FirebaseAuthVersionNum` and `FirebaseAuthVersionStr` are deleted.

‎FirebaseAuth/Sources/SystemService/FIRSecureTokenService.m

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#import "FirebaseAuth/Sources/Public/FirebaseAuth/FIRAuth.h"
2020

2121
#import "FirebaseAuth/Sources/Auth/FIRAuthSerialTaskQueue.h"
22+
#import "FirebaseAuth/Sources/Auth/FIRAuthTokenResult_Internal.h"
2223
#import "FirebaseAuth/Sources/Auth/FIRAuth_Internal.h"
2324
#import "FirebaseAuth/Sources/Backend/FIRAuthBackend.h"
2425
#import "FirebaseAuth/Sources/Backend/FIRAuthRequestConfiguration.h"
@@ -100,11 +101,12 @@ - (void)fetchAccessTokenForcingRefresh:(BOOL)forceRefresh
100101
callback(self->_accessToken, nil, NO);
101102
} else {
102103
FIRLogDebug(kFIRLoggerAuth, @"I-AUT000017", @"Fetching new token from backend.");
103-
[self requestAccessToken:^(NSString *_Nullable token, NSError *_Nullable error,
104+
[self requestAccessToken:YES
105+
callback:^(NSString *_Nullable token, NSError *_Nullable error,
104106
BOOL tokenUpdated) {
105-
complete();
106-
callback(token, error, tokenUpdated);
107-
}];
107+
complete();
108+
callback(token, error, tokenUpdated);
109+
}];
108110
}
109111
}];
110112
}
@@ -160,7 +162,7 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
160162
since only one of those tasks is ever running at a time, and those tasks are the only
161163
access to and mutation of these instance variables.
162164
*/
163-
- (void)requestAccessToken:(FIRFetchAccessTokenCallback)callback {
165+
- (void)requestAccessToken:(BOOL)retryIfExpired callback:(FIRFetchAccessTokenCallback)callback {
164166
FIRSecureTokenRequest *request =
165167
[FIRSecureTokenRequest refreshRequestWithRefreshToken:_refreshToken
166168
requestConfiguration:_requestConfiguration];
@@ -170,6 +172,21 @@ - (void)requestAccessToken:(FIRFetchAccessTokenCallback)callback {
170172
BOOL tokenUpdated = NO;
171173
NSString *newAccessToken = response.accessToken;
172174
if (newAccessToken.length && ![newAccessToken isEqualToString:self->_accessToken]) {
175+
FIRAuthTokenResult *tokenResult =
176+
[FIRAuthTokenResult tokenResultWithToken:newAccessToken];
177+
// There is an edge case where the request for a new access token may be made right
178+
// before the app goes inactive, resulting in the callback being invoked much later
179+
// with an expired access token. This does not fully solve the issue, as if the
180+
// callback is invoked less than an hour after the request is made, a token is not
181+
// re-requested here but the approximateExpirationDate will still be off since that is
182+
// computed at the time the token is received.
183+
if (retryIfExpired &&
184+
[tokenResult.expirationDate timeIntervalSinceNow] <= kFiveMinutes) {
185+
// We only retry once, to avoid an infinite loop in the case that an end-user has
186+
// their local time skewed by over an hour.
187+
[self requestAccessToken:NO callback:callback];
188+
return;
189+
}
173190
self->_accessToken = [newAccessToken copy];
174191
self->_accessTokenExpirationDate = response.approximateExpirationDate;
175192
tokenUpdated = YES;

‎FirebaseAuth/Tests/Unit/FIRAuthTests.m

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2180,7 +2180,9 @@ - (void)testAutomaticTokenRefresh {
21802180
// Sign in a user.
21812181
[self waitForSignIn];
21822182

2183-
// Set up expectation for secureToken RPC made by token refresh task.
2183+
// Set up expectation for secureToken RPC made by token refresh task. We call this twice, because
2184+
// we retry once if the access token is already expired.
2185+
[self mockSecureTokenResponseWithError:nil];
21842186
[self mockSecureTokenResponseWithError:nil];
21852187

21862188
// Verify that the current user's access token is the "old" access token before automatic token
@@ -2266,7 +2268,9 @@ - (void)testAutomaticTokenRefreshRetry {
22662268
// token (kNewAccessToken).
22672269
XCTAssertEqualObjects(kAccessToken, [FIRAuth auth].currentUser.rawAccessToken);
22682270

2269-
// Set up expectation for secureToken RPC made by a successful attempt to refresh tokens.
2271+
// Set up expectation for secureToken RPC made by a successful attempt to refresh tokens. We call
2272+
// this twice, because we retry once if the access token is already expired.
2273+
[self mockSecureTokenResponseWithError:nil];
22702274
[self mockSecureTokenResponseWithError:nil];
22712275

22722276
// Execute saved token refresh task.
@@ -2306,7 +2310,9 @@ - (void)testAutoRefreshAppForegroundedNotification {
23062310
// Verify that current user is still valid with old access token.
23072311
XCTAssertEqualObjects(kAccessToken, [FIRAuth auth].currentUser.rawAccessToken);
23082312

2309-
// Set up expectation for secureToken RPC made by a successful attempt to refresh tokens.
2313+
// Set up expectation for secureToken RPC made by a successful attempt to refresh tokens. We call
2314+
// this twice, because we retry once if the access token is already expired.
2315+
[self mockSecureTokenResponseWithError:nil];
23102316
[self mockSecureTokenResponseWithError:nil];
23112317

23122318
// Execute saved token refresh task.

‎FirebaseAuth/Tests/Unit/FIRUserTests.m

Lines changed: 108 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,15 +1353,86 @@ - (void)testGetIDTokenResultSuccess {
13531353
OCMVerifyAll(_mockBackend);
13541354
}
13551355

1356-
/** @fn testGetIDTokenResultForcingRefreshFailure
1356+
/** @fn testGetIDTokenResultForcingRefreshSameAccessTokenSuccess
1357+
@brief Tests the flow of a successful @c getIDTokenResultForcingRefresh:completion: call when
1358+
the returned access token is the same as the stored access token.
1359+
*/
1360+
- (void)testGetIDTokenResultForcingRefreshSameAccessTokenSuccess {
1361+
id mockGetAccountInfoResponseUser = OCMClassMock([FIRGetAccountInfoResponseUser class]);
1362+
OCMStub([mockGetAccountInfoResponseUser localID]).andReturn(kLocalID);
1363+
OCMStub([mockGetAccountInfoResponseUser email]).andReturn(kEmail);
1364+
OCMStub([mockGetAccountInfoResponseUser displayName]).andReturn(kGoogleDisplayName);
1365+
OCMStub([mockGetAccountInfoResponseUser passwordHash]).andReturn(kPasswordHash);
1366+
XCTestExpectation *expectation = [self expectationWithDescription:@"callback"];
1367+
[self
1368+
signInWithEmailPasswordWithMockUserInfoResponse:mockGetAccountInfoResponseUser
1369+
completion:^(FIRUser *user) {
1370+
OCMExpect([self->_mockBackend
1371+
secureToken:[OCMArg any]
1372+
callback:[OCMArg any]])
1373+
.andCallBlock2(^(
1374+
FIRSecureTokenRequest *_Nullable request,
1375+
FIRSecureTokenResponseCallback callback) {
1376+
XCTAssertEqualObjects(request.APIKey, kAPIKey);
1377+
1378+
dispatch_async(FIRAuthGlobalWorkQueue(), ^() {
1379+
id mockSecureTokenResponse = OCMClassMock(
1380+
[FIRSecureTokenResponse class]);
1381+
OCMStub([mockSecureTokenResponse accessToken])
1382+
.andReturn(kAccessToken);
1383+
callback(mockSecureTokenResponse, nil);
1384+
});
1385+
});
1386+
[user
1387+
getIDTokenResultForcingRefresh:YES
1388+
completion:^(
1389+
FIRAuthTokenResult
1390+
*_Nullable tokenResult,
1391+
NSError *_Nullable error) {
1392+
XCTAssertTrue(
1393+
[NSThread isMainThread]);
1394+
XCTAssertNil(error);
1395+
XCTAssertEqualObjects(
1396+
tokenResult.token,
1397+
kAccessToken);
1398+
XCTAssertTrue(
1399+
tokenResult
1400+
.issuedAtDate &&
1401+
[tokenResult.issuedAtDate
1402+
isKindOfClass:
1403+
[NSDate class]]);
1404+
XCTAssertTrue(
1405+
tokenResult.authDate &&
1406+
[tokenResult.authDate
1407+
isKindOfClass:
1408+
[NSDate class]]);
1409+
XCTAssertTrue(
1410+
tokenResult
1411+
.expirationDate &&
1412+
[tokenResult
1413+
.expirationDate
1414+
isKindOfClass:
1415+
[NSDate class]]);
1416+
XCTAssertTrue(
1417+
tokenResult.claims &&
1418+
[tokenResult.claims
1419+
isKindOfClass:
1420+
[NSDictionary
1421+
class]]);
1422+
[expectation fulfill];
1423+
}];
1424+
}];
1425+
[self waitForExpectationsWithTimeout:kExpectationTimeout handler:nil];
1426+
OCMVerifyAll(_mockBackend);
1427+
}
1428+
1429+
/** @fn testGetIDTokenResultForcingRefreshSuccess
13571430
@brief Tests the flow successful @c getIDTokenResultForcingRefresh:completion: calls.
13581431
*/
13591432
- (void)testGetIDTokenResultForcingRefreshSuccess {
1360-
[self getIDTokenResultForcingRefreshSuccessWithIDToken:kAccessToken];
13611433
[self getIDTokenResultForcingRefreshSuccessWithIDToken:kAccessTokenLength415];
13621434
[self getIDTokenResultForcingRefreshSuccessWithIDToken:kAccessTokenLength416];
13631435
[self getIDTokenResultForcingRefreshSuccessWithIDToken:kAccessTokenLength523];
1364-
[self getIDTokenResultForcingRefreshSuccessWithIDToken:kAccessTokenWithBase64URLCharacter];
13651436
}
13661437

13671438
/** @fn testGetIDTokenResultSuccessWithBase64EncodedURL
@@ -1378,6 +1449,22 @@ - (void)testGetIDTokenResultSuccessWithBase64EncodedURL {
13781449
[self
13791450
signInWithEmailPasswordWithMockUserInfoResponse:mockGetAccountInfoResponseUser
13801451
completion:^(FIRUser *user) {
1452+
id mockSecureTokenResponse =
1453+
OCMClassMock([FIRSecureTokenResponse class]);
1454+
OCMStub([mockSecureTokenResponse accessToken])
1455+
.andReturn(kAccessTokenWithBase64URLCharacter);
1456+
OCMExpect([self->_mockBackend
1457+
secureToken:[OCMArg any]
1458+
callback:[OCMArg any]])
1459+
.andCallBlock2(^(
1460+
FIRSecureTokenRequest *_Nullable request,
1461+
FIRSecureTokenResponseCallback callback) {
1462+
XCTAssertEqualObjects(request.APIKey, kAPIKey);
1463+
1464+
dispatch_async(FIRAuthGlobalWorkQueue(), ^() {
1465+
callback(mockSecureTokenResponse, nil);
1466+
});
1467+
});
13811468
OCMExpect([self->_mockBackend
13821469
secureToken:[OCMArg any]
13831470
callback:[OCMArg any]])
@@ -1387,11 +1474,6 @@ - (void)testGetIDTokenResultSuccessWithBase64EncodedURL {
13871474
XCTAssertEqualObjects(request.APIKey, kAPIKey);
13881475

13891476
dispatch_async(FIRAuthGlobalWorkQueue(), ^() {
1390-
id mockSecureTokenResponse = OCMClassMock(
1391-
[FIRSecureTokenResponse class]);
1392-
OCMStub([mockSecureTokenResponse accessToken])
1393-
.andReturn(
1394-
kAccessTokenWithBase64URLCharacter);
13951477
callback(mockSecureTokenResponse, nil);
13961478
});
13971479
});
@@ -2790,8 +2872,8 @@ - (void)testlinkPhoneCredentialAlreadyExistsError {
27902872
#pragma mark - Helpers
27912873

27922874
/** @fn getIDTokenResultForcingRefreshSuccess
2793-
@brief Helper for testing the flow of a successful @c
2794-
getIDTokenResultForcingRefresh:completion: call.
2875+
@brief Helper for testing the flow of a successful
2876+
@c getIDTokenResultForcingRefresh:completion: call.
27952877
*/
27962878
- (void)getIDTokenResultForcingRefreshSuccessWithIDToken:(NSString *)IDToken {
27972879
id mockGetAccountInfoResponseUser = OCMClassMock([FIRGetAccountInfoResponseUser class]);
@@ -2803,6 +2885,22 @@ - (void)getIDTokenResultForcingRefreshSuccessWithIDToken:(NSString *)IDToken {
28032885
[self
28042886
signInWithEmailPasswordWithMockUserInfoResponse:mockGetAccountInfoResponseUser
28052887
completion:^(FIRUser *user) {
2888+
id mockSecureTokenResponse =
2889+
OCMClassMock([FIRSecureTokenResponse class]);
2890+
OCMStub([mockSecureTokenResponse accessToken])
2891+
.andReturn(IDToken);
2892+
OCMExpect([self->_mockBackend
2893+
secureToken:[OCMArg any]
2894+
callback:[OCMArg any]])
2895+
.andCallBlock2(^(
2896+
FIRSecureTokenRequest *_Nullable request,
2897+
FIRSecureTokenResponseCallback callback) {
2898+
XCTAssertEqualObjects(request.APIKey, kAPIKey);
2899+
2900+
dispatch_async(FIRAuthGlobalWorkQueue(), ^() {
2901+
callback(mockSecureTokenResponse, nil);
2902+
});
2903+
});
28062904
OCMExpect([self->_mockBackend
28072905
secureToken:[OCMArg any]
28082906
callback:[OCMArg any]])
@@ -2812,10 +2910,6 @@ - (void)getIDTokenResultForcingRefreshSuccessWithIDToken:(NSString *)IDToken {
28122910
XCTAssertEqualObjects(request.APIKey, kAPIKey);
28132911

28142912
dispatch_async(FIRAuthGlobalWorkQueue(), ^() {
2815-
id mockSecureTokenResponse = OCMClassMock(
2816-
[FIRSecureTokenResponse class]);
2817-
OCMStub([mockSecureTokenResponse accessToken])
2818-
.andReturn(IDToken);
28192913
callback(mockSecureTokenResponse, nil);
28202914
});
28212915
});

0 commit comments

Comments
 (0)