From 04e48140b55fda48f05a367e5a5df35c581898d9 Mon Sep 17 00:00:00 2001 From: Damir Garifullin Date: Sat, 24 Oct 2026 13:50:01 +0300 Subject: [PATCH] Prevent repeated login prompts after authentication dismissal --- .../Clavis/State/PanelLockController.swift | 61 ++++-- .../State/PanelUnlockPromptLifecycle.swift | 48 +++++ Sources/Clavis/Windows/WindowManager.swift | 7 +- Sources/ClavisCore/Audit/AuditEvent.swift | 2 + .../ClavisCore/Crypto/UserAuthenticator.swift | 13 ++ Tests/ClavisAppTests/PanelLockTests.swift | 7 +- .../PanelUnlockPromptLifecycleTests.swift | 200 ++++++++++++++++++ 7 files changed, 313 insertions(+), 25 deletions(-) create mode 100644 Sources/Clavis/State/PanelUnlockPromptLifecycle.swift create mode 100644 Tests/ClavisAppTests/PanelUnlockPromptLifecycleTests.swift diff --git a/Sources/Clavis/State/PanelLockController.swift b/Sources/Clavis/State/PanelLockController.swift index 993785b..61721b4 100644 --- a/Sources/Clavis/State/PanelLockController.swift +++ b/Sources/Clavis/State/PanelLockController.swift @@ -21,6 +21,11 @@ public final class LAPanelAuthenticator: PanelAuthenticating { public init() {} public func authenticate(reason: String) async throws -> LAContext { + #if DEBUG + guard !ClavisEnvironment.isTesting else { + throw UserAuthenticationError.testAuthenticationNotInjected + } + #endif ClavisLogger.promptDebug("calvis-ui", "LAPanelAuthenticator: prompting system authentication (reason: \"\(reason)\")") let context = LAContext() return try await withCheckedThrowingContinuation { continuation in @@ -215,40 +220,40 @@ public final class PanelLockController: ObservableObject { return } + let generation = promptLifecycle.beganManualAttempt() isUnlocking = true defer { isUnlocking = false } do { ClavisLogger.promptDebug("calvis-ui", "PanelLockController: requesting panel unlock prompt: \"\(ClavisUIStrings.PanelLock.reason)\"") let context = try await authenticator.authenticate(reason: ClavisUIStrings.PanelLock.reason) + guard promptLifecycle.acceptsCompletion(from: generation) else { + context.invalidate() + return + } applyUnlock(context: context) } catch { - // Cancelled or failed unlock stays locked without error alert - lastPromptDismissedAt = now() + // Cancellation stays locked. Focus restoration is not a retry. } } - private var lastPromptDismissedAt: Date? - - /// Prompts for biometric / system authentication when the user opens or focuses the locked window, - /// matching Bitwarden's auto-prompt behavior. - /// - Parameter isWindowFocusEvent: When true, suppresses immediate re-prompting right after the macOS - /// system authentication sheet closes and returns key-window status to the Clavis window. - public func unlockIfNeededOnWindowFocus(isWindowFocusEvent: Bool = false) async { - guard isEffectivelyLocked, !isUnlocking else { return } - if isWindowFocusEvent, - let dismissedAt = lastPromptDismissedAt, - now().timeIntervalSince(dismissedAt) < 1.0 { - return - } + private var promptLifecycle = PanelUnlockPromptLifecycle() + + /// One automatic attempt per locked period; explicit reopening may retry. + public func unlockIfNeededOnWindowFocus( + isWindowFocusEvent: Bool = false, + isUserInitiatedOpen: Bool = false + ) async { + let trigger: PanelUnlockPromptLifecycle.Trigger = isUserInitiatedOpen + ? .explicitOpen : (isWindowFocusEvent ? .windowFocus : .presentation) + guard promptLifecycle.claimAutomaticAttempt( + trigger: trigger, isLocked: isEffectivelyLocked, isAuthenticating: isUnlocking + ) else { return } switch currentMode { case .passwordOrBiometry: await unlock() case .biometryOrPIN: - let outcome = await unlockWithPIN(nil) - if outcome != .unlocked { - lastPromptDismissedAt = now() - } + _ = await unlockWithPIN(nil) case .biometryAndPIN: // Requires PIN entry first break @@ -258,7 +263,12 @@ public final class PanelLockController: ObservableObject { @discardableResult public func authenticateUser(reason: String = ClavisUIStrings.PanelLock.reason) async throws -> LAContext { ClavisLogger.promptDebug("calvis-ui", "PanelLockController: authenticating user: \"\(reason)\"") + let generation = promptLifecycle.beganManualAttempt() let context = try await authenticator.authenticate(reason: reason) + guard promptLifecycle.acceptsCompletion(from: generation) else { + context.invalidate() + throw LAError(.appCancel) + } applyUnlock(context: context) return context } @@ -272,7 +282,12 @@ public final class PanelLockController: ObservableObject { isUnlocking = true defer { isUnlocking = false } + let generation = promptLifecycle.beganManualAttempt() let (outcome, context) = await service.unlock(pin: pin) + guard promptLifecycle.acceptsCompletion(from: generation) else { + context?.invalidate() + return .cancelled + } if outcome == .unlocked { applyUnlock(context: context) } @@ -288,7 +303,12 @@ public final class PanelLockController: ObservableObject { isUnlocking = true defer { isUnlocking = false } + let generation = promptLifecycle.beganManualAttempt() let (outcome, context) = await service.unlockWithPassword() + guard promptLifecycle.acceptsCompletion(from: generation) else { + context?.invalidate() + return .cancelled + } if outcome == .unlocked { applyUnlock(context: context) } @@ -310,11 +330,13 @@ public final class PanelLockController: ObservableObject { defer { isUnlocking = false } ClavisLogger.promptDebug("calvis-ui", "PanelLockController: verifying Mac login password via OpenDirectory") + let generation = promptLifecycle.beganManualAttempt() let verifier = self.loginPasswordVerifier let verified = await Task.detached { verifier(password) }.value + guard promptLifecycle.acceptsCompletion(from: generation) else { return false } if verified { applyUnlock(context: nil) return true @@ -347,6 +369,7 @@ public final class PanelLockController: ObservableObject { } public func lock(reason: PanelLockReason) { + promptLifecycle.didLock(wasAlreadyLocked: isLocked, suppressAutomaticAttempt: reason == .lockNow || reason == .idle) lastLockReason = reason unlockContext?.invalidate() unlockContext = nil diff --git a/Sources/Clavis/State/PanelUnlockPromptLifecycle.swift b/Sources/Clavis/State/PanelUnlockPromptLifecycle.swift new file mode 100644 index 0000000..4a5f373 --- /dev/null +++ b/Sources/Clavis/State/PanelUnlockPromptLifecycle.swift @@ -0,0 +1,48 @@ +import Foundation + +/// Authentication-sheet dismissal returns focus to the locked window. That +/// focus is not a new request to authenticate, regardless of elapsed time. +struct PanelUnlockPromptLifecycle { + enum Trigger { + case presentation + case windowFocus + case explicitOpen + } + + private(set) var generation = UUID() + private(set) var automaticAttemptConsumed = false + + mutating func claimAutomaticAttempt( + trigger: Trigger, + isLocked: Bool, + isAuthenticating: Bool + ) -> Bool { + guard isLocked, !isAuthenticating else { return false } + if trigger == .explicitOpen { + automaticAttemptConsumed = false + } + guard !automaticAttemptConsumed else { return false } + automaticAttemptConsumed = true + return true + } + + /// Manual attempts remain available, but their dismissal must not start an + /// additional automatic attempt when the window becomes key again. + mutating func beganManualAttempt() -> UUID { + automaticAttemptConsumed = true + return generation + } + + mutating func didLock(wasAlreadyLocked: Bool, suppressAutomaticAttempt: Bool = false) { + generation = UUID() + if suppressAutomaticAttempt { + automaticAttemptConsumed = true + } else if !wasAlreadyLocked { + automaticAttemptConsumed = false + } + } + + func acceptsCompletion(from generation: UUID) -> Bool { + self.generation == generation + } +} diff --git a/Sources/Clavis/Windows/WindowManager.swift b/Sources/Clavis/Windows/WindowManager.swift index 27de6be..eb213c5 100644 --- a/Sources/Clavis/Windows/WindowManager.swift +++ b/Sources/Clavis/Windows/WindowManager.swift @@ -67,13 +67,10 @@ public class WindowManager: NSObject, NSWindowDelegate { } if let window = keyManagerWindow { - let wasKeyWindow = window.isKeyWindow && (NSApp?.isActive == true) window.makeKeyAndOrderFront(nil) NSApp?.activate(ignoringOtherApps: true) - if wasKeyWindow { - Task { - await PanelLockController.shared.unlockIfNeededOnWindowFocus() - } + Task { + await PanelLockController.shared.unlockIfNeededOnWindowFocus(isUserInitiatedOpen: true) } return } diff --git a/Sources/ClavisCore/Audit/AuditEvent.swift b/Sources/ClavisCore/Audit/AuditEvent.swift index 7c0675e..68aeb9f 100644 --- a/Sources/ClavisCore/Audit/AuditEvent.swift +++ b/Sources/ClavisCore/Audit/AuditEvent.swift @@ -156,6 +156,8 @@ public struct AuditEvent: Equatable, Sendable { public static func isUserCancellation(_ error: Error) -> Bool { if let authError = error as? UserAuthenticationError { switch authError { + case .testAuthenticationNotInjected: + return false case .timedOut: return true case .rejected(let underlying): diff --git a/Sources/ClavisCore/Crypto/UserAuthenticator.swift b/Sources/ClavisCore/Crypto/UserAuthenticator.swift index 07e709f..a7150d4 100644 --- a/Sources/ClavisCore/Crypto/UserAuthenticator.swift +++ b/Sources/ClavisCore/Crypto/UserAuthenticator.swift @@ -23,10 +23,13 @@ public extension UserAuthenticating { public enum UserAuthenticationError: LocalizedError { case timedOut + case testAuthenticationNotInjected case rejected(Error?) public var errorDescription: String? { switch self { + case .testAuthenticationNotInjected: + return "Tests must inject an authenticator; system authentication is unavailable in tests." case .timedOut: return ClavisUIStrings.Auth.errorTimedOut case .rejected(let error): @@ -40,6 +43,11 @@ public final class LocalUserAuthenticator: UserAuthenticating { @discardableResult public func authenticate(reason: String, policy: LAPolicy = .deviceOwnerAuthentication) throws -> LAContext { + #if DEBUG + guard !ClavisEnvironment.isTesting else { + throw UserAuthenticationError.testAuthenticationNotInjected + } + #endif ClavisLogger.promptDebug("clavis-code", "LocalUserAuthenticator: prompting system authentication (policy: \(policy), reason: \"\(reason)\")") let context = LAContext() context.localizedReason = reason @@ -79,6 +87,11 @@ public final class LocalUserAuthenticator: UserAuthenticating { @discardableResult public func authenticate(reason: String, policy: LAPolicy = .deviceOwnerAuthentication) async throws -> LAContext { + #if DEBUG + guard !ClavisEnvironment.isTesting else { + throw UserAuthenticationError.testAuthenticationNotInjected + } + #endif ClavisLogger.promptDebug("clavis-code", "LocalUserAuthenticator (async): prompting system authentication (policy: \(policy), reason: \"\(reason)\")") let context = LAContext() context.localizedReason = reason diff --git a/Tests/ClavisAppTests/PanelLockTests.swift b/Tests/ClavisAppTests/PanelLockTests.swift index f9542d3..9105739 100644 --- a/Tests/ClavisAppTests/PanelLockTests.swift +++ b/Tests/ClavisAppTests/PanelLockTests.swift @@ -290,10 +290,15 @@ final class PanelLockTests { await controller.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) #expect(fakeAuth.authCount == 1) - // Focusing the locked window again after cooldown prompts Touch ID and unlocks when approved + // Elapsed time does not turn restored focus into consent to retry. currentTime += 2.0 fakeAuth.shouldFail = false await controller.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) + #expect(fakeAuth.authCount == 1) + #expect(controller.isEffectivelyLocked) + + // Explicitly opening Clavis again is a deliberate retry. + await controller.unlockIfNeededOnWindowFocus(isUserInitiatedOpen: true) #expect(fakeAuth.authCount == 2) #expect(!controller.isEffectivelyLocked) } diff --git a/Tests/ClavisAppTests/PanelUnlockPromptLifecycleTests.swift b/Tests/ClavisAppTests/PanelUnlockPromptLifecycleTests.swift new file mode 100644 index 0000000..7c2e282 --- /dev/null +++ b/Tests/ClavisAppTests/PanelUnlockPromptLifecycleTests.swift @@ -0,0 +1,200 @@ +import Foundation +import LocalAuthentication +import Testing +@testable import Clavis +@testable import ClavisCore +@testable import ClavisTestSupport + +@Suite @MainActor +struct PanelUnlockPromptLifecycleTests { + private func controller(auth: PanelAuthenticating, now: @escaping () -> Date = Date.init) -> (PanelLockController, UserDefaults, String) { + let name = "clavis.tests.prompt-lifecycle.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: name)! + let controller = PanelLockController( + defaults: defaults, authenticator: auth, keyring: nil, + recorder: InMemoryAuditRecorder(), now: now, vaultKeyCount: { 1 } + ) + return (controller, defaults, name) + } + + @Test func productionAuthenticationAdaptersCannotPromptFromTests() async throws { + try #require(ClavisEnvironment.isTesting) + let core = LocalUserAuthenticator() + let synchronous: () throws -> LAContext = { try core.authenticate(reason: "Must not prompt") } + #expect(throws: (any Error).self) { try synchronous() } + await #expect(throws: (any Error).self) { + try await core.authenticate(reason: "Must not prompt") + } + await #expect(throws: (any Error).self) { + try await LAPanelAuthenticator().authenticate(reason: "Must not prompt") + } + } + + @Test func initialPresentationClaimsExactlyOneAttempt() { + var gate = PanelUnlockPromptLifecycle() + let claimed1 = gate.claimAutomaticAttempt(trigger: .presentation, isLocked: true, isAuthenticating: false) + #expect(claimed1) + for _ in 0..<100 { + let claimed2 = gate.claimAutomaticAttempt(trigger: .windowFocus, isLocked: true, isAuthenticating: false) + #expect(!claimed2) + let claimed3 = gate.claimAutomaticAttempt(trigger: .presentation, isLocked: true, isAuthenticating: false) + #expect(!claimed3) + } + } + + @Test func explicitReopenRetriesButFocusDoesNot() { + var gate = PanelUnlockPromptLifecycle() + _ = gate.claimAutomaticAttempt(trigger: .presentation, isLocked: true, isAuthenticating: false) + let claimed4 = gate.claimAutomaticAttempt(trigger: .windowFocus, isLocked: true, isAuthenticating: false) + #expect(!claimed4) + let claimed5 = gate.claimAutomaticAttempt(trigger: .explicitOpen, isLocked: true, isAuthenticating: false) + #expect(claimed5) + let claimed6 = gate.claimAutomaticAttempt(trigger: .windowFocus, isLocked: true, isAuthenticating: false) + #expect(!claimed6) + } + + @Test func busyOrUnlockedEventsDoNotResetConsumedAttempt() { + var gate = PanelUnlockPromptLifecycle() + _ = gate.beganManualAttempt() + let claimed7 = gate.claimAutomaticAttempt(trigger: .explicitOpen, isLocked: true, isAuthenticating: true) + #expect(!claimed7) + let claimed8 = gate.claimAutomaticAttempt(trigger: .explicitOpen, isLocked: false, isAuthenticating: false) + #expect(!claimed8) + #expect(gate.automaticAttemptConsumed) + } + + @Test func repeatedLockNotificationsDoNotRestartPrompting() { + var gate = PanelUnlockPromptLifecycle() + let original = gate.beganManualAttempt() + gate.didLock(wasAlreadyLocked: true) + #expect(!gate.acceptsCompletion(from: original)) + let claimed9 = gate.claimAutomaticAttempt(trigger: .windowFocus, isLocked: true, isAuthenticating: false) + #expect(!claimed9) + gate.didLock(wasAlreadyLocked: false) + let claimed10 = gate.claimAutomaticAttempt(trigger: .presentation, isLocked: true, isAuthenticating: false) + #expect(claimed10) + } + + @Test func deliberateLockSuppressesAutomaticAuthenticationUntilReopen() { + var gate = PanelUnlockPromptLifecycle() + gate.didLock(wasAlreadyLocked: false, suppressAutomaticAttempt: true) + let claimed11 = gate.claimAutomaticAttempt(trigger: .presentation, isLocked: true, isAuthenticating: false) + #expect(!claimed11) + let claimed12 = gate.claimAutomaticAttempt(trigger: .windowFocus, isLocked: true, isAuthenticating: false) + #expect(!claimed12) + let claimed13 = gate.claimAutomaticAttempt(trigger: .explicitOpen, isLocked: true, isAuthenticating: false) + #expect(claimed13) + } + + @Test func cancelledLoginDoesNotRepromptAfterAnyElapsedTime() async { + let auth = FakePanelAuthenticator() + auth.shouldFail = true + var time = Date(timeIntervalSince1970: 1_000) + let (lock, defaults, name) = controller(auth: auth, now: { time }) + defer { defaults.removePersistentDomain(forName: name) } + await lock.unlockIfNeededOnWindowFocus() + #expect(auth.authCount == 1) + for seconds in [0.1, 2.0, 60.0, 3_600.0] { + time = time.addingTimeInterval(seconds) + await lock.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) + await lock.unlockIfNeededOnWindowFocus() + } + #expect(auth.authCount == 1) + #expect(lock.isLocked) + } + + @Test func explicitUnlockRemainsAvailableAfterCancellation() async { + let auth = FakePanelAuthenticator() + auth.shouldFail = true + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + await lock.unlockIfNeededOnWindowFocus() + auth.shouldFail = false + await lock.unlock() + #expect(auth.authCount == 2) + #expect(!lock.isLocked) + } + + @Test func deliberateReopenCanRetryFailedAutomaticLogin() async { + let auth = FakePanelAuthenticator() + auth.shouldFail = true + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + await lock.unlockIfNeededOnWindowFocus() + await lock.unlockIfNeededOnWindowFocus(isUserInitiatedOpen: true) + #expect(auth.authCount == 2) + await lock.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) + #expect(auth.authCount == 2) + } + + @Test(arguments: [PanelLockReason.idle, .lockNow]) + func explicitLockDoesNotImmediatelyUnlockItself(_ reason: PanelLockReason) async { + let auth = FakePanelAuthenticator() + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + lock.applyUnlock(context: nil) + lock.lock(reason: reason) + await lock.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) + #expect(auth.authCount == 0) + #expect(lock.isLocked) + await lock.unlockIfNeededOnWindowFocus(isUserInitiatedOpen: true) + #expect(auth.authCount == 1) + } + + @Test func focusStormWhileAuthenticationIsPendingStartsOneRequest() async { + let auth = SuspendedPanelAuthenticator() + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + let request = Task { await lock.unlockIfNeededOnWindowFocus() } + while auth.continuation == nil { await Task.yield() } + for _ in 0..<100 { + await lock.unlockIfNeededOnWindowFocus(isWindowFocusEvent: true) + } + #expect(auth.count == 1) + auth.continuation?.resume(throwing: LAError(.userCancel)) + await request.value + #expect(lock.isLocked) + #expect(!lock.isUnlocking) + } + + @Test func newLockDiscardsSuccessfulPendingAuthentication() async { + let auth = SuspendedPanelAuthenticator() + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + let request = Task { await lock.unlock() } + while auth.continuation == nil { await Task.yield() } + lock.lock(reason: .screenLocked) + let context = MockLAContext() + auth.continuation?.resume(returning: context) + await request.value + #expect(context.didInvalidate) + #expect(lock.isLocked) + #expect(lock.unlockContext == nil) + } + + @Test func settingsAuthenticationCannotUnlockAfterScreenLock() async { + let auth = SuspendedPanelAuthenticator() + let (lock, defaults, name) = controller(auth: auth) + defer { defaults.removePersistentDomain(forName: name) } + let request = Task { + do { _ = try await lock.authenticateUser(); return false } + catch { return true } + } + while auth.continuation == nil { await Task.yield() } + lock.lock(reason: .screenLocked) + let context = MockLAContext() + auth.continuation?.resume(returning: context) + #expect(await request.value) + #expect(context.didInvalidate) + #expect(lock.isLocked) + } +} + +private final class SuspendedPanelAuthenticator: PanelAuthenticating { + var count = 0 + var continuation: CheckedContinuation? + func authenticate(reason: String) async throws -> LAContext { + count += 1 + return try await withCheckedThrowingContinuation { continuation = $0 } + } +}