Skip to content

Commit

Permalink
PM-10270: Set up unlock later (#873)
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-livefront authored Sep 17, 2024
1 parent 54ad9f5 commit a91c7f6
Show file tree
Hide file tree
Showing 17 changed files with 257 additions and 29 deletions.
41 changes: 41 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,13 @@ protocol StateService: AnyObject {
///
func getMasterPasswordHash(userId: String?) async throws -> String?

/// Gets whether the user needs to set up vault unlock methods.
///
/// - Parameter userId: The user ID associated with the value.
/// - Returns: Whether the user needs to set up vault unlock methods.
///
func getNeedsVaultUnlockSetup(userId: String?) async throws -> Bool

/// Gets the last notifications registration date for a user ID.
///
/// - Parameter userId: The user ID of the account. Defaults to the active account if `nil`.
Expand Down Expand Up @@ -464,6 +471,14 @@ protocol StateService: AnyObject {
///
func setMasterPasswordHash(_ hash: String?, userId: String?) async throws

/// Sets whether the user needs to set up vault unlock methods.
///
/// - Parameters:
/// - needsVaultUnlockSetup: Whether the user needs to set up vault unlock methods.
/// - userId: The user ID associated with the value.
///
func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String?) async throws

/// Sets the last notifications registration date for a user ID.
///
/// - Parameters:
Expand Down Expand Up @@ -750,6 +765,14 @@ extension StateService {
try await getMasterPasswordHash(userId: nil)
}

/// Gets whether the active account needs to set up vault unlock methods.
///
/// - Returns: Whether the user needs to set up vault unlock methods.
///
func getNeedsVaultUnlockSetup() async throws -> Bool {
try await getNeedsVaultUnlockSetup(userId: nil)
}

/// Gets the last notifications registration date for the active account.
///
/// - Returns: The last notifications registration date for the active account.
Expand Down Expand Up @@ -936,6 +959,14 @@ extension StateService {
try await setMasterPasswordHash(hash, userId: nil)
}

/// Sets whether the active account needs to set up vault unlock methods.
///
/// - Parameter needsVaultUnlockSetup: Whether the user needs to set up vault unlock methods.
///
func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool) async throws {
try await setNeedsVaultUnlockSetup(needsVaultUnlockSetup, userId: nil)
}

/// Sets the last notifications registration date for the active account.
///
/// - Parameter date: The last notifications registration date.
Expand Down Expand Up @@ -1240,6 +1271,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
return appSettingsStore.masterPasswordHash(userId: userId)
}

func getNeedsVaultUnlockSetup(userId: String?) async throws -> Bool {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.needsVaultUnlockSetup(userId: userId)
}

func getNotificationsLastRegistrationDate(userId: String?) async throws -> Date? {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.notificationsLastRegistrationDate(userId: userId)
Expand Down Expand Up @@ -1461,6 +1497,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.setMasterPasswordHash(hash, userId: userId)
}

func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setNeedsVaultUnlockSetup(needsVaultUnlockSetup, userId: userId)
}

func setNotificationsLastRegistrationDate(_ date: Date?, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setNotificationsLastRegistrationDate(date, userId: userId)
Expand Down
30 changes: 30 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,25 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
}
}

/// `getNeedsVaultUnlockSetup()` returns whether the user needs to set up vault unlock methods.
func test_getNeedsVaultUnlockSetup() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))

let initialValue = try await subject.getNeedsVaultUnlockSetup()
XCTAssertFalse(initialValue)

appSettingsStore.needsVaultUnlockSetup["1"] = true
let needsVaultUnlockSetup = try await subject.getNeedsVaultUnlockSetup()
XCTAssertTrue(needsVaultUnlockSetup)
}

/// `getNeedsVaultUnlockSetup()` throws an error if there isn't an active account.
func test_getNeedsVaultUnlockSetup_noAccount() async throws {
await assertAsyncThrows(error: StateServiceError.noActiveAccount) {
_ = try await subject.getNeedsVaultUnlockSetup()
}
}

/// `getNotificationsLastRegistrationDate()` returns the user's last notifications registration date.
func test_getNotificationsLastRegistrationDate() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
Expand Down Expand Up @@ -1535,6 +1554,17 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(appSettingsStore.masterPasswordHashes, ["1": "1234"])
}

/// `setNeedsVaultUnlockSetup(_:)` sets whether the user needs to set up vault unlock methods.
func test_setNeedsVaultUnlockSetup() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))

try await subject.setNeedsVaultUnlockSetup(true)
XCTAssertEqual(appSettingsStore.needsVaultUnlockSetup, ["1": true])

try await subject.setNeedsVaultUnlockSetup(false, userId: "1")
XCTAssertEqual(appSettingsStore.needsVaultUnlockSetup, ["1": false])
}

