Skip to content

Commit eb954b4

Browse files
fedemkrfedemkr
authored
[PM-8361] Added whether the vault has been unlocked interactively (#743)
1 parent 98961cf commit eb954b4

12 files changed

+261
-33
lines changed

‎BitwardenShared/Core/Auth/Repositories/AuthRepository.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ extension DefaultAuthRepository: AuthRepository {
742742
let id = try await stateService.getActiveAccountId()
743743
let key = KeychainItem.neverLock(userId: id)
744744
let neverlockKey = try await keychainService.getUserAuthKeyValue(for: key)
745-
try await unlockVault(method: .decryptedKey(decryptedUserKey: neverlockKey))
745+
try await unlockVault(method: .decryptedKey(decryptedUserKey: neverlockKey), hadUserInteraction: false)
746746
}
747747

748748
func unlockVaultWithPassword(password: String) async throws {
@@ -862,9 +862,11 @@ extension DefaultAuthRepository: AuthRepository {
862862

863863
/// Attempts to unlock the vault with a given method.
864864
///
865-
/// - Parameter method: The unlocking `InitUserCryptoMethod` method.
866-
///
867-
private func unlockVault(method: InitUserCryptoMethod) async throws {
865+
/// - Parameters:
866+
/// - method: The unlocking `InitUserCryptoMethod` method
867+
/// - hadUserInteraction: If the user interacted with the app to unlock the vault
868+
/// or was unlocked using the never lock key.
869+
private func unlockVault(method: InitUserCryptoMethod, hadUserInteraction: Bool = true) async throws {
868870
let account = try await stateService.getActiveAccount()
869871
let encryptionKeys = try await stateService.getAccountEncryptionKeys()
870872

@@ -920,7 +922,10 @@ extension DefaultAuthRepository: AuthRepository {
920922
}
921923

922924
_ = try await trustDeviceService.trustDeviceIfNeeded()
923-
try await vaultTimeoutService.unlockVault(userId: account.profile.userId)
925+
try await vaultTimeoutService.unlockVault(
926+
userId: account.profile.userId,
927+
hadUserInteraction: hadUserInteraction
928+
)
924929
try await organizationService.initializeOrganizationCrypto()
925930
}
926931

‎BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
922922
await assertAsyncDoesNotThrow {
923923
try await subject.unlockVaultWithNeverlockKey()
924924
}
925+
XCTAssertFalse(vaultTimeoutService.unlockVaultHadUserInteraction)
925926
}
926927

927928
/// `test_unlockVaultWithDeviceKey` attempts to unlock the vault using the device key from the keychain.
@@ -947,6 +948,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
947948
await assertAsyncDoesNotThrow {
948949
try await subject.unlockVaultWithDeviceKey()
949950
}
951+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
950952
}
951953

952954
/// `test_unlockVaultWithDeviceKey` attempts to unlock the vault using the device key from the keychain.
@@ -1182,6 +1184,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
11821184
XCTAssertEqual(authService.hashPasswordPassword, "password")
11831185
XCTAssertEqual(stateService.masterPasswordHashes["1"], "hashed")
11841186
XCTAssertFalse(biometricsRepository.didConfigureBiometricIntegrity)
1187+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
11851188
}
11861189

11871190
/// `unlockVaultWithPassword(password:)` configures biometric integrity refreshes.
@@ -1309,6 +1312,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
13091312
await assertAsyncDoesNotThrow {
13101313
try await subject.unlockVaultWithBiometrics()
13111314
}
1315+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
13121316
}
13131317

13141318
/// `logout` throws an error with no accounts.
@@ -1381,6 +1385,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
13811385
)
13821386
)
13831387
)
1388+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
13841389
}
13851390

13861391
/// `unlockVaultFromLoginWithDevice()` unlocks the vault using the key returned by an approved
@@ -1409,6 +1414,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
14091414
)
14101415
)
14111416
)
1417+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
14121418
}
14131419

