diff --git a/Forji/Forji/Services/AuthenticationService.swift b/Forji/Forji/Services/AuthenticationService.swift index 39948f1..b506ada 100644 --- a/Forji/Forji/Services/AuthenticationService.swift +++ b/Forji/Forji/Services/AuthenticationService.swift @@ -99,11 +99,7 @@ class AuthenticationService { if let instance = currentInstance { let normalizedURL = ForgejoClient.normalizeServerURL(instance.serverURL) do { - try await KeychainManager.shared.deletePassword( - for: normalizedURL, - username: instance.username, - ) - try await KeychainManager.shared.deleteToken( + try await KeychainManager.shared.deleteCredentials( for: normalizedURL, username: instance.username, ) diff --git a/Forji/Forji/Services/KeychainManager.swift b/Forji/Forji/Services/KeychainManager.swift index 7775751..c4e8e69 100644 --- a/Forji/Forji/Services/KeychainManager.swift +++ b/Forji/Forji/Services/KeychainManager.swift @@ -40,6 +40,15 @@ actor KeychainManager { try deleteItem(forKey: key(for: server, username: username, suffix: "_token")) } + // MARK: - Account (both items) + + /// Removes every credential stored for an account: the password and the API token. + /// Use when an account is removed entirely (logout, delete instance) so neither item is orphaned. + func deleteCredentials(for server: String, username: String) throws { + try deletePassword(for: server, username: username) + try deleteToken(for: server, username: username) + } + // MARK: - Synchronous token read (no actor hop) nonisolated static func getTokenSync(for server: String, username: String) -> String? { diff --git a/Forji/Forji/Views/InstanceListView.swift b/Forji/Forji/Views/InstanceListView.swift index 5e8a31d..588917c 100644 --- a/Forji/Forji/Views/InstanceListView.swift +++ b/Forji/Forji/Views/InstanceListView.swift @@ -223,7 +223,7 @@ struct InstanceListView: View { assertionFailure("SwiftData save failed: \(error)") } Task { - try? await KeychainManager.shared.deletePassword(for: normalizedURL, username: username) + try? await KeychainManager.shared.deleteCredentials(for: normalizedURL, username: username) } } diff --git a/Forji/ForjiTests/KeychainManagerTests.swift b/Forji/ForjiTests/KeychainManagerTests.swift index db8dd1d..9ce5324 100644 --- a/Forji/ForjiTests/KeychainManagerTests.swift +++ b/Forji/ForjiTests/KeychainManagerTests.swift @@ -133,4 +133,40 @@ struct KeychainManagerTests { // Clean up try await KeychainManager.shared.deletePassword(for: normalized, username: "user") } + + @Test func keychainDeleteCredentialsRemovesBothPasswordAndToken() async throws { + let server = "https://delete-credentials.example.com" + let username = "user" + + try await KeychainManager.shared.savePassword("mypassword", for: server, username: username) + try await KeychainManager.shared.saveToken("mytoken", for: server, username: username) + + try await KeychainManager.shared.deleteCredentials(for: server, username: username) + + do { + _ = try await KeychainManager.shared.getPassword(for: server, username: username) + Issue.record("Expected KeychainError.notFound for password after deleteCredentials") + } catch is KeychainError { + // Expected + } catch { + Issue.record("Unexpected error: \(error)") + } + + do { + _ = try await KeychainManager.shared.getToken(for: server, username: username) + Issue.record("Expected KeychainError.notFound for token after deleteCredentials") + } catch is KeychainError { + // Expected — this is the assertion that fails under the old leak + } catch { + Issue.record("Unexpected error: \(error)") + } + } + + @Test func keychainDeleteCredentialsNonExistentDoesNotThrow() async throws { + // Deleting credentials for an account with nothing stored must not throw. + try await KeychainManager.shared.deleteCredentials( + for: "https://nonexistent-credentials.example.com", + username: "nobody" + ) + } }