Delete the API token too when removing an account

InstanceListView.deleteInstance cleared only the keychain password, never the
token, so swipe-to-delete left a live API token orphaned in the keychain.
logout() already deleted both. Add KeychainManager.deleteCredentials (password
plus token) and route both logout and deleteInstance through it so the two
paths can't drift again. Adds regression tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
systemBlue 2026-06-01 17:05:09 -04:00
parent 992c628abd
commit 1b013ed100
4 changed files with 47 additions and 6 deletions

View file

@ -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,
)

View file

@ -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? {

View file

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

View file

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