Skip to content

Commit 2eb8ad4

Browse files
david-livefrontdavid-livefront
authored
PM-28355: Clear pin data on hard-logout or security stamp (#6232)
1 parent 28db795 commit 2eb8ad4

File tree

7 files changed

+106
-64
lines changed

7 files changed

+106
-64
lines changed

‎app/src/main/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,11 @@ class AuthDiskSourceImpl(
159159
storeAuthenticatorSyncUnlockKey(userId = userId, authenticatorSyncUnlockKey = null)
160160
storeShowImportLogins(userId = userId, showImportLogins = null)
161161
storeLastLockTimestamp(userId = userId, lastLockTimestamp = null)
162+
storeEncryptedPin(userId = userId, encryptedPin = null)
163+
storePinProtectedUserKey(userId = userId, pinProtectedUserKey = null)
164+
storePinProtectedUserKeyEnvelope(userId = userId, pinProtectedUserKeyEnvelope = null)
162165

163166
// Certain values are never removed as required by the feature requirements:
164-
// * EncryptedPin
165-
// * PinProtectedUserKey
166-
// * PinProtectedUserKeyEnvelope
167167
// * DeviceKey
168168
// * PendingAuthRequest
169169
// * OnboardingStatus

‎app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ class UserLogoutManagerImpl(
4949
override fun logout(userId: String, reason: LogoutReason) {
5050
authDiskSource.userState ?: return
5151
Timber.d("logout reason=$reason")
52-
val isExpired = reason == LogoutReason.SecurityStamp
53-
if (isExpired) {
52+
val isSecurityStamp = reason == LogoutReason.SecurityStamp
53+
if (isSecurityStamp) {
5454
showToast(message = BitwardenString.login_expired)
5555
}
5656

5757
val ableToSwitchToNewAccount = switchUserIfAvailable(
5858
currentUserId = userId,
59-
isExpired = isExpired,
59+
isSecurityStamp = isSecurityStamp,
6060
removeCurrentUserFromAccounts = true,
6161
)
6262

@@ -73,19 +73,24 @@ class UserLogoutManagerImpl(
7373

7474
override fun softLogout(userId: String, reason: LogoutReason) {
7575
Timber.d("softLogout reason=$reason")
76-
val isExpired = reason == LogoutReason.SecurityStamp
77-
if (isExpired) {
76+
val isSecurityStamp = reason == LogoutReason.SecurityStamp
77+
if (isSecurityStamp) {
7878
showToast(message = BitwardenString.login_expired)
7979
}
8080

81-
// Save any data that will still need to be retained after otherwise clearing all dat
81+
// Save any data that will still need to be retained after otherwise clearing all data
8282
val vaultTimeoutInMinutes = settingsDiskSource.getVaultTimeoutInMinutes(userId = userId)
8383
val vaultTimeoutAction = settingsDiskSource.getVaultTimeoutAction(userId = userId)
84+
val encryptedPin = authDiskSource.getEncryptedPin(userId = userId)
85+
val pinProtectedUserKey = authDiskSource.getPinProtectedUserKey(userId = userId)
86+
val pinProtectedUserKeyEnvelope = authDiskSource.getPinProtectedUserKeyEnvelope(
87+
userId = userId,
88+
)
8489

8590
switchUserIfAvailable(
8691
currentUserId = userId,
8792
removeCurrentUserFromAccounts = false,
88-
isExpired = isExpired,
93+
isSecurityStamp = isSecurityStamp,
8994
)
9095

9196
clearData(userId = userId)
@@ -102,6 +107,14 @@ class UserLogoutManagerImpl(
102107
vaultTimeoutAction = vaultTimeoutAction,
103108
)
104109
}
110+
authDiskSource.apply {
111+
storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
112+
storePinProtectedUserKey(userId = userId, pinProtectedUserKey = pinProtectedUserKey)
113+
storePinProtectedUserKeyEnvelope(
114+
userId = userId,
115+
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
116+
)
117+
}
105118
}
106119

107120
private fun clearData(userId: String) {
@@ -123,7 +136,7 @@ class UserLogoutManagerImpl(
123136
private fun switchUserIfAvailable(
124137
currentUserId: String,
125138
removeCurrentUserFromAccounts: Boolean,
126-
isExpired: Boolean = false,
139+
isSecurityStamp: Boolean,
127140
): Boolean {
128141
val currentUserState = authDiskSource.userState ?: return false
129142

@@ -135,7 +148,7 @@ class UserLogoutManagerImpl(
135148

136149
// Check if there is a new active user
137150
return if (updatedAccounts.isNotEmpty()) {
138-
if (currentUserId == currentUserState.activeUserId && !isExpired) {
151+
if (currentUserId == currentUserState.activeUserId && !isSecurityStamp) {
139152
showToast(message = BitwardenString.account_switched_automatically)
140153
}
141154

‎app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ class VaultSyncManagerImpl(
310310
localSecurityStamp?.let {
311311
if (serverSecurityStamp != localSecurityStamp) {
312312
// Ensure UserLogoutManager is available
313-
userLogoutManager.softLogout(
313+
userLogoutManager.logout(
314314
userId = userId,
315315
reason = LogoutReason.SecurityStamp,
316316
)

‎app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,14 @@ class AuthDiskSourceTest {
267267
userId = userId,
268268
biometricsKey = "1234-9876-0192",
269269
)
270-
val pinProtectedUserKey = "pinProtectedUserKey"
270+
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = "encryptedPin")
271271
authDiskSource.storePinProtectedUserKey(
272272
userId = userId,
273-
pinProtectedUserKey = pinProtectedUserKey,
273+
pinProtectedUserKey = "pinProtectedUserKey",
274274
)
275-
val pinProtectedUserKeyEnvelope = "pinProtectedUserKeyEnvelope"
276275
authDiskSource.storePinProtectedUserKeyEnvelope(
277276
userId = userId,
278-
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
277+
pinProtectedUserKeyEnvelope = "pinProtectedUserKeyEnvelope",
279278
)
280279
authDiskSource.storeInvalidUnlockAttempts(
281280
userId = userId,
@@ -310,8 +309,6 @@ class AuthDiskSourceTest {
310309
refreshToken = "refreshToken",
311310
),
312311
)
313-
val encryptedPin = "encryptedPin"
314-
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
315312
authDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = "passwordHash")
316313
authDiskSource.storeAuthenticatorSyncUnlockKey(
317314
userId = userId,
@@ -333,12 +330,6 @@ class AuthDiskSourceTest {
333330
OnboardingStatus.AUTOFILL_SETUP,
334331
authDiskSource.getOnboardingStatus(userId = userId),
335332
)
336-
assertEquals(encryptedPin, authDiskSource.getEncryptedPin(userId = userId))
337-
assertEquals(pinProtectedUserKey, authDiskSource.getPinProtectedUserKey(userId = userId))
338-
assertEquals(
339-
pinProtectedUserKeyEnvelope,
340-
authDiskSource.getPinProtectedUserKeyEnvelope(userId = userId),
341-
)
342333

343334
// These should be cleared
344335
assertNull(authDiskSource.getUserBiometricInitVector(userId = userId))
@@ -357,6 +348,9 @@ class AuthDiskSourceTest {
357348
assertNull(authDiskSource.getIsTdeLoginComplete(userId = userId))
358349
assertNull(authDiskSource.getAuthenticatorSyncUnlockKey(userId = userId))
359350
assertNull(authDiskSource.getShowImportLogins(userId = userId))
351+
assertNull(authDiskSource.getEncryptedPin(userId = userId))
352+
assertNull(authDiskSource.getPinProtectedUserKey(userId = userId))
353+
assertNull(authDiskSource.getPinProtectedUserKeyEnvelope(userId = userId))
360354
}
361355

362356
@Test

‎app/src/test/kotlin/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,14 @@ class FakeAuthDiskSource : AuthDiskSource {
9191
storedBiometricKeys.remove(userId)
9292
storedOrganizationKeys.remove(userId)
9393
storedPinProtectedUserKeyEnvelopes.remove(userId)
94+
storedEncryptedPins.remove(userId)
95+
storedPinProtectedUserKeys.remove(userId)
9496

9597
mutableShouldUseKeyConnectorFlowMap.remove(userId)
9698
mutableOrganizationsFlowMap.remove(userId)
9799
mutablePoliciesFlowMap.remove(userId)
98100
mutableAccountTokensFlowMap.remove(userId)
101+
mutablePinProtectedUserKeyEnvelopesFlowMap.remove(userId)
99102
}
100103

101104
private fun getMutablePinProtectedUserKeyEnvelopeFlow(

‎app/src/test/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt‎

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,30 @@ class UserLogoutManagerTest {
120120
assertDataCleared(userId = userId)
121121
}
122122

123+
@Suppress("MaxLineLength")
124+
@Test
125+
fun `logout with security stamp reason should switch active user and display the login expired toast`() {
126+
val userId = USER_ID_1
127+
every { authDiskSource.userState } returns MULTI_USER_STATE
128+
129+
userLogoutManager.logout(userId = userId, reason = LogoutReason.SecurityStamp)
130+
131+
verify(exactly = 1) {
132+
authDiskSource.userState = SINGLE_USER_STATE_2
133+
toastManager.show(messageId = BitwardenString.login_expired)
134+
}
135+
assertDataCleared(userId = userId)
136+
}
137+
123138
@Suppress("MaxLineLength")
124139
@Test
125140
fun `softLogout should clear most data associated with the given user and remove token data in the authDiskSource`() {
126141
val userId = USER_ID_1
127142
val vaultTimeoutInMinutes = 360
128143
val vaultTimeoutAction = VaultTimeoutAction.LOGOUT
129144
val pinProtectedUserKey = "pinProtectedUserKey"
145+
val pinProtectedUserKeyEnvelope = "pinProtectedUserKeyEnvelope"
146+
val encryptedPin = "encryptedPin"
130147

131148
every { authDiskSource.userState } returns MULTI_USER_STATE
132149
every {
@@ -135,10 +152,26 @@ class UserLogoutManagerTest {
135152
every {
136153
settingsDiskSource.getVaultTimeoutAction(userId = userId)
137154
} returns vaultTimeoutAction
138-
139155
every {
140156
authDiskSource.getPinProtectedUserKeyEnvelope(userId = userId)
141-
} returns pinProtectedUserKey
157+
} returns pinProtectedUserKeyEnvelope
158+
every {
159+
authDiskSource.storePinProtectedUserKeyEnvelope(
160+
userId = userId,
161+
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
162+
)
163+
} just runs
164+
every { authDiskSource.getPinProtectedUserKey(userId = userId) } returns pinProtectedUserKey
165+
every {
166+
authDiskSource.storePinProtectedUserKey(
167+
userId = userId,
168+
pinProtectedUserKey = pinProtectedUserKey,
169+
)
170+
} just runs
171+
every { authDiskSource.getEncryptedPin(userId = userId) } returns encryptedPin
172+
every {
173+
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
174+
} just runs
142175

143176
userLogoutManager.softLogout(userId = userId, reason = LogoutReason.Timeout)
144177

@@ -162,6 +195,15 @@ class UserLogoutManagerTest {
162195
userId = userId,
163196
vaultTimeoutAction = vaultTimeoutAction,
164197
)
198+
authDiskSource.storePinProtectedUserKeyEnvelope(
199+
userId = userId,
200+
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
201+
)
202+
authDiskSource.storePinProtectedUserKey(
203+
userId = userId,
204+
pinProtectedUserKey = pinProtectedUserKey,
205+
)
206+
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
165207
}
166208
}
167209

@@ -171,6 +213,8 @@ class UserLogoutManagerTest {
171213
val vaultTimeoutInMinutes = 360
172214
val vaultTimeoutAction = VaultTimeoutAction.LOGOUT
173215
val pinProtectedUserKey = "pinProtectedUserKey"
216+
val pinProtectedUserKeyEnvelope = "pinProtectedUserKeyEnvelope"
217+
val encryptedPin = "encryptedPin"
174218

175219
every { authDiskSource.userState } returns MULTI_USER_STATE
176220
every {
@@ -181,7 +225,24 @@ class UserLogoutManagerTest {
181225
} returns vaultTimeoutAction
182226
every {
183227
authDiskSource.getPinProtectedUserKeyEnvelope(userId = userId)
184-
} returns pinProtectedUserKey
228+
} returns pinProtectedUserKeyEnvelope
229+
every {
230+
authDiskSource.storePinProtectedUserKeyEnvelope(
231+
userId = userId,
232+
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
233+
)
234+
} just runs
235+
every { authDiskSource.getPinProtectedUserKey(userId = userId) } returns pinProtectedUserKey
236+
every {
237+
authDiskSource.storePinProtectedUserKey(
238+
userId = userId,
239+
pinProtectedUserKey = pinProtectedUserKey,
240+
)
241+
} just runs
242+
every { authDiskSource.getEncryptedPin(userId = userId) } returns encryptedPin
243+
every {
244+
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
245+
} just runs
185246

186247
userLogoutManager.softLogout(userId = userId, reason = LogoutReason.Timeout)
187248

@@ -199,44 +260,15 @@ class UserLogoutManagerTest {
199260
userId = userId,
200261
vaultTimeoutAction = vaultTimeoutAction,
201262
)
202-
}
203-
}
204-
205-
@Suppress("MaxLineLength")
206-
@Test
207-
fun `softLogout with security stamp reason should switch active user and keep previous user in accounts list but display the login expired toast`() {
208-
val userId = USER_ID_1
209-
val vaultTimeoutInMinutes = 360
210-
val vaultTimeoutAction = VaultTimeoutAction.LOGOUT
211-
val pinProtectedUserKey = "pinProtectedUserKey"
212-
213-
every { authDiskSource.userState } returns MULTI_USER_STATE
214-
every {
215-
authDiskSource.getPinProtectedUserKeyEnvelope(userId)
216-
} returns pinProtectedUserKey
217-
every {
218-
settingsDiskSource.getVaultTimeoutInMinutes(userId = userId)
219-
} returns vaultTimeoutInMinutes
220-
every {
221-
settingsDiskSource.getVaultTimeoutAction(userId = userId)
222-
} returns vaultTimeoutAction
223-
224-
userLogoutManager.softLogout(userId = userId, reason = LogoutReason.SecurityStamp)
225-
226-
verify(exactly = 1) {
227-
authDiskSource.userState = UserStateJson(
228-
activeUserId = USER_ID_2,
229-
accounts = MULTI_USER_STATE.accounts,
230-
)
231-
toastManager.show(messageId = BitwardenString.login_expired)
232-
settingsDiskSource.storeVaultTimeoutInMinutes(
263+
authDiskSource.storePinProtectedUserKeyEnvelope(
233264
userId = userId,
234-
vaultTimeoutInMinutes = vaultTimeoutInMinutes,
265+
pinProtectedUserKeyEnvelope = pinProtectedUserKeyEnvelope,
235266
)
236-
settingsDiskSource.storeVaultTimeoutAction(
267+
authDiskSource.storePinProtectedUserKey(
237268
userId = userId,
238-
vaultTimeoutAction = vaultTimeoutAction,
269+
pinProtectedUserKey = pinProtectedUserKey,
239270
)
271+
authDiskSource.storeEncryptedPin(userId = userId, encryptedPin = encryptedPin)
240272
}
241273
}
242274

‎app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ class VaultSyncManagerTest {
126126
}
127127
}
128128
private val userLogoutManager: UserLogoutManager = mockk {
129-
every { softLogout(any(), any()) } just runs
129+
every { logout(userId = any(), reason = LogoutReason.SecurityStamp) } just runs
130130
}
131131
private val userStateManager: UserStateManager = mockk {
132132
val blockSlot = slot<suspend () -> SyncVaultDataResult>()
@@ -786,7 +786,7 @@ class VaultSyncManagerTest {
786786
vaultSyncManager.sync()
787787

788788
coVerify(exactly = 1) {
789-
userLogoutManager.softLogout(userId = userId, reason = LogoutReason.SecurityStamp)
789+
userLogoutManager.logout(userId = userId, reason = LogoutReason.SecurityStamp)
790790
}
791791

792792
coVerify(exactly = 0) {

0 commit comments

Comments
 (0)