From 1bb2635d710b55047c9d2d13e326914afffe0467 Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Thu, 20 Aug 2026 18:13:33 -0300 Subject: [PATCH 1/3] [PM-42197] feat: Thread encryptedByKeyId through cipher requests and add userKeyId to decryption response --- .../UserDecryptionResponseModel.swift | 5 ++ .../Helpers/CipherEncryptionMediator.swift | 2 + .../CipherEncryptionMediatorTests.swift | 6 +- .../BulkShareCiphersRequestModel.swift | 12 ++- .../Request/CipherCreateRequestModel.swift | 5 +- .../Models/Request/CipherRequestModel.swift | 9 ++- .../Vault/Repositories/VaultRepository.swift | 4 + .../Repositories/VaultRepositoryTests.swift | 14 +++- .../API/Cipher/CipherAPIService.swift | 80 ++++++++++++++++--- .../API/Cipher/CipherAPIServiceTests.swift | 4 + .../Cipher/Requests/AddCipherRequest.swift | 11 ++- .../AddCipherWithCollectionsRequest.swift | 9 ++- .../Requests/BulkShareCiphersRequest.swift | 4 +- .../Cipher/Requests/ShareCipherRequest.swift | 9 ++- .../Cipher/Requests/UpdateCipherRequest.swift | 9 ++- .../Core/Vault/Services/CipherService.swift | 39 ++++++--- .../Vault/Services/CipherServiceTests.swift | 22 +++-- .../Fido2CredentialStoreService.swift | 12 ++- .../Fido2CredentialStoreServiceTests.swift | 8 +- .../TestHelpers/MockCipherService.swift | 15 +++- 20 files changed, 224 insertions(+), 55 deletions(-) diff --git a/BitwardenShared/Core/Auth/Models/Response/UserDecryptionResponseModel.swift b/BitwardenShared/Core/Auth/Models/Response/UserDecryptionResponseModel.swift index 1e7c735aa7..9608053d09 100644 --- a/BitwardenShared/Core/Auth/Models/Response/UserDecryptionResponseModel.swift +++ b/BitwardenShared/Core/Auth/Models/Response/UserDecryptionResponseModel.swift @@ -7,4 +7,9 @@ struct UserDecryptionResponseModel: Codable, Equatable { /// The user's master password unlock info. let masterPasswordUnlock: MasterPasswordUnlockResponseModel? + + /// The hex-encoded ID of the user's current key. + /// + /// - Note: `nil` for legacy V1 accounts whose keys carry no key ID. + let userKeyId: String? } diff --git a/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediator.swift b/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediator.swift index b12e0e1703..9237c1d135 100644 --- a/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediator.swift +++ b/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediator.swift @@ -66,6 +66,7 @@ class DefaultCipherEncryptionMediator: CipherEncryptionMediator { if didAddCipherKey { try await cipherService.updateCipherWithServer( cipherEncryptionContext.cipher, + encryptedByKeyId: cipherEncryptionContext.encryptedByKeyId, encryptedFor: cipherEncryptionContext.encryptedFor, ) } @@ -89,6 +90,7 @@ class DefaultCipherEncryptionMediator: CipherEncryptionMediator { try await cipherService.updateCipherWithServer( cipherEncryptionContext.cipher, + encryptedByKeyId: cipherEncryptionContext.encryptedByKeyId, encryptedFor: cipherEncryptionContext.encryptedFor, ) diff --git a/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediatorTests.swift b/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediatorTests.swift index 84ed1447a2..57e786bdb1 100644 --- a/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediatorTests.swift +++ b/BitwardenShared/Core/Vault/Helpers/CipherEncryptionMediatorTests.swift @@ -48,12 +48,13 @@ class CipherEncryptionMediatorTests: BitwardenTestCase { let cipherView = CipherView.fixture(key: nil) let encryptedCipher = Cipher.fixture(key: "encryptedKey") clientService.mockVault.clientCiphers.encryptClosure = { _ in - EncryptionContext(encryptedFor: "userId", cipher: encryptedCipher) + EncryptionContext(encryptedFor: "userId", encryptedByKeyId: "key-1", cipher: encryptedCipher) } let result = try await subject.encryptAndUpdateCipher(cipherView) XCTAssertEqual(cipherService.updateCipherWithServerCiphers, [encryptedCipher]) + XCTAssertEqual(cipherService.updateCipherWithServerEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "userId") XCTAssertEqual(result, encryptedCipher) } @@ -155,7 +156,7 @@ class CipherEncryptionMediatorTests: BitwardenTestCase { let encryptedCipher = Cipher.fixture(key: "encryptedKey") let updatedCipherView = CipherView.fixture(id: "1", key: "decryptedKey") clientService.mockVault.clientCiphers.encryptClosure = { _ in - EncryptionContext(encryptedFor: "userId", cipher: encryptedCipher) + EncryptionContext(encryptedFor: "userId", encryptedByKeyId: "key-1", cipher: encryptedCipher) } delegate.fetchCipherReturnValue = updatedCipherView subject.setDelegate(delegate) @@ -163,6 +164,7 @@ class CipherEncryptionMediatorTests: BitwardenTestCase { let result = try await subject.updateCipherKeyIfNeeded(cipherView) XCTAssertEqual(cipherService.updateCipherWithServerCiphers, [encryptedCipher]) + XCTAssertEqual(cipherService.updateCipherWithServerEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "userId") XCTAssertEqual(delegate.fetchCipherReceivedId, "1") XCTAssertEqual(result, updatedCipherView) diff --git a/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift b/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift index 7cc06ea07d..3d181f0cc3 100644 --- a/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift +++ b/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift @@ -19,10 +19,18 @@ extension BulkShareCiphersRequestModel { /// - Parameters: /// - ciphers: The `Cipher` objects to share. /// - collectionIds: The collection identifiers to share the ciphers with. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. /// - encryptedFor: The user ID who encrypted the ciphers. /// - init(ciphers: [Cipher], collectionIds: [String], encryptedFor: String?) { - self.ciphers = ciphers.map { CipherRequestModel(cipher: $0, encryptedFor: encryptedFor, includeId: true) } + init(ciphers: [Cipher], collectionIds: [String], encryptedByKeyId: String? = nil, encryptedFor: String?) { + self.ciphers = ciphers.map { cipher in + CipherRequestModel( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + includeId: true, + ) + } self.collectionIds = collectionIds } } diff --git a/BitwardenShared/Core/Vault/Models/Request/CipherCreateRequestModel.swift b/BitwardenShared/Core/Vault/Models/Request/CipherCreateRequestModel.swift index ac0cebacfa..472d343323 100644 --- a/BitwardenShared/Core/Vault/Models/Request/CipherCreateRequestModel.swift +++ b/BitwardenShared/Core/Vault/Models/Request/CipherCreateRequestModel.swift @@ -19,9 +19,10 @@ extension CipherCreateRequestModel { /// /// - Parameters: /// - cipher: The `Cipher` used to initialize a `CipherCreateRequestModel`. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - init(cipher: Cipher, encryptedFor: String?) { - self.cipher = CipherRequestModel(cipher: cipher, encryptedFor: encryptedFor) + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String?) { + self.cipher = CipherRequestModel(cipher: cipher, encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor) collectionIds = cipher.collectionIds } } diff --git a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift index 96140d93ae..d50e2ede54 100644 --- a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift +++ b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift @@ -31,6 +31,11 @@ struct CipherRequestModel: JSONRequestBody { /// Driver's license data if the cipher is a driver's license. let driversLicense: CipherDriversLicenseModel? + /// The hex-encoded ID of the key used to encrypt this cipher's fields. + /// + /// - Note: `nil` for legacy AES-CBC-HMAC keys (V1 accounts) that carry no key ID. + let encryptedByKeyId: String? + /// The ID of the user that encrypted the cipher. It should always represent a UserId. /// This is used to check that the user who encrypted the cipher is the same making the request. let encryptedFor: String? @@ -98,10 +103,11 @@ extension CipherRequestModel { /// /// - Parameters: /// - cipher: The `Cipher` used to initialize a `CipherRequestModel`. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. /// - includeId: Whether to include the cipher's ID in the request model. Defaults to `false`. /// - init(cipher: Cipher, encryptedFor: String? = nil, includeId: Bool = false) { + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String? = nil, includeId: Bool = false) { self.init( archivedDate: cipher.archivedDate, attachments2: cipher.attachments?.reduce(into: [String: AttachmentRequestModel]()) { result, attachment in @@ -112,6 +118,7 @@ extension CipherRequestModel { card: cipher.card.map(CipherCardModel.init), data: cipher.data, driversLicense: cipher.driversLicense.map(CipherDriversLicenseModel.init), + encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor, favorite: cipher.favorite, fields: cipher.fields?.map(CipherFieldModel.init), diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift index a755247bad..83df2f9e1c 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift @@ -501,6 +501,7 @@ extension DefaultVaultRepository: VaultRepository { let cipherEncryptionContext = try await clientService.vault().ciphers().encrypt(cipherView: cipher) try await cipherService.addCipherWithServer( cipherEncryptionContext.cipher, + encryptedByKeyId: cipherEncryptionContext.encryptedByKeyId, encryptedFor: cipherEncryptionContext.encryptedFor, ) } @@ -558,6 +559,7 @@ extension DefaultVaultRepository: VaultRepository { try await cipherService.bulkShareCiphersWithServer( encryptionContexts.map(\.cipher), collectionIds: newCollectionIds, + encryptedByKeyId: encryptionContexts.first?.encryptedByKeyId, encryptedFor: encryptedFor, ) } @@ -879,6 +881,7 @@ extension DefaultVaultRepository: VaultRepository { .encrypt(cipherView: organizationCipher) try await cipherService.shareCipherWithServer( organizationCipherEncryptionContext.cipher, + encryptedByKeyId: organizationCipherEncryptionContext.encryptedByKeyId, encryptedFor: organizationCipherEncryptionContext.encryptedFor, ) } @@ -911,6 +914,7 @@ extension DefaultVaultRepository: VaultRepository { let cipherEncryptionContext = try await clientService.vault().ciphers().encrypt(cipherView: cipherView) try await cipherService.updateCipherWithServer( cipherEncryptionContext.cipher, + encryptedByKeyId: cipherEncryptionContext.encryptedByKeyId, encryptedFor: cipherEncryptionContext.encryptedFor, ) } diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift index 31c46ed1c5..0f815b629a 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift @@ -141,6 +141,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b XCTAssertEqual(clientCiphers.encryptReceivedCipherView, cipher) XCTAssertEqual(cipherService.addCipherWithServerCiphers.last, Cipher(cipherView: cipher)) + XCTAssertNil(cipherService.addCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.addCipherWithServerEncryptedFor, "1") } @@ -188,8 +189,8 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b CipherView.fixture(id: "2"), ] let encryptionContexts = [ - EncryptionContext(encryptedFor: "1", cipher: .fixture(id: "1")), - EncryptionContext(encryptedFor: "1", cipher: .fixture(id: "2")), + EncryptionContext(encryptedFor: "1", encryptedByKeyId: "key-1", cipher: .fixture(id: "1")), + EncryptionContext(encryptedFor: "1", encryptedByKeyId: "key-1", cipher: .fixture(id: "2")), ] clientCiphers.prepareCiphersForBulkShareReturnValue = encryptionContexts @@ -201,6 +202,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b // Verify bulk share was called. XCTAssertEqual(cipherService.bulkShareCiphersWithServerCiphers.last, encryptionContexts.map(\.cipher)) XCTAssertEqual(cipherService.bulkShareCiphersWithServerCollectionIds, ["col-1", "col-2"]) + XCTAssertEqual(cipherService.bulkShareCiphersEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.bulkShareCiphersWithServerEncryptedFor, "1") } @@ -257,7 +259,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b ) let encryptionContexts = [ - EncryptionContext(encryptedFor: "1", cipher: cipherAfterAttachmentDelete), + EncryptionContext(encryptedFor: "1", encryptedByKeyId: "key-1", cipher: cipherAfterAttachmentDelete), ] clientCiphers.prepareCiphersForBulkShareReturnValue = encryptionContexts @@ -277,6 +279,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b // Verify bulk share was called. XCTAssertEqual(cipherService.bulkShareCiphersWithServerCiphers.last, encryptionContexts.map(\.cipher)) XCTAssertEqual(cipherService.bulkShareCiphersWithServerCollectionIds, ["col-1"]) + XCTAssertEqual(cipherService.bulkShareCiphersEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.bulkShareCiphersWithServerEncryptedFor, "1") } @@ -1481,6 +1484,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b XCTAssertEqual(clientCiphers.moveToOrganizationReceivedArguments?.organizationId, "5") XCTAssertEqual(cipherService.shareCipherWithServerCiphers.last, Cipher(cipherView: updatedCipher)) + XCTAssertNil(cipherService.shareCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.shareCipherWithServerEncryptedFor, "1") } @@ -1648,6 +1652,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b try await subject.updateCipher(cipher) XCTAssertEqual(clientCiphers.encryptReceivedCipherView, cipher) + XCTAssertNil(cipherService.updateCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "1") } @@ -1664,6 +1669,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b // Verify cipher was unarchived before updating let unarchivedCipher = archivedCipher.update(archivedDate: nil) XCTAssertEqual(clientCiphers.encryptReceivedCipherView, unarchivedCipher) + XCTAssertNil(cipherService.updateCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "1") } @@ -1679,6 +1685,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b // Verify cipher was NOT unarchived (kept archived) XCTAssertEqual(clientCiphers.encryptReceivedCipherView, archivedCipher) + XCTAssertNil(cipherService.updateCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "1") } @@ -1694,6 +1701,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b // Verify cipher was updated normally (no changes) XCTAssertEqual(clientCiphers.encryptReceivedCipherView, cipher) + XCTAssertNil(cipherService.updateCipherWithServerEncryptedByKeyId) XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "1") } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift index 2f486cd09e..e81be8650f 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift @@ -20,10 +20,15 @@ protocol CipherAPIService { /// /// - Parameters: /// - cipher: The cipher that the user is adding. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. /// - Returns: The cipher that was added to the user's vault. /// - func addCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel + func addCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel /// Performs an API request to archive an existing cipher in the user's vault. /// @@ -37,22 +42,29 @@ protocol CipherAPIService { /// /// - Parameters: /// - cipher: The cipher that the user is adding. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. /// - Returns: The cipher that was added to the user's vault. /// - func addCipherWithCollections(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel + func addCipherWithCollections( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel /// Performs an API request to share multiple ciphers with an organization. /// /// - Parameters: /// - ciphers: The ciphers to share. /// - collectionIds: The collection identifiers to share the ciphers with. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. /// - encryptedFor: The user ID who encrypted the ciphers. /// - Returns: The response containing the shared ciphers. /// func bulkShareCiphers( _ ciphers: [Cipher], collectionIds: [String], + encryptedByKeyId: String?, encryptedFor: String?, ) async throws -> BulkShareCiphersResponseModel @@ -126,10 +138,15 @@ protocol CipherAPIService { /// /// - Parameters: /// - cipher: The cipher to share. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. /// - Returns: The cipher that was shared with the organization. /// - func shareCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel + func shareCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel /// Performs an API request to soft delete an existing cipher in the user's vault. /// @@ -149,10 +166,15 @@ protocol CipherAPIService { /// /// - Parameters: /// - cipher: The cipher that the user is updating. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. /// - Returns: The cipher that was updated in the user's vault. /// - func updateCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel + func updateCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel /// Performs an API request to update the collections that a cipher is included in. /// @@ -171,26 +193,44 @@ protocol CipherAPIService { } extension APIService: CipherAPIService { - func addCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel { - try await apiService.send(AddCipherRequest(cipher: cipher, encryptedFor: encryptedFor)) + func addCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel { + try await apiService.send(AddCipherRequest( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + )) } func archiveCipher(withID id: String) async throws -> CipherDetailsResponseModel { try await apiService.send(ArchiveCipherRequest(id: id)) } - func addCipherWithCollections(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel { - try await apiService.send(AddCipherWithCollectionsRequest(cipher: cipher, encryptedFor: encryptedFor)) + func addCipherWithCollections( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel { + try await apiService.send(AddCipherWithCollectionsRequest( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + )) } func bulkShareCiphers( _ ciphers: [Cipher], collectionIds: [String], + encryptedByKeyId: String?, encryptedFor: String?, ) async throws -> BulkShareCiphersResponseModel { try await apiService.send(BulkShareCiphersRequest( ciphers: ciphers, collectionIds: collectionIds, + encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor, )) } @@ -233,8 +273,16 @@ extension APIService: CipherAPIService { )) } - func shareCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel { - try await apiService.send(ShareCipherRequest(cipher: cipher, encryptedFor: encryptedFor)) + func shareCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel { + try await apiService.send(ShareCipherRequest( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + )) } func softDeleteCipher(withID id: String) async throws -> EmptyResponse { @@ -245,8 +293,16 @@ extension APIService: CipherAPIService { try await apiService.send(UnarchiveCipherRequest(id: id)) } - func updateCipher(_ cipher: Cipher, encryptedFor: String?) async throws -> CipherDetailsResponseModel { - let updateRequest = try UpdateCipherRequest(cipher: cipher, encryptedFor: encryptedFor) + func updateCipher( + _ cipher: Cipher, + encryptedByKeyId: String?, + encryptedFor: String?, + ) async throws -> CipherDetailsResponseModel { + let updateRequest = try UpdateCipherRequest( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) return try await apiService.send(updateRequest) } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift index 14a06a3472..04008f2738 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift @@ -37,6 +37,7 @@ class CipherAPIServiceTests: XCTestCase { // swiftlint:disable:this type_body_le let response = try await subject.addCipher( .fixture(), + encryptedByKeyId: nil, encryptedFor: "1", ) @@ -96,6 +97,7 @@ class CipherAPIServiceTests: XCTestCase { // swiftlint:disable:this type_body_le let response = try await subject.addCipherWithCollections( .fixture(collectionIds: ["1", "2", "3"]), + encryptedByKeyId: nil, encryptedFor: "1", ) @@ -215,6 +217,7 @@ class CipherAPIServiceTests: XCTestCase { // swiftlint:disable:this type_body_le .fixture(collectionIds: ["1", "2"], id: "2"), ], collectionIds: ["1", "2"], + encryptedByKeyId: nil, encryptedFor: "user-1", ) @@ -383,6 +386,7 @@ class CipherAPIServiceTests: XCTestCase { // swiftlint:disable:this type_body_le let response = try await subject.shareCipher( .fixture(collectionIds: ["1", "2", "3"], id: "1"), + encryptedByKeyId: nil, encryptedFor: "1", ) diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherRequest.swift index 73208c2013..bf84e15ef8 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherRequest.swift @@ -28,8 +28,13 @@ struct AddCipherRequest: Request { /// /// - Parameters: /// - cipher: The `Cipher` to add to the user's vault. - /// - encryptedFor: The user ID who encrypted the `cipher` - init(cipher: Cipher, encryptedFor: String?) { - requestModel = CipherRequestModel(cipher: cipher, encryptedFor: encryptedFor) + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. + /// - encryptedFor: The user ID who encrypted the `cipher`. + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String?) { + requestModel = CipherRequestModel( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherWithCollectionsRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherWithCollectionsRequest.swift index 2d11dfb67e..f7708a0b2f 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherWithCollectionsRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/AddCipherWithCollectionsRequest.swift @@ -28,8 +28,13 @@ struct AddCipherWithCollectionsRequest: Request { /// /// - Parameters: /// - cipher: The `Cipher` to add to the user's vault. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - init(cipher: Cipher, encryptedFor: String?) { - requestModel = CipherCreateRequestModel(cipher: cipher, encryptedFor: encryptedFor) + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String?) { + requestModel = CipherCreateRequestModel( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift index d772233beb..b9c39cfb6d 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift @@ -36,9 +36,10 @@ struct BulkShareCiphersRequest: Request { /// - Parameters: /// - ciphers: The `Cipher` objects to share with an organization. /// - collectionIds: The collection identifiers to share the ciphers with. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. /// - encryptedFor: The user ID who encrypted the ciphers. /// - init(ciphers: [Cipher], collectionIds: [String], encryptedFor: String?) throws { + init(ciphers: [Cipher], collectionIds: [String], encryptedByKeyId: String? = nil, encryptedFor: String?) throws { // Validate all ciphers have IDs guard ciphers.allSatisfy({ $0.id != nil }) else { throw BulkShareCiphersRequestError.missingCipherId @@ -46,6 +47,7 @@ struct BulkShareCiphersRequest: Request { requestModel = BulkShareCiphersRequestModel( ciphers: ciphers, collectionIds: collectionIds, + encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor, ) } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/ShareCipherRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/ShareCipherRequest.swift index d1649aa964..e28e324451 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/ShareCipherRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/ShareCipherRequest.swift @@ -38,10 +38,15 @@ struct ShareCipherRequest: Request { /// /// - Parameters: /// - cipher: The `Cipher` to share with an organization. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - init(cipher: Cipher, encryptedFor: String?) throws { + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String?) throws { guard let id = cipher.id else { throw ShareCipherRequestError.missingCipherId } self.id = id - requestModel = CipherCreateRequestModel(cipher: cipher, encryptedFor: encryptedFor) + requestModel = CipherCreateRequestModel( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/UpdateCipherRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/UpdateCipherRequest.swift index 587b2e9130..b39fa236bd 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/UpdateCipherRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/UpdateCipherRequest.swift @@ -33,11 +33,16 @@ struct UpdateCipherRequest: Request { /// /// - Parameters: /// - cipher: The `Cipher` to update in the user's vault. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - init(cipher: Cipher, encryptedFor: String?) throws { + init(cipher: Cipher, encryptedByKeyId: String? = nil, encryptedFor: String?) throws { guard let id = cipher.id, !id.isEmpty else { throw CipherAPIServiceError.updateMissingId } self.id = id - requestModel = CipherRequestModel(cipher: cipher, encryptedFor: encryptedFor) + requestModel = CipherRequestModel( + cipher: cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } } diff --git a/BitwardenShared/Core/Vault/Services/CipherService.swift b/BitwardenShared/Core/Vault/Services/CipherService.swift index e112055b8c..697d21fb5e 100644 --- a/BitwardenShared/Core/Vault/Services/CipherService.swift +++ b/BitwardenShared/Core/Vault/Services/CipherService.swift @@ -11,8 +11,9 @@ protocol CipherService { /// /// - Parameters: /// - cipher: The cipher to add. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - func addCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws + func addCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws /// Archives a cipher for the current user both in the backend and in local storage. /// @@ -27,10 +28,12 @@ protocol CipherService { /// - Parameters: /// - ciphers: The ciphers to share. /// - collectionIds: The collection IDs to associate with the ciphers. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. /// - encryptedFor: The user ID who encrypted the ciphers. func bulkShareCiphersWithServer( _ ciphers: [Cipher], collectionIds: [String], + encryptedByKeyId: String?, encryptedFor: String, ) async throws @@ -130,8 +133,9 @@ protocol CipherService { /// /// - Parameters: /// - cipher: The cipher to share. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - func shareCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws + func shareCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws /// Soft deletes a cipher for the current user both in the backend and in local storage. /// @@ -157,8 +161,9 @@ protocol CipherService { /// /// - Parameters: /// - cipher: The cipher to update. + /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the `cipher`. /// - encryptedFor: The user ID who encrypted the `cipher`. - func updateCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws + func updateCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws /// Updates the cipher for the current user in local storage only. /// @@ -222,14 +227,18 @@ class DefaultCipherService: CipherService { } extension DefaultCipherService { - func addCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func addCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { let userId = try await stateService.getActiveAccountId() // Add the cipher in the backend. var response: CipherDetailsResponseModel = if cipher.collectionIds.isEmpty { - try await cipherAPIService.addCipher(cipher, encryptedFor: encryptedFor) + try await cipherAPIService.addCipher(cipher, encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor) } else { - try await cipherAPIService.addCipherWithCollections(cipher, encryptedFor: encryptedFor) + try await cipherAPIService.addCipherWithCollections( + cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } // The API doesn't return the collectionIds, so manually add them back. @@ -255,6 +264,7 @@ extension DefaultCipherService { func bulkShareCiphersWithServer( _ ciphers: [Cipher], collectionIds: [String], + encryptedByKeyId: String?, encryptedFor: String, ) async throws { let userId = try await stateService.getActiveAccountId() @@ -263,6 +273,7 @@ extension DefaultCipherService { let response = try await cipherAPIService.bulkShareCiphers( ciphers, collectionIds: collectionIds, + encryptedByKeyId: encryptedByKeyId, encryptedFor: encryptedFor, ) @@ -408,11 +419,15 @@ extension DefaultCipherService { return updatedCipher } - func shareCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func shareCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { let userId = try await stateService.getActiveAccountId() // Share the cipher from the backend. - var response = try await cipherAPIService.shareCipher(cipher, encryptedFor: encryptedFor) + var response = try await cipherAPIService.shareCipher( + cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) // The API doesn't return the collectionIds, so manually add them back. response.collectionIds = cipher.collectionIds @@ -459,12 +474,16 @@ extension DefaultCipherService { } } - func updateCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func updateCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { let userId = try await stateService.getActiveAccountId() // Update the cipher in the backend. var response: CipherDetailsResponseModel = if cipher.edit { - try await cipherAPIService.updateCipher(cipher, encryptedFor: encryptedFor) + try await cipherAPIService.updateCipher( + cipher, + encryptedByKeyId: encryptedByKeyId, + encryptedFor: encryptedFor, + ) } else { // if the cipher is not editable, update the favorite status and folder only. try await cipherAPIService.updateCipherPreference(cipher) diff --git a/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift b/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift index 9a7dc3bfc0..cc941b0d6c 100644 --- a/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift @@ -52,7 +52,7 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod stateService.activeAccount = .fixtureAccountLogin() client.result = .httpSuccess(testData: .cipherResponse) - try await subject.addCipherWithServer(.fixture(), encryptedFor: "1") + try await subject.addCipherWithServer(.fixture(), encryptedByKeyId: nil, encryptedFor: "1") XCTAssertEqual(client.requests.count, 1) XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers") @@ -65,7 +65,7 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod client.result = .httpSuccess(testData: .cipherResponse) let cipher = Cipher.fixture(collectionIds: ["1"]) - try await subject.addCipherWithServer(cipher, encryptedFor: "1") + try await subject.addCipherWithServer(cipher, encryptedByKeyId: nil, encryptedFor: "1") XCTAssertEqual(client.requests.count, 1) XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers/create") @@ -99,7 +99,12 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod Cipher.fixture(id: "2"), ] let collectionIds = ["col-1", "col-2"] - try await subject.bulkShareCiphersWithServer(ciphers, collectionIds: collectionIds, encryptedFor: "1") + try await subject.bulkShareCiphersWithServer( + ciphers, + collectionIds: collectionIds, + encryptedByKeyId: nil, + encryptedFor: "1", + ) XCTAssertEqual(client.requests.count, 1) XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers/share") @@ -324,7 +329,7 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod stateService.activeAccount = .fixture() let cipher = Cipher.fixture(collectionIds: ["1", "2"], id: "123") - try await subject.shareCipherWithServer(cipher, encryptedFor: "1") + try await subject.shareCipherWithServer(cipher, encryptedByKeyId: nil, encryptedFor: "1") var cipherResponse = try CipherDetailsResponseModel( response: .success(body: APITestData.cipherResponse.data), @@ -413,7 +418,7 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod stateService.activeAccount = .fixtureAccountLogin() client.result = .httpSuccess(testData: .cipherResponse) - try await subject.updateCipherWithServer(.fixture(id: "123"), encryptedFor: "1") + try await subject.updateCipherWithServer(.fixture(id: "123"), encryptedByKeyId: nil, encryptedFor: "1") XCTAssertEqual(client.requests.count, 1) XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers/123") @@ -435,6 +440,7 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod folderId: "folderId", id: "123", ), + encryptedByKeyId: nil, encryptedFor: "1", ) @@ -454,7 +460,11 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod stateService.activeAccount = .fixtureAccountLogin() client.result = .httpSuccess(testData: .cipherResponse) - try await subject.updateCipherWithServer(.fixture(collectionIds: ["1", "2"], id: "123"), encryptedFor: "1") + try await subject.updateCipherWithServer( + .fixture(collectionIds: ["1", "2"], id: "123"), + encryptedByKeyId: nil, + encryptedFor: "1", + ) XCTAssertEqual(client.requests.count, 1) XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers/123") diff --git a/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreService.swift b/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreService.swift index 148c7b05dc..e53811b462 100644 --- a/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreService.swift +++ b/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreService.swift @@ -70,9 +70,17 @@ final class Fido2CredentialStoreService: Fido2CredentialStore { /// - Parameter cred: Cipher/Credential to add/update. func saveCredential(cred: BitwardenSdk.EncryptionContext) async throws { if cred.cipher.id == nil { - try await cipherService.addCipherWithServer(cred.cipher, encryptedFor: cred.encryptedFor) + try await cipherService.addCipherWithServer( + cred.cipher, + encryptedByKeyId: cred.encryptedByKeyId, + encryptedFor: cred.encryptedFor, + ) } else { - try await cipherService.updateCipherWithServer(cred.cipher, encryptedFor: cred.encryptedFor) + try await cipherService.updateCipherWithServer( + cred.cipher, + encryptedByKeyId: cred.encryptedByKeyId, + encryptedFor: cred.encryptedFor, + ) } } diff --git a/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreServiceTests.swift b/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreServiceTests.swift index 6c4134f870..3bbfaff505 100644 --- a/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/Fido2CredentialStoreServiceTests.swift @@ -397,16 +397,20 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable /// `.saveCredential(cred:)` add cipher to server when no id present. func test_saveCredential_add() async throws { try await subject.saveCredential( - cred: EncryptionContext(encryptedFor: "1", cipher: .fixture()), + cred: EncryptionContext(encryptedFor: "1", encryptedByKeyId: "key-1", cipher: .fixture()), ) XCTAssertTrue(cipherService.addCipherWithServerCiphers.count == 1) + XCTAssertEqual(cipherService.addCipherWithServerEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.addCipherWithServerEncryptedFor, "1") } /// `.saveCredential(cred:)` add cipher to server when no id present. func test_saveCredential_update() async throws { - try await subject.saveCredential(cred: EncryptionContext(encryptedFor: "1", cipher: .fixture(id: "1"))) + try await subject.saveCredential( + cred: EncryptionContext(encryptedFor: "1", encryptedByKeyId: "key-1", cipher: .fixture(id: "1")), + ) XCTAssertTrue(cipherService.updateCipherWithServerCiphers.count == 1) + XCTAssertEqual(cipherService.updateCipherWithServerEncryptedByKeyId, "key-1") XCTAssertEqual(cipherService.updateCipherWithServerEncryptedFor, "1") } diff --git a/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift b/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift index 8e770db506..36d2e2a05f 100644 --- a/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift +++ b/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift @@ -6,6 +6,7 @@ import Foundation class MockCipherService: CipherService { var addCipherWithServerCiphers = [Cipher]() + var addCipherWithServerEncryptedByKeyId: String? var addCipherWithServerEncryptedFor: String? var addCipherWithServerResult: Result = .success(()) @@ -15,6 +16,7 @@ class MockCipherService: CipherService { var bulkShareCiphersWithServerCiphers = [[Cipher]]() var bulkShareCiphersWithServerCollectionIds: [String]? + var bulkShareCiphersEncryptedByKeyId: String? var bulkShareCiphersWithServerEncryptedFor: String? var bulkShareCiphersWithServerResult: Result = .success(()) @@ -65,6 +67,7 @@ class MockCipherService: CipherService { var saveAttachmentWithServerResult: Result = .success(.fixture()) var shareCipherWithServerCiphers = [Cipher]() + var shareCipherWithServerEncryptedByKeyId: String? var shareCipherWithServerEncryptedFor: String? var shareCipherWithServerResult: Result = .success(()) @@ -79,6 +82,7 @@ class MockCipherService: CipherService { var updateCipherWithLocalStorageResult: Result = .success(()) var updateCipherWithServerCiphers = [Cipher]() + var updateCipherWithServerEncryptedByKeyId: String? var updateCipherWithServerEncryptedFor: String? var updateCipherWithServerResult: Result = .success(()) @@ -89,8 +93,9 @@ class MockCipherService: CipherService { var unarchivedCipher: Cipher? var unarchiveWithServerResult: Result = .success(()) - func addCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func addCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { addCipherWithServerCiphers.append(cipher) + addCipherWithServerEncryptedByKeyId = encryptedByKeyId addCipherWithServerEncryptedFor = encryptedFor try addCipherWithServerResult.get() } @@ -104,10 +109,12 @@ class MockCipherService: CipherService { func bulkShareCiphersWithServer( _ ciphers: [Cipher], collectionIds: [String], + encryptedByKeyId: String?, encryptedFor: String, ) async throws { bulkShareCiphersWithServerCiphers.append(ciphers) bulkShareCiphersWithServerCollectionIds = collectionIds + bulkShareCiphersEncryptedByKeyId = encryptedByKeyId bulkShareCiphersWithServerEncryptedFor = encryptedFor try bulkShareCiphersWithServerResult.get() } @@ -173,8 +180,9 @@ class MockCipherService: CipherService { return try saveAttachmentWithServerResult.get() } - func shareCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func shareCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { shareCipherWithServerCiphers.append(cipher) + shareCipherWithServerEncryptedByKeyId = encryptedByKeyId shareCipherWithServerEncryptedFor = encryptedFor try shareCipherWithServerResult.get() } @@ -201,8 +209,9 @@ class MockCipherService: CipherService { return try updateCipherWithLocalStorageResult.get() } - func updateCipherWithServer(_ cipher: Cipher, encryptedFor: String) async throws { + func updateCipherWithServer(_ cipher: Cipher, encryptedByKeyId: String?, encryptedFor: String) async throws { updateCipherWithServerCiphers.append(cipher) + updateCipherWithServerEncryptedByKeyId = encryptedByKeyId updateCipherWithServerEncryptedFor = encryptedFor try updateCipherWithServerResult.get() } From 86253d605a5f2e8cabd039ab2c644f8508fceb4d Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Fri, 21 Aug 2026 12:08:26 -0300 Subject: [PATCH 2/3] [PM-42197] feat: Thread EncryptionContext through CXF cipher import service stack --- .../ImportCiphersRepository.swift | 5 ++- .../ImportCiphersRepositoryTests.swift | 3 +- .../API/ImportCiphersAPIService.swift | 8 ++--- .../API/ImportCiphersAPIServiceTests.swift | 17 +++++++--- .../API/Requests/ImportCiphersRequest.swift | 8 ++--- .../Requests/ImportCiphersRequestTests.swift | 31 ++++++++++++++----- .../Tools/Services/ImportCiphersService.swift | 8 ++--- .../Services/ImportCiphersServiceTests.swift | 17 +++++++--- .../MockImportCiphersService.swift | 10 +++--- .../Models/Request/CipherRequestModel.swift | 12 +++++++ .../Request/CipherRequestModelTests.swift | 30 ++++++++++++++++++ 11 files changed, 113 insertions(+), 36 deletions(-) diff --git a/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepository.swift b/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepository.swift index 8a12542707..2a6adffd22 100644 --- a/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepository.swift +++ b/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepository.swift @@ -89,13 +89,12 @@ extension DefaultImportCiphersRepository: ImportCiphersRepository { } let encryptionContexts = try await clientService.exporters().importCxf(payload: accountJsonString) - let ciphers = encryptionContexts.map(\.cipher) await onProgress(0.3) _ = try await importCiphersService .importCiphers( - ciphers: ciphers, + encryptionContexts: encryptionContexts, folders: [], folderRelationships: [], ) @@ -104,7 +103,7 @@ extension DefaultImportCiphersRepository: ImportCiphersRepository { try await syncService.fetchSync(forceSync: true) - let importedCredentialsCount = cxfCredentialsResultBuilder.build(from: ciphers) + let importedCredentialsCount = cxfCredentialsResultBuilder.build(from: encryptionContexts.map(\.cipher)) await onProgress(1.0) diff --git a/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepositoryTests.swift b/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepositoryTests.swift index 43b01fd204..f55d5591e3 100644 --- a/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepositoryTests.swift +++ b/BitwardenShared/Core/Tools/Repositories/ImportCiphersRepositoryTests.swift @@ -105,7 +105,8 @@ class ImportCiphersRepositoryTests: BitwardenTestCase { XCTAssertNotNil(clientService.mockExporters.importCxfReceivedPayload) XCTAssertTrue(importCiphersService.importCiphersCalled) - XCTAssertEqual(importCiphersService.importCiphersCiphers?.count, 9) + XCTAssertEqual(importCiphersService.importCiphersEncryptionContexts?.count, 9) + XCTAssertEqual(importCiphersService.importCiphersEncryptionContexts?[0].encryptedFor, "1") XCTAssertTrue(syncService.didFetchSync) XCTAssertTrue(syncService.fetchSyncForceSync == true) XCTAssertEqual(progressReports, [0.3, 0.8, 1.0]) diff --git a/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIService.swift b/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIService.swift index ddd079cbe8..8a4610b545 100644 --- a/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIService.swift +++ b/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIService.swift @@ -8,12 +8,12 @@ import Networking protocol ImportCiphersAPIService { /// Performs an API request to import ciphers in the vault. /// - Parameters: - /// - ciphers: The ciphers to import. + /// - encryptionContexts: The encryption contexts containing ciphers and their encryption metadata to import. /// - folders: The folders to import. /// - folderRelationships: The cipher<->folder relationships map. The key is the cipher index /// and the value is the folder index in their respective arrays. func importCiphers( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder], folderRelationships: [(key: Int, value: Int)], ) async throws -> EmptyResponse @@ -21,14 +21,14 @@ protocol ImportCiphersAPIService { extension APIService: ImportCiphersAPIService { func importCiphers( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder], folderRelationships: [(key: Int, value: Int)], ) async throws -> EmptyResponse { try await apiService .send( ImportCiphersRequest( - ciphers: ciphers, + encryptionContexts: encryptionContexts, folders: folders, folderRelationships: folderRelationships, ), diff --git a/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIServiceTests.swift b/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIServiceTests.swift index a9f0b202d1..da8d4de1df 100644 --- a/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIServiceTests.swift +++ b/BitwardenShared/Core/Tools/Services/API/ImportCiphersAPIServiceTests.swift @@ -1,3 +1,4 @@ +import BitwardenSdk import TestHelpers import XCTest @@ -28,12 +29,16 @@ class ImportCiphersAPIServiceTests: BitwardenTestCase { // MARK: Tests - /// `importCiphers(ciphers:folders:folderRelationships:)` performs the import ciphers request. + /// `importCiphers(encryptionContexts:folders:folderRelationships:)` performs the import ciphers request. func test_importCiphers() async throws { client.results = [ .httpSuccess(testData: .emptyResponse), ] - _ = try await subject.importCiphers(ciphers: [.fixture()], folders: [], folderRelationships: []) + _ = try await subject.importCiphers( + encryptionContexts: [EncryptionContext(encryptedFor: "user-1", cipher: .fixture())], + folders: [], + folderRelationships: [], + ) XCTAssertEqual(client.requests.count, 1) XCTAssertNotNil(client.requests[0].body) @@ -41,14 +46,18 @@ class ImportCiphersAPIServiceTests: BitwardenTestCase { XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/ciphers/import") } - /// `importCiphers(ciphers:folders:folderRelationships:)` performs the import ciphers request. + /// `importCiphers(encryptionContexts:folders:folderRelationships:)` throws on API failure. func test_importCiphers_throws() async throws { client.results = [ .httpFailure(BitwardenTestError.example), ] await assertAsyncThrows(error: BitwardenTestError.example) { - _ = try await subject.importCiphers(ciphers: [.fixture()], folders: [], folderRelationships: []) + _ = try await subject.importCiphers( + encryptionContexts: [EncryptionContext(encryptedFor: "user-1", cipher: .fixture())], + folders: [], + folderRelationships: [], + ) } } } diff --git a/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequest.swift b/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequest.swift index be4bb10b18..f30ed88eb7 100644 --- a/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequest.swift +++ b/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequest.swift @@ -27,21 +27,21 @@ struct ImportCiphersRequest: Request { /// Initialize a `ImportCiphersRequest` for ciphers, folders and its relattionship. /// - Parameters: - /// - ciphers: Ciphers to import. + /// - encryptionContexts: The encryption contexts containing ciphers and their encryption metadata to import. /// - folders: Folders to import. /// - folderRelationships: The cipher<->folder relationships map. The key is the cipher index /// and the value is the folder index in their respective arrays. init( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder] = [], folderRelationships: [(key: Int, value: Int)] = [], ) throws { - guard !ciphers.isEmpty else { + guard !encryptionContexts.isEmpty else { throw BitwardenError.dataError("There are no ciphers to import.") } requestModel = ImportCiphersRequestModel( - ciphers: ciphers.map { CipherRequestModel(cipher: $0) }, + ciphers: encryptionContexts.map { CipherRequestModel(encryptionContext: $0) }, folders: folders.map { FolderWithIdRequestModel(folder: $0) }, folderRelationships: folderRelationships.map { FolderRelationship(key: $0.key, value: $0.value) }, ) diff --git a/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequestTests.swift b/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequestTests.swift index 7b7e547372..f25dfdef93 100644 --- a/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequestTests.swift +++ b/BitwardenShared/Core/Tools/Services/API/Requests/ImportCiphersRequestTests.swift @@ -1,3 +1,4 @@ +import BitwardenSdk import XCTest @testable import BitwardenShared @@ -8,35 +9,51 @@ import XCTest class ImportCiphersRequestTests: BitwardenTestCase { // MARK: Tests - /// `init(ciphers:folders:folderRelationships:)` initializes the request successfully. + /// `init(encryptionContexts:folders:folderRelationships:)` initializes the request successfully. func test_init() throws { let subject = try ImportCiphersRequest( - ciphers: [.fixture(name: "cipherTest")], + encryptionContexts: [ + EncryptionContext( + encryptedFor: "user-1", + encryptedByKeyId: "key-abc", + cipher: .fixture(name: "cipherTest"), + ), + ], folders: [.fixture(name: "folderTest")], folderRelationships: [(1, 1)], ) XCTAssertEqual(subject.body?.ciphers[0].name, "cipherTest") + XCTAssertEqual(subject.body?.ciphers[0].encryptedFor, "user-1") + XCTAssertEqual(subject.body?.ciphers[0].encryptedByKeyId, "key-abc") XCTAssertEqual(subject.body?.folders[0].name, "folderTest") XCTAssertEqual(subject.body?.folderRelationships[0].key, 1) XCTAssertEqual(subject.body?.folderRelationships[0].value, 1) } - /// `init(ciphers:folders:folderRelationships:)` initializes the request successfully. + /// `init(encryptionContexts:folders:folderRelationships:)` throws when the contexts are empty. func test_init_throws() throws { XCTAssertThrowsError(_ = try ImportCiphersRequest( - ciphers: [], + encryptionContexts: [], )) } /// `path` returns the correct path. func test_path() throws { - let subject = try ImportCiphersRequest(ciphers: [.fixture()]) + let subject = try ImportCiphersRequest( + encryptionContexts: [ + EncryptionContext(encryptedFor: "user-1", cipher: .fixture()), + ], + ) XCTAssertEqual(subject.path, "/ciphers/import") } - /// `method` is `.put`. + /// `method` is `.post`. func test_method() throws { - let subject = try ImportCiphersRequest(ciphers: [.fixture()]) + let subject = try ImportCiphersRequest( + encryptionContexts: [ + EncryptionContext(encryptedFor: "user-1", cipher: .fixture()), + ], + ) XCTAssertEqual(subject.method, .post) } } diff --git a/BitwardenShared/Core/Tools/Services/ImportCiphersService.swift b/BitwardenShared/Core/Tools/Services/ImportCiphersService.swift index 442659e43c..26b8a81242 100644 --- a/BitwardenShared/Core/Tools/Services/ImportCiphersService.swift +++ b/BitwardenShared/Core/Tools/Services/ImportCiphersService.swift @@ -9,12 +9,12 @@ import Foundation protocol ImportCiphersService { /// Performs an API request to import ciphers in the vault. /// - Parameters: - /// - ciphers: The ciphers to import. + /// - encryptionContexts: The encryption contexts containing ciphers and their encryption metadata to import. /// - folders: The folders to import. /// - folderRelationships: The cipher<->folder relationships map. The key is the cipher index /// and the value is the folder index in their respective arrays. func importCiphers( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder], folderRelationships: [(key: Int, value: Int)], ) async throws @@ -42,13 +42,13 @@ class DefaultImportCiphersService: ImportCiphersService { extension DefaultImportCiphersService { func importCiphers( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder], folderRelationships: [(key: Int, value: Int)], ) async throws { _ = try await importCiphersAPIService .importCiphers( - ciphers: ciphers, + encryptionContexts: encryptionContexts, folders: folders, folderRelationships: folderRelationships, ) diff --git a/BitwardenShared/Core/Tools/Services/ImportCiphersServiceTests.swift b/BitwardenShared/Core/Tools/Services/ImportCiphersServiceTests.swift index 350a8d256a..670d290207 100644 --- a/BitwardenShared/Core/Tools/Services/ImportCiphersServiceTests.swift +++ b/BitwardenShared/Core/Tools/Services/ImportCiphersServiceTests.swift @@ -1,3 +1,4 @@ +import BitwardenSdk import TestHelpers import XCTest @@ -33,20 +34,28 @@ class ImportCiphersServiceTests: BitwardenTestCase { // MARK: Tests - /// `importCiphers(ciphers:folders:folderRelationships:)` import the ciphers calling the API. + /// `importCiphers(encryptionContexts:folders:folderRelationships:)` imports the ciphers calling the API. func test_importCiphers_succeeds() async throws { client.results = [.httpSuccess(testData: .emptyResponse)] - try await subject.importCiphers(ciphers: [.fixture()], folders: [], folderRelationships: []) + try await subject.importCiphers( + encryptionContexts: [EncryptionContext(encryptedFor: "user-1", cipher: .fixture())], + folders: [], + folderRelationships: [], + ) let request = try XCTUnwrap(client.requests.first) XCTAssertEqual(request.url.absoluteString, "https://example.com/api/ciphers/import") XCTAssertEqual(request.method, .post) } - /// `importCiphers(ciphers:folders:folderRelationships:)` throws when calling the API. + /// `importCiphers(encryptionContexts:folders:folderRelationships:)` throws when calling the API. func test_importCiphers_throws() async throws { client.results = [.httpFailure(BitwardenTestError.example)] await assertAsyncThrows(error: BitwardenTestError.example) { - try await subject.importCiphers(ciphers: [.fixture()], folders: [], folderRelationships: []) + try await subject.importCiphers( + encryptionContexts: [EncryptionContext(encryptedFor: "user-1", cipher: .fixture())], + folders: [], + folderRelationships: [], + ) } } } diff --git a/BitwardenShared/Core/Tools/Services/TestHelpers/MockImportCiphersService.swift b/BitwardenShared/Core/Tools/Services/TestHelpers/MockImportCiphersService.swift index 0080793d13..b58e584d32 100644 --- a/BitwardenShared/Core/Tools/Services/TestHelpers/MockImportCiphersService.swift +++ b/BitwardenShared/Core/Tools/Services/TestHelpers/MockImportCiphersService.swift @@ -4,20 +4,20 @@ import BitwardenSdk class MockImportCiphersService: ImportCiphersService { var importCiphersCalled = false - var importCiphersCiphers: [Cipher]? + var importCiphersEncryptionContexts: [EncryptionContext]? var importCiphersError: Error? - var importCiphersFolders: [Folder]? var importCiphersFolderRelationships: [(key: Int, value: Int)]? + var importCiphersFolders: [Folder]? func importCiphers( - ciphers: [Cipher], + encryptionContexts: [EncryptionContext], folders: [Folder], folderRelationships: [(key: Int, value: Int)], ) async throws { importCiphersCalled = true - importCiphersCiphers = ciphers - importCiphersFolders = folders + importCiphersEncryptionContexts = encryptionContexts importCiphersFolderRelationships = folderRelationships + importCiphersFolders = folders if let importCiphersError { throw importCiphersError } diff --git a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift index d50e2ede54..e5487f3fd4 100644 --- a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift +++ b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModel.swift @@ -139,4 +139,16 @@ extension CipherRequestModel { type: CipherType(type: cipher.type), ) } + + /// Initialize a `CipherRequestModel` from an `EncryptionContext`. + /// + /// - Parameter encryptionContext: The `EncryptionContext` used to initialize a `CipherRequestModel`. + /// + init(encryptionContext: EncryptionContext) { + self.init( + cipher: encryptionContext.cipher, + encryptedByKeyId: encryptionContext.encryptedByKeyId, + encryptedFor: encryptionContext.encryptedFor, + ) + } } diff --git a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModelTests.swift b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModelTests.swift index dfae4d96f8..2b23a3eeea 100644 --- a/BitwardenShared/Core/Vault/Models/Request/CipherRequestModelTests.swift +++ b/BitwardenShared/Core/Vault/Models/Request/CipherRequestModelTests.swift @@ -136,4 +136,34 @@ struct CipherRequestModelTests { #expect(subject.passport == nil) } + + /// `init(encryptionContext:)` maps `encryptedFor`, `encryptedByKeyId`, and cipher data. + @Test + func init_encryptionContext() { + let context = EncryptionContext( + encryptedFor: "user-1", + encryptedByKeyId: "key-abc", + cipher: .fixture(name: "Test Cipher"), + ) + + let subject = CipherRequestModel(encryptionContext: context) + + #expect(subject.encryptedFor == "user-1") + #expect(subject.encryptedByKeyId == "key-abc") + #expect(subject.name == "Test Cipher") + } + + /// `init(encryptionContext:)` maps a `nil` `encryptedByKeyId` when the context has none. + @Test + func init_encryptionContext_nilEncryptedByKeyId() { + let context = EncryptionContext( + encryptedFor: "user-1", + cipher: .fixture(), + ) + + let subject = CipherRequestModel(encryptionContext: context) + + #expect(subject.encryptedFor == "user-1") + #expect(subject.encryptedByKeyId == nil) + } } From 6d862b7c1dde133ad2fe46ebdc88c61bca355eed Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Fri, 21 Aug 2026 12:44:59 -0300 Subject: [PATCH 3/3] Fix bulk share stamping first context's encryptedByKeyId onto all ciphers --- .../BulkShareCiphersRequestModel.swift | 16 ++++----- .../Vault/Repositories/VaultRepository.swift | 6 ++-- .../Repositories/VaultRepositoryTests.swift | 14 +++----- .../API/Cipher/CipherAPIService.swift | 16 +++------ .../API/Cipher/CipherAPIServiceTests.swift | 7 ++-- .../Requests/BulkShareCiphersRequest.swift | 15 +++------ .../BulkShareCiphersRequestTests.swift | 33 +++++++++++-------- .../Core/Vault/Services/CipherService.swift | 20 ++++------- .../Vault/Services/CipherServiceTests.swift | 12 +++---- .../TestHelpers/MockCipherService.swift | 12 ++----- 10 files changed, 59 insertions(+), 92 deletions(-) diff --git a/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift b/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift index 3d181f0cc3..375e53b200 100644 --- a/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift +++ b/BitwardenShared/Core/Vault/Models/Request/BulkShareCiphersRequestModel.swift @@ -14,20 +14,18 @@ struct BulkShareCiphersRequestModel: JSONRequestBody { } extension BulkShareCiphersRequestModel { - /// Initialize a `BulkShareCiphersRequestModel` from an array of `Cipher` objects. + /// Initialize a `BulkShareCiphersRequestModel` from an array of `EncryptionContext` objects. /// /// - Parameters: - /// - ciphers: The `Cipher` objects to share. + /// - encryptionContexts: The encryption contexts containing the ciphers and per-cipher encryption metadata. /// - collectionIds: The collection identifiers to share the ciphers with. - /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. - /// - encryptedFor: The user ID who encrypted the ciphers. /// - init(ciphers: [Cipher], collectionIds: [String], encryptedByKeyId: String? = nil, encryptedFor: String?) { - self.ciphers = ciphers.map { cipher in + init(encryptionContexts: [EncryptionContext], collectionIds: [String]) { + ciphers = encryptionContexts.map { context in CipherRequestModel( - cipher: cipher, - encryptedByKeyId: encryptedByKeyId, - encryptedFor: encryptedFor, + cipher: context.cipher, + encryptedByKeyId: context.encryptedByKeyId, + encryptedFor: context.encryptedFor, includeId: true, ) } diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift index 83df2f9e1c..190b1a63d9 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift @@ -552,15 +552,13 @@ extension DefaultVaultRepository: VaultRepository { collectionIds: newCollectionIds, ) - guard let encryptedFor = encryptionContexts.first?.encryptedFor else { + guard !encryptionContexts.isEmpty else { return } try await cipherService.bulkShareCiphersWithServer( - encryptionContexts.map(\.cipher), + encryptionContexts, collectionIds: newCollectionIds, - encryptedByKeyId: encryptionContexts.first?.encryptedByKeyId, - encryptedFor: encryptedFor, ) } diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift index 0f815b629a..765f6ce3c9 100644 --- a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift +++ b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift @@ -200,10 +200,8 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b XCTAssertEqual(cipherEncryptionMediator.encryptAndUpdateCipherCallsCount, 2) // Verify bulk share was called. - XCTAssertEqual(cipherService.bulkShareCiphersWithServerCiphers.last, encryptionContexts.map(\.cipher)) + XCTAssertEqual(cipherService.bulkShareCiphersEncryptionContexts.last, encryptionContexts) XCTAssertEqual(cipherService.bulkShareCiphersWithServerCollectionIds, ["col-1", "col-2"]) - XCTAssertEqual(cipherService.bulkShareCiphersEncryptedByKeyId, "key-1") - XCTAssertEqual(cipherService.bulkShareCiphersWithServerEncryptedFor, "1") } /// `bulkShareCiphers()` migrates attachments without an attachment key. @@ -277,10 +275,8 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b XCTAssertThrowsError(try Data(contentsOf: decryptUrl)) // Verify bulk share was called. - XCTAssertEqual(cipherService.bulkShareCiphersWithServerCiphers.last, encryptionContexts.map(\.cipher)) + XCTAssertEqual(cipherService.bulkShareCiphersEncryptionContexts.last, encryptionContexts) XCTAssertEqual(cipherService.bulkShareCiphersWithServerCollectionIds, ["col-1"]) - XCTAssertEqual(cipherService.bulkShareCiphersEncryptedByKeyId, "key-1") - XCTAssertEqual(cipherService.bulkShareCiphersWithServerEncryptedFor, "1") } /// `bulkShareCiphers()` does not call the cipher service when encryption contexts are empty. @@ -292,7 +288,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b try await subject.bulkShareCiphers(ciphers, newOrganizationId: "org-123", newCollectionIds: ["col-1"]) - XCTAssertTrue(cipherService.bulkShareCiphersWithServerCiphers.isEmpty) + XCTAssertTrue(cipherService.bulkShareCiphersEncryptionContexts.isEmpty) } /// `canShowVaultFilter()` returns true if only org and personal ownership policies are disabled. @@ -1337,7 +1333,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b ) // Verify that bulkShareCiphers was called with the correct cipher IDs. - let sharedCipherIds = cipherService.bulkShareCiphersWithServerCiphers.first?.compactMap(\.id) + let sharedCipherIds = cipherService.bulkShareCiphersEncryptionContexts.first?.compactMap(\.cipher.id) XCTAssertEqual(sharedCipherIds?.sorted(), ["1", "2", "4"]) XCTAssertEqual(cipherService.bulkShareCiphersWithServerCollectionIds, ["default-collection-id"]) } @@ -1358,7 +1354,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b try await subject.migratePersonalVault(to: "target-org") // Verify that no bulk share was attempted. - XCTAssertTrue(cipherService.bulkShareCiphersWithServerCiphers.isEmpty) + XCTAssertTrue(cipherService.bulkShareCiphersEncryptionContexts.isEmpty) } /// `migratePersonalVault(to:)` throws an error when no default collection is found. diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift index e81be8650f..8e726670c5 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIService.swift @@ -55,17 +55,13 @@ protocol CipherAPIService { /// Performs an API request to share multiple ciphers with an organization. /// /// - Parameters: - /// - ciphers: The ciphers to share. + /// - encryptionContexts: The encryption contexts containing the ciphers and per-cipher encryption metadata. /// - collectionIds: The collection identifiers to share the ciphers with. - /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. - /// - encryptedFor: The user ID who encrypted the ciphers. /// - Returns: The response containing the shared ciphers. /// func bulkShareCiphers( - _ ciphers: [Cipher], + _ encryptionContexts: [EncryptionContext], collectionIds: [String], - encryptedByKeyId: String?, - encryptedFor: String?, ) async throws -> BulkShareCiphersResponseModel /// Performs an API request to delete an existing attachment in the user's vault. @@ -222,16 +218,12 @@ extension APIService: CipherAPIService { } func bulkShareCiphers( - _ ciphers: [Cipher], + _ encryptionContexts: [EncryptionContext], collectionIds: [String], - encryptedByKeyId: String?, - encryptedFor: String?, ) async throws -> BulkShareCiphersResponseModel { try await apiService.send(BulkShareCiphersRequest( - ciphers: ciphers, + encryptionContexts: encryptionContexts, collectionIds: collectionIds, - encryptedByKeyId: encryptedByKeyId, - encryptedFor: encryptedFor, )) } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift index 04008f2738..b4ac2841bb 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/CipherAPIServiceTests.swift @@ -1,3 +1,4 @@ +import BitwardenSdk import InlineSnapshotTesting import Networking import TestHelpers @@ -213,12 +214,10 @@ class CipherAPIServiceTests: XCTestCase { // swiftlint:disable:this type_body_le let response = try await subject.bulkShareCiphers( [ - .fixture(collectionIds: ["1", "2"], id: "1"), - .fixture(collectionIds: ["1", "2"], id: "2"), + EncryptionContext(encryptedFor: "user-1", cipher: .fixture(collectionIds: ["1", "2"], id: "1")), + EncryptionContext(encryptedFor: "user-1", cipher: .fixture(collectionIds: ["1", "2"], id: "2")), ], collectionIds: ["1", "2"], - encryptedByKeyId: nil, - encryptedFor: "user-1", ) XCTAssertEqual(client.requests.count, 1) diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift index b9c39cfb6d..904e0e13cf 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequest.swift @@ -31,24 +31,19 @@ struct BulkShareCiphersRequest: Request { // MARK: Initialization - /// Initialize a `BulkShareCiphersRequest` for multiple `Cipher` objects. + /// Initialize a `BulkShareCiphersRequest` for multiple `EncryptionContext` objects. /// /// - Parameters: - /// - ciphers: The `Cipher` objects to share with an organization. + /// - encryptionContexts: The encryption contexts containing the ciphers and per-cipher encryption metadata. /// - collectionIds: The collection identifiers to share the ciphers with. - /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. - /// - encryptedFor: The user ID who encrypted the ciphers. /// - init(ciphers: [Cipher], collectionIds: [String], encryptedByKeyId: String? = nil, encryptedFor: String?) throws { - // Validate all ciphers have IDs - guard ciphers.allSatisfy({ $0.id != nil }) else { + init(encryptionContexts: [EncryptionContext], collectionIds: [String]) throws { + guard encryptionContexts.allSatisfy({ $0.cipher.id != nil }) else { throw BulkShareCiphersRequestError.missingCipherId } requestModel = BulkShareCiphersRequestModel( - ciphers: ciphers, + encryptionContexts: encryptionContexts, collectionIds: collectionIds, - encryptedByKeyId: encryptedByKeyId, - encryptedFor: encryptedFor, ) } } diff --git a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequestTests.swift b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequestTests.swift index 2118058f87..f548ef7330 100644 --- a/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequestTests.swift +++ b/BitwardenShared/Core/Vault/Services/API/Cipher/Requests/BulkShareCiphersRequestTests.swift @@ -1,3 +1,4 @@ +import BitwardenSdk import InlineSnapshotTesting import XCTest @@ -12,20 +13,25 @@ class BulkShareCiphersRequestTests: BitwardenTestCase { try await super.setUp() subject = try BulkShareCiphersRequest( - ciphers: [ - .fixture( - collectionIds: ["1", "2"], - id: "123", - revisionDate: Date(year: 2023, month: 10, day: 31), + encryptionContexts: [ + EncryptionContext( + encryptedFor: "user-1", + cipher: .fixture( + collectionIds: ["1", "2"], + id: "123", + revisionDate: Date(year: 2023, month: 10, day: 31), + ), ), - .fixture( - collectionIds: ["1", "2"], - id: "456", - revisionDate: Date(year: 2023, month: 10, day: 31), + EncryptionContext( + encryptedFor: "user-1", + cipher: .fixture( + collectionIds: ["1", "2"], + id: "456", + revisionDate: Date(year: 2023, month: 10, day: 31), + ), ), ], collectionIds: ["1", "2"], - encryptedFor: "user-1", ) } @@ -75,12 +81,11 @@ class BulkShareCiphersRequestTests: BitwardenTestCase { func test_init_missingCipherId() { XCTAssertThrowsError( try BulkShareCiphersRequest( - ciphers: [ - .fixture(id: "123"), - .fixture(id: nil), + encryptionContexts: [ + EncryptionContext(encryptedFor: "user-1", cipher: .fixture(id: "123")), + EncryptionContext(encryptedFor: "user-1", cipher: .fixture(id: nil)), ], collectionIds: ["1"], - encryptedFor: "user-1", ), ) { error in XCTAssertEqual(error as? BulkShareCiphersRequestError, .missingCipherId) diff --git a/BitwardenShared/Core/Vault/Services/CipherService.swift b/BitwardenShared/Core/Vault/Services/CipherService.swift index 697d21fb5e..9c7c033c11 100644 --- a/BitwardenShared/Core/Vault/Services/CipherService.swift +++ b/BitwardenShared/Core/Vault/Services/CipherService.swift @@ -26,15 +26,11 @@ protocol CipherService { /// Shares multiple ciphers with an organization and updates the locally stored data. /// /// - Parameters: - /// - ciphers: The ciphers to share. + /// - encryptionContexts: The encryption contexts containing the ciphers and per-cipher encryption metadata. /// - collectionIds: The collection IDs to associate with the ciphers. - /// - encryptedByKeyId: The hex-encoded ID of the key used to encrypt the ciphers. - /// - encryptedFor: The user ID who encrypted the ciphers. func bulkShareCiphersWithServer( - _ ciphers: [Cipher], + _ encryptionContexts: [EncryptionContext], collectionIds: [String], - encryptedByKeyId: String?, - encryptedFor: String, ) async throws /// Returns the count of ciphers in the data store for the current user. @@ -262,24 +258,20 @@ extension DefaultCipherService { } func bulkShareCiphersWithServer( - _ ciphers: [Cipher], + _ encryptionContexts: [EncryptionContext], collectionIds: [String], - encryptedByKeyId: String?, - encryptedFor: String, ) async throws { let userId = try await stateService.getActiveAccountId() // Share the ciphers with the backend. let response = try await cipherAPIService.bulkShareCiphers( - ciphers, + encryptionContexts, collectionIds: collectionIds, - encryptedByKeyId: encryptedByKeyId, - encryptedFor: encryptedFor, ) // Create a dictionary for quick lookup of original ciphers by ID. - let ciphersById = Dictionary(uniqueKeysWithValues: ciphers.compactMap { cipher in - cipher.id.map { ($0, cipher) } + let ciphersById = Dictionary(uniqueKeysWithValues: encryptionContexts.compactMap { context in + context.cipher.id.map { ($0, context.cipher) } }) // Update ciphers in local storage. diff --git a/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift b/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift index cc941b0d6c..65cd488fb9 100644 --- a/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/CipherServiceTests.swift @@ -88,22 +88,20 @@ class CipherServiceTests: BitwardenTestCase { // swiftlint:disable:this type_bod XCTAssertEqual(cipherDataStore.upsertCipherUserId, "1") } - /// `bulkShareCiphersWithServer(_:collectionIds:encryptedFor:)` shares multiple ciphers with the + /// `bulkShareCiphersWithServer(_:collectionIds:)` shares multiple ciphers with the /// organization and updates the data store. func test_bulkShareCiphersWithServer() async throws { client.result = .httpSuccess(testData: .bulkShareCiphersResponse) stateService.activeAccount = .fixture() - let ciphers = [ - Cipher.fixture(id: "1"), - Cipher.fixture(id: "2"), + let encryptionContexts = [ + EncryptionContext(encryptedFor: "1", cipher: .fixture(id: "1")), + EncryptionContext(encryptedFor: "1", cipher: .fixture(id: "2")), ] let collectionIds = ["col-1", "col-2"] try await subject.bulkShareCiphersWithServer( - ciphers, + encryptionContexts, collectionIds: collectionIds, - encryptedByKeyId: nil, - encryptedFor: "1", ) XCTAssertEqual(client.requests.count, 1) diff --git a/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift b/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift index 36d2e2a05f..2083f7f29f 100644 --- a/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift +++ b/BitwardenShared/Core/Vault/Services/TestHelpers/MockCipherService.swift @@ -14,10 +14,8 @@ class MockCipherService: CipherService { var archiveCipher: Cipher? var archiveCipherResult: Result = .success(()) - var bulkShareCiphersWithServerCiphers = [[Cipher]]() + var bulkShareCiphersEncryptionContexts = [[EncryptionContext]]() var bulkShareCiphersWithServerCollectionIds: [String]? - var bulkShareCiphersEncryptedByKeyId: String? - var bulkShareCiphersWithServerEncryptedFor: String? var bulkShareCiphersWithServerResult: Result = .success(()) var cipherCountResult: Result = .success(0) @@ -107,15 +105,11 @@ class MockCipherService: CipherService { } func bulkShareCiphersWithServer( - _ ciphers: [Cipher], + _ encryptionContexts: [EncryptionContext], collectionIds: [String], - encryptedByKeyId: String?, - encryptedFor: String, ) async throws { - bulkShareCiphersWithServerCiphers.append(ciphers) + bulkShareCiphersEncryptionContexts.append(encryptionContexts) bulkShareCiphersWithServerCollectionIds = collectionIds - bulkShareCiphersEncryptedByKeyId = encryptedByKeyId - bulkShareCiphersWithServerEncryptedFor = encryptedFor try bulkShareCiphersWithServerResult.get() }