Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle when serviceId is not present and store serviceId in keychain #28

Merged
merged 3 commits into from
Mar 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
51 changes: 41 additions & 10 deletions Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
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
Expand Down Expand Up @@ -127,6 +128,8 @@
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 {
Self.logger.error("Failed to remove credentials: \(error)")
}
Expand All @@ -136,8 +139,13 @@
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)")
}
Expand All @@ -146,13 +154,24 @@
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

Check warning on line 170 in Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift#L169-L170

Added lines #L169 - L170 were not covered by tests
}
}
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)")

Check warning on line 174 in Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift#L174

Added line #L174 was not covered by tests
return
}

Expand All @@ -161,13 +180,18 @@

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 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)")
}

// we don't care if removal of the legacy item fails
try? localStorage.delete(storageKey: StorageKeys.activeAccountService)
}


Expand Down Expand Up @@ -237,7 +261,14 @@
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)")

Check warning on line 270 in Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziFirebaseAccount/Models/FirebaseContext.swift#L270

Added line #L270 was not covered by tests
}
throw FirebaseAccountError.setupError
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ final class FirebaseAccountStorageTests: XCTestCase {

continueAfterFailure = false

try disablePasswordAutofill()

try await FirebaseClient.deleteAllAccounts()
try await Task.sleep(for: .seconds(0.5))
}
Expand Down
2 changes: 0 additions & 2 deletions Tests/UITests/TestAppUITests/FirebaseAccountTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/UITests/UITests.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@
repositoryURL = "https://github.com/StanfordSpezi/XCTestExtensions";
requirement = {
kind = upToNextMinorVersion;
minimumVersion = 0.4.7;
minimumVersion = 0.4.10;
};
};
/* End XCRemoteSwiftPackageReference section */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,71 +5,71 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/abseil-cpp-binary.git",
"state" : {
"revision" : "bfc0b6f81adc06ce5121eb23f628473638d67c5c",
"version" : "1.2022062300.0"
"revision" : "df308b8b46607675f2b9ec8e569109008f9155ce",
"version" : "1.2022062300.1"
}
},
{
"identity" : "app-check",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/app-check.git",
"state" : {
"revision" : "5746b2d35c91c50581590ed97abe4c06b5037274",
"version" : "10.18.0"
"revision" : "3e464dad87dad2d29bb29a97836789bf0f8f67d2",
"version" : "10.18.1"
}
},
{
"identity" : "firebase-ios-sdk",
"kind" : "remoteSourceControl",
"location" : "https://github.com/firebase/firebase-ios-sdk",
"state" : {
"revision" : "5de0369ee79ad096c164eb3afeb7921d92a43b58",
"version" : "10.18.0"
"revision" : "be49849dcba96f2b5ee550d4eceb2c0fa27dade4",
"version" : "10.22.1"
}
},
{
"identity" : "googleappmeasurement",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/GoogleAppMeasurement.git",
"state" : {
"revision" : "6b332152355c372ace9966d8ee76ed191f97025e",
"version" : "10.17.0"
"revision" : "482cfa4e5880f0a29f66ecfd60c5a62af28bd1f0",
"version" : "10.22.1"
}
},
{
"identity" : "googledatatransport",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/GoogleDataTransport.git",
"state" : {
"revision" : "aae45a320fd0d11811820335b1eabc8753902a40",
"version" : "9.2.5"
"revision" : "a637d318ae7ae246b02d7305121275bc75ed5565",
"version" : "9.4.0"
}
},
{
"identity" : "googleutilities",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/GoogleUtilities.git",
"state" : {
"revision" : "bc27fad73504f3d4af235de451f02ee22586ebd3",
"version" : "7.12.1"
"revision" : "26c898aed8bed13b8a63057ee26500abbbcb8d55",
"version" : "7.13.1"
}
},
{
"identity" : "grpc-binary",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/grpc-binary.git",
"state" : {
"revision" : "a673bc2937fbe886dd1f99c401b01b6d977a9c98",
"version" : "1.49.1"
"revision" : "ea4cb5cc0c39c732b85386263116d2e2fdbbdc61",
"version" : "1.49.2"
}
},
{
"identity" : "gtm-session-fetcher",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/gtm-session-fetcher.git",
"state" : {
"revision" : "d415594121c9e8a4f9d79cecee0965cf35e74dbd",
"version" : "3.1.1"
"revision" : "76135c9f4e1ac85459d5fec61b6f76ac47ab3a4c",
"version" : "3.3.1"
}
},
{
Expand All @@ -86,107 +86,107 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/firebase/leveldb.git",
"state" : {
"revision" : "9d108e9112aa1d65ce508facf804674546116d9c",
"version" : "1.22.3"
"revision" : "43aaef65e0c665daadf848761d560e446d350d3d",
"version" : "1.22.4"
}
},
{
"identity" : "nanopb",
"kind" : "remoteSourceControl",
"location" : "https://github.com/firebase/nanopb.git",
"state" : {
"revision" : "819d0a2173aff699fb8c364b6fb906f7cdb1a692",
"version" : "2.30909.0"
"revision" : "b7e1104502eca3a213b46303391ca4d3bc8ddec1",
"version" : "2.30910.0"
}
},
{
"identity" : "promises",
"kind" : "remoteSourceControl",
"location" : "https://github.com/google/promises.git",
"state" : {
"revision" : "e70e889c0196c76d22759eb50d6a0270ca9f1d9e",
"version" : "2.3.1"
"revision" : "540318ecedd63d883069ae7f1ed811a2df00b6ac",
"version" : "2.4.0"
}
},
{
"identity" : "spezi",
"kind" : "remoteSourceControl",
"location" : "https://github.com/StanfordSpezi/Spezi",
"state" : {
"revision" : "092eabc50a3600d8a03b43ad0d2dcd02914b223f",
"version" : "0.8.1"
"revision" : "c43e4fa3d3938a847de2b677091a34ddaea5bc76",
"version" : "1.2.3"
}
},
{
"identity" : "speziaccount",
"kind" : "remoteSourceControl",
"location" : "https://github.com/StanfordSpezi/SpeziAccount",
"state" : {
"revision" : "a9d37971e78fbdce02b0f8b57edfd2eecd4bb22e",
"version" : "0.8.1"
"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" : "0.1.0"
"revision" : "01af5b91a54f30ddd121258e81aff2ddc2a99ff9",
"version" : "1.0.4"
}
},
{
"identity" : "spezistorage",
"kind" : "remoteSourceControl",
"location" : "https://github.com/StanfordSpezi/SpeziStorage",
"state" : {
"revision" : "e9be3c2743e462894bf56d41339b040f4060b567",
"version" : "0.5.0"
"revision" : "b958df9b31f24800388a7bfc28f457ce7b82556c",
"version" : "1.0.2"
}
},
{
"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" : "4d2a724d97c8f19ac7de7aa2c046b1cb3ef7b279",
"version" : "1.3.1"
}
},
{
"identity" : "swift-collections",
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-collections.git",
"state" : {
"revision" : "a902f1823a7ff3c9ab2fba0f992396b948eda307",
"version" : "1.0.5"
"revision" : "94cf62b3ba8d4bed62680a282d4c25f9c63c2efb",
"version" : "1.1.0"
}
},
{
"identity" : "swift-protobuf",
"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" : "388a6d6a5be48eff5d98a2c45e0b50f30ed21dc3",
"version" : "0.4.7"
"revision" : "1fe9b8e76aeb7a132af37bfa0892160c9b662dcc",
"version" : "0.4.10"
}
},
{
"identity" : "xctruntimeassertions",
"kind" : "remoteSourceControl",
"location" : "https://github.com/StanfordBDHG/XCTRuntimeAssertions",
"state" : {
"revision" : "9226052589b8faece98861bc3d7b33b3ebfe4f5a",
"version" : "0.2.5"
"revision" : "51da3403f128b120705571ce61e0fe190f8889e6",
"version" : "1.0.1"
}
}
],
Expand Down
Loading