From cd6157797ec2ee859a903ee3f8f1102684888ad7 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Fri, 8 Mar 2024 13:31:35 -0800 Subject: [PATCH 1/3] Handle when serviceId is not present and store serviceId in keychain --- .../FirebaseAuthAuthenticationMethods.swift | 2 +- .../Models/FirebaseContext.swift | 50 +++++++++++++++---- .../xcshareddata/swiftpm/Package.resolved | 45 +++++++---------- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/Sources/SpeziFirebaseAccount/FirebaseAuthAuthenticationMethods.swift b/Sources/SpeziFirebaseAccount/FirebaseAuthAuthenticationMethods.swift index 33f9faf..f456ddf 100644 --- a/Sources/SpeziFirebaseAccount/FirebaseAuthAuthenticationMethods.swift +++ b/Sources/SpeziFirebaseAccount/FirebaseAuthAuthenticationMethods.swift @@ -8,7 +8,7 @@ /// Definition of the authentication methods supported by the FirebaseAccount module. -public struct FirebaseAuthAuthenticationMethods: OptionSet, Codable { +public struct FirebaseAuthAuthenticationMethods: OptionSet, Codable, Sendable { /// E-Mail and password-based authentication. /// /// Please follow the necessary setup steps at [Password Authentication](https://firebase.google.com/docs/auth/ios/password-auth). diff --git a/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift b/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift index 012af13..5e9e90a 100644 --- a/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift +++ b/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift @@ -128,6 +128,10 @@ actor FirebaseContext { do { try secureStorage.deleteCredentials(userId, server: server) } catch { + if let cocoaError = error as? CocoaError, + cocoaError.isFileError { + return // silence any file errors (e.g. file doesn't exist) + } Self.logger.error("Failed to remove credentials: \(error)") } } @@ -136,8 +140,13 @@ actor FirebaseContext { self.lastActiveAccountServiceId = service.id self.lastActiveAccountService = service + do { - try localStorage.store(service.id, storageKey: StorageKeys.activeAccountService) + try secureStorage.store( + credentials: Credentials(username: "_", password: service.id), + server: StorageKeys.activeAccountService, + removeDuplicate: true + ) } catch { Self.logger.error("Failed to store active account service: \(error)") } @@ -146,13 +155,24 @@ actor FirebaseContext { private func loadLastActiveAccountService() { let id: String do { - id = try localStorage.read(storageKey: StorageKeys.activeAccountService) - } catch { - if let cocoaError = error as? CocoaError, - cocoaError.isFileError { - return // silence any file errors (e.g. file doesn't exist) + let credential = try secureStorage.retrieveCredentials("_", server: StorageKeys.activeAccountService) + if let credential { + id = credential.password + } else { + // In previous versions we used the plain local storage for the active key. + do { + id = try localStorage.read(storageKey: StorageKeys.activeAccountService) + } catch { + if let cocoaError = error as? CocoaError, + cocoaError.isFileError { + return // silence any file errors (e.g. file doesn't exist) + } + Self.logger.error("Failed to read last active account service from local storage: \(error)") + return + } } - Self.logger.error("Failed to read last active account service: \(error)") + } catch { + Self.logger.error("Failed to retrieve last active account service from secure storage: \(error)") return } @@ -161,13 +181,16 @@ actor FirebaseContext { private func resetActiveAccountService() { self.lastActiveAccountService = nil - self.lastActiveAccountService = nil + self.lastActiveAccountServiceId = nil do { - try localStorage.delete(storageKey: StorageKeys.activeAccountService) + try secureStorage.deleteCredentials("_", server: StorageKeys.activeAccountService) } catch { Self.logger.error("Failed to remove active account service: \(error)") } + + // we don't care if removal of the legacy item fails + try? localStorage.delete(storageKey: StorageKeys.activeAccountService) } @@ -237,7 +260,14 @@ actor FirebaseContext { case let .user(user): let isNewUser = update.authResult?.additionalUserInfo?.isNewUser ?? false guard let service = update.service else { - Self.logger.error("Failed to dispatch user update due to missing account service!") + Self.logger.error("Failed to dispatch user update due to missing account service identifier on disk!") + do { + // This typically happens if there still is a Account associated in the Keychain but the App was recently deleted. + // Therefore, we reset the user account to allow for easily re-authenticating with firebase. + try Auth.auth().signOut() + } catch { + Self.logger.warning("Tried to remove local user. But Firebase signOut failed with \(error)") + } throw FirebaseAccountError.setupError } diff --git a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index fb334d5..0147cba 100644 --- a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -23,8 +23,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/firebase-ios-sdk", "state" : { - "revision" : "5de0369ee79ad096c164eb3afeb7921d92a43b58", - "version" : "10.18.0" + "revision" : "b880ec8ec927a838c51c12862c6222c30d7097d7", + "version" : "10.20.0" } }, { @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleAppMeasurement.git", "state" : { - "revision" : "6b332152355c372ace9966d8ee76ed191f97025e", - "version" : "10.17.0" + "revision" : "ceec9f28dea12b7cf3dabf18b5ed7621c88fd4aa", + "version" : "10.20.0" } }, { @@ -41,8 +41,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleDataTransport.git", "state" : { - "revision" : "aae45a320fd0d11811820335b1eabc8753902a40", - "version" : "9.2.5" + "revision" : "a732a4b47f59e4f725a2ea10f0c77e93a7131117", + "version" : "9.3.0" } }, { @@ -113,8 +113,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/Spezi", "state" : { - "revision" : "092eabc50a3600d8a03b43ad0d2dcd02914b223f", - "version" : "0.8.1" + "revision" : "bf2f55be72d7ab901aee009dacfab5abf7ad2103", + "version" : "1.0.0" } }, { @@ -122,8 +122,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziAccount", "state" : { - "revision" : "a9d37971e78fbdce02b0f8b57edfd2eecd4bb22e", - "version" : "0.8.1" + "revision" : "f1d92627b8e5053050c48502ccb09a827e5057f1", + "version" : "1.0.0" } }, { @@ -132,7 +132,7 @@ "location" : "https://github.com/StanfordSpezi/SpeziFoundation.git", "state" : { "revision" : "683c66f922a4cfe0882c4a86a43854f613b48541", - "version" : "0.1.0" + "version" : "1.0.0" } }, { @@ -140,17 +140,17 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziStorage", "state" : { - "revision" : "e9be3c2743e462894bf56d41339b040f4060b567", - "version" : "0.5.0" + "revision" : "eaed2220375c35400aa69d1f96a8d32b7e66b1c7", + "version" : "1.0.0" } }, { "identity" : "speziviews", "kind" : "remoteSourceControl", - "location" : "https://github.com/StanfordSpezi/SpeziViews", + "location" : "https://github.com/StanfordSpezi/SpeziViews.git", "state" : { - "revision" : "eac443080926649d09a703483a6dd6f5a8bb7d51", - "version" : "0.6.2" + "revision" : "d49f716e4a4d634604bb0dcd6d53df679b6c1358", + "version" : "1.3.0" } }, { @@ -171,22 +171,13 @@ "version" : "1.25.1" } }, - { - "identity" : "xctestextensions", - "kind" : "remoteSourceControl", - "location" : "https://github.com/StanfordSpezi/XCTestExtensions", - "state" : { - "revision" : "388a6d6a5be48eff5d98a2c45e0b50f30ed21dc3", - "version" : "0.4.7" - } - }, { "identity" : "xctruntimeassertions", "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordBDHG/XCTRuntimeAssertions", "state" : { - "revision" : "9226052589b8faece98861bc3d7b33b3ebfe4f5a", - "version" : "0.2.5" + "revision" : "bb2a287c2544aa846e53670d1ece35e5949567be", + "version" : "1.0.0" } } ], From 4294e2492dd214dc8223ab0b356dc6b1608cf1e9 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Fri, 8 Mar 2024 14:08:16 -0800 Subject: [PATCH 2/3] Better debug output --- .../SpeziFirebaseAccount/Models/FirebaseContext.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift b/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift index 5e9e90a..905d265 100644 --- a/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift +++ b/Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift @@ -58,6 +58,7 @@ actor FirebaseContext { if let lastActiveAccountServiceId, let service = registeredServices.first(where: { $0.id == lastActiveAccountServiceId }) { self.lastActiveAccountService = service + Self.logger.debug("Last active account service is \(service.id)") } // get notified about changes of the User reference @@ -127,11 +128,9 @@ actor FirebaseContext { private nonisolated func removeCredentials(userId: String, server: String) { do { try secureStorage.deleteCredentials(userId, server: server) + } catch SecureStorageError.notFound { + // we don't care if we want to delete something that doesn't exist } catch { - if let cocoaError = error as? CocoaError, - cocoaError.isFileError { - return // silence any file errors (e.g. file doesn't exist) - } Self.logger.error("Failed to remove credentials: \(error)") } } @@ -185,6 +184,8 @@ actor FirebaseContext { do { try secureStorage.deleteCredentials("_", server: StorageKeys.activeAccountService) + } catch SecureStorageError.notFound { + // we don't care if we want to delete something that doesn't exist } catch { Self.logger.error("Failed to remove active account service: \(error)") } From b1395443acc264507fc8095872d6eb92b79deca5 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Fri, 8 Mar 2024 14:38:20 -0800 Subject: [PATCH 3/3] Update dependencies --- .../FirebaseAccountStorageTests.swift | 2 - .../TestAppUITests/FirebaseAccountTests.swift | 2 - .../UITests/UITests.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 87 ++++++++++--------- 4 files changed, 49 insertions(+), 44 deletions(-) diff --git a/Tests/UITests/TestAppUITests/FirebaseAccountStorageTests.swift b/Tests/UITests/TestAppUITests/FirebaseAccountStorageTests.swift index 6aa380a..94176b2 100644 --- a/Tests/UITests/TestAppUITests/FirebaseAccountStorageTests.swift +++ b/Tests/UITests/TestAppUITests/FirebaseAccountStorageTests.swift @@ -17,8 +17,6 @@ final class FirebaseAccountStorageTests: XCTestCase { continueAfterFailure = false - try disablePasswordAutofill() - try await FirebaseClient.deleteAllAccounts() try await Task.sleep(for: .seconds(0.5)) } diff --git a/Tests/UITests/TestAppUITests/FirebaseAccountTests.swift b/Tests/UITests/TestAppUITests/FirebaseAccountTests.swift index 31a5388..d6cee27 100644 --- a/Tests/UITests/TestAppUITests/FirebaseAccountTests.swift +++ b/Tests/UITests/TestAppUITests/FirebaseAccountTests.swift @@ -21,8 +21,6 @@ final class FirebaseAccountTests: XCTestCase { // swiftlint:disable:this type_bo continueAfterFailure = false - try disablePasswordAutofill() - try await FirebaseClient.deleteAllAccounts() try await Task.sleep(for: .seconds(0.5)) } diff --git a/Tests/UITests/UITests.xcodeproj/project.pbxproj b/Tests/UITests/UITests.xcodeproj/project.pbxproj index 0b4d073..6cbb278 100644 --- a/Tests/UITests/UITests.xcodeproj/project.pbxproj +++ b/Tests/UITests/UITests.xcodeproj/project.pbxproj @@ -736,7 +736,7 @@ repositoryURL = "https://github.com/StanfordSpezi/XCTestExtensions"; requirement = { kind = upToNextMinorVersion; - minimumVersion = 0.4.7; + minimumVersion = 0.4.10; }; }; /* End XCRemoteSwiftPackageReference section */ diff --git a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 0147cba..3d698f9 100644 --- a/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Tests/UITests/UITests.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -5,8 +5,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/abseil-cpp-binary.git", "state" : { - "revision" : "bfc0b6f81adc06ce5121eb23f628473638d67c5c", - "version" : "1.2022062300.0" + "revision" : "df308b8b46607675f2b9ec8e569109008f9155ce", + "version" : "1.2022062300.1" } }, { @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/app-check.git", "state" : { - "revision" : "5746b2d35c91c50581590ed97abe4c06b5037274", - "version" : "10.18.0" + "revision" : "3e464dad87dad2d29bb29a97836789bf0f8f67d2", + "version" : "10.18.1" } }, { @@ -23,8 +23,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/firebase-ios-sdk", "state" : { - "revision" : "b880ec8ec927a838c51c12862c6222c30d7097d7", - "version" : "10.20.0" + "revision" : "be49849dcba96f2b5ee550d4eceb2c0fa27dade4", + "version" : "10.22.1" } }, { @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleAppMeasurement.git", "state" : { - "revision" : "ceec9f28dea12b7cf3dabf18b5ed7621c88fd4aa", - "version" : "10.20.0" + "revision" : "482cfa4e5880f0a29f66ecfd60c5a62af28bd1f0", + "version" : "10.22.1" } }, { @@ -41,8 +41,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleDataTransport.git", "state" : { - "revision" : "a732a4b47f59e4f725a2ea10f0c77e93a7131117", - "version" : "9.3.0" + "revision" : "a637d318ae7ae246b02d7305121275bc75ed5565", + "version" : "9.4.0" } }, { @@ -50,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/GoogleUtilities.git", "state" : { - "revision" : "bc27fad73504f3d4af235de451f02ee22586ebd3", - "version" : "7.12.1" + "revision" : "26c898aed8bed13b8a63057ee26500abbbcb8d55", + "version" : "7.13.1" } }, { @@ -59,8 +59,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/grpc-binary.git", "state" : { - "revision" : "a673bc2937fbe886dd1f99c401b01b6d977a9c98", - "version" : "1.49.1" + "revision" : "ea4cb5cc0c39c732b85386263116d2e2fdbbdc61", + "version" : "1.49.2" } }, { @@ -68,8 +68,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/gtm-session-fetcher.git", "state" : { - "revision" : "d415594121c9e8a4f9d79cecee0965cf35e74dbd", - "version" : "3.1.1" + "revision" : "76135c9f4e1ac85459d5fec61b6f76ac47ab3a4c", + "version" : "3.3.1" } }, { @@ -86,8 +86,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/leveldb.git", "state" : { - "revision" : "9d108e9112aa1d65ce508facf804674546116d9c", - "version" : "1.22.3" + "revision" : "43aaef65e0c665daadf848761d560e446d350d3d", + "version" : "1.22.4" } }, { @@ -95,8 +95,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/firebase/nanopb.git", "state" : { - "revision" : "819d0a2173aff699fb8c364b6fb906f7cdb1a692", - "version" : "2.30909.0" + "revision" : "b7e1104502eca3a213b46303391ca4d3bc8ddec1", + "version" : "2.30910.0" } }, { @@ -104,8 +104,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/google/promises.git", "state" : { - "revision" : "e70e889c0196c76d22759eb50d6a0270ca9f1d9e", - "version" : "2.3.1" + "revision" : "540318ecedd63d883069ae7f1ed811a2df00b6ac", + "version" : "2.4.0" } }, { @@ -113,8 +113,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/Spezi", "state" : { - "revision" : "bf2f55be72d7ab901aee009dacfab5abf7ad2103", - "version" : "1.0.0" + "revision" : "c43e4fa3d3938a847de2b677091a34ddaea5bc76", + "version" : "1.2.3" } }, { @@ -122,17 +122,17 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziAccount", "state" : { - "revision" : "f1d92627b8e5053050c48502ccb09a827e5057f1", - "version" : "1.0.0" + "revision" : "a7d289ef3be54de62b25dc92e8f7ff1a0f093906", + "version" : "1.2.1" } }, { "identity" : "spezifoundation", "kind" : "remoteSourceControl", - "location" : "https://github.com/StanfordSpezi/SpeziFoundation.git", + "location" : "https://github.com/StanfordSpezi/SpeziFoundation", "state" : { - "revision" : "683c66f922a4cfe0882c4a86a43854f613b48541", - "version" : "1.0.0" + "revision" : "01af5b91a54f30ddd121258e81aff2ddc2a99ff9", + "version" : "1.0.4" } }, { @@ -140,8 +140,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziStorage", "state" : { - "revision" : "eaed2220375c35400aa69d1f96a8d32b7e66b1c7", - "version" : "1.0.0" + "revision" : "b958df9b31f24800388a7bfc28f457ce7b82556c", + "version" : "1.0.2" } }, { @@ -149,8 +149,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordSpezi/SpeziViews.git", "state" : { - "revision" : "d49f716e4a4d634604bb0dcd6d53df679b6c1358", - "version" : "1.3.0" + "revision" : "4d2a724d97c8f19ac7de7aa2c046b1cb3ef7b279", + "version" : "1.3.1" } }, { @@ -158,8 +158,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-collections.git", "state" : { - "revision" : "a902f1823a7ff3c9ab2fba0f992396b948eda307", - "version" : "1.0.5" + "revision" : "94cf62b3ba8d4bed62680a282d4c25f9c63c2efb", + "version" : "1.1.0" } }, { @@ -167,8 +167,17 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-protobuf.git", "state" : { - "revision" : "07f7f26ded8df9645c072f220378879c4642e063", - "version" : "1.25.1" + "revision" : "65e8f29b2d63c4e38e736b25c27b83e012159be8", + "version" : "1.25.2" + } + }, + { + "identity" : "xctestextensions", + "kind" : "remoteSourceControl", + "location" : "https://github.com/StanfordSpezi/XCTestExtensions", + "state" : { + "revision" : "1fe9b8e76aeb7a132af37bfa0892160c9b662dcc", + "version" : "0.4.10" } }, { @@ -176,8 +185,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/StanfordBDHG/XCTRuntimeAssertions", "state" : { - "revision" : "bb2a287c2544aa846e53670d1ece35e5949567be", - "version" : "1.0.0" + "revision" : "51da3403f128b120705571ce61e0fe190f8889e6", + "version" : "1.0.1" } } ],