From 03563f987a34cb7f53c9ea7a592cd439670e6c34 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Wed, 19 Oct 2016 17:56:43 +0100 Subject: [PATCH 1/6] Remove AuthOptions.force --- Source/ARTAuth.m | 17 +++-------------- Source/ARTAuthOptions.h | 7 ------- Source/ARTAuthOptions.m | 18 +----------------- Source/ARTRest.m | 18 +++--------------- Source/ARTTypes.h | 3 +-- Source/ARTWebSocketTransport.m | 1 - Spec/Auth.swift | 4 ---- 7 files changed, 8 insertions(+), 60 deletions(-) diff --git a/Source/ARTAuth.m b/Source/ARTAuth.m index c65ec57a8..869122ed3 100644 --- a/Source/ARTAuth.m +++ b/Source/ARTAuth.m @@ -129,7 +129,6 @@ - (void)storeOptions:(ARTAuthOptions *)customOptions { self.options.authParams = [customOptions.authParams copy]; self.options.useTokenAuth = customOptions.useTokenAuth; self.options.queryTime = false; - self.options.force = false; } - (ARTTokenParams *)mergeParams:(ARTTokenParams *)customParams { @@ -317,21 +316,14 @@ - (void)authorise:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOp - (void)authorize:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOptions callback:(void (^)(ARTTokenDetails *, NSError *))callback { BOOL requestNewToken = NO; - ARTAuthOptions *replacedOptions; - if ([authOptions isOnlyForceTrue]) { - replacedOptions = [self.options copy]; - replacedOptions.force = YES; - } - else { - replacedOptions = [authOptions copy] ? : [self.options copy]; - } + ARTAuthOptions *replacedOptions = [authOptions copy] ? : [self.options copy]; [self storeOptions:replacedOptions]; ARTTokenParams *currentTokenParams = [self mergeParams:tokenParams]; [self storeParams:currentTokenParams]; // Reuse or not reuse the current token - if (replacedOptions.force == NO && self.tokenDetails) { + if (self.tokenDetails) { if (self.tokenDetails.expires == nil) { [self.logger verbose:@"RS:%p ARTAuth: reuse current token.", _rest]; requestNewToken = NO; @@ -346,10 +338,7 @@ - (void)authorize:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOp } } else { - if (replacedOptions.force == YES) - [self.logger verbose:@"RS:%p ARTAuth: forced requesting new token.", _rest]; - else - [self.logger verbose:@"RS:%p ARTAuth: requesting new token.", _rest]; + [self.logger verbose:@"RS:%p ARTAuth: requesting new token.", _rest]; requestNewToken = YES; } diff --git a/Source/ARTAuthOptions.h b/Source/ARTAuthOptions.h index 4387ed278..21a9e10ab 100644 --- a/Source/ARTAuthOptions.h +++ b/Source/ARTAuthOptions.h @@ -78,11 +78,6 @@ ART_ASSUME_NONNULL_BEGIN */ @property (readwrite, assign, nonatomic) BOOL useTokenAuth; -/** - Indicates that a new token should be requested. - */ -@property (readwrite, assign, nonatomic) BOOL force; - - (instancetype)init; - (instancetype)initWithKey:(NSString *)key; - (instancetype)initWithToken:(NSString *)token; @@ -95,8 +90,6 @@ ART_ASSUME_NONNULL_BEGIN - (BOOL)isMethodGET; - (BOOL)isMethodPOST; -- (BOOL)isOnlyForceTrue; - @end ART_ASSUME_NONNULL_END diff --git a/Source/ARTAuthOptions.m b/Source/ARTAuthOptions.m index a282a4ff8..2693d922e 100644 --- a/Source/ARTAuthOptions.m +++ b/Source/ARTAuthOptions.m @@ -70,7 +70,6 @@ - (id)copyWithZone:(NSZone *)zone { options.authParams = self.authParams; options.queryTime = self.queryTime; options.useTokenAuth = self.useTokenAuth; - options.force = self.force; return options; } @@ -120,9 +119,7 @@ - (ARTAuthOptions *)mergeWith:(ARTAuthOptions *)precedenceOptions { merged.queryTime = precedenceOptions.queryTime; if (precedenceOptions.useTokenAuth) merged.useTokenAuth = precedenceOptions.useTokenAuth; - if (precedenceOptions.force) - merged.force = precedenceOptions.force; - + return merged; } @@ -134,17 +131,4 @@ - (BOOL)isMethodGET { return [_authMethod isEqualToString:@"GET"]; } -- (BOOL)isOnlyForceTrue { - return self.key == nil && - self.token == nil && - self.tokenDetails == nil && - self.authCallback == nil && - self.authUrl == nil && - self.authHeaders == nil && - self.authParams == nil && - self.queryTime == NO && - self.useTokenAuth == NO && - self.force == YES; -} - @end diff --git a/Source/ARTRest.m b/Source/ARTRest.m index 7cd767f20..a5546527c 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -94,10 +94,7 @@ - (void)executeRequest:(NSMutableURLRequest *)request withAuthOption:(ARTAuthent [self executeRequest:request completion:callback]; break; case ARTAuthenticationOn: - [self executeRequestWithAuthentication:request withMethod:self.auth.method force:NO completion:callback]; - break; - case ARTAuthenticationNewToken: - [self executeRequestWithAuthentication:request withMethod:self.auth.method force:YES completion:callback]; + [self executeRequestWithAuthentication:request withMethod:self.auth.method completion:callback]; break; case ARTAuthenticationUseBasic: [self executeRequestWithAuthentication:request withMethod:ARTAuthMethodBasic completion:callback]; @@ -106,11 +103,7 @@ - (void)executeRequest:(NSMutableURLRequest *)request withAuthOption:(ARTAuthent } - (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMethod:(ARTAuthMethod)method completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback { - [self executeRequestWithAuthentication:request withMethod:method force:NO completion:callback]; -} - -- (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMethod:(ARTAuthMethod)method force:(BOOL)force completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback { - [self prepareAuthorisationHeader:method force:force completion:^(NSString *authorization, NSError *error) { + [self prepareAuthorisationHeader:method completion:^(NSString *authorization, NSError *error) { if (error && callback) { callback(nil, nil, error); } else { @@ -142,7 +135,7 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT if (dataError.code >= 40140 && dataError.code < 40150) { // Send it again, requesting a new token (forward callback) [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p requesting new token", self]; - [self executeRequest:request withAuthOption:ARTAuthenticationNewToken completion:callback]; + [self executeRequest:request withAuthOption:ARTAuthenticationOn completion:callback]; return; } else { // Return error with HTTP StatusCode if ARTErrorStatusCode does not exist @@ -198,10 +191,6 @@ - (NSString *)currentHost { } - (void)prepareAuthorisationHeader:(ARTAuthMethod)method completion:(void (^)(NSString *authorization, NSError *error))callback { - [self prepareAuthorisationHeader:method force:NO completion:callback]; -} - -- (void)prepareAuthorisationHeader:(ARTAuthMethod)method force:(BOOL)force completion:(void (^)(NSString *authorization, NSError *error))callback { [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p calculating authorization %lu", self, (unsigned long)method]; // FIXME: use encoder and should be managed on ARTAuth if (method == ARTAuthMethodBasic) { @@ -211,7 +200,6 @@ - (void)prepareAuthorisationHeader:(ARTAuthMethod)method force:(BOOL)force compl if (callback) callback([NSString stringWithFormat:@"Basic %@", keyBase64], nil); } else { - self.options.force = force; [self.auth authorize:nil options:self.options callback:^(ARTTokenDetails *tokenDetails, NSError *error) { if (error) { if (callback) callback(nil, error); diff --git a/Source/ARTTypes.h b/Source/ARTTypes.h index f491b240c..0e328a881 100644 --- a/Source/ARTTypes.h +++ b/Source/ARTTypes.h @@ -23,8 +23,7 @@ typedef NS_ENUM(NSUInteger, ARTAuthentication) { ARTAuthenticationOff, ARTAuthenticationOn, - ARTAuthenticationUseBasic, - ARTAuthenticationNewToken + ARTAuthenticationUseBasic }; typedef NS_ENUM(NSUInteger, ARTAuthMethod) { diff --git a/Source/ARTWebSocketTransport.m b/Source/ARTWebSocketTransport.m index 3432a918d..a41648f9b 100644 --- a/Source/ARTWebSocketTransport.m +++ b/Source/ARTWebSocketTransport.m @@ -92,7 +92,6 @@ - (void)connectForcingNewToken:(BOOL)forceNewToken { ARTClientOptions *options = self.options; if (forceNewToken) { options = [options copy]; - options.force = true; } if ([options isBasicAuth]) { // Basic diff --git a/Spec/Auth.swift b/Spec/Auth.swift index bfeb4fc6c..8d66b6887 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -1270,7 +1270,6 @@ class Auth : QuickSpec { tokenParams.clientId = nil let authOptions = ARTAuthOptions() - authOptions.force = true authOptions.queryTime = true authOptions.key = options.key @@ -1784,7 +1783,6 @@ class Auth : QuickSpec { authOptions.authParams?.append(NSURLQueryItem(name: "type", value: "text")) authOptions.authParams?.append(NSURLQueryItem(name: "body", value: token)) authOptions.authHeaders = ["X-Ably":"Test"] - authOptions.force = true authOptions.queryTime = true waitUntil(timeout: testTimeout) { done in @@ -1803,7 +1801,6 @@ class Auth : QuickSpec { XCTFail("TokenDetails is nil"); done(); return } expect(testHTTPExecutor.requests.last?.URL?.host).to(equal("echo.ably.io")) - expect(auth.options.force).to(beFalse()) expect(auth.options.authUrl!.host).to(equal("echo.ably.io")) expect(auth.options.authHeaders!["X-Ably"]).to(equal("Test")) expect(tokenDetails.token).to(equal(token)) @@ -2702,7 +2699,6 @@ class Auth : QuickSpec { // reauthorise let reauthOptions = ARTAuthOptions(); reauthOptions.tokenDetails = secondTokenDetails - reauthOptions.force = true waitUntil(timeout: testTimeout) { done in realtime.auth.authorize(nil, options: reauthOptions) { reauthTokenDetails, error in From f54e7776d8a5c6ec7ab89021e85f49d2966aabc7 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Wed, 26 Oct 2016 18:40:55 +0100 Subject: [PATCH 2/6] Fix: make a single attempt to reissue the token and resend the request --- Source/ARTAuth+Private.h | 9 +++ Source/ARTAuth.m | 77 ++++++++++----------- Source/ARTRealtime.m | 19 +++--- Source/ARTRest+Private.h | 2 - Source/ARTRest.m | 94 +++++++++++++++++--------- Source/ARTStatus.h | 5 ++ Source/ARTStatus.m | 2 + Source/ARTTypes.h | 3 +- Source/ARTWebSocketTransport+Private.h | 4 ++ Source/ARTWebSocketTransport.h | 6 -- Source/ARTWebSocketTransport.m | 71 +++++++++++-------- 11 files changed, 174 insertions(+), 118 deletions(-) diff --git a/Source/ARTAuth+Private.h b/Source/ARTAuth+Private.h index aa9679ae7..f7458b5b8 100644 --- a/Source/ARTAuth+Private.h +++ b/Source/ARTAuth+Private.h @@ -38,6 +38,15 @@ ART_ASSUME_NONNULL_BEGIN // Discard the cached local clock offset - (void)discardTimeOffset; +// Configured options does have a means to renew the token automatically. +- (BOOL)canRenewTokenAutomatically:(ARTAuthOptions *)options; + +/// Does the client have a means to renew the token automatically. +- (BOOL)tokenIsRenewable; + +/// Does the client have a valid token (i.e. not expired). +- (BOOL)tokenRemainsValid; + // Private TokenDetails setter for testing only - (void)setTokenDetails:(ARTTokenDetails *)tokenDetails; diff --git a/Source/ARTAuth.m b/Source/ARTAuth.m index 869122ed3..b19d2aa7c 100644 --- a/Source/ARTAuth.m +++ b/Source/ARTAuth.m @@ -183,6 +183,26 @@ - (NSMutableURLRequest *)buildRequest:(ARTAuthOptions *)options withParams:(ARTT return request; } +- (BOOL)tokenIsRenewable { + return [self canRenewTokenAutomatically:self.options]; +} + +- (BOOL)canRenewTokenAutomatically:(ARTAuthOptions *)options { + return options.authCallback || options.authUrl || options.key; +} + +- (BOOL)tokenRemainsValid { + if (self.tokenDetails && self.tokenDetails.token) { + if (self.tokenDetails.expires == nil) { + return YES; + } + else if ([self.tokenDetails.expires timeIntervalSinceDate:[self currentDate]] > 0) { + return YES; + } + } + return NO; +} + - (void)requestToken:(ARTTokenParams *)tokenParams withOptions:(ARTAuthOptions *)authOptions callback:(void (^)(ARTTokenDetails *, NSError *))callback { @@ -191,8 +211,8 @@ - (void)requestToken:(ARTTokenParams *)tokenParams withOptions:(ARTAuthOptions * ARTTokenParams *currentTokenParams = tokenParams ? tokenParams : _tokenParams; currentTokenParams.timestamp = [self currentDate]; - if (replacedOptions.key == nil && replacedOptions.authCallback == nil && replacedOptions.authUrl == nil) { - callback(nil, [ARTErrorInfo createWithCode:ARTStateRequestTokenFailed message:@"no means to renew the token is provided (either an API key, authCallback or authUrl)"]); + if (![self canRenewTokenAutomatically:replacedOptions]) { + callback(nil, [ARTErrorInfo createWithCode:ARTStateRequestTokenFailed message:ARTAblyMessageNoMeansToRenewToken]); return; } @@ -314,7 +334,6 @@ - (void)authorise:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOp } - (void)authorize:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOptions callback:(void (^)(ARTTokenDetails *, NSError *))callback { - BOOL requestNewToken = NO; ARTAuthOptions *replacedOptions = [authOptions copy] ? : [self.options copy]; [self storeOptions:replacedOptions]; @@ -322,46 +341,22 @@ - (void)authorize:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)authOp ARTTokenParams *currentTokenParams = [self mergeParams:tokenParams]; [self storeParams:currentTokenParams]; - // Reuse or not reuse the current token - if (self.tokenDetails) { - if (self.tokenDetails.expires == nil) { - [self.logger verbose:@"RS:%p ARTAuth: reuse current token.", _rest]; - requestNewToken = NO; - } - else if ([self.tokenDetails.expires timeIntervalSinceDate:[self currentDate]] > 0) { - [self.logger verbose:@"RS:%p ARTAuth: current token has not expired yet. Reusing token details.", _rest]; - requestNewToken = NO; - } - else { - [self.logger verbose:@"RS:%p ARTAuth: current token has expired. Requesting new token.", _rest]; - requestNewToken = YES; - } - } - else { - [self.logger verbose:@"RS:%p ARTAuth: requesting new token.", _rest]; - requestNewToken = YES; - } - - if (requestNewToken) { - [self requestToken:currentTokenParams withOptions:replacedOptions callback:^(ARTTokenDetails *tokenDetails, NSError *error) { - if (error) { - [self.logger verbose:@"RS:%p ARTAuth: token request failed: %@", _rest, error]; - if (callback) { - callback(nil, error); - } - } else { - _tokenDetails = tokenDetails; - [self.logger verbose:@"RS:%p ARTAuth: token request succeeded: %@", _rest, tokenDetails]; - if (callback) { - callback(self.tokenDetails, nil); - } + // Request always a new token + [self.logger verbose:@"RS:%p ARTAuth: requesting new token.", _rest]; + [self requestToken:currentTokenParams withOptions:replacedOptions callback:^(ARTTokenDetails *tokenDetails, NSError *error) { + if (error) { + [self.logger verbose:@"RS:%p ARTAuth: token request failed: %@", _rest, error]; + if (callback) { + callback(nil, error); + } + } else { + _tokenDetails = tokenDetails; + [self.logger verbose:@"RS:%p ARTAuth: token request succeeded: %@", _rest, tokenDetails]; + if (callback) { + callback(self.tokenDetails, nil); } - }]; - } else { - if (callback) { - callback(self.tokenDetails, nil); } - } + }]; } - (void)createTokenRequest:(ARTTokenParams *)tokenParams options:(ARTAuthOptions *)options callback:(void (^)(ARTTokenRequest *, NSError *))callback { diff --git a/Source/ARTRealtime.m b/Source/ARTRealtime.m index e79d87f9b..276e837fc 100644 --- a/Source/ARTRealtime.m +++ b/Source/ARTRealtime.m @@ -14,6 +14,7 @@ #import "ARTDefault.h" #import "ARTRest+Private.h" #import "ARTAuth+Private.h" +#import "ARTTokenDetails.h" #import "ARTMessage.h" #import "ARTClientOptions.h" #import "ARTChannelOptions.h" @@ -462,7 +463,7 @@ - (void)onDisconnected:(ARTProtocolMessage *)message { [self.logger info:@"R:%p ARTRealtime disconnected", self]; ARTErrorInfo *error = message.error; if ([self shouldRenewToken:&error]) { - [self connectWithRenewedToken]; + [self transportReconnectWithRenewedToken]; return; } [self transition:ARTRealtimeDisconnected withErrorInfo:error]; @@ -490,7 +491,7 @@ - (void)onError:(ARTProtocolMessage *)message { } else { ARTErrorInfo *error = message.error; if ([self shouldRenewToken:&error]) { - [self connectWithRenewedToken]; + [self transportReconnectWithRenewedToken]; return; } [self.connection setId:nil]; @@ -501,19 +502,20 @@ - (void)onError:(ARTProtocolMessage *)message { - (BOOL)shouldRenewToken:(ARTErrorInfo **)errorPtr { if (!_renewingToken && errorPtr && *errorPtr && (*errorPtr).statusCode == 401 && (*errorPtr).code >= 40140 && (*errorPtr).code < 40150) { - if ([self isTokenRenewable]) { + if ([self.auth tokenIsRenewable]) { return YES; } - *errorPtr = [ARTErrorInfo createWithCode:ARTStateRequestTokenFailed message:@"no means to renew the token is provided (either an API key, authCallback or authUrl)"]; + *errorPtr = [ARTErrorInfo createWithCode:ARTStateRequestTokenFailed message:ARTAblyMessageNoMeansToRenewToken]; } return NO; } -- (BOOL)isTokenRenewable { - return self.options.authCallback || self.options.authUrl || self.options.key; +- (void)transportReconnectWithHost:(NSString *)host { + [self.transport setHost:host]; + [self.transport connect]; } -- (void)connectWithRenewedToken { +- (void)transportReconnectWithRenewedToken { _renewingToken = true; [_transport close]; _transport = [[_transportClass alloc] initWithRest:self.rest options:self.options resumeKey:_transport.resumeKey connectionSerial:_transport.connectionSerial]; @@ -715,8 +717,7 @@ - (BOOL)reconnectWithFallback { if (host != nil) { [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p host is down; retrying realtime connection at %@", self, host]; self.rest.prioritizedHost = host; - [self.transport setHost:host]; - [self.transport connect]; + [self transportReconnectWithHost:host]; return true; } else { _fallbacks = nil; diff --git a/Source/ARTRest+Private.h b/Source/ARTRest+Private.h index 8f2014ff6..c99f65b2f 100644 --- a/Source/ARTRest+Private.h +++ b/Source/ARTRest+Private.h @@ -44,8 +44,6 @@ ART_ASSUME_NONNULL_BEGIN - (void)executeRequest:(NSMutableURLRequest *)request withAuthOption:(ARTAuthentication)authOption completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback; -- (void)prepareAuthorisationHeader:(ARTAuthMethod)method completion:(void (^)(NSString *__art_nonnull authorization, NSError *__art_nullable error))callback; - - (id)internetIsUp:(void (^)(BOOL isUp))cb; @end diff --git a/Source/ARTRest.m b/Source/ARTRest.m index a5546527c..1e48ad1c7 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -94,7 +94,10 @@ - (void)executeRequest:(NSMutableURLRequest *)request withAuthOption:(ARTAuthent [self executeRequest:request completion:callback]; break; case ARTAuthenticationOn: - [self executeRequestWithAuthentication:request withMethod:self.auth.method completion:callback]; + [self executeRequestWithAuthentication:request withMethod:self.auth.method force:NO completion:callback]; + break; + case ARTAuthenticationNewToken: + [self executeRequestWithAuthentication:request withMethod:self.auth.method force:YES completion:callback]; break; case ARTAuthenticationUseBasic: [self executeRequestWithAuthentication:request withMethod:ARTAuthMethodBasic completion:callback]; @@ -103,15 +106,41 @@ - (void)executeRequest:(NSMutableURLRequest *)request withAuthOption:(ARTAuthent } - (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMethod:(ARTAuthMethod)method completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback { - [self prepareAuthorisationHeader:method completion:^(NSString *authorization, NSError *error) { - if (error && callback) { - callback(nil, nil, error); - } else { - // RFC7235 + [self executeRequestWithAuthentication:request withMethod:method force:NO completion:callback]; +} + +- (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMethod:(ARTAuthMethod)method force:(BOOL)force completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback { + [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p calculating authorization %lu", self, (unsigned long)method]; + if (method == ARTAuthMethodBasic) { + // Basic + NSString *authorization = [self prepareBasicAuthorisationHeader:self.options.key]; + [request setValue:authorization forHTTPHeaderField:@"Authorization"]; + [self.logger verbose:@"RS:%p ARTRest: %@", self, authorization]; + [self executeRequest:request completion:callback]; + } + else { + if (!force && [self.auth tokenRemainsValid]) { + // Reuse token + NSString *authorization = [self prepareTokenAuthorisationHeader:self.auth.tokenDetails.token]; + [self.logger verbose:@"RS:%p ARTRest reusing token: authorization bearer in Base64 %@", self, authorization]; [request setValue:authorization forHTTPHeaderField:@"Authorization"]; [self executeRequest:request completion:callback]; } - }]; + else { + // New Token + [self.auth authorize:nil options:self.options callback:^(ARTTokenDetails *tokenDetails, NSError *error) { + if (error) { + [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p ARTRest reissuing token failed %@", self, error]; + if (callback) callback(nil, nil, error); + return; + } + NSString *authorization = [self prepareTokenAuthorisationHeader:tokenDetails.token]; + [self.logger verbose:@"RS:%p ARTRest reissuing token: authorization bearer in Base64 %@", self, authorization]; + [request setValue:authorization forHTTPHeaderField:@"Authorization"]; + [self executeRequest:request completion:callback]; + }]; + } + } } - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback { @@ -132,10 +161,10 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT [self.httpExecutor executeRequest:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) { if (response.statusCode >= 400) { NSError *dataError = [self->_encoders[response.MIMEType] decodeError:data]; - if (dataError.code >= 40140 && dataError.code < 40150) { - // Send it again, requesting a new token (forward callback) - [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p requesting new token", self]; - [self executeRequest:request withAuthOption:ARTAuthenticationOn completion:callback]; + if ([self shouldRenewToken:&dataError]) { + [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p retry request %@", self, request]; + // Make a single attempt to reissue the token and resend the request + [self executeRequest:request withAuthOption:ARTAuthenticationNewToken completion:callback]; return; } else { // Return error with HTTP StatusCode if ARTErrorStatusCode does not exist @@ -169,6 +198,17 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT }]; } +- (BOOL)shouldRenewToken:(NSError **)errorPtr { + if (errorPtr && *errorPtr && + (*errorPtr).code >= 40140 && (*errorPtr).code < 40150) { + if ([self.auth tokenIsRenewable]) { + return YES; + } + *errorPtr = (NSError *)[ARTErrorInfo createWithCode:ARTStateRequestTokenFailed message:ARTAblyMessageNoMeansToRenewToken]; + } + return NO; +} + - (BOOL)shouldRetryWithFallback:(NSMutableURLRequest *)request response:(NSHTTPURLResponse *)response error:(NSError *)error { if (response.statusCode >= 500 && response.statusCode <= 504) { return YES; @@ -190,27 +230,17 @@ - (NSString *)currentHost { return self.options.restHost; } -- (void)prepareAuthorisationHeader:(ARTAuthMethod)method completion:(void (^)(NSString *authorization, NSError *error))callback { - [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p calculating authorization %lu", self, (unsigned long)method]; - // FIXME: use encoder and should be managed on ARTAuth - if (method == ARTAuthMethodBasic) { - // Include key Base64 encoded in an Authorization header (RFC7235) - NSData *keyData = [self.options.key dataUsingEncoding:NSUTF8StringEncoding]; - NSString *keyBase64 = [keyData base64EncodedStringWithOptions:0]; - if (callback) callback([NSString stringWithFormat:@"Basic %@", keyBase64], nil); - } - else { - [self.auth authorize:nil options:self.options callback:^(ARTTokenDetails *tokenDetails, NSError *error) { - if (error) { - if (callback) callback(nil, error); - return; - } - NSData *tokenData = [tokenDetails.token dataUsingEncoding:NSUTF8StringEncoding]; - NSString *tokenBase64 = [tokenData base64EncodedStringWithOptions:0]; - [self.logger verbose:@"RS:%p ARTRest: authorization bearer in Base64 %@", self, tokenBase64]; - if (callback) callback([NSString stringWithFormat:@"Bearer %@", tokenBase64], nil); - }]; - } +- (NSString *)prepareBasicAuthorisationHeader:(NSString *)key { + // Include key Base64 encoded in an Authorization header (RFC7235) + NSData *keyData = [key dataUsingEncoding:NSUTF8StringEncoding]; + NSString *keyBase64 = [keyData base64EncodedStringWithOptions:0]; + return [NSString stringWithFormat:@"Basic %@", keyBase64]; +} + +- (NSString *)prepareTokenAuthorisationHeader:(NSString *)token { + NSData *tokenData = [token dataUsingEncoding:NSUTF8StringEncoding]; + NSString *tokenBase64 = [tokenData base64EncodedStringWithOptions:0]; + return [NSString stringWithFormat:@"Bearer %@", tokenBase64]; } - (void)time:(void(^)(NSDate *time, NSError *error))callback { diff --git a/Source/ARTStatus.h b/Source/ARTStatus.h index 376cb6310..36753bf91 100644 --- a/Source/ARTStatus.h +++ b/Source/ARTStatus.h @@ -48,6 +48,11 @@ FOUNDATION_EXPORT NSString *const ARTAblyErrorDomain; */ FOUNDATION_EXPORT NSString *const ARTFallbackIncompatibleOptionsException; +/** + Ably client error messages + */ +FOUNDATION_EXPORT NSString *const ARTAblyMessageNoMeansToRenewToken; + /** Ably client error class */ diff --git a/Source/ARTStatus.m b/Source/ARTStatus.m index 9bc48b0dc..9833be8e8 100644 --- a/Source/ARTStatus.m +++ b/Source/ARTStatus.m @@ -15,6 +15,8 @@ NSString *const ARTFallbackIncompatibleOptionsException = @"ARTFallbackIncompatibleOptionsException"; +NSString *const ARTAblyMessageNoMeansToRenewToken = @"no means to renew the token is provided (either an API key, authCallback or authUrl)"; + NSInteger getStatusFromCode(NSInteger code) { return code / 100; } diff --git a/Source/ARTTypes.h b/Source/ARTTypes.h index 0e328a881..f491b240c 100644 --- a/Source/ARTTypes.h +++ b/Source/ARTTypes.h @@ -23,7 +23,8 @@ typedef NS_ENUM(NSUInteger, ARTAuthentication) { ARTAuthenticationOff, ARTAuthenticationOn, - ARTAuthenticationUseBasic + ARTAuthenticationUseBasic, + ARTAuthenticationNewToken }; typedef NS_ENUM(NSUInteger, ARTAuthMethod) { diff --git a/Source/ARTWebSocketTransport+Private.h b/Source/ARTWebSocketTransport+Private.h index 29753a90a..c893d76c7 100644 --- a/Source/ARTWebSocketTransport+Private.h +++ b/Source/ARTWebSocketTransport+Private.h @@ -27,12 +27,16 @@ ART_ASSUME_NONNULL_BEGIN @property (readonly, strong, nonatomic) ARTAuth *auth; @property (readonly, strong, nonatomic) ARTClientOptions *options; +@property (readwrite, assign, nonatomic) BOOL closing; + @property (readwrite, strong, nonatomic, art_nullable) SRWebSocket *websocket; @property (readwrite, strong, nonatomic, art_nullable) NSURL *websocketURL; - (void)sendWithData:(NSData *)data; - (void)receiveWithData:(NSData *)data; +- (NSURL *)setupWebSocket:(__GENERIC(NSArray, NSURLQueryItem *) *)params withOptions:(ARTClientOptions *)options resumeKey:(NSString *__art_nullable)resumeKey connectionSerial:(NSNumber *__art_nullable)connectionSerial; + @end ART_ASSUME_NONNULL_END diff --git a/Source/ARTWebSocketTransport.h b/Source/ARTWebSocketTransport.h index abf0cd621..3e26e2cff 100644 --- a/Source/ARTWebSocketTransport.h +++ b/Source/ARTWebSocketTransport.h @@ -27,12 +27,6 @@ ART_ASSUME_NONNULL_BEGIN @property (readonly, getter=getIsConnected) BOOL isConnected; -@property (readwrite, assign, nonatomic) BOOL closing; - -- (NSURL *)setupWebSocket:(__GENERIC(NSArray, NSURLQueryItem *) *)params withOptions:(ARTClientOptions *)options resumeKey:(NSString *__art_nullable)resumeKey connectionSerial:(NSNumber *__art_nullable)connectionSerial; - -- (BOOL)getIsConnected; - @end ART_ASSUME_NONNULL_END diff --git a/Source/ARTWebSocketTransport.m b/Source/ARTWebSocketTransport.m index a41648f9b..a6b16d897 100644 --- a/Source/ARTWebSocketTransport.m +++ b/Source/ARTWebSocketTransport.m @@ -10,7 +10,7 @@ #import "ARTRest.h" #import "ARTRest+Private.h" -#import "ARTAuth.h" +#import "ARTAuth+Private.h" #import "ARTProtocolMessage.h" #import "ARTClientOptions.h" #import "ARTTokenParams.h" @@ -89,38 +89,55 @@ - (void)connect { - (void)connectForcingNewToken:(BOOL)forceNewToken { [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p websocket connect", _delegate, self]; - ARTClientOptions *options = self.options; - if (forceNewToken) { - options = [options copy]; - } + ARTClientOptions *options = [self.options copy]; + if ([options isBasicAuth]) { // Basic - NSURLQueryItem *keyParam = [NSURLQueryItem queryItemWithName:@"key" value:options.key]; - [self setupWebSocket:@[keyParam] withOptions:options resumeKey:self.resumeKey connectionSerial:self.connectionSerial]; - // Connect - [self.websocket open]; + [self connectWithKey:options.key]; } else { + // Token [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p connecting with token auth; authorising", _delegate, self]; __weak ARTWebSocketTransport *selfWeak = self; - // Token - [self.auth authorize:nil options:options callback:^(ARTTokenDetails *tokenDetails, NSError *error) { - [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p authorised: %@ error: %@", _delegate, self, tokenDetails, error]; - ARTWebSocketTransport *selfStrong = selfWeak; - if (!selfStrong) return; - - if (error) { - [selfStrong.logger error:@"R:%p WS:%p ARTWebSocketTransport: token auth failed with %@", _delegate, self, error.description]; - [selfStrong.delegate realtimeTransportFailed:selfStrong withError:[[ARTRealtimeTransportError alloc] initWithError:error type:ARTRealtimeTransportErrorTypeAuth url:self.websocketURL]]; - return; - } - - NSURLQueryItem *accessTokenParam = [NSURLQueryItem queryItemWithName:@"accessToken" value:(tokenDetails.token)]; - [selfStrong setupWebSocket:@[accessTokenParam] withOptions:selfStrong.options resumeKey:self.resumeKey connectionSerial:self.connectionSerial]; - // Connect - [selfStrong.websocket open]; - }]; - } + + if (!forceNewToken && [self.auth tokenRemainsValid]) { + // Reuse token + [self connectWithToken:self.auth.tokenDetails.token]; + } + else { + // New Token + [self.auth authorize:nil options:options callback:^(ARTTokenDetails *tokenDetails, NSError *error) { + ARTWebSocketTransport *selfStrong = selfWeak; + if (!selfStrong) return; + + [selfStrong.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p authorised: %@ error: %@", _delegate, self, tokenDetails, error]; + + if (error) { + [selfStrong.logger error:@"R:%p WS:%p ARTWebSocketTransport: token auth failed with %@", _delegate, self, error.description]; + [selfStrong.delegate realtimeTransportFailed:selfStrong withError:[[ARTRealtimeTransportError alloc] initWithError:error type:ARTRealtimeTransportErrorTypeAuth url:self.websocketURL]]; + return; + } + + [selfStrong connectWithToken:tokenDetails.token]; + }]; + } + } +} + +- (void)connectWithKey:(NSString *)key { + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p websocket connect with key", _delegate, self]; + NSURLQueryItem *keyParam = [NSURLQueryItem queryItemWithName:@"key" value:key]; + [self setupWebSocket:@[keyParam] withOptions:self.options resumeKey:self.resumeKey connectionSerial:self.connectionSerial]; + // Connect + [self.websocket open]; +} + +- (void)connectWithToken:(NSString *)token { + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p WS:%p websocket connect with token", _delegate, self]; + NSURLQueryItem *accessTokenParam = [NSURLQueryItem queryItemWithName:@"accessToken" value:token]; + [self setupWebSocket:@[accessTokenParam] withOptions:self.options resumeKey:self.resumeKey connectionSerial:self.connectionSerial]; + // Connect + [self.websocket open]; } - (BOOL)getIsConnected { From 2fdb883f10110aee5a004471646f0eeaec25ba90 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 27 Oct 2016 10:25:39 +0100 Subject: [PATCH 3/6] Fix RSA10a --- Spec/Auth.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Spec/Auth.swift b/Spec/Auth.swift index 8d66b6887..63b458a6e 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -1678,6 +1678,7 @@ class Auth : QuickSpec { // RSA10a it("should create a token if needed and use it") { let options = AblyTests.clientOptions(requestToken: true) + options.key = AblyTests.commonAppSetup().key waitUntil(timeout: testTimeout) { done in // Client with Token let rest = ARTRest(options: options) @@ -1691,7 +1692,7 @@ class Auth : QuickSpec { guard let tokenDetails = tokenDetails else { XCTFail("TokenDetails is nil"); done(); return } - expect(tokenDetails.token).to(equal(options.token)) + expect(tokenDetails.token).toNot(equal(options.token)) publishTestMessage(rest, completion: { error in expect(error).to(beNil()) From 095122d131a4e8245b51a289100c42fb7dcb6d64 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 27 Oct 2016 10:25:48 +0100 Subject: [PATCH 4/6] Fix RSC9 --- Spec/RestClient.swift | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Spec/RestClient.swift b/Spec/RestClient.swift index 57c9fa482..e3c888d92 100644 --- a/Spec/RestClient.swift +++ b/Spec/RestClient.swift @@ -371,22 +371,30 @@ class RestClient: QuickSpec { // RSC9 it("should use Auth to manage authentication") { let options = AblyTests.clientOptions() - options.tokenDetails = getTestTokenDetails() + guard let testTokenDetails = getTestTokenDetails() else { + fail("No test token details"); return + } + options.tokenDetails = testTokenDetails + options.authCallback = { tokenParams, completion in + completion(testTokenDetails, nil) + } + + let client = ARTRest(options: options) + expect(client.auth).to(beAnInstanceOf(ARTAuth.self)) waitUntil(timeout: testTimeout) { done in - ARTRest(options: options).auth.authorize(nil, options: nil) { tokenDetails, error in + client.auth.authorize(nil, options: nil) { tokenDetails, error in if let e = error { XCTFail(e.description) done() return } guard let tokenDetails = tokenDetails else { - XCTFail("expected tokenDetails not to be nil when error is nil") + XCTFail("expected tokenDetails to not be nil when error is nil") done() return } - // Use the same token because it is valid - expect(tokenDetails.token).to(equal(options.tokenDetails!.token)) + expect(tokenDetails.token).to(equal(testTokenDetails.token)) done() } } From bace5e4314fd205c350958d052c53c9410e51917 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 27 Oct 2016 10:52:24 +0100 Subject: [PATCH 5/6] Remove `prepareAuthorisationHeader` access from test suite --- Spec/Auth.swift | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/Spec/Auth.swift b/Spec/Auth.swift index 63b458a6e..39e0af18a 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -274,7 +274,8 @@ class Auth : QuickSpec { it("on rest") { let expectedClientId = "client_string" - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.commonAppSetup() + options.useTokenAuth = true options.clientId = expectedClientId let client = ARTRest(options: options) @@ -282,11 +283,13 @@ class Auth : QuickSpec { waitUntil(timeout: testTimeout) { done in // Token - client.prepareAuthorisationHeader(ARTAuthMethod.Token) { token, error in - if let e = error { - XCTFail(e.description) + client.auth.authorize(nil, options: nil) { tokenDetails, error in + expect(error).to(beNil()) + expect(client.auth.method).to(equal(ARTAuthMethod.Token)) + guard let tokenDetails = tokenDetails else { + fail("TokenDetails is nil"); done(); return } - expect(client.auth.clientId).to(equal(expectedClientId)) + expect(tokenDetails.clientId).to(equal(expectedClientId)) done() } } @@ -349,19 +352,17 @@ class Auth : QuickSpec { // RSA15b it("should permit to be unauthenticated") { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.commonAppSetup() options.clientId = nil let clientBasic = ARTRest(options: options) waitUntil(timeout: testTimeout) { done in // Basic - clientBasic.prepareAuthorisationHeader(ARTAuthMethod.Basic) { token, error in - if let e = error { - XCTFail(e.description) - } + clientBasic.auth.authorize(nil, options: nil) { tokenDetails, error in + expect(error).to(beNil()) expect(clientBasic.auth.clientId).to(beNil()) - options.tokenDetails = clientBasic.auth.tokenDetails + options.tokenDetails = tokenDetails done() } } @@ -370,16 +371,12 @@ class Auth : QuickSpec { waitUntil(timeout: testTimeout) { done in // Last TokenDetails - clientToken.prepareAuthorisationHeader(ARTAuthMethod.Token) { token, error in - if let e = error { - XCTFail(e.description) - } + clientToken.auth.authorize(nil, options: nil) { tokenDetails, error in + expect(error).to(beNil()) expect(clientToken.auth.clientId).to(beNil()) done() } } - - // TODO: Realtime.connectionDetails } // RSA15c @@ -602,8 +599,9 @@ class Auth : QuickSpec { // RSA7b2 it("when tokenRequest or tokenDetails has clientId not null or wildcard string") { - let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) + let options = AblyTests.commonAppSetup() options.clientId = "client_string" + options.useTokenAuth = true let client = ARTRest(options: options) client.httpExecutor = testHTTPExecutor @@ -611,10 +609,9 @@ class Auth : QuickSpec { // TokenDetails waitUntil(timeout: 10) { done in // Token - client.prepareAuthorisationHeader(ARTAuthMethod.Token) { token, error in - if let e = error { - XCTFail(e.description) - } + client.auth.authorize(nil, options: nil) { token, error in + expect(error).to(beNil()) + expect(client.auth.method).to(equal(ARTAuthMethod.Token)) expect(client.auth.clientId).to(equal(options.clientId)) done() } From 6bef50917cab400956229dd5fa8ae781b9649a0b Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Tue, 1 Nov 2016 12:39:32 +0000 Subject: [PATCH 6/6] Fix RSA10a --- Spec/Auth.swift | 68 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/Spec/Auth.swift b/Spec/Auth.swift index 39e0af18a..7a9dbc7f6 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -1670,32 +1670,60 @@ class Auth : QuickSpec { } // RSA10 - describe("authorise") { + describe("authorize") { // RSA10a - it("should create a token if needed and use it") { - let options = AblyTests.clientOptions(requestToken: true) - options.key = AblyTests.commonAppSetup().key + it("should always create a token") { + let options = AblyTests.commonAppSetup() + options.useTokenAuth = true + let rest = ARTRest(options: options) + let channel = rest.channels.get("test") + waitUntil(timeout: testTimeout) { done in - // Client with Token - let rest = ARTRest(options: options) - publishTestMessage(rest, completion: { error in + channel.publish(nil, data: "first check") { error in expect(error).to(beNil()) - expect(rest.auth.method).to(equal(ARTAuthMethod.Token)) + done() + } + } - // Reuse the valid token - rest.auth.authorize(nil, options: nil, callback: { tokenDetails, error in - expect(rest.auth.method).to(equal(ARTAuthMethod.Token)) - guard let tokenDetails = tokenDetails else { - XCTFail("TokenDetails is nil"); done(); return - } - expect(tokenDetails.token).toNot(equal(options.token)) + // Check that token exists + expect(rest.auth.method).to(equal(ARTAuthMethod.Token)) + guard let firstTokenDetails = rest.auth.tokenDetails else { + fail("TokenDetails is nil"); return + } + expect(firstTokenDetails.token).toNot(beNil()) - publishTestMessage(rest, completion: { error in - expect(error).to(beNil()) - done() - }) - }) + waitUntil(timeout: testTimeout) { done in + channel.publish(nil, data: "second check") { error in + expect(error).to(beNil()) + done() + } + } + + // Check that token has not changed + expect(rest.auth.method).to(equal(ARTAuthMethod.Token)) + guard let secondTokenDetails = rest.auth.tokenDetails else { + fail("TokenDetails is nil"); return + } + expect(firstTokenDetails).to(beIdenticalTo(secondTokenDetails)) + + waitUntil(timeout: testTimeout) { done in + rest.auth.authorize(nil, options: nil, callback: { tokenDetails, error in + expect(error).to(beNil()) + guard let tokenDetails = tokenDetails else { + XCTFail("TokenDetails is nil"); done(); return + } + // Check that token has changed + expect(tokenDetails.token).toNot(equal(firstTokenDetails.token)) + + channel.publish(nil, data: "third check") { error in + expect(error).to(beNil()) + guard let thirdTokenDetails = rest.auth.tokenDetails else { + fail("TokenDetails is nil"); return + } + expect(thirdTokenDetails.token).to(equal(tokenDetails.token)) + done() + } }) } }