diff --git a/AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift b/AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift index 5978295aaf..e4ac326f03 100644 --- a/AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift +++ b/AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift @@ -251,7 +251,7 @@ public class ServiceContainer: Services { ) let sharedCryptographyService = DefaultAuthenticatorCryptographyService( - sharedKeychainRepository: sharedKeychainRepository + sharedKeychainRepository: sharedKeychainRepository, ) let sharedDataStore = AuthenticatorBridgeDataStore( diff --git a/AuthenticatorShared/Core/Platform/Services/Services.swift b/AuthenticatorShared/Core/Platform/Services/Services.swift index d9668e94f8..e55e044a71 100644 --- a/AuthenticatorShared/Core/Platform/Services/Services.swift +++ b/AuthenticatorShared/Core/Platform/Services/Services.swift @@ -16,9 +16,9 @@ typealias Services = HasAppInfoService & HasNotificationCenterService & HasPasteboardService & HasStateService - & HasTimeProvider & HasTOTPExpirationManagerFactory & HasTOTPService + & HasTimeProvider /// Protocol for an object that provides an `Application` /// diff --git a/AuthenticatorShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift b/AuthenticatorShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift index 06cce2b88d..9c92779d36 100644 --- a/AuthenticatorShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift +++ b/AuthenticatorShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift @@ -26,7 +26,7 @@ extension ServiceContainer { stateService: StateService = MockStateService(), timeProvider: TimeProvider = MockTimeProvider(.currentTime), totpExpirationManagerFactory: TOTPExpirationManagerFactory = MockTOTPExpirationManagerFactory(), - totpService: TOTPService = MockTOTPService() + totpService: TOTPService = MockTOTPService(), ) -> ServiceContainer { ServiceContainer( application: application, @@ -48,7 +48,7 @@ extension ServiceContainer { stateService: stateService, timeProvider: timeProvider, totpExpirationManagerFactory: totpExpirationManagerFactory, - totpService: totpService + totpService: totpService, ) } } diff --git a/BitwardenAutoFillExtension/CredentialProviderViewController.swift b/BitwardenAutoFillExtension/CredentialProviderViewController.swift index dd67df9a47..35ab53c37a 100644 --- a/BitwardenAutoFillExtension/CredentialProviderViewController.swift +++ b/BitwardenAutoFillExtension/CredentialProviderViewController.swift @@ -319,18 +319,7 @@ extension CredentialProviderViewController: AppExtensionDelegate { var isInAppExtension: Bool { true } var uri: String? { - guard let serviceIdentifiers = context?.serviceIdentifiers, - let serviceIdentifier = serviceIdentifiers.first - else { return nil } - - return switch serviceIdentifier.type { - case .domain: - "https://" + serviceIdentifier.identifier - case .URL: - serviceIdentifier.identifier - @unknown default: - serviceIdentifier.identifier - } + context?.uri } func completeAutofillRequest(username: String, password: String, fields: [(String, String)]?) { diff --git a/BitwardenKit/Core/Platform/Extensions/String.swift b/BitwardenKit/Core/Platform/Extensions/String.swift index d899a88bee..2889903a10 100644 --- a/BitwardenKit/Core/Platform/Extensions/String.swift +++ b/BitwardenKit/Core/Platform/Extensions/String.swift @@ -139,6 +139,38 @@ public extension String { } } + /// Returns a normalized URL string with an HTTPS scheme and no trailing slash. + /// + /// This method performs two normalization operations: + /// 1. Removes any trailing slash from the URL + /// 2. Prefixes the URL with `https://` if no scheme (`http://` or `https://`) is present + /// + /// If the URL already has an `http://` or `https://` scheme, it is preserved as-is + /// (after removing any trailing slash). + /// + /// - Returns: A normalized URL string suitable for consistent URL matching and comparison. + /// + /// # Examples + /// ```swift + /// "example.com".httpsNormalized() // "https://example.com" + /// "example.com/".httpsNormalized() // "https://example.com" + /// "http://example.com".httpsNormalized() // "http://example.com" + /// "https://example.com/".httpsNormalized() // "https://example.com" + /// ``` + /// + func httpsNormalized() -> String { + let stringUrl = if hasSuffix("/") { + String(dropLast()) + } else { + self + } + + guard stringUrl.starts(with: "https://") || stringUrl.starts(with: "http://") else { + return "https://" + stringUrl + } + return stringUrl + } + /// Creates a new string that has been encoded for use in a url or request header. /// /// - Returns: A `String` encoded for use in a url or request header. diff --git a/BitwardenKit/Core/Platform/Extensions/StringTests.swift b/BitwardenKit/Core/Platform/Extensions/StringTests.swift index c53d5f1922..ad871784df 100644 --- a/BitwardenKit/Core/Platform/Extensions/StringTests.swift +++ b/BitwardenKit/Core/Platform/Extensions/StringTests.swift @@ -47,6 +47,42 @@ class StringTests: BitwardenTestCase { XCTAssertEqual(subject.hexSHA256Hash, expected) } + /// `httpsNormalized()` adds HTTPS prefix when no scheme is present. + func test_httpsNormalized_addsHttpsPrefix() { + XCTAssertEqual("example.com".httpsNormalized(), "https://example.com") + XCTAssertEqual("bitwarden.com".httpsNormalized(), "https://bitwarden.com") + XCTAssertEqual("sub.domain.example.com".httpsNormalized(), "https://sub.domain.example.com") + } + + /// `httpsNormalized()` removes trailing slash. + func test_httpsNormalized_removesTrailingSlash() { + XCTAssertEqual("example.com/".httpsNormalized(), "https://example.com") + XCTAssertEqual("https://example.com/".httpsNormalized(), "https://example.com") + XCTAssertEqual("http://example.com/".httpsNormalized(), "http://example.com") + } + + /// `httpsNormalized()` preserves existing HTTPS scheme. + func test_httpsNormalized_preservesHttpsScheme() { + XCTAssertEqual("https://example.com".httpsNormalized(), "https://example.com") + XCTAssertEqual("https://bitwarden.com".httpsNormalized(), "https://bitwarden.com") + XCTAssertEqual("https://example.com:8080".httpsNormalized(), "https://example.com:8080") + } + + /// `httpsNormalized()` preserves existing HTTP scheme. + func test_httpsNormalized_preservesHttpScheme() { + XCTAssertEqual("http://example.com".httpsNormalized(), "http://example.com") + XCTAssertEqual("http://bitwarden.com".httpsNormalized(), "http://bitwarden.com") + XCTAssertEqual("http://localhost:8080".httpsNormalized(), "http://localhost:8080") + } + + /// `httpsNormalized()` handles URLs with paths correctly. + func test_httpsNormalized_withPaths() { + XCTAssertEqual("example.com/path".httpsNormalized(), "https://example.com/path") + XCTAssertEqual("https://example.com/path".httpsNormalized(), "https://example.com/path") + XCTAssertEqual("example.com/path/to/resource".httpsNormalized(), "https://example.com/path/to/resource") + XCTAssertEqual("https://example.com/path/".httpsNormalized(), "https://example.com/path") + } + /// `isValidURL` returns `true` for a valid URL. func test_isBitwardenAppScheme() { XCTAssertTrue("bitwarden".isBitwardenAppScheme) diff --git a/BitwardenKit/Core/Platform/Extensions/URL.swift b/BitwardenKit/Core/Platform/Extensions/URL.swift index 95e3b6791c..ef9787200e 100644 --- a/BitwardenKit/Core/Platform/Extensions/URL.swift +++ b/BitwardenKit/Core/Platform/Extensions/URL.swift @@ -21,16 +21,7 @@ public extension URL { /// Returns a sanitized version of the URL. This will add a https scheme to the URL if the /// scheme is missing and remove a trailing slash. var sanitized: URL { - let stringUrl = if absoluteString.hasSuffix("/") { - String(absoluteString.dropLast()) - } else { - absoluteString - } - - guard stringUrl.starts(with: "https://") || stringUrl.starts(with: "http://") else { - return URL(string: "https://" + stringUrl) ?? self - } - return URL(string: stringUrl) ?? self + URL(string: absoluteString.httpsNormalized()) ?? self } /// Returns a string of the URL with the scheme removed (e.g. `send.bitwarden.com/39ngaol3`). diff --git a/BitwardenKit/UI/Platform/Application/Utilities/InputValidator/InputValidator.swift b/BitwardenKit/UI/Platform/Application/Utilities/InputValidator/InputValidator.swift index deda74a8c8..8ee067942f 100644 --- a/BitwardenKit/UI/Platform/Application/Utilities/InputValidator/InputValidator.swift +++ b/BitwardenKit/UI/Platform/Application/Utilities/InputValidator/InputValidator.swift @@ -1,4 +1,4 @@ -// - MARK: Input Validator +// MARK: Input Validator /// A protocol for an object that handles validating input for a field. /// diff --git a/BitwardenShared/Core/Autofill/Services/AutofillCredentialService+AppExtensionTests.swift b/BitwardenShared/Core/Autofill/Services/AutofillCredentialService+AppExtensionTests.swift index a4f8588b8c..5e6728ab14 100644 --- a/BitwardenShared/Core/Autofill/Services/AutofillCredentialService+AppExtensionTests.swift +++ b/BitwardenShared/Core/Autofill/Services/AutofillCredentialService+AppExtensionTests.swift @@ -103,7 +103,7 @@ class AutofillCredentialServiceAppExtensionTests: BitwardenTestCase { /// `syncIdentities(vaultLockStatus:)` doesn't update the credential identity store with the identities /// from the user's vault when the app context is `.appExtension`. - func test_syncIdentities_appExtensionContext() { // swiftlint:disable:this function_body_length + func test_syncIdentities_appExtensionContext() { prepareDataForIdentitiesReplacement() vaultTimeoutService.vaultLockStatusSubject.send(VaultLockStatus(isVaultLocked: false, userId: "1")) diff --git a/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContext.swift b/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContext.swift index c8c870eb29..2dcc2f3853 100644 --- a/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContext.swift +++ b/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContext.swift @@ -18,6 +18,8 @@ public protocol CredentialProviderContext { var flowWithUserInteraction: Bool { get } /// The `ASCredentialServiceIdentifier` array depending on the `ExtensionMode`. var serviceIdentifiers: [ASCredentialServiceIdentifier] { get } + /// The URI of the credential to autofill. + var uri: String? { get } } /// Default implementation of `CredentialProviderContext`. @@ -89,6 +91,34 @@ public struct DefaultCredentialProviderContext: CredentialProviderContext { } } + public var uri: String? { + guard let serviceIdentifier = serviceIdentifiers.first else { + // WORKAROUND: iOS does not consistently send `serviceIdentifiers` in the Fido2 + Passwords + // vault list flow (.autofillFido2VaultList). As a fallback, we use the `relyingPartyIdentifier` + // as the URI for filtering, which provides similar functionality. + // + // This fallback should be retained even if Apple fixes the primary issue, as it ensures + // resilience against future OS regressions and edge cases. + // + // Related: iOS Autofill API behavior - serviceIdentifiers may be empty in certain contexts. + if case let .autofillFido2VaultList(_, passkeyParameters) = extensionMode, + !passkeyParameters.relyingPartyIdentifier.isEmpty { + return passkeyParameters.relyingPartyIdentifier.httpsNormalized() + } + + return nil + } + + return switch serviceIdentifier.type { + case .domain: + "https://" + serviceIdentifier.identifier + case .URL: + serviceIdentifier.identifier + @unknown default: + serviceIdentifier.identifier + } + } + /// Initializes the context. /// - Parameter extensionMode: The mode of the extension. public init(_ extensionMode: AutofillExtensionMode) { diff --git a/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContextTests.swift b/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContextTests.swift index 182cee086d..4d42290125 100644 --- a/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContextTests.swift +++ b/BitwardenShared/Core/Autofill/Utilities/CredentialProviderContext/CredentialProviderContextTests.swift @@ -3,7 +3,7 @@ import XCTest @testable import BitwardenShared -class CredentialProviderContextTests: BitwardenTestCase { +class CredentialProviderContextTests: BitwardenTestCase { // swiftlint:disable:this type_body_length // MARK: Tests /// `getter:authCompletionRoute` the corresponding route depending on the context mode. @@ -294,6 +294,103 @@ class CredentialProviderContextTests: BitwardenTestCase { let subject6 = DefaultCredentialProviderContext(.autofillText) XCTAssertEqual(subject6.serviceIdentifiers, expectedIdentifiers) } + + /// `getter:uri` returns the URI with https prefix when service identifier is a domain. + func test_uri_domain() { + let serviceIdentifier = ASCredentialServiceIdentifier.fixture( + identifier: "example.com", + type: .domain, + ) + let subject = DefaultCredentialProviderContext(.autofillVaultList([serviceIdentifier])) + XCTAssertEqual(subject.uri, "https://example.com") + } + + /// `getter:uri` returns the URI as-is when service identifier is a URL. + func test_uri_url() { + let serviceIdentifier = ASCredentialServiceIdentifier.fixture( + identifier: "https://example.com/path", + type: .URL, + ) + let subject = DefaultCredentialProviderContext(.autofillVaultList([serviceIdentifier])) + XCTAssertEqual(subject.uri, "https://example.com/path") + } + + /// `getter:uri` returns the first service identifier when multiple identifiers exist. + func test_uri_multipleServiceIdentifiers() { + let identifiers = [ + ASCredentialServiceIdentifier.fixture(identifier: "first.com", type: .domain), + ASCredentialServiceIdentifier.fixture(identifier: "second.com", type: .domain), + ASCredentialServiceIdentifier.fixture(identifier: "third.com", type: .domain), + ] + let subject = DefaultCredentialProviderContext(.autofillVaultList(identifiers)) + XCTAssertEqual(subject.uri, "https://first.com") + } + + /// `getter:uri` returns relying party identifier as fallback for autofillFido2VaultList + /// when service identifiers are empty, normalized with HTTPS prefix. + func test_uri_autofillFido2VaultList_relyingPartyFallback() { + let parameters = MockPasskeyCredentialRequestParameters(relyingPartyIdentifier: "passkey.example.com") + let subject = DefaultCredentialProviderContext(.autofillFido2VaultList([], parameters)) + XCTAssertEqual(subject.uri, "https://passkey.example.com") + } + + /// `getter:uri` returns relying party identifier normalized, preserving existing HTTPS scheme. + func test_uri_autofillFido2VaultList_relyingPartyWithHttpsScheme() { + let parameters = MockPasskeyCredentialRequestParameters(relyingPartyIdentifier: "https://passkey.example.com") + let subject = DefaultCredentialProviderContext(.autofillFido2VaultList([], parameters)) + XCTAssertEqual(subject.uri, "https://passkey.example.com") + } + + /// `getter:uri` returns the service identifier URI when available for autofillFido2VaultList, + /// ignoring the relying party identifier. + func test_uri_autofillFido2VaultList_withServiceIdentifiers() { + let serviceIdentifier = ASCredentialServiceIdentifier.fixture( + identifier: "actual.example.com", + type: .domain, + ) + let parameters = MockPasskeyCredentialRequestParameters(relyingPartyIdentifier: "fallback.example.com") + let subject = DefaultCredentialProviderContext(.autofillFido2VaultList([serviceIdentifier], parameters)) + XCTAssertEqual(subject.uri, "https://actual.example.com") + } + + /// `getter:uri` returns nil when autofillFido2VaultList has empty service identifiers + /// and empty relying party identifier. + func test_uri_autofillFido2VaultList_emptyRelyingParty() { + let parameters = MockPasskeyCredentialRequestParameters(relyingPartyIdentifier: "") + let subject = DefaultCredentialProviderContext(.autofillFido2VaultList([], parameters)) + XCTAssertNil(subject.uri) + } + + /// `getter:uri` returns nil when no service identifiers and not autofillFido2VaultList mode. + func test_uri_nil() { + let subject1 = DefaultCredentialProviderContext(.autofillCredential(.fixture(), userInteraction: false)) + XCTAssertNil(subject1.uri) + + let subject2 = DefaultCredentialProviderContext( + .autofillFido2Credential(MockPasskeyCredentialRequest(), userInteraction: false), + ) + XCTAssertNil(subject2.uri) + + let subject3 = DefaultCredentialProviderContext(.configureAutofill) + XCTAssertNil(subject3.uri) + + let subject4 = DefaultCredentialProviderContext(.registerFido2Credential(MockPasskeyCredentialRequest())) + XCTAssertNil(subject4.uri) + + let subject5 = DefaultCredentialProviderContext( + .autofillOTPCredential(MockOneTimeCodeCredentialIdentity(), userInteraction: false), + ) + XCTAssertNil(subject5.uri) + + let subject6 = DefaultCredentialProviderContext(.autofillText) + XCTAssertNil(subject6.uri) + } + + /// `getter:uri` returns nil when service identifiers are empty for autofillVaultList. + func test_uri_autofillVaultList_empty() { + let subject = DefaultCredentialProviderContext(.autofillVaultList([])) + XCTAssertNil(subject.uri) + } } class MockPasskeyCredentialRequest: PasskeyCredentialRequest {} diff --git a/BitwardenShared/Core/Platform/Services/NotificationService.swift b/BitwardenShared/Core/Platform/Services/NotificationService.swift index 84867a2deb..6d2fc255ad 100644 --- a/BitwardenShared/Core/Platform/Services/NotificationService.swift +++ b/BitwardenShared/Core/Platform/Services/NotificationService.swift @@ -398,4 +398,4 @@ class DefaultNotificationService: NotificationService { errorReporter.log(error: error) } } -} +} // swiftlint:disable:this file_length diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index d4eb65301e..d3db2ded5f 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -723,9 +723,23 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le let collectionHelper = DefaultCollectionHelper(organizationService: organizationService) + let fido2UserInterfaceHelper = DefaultFido2UserInterfaceHelper( + fido2UserVerificationMediator: DefaultFido2UserVerificationMediator( + authRepository: authRepository, + stateService: stateService, + userVerificationHelper: DefaultUserVerificationHelper( + authRepository: authRepository, + errorReporter: errorReporter, + localAuthService: localAuthService, + ), + userVerificationRunner: DefaultUserVerificationRunner(), + ), + ) + let vaultListDirectorStrategyFactory = DefaultVaultListDirectorStrategyFactory( cipherService: cipherService, collectionService: collectionService, + fido2UserInterfaceHelper: fido2UserInterfaceHelper, folderService: folderService, vaultListBuilderFactory: DefaultVaultListSectionsBuilderFactory( clientService: clientService, @@ -775,19 +789,6 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le vaultTimeoutService: vaultTimeoutService, ) - let fido2UserInterfaceHelper = DefaultFido2UserInterfaceHelper( - fido2UserVerificationMediator: DefaultFido2UserVerificationMediator( - authRepository: authRepository, - stateService: stateService, - userVerificationHelper: DefaultUserVerificationHelper( - authRepository: authRepository, - errorReporter: errorReporter, - localAuthService: localAuthService, - ), - userVerificationRunner: DefaultUserVerificationRunner(), - ), - ) - #if DEBUG let fido2CredentialStore = DebuggingFido2CredentialStoreService( fido2CredentialStore: Fido2CredentialStoreService( diff --git a/BitwardenShared/Core/Vault/Extensions/CipherListView+Extensions.swift b/BitwardenShared/Core/Vault/Extensions/CipherListView+Extensions.swift index ded64f6ad0..a09699bb6f 100644 --- a/BitwardenShared/Core/Vault/Extensions/CipherListView+Extensions.swift +++ b/BitwardenShared/Core/Vault/Extensions/CipherListView+Extensions.swift @@ -2,6 +2,23 @@ import BitwardenResources import BitwardenSdk extension CipherListView { + /// Determines whether the cipher can be used in basic password autofill operations. + /// + /// A cipher qualifies for basic login autofill if it's a login type and contains at least one + /// of the following copyable fields: username, password, or TOTP code. + /// + /// - Returns: `true` if the cipher can be used for basic password autofill, `false` otherwise. + var canBeUsedInBasicLoginAutofill: Bool { + type.isLogin && copyableFields.contains { copyableField in + switch copyableField { + case .loginPassword, .loginTotp, .loginUsername: + true + default: + false + } + } + } + /// Whether the cipher passes the `.restrictItemTypes` policy based on the organizations restricted. /// /// - Parameters: diff --git a/BitwardenShared/Core/Vault/Extensions/CipherListViewExtensionsTests.swift b/BitwardenShared/Core/Vault/Extensions/CipherListViewExtensionsTests.swift index d6a3b65fe7..37f1289fd7 100644 --- a/BitwardenShared/Core/Vault/Extensions/CipherListViewExtensionsTests.swift +++ b/BitwardenShared/Core/Vault/Extensions/CipherListViewExtensionsTests.swift @@ -8,6 +8,96 @@ import XCTest class CipherListViewExtensionsTests: BitwardenTestCase { // MARK: Tests + /// `canBeUsedInBasicLoginAutofill` returns `false` when the cipher is not a login type. + func test_canBeUsedInBasicLoginAutofill_nonLoginType() { + XCTAssertFalse(CipherListView.fixture(type: .card(.fixture())).canBeUsedInBasicLoginAutofill) + XCTAssertFalse(CipherListView.fixture(type: .identity).canBeUsedInBasicLoginAutofill) + XCTAssertFalse(CipherListView.fixture(type: .secureNote).canBeUsedInBasicLoginAutofill) + XCTAssertFalse(CipherListView.fixture(type: .sshKey).canBeUsedInBasicLoginAutofill) + } + + /// `canBeUsedInBasicLoginAutofill` returns `false` when the login has no copyable login fields. + func test_canBeUsedInBasicLoginAutofill_noLoginFields() { + XCTAssertFalse( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `false` when the login has only non-login copyable fields. + func test_canBeUsedInBasicLoginAutofill_onlyNonLoginFields() { + XCTAssertFalse( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.cardNumber, .cardSecurityCode], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `true` when the login has a username field. + func test_canBeUsedInBasicLoginAutofill_hasUsername() { + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginUsername], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `true` when the login has a password field. + func test_canBeUsedInBasicLoginAutofill_hasPassword() { + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginPassword], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `true` when the login has a TOTP field. + func test_canBeUsedInBasicLoginAutofill_hasTotp() { + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginTotp], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `true` when the login has multiple login fields. + func test_canBeUsedInBasicLoginAutofill_hasMultipleLoginFields() { + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginUsername, .loginPassword, .loginTotp], + ).canBeUsedInBasicLoginAutofill, + ) + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginUsername, .loginPassword], + ).canBeUsedInBasicLoginAutofill, + ) + } + + /// `canBeUsedInBasicLoginAutofill` returns `true` when the login has login fields mixed with other fields. + func test_canBeUsedInBasicLoginAutofill_hasLoginFieldsWithOtherFields() { + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.loginUsername, .cardNumber], + ).canBeUsedInBasicLoginAutofill, + ) + XCTAssertTrue( + CipherListView.fixture( + type: .login(.fixture()), + copyableFields: [.cardSecurityCode, .loginPassword, .identityUsername], + ).canBeUsedInBasicLoginAutofill, + ) + } + /// `passesRestrictItemTypesPolicy(_:)` passes the policy when there are no organization IDs. func test_passesRestrictItemTypesPolicy_noOrgIds() { XCTAssertTrue(CipherListView.fixture().passesRestrictItemTypesPolicy([])) diff --git a/BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListSectionsBuilder+Extensions.swift b/BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListSectionsBuilder+Extensions.swift index cd7f07f57e..20d94b5e58 100644 --- a/BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListSectionsBuilder+Extensions.swift +++ b/BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListSectionsBuilder+Extensions.swift @@ -8,6 +8,10 @@ extension MockVaultListSectionsBuilder { helper.recordCall("addAutofillPasswordsSection") return self } + addAutofillCombinedMultipleSectionClosure = { (_: String?) -> VaultListSectionsBuilder in + helper.recordCall("addAutofillCombinedMultipleSection") + return self + } addAutofillCombinedSingleSectionClosure = { () -> VaultListSectionsBuilder in helper.recordCall("addAutofillCombinedSingleSection") return self diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparator.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparator.swift index e5bc23568d..171c4c8e12 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparator.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparator.swift @@ -15,7 +15,20 @@ protocol VaultListDataPreparator { // sourcery: AutoMockable func prepareAutofillPasswordsData( from ciphers: [Cipher], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? + ) async -> VaultListPreparedData? + + /// Prepares autofill's data on passwords + Fido2 combined in multiple sections for the vault list builder. + /// - Parameters: + /// - ciphers: An array of `Cipher` objects to be processed. + /// - filter: A `VaultListFilter` object that defines the filtering criteria for the vault list. + /// - withFido2Credentials: Available Fido2 credentials to build the vault list section. + /// - Returns: An optional `VaultListPreparedData` object containing the prepared data for the vault list. + /// Returns `nil` if the vault is empty. + func prepareAutofillCombinedMultipleData( + from ciphers: [Cipher], + filter: VaultListFilter, + withFido2Credentials fido2Credentials: [CipherView]?, + ) async -> VaultListPreparedData? /// Prepares autofill's data on passwords + Fido2 combined in a single section for the vault list builder. /// - Parameters: @@ -26,7 +39,7 @@ protocol VaultListDataPreparator { // sourcery: AutoMockable func prepareAutofillCombinedSingleData( from ciphers: [Cipher], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? + ) async -> VaultListPreparedData? /// Prepares data for the vault list builder. /// - Parameters: @@ -41,7 +54,7 @@ protocol VaultListDataPreparator { // sourcery: AutoMockable collections: [Collection], folders: [Folder], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? + ) async -> VaultListPreparedData? /// Prepares group data for the vault list builder. /// - Parameters: @@ -56,7 +69,7 @@ protocol VaultListDataPreparator { // sourcery: AutoMockable collections: [Collection], folders: [Folder], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? + ) async -> VaultListPreparedData? } /// Default implementation of `VaultListDataPreparator`. @@ -85,7 +98,7 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { func prepareAutofillPasswordsData( from ciphers: [Cipher], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? { + ) async -> VaultListPreparedData? { guard !ciphers.isEmpty, let uri = filter.uri else { return nil } @@ -98,7 +111,9 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { await ciphersClientWrapperService.decryptAndProcessCiphersInBatch( ciphers: ciphers, ) { decryptedCipher in - guard decryptedCipher.deletedDate == nil, + guard decryptedCipher.type.isLogin, + decryptedCipher.deletedDate == nil, + decryptedCipher.canBeUsedInBasicLoginAutofill, decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else { return } @@ -114,10 +129,55 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { return preparedDataBuilder.build() } + func prepareAutofillCombinedMultipleData( + from ciphers: [Cipher], + filter: VaultListFilter, + withFido2Credentials fido2Credentials: [CipherView]?, + ) async -> VaultListPreparedData? { + guard !ciphers.isEmpty, let uri = filter.uri else { + return nil + } + + let cipherMatchingHelper = await cipherMatchingHelperFactory.make(uri: uri) + + var preparedDataBuilder = vaultListPreparedDataBuilderFactory.make() + let restrictedOrganizationIds = await prepareRestrictedOrganizationIds(builder: preparedDataBuilder) + + await ciphersClientWrapperService.decryptAndProcessCiphersInBatch( + ciphers: ciphers, + ) { decryptedCipher in + guard decryptedCipher.type.isLogin, + decryptedCipher.deletedDate == nil, + decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else { + return + } + + let matchResult = cipherMatchingHelper.doesCipherMatch(cipher: decryptedCipher) + guard matchResult != .none else { + return + } + + if let fido2Credentials, + decryptedCipher.type.loginListView?.hasFido2 == true, + fido2Credentials.contains(where: { $0.id == decryptedCipher.id }) { + preparedDataBuilder = await preparedDataBuilder.addFido2Item(cipher: decryptedCipher) + } + + if decryptedCipher.canBeUsedInBasicLoginAutofill { + preparedDataBuilder = await preparedDataBuilder.addItem( + forGroup: .login, + with: decryptedCipher, + ) + } + } + + return preparedDataBuilder.build() + } + func prepareAutofillCombinedSingleData( from ciphers: [Cipher], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? { + ) async -> VaultListPreparedData? { guard !ciphers.isEmpty, let uri = filter.uri else { return nil } @@ -130,7 +190,8 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { await ciphersClientWrapperService.decryptAndProcessCiphersInBatch( ciphers: ciphers, ) { decryptedCipher in - guard decryptedCipher.deletedDate == nil, + guard decryptedCipher.type.isLogin, + decryptedCipher.deletedDate == nil, decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else { return } @@ -159,7 +220,7 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { collections: [Collection], folders: [Folder], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? { + ) async -> VaultListPreparedData? { guard !ciphers.isEmpty else { return nil } @@ -205,7 +266,7 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator { collections: [Collection], folders: [Folder], filter: VaultListFilter, - ) async throws -> VaultListPreparedData? { + ) async -> VaultListPreparedData? { guard !ciphers.isEmpty, let group = filter.group else { return nil } diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparatorTests.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparatorTests.swift index cf599fdb6f..ac6156a510 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparatorTests.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDataPreparatorTests.swift @@ -78,7 +78,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareAutofillCombinedSingleData(from:filter:)` returns `nil` when no ciphers passed. func test_prepareAutofillCombinedSingleData_noCiphers() async throws { - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( from: [], filter: VaultListFilter(uri: "https://example.com"), ) @@ -88,7 +88,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareAutofillCombinedSingleData(from:filter:)` returns `nil` when filter passed doesn't /// have the URI to filter. func test_prepareAutofillCombinedSingleData_noFilterUri() async throws { - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( from: [.fixture()], filter: VaultListFilter(), ) @@ -103,10 +103,11 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi hasFido2: false, uris: [.fixture(uri: "https://example.com", match: .exact)], ), + copyableFields: [.loginPassword], ) cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( from: [ .fixture( login: .fixture( @@ -125,17 +126,19 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi XCTAssertNotNil(result) } - /// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data for a cipher with login and Fido2. + /// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data for a cipher with login and Fido2, + /// adding it only to Fido2 items (not to group items, unlike the multiple mode). func test_prepareAutofillCombinedSingleData_returnsPreparedDataForLoginWithFido2() async throws { ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( login: .fixture( hasFido2: true, uris: [.fixture(uri: "https://example.com", match: .exact)], ), + copyableFields: [.loginPassword], ) cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( from: [ .fixture( login: .fixture( @@ -168,7 +171,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ] cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( from: [ .fixture(type: .card), ], @@ -181,6 +184,38 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi XCTAssertNotNil(result) } + /// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data including + /// ciphers without copyable login fields. + @MainActor + func test_prepareAutofillCombinedSingleData_noCopyableLoginFields() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: false, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedSingleData( + from: [ + .fixture( + login: .fixture( + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + "addItemForGroup", + ]) + XCTAssertNotNil(result) + } + /// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data filtering out /// cipher as it's deleted. @MainActor @@ -191,7 +226,218 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ) cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillCombinedSingleData( + let result = await subject.prepareAutofillCombinedSingleData( + from: [ + .fixture( + login: .fixture( + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns `nil` when no ciphers passed. + func test_prepareAutofillCombinedMultipleData_noCiphers() async throws { + let result = await subject.prepareAutofillCombinedMultipleData( + from: [], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + XCTAssertNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns `nil` + /// when filter passed doesn't have the URI to filter. + func test_prepareAutofillCombinedMultipleData_noFilterUri() async throws { + let result = await subject.prepareAutofillCombinedMultipleData( + from: [.fixture()], + filter: VaultListFilter(), + withFido2Credentials: nil, + ) + XCTAssertNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data for a cipher + /// with login and no Fido2. + func test_prepareAutofillCombinedMultipleData_returnsPreparedDataForLoginNoFido2() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + login: .fixture( + hasFido2: false, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [.loginPassword], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + login: .fixture( + fido2Credentials: [], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + "addItemForGroup", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data for a cipher + /// with login and Fido2. + func test_prepareAutofillCombinedMultipleData_returnsPreparedDataForLoginWithFido2() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: true, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [.loginPassword], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + id: "1", + login: .fixture( + fido2Credentials: [.fixture()], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: [.fixture(id: "1")], + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + "addFido2Item", + "addItemForGroup", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data for a cipher + /// with login and Fido2 but no Fido2 credentials provided. + func test_prepareAutofillCombinedMultipleData_returnsDataWithFido2NoCredentialsProvided() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: true, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [.loginPassword], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + id: "1", + login: .fixture( + fido2Credentials: [.fixture()], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + "addItemForGroup", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data for a cipher + /// with login and Fido2 but cipher ID doesn't match credentials. + func test_prepareAutofillCombinedMultipleData_returnsDataWithFido2NonMatchingCredentials() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: true, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [.loginPassword], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + id: "1", + login: .fixture( + fido2Credentials: [.fixture()], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: [.fixture(id: "2")], + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + "addItemForGroup", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data filtering out + /// cipher as it doesn't pass restrict item type policy. + @MainActor + func test_prepareAutofillCombinedMultipleData_doesNotPassRestrictItemPolicy() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + organizationId: "1", + type: .card(.fixture()), + ) + policyService.policyAppliesToUserPolicies = [ + .fixture(organizationId: "1"), + ] + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture(type: .card), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data filtering out + /// cipher as it's deleted. + @MainActor + func test_prepareAutofillCombinedMultipleData_deletedCipher() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + deletedDate: .now, + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( from: [ .fixture( login: .fixture( @@ -200,6 +446,94 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ), ], filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data filtering out + /// cipher as it's not a login type. + @MainActor + func test_prepareAutofillCombinedMultipleData_nonLoginCipher() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + type: .card(.fixture()), + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture(type: .card), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data filtering out + /// cipher as it doesn't have any copyable login fields. + @MainActor + func test_prepareAutofillCombinedMultipleData_noCopyableLoginFields() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: false, + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + copyableFields: [], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + login: .fixture( + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + } + + /// `prepareAutofillCombinedMultipleData(from:filter:withFido2Credentials:)` returns the prepared data filtering out + /// cipher as it doesn't match the URI. + @MainActor + func test_prepareAutofillCombinedMultipleData_nonMatchingCipher() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + login: .fixture( + hasFido2: false, + uris: [.fixture(uri: "https://other.com", match: .exact)], + ), + ) + cipherMatchingHelper.doesCipherMatchReturnValue = CipherMatchResult.none + + let result = await subject.prepareAutofillCombinedMultipleData( + from: [ + .fixture( + login: .fixture( + uris: [.fixture(uri: "https://other.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + withFido2Credentials: nil, ) XCTAssertEqual(mockCallOrderHelper.callOrder, [ @@ -210,7 +544,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareData(from:collections:folders:filter:)` returns `nil` when no ciphers passed. func test_prepareData_noCiphers() async throws { - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [], collections: [], folders: [], @@ -223,7 +557,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi func test_prepareData_returnsPreparedDataNoFilteringOutCipher() async throws { ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture() - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -250,7 +584,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi func test_prepareData_returnsPreparedDataNoFilteringOutCipherNoTOTPGroup() async throws { ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture() - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -279,7 +613,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi organizationId: "1", ) - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -305,7 +639,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ) policyService.policyAppliesToUserPolicies = [.fixture(organizationId: "1")] - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture(organizationId: "1", type: .card)], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -331,7 +665,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi policyService.policyAppliesToUserPolicies = [.fixture(organizationId: "2")] - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture(organizationId: "1", type: .card)], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -362,7 +696,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi deletedDate: .now, ) - let result = try await subject.prepareData( + let result = await subject.prepareData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -380,7 +714,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareGroupData(from:collections:folders:filter:)` returns `nil` when no ciphers passed. func test_prepareGroupData_noCiphers() async throws { - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [], collections: [], folders: [], @@ -398,7 +732,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi organizationId: "1", ) - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -424,7 +758,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ) policyService.policyAppliesToUserPolicies = [.fixture(organizationId: "1")] - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture(organizationId: "1", type: .card)], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -443,7 +777,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// when deleted date is set and vault filter is not trash. @MainActor func test_prepareGroupData_cipherDeleteDateSet_vaultNotTrash() async throws { - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture(deletedDate: .now)], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -466,7 +800,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi id: "1", ) - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -490,7 +824,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi id: "1", ) - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -533,7 +867,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ) policyService.policyAppliesToUserPolicies = [.fixture(organizationId: "2")] - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], @@ -551,7 +885,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareAutofillPasswordsData(from::filter:)` returns `nil` when no ciphers passed. func test_prepareAutofillPasswordsData_noCiphers() async throws { - let result = try await subject.prepareAutofillPasswordsData( + let result = await subject.prepareAutofillPasswordsData( from: [], filter: VaultListFilter(), ) @@ -561,7 +895,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareAutofillPasswordsData(from::filter:)` returns `nil` when filter passed doesn't /// have the URI to filter. func test_prepareAutofillPasswordsData_noLoginUris() async throws { - let result = try await subject.prepareAutofillPasswordsData( + let result = await subject.prepareAutofillPasswordsData( from: [.fixture()], filter: VaultListFilter(), ) @@ -570,10 +904,12 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// `prepareAutofillPasswordsData(from:filter:)` returns the prepared data without filtering out cipher. func test_prepareAutofillPasswordsData_returnsPreparedDataNoFilteringOutCipher() async throws { - ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture() + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + copyableFields: [.loginPassword], + ) cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillPasswordsData( + let result = await subject.prepareAutofillPasswordsData( from: [ .fixture( login: .fixture( @@ -607,7 +943,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ] cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillPasswordsData( + let result = await subject.prepareAutofillPasswordsData( from: [ .fixture( type: .card, @@ -623,6 +959,34 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi XCTAssertNil(cipherMatchingHelper.doesCipherMatchReceivedCipher) } + /// `prepareAutofillPasswordsData(from:filter:)` returns the prepared data filtering out cipher + /// as it doesn't have any copyable login fields. + @MainActor + func test_prepareAutofillPasswordsData_noCopyableLoginFields() async throws { + ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture( + id: "1", + copyableFields: [], + ) + cipherMatchingHelper.doesCipherMatchReturnValue = .exact + + let result = await subject.prepareAutofillPasswordsData( + from: [ + .fixture( + login: .fixture( + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + ), + ], + filter: VaultListFilter(uri: "https://example.com"), + ) + + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "prepareRestrictItemsPolicyOrganizations", + ]) + XCTAssertNotNil(result) + XCTAssertNil(cipherMatchingHelper.doesCipherMatchReceivedCipher) + } + /// `prepareAutofillPasswordsData(from:filter:)` returns the prepared data filtering out cipher as it's deleted. @MainActor func test_prepareAutofillPasswordsData_deletedCipher() async throws { @@ -632,7 +996,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi ) cipherMatchingHelper.doesCipherMatchReturnValue = .exact - let result = try await subject.prepareAutofillPasswordsData( + let result = await subject.prepareAutofillPasswordsData( from: [ .fixture( login: .fixture( @@ -655,7 +1019,7 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi /// Tests `prepareGroupData(from:collections:folders:filter:)` generically for most groups. /// - Parameter group: The group to test. private func prepareGroupDataGenericTest(group: VaultListGroup) async throws { - let result = try await subject.prepareGroupData( + let result = await subject.prepareGroupData( from: [.fixture()], collections: [.fixture(id: "1"), .fixture(id: "2")], folders: [.fixture(id: "1"), .fixture(id: "2"), .fixture(id: "3")], diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategy.swift new file mode 100644 index 0000000000..545e9b20a7 --- /dev/null +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategy.swift @@ -0,0 +1,65 @@ +import BitwardenKit +import BitwardenSdk +import Combine + +// MARK: - CombinedMultipleAutofillVaultListDirectorStrategy + +/// The director strategy to be used to build the Autofill's passwords + Fido2 combined in multiple sections. +/// This would show two sections where passwords and Fido2 credentials are displayed in each section accordingly. +struct CombinedMultipleAutofillVaultListDirectorStrategy: VaultListDirectorStrategy { + // MARK: Properties + + /// The factory for creating vault list builders. + let builderFactory: VaultListSectionsBuilderFactory + /// The service used to manage syncing and updates to the user's ciphers. + let cipherService: CipherService + /// A helper to be used on Fido2 flows that requires user interaction and extends the capabilities + /// of the `Fido2UserInterface` from the SDK. + let fido2UserInterfaceHelper: Fido2UserInterfaceHelper + /// The helper used to prepare data for the vault list builder. + let vaultListDataPreparator: VaultListDataPreparator + + func build( + filter: VaultListFilter, + ) async throws -> AsyncThrowingPublisher> { + try await Publishers.CombineLatest( + cipherService.ciphersPublisher(), + fido2UserInterfaceHelper.availableCredentialsForAuthenticationPublisher(), + ) + .asyncTryMap { ciphers, availableFido2Credentials in + try await build( + from: ciphers, + filter: filter, + withFido2Credentials: availableFido2Credentials, + ) + } + .eraseToAnyPublisher() + .values + } + + // MARK: Private methods + + /// Builds the vault list sections. + /// - Parameters: + /// - ciphers: Ciphers to filter and include in the sections. + /// - filter: Filter to be used to build the sections. + /// - withFido2Credentials: Available Fido2 credentials to build the vault list section. + /// - Returns: Sections to be displayed to the user. + private func build( + from ciphers: [Cipher], + filter: VaultListFilter, + withFido2Credentials fido2Credentials: [CipherView]?, + ) async throws -> VaultListData { + guard let preparedData = await vaultListDataPreparator.prepareAutofillCombinedMultipleData( + from: ciphers, + filter: filter, + withFido2Credentials: fido2Credentials, + ) else { + return VaultListData() + } + + return builderFactory.make(withData: preparedData) + .addAutofillCombinedMultipleSection(rpID: filter.rpID) + .build() + } +} diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategyTests.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategyTests.swift new file mode 100644 index 0000000000..54a5371d8c --- /dev/null +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedMultipleAutofillVaultListDirectorStrategyTests.swift @@ -0,0 +1,173 @@ +import BitwardenSdk +import Combine +import XCTest + +@testable import BitwardenShared + +// MARK: - CombinedMultipleAutofillVaultListDirectorStrategyTests + +class CombinedMultipleAutofillVaultListDirectorStrategyTests: BitwardenTestCase { // swiftlint:disable:this type_name + // MARK: Properties + + var cipherService: MockCipherService! + var fido2UserInterfaceHelper: MockFido2UserInterfaceHelper! + var mockCallOrderHelper: MockCallOrderHelper! + var subject: CombinedMultipleAutofillVaultListDirectorStrategy! + var vaultListDataPreparator: MockVaultListDataPreparator! + var vaultListSectionsBuilder: MockVaultListSectionsBuilder! + var vaultListSectionsBuilderFactory: MockVaultListSectionsBuilderFactory! + + // MARK: Setup & Teardown + + override func setUp() { + super.setUp() + + cipherService = MockCipherService() + fido2UserInterfaceHelper = MockFido2UserInterfaceHelper() + vaultListDataPreparator = MockVaultListDataPreparator() + + vaultListSectionsBuilder = MockVaultListSectionsBuilder() + mockCallOrderHelper = vaultListSectionsBuilder.setUpCallOrderHelper() + vaultListSectionsBuilderFactory = MockVaultListSectionsBuilderFactory() + vaultListSectionsBuilderFactory.makeReturnValue = vaultListSectionsBuilder + + subject = CombinedMultipleAutofillVaultListDirectorStrategy( + builderFactory: vaultListSectionsBuilderFactory, + cipherService: cipherService, + fido2UserInterfaceHelper: fido2UserInterfaceHelper, + vaultListDataPreparator: vaultListDataPreparator, + ) + } + + override func tearDown() { + super.tearDown() + + cipherService = nil + fido2UserInterfaceHelper = nil + mockCallOrderHelper = nil + vaultListDataPreparator = nil + vaultListSectionsBuilder = nil + vaultListSectionsBuilderFactory = nil + subject = nil + } + + // MARK: Tests + + /// `build(filter:)` returns empty when preparing data fails to return data. + @MainActor + func test_build_returnsEmptyWhenPreparingDataFailsToReturnData() async throws { + cipherService.ciphersSubject.value = [.fixture()] + fido2UserInterfaceHelper.credentialsForAuthenticationSubject.value = nil + + vaultListDataPreparator.prepareAutofillCombinedMultipleDataReturnValue = nil + + var iteratorPublisher = try await subject.build(filter: VaultListFilter()).makeAsyncIterator() + let vaultListData = try await iteratorPublisher.next() + + XCTAssertEqual(vaultListData, VaultListData()) + } + + /// `build(filter:)` returns empty when preparing data fails to return data with Fido2 credentials. + @MainActor + func test_build_returnsEmptyWhenPreparingDataFailsToReturnDataWithFido2Credentials() async throws { + cipherService.ciphersSubject.value = [.fixture()] + fido2UserInterfaceHelper.credentialsForAuthenticationSubject.value = [.fixture()] + + vaultListDataPreparator.prepareAutofillCombinedMultipleDataReturnValue = nil + + var iteratorPublisher = try await subject.build(filter: VaultListFilter()).makeAsyncIterator() + let vaultListData = try await iteratorPublisher.next() + + XCTAssertEqual(vaultListData, VaultListData()) + } + + /// `build(filter:)` returns the sections built for combined multiple sections autofill without Fido2 credentials. + @MainActor + func test_build_returnsSectionsBuiltForCombinedMultipleSectionsAutofillNoFido2() async throws { + cipherService.ciphersSubject.value = [.fixture()] + fido2UserInterfaceHelper.credentialsForAuthenticationSubject.value = nil + + vaultListDataPreparator.prepareAutofillCombinedMultipleDataReturnValue = VaultListPreparedData() + + vaultListSectionsBuilder.buildReturnValue = VaultListData( + sections: [ + VaultListSection(id: "TestID1", items: [.fixture()], name: "Test1"), + VaultListSection(id: "TestID2", items: [.fixture()], name: "Test2"), + ], + ) + + var iteratorPublisher = try await subject.build( + filter: VaultListFilter( + mode: .combinedMultipleSections, + ), + ).makeAsyncIterator() + let result = try await iteratorPublisher.next() + let vaultListData = try XCTUnwrap(result) + + XCTAssertEqual(vaultListData.sections.map(\.id), ["TestID1", "TestID2"]) + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "addAutofillCombinedMultipleSection", + ]) + } + + /// `build(filter:)` returns the sections built for combined multiple sections autofill with Fido2 credentials. + @MainActor + func test_build_returnsSectionsBuiltForCombinedMultipleSectionsAutofillWithFido2() async throws { + cipherService.ciphersSubject.value = [.fixture(id: "1")] + let fido2Credentials: [CipherView] = [.fixture(id: "1"), .fixture(id: "2")] + fido2UserInterfaceHelper.credentialsForAuthenticationSubject.value = fido2Credentials + + vaultListDataPreparator.prepareAutofillCombinedMultipleDataReturnValue = VaultListPreparedData() + + vaultListSectionsBuilder.buildReturnValue = VaultListData( + sections: [ + VaultListSection(id: "TestID1", items: [.fixture()], name: "Test1"), + VaultListSection(id: "TestID2", items: [.fixture()], name: "Test2"), + VaultListSection(id: "TestID3", items: [.fixture()], name: "Test3"), + ], + ) + + var iteratorPublisher = try await subject.build( + filter: VaultListFilter( + mode: .combinedMultipleSections, + ), + ).makeAsyncIterator() + let result = try await iteratorPublisher.next() + let vaultListData = try XCTUnwrap(result) + + XCTAssertEqual(vaultListData.sections.map(\.id), ["TestID1", "TestID2", "TestID3"]) + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "addAutofillCombinedMultipleSection", + ]) + } + + /// `build(filter:)` returns sections without Fido2 section when Fido2 credentials exist but rpID is nil. + @MainActor + func test_build_withFido2CredentialsButNilRpID_doesNotAddFido2Section() async throws { + cipherService.ciphersSubject.value = [.fixture(id: "1")] + let fido2Credentials: [CipherView] = [.fixture(id: "1"), .fixture(id: "2")] + fido2UserInterfaceHelper.credentialsForAuthenticationSubject.value = fido2Credentials + + vaultListDataPreparator.prepareAutofillCombinedMultipleDataReturnValue = VaultListPreparedData() + + vaultListSectionsBuilder.buildReturnValue = VaultListData( + sections: [ + VaultListSection(id: "TestID1", items: [.fixture()], name: "Test1"), + ], + ) + + var iteratorPublisher = try await subject.build( + filter: VaultListFilter( + mode: .combinedMultipleSections, + rpID: nil, + ), + ).makeAsyncIterator() + let result = try await iteratorPublisher.next() + let vaultListData = try XCTUnwrap(result) + + XCTAssertEqual(vaultListData.sections.map(\.id), ["TestID1"]) + XCTAssertEqual(mockCallOrderHelper.callOrder, [ + "addAutofillCombinedMultipleSection", + ]) + } +} diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedSingleAutofillVaultListDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedSingleAutofillVaultListDirectorStrategy.swift index b05923652a..060ffc5ea7 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedSingleAutofillVaultListDirectorStrategy.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedSingleAutofillVaultListDirectorStrategy.swift @@ -38,7 +38,7 @@ struct CombinedSingleAutofillVaultListDirectorStrategy: VaultListDirectorStrateg from ciphers: [Cipher], filter: VaultListFilter, ) async throws -> VaultListData { - guard let preparedData = try await vaultListDataPreparator.prepareAutofillCombinedSingleData( + guard let preparedData = await vaultListDataPreparator.prepareAutofillCombinedSingleData( from: ciphers, filter: filter, ) else { diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListDirectorStrategy.swift index ee351acd8c..95220d8da3 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListDirectorStrategy.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListDirectorStrategy.swift @@ -51,7 +51,7 @@ struct MainVaultListDirectorStrategy: VaultListDirectorStrategy { ) async throws -> VaultListData { guard !ciphers.isEmpty else { return VaultListData() } - guard let preparedData = try await vaultListDataPreparator.prepareData( + guard let preparedData = await vaultListDataPreparator.prepareData( from: ciphers, collections: collections, folders: folders, diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListGroupDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListGroupDirectorStrategy.swift index 10468b8d7a..4742dd0383 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListGroupDirectorStrategy.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/MainVaultListGroupDirectorStrategy.swift @@ -52,7 +52,7 @@ struct MainVaultListGroupDirectorStrategy: VaultListDirectorStrategy { ) async throws -> VaultListData { guard !ciphers.isEmpty else { return VaultListData() } - guard let preparedGroupData = try await vaultListDataPreparator.prepareGroupData( + guard let preparedGroupData = await vaultListDataPreparator.prepareGroupData( from: ciphers, collections: collections, folders: folders, diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/PasswordsAutofillVaultListDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/PasswordsAutofillVaultListDirectorStrategy.swift index ded288e2d2..b63e7cbece 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/PasswordsAutofillVaultListDirectorStrategy.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/PasswordsAutofillVaultListDirectorStrategy.swift @@ -39,7 +39,7 @@ struct PasswordsAutofillVaultListDirectorStrategy: VaultListDirectorStrategy { ) async throws -> VaultListData { guard !ciphers.isEmpty else { return VaultListData() } - guard let preparedData = try await vaultListDataPreparator.prepareAutofillPasswordsData( + guard let preparedData = await vaultListDataPreparator.prepareAutofillPasswordsData( from: ciphers, filter: filter, ) else { diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategy.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategy.swift index 886b8911f3..ee5f869410 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategy.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategy.swift @@ -4,11 +4,44 @@ import Combine // MARK: - VaultListDirectorStrategy -/// A protocol for a strategy for a vault list director that determines how the vault list sections are built. +/// A strategy pattern protocol that defines how vault list sections are built and displayed. /// -/// The strategies conformed by this protocol are built using a `VaultListDirectorStrategyFactory`. -/// Additionally, typically one strategy goes one-to-one with each vault list representation/view. -/// Example: `MainVaultListDirectorStrategy` is used to build the sections in the main vault tab of the app. +/// ## Overview +/// Each conforming strategy encapsulates the logic for building vault list sections for a specific +/// context or view in the app. The strategy determines: +/// - Which data sources to observe (ciphers, collections, folders) +/// - How to filter and transform that data +/// - What sections and items to display +/// +/// ## Architecture +/// Strategies are created by `VaultListDirectorStrategyFactory` and typically have a one-to-one +/// relationship with vault list views. Each strategy: +/// 1. Subscribes to relevant data publishers (ciphers, collections, folders) +/// 2. Transforms the data based on the provided filter +/// 3. Returns a reactive stream of `VaultListData` that updates when underlying data changes +/// +/// ## Current Implementations +/// - `MainVaultListDirectorStrategy`: Main vault tab showing all items organized by type +/// - `MainVaultListGroupDirectorStrategy`: Main vault filtered by some `VaultListGroup` +/// - `PasswordsAutofillVaultListDirectorStrategy`: Passwords only Autofill extension vault list view +/// - `CombinedSingleAutofillVaultListDirectorStrategy`: Autofill extension vault list view +/// combining passwords + Fido2 credentials in one section. +/// - `CombinedMultipleAutofillVaultListDirectorStrategy`: Autofill extension vault list view +/// combining passwords + Fido2 credentials in different sections. +/// +/// ## Example Usage +/// ```swift +/// let strategy = factory.make(filter: .allVaults) +/// let dataPublisher = try await strategy.build(filter: .allVaults) +/// +/// for try await data in dataPublisher { +/// // Update UI with new vault list sections +/// updateSections(data.sections) +/// } +/// ``` +/// +/// - Note: Strategies are typically struct-based value types with injected dependencies +/// (services, factories) to maintain testability and immutability. protocol VaultListDirectorStrategy { // sourcery: AutoMockable /// Builds the vault list sections. /// - Parameters: diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactory.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactory.swift index 7fe9c135c1..8ce431753a 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactory.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactory.swift @@ -3,6 +3,8 @@ import BitwardenKit // MARK: - VaultListDirectorStrategyFactory /// Factory to create `VaultListDirectorStrategy`. +/// `VaultListDirectorStrategy` implementations should never be created directly - one should always +/// create them by using this factory. protocol VaultListDirectorStrategyFactory { // sourcery: AutoMockable /// Makes a `VaultListDirectorStrategy` from the specified filter. func make(filter: VaultListFilter) -> VaultListDirectorStrategy @@ -16,6 +18,9 @@ struct DefaultVaultListDirectorStrategyFactory: VaultListDirectorStrategyFactory let cipherService: CipherService /// The service for managing the collections for the user. let collectionService: CollectionService + /// A helper to be used on Fido2 flows that requires user interaction and extends the capabilities + /// of the `Fido2UserInterface` from the SDK. + let fido2UserInterfaceHelper: Fido2UserInterfaceHelper /// The service used to manage syncing and updates to the user's folders. let folderService: FolderService /// The factory for creating vault list builders. @@ -25,10 +30,11 @@ struct DefaultVaultListDirectorStrategyFactory: VaultListDirectorStrategyFactory func make(filter: VaultListFilter) -> VaultListDirectorStrategy { switch filter.mode { - case .passwords: - return PasswordsAutofillVaultListDirectorStrategy( + case .combinedMultipleSections: + return CombinedMultipleAutofillVaultListDirectorStrategy( builderFactory: vaultListBuilderFactory, cipherService: cipherService, + fido2UserInterfaceHelper: fido2UserInterfaceHelper, vaultListDataPreparator: vaultListDataPreparator, ) case .combinedSingleSection: @@ -37,6 +43,12 @@ struct DefaultVaultListDirectorStrategyFactory: VaultListDirectorStrategyFactory cipherService: cipherService, vaultListDataPreparator: vaultListDataPreparator, ) + case .passwords: + return PasswordsAutofillVaultListDirectorStrategy( + builderFactory: vaultListBuilderFactory, + cipherService: cipherService, + vaultListDataPreparator: vaultListDataPreparator, + ) default: if filter.group != nil { return MainVaultListGroupDirectorStrategy( diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactoryTests.swift b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactoryTests.swift index c0c78690ff..375f3da0ff 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactoryTests.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListDirectors/VaultListDirectorStrategyFactoryTests.swift @@ -18,6 +18,7 @@ class VaultListDirectorStrategyFactoryTests: BitwardenTestCase { subject = DefaultVaultListDirectorStrategyFactory( cipherService: MockCipherService(), collectionService: MockCollectionService(), + fido2UserInterfaceHelper: MockFido2UserInterfaceHelper(), folderService: MockFolderService(), vaultListBuilderFactory: MockVaultListSectionsBuilderFactory(), vaultListDataPreparator: MockVaultListDataPreparator(), @@ -32,6 +33,13 @@ class VaultListDirectorStrategyFactoryTests: BitwardenTestCase { // MARK: Tests + /// `make(filter:)` returns `CombinedMultipleAutofillVaultListDirectorStrategy` when + /// filtering by mode `.combinedMultipleSections`. + func test_make_returnsCombinedMultipleAutofillVaultListDirectorStrategy() { + let stragegy = subject.make(filter: VaultListFilter(mode: .combinedMultipleSections)) + XCTAssertTrue(stragegy is CombinedMultipleAutofillVaultListDirectorStrategy) + } + /// `make(filter:)` returns `CombinedSingleAutofillVaultListDirectorStrategy` when /// filtering by mode `.combinedSingleSection`. func test_make_returnsCombinedSingleAutofillVaultListDirectorStrategy() { diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilder.swift b/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilder.swift index eea6b58e5e..93ee10d6d4 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilder.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilder.swift @@ -9,6 +9,11 @@ import OSLog /// A protocol for a vault list builder which helps build items and sections for the vault lists. protocol VaultListSectionsBuilder { // sourcery: AutoMockable + /// Adds a section with passwords and Fido2 items combined for Autofill vault list in multiple sections. + /// - Parameter rpID: The relying party identifier of the Fido2 request. + /// - Returns: The builder for fluent code. + func addAutofillCombinedMultipleSection(rpID: String?) -> VaultListSectionsBuilder + /// Adds a section with passwords and Fido2 items for Autofill vault list in a combined single section. /// - Returns: The builder for fluent code. func addAutofillCombinedSingleSection() -> VaultListSectionsBuilder @@ -110,6 +115,32 @@ class DefaultVaultListSectionsBuilder: VaultListSectionsBuilder { // MARK: Methods + func addAutofillCombinedMultipleSection(rpID: String?) -> VaultListSectionsBuilder { + if !preparedData.fido2Items.isEmpty, let rpID { + vaultListData.sections.append(VaultListSection( + id: Localizations.passkeysForX(rpID), + items: preparedData.fido2Items.sorted(using: VaultListItem.defaultSortDescriptor), + name: Localizations.passkeysForX(rpID), + )) + } + + if !preparedData.groupItems.isEmpty { + let passwordsSectionName = if let rpID { + Localizations.passwordsForX(rpID) + } else { + Localizations.passwords + } + + vaultListData.sections.append(VaultListSection( + id: passwordsSectionName, + items: preparedData.groupItems.sorted(using: VaultListItem.defaultSortDescriptor), + name: passwordsSectionName, + )) + } + + return self + } + func addAutofillCombinedSingleSection() -> VaultListSectionsBuilder { guard !preparedData.groupItems.isEmpty || !preparedData.fido2Items.isEmpty else { return self @@ -370,4 +401,4 @@ struct VaultListPreparedData { /// Organization Ids with `.restrictItemTypes` policy enabled. var restrictedOrganizationIds: [String] = [] var totpItemsCount: Int = 0 -} +} // swiftlint:disable:this file_length diff --git a/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilderTests.swift b/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilderTests.swift index cf444ea755..4c8e4a66be 100644 --- a/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilderTests.swift +++ b/BitwardenShared/Core/Vault/Helpers/VaultListSectionsBuilderTests.swift @@ -32,6 +32,115 @@ class VaultListSectionsBuilderTests: BitwardenTestCase { // swiftlint:disable:th // MARK: Tests + /// `addAutofillCombinedMultipleSection()` adds separate sections for passwords and Fido2 items with rpID. + func test_addAutofillCombinedMultipleSection() { + setUpSubject(withData: VaultListPreparedData( + fido2Items: [ + .fixture(cipherListView: .fixture(id: "3", name: "Fido2-1"), fido2CredentialAutofillView: .fixture()), + .fixture(cipherListView: .fixture(id: "6", name: "zFido2-2"), fido2CredentialAutofillView: .fixture()), + ], + groupItems: [ + .fixture(cipherListView: .fixture(id: "1", name: "Password-3")), + .fixture(cipherListView: .fixture(id: "2", name: "Password-1")), + .fixture(cipherListView: .fixture(id: "4", name: "Password-2")), + ], + )) + + let vaultListData = subject.addAutofillCombinedMultipleSection(rpID: "example.com").build() + + assertInlineSnapshot(of: vaultListData.sections.dump(), as: .lines) { + """ + Section[Passkeys for example.com]: Passkeys for example.com + - Cipher: Fido2-1 + - Cipher: zFido2-2 + Section[Passwords for example.com]: Passwords for example.com + - Cipher: Password-1 + - Cipher: Password-2 + - Cipher: Password-3 + """ + } + } + + /// `addAutofillCombinedMultipleSection()` adds only passwords section when no rpID provided. + func test_addAutofillCombinedMultipleSection_noRpID() { + setUpSubject(withData: VaultListPreparedData( + fido2Items: [ + .fixture(cipherListView: .fixture(id: "3", name: "Fido2-1"), fido2CredentialAutofillView: .fixture()), + ], + groupItems: [ + .fixture(cipherListView: .fixture(id: "1", name: "Password-1")), + .fixture(cipherListView: .fixture(id: "2", name: "Password-2")), + ], + )) + + let vaultListData = subject.addAutofillCombinedMultipleSection(rpID: nil).build() + + assertInlineSnapshot(of: vaultListData.sections.dump(), as: .lines) { + """ + Section[Passwords]: Passwords + - Cipher: Password-1 + - Cipher: Password-2 + """ + } + } + + /// `addAutofillCombinedMultipleSection()` adds only passwords section when Fido2 items exist but no rpID. + func test_addAutofillCombinedMultipleSection_onlyPasswords() { + setUpSubject(withData: VaultListPreparedData( + fido2Items: [], + groupItems: [ + .fixture(cipherListView: .fixture(id: "1", name: "Password-3")), + .fixture(cipherListView: .fixture(id: "2", name: "Password-1")), + ], + )) + + let vaultListData = subject.addAutofillCombinedMultipleSection(rpID: "example.com").build() + + assertInlineSnapshot(of: vaultListData.sections.dump(), as: .lines) { + """ + Section[Passwords for example.com]: Passwords for example.com + - Cipher: Password-1 + - Cipher: Password-3 + """ + } + } + + /// `addAutofillCombinedMultipleSection()` adds only Fido2 section when no passwords but rpID provided. + func test_addAutofillCombinedMultipleSection_onlyFido2() { + setUpSubject(withData: VaultListPreparedData( + fido2Items: [ + .fixture(cipherListView: .fixture(id: "1", name: "Fido2-2"), fido2CredentialAutofillView: .fixture()), + .fixture(cipherListView: .fixture(id: "2", name: "Fido2-1"), fido2CredentialAutofillView: .fixture()), + ], + groupItems: [], + )) + + let vaultListData = subject.addAutofillCombinedMultipleSection(rpID: "example.com").build() + + assertInlineSnapshot(of: vaultListData.sections.dump(), as: .lines) { + """ + Section[Passkeys for example.com]: Passkeys for example.com + - Cipher: Fido2-1 + - Cipher: Fido2-2 + """ + } + } + + /// `addAutofillCombinedMultipleSection()` doesn't add any sections when no items available. + func test_addAutofillCombinedMultipleSection_empty() { + setUpSubject(withData: VaultListPreparedData( + fido2Items: [], + groupItems: [], + )) + + let vaultListData = subject.addAutofillCombinedMultipleSection(rpID: "example.com").build() + + assertInlineSnapshot(of: vaultListData.sections.dump(), as: .lines) { + """ + """ + } + } + /// `addAutofillCombinedSingleSection()` adds a vault section combinining passwords and Fido2 items. func test_addAutofillCombinedSingleSection() { setUpSubject(withData: VaultListPreparedData( diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift index 8d2b618208..b006632480 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift @@ -1329,7 +1329,7 @@ extension DefaultVaultRepository: VaultRepository { .values } - func ciphersAutofillPublisher( // swiftlint:disable:this function_body_length + func ciphersAutofillPublisher( availableFido2CredentialsPublisher: AnyPublisher<[BitwardenSdk.CipherView]?, Error>, mode: AutofillListMode, group: VaultListGroup? = nil, @@ -1346,13 +1346,14 @@ extension DefaultVaultRepository: VaultRepository { group: group, ), ) - case .combinedSingleSection, .passwords: + case .combinedMultipleSections, .combinedSingleSection, .passwords: try await vaultListPublisher( filter: VaultListFilter( addTOTPGroup: false, addTrashGroup: false, filterType: .allVaults, mode: mode, + rpID: rpID, uri: uri, ), ) @@ -1364,30 +1365,6 @@ extension DefaultVaultRepository: VaultRepository { group: .totp, ), ) - default: - try await Publishers.CombineLatest( - cipherService.ciphersPublisher(), - availableFido2CredentialsPublisher, - ) - .asyncTryMap { ciphers, availableFido2Credentials in - let decryptedCiphers = try await self.clientService.vault().ciphers() - .decryptListWithFailures(ciphers: ciphers).successes - let matchingCiphers = await DefaultCipherMatchingHelper( - settingsService: self.settingsService, - stateService: self.stateService, - ) - .ciphersMatching(uri: uri, ciphers: decryptedCiphers) - - return try await self.createAutofillListSections( - availableFido2Credentials: availableFido2Credentials, - from: matchingCiphers, - mode: mode, - rpID: rpID, - searchText: nil, - ) - } - .eraseToAnyPublisher() - .values } } diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift index 5fc82c85b5..7a84e8ed53 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift @@ -263,104 +263,88 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b /// returns a publisher for the list of a user's ciphers matching a URI in `.combinedMultipleSections` mode. func test_ciphersAutofillPublisher_mode_combinedMultipleSections() async throws { // swiftlint:disable:previous function_body_length - let expectedCipher = Cipher.fixture( - id: "1", - login: .fixture(uris: [.fixture(uri: "https://bitwarden.com", match: .exact)]), - name: "Bitwarden", - ) - - let cipherFixtures: [Cipher] = [ - expectedCipher, - .fixture( - creationDate: Date(year: 2024, month: 1, day: 1), - id: "2", - login: .fixture(uris: [.fixture(uri: "https://example.com", match: .exact)]), - name: "Example", - revisionDate: Date(year: 2024, month: 1, day: 1), - ), - .fixture(id: "3", login: .fixture(), name: "Café", type: .login), - .fixture(id: "4"), - ] - - let ciphers: [Cipher] = [ - expectedCipher, - cipherFixtures[1], - ] - cipherService.ciphersSubject.value = ciphers - - let expectedCredentialId = Data(repeating: 123, count: 16) - setupDefaultDecryptFido2AutofillCredentialsMocker(expectedCredentialId: expectedCredentialId) - - let expectedCiphersInFido2Section = [ - CipherListView(cipher: expectedCipher), - CipherListView(cipher: cipherFixtures[2]), - CipherListView(cipher: cipherFixtures[3]), - ] - cipherService.fetchCipherByIdResult = { cipherId in - switch cipherId { - case "1": - .success(expectedCipher) - case "3": - .success(cipherFixtures[2]) - case "4": - .success(cipherFixtures[3]) - default: - .success(.fixture()) - } - } - - await fido2UserInterfaceHelper.credentialsForAuthenticationSubject.send([ - CipherView(cipher: expectedCipher), - CipherView(cipher: cipherFixtures[2]), - CipherView(cipher: cipherFixtures[3]), - ]) - - let expectedRpID = "myApp.com" - var iterator = try await subject.ciphersAutofillPublisher( - availableFido2CredentialsPublisher: fido2UserInterfaceHelper - .availableCredentialsForAuthenticationPublisher(), - mode: .combinedMultipleSections, - rpID: expectedRpID, - uri: "https://example.com", - ).makeAsyncIterator() - let vaultListData = try await iterator.next() - let sections = try XCTUnwrap(vaultListData?.sections) - - XCTAssertEqual( - sections[0], + let expectedSections = [ VaultListSection( - id: Localizations.passkeysForX(expectedRpID), - items: expectedCiphersInFido2Section.map { cipherView in + id: Localizations.passkeysForX("myApp.com"), + items: [ VaultListItem( - cipherListView: cipherView, + cipherListView: .fixture( + id: "1", + login: .fixture( + fido2Credentials: [.fixture()], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + name: "Example 1", + creationDate: Date(year: 2024, month: 1, day: 1), + revisionDate: Date(year: 2024, month: 1, day: 1), + ), fido2CredentialAutofillView: .fixture( - credentialId: expectedCredentialId, - cipherId: cipherView.id ?? "", - rpId: expectedRpID, + credentialId: Data(repeating: 123, count: 16), + cipherId: "1", + rpId: "myApp.com", ), - )! - }, - name: Localizations.passkeysForX(expectedRpID), + )!, + VaultListItem( + cipherListView: .fixture( + id: "3", + login: .fixture( + fido2Credentials: [.fixture()], + uris: [.fixture(uri: "https://example.com", match: .exact)], + ), + name: "Example 3", + creationDate: Date(year: 2024, month: 1, day: 1), + revisionDate: Date(year: 2024, month: 1, day: 1), + ), + fido2CredentialAutofillView: .fixture( + credentialId: Data(repeating: 123, count: 16), + cipherId: "3", + rpId: "myApp.com", + ), + )!, + ], + name: Localizations.passkeysForX("myApp.com"), ), - ) - XCTAssertEqual( - sections[1], VaultListSection( - id: Localizations.passwordsForX(expectedRpID), + id: Localizations.passwordsForX("myApp.com"), items: [ VaultListItem( cipherListView: .fixture( id: "2", login: .fixture(uris: [.fixture(uri: "https://example.com", match: .exact)]), - name: "Example", + name: "Example 2", creationDate: Date(year: 2024, month: 1, day: 1), revisionDate: Date(year: 2024, month: 1, day: 1), ), )!, ], - name: Localizations.passwordsForX(expectedRpID), + name: Localizations.passwordsForX("myApp.com"), ), - ) + ] + + let publisher = Just(VaultListData(sections: expectedSections)) + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + + vaultListDirectorStrategy.buildReturnValue = AsyncThrowingPublisher(publisher) + + var iterator = try await subject.ciphersAutofillPublisher( + availableFido2CredentialsPublisher: fido2UserInterfaceHelper + .availableCredentialsForAuthenticationPublisher(), + mode: .combinedMultipleSections, + rpID: "myApp.com", + uri: "https://example.com", + ).makeAsyncIterator() + let publishedSections = try await iterator.next()?.sections + + try assertInlineSnapshot(of: XCTUnwrap(publishedSections).dump(), as: .lines) { + """ + Section[Passkeys for myApp.com]: Passkeys for myApp.com + - Cipher: Example 1 + - Cipher: Example 3 + Section[Passwords for myApp.com]: Passwords for myApp.com + - Cipher: Example 2 + """ + } } /// `ciphersAutofillPublisher(availableFido2CredentialsPublisher:mode:rpID:uri:)`