diff --git a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSToken.swift b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSToken.swift index 3867f661e89..3f5ad9951ae 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSToken.swift +++ b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSToken.swift @@ -15,8 +15,10 @@ #if !os(macOS) import Foundation + // TODO(ncooke3): I believe this could be made a struct now. + /// A data structure for an APNs token. - class AuthAPNSToken { + final class AuthAPNSToken: Sendable { let data: Data let type: AuthAPNSTokenType @@ -30,13 +32,13 @@ } /// The uppercase hexadecimal string form of the APNs token data. - lazy var string: String = { + var string: String { let byteArray = [UInt8](data) var s = "" for byte in byteArray { s.append(String(format: "%02X", byte)) } return s - }() + } } #endif diff --git a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift index 7ced32a77cd..f7da822342e 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift +++ b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift @@ -24,24 +24,25 @@ // Protocol to help with unit tests. protocol AuthAPNSTokenApplication { - func registerForRemoteNotifications() + @MainActor func registerForRemoteNotifications() } extension UIApplication: AuthAPNSTokenApplication {} /// A class to manage APNs token in memory. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) - class AuthAPNSTokenManager { + class AuthAPNSTokenManager: @unchecked Sendable /* TODO: sendable */ { /// The timeout for registering for remote notification. /// /// Only tests should access this property. - var timeout: TimeInterval = 5 + let timeout: TimeInterval /// Initializes the instance. /// - Parameter application: The `UIApplication` to request the token from. /// - Returns: The initialized instance. - init(withApplication application: AuthAPNSTokenApplication) { + init(withApplication application: sending AuthAPNSTokenApplication, timeout: TimeInterval = 5) { self.application = application + self.timeout = timeout } /// Attempts to get the APNs token. @@ -49,13 +50,15 @@ /// token becomes available, or when timeout occurs, whichever happens earlier. /// /// This function is internal to make visible for tests. - func getTokenInternal(callback: @escaping (Result) -> Void) { + func getTokenInternal(callback: @escaping @Sendable (Result) -> Void) { if let token = tokenStore { callback(.success(token)) return } if pendingCallbacks.count > 0 { pendingCallbacks.append(callback) + // TODO(ncooke3): This is likely a bug in that the async wrapper method + // cannot make forward progress. return } pendingCallbacks = [callback] diff --git a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenType.swift b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenType.swift index a11ab157bd4..057676003b0 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenType.swift +++ b/FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenType.swift @@ -19,7 +19,7 @@ /// /// This enum is available on iOS, macOS Catalyst, tvOS, and watchOS only. - @objc(FIRAuthAPNSTokenType) public enum AuthAPNSTokenType: Int { + @objc(FIRAuthAPNSTokenType) public enum AuthAPNSTokenType: Int, Sendable { /// Unknown token type. /// /// The actual token type will be detected from the provisioning profile in the app's bundle. diff --git a/FirebaseAuth/Sources/Swift/SystemService/AuthAppCredential.swift b/FirebaseAuth/Sources/Swift/SystemService/AuthAppCredential.swift index beb0d15a7b1..e5624eb4106 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/AuthAppCredential.swift +++ b/FirebaseAuth/Sources/Swift/SystemService/AuthAppCredential.swift @@ -16,12 +16,12 @@ import Foundation /// A class represents a credential that proves the identity of the app. @objc(FIRAuthAppCredential) // objc Needed for decoding old versions -class AuthAppCredential: NSObject, NSSecureCoding { +final class AuthAppCredential: NSObject, NSSecureCoding, Sendable { /// The server acknowledgement of receiving client's claim of identity. - var receipt: String + let receipt: String /// The secret that the client received from server via a trusted channel, if ever. - var secret: String? + let secret: String? /// Initializes the instance. /// - Parameter receipt: The server acknowledgement of receiving client's claim of identity. diff --git a/FirebaseAuth/Sources/Swift/SystemService/SecureTokenService.swift b/FirebaseAuth/Sources/Swift/SystemService/SecureTokenService.swift index a0cfa6faed0..0faa99e25f7 100644 --- a/FirebaseAuth/Sources/Swift/SystemService/SecureTokenService.swift +++ b/FirebaseAuth/Sources/Swift/SystemService/SecureTokenService.swift @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import Foundation +import FirebaseCoreInternal private let kFiveMinutes = 5 * 60.0 @@ -114,12 +114,17 @@ actor SecureTokenServiceInternal { /// A class represents a credential that proves the identity of the app. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) @objc(FIRSecureTokenService) // objc Needed for decoding old versions -class SecureTokenService: NSObject, NSSecureCoding { +final class SecureTokenService: NSObject, NSSecureCoding, Sendable { /// Internal actor to enforce serialization private let internalService: SecureTokenServiceInternal /// The configuration for making requests to server. - var requestConfiguration: AuthRequestConfiguration? + var requestConfiguration: AuthRequestConfiguration? { + get { _requestConfiguration.withLock { $0 } } + set { _requestConfiguration.withLock { $0 = newValue } } + } + + let _requestConfiguration: FIRAllocatedUnfairLock /// The cached access token. /// @@ -130,20 +135,29 @@ class SecureTokenService: NSObject, NSSecureCoding { /// - Note: The atomic wrapper can be removed when the SDK is fully /// synchronized with structured concurrency. var accessToken: String { - get { accessTokenLock.withLock { _accessToken } } - set { accessTokenLock.withLock { _accessToken = newValue } } + get { _accessToken.withLock { $0 } } + set { _accessToken.withLock { $0 = newValue } } } - private var _accessToken: String - private let accessTokenLock = NSLock() + private let _accessToken: FIRAllocatedUnfairLock /// The refresh token for the user, or `nil` if the user has yet completed sign-in flow. /// /// This property needs to be set manually after the instance is decoded from archive. - var refreshToken: String? + var refreshToken: String? { + get { _refreshToken.withLock { $0 } } + set { _refreshToken.withLock { $0 = newValue } } + } + + private let _refreshToken: FIRAllocatedUnfairLock /// The expiration date of the cached access token. - var accessTokenExpirationDate: Date? + var accessTokenExpirationDate: Date? { + get { _accessTokenExpirationDate.withLock { $0 } } + set { _accessTokenExpirationDate.withLock { $0 = newValue } } + } + + private let _accessTokenExpirationDate: FIRAllocatedUnfairLock /// Creates a `SecureTokenService` with access and refresh tokens. /// - Parameter requestConfiguration: The configuration for making requests to server. @@ -155,10 +169,10 @@ class SecureTokenService: NSObject, NSSecureCoding { accessTokenExpirationDate: Date?, refreshToken: String) { internalService = SecureTokenServiceInternal() - self.requestConfiguration = requestConfiguration - _accessToken = accessToken - self.accessTokenExpirationDate = accessTokenExpirationDate - self.refreshToken = refreshToken + _requestConfiguration = FIRAllocatedUnfairLock(initialState: requestConfiguration) + _accessToken = FIRAllocatedUnfairLock(initialState: accessToken) + _accessTokenExpirationDate = FIRAllocatedUnfairLock(initialState: accessTokenExpirationDate) + _refreshToken = FIRAllocatedUnfairLock(initialState: refreshToken) } /// Fetch a fresh ephemeral access token for the ID associated with this instance. The token diff --git a/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift b/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift index 3406149ee90..0dec3c5e4ce 100644 --- a/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift +++ b/FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift @@ -17,6 +17,7 @@ import XCTest @testable import FirebaseAuth + import FirebaseCoreInternal @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) class AuthAPNSTokenManagerTests: XCTestCase { @@ -61,10 +62,10 @@ func testCallback() throws { let expectation = self.expectation(description: #function) XCTAssertFalse(fakeApplication!.registerCalled) - var firstCallbackCalled = false + let firstCallbackCalled = FIRAllocatedUnfairLock(initialState: false) let manager = try XCTUnwrap(manager) manager.getTokenInternal { result in - firstCallbackCalled = true + firstCallbackCalled.withLock { $0 = true } switch result { case let .success(token): XCTAssertEqual(token.data, self.data) @@ -73,12 +74,12 @@ XCTFail("Unexpected error: \(error)") } } - XCTAssertFalse(firstCallbackCalled) + XCTAssertFalse(firstCallbackCalled.value()) // Add second callback, which is yet to be called either. - var secondCallbackCalled = false + let secondCallbackCalled = FIRAllocatedUnfairLock(initialState: false) manager.getTokenInternal { result in - secondCallbackCalled = true + secondCallbackCalled.withLock { $0 = true } switch result { case let .success(token): XCTAssertEqual(token.data, self.data) @@ -87,25 +88,25 @@ XCTFail("Unexpected error: \(error)") } } - XCTAssertFalse(secondCallbackCalled) + XCTAssertFalse(secondCallbackCalled.value()) // Setting nil token shouldn't trigger either callbacks. manager.token = nil - XCTAssertFalse(firstCallbackCalled) - XCTAssertFalse(secondCallbackCalled) + XCTAssertFalse(firstCallbackCalled.value()) + XCTAssertFalse(secondCallbackCalled.value()) XCTAssertNil(manager.token) // Setting a real token should trigger both callbacks. manager.token = AuthAPNSToken(withData: data!, type: .sandbox) - XCTAssertTrue(firstCallbackCalled) - XCTAssertTrue(secondCallbackCalled) + XCTAssertTrue(firstCallbackCalled.value()) + XCTAssertTrue(secondCallbackCalled.value()) XCTAssertEqual(manager.token?.data, data) XCTAssertEqual(manager.token?.type, .sandbox) // Add third callback, which should be called back immediately. - var thirdCallbackCalled = false + let thirdCallbackCalled = FIRAllocatedUnfairLock(initialState: false) manager.getTokenInternal { result in - thirdCallbackCalled = true + thirdCallbackCalled.withLock { $0 = true } switch result { case let .success(token): XCTAssertEqual(token.data, self.data) @@ -114,7 +115,7 @@ XCTFail("Unexpected error: \(error)") } } - XCTAssertTrue(thirdCallbackCalled) + XCTAssertTrue(thirdCallbackCalled.value()) // In the main thread, Verify the that the fake `registerForRemoteNotifications` was called. DispatchQueue.main.async { @@ -129,9 +130,12 @@ */ func testTimeout() throws { // Set up timeout. + manager = AuthAPNSTokenManager( + withApplication: fakeApplication!, + timeout: kRegistrationTimeout + ) let manager = try XCTUnwrap(manager) XCTAssertGreaterThan(try XCTUnwrap(manager.timeout), 0) - manager.timeout = kRegistrationTimeout // Add callback to time out. let expectation = self.expectation(description: #function) @@ -166,12 +170,15 @@ */ func testCancel() throws { // Set up timeout. + manager = AuthAPNSTokenManager( + withApplication: fakeApplication!, + timeout: kRegistrationTimeout + ) let manager = try XCTUnwrap(manager) XCTAssertGreaterThan(try XCTUnwrap(manager.timeout), 0) - manager.timeout = kRegistrationTimeout // Add callback to cancel. - var callbackCalled = false + let callbackCalled = FIRAllocatedUnfairLock(initialState: false) manager.getTokenInternal { result in switch result { case let .success(token): @@ -179,10 +186,10 @@ case let .failure(error): XCTAssertEqual(error as NSError, self.error as NSError) } - XCTAssertFalse(callbackCalled) // verify callback is not called twice - callbackCalled = true + XCTAssertFalse(callbackCalled.value()) // verify callback is not called twice + callbackCalled.withLock { $0 = true } } - XCTAssertFalse(callbackCalled) + XCTAssertFalse(callbackCalled.value()) // Call cancel. manager.cancel(withError: error) diff --git a/FirebaseAuth/Tests/Unit/AuthTests.swift b/FirebaseAuth/Tests/Unit/AuthTests.swift index dc39e1448cb..e1057f247b2 100644 --- a/FirebaseAuth/Tests/Unit/AuthTests.swift +++ b/FirebaseAuth/Tests/Unit/AuthTests.swift @@ -2291,7 +2291,7 @@ class AuthTests: RPCBaseTests { #if os(iOS) func testAppDidRegisterForRemoteNotifications_APNSTokenUpdated() { - class FakeAuthTokenManager: AuthAPNSTokenManager { + class FakeAuthTokenManager: AuthAPNSTokenManager, @unchecked Sendable { override var token: AuthAPNSToken? { get { return tokenStore @@ -2310,7 +2310,7 @@ class AuthTests: RPCBaseTests { } func testAppDidFailToRegisterForRemoteNotifications_TokenManagerCancels() { - class FakeAuthTokenManager: AuthAPNSTokenManager { + class FakeAuthTokenManager: AuthAPNSTokenManager, @unchecked Sendable { var cancelled = false override func cancel(withError error: Error) { cancelled = true diff --git a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift b/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift index 94880a3d856..e5a3e3bf766 100644 --- a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift +++ b/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift @@ -923,7 +923,7 @@ } } - class FakeTokenManager: AuthAPNSTokenManager { + class FakeTokenManager: AuthAPNSTokenManager, @unchecked Sendable { override func getTokenInternal(callback: @escaping (Result) -> Void) { let error = NSError(domain: "dummy domain", code: AuthErrorCode.missingAppToken.rawValue) callback(.failure(error))