/// `setNotificationsLastRegistrationDate(_:)` sets the last notifications registration date for a user.
func test_setNotificationsLastRegistrationDate() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ protocol AppSettingsStore: AnyObject {
///
func masterPasswordHash(userId: String) -> String?

/// Gets whether the user needs to set up vault unlock methods.
///
/// - Parameter userId: The user ID associated with the value.
/// - Returns: Whether the user needs to set up vault unlock methods.
///
func needsVaultUnlockSetup(userId: String) -> Bool

/// Gets the last date the user successfully registered for push notifications.
///
/// - Parameter userId: The user ID associated with the last notifications registration date.
Expand Down Expand Up @@ -305,6 +312,14 @@ protocol AppSettingsStore: AnyObject {
///
func setMasterPasswordHash(_ hash: String?, userId: String)

/// Sets whether the user needs to set up vault unlock methods.
///
/// - Parameters:
/// - needsVaultUnlockSetup: Whether the user needs to set up vault unlock methods.
/// - userId: The user ID associated with the value.
///
func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String)

/// Sets the last notifications registration date for a user ID.
///
/// - Parameters:
Expand Down Expand Up @@ -587,6 +602,7 @@ extension DefaultAppSettingsStore: AppSettingsStore {
case loginRequest
case masterPasswordHash(userId: String)
case migrationVersion
case needsVaultUnlockSetup(userId: String)
case notificationsLastRegistrationDate(userId: String)
case passwordGenerationOptions(userId: String)
case pinProtectedUserKey(userId: String)
Expand Down Expand Up @@ -656,6 +672,8 @@ extension DefaultAppSettingsStore: AppSettingsStore {
key = "keyHash_\(userId)"
case .migrationVersion:
key = "migrationVersion"
case let .needsVaultUnlockSetup(userId):
key = "needsVaultUnlockSetup_\(userId)"
case let .notificationsLastRegistrationDate(userId):
key = "pushLastRegistrationDate_\(userId)"
case let .passwordGenerationOptions(userId):
Expand Down Expand Up @@ -836,6 +854,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
fetch(for: .masterPasswordHash(userId: userId))
}

func needsVaultUnlockSetup(userId: String) -> Bool {
fetch(for: .needsVaultUnlockSetup(userId: userId))
}

func notificationsLastRegistrationDate(userId: String) -> Date? {
fetch(for: .notificationsLastRegistrationDate(userId: userId)).map { Date(timeIntervalSince1970: $0) }
}
Expand Down Expand Up @@ -914,6 +936,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(hash, for: .masterPasswordHash(userId: userId))
}

func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String) {
store(needsVaultUnlockSetup, for: .needsVaultUnlockSetup(userId: userId))
}

func setNotificationsLastRegistrationDate(_ date: Date?, userId: String) {
store(date?.timeIntervalSince1970, for: .notificationsLastRegistrationDate(userId: userId))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,23 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_
XCTAssertEqual(userDefaults.double(forKey: "bwPreferencesStorage:pushLastRegistrationDate_2"), 1_696_204_800.0)
}

/// `needsVaultUnlockSetup(userId:)` returns `false` if there isn't a previously stored value.
func test_needsVaultUnlockSetup_isInitiallyFalse() {
XCTAssertFalse(subject.needsVaultUnlockSetup(userId: "-1"))
}

/// `needsVaultUnlockSetup(userId:)` can be used to get whether the user needs to set up vault
/// unlock methods.
func test_needsVaultUnlockSetup__withValue() {
subject.setNeedsVaultUnlockSetup(true, userId: "1")
subject.setNeedsVaultUnlockSetup(false, userId: "2")

XCTAssertTrue(subject.needsVaultUnlockSetup(userId: "1"))
XCTAssertFalse(subject.needsVaultUnlockSetup(userId: "2"))
XCTAssertTrue(userDefaults.bool(forKey: "bwPreferencesStorage:needsVaultUnlockSetup_1"))
XCTAssertFalse(userDefaults.bool(forKey: "bwPreferencesStorage:needsVaultUnlockSetup_2"))
}

