[Auth] Fix async/await crash from implicitly unwrapped nil error (#13472)

Author: ncooke3

FirebaseAuth/CHANGELOG.md

@@ -3,6 +3,8 @@
 - [added] Added custom provider support to `AuthProviderID`. Note that this change will be breaking
   to any code that implemented an exhaustive `switch` on `AuthProviderID` in 11.0.0 - the `switch`
   will need expansion. (#13429)
+- [fixed] Fix crash introduced in 11.0.0 in phone authentication flow from
+  implicitly unwrapping `nil` error after a token timeout. (#13470)
 
 # 11.0.0
 - [fixed] Fixed auth domain matching code to prioritize matching `firebaseapp.com` over `web.app`

FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift

@@ -49,9 +49,9 @@
     /// token becomes available, or when timeout occurs, whichever happens earlier.
     ///
     /// This function is internal to make visible for tests.
-    func getTokenInternal(callback: @escaping (AuthAPNSToken?, Error?) -> Void) {
+    func getTokenInternal(callback: @escaping (Result<AuthAPNSToken, Error>) -> Void) {
       if let token = tokenStore {
-        callback(token, nil)
+        callback(.success(token))
         return
       }
       if pendingCallbacks.count > 0 {
@@ -68,18 +68,19 @@
       kAuthGlobalWorkQueue.asyncAfter(deadline: deadline) {
         // Only cancel if the pending callbacks remain the same, i.e., not triggered yet.
         if applicableCallbacks.count == self.pendingCallbacks.count {
-          self.callback(withToken: nil, error: nil)
+          self.callback(.failure(AuthErrorUtils.missingAppTokenError(underlyingError: nil)))
         }
       }
     }
 
     func getToken() async throws -> AuthAPNSToken {
       return try await withCheckedThrowingContinuation { continuation in
-        self.getTokenInternal { token, error in
-          if let token {
+        self.getTokenInternal { result in
+          switch result {
+          case let .success(token):
             continuation.resume(returning: token)
-          } else {
-            continuation.resume(throwing: error!)
+          case let .failure(error):
+            continuation.resume(throwing: error)
           }
         }
       }
@@ -105,7 +106,7 @@
           newToken = AuthAPNSToken(withData: setToken.data, type: detectedTokenType)
         }
         tokenStore = newToken
-        callback(withToken: newToken, error: nil)
+        callback(.success(newToken))
       }
     }
 
@@ -115,18 +116,18 @@
     /// Cancels any pending `getTokenWithCallback:` request.
     /// - Parameter error: The error to return .
     func cancel(withError error: Error) {
-      callback(withToken: nil, error: error)
+      callback(.failure(error))
     }
 
     /// Enable unit test faking.
     var application: AuthAPNSTokenApplication
-    private var pendingCallbacks: [(AuthAPNSToken?, Error?) -> Void] = []
+    private var pendingCallbacks: [(Result<AuthAPNSToken, Error>) -> Void] = []
 
-    private func callback(withToken token: AuthAPNSToken?, error: Error?) {
+    private func callback(_ result: Result<AuthAPNSToken, Error>) {
       let pendingCallbacks = self.pendingCallbacks
       self.pendingCallbacks = []
       for callback in pendingCallbacks {
-        callback(token, error)
+        callback(result)
       }
     }
 

FirebaseAuth/Tests/SampleSwift/AuthenticationExample/ViewControllers/AuthViewController.swift

@@ -468,14 +468,14 @@ class AuthViewController: UIViewController, DataSourceProviderDelegate {
   }
 
   private func verifyClient() {
-    AppManager.shared.auth().tokenManager.getTokenInternal { token, error in
-      if token == nil {
+    AppManager.shared.auth().tokenManager.getTokenInternal { result in
+      guard case let .success(token) = result else {
         print("Verify iOS Client failed.")
         return
       }
       let request = VerifyClientRequest(
-        withAppToken: token?.string,
-        isSandbox: token?.type == .sandbox,
+        withAppToken: token.string,
+        isSandbox: token.type == .sandbox,
         requestConfiguration: AppManager.shared.auth().requestConfiguration
       )
 

FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift

@@ -63,21 +63,29 @@
       XCTAssertFalse(fakeApplication!.registerCalled)
       var firstCallbackCalled = false
       let manager = try XCTUnwrap(manager)
-      manager.getTokenInternal { token, error in
+      manager.getTokenInternal { result in
         firstCallbackCalled = true
-        XCTAssertEqual(token?.data, self.data)
-        XCTAssertEqual(token?.type, .sandbox)
-        XCTAssertNil(error)
+        switch result {
+        case let .success(token):
+          XCTAssertEqual(token.data, self.data)
+          XCTAssertEqual(token.type, .sandbox)
+        case let .failure(error):
+          XCTFail("Unexpected error: \(error)")
+        }
       }
       XCTAssertFalse(firstCallbackCalled)
 
       // Add second callback, which is yet to be called either.
       var secondCallbackCalled = false
-      manager.getTokenInternal { token, error in
+      manager.getTokenInternal { result in
         secondCallbackCalled = true
-        XCTAssertEqual(token?.data, self.data)
-        XCTAssertEqual(token?.type, .sandbox)
-        XCTAssertNil(error)
+        switch result {
+        case let .success(token):
+          XCTAssertEqual(token.data, self.data)
+          XCTAssertEqual(token.type, .sandbox)
+        case let .failure(error):
+          XCTFail("Unexpected error: \(error)")
+        }
       }
       XCTAssertFalse(secondCallbackCalled)
 
@@ -96,11 +104,15 @@
 
       // Add third callback, which should be called back immediately.
       var thirdCallbackCalled = false
-      manager.getTokenInternal { token, error in
+      manager.getTokenInternal { result in
         thirdCallbackCalled = true
-        XCTAssertEqual(token?.data, self.data)
-        XCTAssertEqual(token?.type, .sandbox)
-        XCTAssertNil(error)
+        switch result {
+        case let .success(token):
+          XCTAssertEqual(token.data, self.data)
+          XCTAssertEqual(token.type, .sandbox)
+        case let .failure(error):
+          XCTFail("Unexpected error: \(error)")
+        }
       }
       XCTAssertTrue(thirdCallbackCalled)
 
@@ -123,9 +135,16 @@
 
       // Add callback to time out.
       let expectation = self.expectation(description: #function)
-      manager.getTokenInternal { token, error in
-        XCTAssertNil(token)
-        XCTAssertNil(error)
+      manager.getTokenInternal { result in
+        switch result {
+        case let .success(token):
+          XCTFail("Unexpected success: \(token)")
+        case let .failure(error):
+          XCTAssertEqual(
+            error as NSError,
+            AuthErrorUtils.missingAppTokenError(underlyingError: nil) as NSError
+          )
+        }
         expectation.fulfill()
       }
       // Time out.
@@ -153,9 +172,13 @@
 
       // Add callback to cancel.
       var callbackCalled = false
-      manager.getTokenInternal { token, error in
-        XCTAssertNil(token)
-        XCTAssertEqual(error as? NSError, self.error as NSError)
+      manager.getTokenInternal { result in
+        switch result {
+        case let .success(token):
+          XCTFail("Unexpected success: \(token)")
+        case let .failure(error):
+          XCTAssertEqual(error as NSError, self.error as NSError)
+        }
         XCTAssertFalse(callbackCalled) // verify callback is not called twice
         callbackCalled = true
       }

FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift

@@ -690,9 +690,9 @@
     }
 
     class FakeTokenManager: AuthAPNSTokenManager {
-      override func getTokenInternal(callback: @escaping (AuthAPNSToken?, Error?) -> Void) {
+      override func getTokenInternal(callback: @escaping (Result<AuthAPNSToken, Error>) -> Void) {
         let error = NSError(domain: "dummy domain", code: AuthErrorCode.missingAppToken.rawValue)
-        callback(nil, error)
+        callback(.failure(error))
       }
     }