From bbc0be1bc7fccb9d99cb4542f547afde7dcd0d0c Mon Sep 17 00:00:00 2001 From: Alex Nachbaur Date: Wed, 25 Sep 2024 18:49:54 -0700 Subject: [PATCH] Report any errors that occur during interactions with Credential.default --- .github/workflows/tests.yaml | 3 +- .../Internal/KeychainTokenStorage.swift | 18 ++++++---- .../Credential+Extensions.swift | 3 ++ .../Internal/CredentialCoordinatorImpl.swift | 33 +++++++++++++------ .../CredentialCoordinatorTests.swift | 13 +++++++- 5 files changed, 52 insertions(+), 18 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f0ae1add5..5b7134f21 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -21,7 +21,7 @@ on: - 'Tests/**/*.swift' env: - DEVELOPER_DIR: /Applications/Xcode_15.4.app/Contents/Developer + DEVELOPER_DIR: /Applications/Xcode_16.app/Contents/Developer NSUnbufferedIO: YES jobs: @@ -71,6 +71,7 @@ jobs: destination: - "platform=iOS Simulator,OS=16.4,name=iPhone 14 Pro Max" - "platform=iOS Simulator,OS=17.5,name=iPhone 15 Pro Max" + - "platform=iOS Simulator,OS=18.0,name=iPhone 16 Pro Max" - "platform=tvOS Simulator,OS=17.5,name=Apple TV" - "platform=visionOS Simulator,OS=1.2,name=Apple Vision Pro" - "platform=watchOS Simulator,OS=10.5,name=Apple Watch Series 7 (45mm)" diff --git a/Sources/AuthFoundation/Token Management/Internal/KeychainTokenStorage.swift b/Sources/AuthFoundation/Token Management/Internal/KeychainTokenStorage.swift index 1353eef68..6ae268ae1 100644 --- a/Sources/AuthFoundation/Token Management/Internal/KeychainTokenStorage.swift +++ b/Sources/AuthFoundation/Token Management/Internal/KeychainTokenStorage.swift @@ -26,15 +26,21 @@ final class KeychainTokenStorage: TokenStorage { weak var delegate: TokenStorageDelegate? private(set) lazy var defaultTokenID: String? = { - guard let defaultResult = try? Keychain + do { + let defaultResult = try Keychain .Search(account: KeychainTokenStorage.defaultTokenName) - .get(), - let id = String(data: defaultResult.value, encoding: .utf8) - else { + .get() + guard let id = String(data: defaultResult.value, encoding: .utf8) + else { + return nil + } + + return id + } catch { + NSLog("Unexpected error while loading the default token ID from the keychain: %@", error.localizedDescription) + NotificationCenter.default.post(name: .credentialDefaultError, object: error) return nil } - - return id }() func setDefaultTokenID(_ id: String?) throws { diff --git a/Sources/AuthFoundation/User Management/Credential+Extensions.swift b/Sources/AuthFoundation/User Management/Credential+Extensions.swift index b0bcdf5da..a49afbf4b 100644 --- a/Sources/AuthFoundation/User Management/Credential+Extensions.swift +++ b/Sources/AuthFoundation/User Management/Credential+Extensions.swift @@ -36,6 +36,9 @@ extension Notification.Name { /// Notification broadcast when a credential fails to refresh. public static let credentialRefreshFailed = Notification.Name("com.okta.credential.refresh.failed") + + /// Notification broadcast when an internal error occurs with the default credential static member. + public static let credentialDefaultError = Notification.Name("com.okta.credential.default.error") } @available(iOS 13.0, tvOS 13.0, macOS 10.15, watchOS 6, *) diff --git a/Sources/AuthFoundation/User Management/Internal/CredentialCoordinatorImpl.swift b/Sources/AuthFoundation/User Management/Internal/CredentialCoordinatorImpl.swift index 2b0fbe521..4fca1f58d 100644 --- a/Sources/AuthFoundation/User Management/Internal/CredentialCoordinatorImpl.swift +++ b/Sources/AuthFoundation/User Management/Internal/CredentialCoordinatorImpl.swift @@ -27,10 +27,15 @@ final class CredentialCoordinatorImpl: CredentialCoordinator { didSet { tokenStorage.delegate = self - _default = try? CredentialCoordinatorImpl.defaultCredential( - tokenStorage: tokenStorage, - credentialDataSource: credentialDataSource, - coordinator: self) + do { + _default = try CredentialCoordinatorImpl.defaultCredential( + tokenStorage: tokenStorage, + credentialDataSource: credentialDataSource, + coordinator: self) + } catch { + NSLog("Unexpected error when initializing the initial default credential during TokenStorage assignment: %@", error.localizedDescription) + NotificationCenter.default.post(name: .credentialDefaultError, object: error) + } } } @@ -41,19 +46,27 @@ final class CredentialCoordinatorImpl: CredentialCoordinator { credentialDataSource: credentialDataSource, coordinator: self) } catch { - // Placeholder for when logging is added in a future release + NSLog("Unexpected error when initializing the initial value for the default credential: %@", error.localizedDescription) + NotificationCenter.default.post(name: .credentialDefaultError, object: error) return nil } }() var `default`: Credential? { get { _default } set { - if let token = newValue?.token { - try? tokenStorage.add(token: token, - metadata: Token.Metadata(id: token.id), - security: Credential.Security.standard) + do { + if let token = newValue?.token, + !tokenStorage.allIDs.contains(token.id) + { + try tokenStorage.add(token: token, + metadata: Token.Metadata(id: token.id), + security: Credential.Security.standard) + } + try tokenStorage.setDefaultTokenID(newValue?.id) + } catch { + NSLog("Unexpected error while assigning a default credential: %@", error.localizedDescription) + NotificationCenter.default.post(name: .credentialDefaultError, object: error) } - try? tokenStorage.setDefaultTokenID(newValue?.id) } } diff --git a/Tests/AuthFoundationTests/CredentialCoordinatorTests.swift b/Tests/AuthFoundationTests/CredentialCoordinatorTests.swift index f85220145..de60b0140 100644 --- a/Tests/AuthFoundationTests/CredentialCoordinatorTests.swift +++ b/Tests/AuthFoundationTests/CredentialCoordinatorTests.swift @@ -73,7 +73,18 @@ final class UserCoordinatorTests: XCTestCase { XCTAssertEqual(try coordinator.with(id: token.id, prompt: nil, authenticationContext: nil), credential) } + func testDirectAssignmentToCredentialDefault() throws { + XCTAssertNil(coordinator.default) + let token = Token.simpleMockToken + coordinator.default = try coordinator.store(token: token, tags: [:], security: Credential.Security.standard) + XCTAssertNotNil(coordinator.default) + XCTAssertEqual(coordinator.default?.id, token.id) + } + func testImplicitCredentialForToken() throws { + XCTAssertTrue(storage.allIDs.isEmpty) + XCTAssertNil(coordinator.default) + let credential = try coordinator.store(token: token, tags: [:], security: []) XCTAssertEqual(storage.allIDs, [token.id]) @@ -81,7 +92,7 @@ final class UserCoordinatorTests: XCTestCase { XCTAssertEqual(coordinator.default, credential) } - func testNotifications() throws { + func testCredentialChangedNotification() throws { let oldCredential = coordinator.default let recorder = NotificationRecorder(observing: [.defaultCredentialChanged])