diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 2baa5410013..3b49b92cf99 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -303,6 +303,7 @@ import { import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; import { CipherRiskService } from "@bitwarden/common/vault/abstractions/cipher-risk.service"; +import { CipherSdkService } from "@bitwarden/common/vault/abstractions/cipher-sdk.service"; import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherFileUploadService as CipherFileUploadServiceAbstraction } from "@bitwarden/common/vault/abstractions/file-upload/cipher-file-upload.service"; import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction"; @@ -321,6 +322,7 @@ import { CipherAuthorizationService, DefaultCipherAuthorizationService, } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { DefaultCipherSdkService } from "@bitwarden/common/vault/services/cipher-sdk.service"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { DefaultCipherArchiveService } from "@bitwarden/common/vault/services/default-cipher-archive.service"; import { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; @@ -590,6 +592,11 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultDomainSettingsService, deps: [StateProvider, PolicyServiceAbstraction, AccountService], }), + safeProvider({ + provide: CipherSdkService, + useClass: DefaultCipherSdkService, + deps: [SdkService, LogService], + }), safeProvider({ provide: CipherServiceAbstraction, useFactory: ( @@ -607,7 +614,7 @@ const safeProviders: SafeProvider[] = [ logService: LogService, cipherEncryptionService: CipherEncryptionService, messagingService: MessagingServiceAbstraction, - sdkService: SdkService, + cipherSdkService: CipherSdkService, ) => new CipherService( keyService, @@ -624,7 +631,7 @@ const safeProviders: SafeProvider[] = [ logService, cipherEncryptionService, messagingService, - sdkService, + cipherSdkService, ), deps: [ KeyService, @@ -641,7 +648,7 @@ const safeProviders: SafeProvider[] = [ LogService, CipherEncryptionService, MessagingServiceAbstraction, - SdkService, + CipherSdkService, ], }), safeProvider({ diff --git a/libs/common/src/vault/abstractions/cipher-sdk.service.ts b/libs/common/src/vault/abstractions/cipher-sdk.service.ts new file mode 100644 index 00000000000..e5ac75b7f22 --- /dev/null +++ b/libs/common/src/vault/abstractions/cipher-sdk.service.ts @@ -0,0 +1,37 @@ +import { UserId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +/** + * Service responsible for cipher operations using the SDK. + */ +export abstract class CipherSdkService { + /** + * Creates a new cipher on the server using the SDK. + * + * @param cipherView The cipher view to create + * @param userId The user ID to use for SDK client + * @param orgAdmin Whether this is an organization admin operation + * @returns A promise that resolves to the created cipher view + */ + abstract createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise; + + /** + * Updates a cipher on the server using the SDK. + * + * @param cipher The cipher view to update + * @param userId The user ID to use for SDK client + * @param originalCipherView The original cipher view before changes (optional, used for admin operations) + * @param orgAdmin Whether this is an organization admin operation + * @returns A promise that resolves to the updated cipher view + */ + abstract updateWithServer( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise; +} diff --git a/libs/common/src/vault/services/cipher-sdk.service.spec.ts b/libs/common/src/vault/services/cipher-sdk.service.spec.ts new file mode 100644 index 00000000000..6d5590f9860 --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.spec.ts @@ -0,0 +1,250 @@ +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { UserId, CipherId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +import { CipherType } from "../enums/cipher-type"; + +import { DefaultCipherSdkService } from "./cipher-sdk.service"; + +describe("DefaultCipherSdkService", () => { + const sdkService = mock(); + const logService = mock(); + const userId = "test-user-id" as UserId; + const cipherId = "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId; + const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId; + + let cipherSdkService: DefaultCipherSdkService; + let mockSdkClient: any; + let mockCiphersSdk: any; + let mockAdminSdk: any; + let mockVaultSdk: any; + + beforeEach(() => { + // Mock the SDK client chain for admin operations + mockAdminSdk = { + create: jest.fn(), + edit: jest.fn(), + }; + mockCiphersSdk = { + create: jest.fn(), + edit: jest.fn(), + admin: jest.fn().mockReturnValue(mockAdminSdk), + }; + mockVaultSdk = { + ciphers: jest.fn().mockReturnValue(mockCiphersSdk), + }; + const mockSdkValue = { + vault: jest.fn().mockReturnValue(mockVaultSdk), + }; + mockSdkClient = { + take: jest.fn().mockReturnValue({ + value: mockSdkValue, + [Symbol.dispose]: jest.fn(), + }), + }; + + // Mock sdkService to return the mock client + sdkService.userClient$.mockReturnValue(of(mockSdkClient)); + + cipherSdkService = new DefaultCipherSdkService(sdkService, logService); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("createWithServer()", () => { + it("should create cipher using SDK when orgAdmin is false", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Test Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockCiphersSdk.create.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.createWithServer(cipherView, userId, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.create).toHaveBeenCalledWith( + expect.objectContaining({ + name: cipherView.name, + organizationId: expect.anything(), + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result?.name).toBe(cipherView.name); + }); + + it("should create cipher using SDK admin API when orgAdmin is true", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Test Admin Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.create.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.createWithServer(cipherView, userId, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.create).toHaveBeenCalledWith( + expect.objectContaining({ + name: cipherView.name, + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result?.name).toBe(cipherView.name); + }); + + it("should return void and log error when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + const result = await cipherSdkService.createWithServer(cipherView, userId); + + expect(result).toBeUndefined(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to create cipher"), + ); + }); + + it("should return void and log error when SDK throws an error", async () => { + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + mockCiphersSdk.create.mockRejectedValue(new Error("SDK error")); + + const result = await cipherSdkService.createWithServer(cipherView, userId); + + expect(result).toBeUndefined(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to create cipher"), + ); + }); + }); + + describe("updateWithServer()", () => { + it("should update cipher using SDK when orgAdmin is false", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockCiphersSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer(cipherView, userId, undefined, false); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should update cipher using SDK admin API when orgAdmin is true", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Admin Cipher"; + cipherView.organizationId = orgId; + + const originalCipherView = new CipherView(); + originalCipherView.id = cipherId; + originalCipherView.name = "Original Cipher"; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer( + cipherView, + userId, + originalCipherView, + true, + ); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + originalCipherView.toSdkCipherView(), + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should update cipher using SDK admin API without originalCipherView", async () => { + const cipherView = new CipherView(); + cipherView.id = cipherId; + cipherView.type = CipherType.Login; + cipherView.name = "Updated Admin Cipher"; + cipherView.organizationId = orgId; + + const mockSdkCipherView = cipherView.toSdkCipherView(); + mockAdminSdk.edit.mockResolvedValue(mockSdkCipherView); + + const result = await cipherSdkService.updateWithServer(cipherView, userId, undefined, true); + + expect(sdkService.userClient$).toHaveBeenCalledWith(userId); + expect(mockVaultSdk.ciphers).toHaveBeenCalled(); + expect(mockCiphersSdk.admin).toHaveBeenCalled(); + expect(mockAdminSdk.edit).toHaveBeenCalledWith( + expect.objectContaining({ + id: expect.anything(), + name: cipherView.name, + }), + undefined, + ); + expect(result).toBeInstanceOf(CipherView); + expect(result.name).toBe(cipherView.name); + }); + + it("should throw error and log when SDK client is not available", async () => { + sdkService.userClient$.mockReturnValue(of(null)); + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + await expect( + cipherSdkService.updateWithServer(cipherView, userId, undefined, false), + ).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to update cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + mockCiphersSdk.edit.mockRejectedValue(new Error("SDK error")); + + await expect( + cipherSdkService.updateWithServer(cipherView, userId, undefined, false), + ).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to update cipher"), + ); + }); + }); +}); diff --git a/libs/common/src/vault/services/cipher-sdk.service.ts b/libs/common/src/vault/services/cipher-sdk.service.ts new file mode 100644 index 00000000000..e17e1db3abf --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.ts @@ -0,0 +1,79 @@ +import { firstValueFrom, switchMap, catchError, of } from "rxjs"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; + +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; + +export class DefaultCipherSdkService implements CipherSdkService { + constructor( + private sdkService: SdkService, + private logService: LogService, + ) {} + + async createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + const sdkCreateRequest = cipherView.toSdkCreateCipherRequest(); + let result: SdkCipherView; + if (orgAdmin) { + result = await ref.value.vault().ciphers().admin().create(sdkCreateRequest); + } else { + result = await ref.value.vault().ciphers().create(sdkCreateRequest); + } + return CipherView.fromSdkCipherView(result); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to create cipher: ${error}`); + return of(undefined); + }), + ), + ); + } + + async updateWithServer( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + return await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + switchMap(async (sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + using ref = sdk.take(); + const sdkUpdateRequest = cipher.toSdkUpdateCipherRequest(); + let result: SdkCipherView; + if (orgAdmin) { + result = await ref.value + .vault() + .ciphers() + .admin() + .edit(sdkUpdateRequest, originalCipherView?.toSdkCipherView()); + } else { + result = await ref.value.vault().ciphers().edit(sdkUpdateRequest); + } + return CipherView.fromSdkCipherView(result); + }), + catchError((error: unknown) => { + this.logService.error(`Failed to update cipher: ${error}`); + throw error; + }), + ), + ); + } +} diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index 8f061e3a73d..4f98ba62a1c 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -21,7 +21,6 @@ import { EncString } from "../../key-management/crypto/models/enc-string"; import { UriMatchStrategy } from "../../models/domain/domain-service"; import { ConfigService } from "../../platform/abstractions/config/config.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; -import { SdkService } from "../../platform/abstractions/sdk/sdk.service"; import { Utils } from "../../platform/misc/utils"; import { EncArrayBuffer } from "../../platform/models/domain/enc-array-buffer"; import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; @@ -29,6 +28,7 @@ import { ContainerService } from "../../platform/services/container.service"; import { CipherId, UserId, OrganizationId, CollectionId } from "../../types/guid"; import { CipherKey, OrgKey, UserKey } from "../../types/key"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; import { EncryptionContext } from "../abstractions/cipher.service"; import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service"; import { SearchService } from "../abstractions/search.service"; @@ -110,7 +110,7 @@ describe("Cipher Service", () => { const stateProvider = new FakeStateProvider(accountService); const cipherEncryptionService = mock(); const messageSender = mock(); - const sdkService = mock(); + const cipherSdkService = mock(); const userId = "TestUserId" as UserId; const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId; @@ -147,7 +147,7 @@ describe("Cipher Service", () => { logService, cipherEncryptionService, messageSender, - sdkService, + cipherSdkService, ); encryptionContext = { cipher: new Cipher(cipherData), encryptedFor: userId }; @@ -210,48 +210,11 @@ describe("Cipher Service", () => { }); describe("createWithServer()", () => { - let mockSdkClient: any; - let mockCiphersSdk: any; - let mockAdminSdk: any; - let mockVaultSdk: any; - let sdkTestCipher: Cipher; - let sdkTestCipherData: CipherData; - beforeEach(() => { - // Mock encrypt to return encryptionContext for legacy path jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); - // Mock decrypt to return cipherView for result jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { return new CipherView(cipher); }); - - // Create cipher data with valid UUIDs for SDK tests - sdkTestCipherData = cipherData; - sdkTestCipher = new Cipher(sdkTestCipherData); - - // Mock the SDK client chain - define mockAdminSdk first before referencing it - mockAdminSdk = { - create: jest.fn(), - }; - mockCiphersSdk = { - create: jest.fn(), - admin: jest.fn().mockReturnValue(mockAdminSdk), - }; - mockVaultSdk = { - ciphers: jest.fn().mockReturnValue(mockCiphersSdk), - }; - const mockSdkValue = { - vault: jest.fn().mockReturnValue(mockVaultSdk), - }; - mockSdkClient = { - take: jest.fn().mockReturnValue({ - value: mockSdkValue, - [Symbol.dispose]: jest.fn(), - }), - }; - - // Mock sdkService to return the mock client - sdkService.userClient$.mockReturnValue(of(mockSdkClient)); }); it("should call apiService.postCipherAdmin when orgAdmin param is true and the cipher orgId != null", async () => { @@ -267,7 +230,6 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); - expect(mockSdkClient.take).not.toHaveBeenCalled(); }); it("should call apiService.postCipher when orgAdmin param is true and the cipher orgId is null", async () => { @@ -284,7 +246,6 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); - expect(mockSdkClient.take).not.toHaveBeenCalled(); }); it("should call apiService.postCipherCreate if collectionsIds != null", async () => { @@ -301,7 +262,6 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); - expect(mockSdkClient.take).not.toHaveBeenCalled(); }); it("should call apiService.postCipher when orgAdmin and collectionIds logic is false", async () => { @@ -317,103 +277,41 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); - expect(mockSdkClient.take).not.toHaveBeenCalled(); }); - it("should use SDK to create cipher when feature flag is enabled", async () => { + it("should delegate to cipherSdkService when feature flag is enabled", async () => { configService.getFeatureFlag .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) .mockResolvedValue(true); - const cipherView = new CipherView(sdkTestCipher); - const mockSdkCipherView = cipherView.toSdkCipherView(); + const cipherView = new CipherView(encryptionContext.cipher); + const expectedResult = new CipherView(encryptionContext.cipher); - // Mock SDK create to return a cipher view - mockCiphersSdk.create.mockResolvedValue(mockSdkCipherView); + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "createWithServer") + .mockResolvedValue(expectedResult); + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); const apiSpy = jest.spyOn(apiService, "postCipher"); + const result = await cipherService.createWithServer(cipherView, userId); + expect(cipherSdkServiceSpy).toHaveBeenCalledWith(cipherView, userId, undefined); expect(apiSpy).not.toHaveBeenCalled(); - expect(sdkService.userClient$).toHaveBeenCalledWith(userId); - expect(mockCiphersSdk.create).toHaveBeenCalledWith( - expect.objectContaining({ - name: cipherView.name, - organizationId: expect.anything(), - }), - ); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); expect(result).toBeInstanceOf(CipherView); - expect(result.name).toBe(cipherView.name); - }); - - it("should use SDK to create admin cipher when feature flag is enabled and admin flag is passed", async () => { - configService.getFeatureFlag - .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) - .mockResolvedValue(true); - - const cipherView = new CipherView(sdkTestCipher); - const mockSdkCipherView = cipherView.toSdkCipherView(); - - // Mock SDK admin create to return a cipher view - mockAdminSdk.create.mockResolvedValue(mockSdkCipherView); - - const apiSpy = jest.spyOn(apiService, "postCipherAdmin"); - const result = await cipherService.createWithServer(cipherView, userId, true); - - expect(apiSpy).not.toHaveBeenCalled(); - expect(sdkService.userClient$).toHaveBeenCalledWith(userId); - expect(mockCiphersSdk.admin).toHaveBeenCalled(); - expect(mockAdminSdk.create).toHaveBeenCalledWith( - expect.objectContaining({ - name: cipherView.name, - }), - ); - expect(result).toBeInstanceOf(CipherView); - expect(result.name).toBe(cipherView.name); }); }); describe("updateWithServer()", () => { - let mockSdkClient: any; - let mockCiphersSdk: any; - let mockAdminSdk: any; - let mockVaultSdk: any; - beforeEach(() => { - // Mock encrypt to return encryptionContext for legacy path jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); - // Mock decrypt to return cipherView for result jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { return new CipherView(cipher); }); - // Mock upsert to return the cipher data jest.spyOn(cipherService, "upsert").mockResolvedValue({ [cipherData.id as CipherId]: cipherData, }); - - // Mock the SDK client chain for admin operations - mockAdminSdk = { - edit: jest.fn(), - }; - mockCiphersSdk = { - edit: jest.fn(), - admin: jest.fn().mockReturnValue(mockAdminSdk), - }; - mockVaultSdk = { - ciphers: jest.fn().mockReturnValue(mockCiphersSdk), - }; - const mockSdkValue = { - vault: jest.fn().mockReturnValue(mockVaultSdk), - }; - mockSdkClient = { - take: jest.fn().mockReturnValue({ - value: mockSdkValue, - [Symbol.dispose]: jest.fn(), - }), - }; - - // Mock sdkService to return the mock client - sdkService.userClient$.mockReturnValue(of(mockSdkClient)); }); it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => { @@ -421,7 +319,6 @@ describe("Cipher Service", () => { .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) .mockResolvedValue(false); - // Create a fresh cipher with organizationId for this test const testCipher = new Cipher(cipherData); testCipher.organizationId = orgId; const testContext = { cipher: testCipher, encryptedFor: userId }; @@ -470,47 +367,47 @@ describe("Cipher Service", () => { expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj); }); - it("should use SDK to update cipher when feature flag is enabled", async () => { + it("should delegate to cipherSdkService when feature flag is enabled", async () => { configService.getFeatureFlag .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) .mockResolvedValue(true); const testCipher = new Cipher(cipherData); const cipherView = new CipherView(testCipher); - const mockSdkCipherView = cipherView.toSdkCipherView(); + const expectedResult = new CipherView(testCipher); - // Mock SDK edit to return a cipher view - mockCiphersSdk.edit.mockResolvedValue(mockSdkCipherView); + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "updateWithServer") + .mockResolvedValue(expectedResult); + + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); + const apiSpy = jest.spyOn(apiService, "putCipher"); const result = await cipherService.updateWithServer(cipherView, userId); - expect(sdkService.userClient$).toHaveBeenCalledWith(userId); - expect(mockCiphersSdk.edit).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.anything(), - name: cipherView.name, - }), - ); + expect(cipherSdkServiceSpy).toHaveBeenCalledWith(cipherView, userId, undefined, undefined); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); expect(result).toBeInstanceOf(CipherView); - expect(result.name).toBe(cipherView.name); }); - it("should use SDK admin API when orgAdmin is true", async () => { + it("should delegate to cipherSdkService with orgAdmin when feature flag is enabled", async () => { configService.getFeatureFlag .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) .mockResolvedValue(true); - // sdkTestCipherData already has a valid organizationId, use it directly const testCipher = new Cipher(cipherData); const cipherView = new CipherView(testCipher); const originalCipherView = new CipherView(testCipher); - const mockSdkCipherView = cipherView.toSdkCipherView(); + const expectedResult = new CipherView(testCipher); + const cipherSdkServiceSpy = jest + .spyOn(cipherSdkService, "updateWithServer") + .mockResolvedValue(expectedResult); + + const clearCacheSpy = jest.spyOn(cipherService, "clearCache"); const apiSpy = jest.spyOn(apiService, "putCipherAdmin"); - // Mock SDK admin edit to return a cipher view - mockAdminSdk.edit.mockResolvedValue(mockSdkCipherView); - const result = await cipherService.updateWithServer( cipherView, userId, @@ -518,16 +415,14 @@ describe("Cipher Service", () => { true, ); - expect(apiSpy).not.toHaveBeenCalled(); - expect(sdkService.userClient$).toHaveBeenCalledWith(userId); - expect(mockCiphersSdk.admin).toHaveBeenCalled(); - expect(mockAdminSdk.edit).toHaveBeenCalledWith( - expect.objectContaining({ - id: expect.anything(), - name: cipherView.name, - }), - originalCipherView.toSdkCipherView(), + expect(cipherSdkServiceSpy).toHaveBeenCalledWith( + cipherView, + userId, + originalCipherView, + true, ); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); expect(result).toBeInstanceOf(CipherView); }); }); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index b31ad648b98..53d7666e304 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1,9 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { - catchError, combineLatest, - EMPTY, filter, firstValueFrom, map, @@ -19,7 +17,7 @@ import { MessageSender } from "@bitwarden/common/platform/messaging"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; -import { CipherListView, CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; +import { CipherListView } from "@bitwarden/sdk-internal"; import { ApiService } from "../../abstractions/api.service"; import { AccountService } from "../../auth/abstractions/account.service"; @@ -34,7 +32,7 @@ import { ListResponse } from "../../models/response/list.response"; import { View } from "../../models/view/view"; import { ConfigService } from "../../platform/abstractions/config/config.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; -import { SdkService, uuidAsString } from "../../platform/abstractions/sdk/sdk.service"; +import { uuidAsString } from "../../platform/abstractions/sdk/sdk.service"; import { Utils } from "../../platform/misc/utils"; import Domain from "../../platform/models/domain/domain-base"; import { EncArrayBuffer } from "../../platform/models/domain/enc-array-buffer"; @@ -44,6 +42,7 @@ import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid import { OrgKey, UserKey } from "../../types/key"; import { filterOutNullish, perUserCache$ } from "../../vault/utils/observable-utilities"; import { CipherEncryptionService } from "../abstractions/cipher-encryption.service"; +import { CipherSdkService } from "../abstractions/cipher-sdk.service"; import { CipherService as CipherServiceAbstraction, EncryptionContext, @@ -122,7 +121,7 @@ export class CipherService implements CipherServiceAbstraction { private logService: LogService, private cipherEncryptionService: CipherEncryptionService, private messageSender: MessageSender, - private sdkService: SdkService, + private cipherSdkService: CipherSdkService, ) {} localData$(userId: UserId): Observable> { @@ -930,30 +929,11 @@ export class CipherService implements CipherServiceAbstraction { userId: UserId, orgAdmin?: boolean, ): Promise { - const resultCipherView = await firstValueFrom( - this.sdkService.userClient$(userId).pipe( - switchMap(async (sdk) => { - if (!sdk) { - throw new Error("SDK not available"); - } - using ref = sdk.take(); - const sdkCreateRequest = cipherView.toSdkCreateCipherRequest(); - let result: SdkCipherView; - if (orgAdmin) { - result = await ref.value.vault().ciphers().admin().create(sdkCreateRequest); - } else { - result = await ref.value.vault().ciphers().create(sdkCreateRequest); - } - return CipherView.fromSdkCipherView(result); - }), - catchError((error: unknown) => { - this.logService.error(`Failed to create cipher: ${error}`); - return EMPTY; - }), - ), + const resultCipherView = await this.cipherSdkService.createWithServer( + cipherView, + userId, + orgAdmin, ); - // The SDK manages the cache of encrypted ciphers, but doesn't update the cache of decrypted ciphers. - // Calling `this.clearCache` forces the decrypted cache to refresh. await this.clearCache(userId); return resultCipherView; } @@ -1010,34 +990,12 @@ export class CipherService implements CipherServiceAbstraction { originalCipherView?: CipherView, orgAdmin?: boolean, ): Promise { - const resultCipherView = await firstValueFrom( - this.sdkService.userClient$(userId).pipe( - switchMap(async (sdk) => { - if (!sdk) { - throw new Error("SDK not available"); - } - using ref = sdk.take(); - const sdkUpdateRequest = cipher.toSdkUpdateCipherRequest(); - let result: SdkCipherView; - if (orgAdmin) { - result = await ref.value - .vault() - .ciphers() - .admin() - .edit(sdkUpdateRequest, originalCipherView?.toSdkCipherView()); - } else { - result = await ref.value.vault().ciphers().edit(sdkUpdateRequest); - } - return CipherView.fromSdkCipherView(result); - }), - catchError((error: unknown) => { - this.logService.error(`Failed to update cipher: ${error}`); - return EMPTY; - }), - ), + const resultCipherView = await this.cipherSdkService.updateWithServer( + cipher, + userId, + originalCipherView, + orgAdmin, ); - // The SDK manages the cache of encrypted ciphers, but doesn't update the cache of decrypted ciphers. - // Calling `this.clearCache` forces the decrypted cache to refresh. await this.clearCache(userId); return resultCipherView; }