From ba11dff7ae2fd0ae2aa054a40c9240233f968988 Mon Sep 17 00:00:00 2001 From: Nik Gilmore Date: Fri, 3 Oct 2025 16:16:09 -0700 Subject: [PATCH] Add calls to use SDK entirely for cipher sharing operation. --- .../abstractions/cipher-encryption.service.ts | 16 ++- .../src/vault/services/cipher.service.ts | 111 ++++++++---------- .../default-cipher-encryption.service.ts | 72 +++++++++++- 3 files changed, 134 insertions(+), 65 deletions(-) diff --git a/libs/common/src/vault/abstractions/cipher-encryption.service.ts b/libs/common/src/vault/abstractions/cipher-encryption.service.ts index 6057a91bae5..bca8a2ab2f5 100644 --- a/libs/common/src/vault/abstractions/cipher-encryption.service.ts +++ b/libs/common/src/vault/abstractions/cipher-encryption.service.ts @@ -2,7 +2,7 @@ import { UserKey } from "@bitwarden/common/types/key"; import { EncryptionContext } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherListView } from "@bitwarden/sdk-internal"; -import { UserId, OrganizationId } from "../../types/guid"; +import { UserId, OrganizationId, CollectionId } from "../../types/guid"; import { Cipher } from "../models/domain/cipher"; import { AttachmentView } from "../models/view/attachment.view"; import { CipherView } from "../models/view/cipher.view"; @@ -33,6 +33,20 @@ export abstract class CipherEncryptionService { userId: UserId, ): Promise; + abstract share( + model: CipherView, + organizationId: OrganizationId, + userId: UserId, + collectionIds: CollectionId[], + ): Promise; + + abstract shareMany( + models: CipherView[], + organizationId: OrganizationId, + userId: UserId, + collectionIds: CollectionId[], + ): Promise; + /** * Encrypts a cipher for a given userId with a new key for key rotation. * @param model The cipher view to encrypt diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 6504fd1a97d..2e5a65b55d6 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -912,40 +912,33 @@ export class CipherService implements CipherServiceAbstraction { await this.adjustCipherHistory(cipher, userId, originalCipher); - let encCipher: EncryptionContext; if (sdkCipherEncryptionEnabled) { - // The SDK does not expect the cipher to already have an organizationId. It will result in the wrong - // cipher encryption key being used during the move to organization operation. - if (cipher.organizationId != null) { - throw new Error("Cipher is already associated with an organization."); - } - - encCipher = await this.cipherEncryptionService.moveToOrganization( + return await this.cipherEncryptionService.share( cipher, organizationId as OrganizationId, userId, + collectionIds as CollectionId[], ); - encCipher.cipher.collectionIds = collectionIds; - } else { - // This old attachment logic is safe to remove after it is replaced in PM-22750; which will require fixing - // the attachment before sharing. - const attachmentPromises: Promise[] = []; - if (cipher.attachments != null) { - cipher.attachments.forEach((attachment) => { - if (attachment.key == null) { - attachmentPromises.push( - this.shareAttachmentWithServer(attachment, cipher.id, organizationId), - ); - } - }); - } - await Promise.all(attachmentPromises); - - cipher.organizationId = organizationId; - cipher.collectionIds = collectionIds; - encCipher = await this.encryptSharedCipher(cipher, userId); } + // This old attachment logic is safe to remove after it is replaced in PM-22750; which will require fixing + // the attachment before sharing. + const attachmentPromises: Promise[] = []; + if (cipher.attachments != null) { + cipher.attachments.forEach((attachment) => { + if (attachment.key == null) { + attachmentPromises.push( + this.shareAttachmentWithServer(attachment, cipher.id, organizationId), + ); + } + }); + } + await Promise.all(attachmentPromises); + + cipher.organizationId = organizationId; + cipher.collectionIds = collectionIds; + const encCipher = await this.encryptSharedCipher(cipher, userId); + const request = new CipherShareRequest(encCipher); const response = await this.apiService.putShareCipher(cipher.id, request); const data = new CipherData(response, collectionIds); @@ -962,25 +955,17 @@ export class CipherService implements CipherServiceAbstraction { const sdkCipherEncryptionEnabled = await this.configService.getFeatureFlag( FeatureFlag.PM22136_SdkCipherEncryption, ); - const promises: Promise[] = []; - const encCiphers: Cipher[] = []; - for (const cipher of ciphers) { - if (sdkCipherEncryptionEnabled) { - // The SDK does not expect the cipher to already have an organizationId. It will result in the wrong - // cipher encryption key being used during the move to organization operation. - if (cipher.organizationId != null) { - throw new Error("Cipher is already associated with an organization."); - } - - promises.push( - this.cipherEncryptionService - .moveToOrganization(cipher, organizationId as OrganizationId, userId) - .then((encCipher) => { - encCipher.cipher.collectionIds = collectionIds; - encCiphers.push(encCipher.cipher); - }), - ); - } else { + if (sdkCipherEncryptionEnabled) { + return await this.cipherEncryptionService.shareMany( + ciphers, + organizationId as OrganizationId, + userId, + collectionIds as CollectionId[], + ); + } else { + const promises: Promise[] = []; + const encCiphers: Cipher[] = []; + for (const cipher of ciphers) { cipher.organizationId = organizationId; cipher.collectionIds = collectionIds; promises.push( @@ -989,26 +974,26 @@ export class CipherService implements CipherServiceAbstraction { }), ); } - } - await Promise.all(promises); - const request = new CipherBulkShareRequest(encCiphers, collectionIds, userId); - try { - const response = await this.apiService.putShareCiphers(request); - const responseMap = new Map(response.data.map((r) => [r.id, r])); + await Promise.all(promises); + const request = new CipherBulkShareRequest(encCiphers, collectionIds, userId); + try { + const response = await this.apiService.putShareCiphers(request); + const responseMap = new Map(response.data.map((r) => [r.id, r])); - encCiphers.forEach((cipher) => { - const matchingCipher = responseMap.get(cipher.id); - if (matchingCipher) { - cipher.revisionDate = new Date(matchingCipher.revisionDate); + encCiphers.forEach((cipher) => { + const matchingCipher = responseMap.get(cipher.id); + if (matchingCipher) { + cipher.revisionDate = new Date(matchingCipher.revisionDate); + } + }); + await this.upsert(encCiphers.map((c) => c.toCipherData())); + } catch (e) { + for (const cipher of ciphers) { + cipher.organizationId = null; + cipher.collectionIds = null; } - }); - await this.upsert(encCiphers.map((c) => c.toCipherData())); - } catch (e) { - for (const cipher of ciphers) { - cipher.organizationId = null; - cipher.collectionIds = null; + throw e; } - throw e; } } diff --git a/libs/common/src/vault/services/default-cipher-encryption.service.ts b/libs/common/src/vault/services/default-cipher-encryption.service.ts index 3f03e0f5e9e..276e6c0de3c 100644 --- a/libs/common/src/vault/services/default-cipher-encryption.service.ts +++ b/libs/common/src/vault/services/default-cipher-encryption.service.ts @@ -11,7 +11,7 @@ import { import { LogService } from "../../platform/abstractions/log.service"; import { SdkService, asUuid, uuidAsString } from "../../platform/abstractions/sdk/sdk.service"; -import { UserId, OrganizationId } from "../../types/guid"; +import { UserId, OrganizationId, CollectionId } from "../../types/guid"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; import { CipherType } from "../enums"; import { Cipher } from "../models/domain/cipher"; @@ -86,6 +86,76 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService { ); } + async shareMany( + models: CipherView[], + organizationId: OrganizationId, + userId: UserId, + collectionIds: CollectionId[], + ): Promise { + return firstValueFrom( + this.sdkService.userClient$(userId).pipe( + map(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + + using ref = sdk.take(); + const sdkCipherViews = models.map((model) => this.toSdkCipherView(model, ref.value)); + + const ciphers = await ref.value + .vault() + .ciphers() + .share_ciphers_bulk( + sdkCipherViews, + asUuid(organizationId), + collectionIds.map((id) => asUuid(id)), + ); + + return ciphers.map((c) => Cipher.fromSdkCipher(c)); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to move cipher to organization: ${error}`); + return EMPTY; + }), + ), + ); + } + + async share( + model: CipherView, + organizationId: OrganizationId, + userId: UserId, + collectionIds: CollectionId[], + ): Promise { + return firstValueFrom( + this.sdkService.userClient$(userId).pipe( + map(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + + using ref = sdk.take(); + const sdkCipherView = this.toSdkCipherView(model, ref.value); + + const movedCipher = await ref.value + .vault() + .ciphers() + .share_cipher( + sdkCipherView, + asUuid(organizationId), + collectionIds.map((id) => asUuid(id)), + ); + + return Cipher.fromSdkCipher(movedCipher); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to move cipher to organization: ${error}`); + return EMPTY; + }), + ), + ); + } + async encryptCipherForRotation( model: CipherView, userId: UserId,