/// `passwordGenerationOptions(userId:)` returns `nil` if there isn't a previously stored value.
func test_passwordGenerationOptions_isInitiallyNil() {
XCTAssertNil(subject.passwordGenerationOptions(userId: "-1"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class MockAppSettingsStore: AppSettingsStore {
var lastActiveTime = [String: Date]()
var lastSyncTimeByUserId = [String: Date]()
var masterPasswordHashes = [String: String]()
var needsVaultUnlockSetup = [String: Bool]()
var notificationsLastRegistrationDates = [String: Date]()
var passwordGenerationOptions = [String: PasswordGenerationOptions]()
var pinProtectedUserKey = [String: String]()
Expand Down Expand Up @@ -101,6 +102,10 @@ class MockAppSettingsStore: AppSettingsStore {
masterPasswordHashes[userId]
}

func needsVaultUnlockSetup(userId: String) -> Bool {
needsVaultUnlockSetup[userId] ?? false
}

func notificationsLastRegistrationDate(userId: String) -> Date? {
notificationsLastRegistrationDates[userId]
}
Expand Down Expand Up @@ -181,6 +186,10 @@ class MockAppSettingsStore: AppSettingsStore {
notificationsLastRegistrationDates[userId] = date
}

func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String) {
self.needsVaultUnlockSetup[userId] = needsVaultUnlockSetup
}

func setPasswordGenerationOptions(_ options: PasswordGenerationOptions?, userId: String) {
guard let options else {
passwordGenerationOptions.removeValue(forKey: userId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var lastSyncTimeSubject = CurrentValueSubject<Date?, Never>(nil)
var lastUserShouldConnectToWatch = false
var masterPasswordHashes = [String: String]()
var needsVaultUnlockSetup = [String: Bool]()
var notificationsLastRegistrationDates = [String: Date]()
var notificationsLastRegistrationError: Error?
var passwordGenerationOptions = [String: PasswordGenerationOptions]()
Expand Down Expand Up @@ -232,6 +233,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
return masterPasswordHashes[userId]
}

func getNeedsVaultUnlockSetup(userId: String?) async throws -> Bool {
let userId = try unwrapUserId(userId)
return needsVaultUnlockSetup[userId] ?? false
}

func getNotificationsLastRegistrationDate(userId: String?) async throws -> Date? {
if let notificationsLastRegistrationError {
throw notificationsLastRegistrationError
Expand Down Expand Up @@ -429,6 +435,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
masterPasswordHashes[userId] = hash
}

func setNeedsVaultUnlockSetup(_ needsVaultUnlockSetup: Bool, userId: String?) async throws {
let userId = try unwrapUserId(userId)
self.needsVaultUnlockSetup[userId] = needsVaultUnlockSetup
}

func setNotificationsLastRegistrationDate(_ date: Date?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
notificationsLastRegistrationDates[userId] = date
Expand Down
3 changes: 3 additions & 0 deletions BitwardenShared/UI/Auth/AuthCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ final class AuthCoordinator: NSObject, // swiftlint:disable:this type_body_lengt

func navigate(to route: AuthRoute, context: AnyObject?) { // swiftlint:disable:this function_body_length
switch route {
case .autofillSetup:
// TODO: PM-10278 Add autofill setup screen
break
case let .captcha(url, callbackUrlScheme):
showCaptcha(
url: url,
Expand Down
3 changes: 3 additions & 0 deletions BitwardenShared/UI/Auth/AuthRoute.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import Foundation

/// A route to a specific screen in the authentication flow.
public enum AuthRoute: Equatable {
/// A route to the autofill setup screen.
case autofillSetup

/// A route to the captcha screen.
case captcha(url: URL, callbackUrlScheme: String)

Expand Down
21 changes: 21 additions & 0 deletions BitwardenShared/UI/Auth/Extensions/Alert+Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,27 @@ extension Alert {
)
}

/// An alert confirming that the user wants to finish setting up their vault unlock methods
/// later in settings.
///
/// - Parameter action: The action taken when the user taps on Confirm to finish setting up
/// their vault unlock methods later in settings.
/// - Returns: An alert confirming that the user wants to finish setting up their vault unlock
/// methods later in settings.
///
static func setUpUnlockMethodLater(action: @escaping () async -> Void) -> Alert {
Alert(
title: Localizations.setUpLaterQuestion,
message: Localizations.youCanFinishSetupUnlockAnytimeDescriptionLong,
alertActions: [
AlertAction(title: Localizations.cancel, style: .cancel),
AlertAction(title: Localizations.confirm, style: .default) { _ in
await action()
},
]
)
}

/// An alert asking the user if they want to switch to the already existing account when adding
/// a new account.
///
Expand Down
22 changes: 22 additions & 0 deletions BitwardenShared/UI/Auth/Extensions/AlertAuthTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,28 @@ class AlertAuthTests: BitwardenTestCase {
await fulfillment(of: [expectation], timeout: 3)
}

/// `setUpUnlockMethodLater(action:)` builds an `Alert` confirming the user wants to set up
/// their unlock methods later.
func test_setUpUnlockMethodLater() async throws {
var actionCalled = false
let subject = Alert.setUpUnlockMethodLater {
actionCalled = true
}

XCTAssertEqual(subject.title, Localizations.setUpLaterQuestion)
XCTAssertEqual(subject.message, Localizations.youCanFinishSetupUnlockAnytimeDescriptionLong)
XCTAssertEqual(subject.preferredStyle, .alert)
XCTAssertEqual(subject.alertActions.count, 2)
XCTAssertEqual(subject.alertActions[0].title, Localizations.cancel)
XCTAssertEqual(subject.alertActions[1].title, Localizations.confirm)

try await subject.tapAction(title: Localizations.cancel)
XCTAssertFalse(actionCalled)

try await subject.tapAction(title: Localizations.confirm)
XCTAssertTrue(actionCalled)
}

/// `switchToExistingAccount(action:)` builds an `Alert` for switching to an existing account.
func test_switchToExistingAccount() async throws {
var actionCalled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
/// Actions that can be processed by a `VaultUnlockSetupProcessor`.
///
enum VaultUnlockSetupAction: Equatable {
/// The continue button was tapped.
case continueFlow

/// The set up later button was tapped.
case setUpLater
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
/// Effects handled by the `VaultUnlockSetupProcessor`.
///
enum VaultUnlockSetupEffect: Equatable {
/// The continue button was tapped.
case continueFlow

/// Any initial data for the view should be loaded.
case loadData

Expand Down
Loading

0 comments on commit a91c7f6

Please sign in to comment.