Add strict nil token / error handling in FIRAppAttestAPIService

firebase · Aug 3, 2023 · 3687479 · 3687479
1 parent 5a7af33
commit 3687479
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 55 deletions.
diff --git a/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAPIService.m b/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAPIService.m
@@ -106,22 +106,29 @@ - (instancetype)initWithAPIService:(id<FIRAppCheckAPIServiceProtocol>)APIService
 #pragma mark - Challenge response parsing
 
 - (FBLPromise<NSData *> *)randomChallengeWithAPIResponse:(GULURLSessionDataResponse *)response {
-  return [FBLPromise onQueue:[self backgroundQueue]
-                          do:^id _Nullable {
-                            NSError *error;
-
-                            NSData *randomChallenge =
-                                [self randomChallengeFromResponseBody:response.HTTPBody
-                                                                error:&error];
-
-                            return randomChallenge ?: error;
-                          }];
+  return [FBLPromise
+      onQueue:[self backgroundQueue]
+           do:^id _Nullable {
+             NSError *error;
+
+             NSData *randomChallenge = [self randomChallengeFromResponseBody:response.HTTPBody
+                                                                       error:&error];
+             if (randomChallenge) {
+               return randomChallenge;
+             } else if (error) {
+               return error;
+             } else {
+               return [FIRAppCheckErrorUtil
+                   errorWithFailureReason:@"Failed to parse random challenge response."];
+             }
+           }];
 }
 
-- (nullable NSData *)randomChallengeFromResponseBody:(NSData *)response error:(NSError **)outError {
+- (nullable NSData *)randomChallengeFromResponseBody:(NSData *)response
+                                               error:(NSError **_Nonnull)outError {
+  NSParameterAssert(outError);
   if (response.length <= 0) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil errorWithFailureReason:@"Empty server response body."], outError);
+    *outError = [FIRAppCheckErrorUtil errorWithFailureReason:@"Empty server response body."];
     return nil;
   }
 
@@ -131,18 +138,26 @@ - (nullable NSData *)randomChallengeFromResponseBody:(NSData *)response error:(N
                                                                  error:&JSONError];
 
   if (![responseDict isKindOfClass:[NSDictionary class]]) {
-    FIRAppCheckSetErrorToPointer([FIRAppCheckErrorUtil JSONSerializationError:JSONError], outError);
+    if (JSONError) {
+      *outError = [FIRAppCheckErrorUtil JSONSerializationError:JSONError];
+    } else {
+      *outError = [FIRAppCheckErrorUtil
+          errorWithFailureReason:@"Unexpected top-level JSON object in random challenge response."];
+    }
     return nil;
   }
 
   NSString *challenge = responseDict[@"challenge"];
   if (![challenge isKindOfClass:[NSString class]]) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil appCheckTokenResponseErrorWithMissingField:@"challenge"], outError);
+    *outError = [FIRAppCheckErrorUtil appCheckTokenResponseErrorWithMissingField:@"challenge"];
     return nil;
   }
 
   NSData *randomChallenge = [[NSData alloc] initWithBase64EncodedString:challenge options:0];
+  if (!randomChallenge) {
+    *outError =
+        [FIRAppCheckErrorUtil errorWithFailureReason:@"Failed to decode random challenge data."];
+  }
   return randomChallenge;
 }
 
@@ -160,17 +175,23 @@ - (nullable NSData *)randomChallengeFromResponseBody:(NSData *)response error:(N
                                               body:HTTPBody
                                  additionalHeaders:@{kContentTypeKey : kJSONContentType}];
       })
-      .thenOn(
-          [self backgroundQueue], ^id _Nullable(GULURLSessionDataResponse *_Nullable URLResponse) {
-            NSError *error;
-
-            __auto_type response =
-                [[FIRAppAttestAttestationResponse alloc] initWithResponseData:URLResponse.HTTPBody
-                                                                  requestDate:[NSDate date]
-                                                                        error:&error];
-
-            return response ?: error;
-          });
+      .thenOn([self backgroundQueue], ^id(GULURLSessionDataResponse *_Nullable URLResponse) {
+        NSError *error;
+
+        __auto_type response =
+            [[FIRAppAttestAttestationResponse alloc] initWithResponseData:URLResponse.HTTPBody
+                                                              requestDate:[NSDate date]
+                                                                    error:&error];
+
+        if (response) {
+          return response;
+        } else if (error) {
+          return error;
+        } else {
+          return [FIRAppCheckErrorUtil
+              errorWithFailureReason:@"Failed to initialize attestation response."];
+        }
+      });
 }
 
 #pragma mark - Request HTTP Body
@@ -240,10 +261,12 @@ - (NSString *)base64StringWithData:(NSData *)data {
 }
 
 - (NSURL *)URLForEndpoint:(NSString *)endpoint {
-  NSString *URL = [[self class] URLWithBaseURL:self.APIService.baseURL
-                                     projectID:self.projectID
-                                         appID:self.appID];
-  return [NSURL URLWithString:[NSString stringWithFormat:@"%@:%@", URL, endpoint]];
+  NSString *baseURL = [[self class] URLWithBaseURL:self.APIService.baseURL
+                                         projectID:self.projectID
+                                             appID:self.appID];
+  NSURL *url = [NSURL URLWithString:[NSString stringWithFormat:@"%@:%@", baseURL, endpoint]];
+  NSAssert(url != nil, @"URL for endpoint '%@' is nil.", endpoint);
+  return url;
 }
 
 + (NSString *)URLWithBaseURL:(NSString *)baseURL