14141420
/// `unlockVaultWithPIN(_:)` unlocks the vault with the user's PIN.
@@ -1437,6 +1443,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
14371443
)
14381444
)
14391445
XCTAssertFalse(vaultTimeoutService.isLocked(userId: "1"))
1446+
XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction)
14401447
}
14411448

14421449
/// `unlockVaultWithPIN(_:)` throws an error if there's no pin.

‎BitwardenShared/Core/Platform/Services/ServiceContainer.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
542542
let fido2UserInterfaceHelper = DefaultFido2UserInterfaceHelper(
543543
fido2UserVerificationMediator: DefaultFido2UserVerificationMediator(
544544
authRepository: authRepository,
545+
stateService: stateService,
545546
userVerificationHelper: DefaultUserVerificationHelper(
546547
authRepository: authRepository,
547548
errorReporter: errorReporter,

‎BitwardenShared/Core/Platform/Services/StateService.swift

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ protocol StateService: AnyObject {
4949
///
5050
func getAccountEncryptionKeys(userId: String?) async throws -> AccountEncryptionKeys
5151

52+
/// Gets whether the user has unlocked their account in the current session interactively.
53+
/// - Parameter userId: The user ID of the account. Defaults to the active account if `nil`.
54+
func getAccountHasBeenUnlockedInteractively(userId: String?) async throws -> Bool
55+
5256
/// Gets all accounts.
5357
///
5458
/// - Returns: The known user accounts.
@@ -312,6 +316,12 @@ protocol StateService: AnyObject {
312316
///
313317
func setAccountEncryptionKeys(_ encryptionKeys: AccountEncryptionKeys, userId: String?) async throws
314318

319+
/// Sets whether the user has unlocked their account in the current session interactively.
320+
/// - Parameters:
321+
/// - userId: The user ID of the account. Defaults to the active account if `nil`.
322+
/// - value: Whether the user has unlocked their account in the current session.
323+
func setAccountHasBeenUnlockedInteractively(userId: String?, value: Bool) async throws
324+
315325
/// Sets the active account.
316326
///
317327
/// - Parameter userId: The user Id of the account to set as active.
@@ -593,6 +603,11 @@ extension StateService {
593603
try await getAccountEncryptionKeys(userId: nil)
594604
}
595605

606+
/// Gets whether the user has unlocked their account in the current session interactively.
607+
func getAccountHasBeenUnlockedInteractively() async throws -> Bool {
608+
try await getAccountHasBeenUnlockedInteractively(userId: nil)
609+
}
610+
596611
/// Gets either a valid account id or the active account id.
597612
///
598613
/// - Parameter userId: The possible user id.
@@ -804,6 +819,12 @@ extension StateService {
804819
try await setAccountEncryptionKeys(encryptionKeys, userId: nil)
805820
}
806821

822+
/// Sets whether the user has unlocked their account in the current session interactively.
823+
/// - Parameter value: Whether the user has unlocked their account in the current session
824+
func setAccountHasBeenUnlockedInteractively(value: Bool) async throws {
825+
try await setAccountHasBeenUnlockedInteractively(userId: nil, value: value)
826+
}
827+
807828
/// Sets the allow sync on refresh value for the active account.
808829
///
809830
/// - Parameter allowSyncOnRefresh: The allow sync on refresh value.
@@ -1030,7 +1051,7 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
10301051

10311052
func clearPins() async throws {
10321053
let userId = try getActiveAccountUserId()
1033-
accountVolatileData.removeValue(forKey: userId)
1054+
accountVolatileData[userId]?.pinProtectedUserKey = nil
10341055
appSettingsStore.setEncryptedPin(nil, userId: userId)
10351056
appSettingsStore.setPinProtectedUserKey(key: nil, userId: userId)
10361057
}
@@ -1075,6 +1096,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
10751096
)
10761097
}
10771098

1099+
func getAccountHasBeenUnlockedInteractively(userId: String?) async throws -> Bool {
1100+
let userId = try userId ?? getActiveAccountUserId()
1101+
return accountVolatileData[userId]?.hasBeenUnlockedInteractively == true
1102+
}
1103+
10781104
func getAccounts() throws -> [Account] {
10791105
guard let accounts = appSettingsStore.state?.accounts else {
10801106
throw StateServiceError.noAccounts
@@ -1269,6 +1295,14 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
12691295
appSettingsStore.setEncryptedUserKey(key: encryptionKeys.encryptedUserKey, userId: userId)
12701296
}
12711297

1298+
func setAccountHasBeenUnlockedInteractively(userId: String?, value: Bool) async throws {
1299+
let userId = try userId ?? getActiveAccountUserId()
1300+
accountVolatileData[
1301+
userId,
1302+
default: AccountVolatileData()
1303+
].hasBeenUnlockedInteractively = value
1304+
}
1305+
12721306
func setActiveAccount(userId: String) async throws {
12731307
guard var state = appSettingsStore.state else { return }
12741308
defer { appSettingsStore.state = state }
@@ -1513,7 +1547,10 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
15131547
///
15141548
struct AccountVolatileData {
15151549
/// The pin protected user key.
1516-
var pinProtectedUserKey: String = ""
1550+
var pinProtectedUserKey: String?
1551+
1552+
/// Whether the account has been unlocked with user interaction.
1553+
var hasBeenUnlockedInteractively = false
15171554
}
15181555

15191556
// MARK: Biometrics

‎BitwardenShared/Core/Platform/Services/StateServiceTests.swift

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,46 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
253253
}
254254
}
255255

256+
/// `getAccountHasBeenUnlockedInteractively()` gets the default value from the active user.
257+
func test_getAccountHasBeenUnlockedInteractively_default() async throws {
258+
appSettingsStore.state = State.fixture(
259+
accounts: [
260+
"1": Account.fixture(),
261+
],
262+
activeUserId: "1"
263+
)
264+
let result = try await subject.getAccountHasBeenUnlockedInteractively()
265+
XCTAssertFalse(result)
266+
}
267+
268+
/// `getAccountHasBeenUnlockedInteractively()` gets the value from the active user.
269+
func test_getAccountHasBeenUnlockedInteractively() async throws {
270+
appSettingsStore.state = State.fixture(
271+
accounts: [
272+
"1": Account.fixture(),
273+
],
274+
activeUserId: "1"
275+
)
276+
try await subject.setAccountHasBeenUnlockedInteractively(value: true)
277+
let result = try await subject.getAccountHasBeenUnlockedInteractively()
278+
XCTAssertTrue(result)
279+
}
280+
281+
/// `getAccountHasBeenUnlockedInteractively(userId:)` gets the value from the given user.
282+
func test_getAccountHasBeenUnlockedInteractively_givenUser() async throws {
283+
try await subject.setAccountHasBeenUnlockedInteractively(userId: "2", value: true)
284+
let result = try await subject.getAccountHasBeenUnlockedInteractively(userId: "2")
285+
XCTAssertTrue(result)
286+
}
287+
288+
/// `getAccountHasBeenUnlockedInteractively()` gets the value from the given user.
289+
func test_getAccountHasBeenUnlockedInteractively_throwsGettingTheUser() async throws {
290+
appSettingsStore.state?.activeUserId = nil
291+
await assertAsyncThrows(error: StateServiceError.noActiveAccount) {
292+
_ = try await subject.getAccountHasBeenUnlockedInteractively()
293+
}
294+
}
295+
256296
/// `getActiveAccount()` returns the active account.
257297
func test_getActiveAccount() async throws {
258298
let account = Account.fixture(profile: .fixture(userId: "2"))
@@ -1312,6 +1352,42 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
13121352
XCTAssertEqual(appSettingsStore.disableAutoTotpCopyByUserId["1"], false)
13131353
}
13141354

1355+
/// `setAccountHasBeenUnlockedInteractively(userId:value:)` updates volatile data
1356+
func test_setAccountHasBeenUnlockedInteractively() async throws {
1357+
try await subject.setAccountHasBeenUnlockedInteractively(userId: "1", value: true)
1358+
let result = await subject.accountVolatileData["1"]?.hasBeenUnlockedInteractively ?? false
1359+
XCTAssertTrue(result)
1360+
}
1361+
1362+
/// `setAccountHasBeenUnlockedInteractively(userId:value:)` updates volatile data for existing user.
1363+
func test_setAccountHasBeenUnlockedInteractively_updateExisting() async throws {
1364+
try await subject.setAccountHasBeenUnlockedInteractively(userId: "1", value: true)
1365+
try await subject.setAccountHasBeenUnlockedInteractively(userId: "1", value: false)
1366+
let result = await subject.accountVolatileData["1"]?.hasBeenUnlockedInteractively ?? false
1367+
XCTAssertFalse(result)
1368+
}
1369+
1370+
/// `setAccountHasBeenUnlockedInteractively(value:)` updates volatile data for current user.
1371+
func test_setAccountHasBeenUnlockedInteractively_updateByCurrentUser() async throws {
1372+
appSettingsStore.state = State.fixture(
1373+
accounts: [
1374+
"1": Account.fixture(),
1375+
],
1376+
activeUserId: "1"
1377+
)
1378+
try await subject.setAccountHasBeenUnlockedInteractively(value: true)
1379+
let result = await subject.accountVolatileData["1"]?.hasBeenUnlockedInteractively ?? false
1380+
XCTAssertTrue(result)
1381+
}
1382+
1383+
/// `setAccountHasBeenUnlockedInteractively(value:)` throws if it throws when getting the user id.
1384+
func test_setAccountHasBeenUnlockedInteractively_throwsWhenGettingUserId() async throws {
1385+
appSettingsStore.state?.activeUserId = nil
1386+
await assertAsyncThrows(error: StateServiceError.noActiveAccount) {
1387+
_ = try await subject.setAccountHasBeenUnlockedInteractively(value: true)
1388+
}
1389+
}
1390+
13151391
/// `setActiveAccount(userId: )` succeeds if there is a matching account
13161392
func test_setActiveAccount_match_multi() async throws {
13171393
let account1 = Account.fixture(profile: .fixture(userId: "1"))

‎BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
3737
var lastActiveTime = [String: Date]()
3838
var loginRequest: LoginRequestNotification?
3939
var getAccountEncryptionKeysError: Error?
40+
// swiftlint:disable:next identifier_name
41+
var getAccountHasBeenUnlockedInteractivelyResult: Result<Bool, Error> = .success(false)
4042
var getBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
4143
var getBiometricIntegrityStateError: Error?
4244
var lastSyncTimeByUserId = [String: Date]()
@@ -53,6 +55,9 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
5355
var showWebIconsSubject = CurrentValueSubject<Bool, Never>(true)
5456
var timeoutAction = [String: SessionTimeoutAction]()
5557
var serverConfig = [String: ServerConfig]()
58+
var setAccountHasBeenUnlockedInteractivelyHasBeenCalled = false // swiftlint:disable:this identifier_name
59+
// swiftlint:disable:next identifier_name
60+
var setAccountHasBeenUnlockedInteractivelyResult: Result<Void, Error> = .success(())
5661
var setBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
5762
var setBiometricIntegrityStateError: Error?
5863
var shouldCheckOrganizationUnassignedItems = [String: Bool?]()
@@ -109,6 +114,10 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
109114
return encryptionKeys
110115
}
111116

117+
func getAccountHasBeenUnlockedInteractively(userId: String?) async throws -> Bool {
118+
try getAccountHasBeenUnlockedInteractivelyResult.get()
119+
}
120+
112121
func getAccount(userId: String?) async throws -> BitwardenShared.Account {
113122
let id = try await getAccountIdOrActiveId(userId: userId)
114123
if let activeAccount,
@@ -297,6 +306,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
297306
accountEncryptionKeys[userId] = encryptionKeys
298307
}
299308

309+
func setAccountHasBeenUnlockedInteractively(userId: String?, value: Bool) async throws {
310+
setAccountHasBeenUnlockedInteractivelyHasBeenCalled = true
311+
try setAccountHasBeenUnlockedInteractivelyResult.get()
312+
}
313+
300314
func setActiveAccount(userId: String) async throws {
301315
guard let accounts,
302316
let match = accounts.first(where: { account in

‎BitwardenShared/Core/Vault/Services/TestHelpers/MockVaultTimeoutService.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class MockVaultTimeoutService: VaultTimeoutService {
1111
var shouldSessionTimeoutError: Error?
1212
var timeProvider = MockTimeProvider(.currentTime)
1313
var sessionTimeoutValueError: Error?
14+
var unlockVaultHadUserInteraction = false
1415
var vaultTimeout = [String: SessionTimeoutValue]()
1516
var vaultLockStatusSubject = CurrentValueSubject<VaultLockStatus?, Never>(nil)
1617

@@ -47,9 +48,10 @@ class MockVaultTimeoutService: VaultTimeoutService {
4748
return shouldSessionTimeout[userId] ?? false
4849
}
4950

50-
func unlockVault(userId: String?) async throws {
51+
func unlockVault(userId: String?, hadUserInteraction: Bool) async throws {
5152
guard let userId else { return }
5253
isClientLocked[userId] = false
54+
unlockVaultHadUserInteraction = hadUserInteraction
5355
}
5456

5557
func remove(userId: String?) async {

‎BitwardenShared/Core/Vault/Services/VaultTimeoutService.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ protocol VaultTimeoutService: AnyObject {
5959

6060
/// Unlocks the user's vault
6161
///
62-
/// - Parameter userId: The userId of the account to unlock.
62+
/// - Parameters:
63+
/// - userId: The userId of the account to unlock.
6364
/// Defaults to the active account if nil
64-
///
65-
func unlockVault(userId: String?) async throws
65+
/// - hadUserInteraction: Whether the user had any interaction with the app to unlock the vault
66+
/// or the never lock key was used.
67+
func unlockVault(userId: String?, hadUserInteraction: Bool) async throws
6668

6769
/// Gets the `SessionTimeoutValue` for a user.
6870
///
@@ -143,12 +145,14 @@ class DefaultVaultTimeoutService: VaultTimeoutService {
143145
guard let id = try? await stateService.getAccountIdOrActiveId(userId: userId) else { return }
144146
try? await clientService.removeClient(for: id)
145147
vaultLockStatusSubject.value[id] = true
148+
try? await stateService.setAccountHasBeenUnlockedInteractively(userId: id, value: false)
146149
}
147150

148151
func remove(userId: String?) async {
149152
guard let id = try? await stateService.getAccountIdOrActiveId(userId: userId) else { return }
150153
try? await clientService.removeClient(for: id)
151154
vaultLockStatusSubject.value.removeValue(forKey: id)
155+
try? await stateService.setAccountHasBeenUnlockedInteractively(userId: id, value: false)
152156
}
153157

154158
func setLastActiveTime(userId: String) async throws {
@@ -159,9 +163,10 @@ class DefaultVaultTimeoutService: VaultTimeoutService {
159163
try await stateService.setVaultTimeout(value: value, userId: userId)
160164
}
161165

162-
func unlockVault(userId: String?) async throws {
166+
func unlockVault(userId: String?, hadUserInteraction: Bool) async throws {
163167
guard let id = try? await stateService.getAccountIdOrActiveId(userId: userId) else { return }
164168
vaultLockStatusSubject.value[id] = false
169+
try await stateService.setAccountHasBeenUnlockedInteractively(userId: id, value: hadUserInteraction)
165170
}
166171

167172
func sessionTimeoutValue(userId: String?) async throws -> SessionTimeoutValue {

0 commit comments

Comments
 (0)