Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIT-2316: Fix vault timeout migration #635

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
Loading