Skip to content

Commit

Permalink
BIT-2316: Fix vault timeout migration (#635)
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-livefront authored May 14, 2024
1 parent dc90c1c commit eb275cb
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 67 deletions.
2 changes: 1 addition & 1 deletion BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,7 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le

func setVaultTimeout(value: SessionTimeoutValue, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setVaultTimeout(key: value.rawValue, userId: userId)
appSettingsStore.setVaultTimeout(minutes: value.rawValue, userId: userId)
}

func updateProfile(from response: ProfileResponseModel, userId: String) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,13 @@ protocol AppSettingsStore: AnyObject {
///
func setUnsuccessfulUnlockAttempts(_ attempts: Int, userId: String)

/// Sets the user's session timeout date.
/// Sets the user's session timeout, in minutes.
///
/// - Parameters:
/// - key: The session timeout date.
/// - userId: The user ID associated with the session timeout date.
/// - key: The session timeout, in minutes.
/// - userId: The user ID associated with the session timeout.
///
func setVaultTimeout(key: Int, userId: String)
func setVaultTimeout(minutes: Int, userId: String)

/// Sets the username generation options for a user ID.
///
Expand Down Expand Up @@ -388,10 +388,10 @@ protocol AppSettingsStore: AnyObject {
///
func unsuccessfulUnlockAttempts(userId: String) -> Int

/// Returns the session timeout date.
/// Returns the session timeout in minutes.
///
/// - Parameter userId: The user ID associated with the session timeout date.
/// - Returns: The user's session timeout date.
/// - Parameter userId: The user ID associated with the session timeout.
/// - Returns: The user's session timeout in minutes.
///
func vaultTimeout(userId: String) -> Int?

Expand Down Expand Up @@ -864,8 +864,8 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(options, for: .usernameGenerationOptions(userId: userId))
}

func setVaultTimeout(key: Int, userId: String) {
store(key, for: .vaultTimeout(userId: userId))
func setVaultTimeout(minutes: Int, userId: String) {
store(minutes, for: .vaultTimeout(userId: userId))
}

func timeoutAction(userId: String) -> Int? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_

/// `.vaultTimeout(userId:)` returns the correct vault timeout value.
func test_vaultTimeout() throws {
subject.setVaultTimeout(key: 60, userId: "1")
subject.setVaultTimeout(minutes: 60, userId: "1")

XCTAssertEqual(subject.vaultTimeout(userId: "1"), 60)
XCTAssertEqual(userDefaults.double(forKey: "bwPreferencesStorage:vaultTimeout_1"), 60)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ class MockAppSettingsStore: AppSettingsStore {
usernameGenerationOptions[userId] = options
}

func setVaultTimeout(key: Int, userId: String) {
vaultTimeout[userId] = key
func setVaultTimeout(minutes: Int, userId: String) {
vaultTimeout[userId] = minutes
}

func shouldTrustDevice(userId: String) -> Bool? {
Expand Down
2 changes: 1 addition & 1 deletion BitwardenShared/Core/Vault/Services/PolicyService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ extension DefaultPolicyService {

for policy in policies {
guard let policyTimeoutValue = policy[.minutes]?.intValue else { continue }
timeoutValue = policyTimeoutValue * 60
timeoutValue = policyTimeoutValue

// If the policy's timeout action is not lock or logOut, there is no policy timeout action.
// In that case, we would present both timeout action options to the user.
Expand Down
4 changes: 2 additions & 2 deletions BitwardenShared/Core/Vault/Services/PolicyServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class PolicyServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod

let policyValues = try await subject.fetchTimeoutPolicyValues()

XCTAssertEqual(policyValues?.value, 60 * 60)
XCTAssertEqual(policyValues?.value, 60)
XCTAssertEqual(policyValues?.action, .lock)
}

Expand All @@ -334,7 +334,7 @@ class PolicyServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod

let policyValues = try await subject.fetchTimeoutPolicyValues()

XCTAssertEqual(policyValues?.value, 60 * 60)
XCTAssertEqual(policyValues?.value, 60)
XCTAssertNil(policyValues?.action)
}

Expand Down
2 changes: 1 addition & 1 deletion BitwardenShared/Core/Vault/Services/SyncServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class SyncServiceTests: BitwardenTestCase {
stateService.activeAccount = .fixture()
stateService.vaultTimeout["1"] = .never

policyService.fetchTimeoutPolicyValuesResult = .success((.lock, 15 * 60))
policyService.fetchTimeoutPolicyValuesResult = .success((.lock, 15))

try await subject.fetchSync(forceSync: false)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct CountdownDatePicker: UIViewRepresentable {

// MARK: Properties

/// The count down durration.
/// The count down duration, in seconds.
@Binding var duration: Int

// MARK: Methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ enum AccountSecurityAction: Equatable {
/// Clears the two step login URL after the web app has been opened.
case clearTwoStepLoginUrl

/// Sets the custom session timeout value in seconds.
case customTimeoutValueSecondsChanged(Int)

/// The delete account button was pressed.
case deleteAccountPressed

Expand All @@ -26,9 +29,6 @@ enum AccountSecurityAction: Equatable {
/// The session timeout value has changed.
case sessionTimeoutValueChanged(SessionTimeoutValue)

/// Sets the custom session timeout value.
case customTimeoutValueChanged(Int)

/// Unlock with pin code was toggled.
case toggleUnlockWithPINCode(Bool)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ final class AccountSecurityProcessor: StateProcessor<
state.fingerprintPhraseUrl = nil
case .clearTwoStepLoginUrl:
state.twoStepLoginUrl = nil
case let .customTimeoutValueChanged(newValue):
setVaultTimeout(value: .custom(newValue))
case let .customTimeoutValueSecondsChanged(seconds):
setVaultTimeout(value: .custom(seconds / 60))
case .deleteAccountPressed:
coordinator.navigate(to: .deleteAccount)
case .logout:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th

/// `perform(_:)` with `.appeared` sets the policy related state properties when the policy is enabled.
func test_perform_appeared_timeoutPolicyEnabled() async throws {
policyService.fetchTimeoutPolicyValuesResult = .success((.logout, 3600))
policyService.fetchTimeoutPolicyValuesResult = .success((.logout, 60))

await subject.perform(.appeared)

XCTAssertEqual(subject.state.isTimeoutPolicyEnabled, true)
XCTAssertEqual(subject.state.policyTimeoutAction, .logout)
XCTAssertEqual(subject.state.policyTimeoutValue, 3600)
XCTAssertEqual(subject.state.policyTimeoutValue, 60)
XCTAssertEqual(subject.state.availableTimeoutActions, [.logout])
XCTAssertEqual(subject.state.policyTimeoutHours, 1)
XCTAssertEqual(subject.state.policyTimeoutMinutes, 0)
Expand All @@ -106,12 +106,12 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
/// `perform(_:)` with `.appeared` sets the policy related state properties when the policy is enabled,
/// but the policy doesn't return an action.
func test_perform_appeared_timeoutPolicyEnabled_noPolicyAction() async throws {
policyService.fetchTimeoutPolicyValuesResult = .success((nil, 3660))
policyService.fetchTimeoutPolicyValuesResult = .success((nil, 61))

await subject.perform(.appeared)

XCTAssertEqual(subject.state.isTimeoutPolicyEnabled, true)
XCTAssertEqual(subject.state.policyTimeoutValue, 3660)
XCTAssertEqual(subject.state.policyTimeoutValue, 61)
XCTAssertEqual(subject.state.availableTimeoutActions, [.lock, .logout])
XCTAssertEqual(subject.state.policyTimeoutHours, 1)
XCTAssertEqual(subject.state.policyTimeoutMinutes, 1)
Expand All @@ -131,7 +131,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th

/// `perform(_:)` with `.appeared` sets the policy related state properties when the policy is enabled.
func test_perform_appeared_timeoutPolicyEnabled_oddTime() async throws {
policyService.fetchTimeoutPolicyValuesResult = .success((.logout, 3660))
policyService.fetchTimeoutPolicyValuesResult = .success((.logout, 61))

await subject.perform(.appeared)

Expand Down Expand Up @@ -234,6 +234,17 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
XCTAssertNil(subject.state.twoStepLoginUrl)
}

/// `receive(_:)` with `customTimeoutValueSecondsChanged(_:)` updates the custom session timeout value in the state.
func test_receive_customTimeoutValueSecondsChanged() {
XCTAssertEqual(subject.state.customTimeoutValueSeconds, 60)

let account = Account.fixture()
authRepository.activeAccount = account

subject.receive(.customTimeoutValueSecondsChanged(120))
waitFor(subject.state.customTimeoutValueSeconds == 120)
}

/// `receive(_:)` with `.deleteAccountPressed` shows the `DeleteAccountView`.
func test_receive_deleteAccountPressed() throws {
subject.receive(.deleteAccountPressed)
Expand Down Expand Up @@ -406,7 +417,7 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
let account = Account.fixture()
authRepository.activeAccount = account
subject.state.isTimeoutPolicyEnabled = true
subject.state.policyTimeoutValue = 60
subject.state.policyTimeoutValue = 1

subject.receive(.sessionTimeoutValueChanged(.fourHours))

Expand All @@ -430,17 +441,6 @@ class AccountSecurityProcessorTests: BitwardenTestCase { // swiftlint:disable:th
waitFor(errorReporter.errors.last as? BitwardenTestError == BitwardenTestError.example)
}

/// `receive(_:)` with `setCustomSessionTimeoutValue(_:)` updates the custom session timeout value in the state.
func test_receive_setCustomSessionTimeoutValue() {
XCTAssertEqual(subject.state.customTimeoutValue, 60)

let account = Account.fixture()
authRepository.activeAccount = account

subject.receive(.customTimeoutValueChanged(120))
waitFor(subject.state.customTimeoutValue == 120)
}

/// `receive(_:)` with `.toggleUnlockWithPINCode` updates the state when submit has been pressed.
func test_receive_toggleUnlockWithPINCode_toggleOff() {
subject.state.isUnlockWithPINCodeOn = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ public enum SessionTimeoutValue: RawRepresentable, CaseIterable, Equatable, Menu
public var rawValue: Int {
switch self {
case .immediately: 0
case .oneMinute: 60
case .fiveMinutes: 300
case .fifteenMinutes: 900
case .thirtyMinutes: 1800
case .oneHour: 3600
case .fourHours: 14400
case .oneMinute: 1
case .fiveMinutes: 5
case .fifteenMinutes: 15
case .thirtyMinutes: 30
case .oneHour: 60
case .fourHours: 240
case .onAppRestart: -1
case .never: -2
case let .custom(customValue): customValue
Expand All @@ -109,17 +109,17 @@ public enum SessionTimeoutValue: RawRepresentable, CaseIterable, Equatable, Menu
switch rawValue {
case 0:
self = .immediately
case 60:
case 1:
self = .oneMinute
case 300:
case 5:
self = .fiveMinutes
case 900:
case 15:
self = .fifteenMinutes
case 1800:
case 30:
self = .thirtyMinutes
case 3600:
case 60:
self = .oneHour
case 14400:
case 240:
self = .fourHours
case -1:
self = .onAppRestart
Expand Down Expand Up @@ -218,20 +218,20 @@ struct AccountSecurityState: Equatable {

/// The accessibility label used for the custom timeout value.
var customTimeoutAccessibilityLabel: String {
customTimeoutValue.timeInHoursMinutes(shouldSpellOut: true)
customTimeoutValueSeconds.timeInHoursMinutes(shouldSpellOut: true)
}

/// The custom session timeout value, initially set to 60 seconds.
var customTimeoutValue: Int {
guard case let .custom(customValue) = sessionTimeoutValue else {
/// The custom session timeout value, in seconds, initially set to 60 seconds.
var customTimeoutValueSeconds: Int {
guard case let .custom(customValueInMinutes) = sessionTimeoutValue, customValueInMinutes > 0 else {
return 60
}
return customValue
return customValueInMinutes * 60
}

/// The string representation of the custom session timeout value.
var customTimeoutString: String {
customTimeoutValue.timeInHoursMinutes()
customTimeoutValueSeconds.timeInHoursMinutes()
}

/// Whether the user has a method to unlock the vault (master password, pin set, or biometrics
Expand All @@ -258,11 +258,11 @@ struct AccountSecurityState: Equatable {

/// The policy's timeout value in hours.
var policyTimeoutHours: Int {
policyTimeoutValue / (60 * 60)
policyTimeoutValue / 60
}

/// The policy's timeout value in minutes.
var policyTimeoutMinutes: Int {
policyTimeoutValue / 60 % 60
policyTimeoutValue % 60
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ struct AccountSecurityView: View {
title: Localizations.custom,
customTimeoutValue: store.state.customTimeoutString,
pickerValue: store.binding(
get: \.customTimeoutValue,
send: AccountSecurityAction.customTimeoutValueChanged
get: \.customTimeoutValueSeconds,
send: AccountSecurityAction.customTimeoutValueSecondsChanged
),
customTimeoutAccessibilityLabel: store.state.customTimeoutAccessibilityLabel
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class AccountSecurityViewTests: BitwardenTestCase {
enabled: false,
hasValidIntegrity: true
),
sessionTimeoutValue: .custom(60)
sessionTimeoutValue: .custom(1)
)
)
)
Expand All @@ -142,7 +142,7 @@ class AccountSecurityViewTests: BitwardenTestCase {
enabled: true,
hasValidIntegrity: true
),
sessionTimeoutValue: .custom(60)
sessionTimeoutValue: .custom(1)
)
)
)
Expand All @@ -161,7 +161,7 @@ class AccountSecurityViewTests: BitwardenTestCase {
enabled: true,
hasValidIntegrity: false
),
sessionTimeoutValue: .custom(60)
sessionTimeoutValue: .custom(1)
)
)
)
Expand All @@ -174,7 +174,7 @@ class AccountSecurityViewTests: BitwardenTestCase {
let subject = AccountSecurityView(
store: Store(
processor: StateProcessor(
state: AccountSecurityState(sessionTimeoutValue: .custom(60))
state: AccountSecurityState(sessionTimeoutValue: .custom(1))
)
)
)
Expand Down Expand Up @@ -203,7 +203,7 @@ class AccountSecurityViewTests: BitwardenTestCase {
processor: StateProcessor(
state: AccountSecurityState(
isTimeoutPolicyEnabled: true,
sessionTimeoutValue: .custom(60)
sessionTimeoutValue: .custom(1)
)
)
)
Expand Down

0 comments on commit eb275cb

Please sign in to comment.