Skip to content

Commit 29cc027

Browse files
PM-9002: Fix vault early timeout (#732)
1 parent af1798d commit 29cc027

File tree

3 files changed

+87
-15
lines changed

3 files changed

+87
-15
lines changed

BitwardenShared/Core/Vault/Services/VaultTimeoutService.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,7 @@ class DefaultVaultTimeoutService: VaultTimeoutService {
117117
// MARK: Methods
118118

119119
func hasPassedSessionTimeout(userId: String) async throws -> Bool {
120-
guard let lastActiveTime = try await stateService.getLastActiveTime(userId: userId) else { return true }
121120
let vaultTimeout = try await sessionTimeoutValue(userId: userId)
122-
123121
switch vaultTimeout {
124122
case .never,
125123
.onAppRestart:
@@ -128,8 +126,11 @@ class DefaultVaultTimeoutService: VaultTimeoutService {
128126
return false
129127
default:
130128
// Otherwise, calculate a timeout.
129+
guard let lastActiveTime = try await stateService.getLastActiveTime(userId: userId)
130+
else { return true }
131+
131132
return timeProvider.presentTime.timeIntervalSince(lastActiveTime)
132-
>= TimeInterval(vaultTimeout.rawValue)
133+
>= TimeInterval(vaultTimeout.seconds)
133134
}
134135
}
135136

BitwardenShared/Core/Vault/Services/VaultTimeoutServiceTests.swift

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,39 +45,104 @@ final class VaultTimeoutServiceTests: BitwardenTestCase {
4545

4646
// MARK: Tests
4747

48-
/// `.hasPassedSessionTimeout()` returns false if the user should not be timed out.
49-
func test_hasPassedSessionTimeout_false() async throws {
48+
/// `.hasPassedSessionTimeout()` returns true if the user should be timed out.
49+
func test_hasPassedSessionTimeout() async throws {
5050
let account = Account.fixture()
5151
stateService.activeAccount = account
52-
stateService.lastActiveTime[account.profile.userId] = Date()
53-
stateService.vaultTimeout[account.profile.userId] = .custom(120)
52+
stateService.vaultTimeout[account.profile.userId] = .fiveMinutes
5453

55-
let shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
54+
let currentTime = Date(year: 2024, month: 1, day: 2, hour: 6, minute: 0)
55+
timeProvider.timeConfig = .mockTime(currentTime)
56+
57+
// Last active 4 minutes ago, no timeout.
58+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
59+
.date(byAdding: .minute, value: -4, to: currentTime)
60+
var shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
5661
XCTAssertFalse(shouldTimeout)
62+
63+
// Last active 5 minutes ago, timeout.
64+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
65+
.date(byAdding: .minute, value: -5, to: currentTime)
66+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
67+
XCTAssertTrue(shouldTimeout)
68+
69+
// Last active 6 minutes ago, timeout.
70+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
71+
.date(byAdding: .minute, value: -6, to: currentTime)
72+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
73+
XCTAssertTrue(shouldTimeout)
74+
75+
// Last active in the distant past, timeout.
76+
stateService.lastActiveTime[account.profile.userId] = .distantPast
77+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
78+
XCTAssertTrue(shouldTimeout)
5779
}
5880

59-
/// `.hasPassedSessionTimeout()` returns false if the user's vault timeout value is negative.
60-
func test_hasPassedSessionTimeout_never() async throws {
81+
/// `.hasPassedSessionTimeout()` returns false for a timeout value of app restart.
82+
func test_hasPassedSessionTimeout_appRestart() async throws {
6183
let account = Account.fixture()
6284
stateService.activeAccount = account
63-
stateService.lastActiveTime[account.profile.userId] = Date()
64-
stateService.vaultTimeout[account.profile.userId] = .never
85+
stateService.lastActiveTime[account.profile.userId] = .distantPast
86+
stateService.vaultTimeout[account.profile.userId] = .onAppRestart
6587

6688
let shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
6789
XCTAssertFalse(shouldTimeout)
6890
}
6991

70-
/// `.hasPassedSessionTimeout()` returns true if the user should be timed out.
71-
func test_hasPassedSessionTimeout_true() async throws {
92+
/// `.hasPassedSessionTimeout()` returns true if the user should be timed out for a custom timeout value.
93+
func test_hasPassedSessionTimeout_custom() async throws {
7294
let account = Account.fixture()
7395
stateService.activeAccount = account
96+
stateService.vaultTimeout[account.profile.userId] = .custom(120)
97+
98+
let currentTime = Date(year: 2024, month: 1, day: 2, hour: 6, minute: 0)
99+
timeProvider.timeConfig = .mockTime(currentTime)
100+
101+
// Last active 119 minutes ago, no timeout.
102+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
103+
.date(byAdding: .minute, value: -119, to: currentTime)
104+
var shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
105+
XCTAssertFalse(shouldTimeout)
106+
107+
// Last active 120 minutes ago, timeout.
108+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
109+
.date(byAdding: .minute, value: -120, to: currentTime)
110+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
111+
XCTAssertTrue(shouldTimeout)
112+
113+
// Last active 121 minutes ago, timeout.
114+
stateService.lastActiveTime[account.profile.userId] = Calendar.current
115+
.date(byAdding: .minute, value: -121, to: currentTime)
116+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
117+
XCTAssertTrue(shouldTimeout)
118+
119+
// Last active in the distant past, timeout.
74120
stateService.lastActiveTime[account.profile.userId] = .distantPast
75-
stateService.vaultTimeout[account.profile.userId] = .oneMinute
121+
shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
122+
XCTAssertTrue(shouldTimeout)
123+
}
124+
125+
/// `.hasPassedSessionTimeout()` returns true if there's no last active time recorded for the user.
126+
func test_hasPassedSessionTimeout_noLastActiveTime() async throws {
127+
let account = Account.fixture()
128+
stateService.activeAccount = account
129+
stateService.vaultTimeout[account.profile.userId] = .fiveMinutes
76130

77131
let shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
78132
XCTAssertTrue(shouldTimeout)
79133
}
80134

135+
/// `.hasPassedSessionTimeout()` returns false if the user's vault timeout value is negative.
136+
func test_hasPassedSessionTimeout_never() async throws {
137+
let account = Account.fixture()
138+
stateService.activeAccount = account
139+
stateService.lastActiveTime[account.profile.userId] = .distantPast
140+
stateService.vaultTimeout[account.profile.userId] = .never
141+
142+
let shouldTimeout = try await subject.hasPassedSessionTimeout(userId: account.profile.userId)
143+
XCTAssertFalse(shouldTimeout)
144+
}
145+
81146
/// Tests that locking and unlocking the vault works correctly.
82147
func test_lock_unlock() async throws {
83148
let account = Account.fixtureAccountLogin()

BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityState.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ public enum SessionTimeoutValue: RawRepresentable, CaseIterable, Equatable, Menu
9090
}
9191
}
9292

93+
/// The session timeout value in seconds.
94+
var seconds: Int {
95+
rawValue * 60
96+
}
97+
98+
/// The session timeout value in minutes.
9399
public var rawValue: Int {
94100
switch self {
95101
case .immediately: 0

0 commit comments

Comments
 (0)