From 8678217beb5118c6547ce8d19313b1031b9657e7 Mon Sep 17 00:00:00 2001 From: Rebecka Gulliksson Date: Mon, 23 May 2016 15:09:13 +0200 Subject: [PATCH] Be more lenient about refresh tokens. --- Source/OIDAuthState.m | 122 +++++++++++++++++++++------------- Source/OIDError.h | 6 ++ UnitTests/OIDAuthStateTests.m | 64 ++++++++++++++++++ 3 files changed, 145 insertions(+), 47 deletions(-) diff --git a/Source/OIDAuthState.m b/Source/OIDAuthState.m index edf8d1675..ea137fd5e 100644 --- a/Source/OIDAuthState.m +++ b/Source/OIDAuthState.m @@ -398,64 +398,72 @@ - (void)performActionWithFreshTokens:(OIDAuthStateAction)action { - (void)performActionWithFreshTokens:(OIDAuthStateAction)action additionalRefreshParameters: (nullable NSDictionary *)additionalParameters { - if (!_refreshToken) { - [OIDErrorUtilities raiseException:kRefreshTokenRequestException]; - } - - if ([self.accessTokenExpirationDate timeIntervalSinceNow] > kExpiryTimeTolerance - && !_needsTokenRefresh) { + if ([self isTokenFresh]) { // access token is valid within tolerance levels, perform action dispatch_async(dispatch_get_main_queue(), ^() { action(self.accessToken, self.idToken, nil); }); - } else { - // else, first refresh the token, then perform action - _needsTokenRefresh = NO; - NSAssert(_pendingActionsSyncObject, @"_pendingActionsSyncObject cannot be nil"); - @synchronized(_pendingActionsSyncObject) { - // if a token is already in the process of being refreshed, adds to pending actions - if (_pendingActions) { - [_pendingActions addObject:action]; - return; - } + return; + } - // creates a list of pending actions, starting with this one - _pendingActions = [NSMutableArray arrayWithObject:action]; + if (!_refreshToken) { + // no refresh token available and token have expired + NSError *tokenRefreshError = [ + OIDErrorUtilities errorWithCode:OIDErrorCodeTokenRefreshError + underlyingError:nil + description:@"Unable to refresh expired token without a refresh token."]; + dispatch_async(dispatch_get_main_queue(), ^() { + action(nil, nil, tokenRefreshError); + }); + return; + } + + // access token is expired, first refresh the token, then perform action + _needsTokenRefresh = NO; + NSAssert(_pendingActionsSyncObject, @"_pendingActionsSyncObject cannot be nil"); + @synchronized(_pendingActionsSyncObject) { + // if a token is already in the process of being refreshed, adds to pending actions + if (_pendingActions) { + [_pendingActions addObject:action]; + return; } - // refresh the tokens - OIDTokenRequest *tokenRefreshRequest = - [self tokenRefreshRequestWithAdditionalParameters:additionalParameters]; - [OIDAuthorizationService performTokenRequest:tokenRefreshRequest - callback:^(OIDTokenResponse *_Nullable response, - NSError *_Nullable error) { - dispatch_async(dispatch_get_main_queue(), ^() { - // update OIDAuthState based on response - if (response) { - [self updateWithTokenResponse:response error:nil]; + // creates a list of pending actions, starting with this one + _pendingActions = [NSMutableArray arrayWithObject:action]; + } + + // refresh the tokens + OIDTokenRequest *tokenRefreshRequest = + [self tokenRefreshRequestWithAdditionalParameters:additionalParameters]; + [OIDAuthorizationService performTokenRequest:tokenRefreshRequest + callback:^(OIDTokenResponse *_Nullable response, + NSError *_Nullable error) { + dispatch_async(dispatch_get_main_queue(), ^() { + // update OIDAuthState based on response + if (response) { + [self updateWithTokenResponse:response error:nil]; + } else { + if (error.domain == OIDOAuthTokenErrorDomain) { + [self updateWithAuthorizationError:error]; } else { - if (error.domain == OIDOAuthTokenErrorDomain) { - [self updateWithAuthorizationError:error]; - } else { - if ([_errorDelegate respondsToSelector: - @selector(authState:didEncounterTransientError:)]) { - [_errorDelegate authState:self didEncounterTransientError:error]; - } + if ([_errorDelegate respondsToSelector: + @selector(authState:didEncounterTransientError:)]) { + [_errorDelegate authState:self didEncounterTransientError:error]; } } + } - // nil the pending queue and process everything that was queued up - NSArray *actionsToProcess; - @synchronized(_pendingActionsSyncObject) { - actionsToProcess = _pendingActions; - _pendingActions = nil; - } - for (OIDAuthStateAction actionToProcess in actionsToProcess) { - actionToProcess(self.accessToken, self.idToken, error); - } - }); - }]; - } + // nil the pending queue and process everything that was queued up + NSArray *actionsToProcess; + @synchronized(_pendingActionsSyncObject) { + actionsToProcess = _pendingActions; + _pendingActions = nil; + } + for (OIDAuthStateAction actionToProcess in actionsToProcess) { + actionToProcess(self.accessToken, self.idToken, error); + } + }); + }]; } #pragma mark - Deprecated @@ -466,6 +474,26 @@ - (void)withFreshTokensPerformAction:(OIDAuthStateAction)action { #pragma mark - +/*! @fn isTokenFresh + @brief Determines whether a token refresh request must be made to refresh the tokens. + */ +- (BOOL)isTokenFresh { + if (_needsTokenRefresh) { + // forced refresh + return NO; + } + + if (!self.accessTokenExpirationDate) { + // if there is no expiration time but we have an access token, it is assumed + // to never expire + return !!self.accessToken; + } + + // has the token expired + BOOL tokenExpired = [self.accessTokenExpirationDate timeIntervalSinceNow] > kExpiryTimeTolerance; + return tokenExpired; +} + @end diff --git a/Source/OIDError.h b/Source/OIDError.h index ed58f8251..6ed17e6c9 100644 --- a/Source/OIDError.h +++ b/Source/OIDError.h @@ -118,6 +118,12 @@ typedef NS_ENUM(NSInteger, OIDErrorCode) { request in the default browser. */ OIDErrorCodeBrowserOpenError = -10, + + /*! @brief Indicates a problem when trying to refresh the tokens. + */ + OIDErrorCodeTokenRefreshError = -11, + + }; /*! @brief Enum of all possible OAuth error codes as defined by RFC6749 diff --git a/UnitTests/OIDAuthStateTests.m b/UnitTests/OIDAuthStateTests.m index 1c274745c..55ea5b327 100644 --- a/UnitTests/OIDAuthStateTests.m +++ b/UnitTests/OIDAuthStateTests.m @@ -24,6 +24,13 @@ #import "Source/OIDAuthorizationResponse.h" #import "Source/OIDErrorUtilities.h" #import "Source/OIDTokenResponse.h" +#import "OIDTokenRequestTests.h" + +@interface OIDAuthState (Testing) + // expose private method for simple testing +- (BOOL)isTokenFresh; +@end + @interface OIDAuthStateTests () @end @@ -350,5 +357,62 @@ - (void)testSecureCoding { XCTAssertEqual(authStateCopy.authorizationError.code, authState.authorizationError.code); } +- (void)testIsTokenFreshWithFreshToken { + OIDAuthorizationResponse *authorizationResponse = + [OIDAuthorizationResponseTests testInstanceCodeFlow]; + OIDTokenRequest *tokenRequest = [OIDTokenRequestTests testInstance]; + OIDTokenResponse *tokenResponse = + [[OIDTokenResponse alloc] initWithRequest:tokenRequest + parameters:@{ + @"access_token" : @"abc123", + @"expires_in" : @(3600) + }]; + + OIDAuthState *authState = [ + [OIDAuthState alloc] initWithAuthorizationResponse:authorizationResponse + tokenResponse:tokenResponse]; + XCTAssertEqual([authState isTokenFresh], YES); +} + +- (void)testIsTokenFreshWithExpiredToken { + OIDAuthorizationResponse *authorizationResponse = + [OIDAuthorizationResponseTests testInstanceCodeFlow]; + OIDTokenRequest *tokenRequest = [OIDTokenRequestTests testInstance]; + OIDTokenResponse *tokenResponse = + [[OIDTokenResponse alloc] initWithRequest:tokenRequest + parameters:@{ + @"access_token" : @"abc123", + @"expires_in" : @(0) + }]; + + OIDAuthState *authState = [ + [OIDAuthState alloc] initWithAuthorizationResponse:authorizationResponse + tokenResponse:tokenResponse]; + XCTAssertEqual([authState isTokenFresh], NO); +} + + +- (void)testIsTokenFreshRespectsTokenRefreshOverride { + OIDAuthState *authState = [[self class] testInstance]; + [authState setNeedsTokenRefresh]; + XCTAssertEqual([authState isTokenFresh], NO); +} + +- (void)testIsTokenFreshHandlesTokenWithoutExpirationTime { + OIDAuthorizationResponse *authorizationResponse = + [OIDAuthorizationResponseTests testInstanceCodeFlow]; + OIDTokenRequest *tokenRequest = [OIDTokenRequestTests testInstance]; + OIDTokenResponse *tokenResponse = + [[OIDTokenResponse alloc] initWithRequest:tokenRequest + parameters:@{ + @"access_token" : @"abc123", + }]; + + OIDAuthState *authState = [ + [OIDAuthState alloc] initWithAuthorizationResponse:authorizationResponse + tokenResponse:tokenResponse]; + XCTAssertEqual([authState isTokenFresh], YES); +} + @end