diff --git a/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAttestationResponse.h b/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAttestationResponse.h
@@ -36,7 +36,7 @@ NS_ASSUME_NONNULL_BEGIN
 /// Init with the server response.
 - (nullable instancetype)initWithResponseData:(NSData *)response
                                   requestDate:(NSDate *)requestDate
-                                        error:(NSError **)outError;
+                                        error:(NSError **_Nonnull)outError;
 
 @end
 

diff --git a/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAttestationResponse.m b/FirebaseAppCheck/Sources/AppAttestProvider/API/FIRAppAttestAttestationResponse.m
@@ -19,6 +19,8 @@
 #import "FirebaseAppCheck/Sources/Core/APIService/FIRAppCheckToken+APIResponse.h"
 #import "FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h"
 
+NS_ASSUME_NONNULL_BEGIN
+
 static NSString *const kResponseFieldAppCheckTokenDict = @"appCheckToken";
 static NSString *const kResponseFieldArtifact = @"artifact";
 
@@ -35,13 +37,12 @@ - (instancetype)initWithArtifact:(NSData *)artifact token:(FIRAppCheckToken *)to
 
 - (nullable instancetype)initWithResponseData:(NSData *)response
                                   requestDate:(NSDate *)requestDate
-                                        error:(NSError **)outError {
+                                        error:(NSError **_Nonnull)outError {
+  NSParameterAssert(outError);
   if (response.length <= 0) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil
-            errorWithFailureReason:
-                @"Failed to parse the initial handshake response. Empty server response body."],
-        outError);
+    *outError = [FIRAppCheckErrorUtil
+        errorWithFailureReason:
+            @"Failed to parse the initial handshake response. Empty server response body."];
     return nil;
   }
 
@@ -51,46 +52,54 @@ - (nullable instancetype)initWithResponseData:(NSData *)response
                                                                  error:&JSONError];
 
   if (![responseDict isKindOfClass:[NSDictionary class]]) {
-    FIRAppCheckSetErrorToPointer([FIRAppCheckErrorUtil JSONSerializationError:JSONError], outError);
+    if (JSONError) {
+      *outError = [FIRAppCheckErrorUtil JSONSerializationError:JSONError];
+    } else {
+      *outError = [FIRAppCheckErrorUtil
+          errorWithFailureReason:
+              @"Unexpected top-level JSON object in initial handshake response."];
+    }
     return nil;
   }
 
   NSString *artifactBase64String = responseDict[kResponseFieldArtifact];
   if (![artifactBase64String isKindOfClass:[NSString class]]) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil
-            appAttestAttestationResponseErrorWithMissingField:kResponseFieldArtifact],
-        outError);
+    *outError = [FIRAppCheckErrorUtil
+        appAttestAttestationResponseErrorWithMissingField:kResponseFieldArtifact];
     return nil;
   }
   NSData *artifactData = [[NSData alloc] initWithBase64EncodedString:artifactBase64String
                                                              options:0];
   if (artifactData == nil) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil
-            appAttestAttestationResponseErrorWithMissingField:kResponseFieldArtifact],
-        outError);
+    *outError = [FIRAppCheckErrorUtil
+        appAttestAttestationResponseErrorWithMissingField:kResponseFieldArtifact];
     return nil;
   }
 
   NSDictionary *appCheckTokenDict = responseDict[kResponseFieldAppCheckTokenDict];
   if (![appCheckTokenDict isKindOfClass:[NSDictionary class]]) {
-    FIRAppCheckSetErrorToPointer(
-        [FIRAppCheckErrorUtil
-            appAttestAttestationResponseErrorWithMissingField:kResponseFieldAppCheckTokenDict],
-        outError);
+    *outError = [FIRAppCheckErrorUtil
+        appAttestAttestationResponseErrorWithMissingField:kResponseFieldAppCheckTokenDict];
     return nil;
   }
 
+  NSError *tokenError;
   FIRAppCheckToken *appCheckToken = [[FIRAppCheckToken alloc] initWithResponseDict:appCheckTokenDict
                                                                        requestDate:requestDate
-                                                                             error:outError];
+                                                                             error:&tokenError];
 
-  if (appCheckToken == nil) {
-    return nil;
+  if (appCheckToken) {
+    return [self initWithArtifact:artifactData token:appCheckToken];
+  } else if (tokenError) {
+    *outError = tokenError;
+  } else {
+    *outError =
+        [FIRAppCheckErrorUtil errorWithFailureReason:@"Failed to parse App Check token response."];
   }
 
-  return [self initWithArtifact:artifactData token:appCheckToken];
+  return nil;
 }
 
+NS_ASSUME_NONNULL_END
+
 @end