Remove AuthOptions.force (#527)

* Remove AuthOptions.force * Fix: make a single attempt to reissue the token and resend the request * Fix RSA10a * Fix RSC9 * Remove `prepareAuthorisationHeader` access from test suite * Fix RSA10a
ably · Nov 1, 2016 · 9d1a8af · 9d1a8af
1 parent ceae327
commit 9d1a8af
Show file tree

Hide file tree

Showing 14 changed files with 243 additions and 205 deletions.
diff --git 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
@@ -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 {
@@ -184,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 {
 
@@ -192,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;
     }
 
@@ -315,64 +334,29 @@ - (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.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 {
-        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];
-        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/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
@@ -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/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
@@ -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<ARTCancellable>)internetIsUp:(void (^)(BOOL isUp))cb;
 
 @end

diff --git a/Source/ARTRest.m b/Source/ARTRest.m
@@ -110,15 +110,37 @@ - (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMeth
 }
 
 - (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) {
-        if (error && callback) {
-            callback(nil, nil, error);
-        } else {
-            // RFC7235
+    [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 {
@@ -139,9 +161,9 @@ - (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];
+            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 {
@@ -176,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;
@@ -197,32 +230,17 @@ - (NSString *)currentHost {
     return self.options.restHost;
 }
 
-- (void)prepareAuthorisationHeader:(ARTAuthMethod)method completion:(void (^)(NSString *authorization, NSError *error))callback {
-    [self prepareAuthorisationHeader:method force:NO completion:callback];
+- (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];
 }
 
-- (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) {
-        // 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.options.force = force;
-        [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 *)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
@@ -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
  */
-Original file line number
+Diff line change
@@ Expand Up / @@ -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<ARTCancellable>)internetIsUp:(void (^)(BOOL isUp))cb;
     @end
@@ Expand Down @@