From 281d2a8bc8327cc9816efaa7f7ab46c6976485b2 Mon Sep 17 00:00:00 2001 From: Bradley Mackey Date: Tue, 15 Sep 2026 10:34:33 +0400 Subject: [PATCH] Extract the autofill credential resolver and cover its gating The QuickType no-interaction path had zero tests despite carrying the auth gate and the HOTP refusal. Move the logic verbatim into an injectable AutofillOTPCredentialResolver (clock replaces raw Date()), reduce the view controller to an Outcome switch with the same indistinct ASExtensionErrors, and cover resolver and view model. Also wire the missing TestPlanReference into the VaultiOSAutofillTests scheme so its locale-pinned snapshots run correctly standalone. Co-Authored-By: Claude Fable 5 --- .../xcschemes/VaultiOSAutofillTests.xcscheme | 9 +- .../AutofillOTPCredentialResolver.swift | 71 ++++++++ ...aultCredentialProviderViewController.swift | 56 ++----- .../AutofillOTPCredentialResolverTests.swift | 154 ++++++++++++++++++ .../VaultAutofillViewModelTests.swift | 75 +++++++++ 5 files changed, 320 insertions(+), 45 deletions(-) create mode 100644 Vault/Sources/VaultiOSAutofill/Presentation/AutofillOTPCredentialResolver.swift create mode 100644 Vault/Tests/VaultiOSAutofillTests/AutofillOTPCredentialResolverTests.swift create mode 100644 Vault/Tests/VaultiOSAutofillTests/VaultAutofillViewModelTests.swift diff --git a/Vault/.swiftpm/xcode/xcshareddata/xcschemes/VaultiOSAutofillTests.xcscheme b/Vault/.swiftpm/xcode/xcshareddata/xcschemes/VaultiOSAutofillTests.xcscheme index 7b570d693..3e17c898b 100644 --- a/Vault/.swiftpm/xcode/xcshareddata/xcschemes/VaultiOSAutofillTests.xcscheme +++ b/Vault/.swiftpm/xcode/xcshareddata/xcschemes/VaultiOSAutofillTests.xcscheme @@ -11,8 +11,13 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES" - shouldAutocreateTestPlan = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES"> + + + + diff --git a/Vault/Sources/VaultiOSAutofill/Presentation/AutofillOTPCredentialResolver.swift b/Vault/Sources/VaultiOSAutofill/Presentation/AutofillOTPCredentialResolver.swift new file mode 100644 index 000000000..aaf203c2f --- /dev/null +++ b/Vault/Sources/VaultiOSAutofill/Presentation/AutofillOTPCredentialResolver.swift @@ -0,0 +1,71 @@ +import Foundation +import VaultCore +import VaultFeed + +/// Resolves a QuickType-bar OTP credential request without user +/// interaction. Extracted from `VaultCredentialProviderViewController` so +/// the security-relevant gating — auth-required items and HOTP counters +/// must never be served without interaction — is unit-testable. +@MainActor +struct AutofillOTPCredentialResolver { + enum Outcome: Equatable { + /// A rendered TOTP code, safe to return without interaction. + case code(String) + /// The item is auth-gated or is an HOTP code (whose counter must + /// not increment without UI). The system shows the extension UI. + case userInteractionRequired + /// The record identifier is missing, malformed, or matches no + /// unlocked OTP item. + case notFound + /// Retrieval or code rendering failed. + case failure + } + + private let retrieveItems: () async throws -> VaultRetrievalResult + private let copyActionHandler: any VaultItemCopyActionHandler + private let clock: any EpochClock + + init( + retrieveItems: @escaping () async throws -> VaultRetrievalResult, + copyActionHandler: any VaultItemCopyActionHandler, + clock: any EpochClock, + ) { + self.retrieveItems = retrieveItems + self.copyActionHandler = copyActionHandler + self.clock = clock + } + + func resolve(recordIdentifier: String?) async -> Outcome { + guard let recordIdentifier, let itemUUID = UUID(uuidString: recordIdentifier) else { + return .notFound + } + + do { + let result = try await retrieveItems() + + guard let vaultItem = result.items.first(where: { $0.id.rawValue == itemUUID }), + let otpCode = vaultItem.item.otpCode + else { + return .notFound + } + + // Check if the item requires authentication to access. + let copyAction = copyActionHandler.textToCopyForVaultItem(id: vaultItem.id) + if copyAction?.requiresAuthenticationToCopy == true { + return .userInteractionRequired + } + + switch otpCode.type { + case let .totp(period): + let totpCode = TOTPAuthCode(period: period, data: otpCode.data) + let epochSeconds = UInt64(clock.currentTime) + return try .code(totpCode.renderCode(epochSeconds: epochSeconds)) + case .hotp: + // HOTP codes require user interaction to increment the counter. + return .userInteractionRequired + } + } catch { + return .failure + } + } +} diff --git a/Vault/Sources/VaultiOSAutofill/VaultCredentialProviderViewController.swift b/Vault/Sources/VaultiOSAutofill/VaultCredentialProviderViewController.swift index bbe7d0411..b3b150b01 100644 --- a/Vault/Sources/VaultiOSAutofill/VaultCredentialProviderViewController.swift +++ b/Vault/Sources/VaultiOSAutofill/VaultCredentialProviderViewController.swift @@ -95,52 +95,22 @@ open class VaultCredentialProviderViewController: ASCredentialProviderViewContro @MainActor private func provideOTPCredential(for request: any ASCredentialRequest) async { - // Extract the credential identity and record identifier - guard let identity = request.credentialIdentity as? ASOneTimeCodeCredentialIdentity, - let recordIdentifier = identity.recordIdentifier, - let itemUUID = UUID(uuidString: recordIdentifier) - else { - extensionContext.cancelRequest(withError: ASExtensionError(.credentialIdentityNotFound)) - return - } - - do { - // Retrieve all items from the vault to find the matching OTP item - let result = try await VaultRoot.vaultStore.retrieve(query: .init()) - - guard let vaultItem = result.items.first(where: { $0.id.rawValue == itemUUID }), - let otpCode = vaultItem.item.otpCode - else { - extensionContext.cancelRequest(withError: ASExtensionError(.credentialIdentityNotFound)) - return - } - - // Check if the item requires authentication to access - let copyAction = VaultRoot.vaultItemCopyHandler.textToCopyForVaultItem(id: vaultItem.id) - if copyAction?.requiresAuthenticationToCopy == true { - // Require user interaction for authentication - extensionContext.cancelRequest(withError: ASExtensionError(.userInteractionRequired)) - return - } - - // Generate the OTP code based on the type - let codeString: String - switch otpCode.type { - case let .totp(period): - let totpCode = TOTPAuthCode(period: period, data: otpCode.data) - let epochSeconds = UInt64(Date().timeIntervalSince1970) - codeString = try totpCode.renderCode(epochSeconds: epochSeconds) - case .hotp: - // HOTP codes require user interaction to increment counter - extensionContext.cancelRequest(withError: ASExtensionError(.userInteractionRequired)) - return - } + let identity = request.credentialIdentity as? ASOneTimeCodeCredentialIdentity + let resolver = AutofillOTPCredentialResolver( + retrieveItems: { try await VaultRoot.vaultStore.retrieve(query: .init()) }, + copyActionHandler: VaultRoot.vaultItemCopyHandler, + clock: VaultRoot.clock, + ) - // Complete the request with the generated code + switch await resolver.resolve(recordIdentifier: identity?.recordIdentifier) { + case let .code(codeString): let credential = ASOneTimeCodeCredential(code: codeString) extensionContext.completeOneTimeCodeRequest(using: credential, completionHandler: nil) - - } catch { + case .userInteractionRequired: + extensionContext.cancelRequest(withError: ASExtensionError(.userInteractionRequired)) + case .notFound: + extensionContext.cancelRequest(withError: ASExtensionError(.credentialIdentityNotFound)) + case .failure: extensionContext.cancelRequest(withError: ASExtensionError(.failed)) } } diff --git a/Vault/Tests/VaultiOSAutofillTests/AutofillOTPCredentialResolverTests.swift b/Vault/Tests/VaultiOSAutofillTests/AutofillOTPCredentialResolverTests.swift new file mode 100644 index 000000000..6a5fd9799 --- /dev/null +++ b/Vault/Tests/VaultiOSAutofillTests/AutofillOTPCredentialResolverTests.swift @@ -0,0 +1,154 @@ +import Foundation +import FoundationExtensions +import Testing +import VaultCore +import VaultFeed +@testable import VaultiOSAutofill + +@MainActor +struct AutofillOTPCredentialResolverTests { + @Test + func resolve_nilRecordIdentifier_returnsNotFound() async { + let sut = makeSUT() + + let outcome = await sut.resolve(recordIdentifier: nil) + + #expect(outcome == .notFound) + } + + @Test + func resolve_malformedRecordIdentifier_returnsNotFound() async { + let sut = makeSUT() + + let outcome = await sut.resolve(recordIdentifier: "not-a-uuid") + + #expect(outcome == .notFound) + } + + @Test + func resolve_unknownItemID_returnsNotFound() async { + let sut = makeSUT(items: [makeOTPItem(type: .totp())]) + + let outcome = await sut.resolve(recordIdentifier: UUID().uuidString) + + #expect(outcome == .notFound) + } + + @Test + func resolve_nonOTPItem_returnsNotFound() async { + let note = anyVaultItem() + let sut = makeSUT(items: [note]) + + let outcome = await sut.resolve(recordIdentifier: note.id.rawValue.uuidString) + + #expect(outcome == .notFound) + } + + @Test + func resolve_authRequiredItem_returnsUserInteractionRequired() async { + // The security gate: an item whose copy action demands device + // authentication must never be served without interaction. + let item = makeOTPItem(type: .totp()) + let sut = makeSUT(items: [item], requiresAuthenticationToCopy: true) + + let outcome = await sut.resolve(recordIdentifier: item.id.rawValue.uuidString) + + #expect(outcome == .userInteractionRequired) + } + + @Test + func resolve_hotpItem_returnsUserInteractionRequired() async { + // HOTP counters must not increment without UI. + let item = makeOTPItem(type: .hotp()) + let sut = makeSUT(items: [item]) + + let outcome = await sut.resolve(recordIdentifier: item.id.rawValue.uuidString) + + #expect(outcome == .userInteractionRequired) + } + + @Test + func resolve_totpItem_returnsCodeRenderedForClockEpoch() async throws { + let code = makeTOTPCode(period: 30) + let item = VaultItem(metadata: anyVaultItemMetadata(), item: .otpCode(code)) + let clock = EpochClockMock(currentTime: 1_234_567_890) + let sut = makeSUT(items: [item], clock: clock) + + let outcome = await sut.resolve(recordIdentifier: item.id.rawValue.uuidString) + + let expected = try TOTPAuthCode(period: 30, data: code.data) + .renderCode(epochSeconds: 1_234_567_890) + #expect(outcome == .code(expected)) + } + + @Test + func resolve_retrievalError_returnsFailure() async { + struct RetrievalError: Error {} + let sut = makeSUT(retrieveItems: { throw RetrievalError() }) + + let outcome = await sut.resolve(recordIdentifier: UUID().uuidString) + + #expect(outcome == .failure) + } +} + +// MARK: - Helpers + +extension AutofillOTPCredentialResolverTests { + private func makeSUT( + items: [VaultItem] = [], + requiresAuthenticationToCopy: Bool = false, + clock: EpochClockMock = EpochClockMock(currentTime: 100), + ) -> AutofillOTPCredentialResolver { + makeSUT( + retrieveItems: { .init(items: items) }, + requiresAuthenticationToCopy: requiresAuthenticationToCopy, + clock: clock, + ) + } + + private func makeSUT( + retrieveItems: @escaping () async throws -> VaultRetrievalResult, + requiresAuthenticationToCopy: Bool = false, + clock: EpochClockMock = EpochClockMock(currentTime: 100), + ) -> AutofillOTPCredentialResolver { + AutofillOTPCredentialResolver( + retrieveItems: retrieveItems, + copyActionHandler: CopyActionHandlerStub(requiresAuthenticationToCopy: requiresAuthenticationToCopy), + clock: clock, + ) + } + + private func makeOTPItem(type: OTPAuthType) -> VaultItem { + VaultItem( + metadata: anyVaultItemMetadata(), + item: .otpCode(OTPAuthCode( + type: type, + data: makeOTPData(), + )), + ) + } + + private func makeTOTPCode(period: UInt64) -> OTPAuthCode { + OTPAuthCode(type: .totp(period: period), data: makeOTPData()) + } + + private func makeOTPData() -> OTPAuthCodeData { + OTPAuthCodeData( + secret: .init(data: Data.random(count: 50), format: .base32), + accountName: "Some Account", + ) + } + + private struct CopyActionHandlerStub: VaultItemCopyActionHandler { + let requiresAuthenticationToCopy: Bool + + func textToCopyForVaultItem(id _: Identifier) -> VaultTextCopyAction? { + VaultTextCopyAction( + text: "123456", + requiresAuthenticationToCopy: requiresAuthenticationToCopy, + contentType: .otp, + ) + } + } +} diff --git a/Vault/Tests/VaultiOSAutofillTests/VaultAutofillViewModelTests.swift b/Vault/Tests/VaultiOSAutofillTests/VaultAutofillViewModelTests.swift new file mode 100644 index 000000000..b59e305b3 --- /dev/null +++ b/Vault/Tests/VaultiOSAutofillTests/VaultAutofillViewModelTests.swift @@ -0,0 +1,75 @@ +import Combine +import Foundation +import FoundationExtensions +import TestHelpers +import Testing +import VaultSettings +@testable import VaultiOSAutofill + +@MainActor +struct VaultAutofillViewModelTests { + @Test + func init_displaysNoFeature() throws { + let sut = try makeSUT() + + #expect(sut.feature == nil) + } + + @Test + func show_setsDisplayedFeature() throws { + let sut = try makeSUT() + + sut.show(feature: .showAllCodesSelector) + + #expect(sut.feature == .showAllCodesSelector) + } + + @Test + func dismissConfiguration_publishesDismiss() throws { + let sut = try makeSUT() + var dismissCount = 0 + let cancellable = sut.configurationDismissPublisher.sink { dismissCount += 1 } + defer { cancellable.cancel() } + + sut.dismissConfiguration() + + #expect(dismissCount == 1) + } + + @Test + func textToInsertPublisher_filtersBlankStrings() throws { + let sut = try makeSUT() + var received = [String]() + let cancellable = sut.textToInsertPublisher.sink { received.append($0) } + defer { cancellable.cancel() } + + sut.textToInsertSubject.send("123456") + sut.textToInsertSubject.send("") + sut.textToInsertSubject.send(" ") + sut.textToInsertSubject.send("654321") + + #expect(received == ["123456", "654321"]) + } + + @Test + func cancelRequestPublisher_forwardsReason() throws { + let sut = try makeSUT() + var received = [VaultAutofillViewModel.RequestCancelReason]() + let cancellable = sut.cancelRequestPublisher.sink { received.append($0) } + defer { cancellable.cancel() } + + sut.cancelRequestSubject.send(.userCancelled) + + #expect(received == [.userCancelled]) + } +} + +// MARK: - Helpers + +extension VaultAutofillViewModelTests { + private func makeSUT() throws -> VaultAutofillViewModel { + try VaultAutofillViewModel( + localSettings: LocalSettings(defaults: Defaults.nonPersistent()), + ) + } +}