PM-12313: Update account setup progress when enabling autofill or vault unlock methods (#953)

This commit is contained in:
Matt Czech 2024-09-23 09:07:58 -05:00 committed by GitHub
parent 72cc8bd991
commit ce4f45a6b0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 239 additions and 4 deletions

View File

@ -13,6 +13,10 @@ protocol AutofillCredentialServiceDelegate: AnyObject {
/// A service which manages the ciphers exposed to the system for AutoFill suggestions.
///
protocol AutofillCredentialService: AnyObject {
/// Returns whether autofilling credentials via the extension is enabled.
///
func isAutofillCredentialsEnabled() async -> Bool
/// Returns a `ASPasswordCredential` that matches the user-requested credential which can be
/// used for autofill.
///
@ -263,6 +267,10 @@ class DefaultAutofillCredentialService {
}
extension DefaultAutofillCredentialService: AutofillCredentialService {
func isAutofillCredentialsEnabled() async -> Bool {
await identityStore.state().isEnabled
}
func provideCredential(
for id: String,
autofillCredentialServiceDelegate: AutofillCredentialServiceDelegate,

View File

@ -77,6 +77,17 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
// MARK: Tests
/// `isAutofillCredentialsEnabled()` returns whether autofilling credentials is enabled.
func test_isAutofillCredentialsEnabled() async {
identityStore.state.mockIsEnabled = false
var isEnabled = await subject.isAutofillCredentialsEnabled()
XCTAssertFalse(isEnabled)
identityStore.state.mockIsEnabled = true
isEnabled = await subject.isAutofillCredentialsEnabled()
XCTAssertTrue(isEnabled)
}
/// `provideCredential(for:)` returns the credential containing the username and password for
/// the specified ID.
func test_provideCredential() async throws {

View File

@ -6,10 +6,15 @@ import BitwardenSdk
// MARK: - MockAutofillCredentialService
class MockAutofillCredentialService: AutofillCredentialService {
var isAutofillCredentialsEnabled = false
var provideCredentialPasswordCredential: ASPasswordCredential?
var provideCredentialError: Error?
var provideFido2CredentialResult: Result<PasskeyAssertionCredential, Error> = .failure(BitwardenTestError.example)
func isAutofillCredentialsEnabled() async -> Bool {
isAutofillCredentialsEnabled
}
func provideCredential(
for id: String,
autofillCredentialServiceDelegate: AutofillCredentialServiceDelegate,

View File

@ -6,6 +6,7 @@ import Foundation
class MockStateService: StateService { // swiftlint:disable:this type_body_length
var accountEncryptionKeys = [String: AccountEncryptionKeys]()
var accountSetupAutofill = [String: AccountSetupProgress]()
var accountSetupAutofillError: Error?
var accountSetupVaultUnlock = [String: AccountSetupProgress]()
var accountTokens: Account.AccountTokens?
var accountVolatileData: [String: AccountVolatileData] = [:]
@ -350,6 +351,9 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
func setAccountSetupAutofill(_ autofillSetup: AccountSetupProgress?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
if let accountSetupAutofillError {
throw accountSetupAutofillError
}
accountSetupAutofill[userId] = autofillSetup
}

View File

@ -147,6 +147,7 @@ public class AppProcessor {
await services.migrationService.performMigrations()
await services.environmentService.loadURLsForActiveAccount()
_ = await services.configService.getConfig()
await completeAutofillAccountSetupIfEnabled()
if let initialRoute {
coordinator.navigate(to: initialRoute)
@ -196,6 +197,27 @@ public class AppProcessor {
// MARK: Autofill Methods
/// If the native create account feature flag and the autofill extension are enabled, this marks
/// any user's autofill account setup completed. This should be called on app startup.
///
func completeAutofillAccountSetupIfEnabled() async {
guard await services.configService.getFeatureFlag(.nativeCreateAccountFlow),
await services.autofillCredentialService.isAutofillCredentialsEnabled()
else { return }
do {
let accounts = try await services.stateService.getAccounts()
for account in accounts {
let userId = account.profile.userId
guard let progress = try await services.stateService.getAccountSetupAutofill(userId: userId),
progress != .complete
else { continue }
try await services.stateService.setAccountSetupAutofill(.complete, userId: userId)
}
} catch {
services.errorReporter.log(error: error)
}
}
/// Returns a `ASPasswordCredential` that matches the user-requested credential which can be
/// used for autofill.
///

View File

@ -13,6 +13,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
var authRepository: MockAuthRepository!
var autofillCredentialService: MockAutofillCredentialService!
var clientService: MockClientService!
var configService: MockConfigService!
var coordinator: MockCoordinator<AppRoute, AppEvent>!
var errorReporter: MockErrorReporter!
var fido2UserInterfaceHelper: MockFido2UserInterfaceHelper!
@ -38,6 +39,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
authRepository = MockAuthRepository()
autofillCredentialService = MockAutofillCredentialService()
clientService = MockClientService()
configService = MockConfigService()
coordinator = MockCoordinator()
appModule.authRouter = router
appModule.appCoordinator = coordinator
@ -59,6 +61,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
authRepository: authRepository,
autofillCredentialService: autofillCredentialService,
clientService: clientService,
configService: configService,
errorReporter: errorReporter,
eventService: eventService,
fido2UserInterfaceHelper: fido2UserInterfaceHelper,
@ -81,6 +84,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
authRepository = nil
autofillCredentialService = nil
clientService = nil
configService = nil
coordinator = nil
errorReporter = nil
fido2UserInterfaceHelper = nil
@ -627,6 +631,84 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(migrationService.didPerformMigrations, true)
}
/// `start(navigator:)` doesn't complete the accounts autofill setup if autofill is disabled.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_autofillDisabled() async {
autofillCredentialService.isAutofillCredentialsEnabled = false
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater
let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)
XCTAssertEqual(stateService.accountSetupAutofill, ["1": .setUpLater])
}
/// `start(navigator:)` doesn't complete the accounts autofill setup if the native create
/// account flow feature flag is disabled.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_featureFlagDisabled() async {
autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = false
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater
let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)
XCTAssertEqual(stateService.accountSetupAutofill, ["1": .setUpLater])
}
/// `start(navigator:)` logs an error if one occurs while updating the account's autofill setup.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_error() async {
autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater
stateService.accountSetupAutofillError = BitwardenTestError.example
let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)
XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example])
XCTAssertEqual(stateService.accountSetupAutofill, ["1": .setUpLater])
}
/// `start(navigator:)` doesn't update the user's autofill setup progress if they have no
/// current progress recorded.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_noProgress() async {
autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)
XCTAssertTrue(stateService.accountSetupAutofill.isEmpty)
}
/// `start(navigator:)` completes the user's autofill setup progress if autofill is enabled and
/// they previously choose to set it up later.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_success() async {
autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater
let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)
XCTAssertEqual(stateService.accountSetupAutofill, ["1": .complete])
}
/// `unlockVaultWithNeverlockKey()` unlocks it calling the auth repository.
func test_unlockVaultWithNeverlockKey() async throws {
try await subject.unlockVaultWithNeverlockKey()

View File

@ -14,6 +14,7 @@ final class AccountSecurityProcessor: StateProcessor<
typealias Services = HasAuthRepository
& HasBiometricsRepository
& HasConfigService
& HasErrorReporter
& HasPolicyService
& HasSettingsRepository
@ -124,6 +125,22 @@ final class AccountSecurityProcessor: StateProcessor<
}
}
/// If the native create account feature flag is enabled, this marks the user's vault unlock
/// account setup complete. This should be called whenever PIN or biometrics unlock has been
/// turned on.
///
private func completeAccountSetupVaultUnlockIfNeeded() async {
guard await services.configService.getFeatureFlag(.nativeCreateAccountFlow) else { return }
do {
guard let progress = try await services.stateService.getAccountSetupVaultUnlock(),
progress != .complete
else { return }
try await services.stateService.setAccountSetupVaultUnlock(.complete)
} catch {
services.errorReporter.log(error: error)
}
}
/// Load any initial data for the view.
private func loadData() async {
do {
@ -274,6 +291,10 @@ final class AccountSecurityProcessor: StateProcessor<
// Refresh vault timeout action in case the user doesn't have a password and biometric
// unlock was disabled.
await refreshVaultTimeoutAction()
if enabled {
await completeAccountSetupVaultUnlockIfNeeded()
}
}
/// Shows an alert prompting the user to enter their PIN. If set successfully, the toggle will be turned on.
@ -288,5 +309,9 @@ final class AccountSecurityProcessor: StateProcessor<
// Refresh vault timeout action in case the user doesn't have a password and the pin was disabled.
await refreshVaultTimeoutAction()
if isOn {
await completeAccountSetupVaultUnlockIfNeeded()
}
}
}

