diff --git a/FirebaseAuth/CHANGELOG.md b/FirebaseAuth/CHANGELOG.md index 66a0b4101eb..0f6d994d704 100644 --- a/FirebaseAuth/CHANGELOG.md +++ b/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` diff --git a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift index fdbe5c6bb74..49521cae813 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift +++ b/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) -> 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) -> Void] = [] - private func callback(withToken token: AuthAPNSToken?, error: Error?) { + private func callback(_ result: Result) { let pendingCallbacks = self.pendingCallbacks self.pendingCallbacks = [] for callback in pendingCallbacks { - callback(token, error) + callback(result) } } diff --git a/FirebaseAuth/Tests/SampleSwift/AuthenticationExample/ViewControllers/AuthViewController.swift b/FirebaseAuth/Tests/SampleSwift/AuthenticationExample/ViewControllers/AuthViewController.swift index 78b0295edb8..fdde2e0757d 100644 --- a/FirebaseAuth/Tests/SampleSwift/AuthenticationExample/ViewControllers/AuthViewController.swift +++ b/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 ) diff --git a/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift b/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift index 585cd217b98..3406149ee90 100644 --- a/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift +++ b/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 } diff --git a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift b/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift index 9b1e1caaf30..0030c52776d 100644 --- a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift +++ b/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) -> Void) { let error = NSError(domain: "dummy domain", code: AuthErrorCode.missingAppToken.rawValue) - callback(nil, error) + callback(.failure(error)) } }