Skip to content

Commit ce5501d

Browse files
SaintPatrckSaintPatrck
committed
[PM-21782] Pass encryptedFor to cipher functions
This commit updates several functions to accept an `encryptedFor` parameter, which specifies the user ID for whom the cipher is encrypted. This property is used by the server to verify the cipher is encrypted by the correct user. If verification fails the server responds with an appropriate error message. This change affects the following: - `toEncryptedNetworkCipher` and `toEncryptedNetworkCipherResponse` extension functions for `Cipher` now require an `encryptedFor` parameter. - `CipherJsonRequest` and `SyncResponseJson.Cipher` now include an `encryptedFor` field. - SDK functions like `encryptCipher` and `decryptFile` have been updated to align with these changes. Additionally, this update includes the following SDK related changes: - Adding the `encryptedFor` related logic (bitwarden/sdk-internal#278) - Update in protocol and objects naming to have ``{foo}Client[Protocol]`` instead of `Client{foo}[Protocol]` (bitwarden/sdk-internal#224) - Update attachments decryption to use `AttachmentView` instead of `Attachment` (bitwarden/sdk-internal#255)
1 parent 00ded69 commit ce5501d

File tree

20 files changed

+375
-137
lines changed

20 files changed

+375
-137
lines changed

‎app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import com.bitwarden.vault.CipherListView
2222
import com.bitwarden.vault.CipherView
2323
import com.bitwarden.vault.Collection
2424
import com.bitwarden.vault.CollectionView
25+
import com.bitwarden.vault.EncryptionContext
2526
import com.bitwarden.vault.Folder
2627
import com.bitwarden.vault.FolderView
2728
import com.bitwarden.vault.PasswordHistory
@@ -187,7 +188,7 @@ interface VaultSdkSource {
187188
suspend fun encryptCipher(
188189
userId: String,
189190
cipherView: CipherView,
190-
): Result<Cipher>
191+
): Result<EncryptionContext>
191192

192193
/**
193194
* Decrypts a [Cipher] for the user with the given [userId], returning a [CipherView] wrapped
@@ -349,13 +350,13 @@ interface VaultSdkSource {
349350
): Result<List<FolderView>>
350351

351352
/**
352-
* Decrypts a [cipher] [attachment] file found at [encryptedFilePath] saving it at
353+
* Decrypts a [cipher] [attachmentView] file found at [encryptedFilePath] saving it at
353354
* [decryptedFilePath] for the user with the given [userId]
354355
*/
355356
suspend fun decryptFile(
356357
userId: String,
357358
cipher: Cipher,
358-
attachment: Attachment,
359+
attachmentView: AttachmentView,
359360
encryptedFilePath: String,
360361
decryptedFilePath: String,
361362
): Result<Unit>

‎app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.bitwarden.vault.CipherListView
2525
import com.bitwarden.vault.CipherView
2626
import com.bitwarden.vault.Collection
2727
import com.bitwarden.vault.CollectionView
28+
import com.bitwarden.vault.EncryptionContext
2829
import com.bitwarden.vault.Folder
2930
import com.bitwarden.vault.FolderView
3031
import com.bitwarden.vault.PasswordHistory
@@ -269,7 +270,7 @@ class VaultSdkSourceImpl(
269270
override suspend fun encryptCipher(
270271
userId: String,
271272
cipherView: CipherView,
272-
): Result<Cipher> =
273+
): Result<EncryptionContext> =
273274
runCatchingWithLogs {
274275
getClient(userId = userId)
275276
.vault()
@@ -389,7 +390,7 @@ class VaultSdkSourceImpl(
389390
override suspend fun decryptFile(
390391
userId: String,
391392
cipher: Cipher,
392-
attachment: Attachment,
393+
attachmentView: AttachmentView,
393394
encryptedFilePath: String,
394395
decryptedFilePath: String,
395396
): Result<Unit> =
@@ -399,7 +400,7 @@ class VaultSdkSourceImpl(
399400
.attachments()
400401
.decryptFile(
401402
cipher = cipher,
402-
attachment = attachment,
403+
attachment = attachmentView,
403404
encryptedFilePath = encryptedFilePath,
404405
decryptedFilePath = decryptedFilePath,
405406
)

‎app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package com.x8bit.bitwarden.data.vault.datasource.sdk.model
33
import com.bitwarden.annotation.OmitFromCoverage
44
import com.bitwarden.fido.Fido2CredentialAutofillView
55
import com.bitwarden.sdk.Fido2CredentialStore
6-
import com.bitwarden.vault.Cipher
76
import com.bitwarden.vault.CipherListView
87
import com.bitwarden.vault.CipherView
8+
import com.bitwarden.vault.EncryptionContext
99
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
1010
import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials
1111
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
@@ -91,11 +91,12 @@ class Fido2CredentialStoreImpl(
9191
/**
9292
* Save the provided [cred] to the users vault.
9393
*/
94-
override suspend fun saveCredential(cred: Cipher) {
95-
val userId = getActiveUserIdOrThrow()
96-
94+
override suspend fun saveCredential(cred: EncryptionContext) {
9795
vaultSdkSource
98-
.decryptCipher(userId, cred)
96+
.decryptCipher(
97+
userId = cred.encryptedFor,
98+
cipher = cred.cipher,
99+
)
99100
.map { decryptedCipherView ->
100101
decryptedCipherView.id
101102
?.let { vaultRepository.updateCipher(it, decryptedCipherView) }

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

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
1212
import com.bitwarden.network.model.UpdateCipherResponseJson
1313
import com.bitwarden.network.service.CiphersService
1414
import com.bitwarden.vault.AttachmentView
15-
import com.bitwarden.vault.Cipher
1615
import com.bitwarden.vault.CipherView
16+
import com.bitwarden.vault.EncryptionContext
1717
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
1818
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
1919
import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager
@@ -53,6 +53,7 @@ class CipherManagerImpl(
5353
override suspend fun createCipher(cipherView: CipherView): CreateCipherResult {
5454
val userId = activeUserId
5555
?: return CreateCipherResult.Error(error = NoActiveUserException())
56+
5657
return vaultSdkSource
5758
.encryptCipher(
5859
userId = userId,
@@ -80,10 +81,10 @@ class CipherManagerImpl(
8081
userId = userId,
8182
cipherView = cipherView,
8283
)
83-
.flatMap { cipher ->
84+
.flatMap {
8485
ciphersService.createCipherInOrganization(
8586
body = CreateCipherInOrganizationJsonRequest(
86-
cipher = cipher.toEncryptedNetworkCipher(),
87+
cipher = it.toEncryptedNetworkCipher(),
8788
collectionIds = collectionIds,
8889
),
8990
)
@@ -123,10 +124,15 @@ class CipherManagerImpl(
123124
?: return DeleteCipherResult.Error(error = NoActiveUserException())
124125
return cipherView
125126
.encryptCipherAndCheckForMigration(userId = userId, cipherId = cipherId)
126-
.flatMap { cipher ->
127+
.flatMap { encryptionContext ->
127128
ciphersService
128129
.softDeleteCipher(cipherId = cipherId)
129-
.flatMap { vaultSdkSource.decryptCipher(userId = userId, cipher = cipher) }
130+
.flatMap {
131+
vaultSdkSource.decryptCipher(
132+
userId = userId,
133+
cipher = encryptionContext.cipher,
134+
)
135+
}
130136
}
131137
.flatMap {
132138
vaultSdkSource.encryptCipher(
@@ -165,7 +171,7 @@ class CipherManagerImpl(
165171
cipherId: String,
166172
attachmentId: String,
167173
cipherView: CipherView,
168-
): Result<Cipher> {
174+
): Result<EncryptionContext> {
169175
val userId = activeUserId ?: return NoActiveUserException().asFailure()
170176
return ciphersService
171177
.deleteCipherAttachment(
@@ -181,10 +187,10 @@ class CipherManagerImpl(
181187
)
182188
.encryptCipherAndCheckForMigration(userId = userId, cipherId = cipherId)
183189
}
184-
.onSuccess { cipher ->
190+
.onSuccess { encryptionContext ->
185191
vaultDiskSource.saveCipher(
186192
userId = userId,
187-
cipher = cipher.toEncryptedNetworkCipherResponse(),
193+
cipher = encryptionContext.toEncryptedNetworkCipherResponse(),
188194
)
189195
}
190196
}
@@ -220,10 +226,10 @@ class CipherManagerImpl(
220226
userId = userId,
221227
cipherView = cipherView,
222228
)
223-
.flatMap { cipher ->
229+
.flatMap {
224230
ciphersService.updateCipher(
225231
cipherId = cipherId,
226-
body = cipher.toEncryptedNetworkCipher(),
232+
body = it.toEncryptedNetworkCipher(),
227233
)
228234
}
229235
.map { response ->
@@ -263,11 +269,11 @@ class CipherManagerImpl(
263269
)
264270
}
265271
.flatMap { vaultSdkSource.encryptCipher(userId = userId, cipherView = it) }
266-
.flatMap { cipher ->
272+
.flatMap {
267273
ciphersService.shareCipher(
268274
cipherId = cipherId,
269275
body = ShareCipherJsonRequest(
270-
cipher = cipher.toEncryptedNetworkCipher(),
276+
cipher = it.toEncryptedNetworkCipher(),
271277
collectionIds = collectionIds,
272278
),
273279
)
@@ -301,10 +307,10 @@ class CipherManagerImpl(
301307
cipherView = cipherView.copy(collectionIds = collectionIds),
302308
)
303309
}
304-
.onSuccess { cipher ->
310+
.onSuccess { encryptionContext ->
305311
vaultDiskSource.saveCipher(
306312
userId = userId,
307-
cipher = cipher.toEncryptedNetworkCipherResponse(),
313+
cipher = encryptionContext.toEncryptedNetworkCipherResponse(),
308314
)
309315
}
310316
.fold(
@@ -362,14 +368,14 @@ class CipherManagerImpl(
362368
userId = userId,
363369
cipherId = requireNotNull(cipherView.id),
364370
)
365-
.flatMap { cipher ->
371+
.flatMap { encryptionContext ->
366372
fileManager
367373
.writeUriToCache(fileUri = fileUri)
368374
.flatMap { cacheFile ->
369375
vaultSdkSource
370376
.encryptAttachment(
371377
userId = userId,
372-
cipher = cipher,
378+
cipher = encryptionContext.cipher,
373379
attachmentView = attachmentView,
374380
decryptedFilePath = cacheFile.absolutePath,
375381
encryptedFilePath = "${cacheFile.absolutePath}.enc",
@@ -447,10 +453,10 @@ class CipherManagerImpl(
447453
cipherId = requireNotNull(cipherView.id),
448454
)
449455
.fold(
450-
onSuccess = { it },
456+
onSuccess = { it.cipher },
451457
onFailure = { return it.asFailure() },
452458
)
453-
val attachment = cipher.attachments?.find { it.id == attachmentId }
459+
val attachmentView = cipherView.attachments?.find { it.id == attachmentId }
454460
?: return IllegalStateException("No attachment to download").asFailure()
455461

456462
val attachmentData = ciphersService
@@ -479,7 +485,7 @@ class CipherManagerImpl(
479485
.decryptFile(
480486
userId = userId,
481487
cipher = cipher,
482-
attachment = attachment,
488+
attachmentView = attachmentView,
483489
encryptedFilePath = encryptedFile.path,
484490
decryptedFilePath = decryptedFile.path,
485491
)
@@ -494,17 +500,17 @@ class CipherManagerImpl(
494500
private suspend fun CipherView.encryptCipherAndCheckForMigration(
495501
userId: String,
496502
cipherId: String,
497-
): Result<Cipher> =
503+
): Result<EncryptionContext> =
498504
vaultSdkSource
499505
.encryptCipher(userId = userId, cipherView = this)
500-
.flatMap {
506+
.flatMap { encryptionContext ->
501507
// We only migrate the cipher if the original cipher did not have a key and the
502508
// new cipher does. This means the SDK created the key and migration is required.
503-
if (it.key != null && this.key == null) {
509+
if (encryptionContext.cipher.key != null && this.key == null) {
504510
ciphersService
505511
.updateCipher(
506512
cipherId = cipherId,
507-
body = it.toEncryptedNetworkCipher(),
513+
body = encryptionContext.toEncryptedNetworkCipher(),
508514
)
509515
.flatMap { response ->
510516
when (response) {
@@ -520,12 +526,16 @@ class CipherManagerImpl(
520526
userId = userId,
521527
cipher = response.cipher,
522528
)
523-
response.cipher.toEncryptedSdkCipher().asSuccess()
529+
EncryptionContext(
530+
encryptedFor = encryptionContext.encryptedFor,
531+
cipher = response.cipher.toEncryptedSdkCipher(),
532+
)
533+
.asSuccess()
524534
}
525535
}
526536
}
527537
} else {
528-
it.asSuccess()
538+
encryptionContext.asSuccess()
529539
}
530540
}
531541

@@ -541,7 +551,7 @@ class CipherManagerImpl(
541551
?: return IllegalStateException("CipherView must have an ID").asFailure()
542552
var migratedCipherView = cipherView
543553
.encryptCipherAndCheckForMigration(userId = userId, cipherId = cipherViewId)
544-
.flatMap { vaultSdkSource.decryptCipher(userId = userId, cipher = it) }
554+
.flatMap { vaultSdkSource.decryptCipher(userId = userId, cipher = it.cipher) }
545555
.getOrElse { return it.asFailure() }
546556

547557
attachmentViewsToMigrate
@@ -574,7 +584,12 @@ class CipherManagerImpl(
574584
cipherId = cipherViewId,
575585
)
576586
}
577-
.flatMap { vaultSdkSource.decryptCipher(userId = userId, cipher = it) }
587+
.flatMap {
588+
vaultSdkSource.decryptCipher(
589+
userId = userId,
590+
cipher = it.cipher,
591+
)
592+
}
578593
.onSuccess { migratedCipherView = it }
579594
}
580595
?: IllegalStateException("AttachmentView must have an ID").asFailure()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ class VaultLockManagerImpl(
181181
email = email,
182182
privateKey = privateKey,
183183
method = initUserCryptoMethod,
184+
userId = userId,
184185
),
185186
)
186187
.flatMap { result ->

‎app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensions.kt

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.bitwarden.vault.CipherPermissions
2020
import com.bitwarden.vault.CipherRepromptType
2121
import com.bitwarden.vault.CipherType
2222
import com.bitwarden.vault.CipherView
23+
import com.bitwarden.vault.EncryptionContext
2324
import com.bitwarden.vault.Fido2Credential
2425
import com.bitwarden.vault.Field
2526
import com.bitwarden.vault.FieldType
@@ -37,8 +38,12 @@ import java.time.ZonedDateTime
3738
/**
3839
* Converts a Bitwarden SDK [Cipher] object to a corresponding
3940
* [SyncResponseJson.Cipher] object.
41+
*
42+
* @param encryptedFor The ID of the user who this cipher is encrypted for.
4043
*/
41-
fun Cipher.toEncryptedNetworkCipher(): CipherJsonRequest =
44+
fun Cipher.toEncryptedNetworkCipher(
45+
encryptedFor: String,
46+
): CipherJsonRequest =
4247
CipherJsonRequest(
4348
notes = notes,
4449
attachments = attachments
@@ -59,13 +64,18 @@ fun Cipher.toEncryptedNetworkCipher(): CipherJsonRequest =
5964
card = card?.toEncryptedNetworkCard(),
6065
key = key,
6166
sshKey = sshKey?.toEncryptedNetworkSshKey(),
67+
encryptedFor = encryptedFor,
6268
)
6369

6470
/**
6571
* Converts a Bitwarden SDK [Cipher] object to a corresponding
6672
* [SyncResponseJson.Cipher] object.
73+
*
74+
* @param encryptedFor The ID of the user who this cipher is encrypted for.
6775
*/
68-
fun Cipher.toEncryptedNetworkCipherResponse(): SyncResponseJson.Cipher =
76+
fun Cipher.toEncryptedNetworkCipherResponse(
77+
encryptedFor: String,
78+
): SyncResponseJson.Cipher =
6979
SyncResponseJson.Cipher(
7080
notes = notes,
7181
reprompt = reprompt.toNetworkRepromptType(),
@@ -92,6 +102,7 @@ fun Cipher.toEncryptedNetworkCipherResponse(): SyncResponseJson.Cipher =
92102
id = id.orEmpty(),
93103
shouldViewPassword = viewPassword,
94104
key = key,
105+
encryptedFor = encryptedFor,
95106
)
96107

97108
/**
@@ -626,3 +637,17 @@ fun List<CipherListView>.sortAlphabetically(): List<CipherListView> {
626637
},
627638
)
628639
}
640+
641+
/**
642+
* Converts a Bitwarden SDK [EncryptionContext] object to a corresponding [CipherJsonRequest]
643+
* object.
644+
*/
645+
fun EncryptionContext.toEncryptedNetworkCipher(): CipherJsonRequest =
646+
cipher.toEncryptedNetworkCipher(encryptedFor = encryptedFor)
647+
648+
/**
649+
* Converts a Bitwarden SDK [EncryptionContext] object to a corresponding [SyncResponseJson.Cipher]
650+
* object.
651+
*/
652+
fun EncryptionContext.toEncryptedNetworkCipherResponse(): SyncResponseJson.Cipher =
653+
cipher.toEncryptedNetworkCipherResponse(encryptedFor = encryptedFor)

‎app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceTest.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,8 @@ private const val CIPHER_JSON = """
419419
"publicKey": "mockPublicKey-1",
420420
"privateKey": "mockPrivateKey-1",
421421
"keyFingerprint": "mockKeyFingerprint-1"
422-
}
422+
},
423+
"encryptedFor": "mockEncryptedFor-1"
423424
}
424425
"""
425426

0 commit comments

Comments
 (0)