View File

@ -8,6 +8,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
var appSettingsStore: MockAppSettingsStore!
var authRepository: MockAuthRepository!
var biometricsRepository: MockBiometricsRepository!
var configService: MockConfigService!
var coordinator: MockCoordinator<SettingsRoute, SettingsEvent>!
var errorReporter: MockErrorReporter!
var policyService: MockPolicyService!
@ -26,6 +27,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
appSettingsStore = MockAppSettingsStore()
authRepository = MockAuthRepository()
biometricsRepository = MockBiometricsRepository()
configService = MockConfigService()
coordinator = MockCoordinator<SettingsRoute, SettingsEvent>()
errorReporter = MockErrorReporter()
policyService = MockPolicyService()
@ -40,6 +42,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
services: ServiceContainer.withMocks(
authRepository: authRepository,
biometricsRepository: biometricsRepository,
configService: configService,
errorReporter: errorReporter,
policyService: policyService,
settingsRepository: settingsRepository,
@ -58,6 +61,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
appSettingsStore = nil
authRepository = nil
biometricsRepository = nil
configService = nil
coordinator = nil
errorReporter = nil
policyService = nil
@ -270,6 +274,8 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
/// `perform(_:)` with `.toggleUnlockWithBiometrics` disables biometrics unlock and updates the state.
@MainActor
func test_perform_toggleUnlockWithBiometrics_disable() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
subject.state.biometricUnlockStatus = .available(.faceID, enabled: true, hasValidIntegrity: true)
vaultUnlockSetupHelper.setBiometricUnlockStatus = .available(
.faceID,
@ -281,6 +287,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
waitFor { !subject.state.biometricUnlockStatus.isEnabled }
XCTAssertTrue(vaultUnlockSetupHelper.setBiometricUnlockCalled)
XCTAssertTrue(stateService.accountSetupVaultUnlock.isEmpty)
XCTAssertFalse(subject.state.biometricUnlockStatus.isEnabled)
}
@ -291,6 +298,8 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
func test_perform_toggleUnlockWithBiometrics_disable_noPassword() async {
authRepository.activeAccount = .fixtureWithTdeNoPassword()
authRepository.sessionTimeoutAction["1"] = .logout
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
subject.state.biometricUnlockStatus = .available(.faceID, enabled: true, hasValidIntegrity: true)
subject.state.sessionTimeoutAction = .lock
vaultUnlockSetupHelper.setBiometricUnlockStatus = .available(
@ -303,6 +312,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
waitFor { !subject.state.biometricUnlockStatus.isEnabled }
XCTAssertTrue(vaultUnlockSetupHelper.setBiometricUnlockCalled)
XCTAssertTrue(stateService.accountSetupVaultUnlock.isEmpty)
XCTAssertFalse(subject.state.biometricUnlockStatus.isEnabled)
XCTAssertEqual(subject.state.sessionTimeoutAction, .logout)
}
@ -310,28 +320,35 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
/// `perform(_:)` with `.toggleUnlockWithBiometrics` enables biometrics unlock and updates the state.
@MainActor
func test_perform_toggleUnlockWithBiometrics_enable() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accountSetupVaultUnlock["1"] = .setUpLater
subject.state.biometricUnlockStatus = .available(.faceID, enabled: true, hasValidIntegrity: true)
vaultUnlockSetupHelper.setBiometricUnlockStatus = .available(
.faceID,
enabled: false,
enabled: true,
hasValidIntegrity: false
)
await subject.perform(.toggleUnlockWithBiometrics(false))
waitFor { !subject.state.biometricUnlockStatus.isEnabled }
await subject.perform(.toggleUnlockWithBiometrics(true))
waitFor { subject.state.biometricUnlockStatus.isEnabled }
XCTAssertTrue(vaultUnlockSetupHelper.setBiometricUnlockCalled)
XCTAssertFalse(subject.state.biometricUnlockStatus.isEnabled)
XCTAssertEqual(stateService.accountSetupVaultUnlock, ["1": .complete])
XCTAssertTrue(subject.state.biometricUnlockStatus.isEnabled)
}
/// `perform(_:)` with `.toggleUnlockWithPINCode` disables pin unlock and updates the state.
@MainActor
func test_perform_toggleUnlockWithPINCode_disable() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
vaultUnlockSetupHelper.setPinUnlockResult = true
await subject.perform(.toggleUnlockWithPINCode(false))
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertTrue(stateService.accountSetupVaultUnlock.isEmpty)
XCTAssertTrue(subject.state.isUnlockWithPINCodeOn)
}
@ -342,12 +359,15 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
func test_perform_toggleUnlockWithPINCode_disable_noPassword() async {
authRepository.activeAccount = .fixtureWithTdeNoPassword()
authRepository.sessionTimeoutAction["1"] = .logout
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
subject.state.sessionTimeoutAction = .lock
vaultUnlockSetupHelper.setPinUnlockResult = true
await subject.perform(.toggleUnlockWithPINCode(false))
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertTrue(stateService.accountSetupVaultUnlock.isEmpty)
XCTAssertTrue(subject.state.isUnlockWithPINCodeOn)
XCTAssertEqual(subject.state.sessionTimeoutAction, .logout)
}
@ -355,6 +375,9 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
/// `perform(_:)` with `.toggleUnlockWithPINCode` enables pin unlock and updates the state.
@MainActor
func test_receive_toggleUnlockWithPINCode_enable() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accountSetupVaultUnlock["1"] = .setUpLater
subject.state.isUnlockWithPINCodeOn = true
vaultUnlockSetupHelper.setPinUnlockResult = false
@ -362,6 +385,60 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
waitFor { !subject.state.isUnlockWithPINCodeOn }
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertEqual(stateService.accountSetupVaultUnlock, ["1": .complete])
XCTAssertFalse(subject.state.isUnlockWithPINCodeOn)
}
/// `perform(_:)` with `.toggleUnlockWithPINCode` doesn't update the user's vault unlock setup
/// progress if the native create account feature flag is disabled.
@MainActor
func test_receive_toggleUnlockWithPINCode_enable_nativeCreateAccountDisabled() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = false
stateService.activeAccount = .fixture()
stateService.accountSetupVaultUnlock["1"] = .setUpLater
subject.state.isUnlockWithPINCodeOn = true
vaultUnlockSetupHelper.setPinUnlockResult = false
await subject.perform(.toggleUnlockWithPINCode(true))
waitFor { !subject.state.isUnlockWithPINCodeOn }
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertEqual(stateService.accountSetupVaultUnlock, ["1": .setUpLater])
XCTAssertFalse(subject.state.isUnlockWithPINCodeOn)
}
/// `perform(_:)` with `.toggleUnlockWithPINCode` doesn't update the user's vault unlock setup
/// progress if they don't yet have any progress.
@MainActor
func test_receive_toggleUnlockWithPINCode_enable_noVaultUnlockSetupProgress() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = false
stateService.activeAccount = .fixture()
subject.state.isUnlockWithPINCodeOn = true
vaultUnlockSetupHelper.setPinUnlockResult = false
await subject.perform(.toggleUnlockWithPINCode(true))
waitFor { !subject.state.isUnlockWithPINCodeOn }
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertTrue(stateService.accountSetupVaultUnlock.isEmpty)
XCTAssertFalse(subject.state.isUnlockWithPINCodeOn)
}
/// `perform(_:)` with `.toggleUnlockWithPINCode` logs an error if one occurs updating the
/// user's vault unlock setup progress.
@MainActor
func test_receive_toggleUnlockWithPINCode_enable_vaultUnlockSetupError() async {
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.accountSetupVaultUnlock["1"] = .setUpLater
subject.state.isUnlockWithPINCodeOn = true
vaultUnlockSetupHelper.setPinUnlockResult = false
await subject.perform(.toggleUnlockWithPINCode(true))
waitFor { !subject.state.isUnlockWithPINCodeOn }
XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount])
XCTAssertTrue(vaultUnlockSetupHelper.setPinUnlockCalled)
XCTAssertEqual(stateService.accountSetupVaultUnlock, ["1": .setUpLater])
XCTAssertFalse(subject.state.isUnlockWithPINCodeOn)
}

View File

@ -50,6 +50,7 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d
& HasAuthRepository
& HasAuthService
& HasBiometricsRepository
& HasConfigService
& HasEnvironmentService
& HasErrorReporter
& HasExportVaultService