diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 81d79df569c..6a5f6774474 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -142,7 +142,7 @@ jobs: run: cargo +nightly udeps --workspace --all-features --all-targets - name: Install cargo-deny - uses: taiki-e/install-action@2e9d707ef49c9b094d45955b60c7e5c0dfedeb14 # v2.66.5 + uses: taiki-e/install-action@542cebaaed782771e619bd5609d97659d109c492 # v2.66.7 with: tool: cargo-deny@0.18.6 diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 61085828cf2..8e2c3279687 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -5001,6 +5001,9 @@ } } }, + "downloadAttachmentLabel": { + "message": "Download Attachment" + }, "downloadBitwarden": { "message": "Download Bitwarden" }, diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index ab16788ea6f..a927c75dba0 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -767,7 +767,6 @@ describe("NotificationBackground", () => { let createWithServerSpy: jest.SpyInstance; let updateWithServerSpy: jest.SpyInstance; let folderExistsSpy: jest.SpyInstance; - let cipherEncryptSpy: jest.SpyInstance; beforeEach(() => { activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); @@ -791,7 +790,6 @@ describe("NotificationBackground", () => { createWithServerSpy = jest.spyOn(cipherService, "createWithServer"); updateWithServerSpy = jest.spyOn(cipherService, "updateWithServer"); folderExistsSpy = jest.spyOn(notificationBackground as any, "folderExists"); - cipherEncryptSpy = jest.spyOn(cipherService, "encrypt"); accountService.activeAccount$ = activeAccountSubject; }); @@ -1190,13 +1188,7 @@ describe("NotificationBackground", () => { folderExistsSpy.mockResolvedValueOnce(false); convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView); editItemSpy.mockResolvedValueOnce(undefined); - cipherEncryptSpy.mockResolvedValueOnce({ - cipher: { - ...cipherView, - id: "testId", - }, - encryptedFor: userId, - }); + createWithServerSpy.mockResolvedValueOnce(cipherView); sendMockExtensionMessage(message, sender); await flushPromises(); @@ -1205,7 +1197,6 @@ describe("NotificationBackground", () => { queueMessage, null, ); - expect(cipherEncryptSpy).toHaveBeenCalledWith(cipherView, "testId"); expect(createWithServerSpy).toHaveBeenCalled(); expect(tabSendMessageDataSpy).toHaveBeenCalledWith( sender.tab, @@ -1241,13 +1232,6 @@ describe("NotificationBackground", () => { folderExistsSpy.mockResolvedValueOnce(true); convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView); editItemSpy.mockResolvedValueOnce(undefined); - cipherEncryptSpy.mockResolvedValueOnce({ - cipher: { - ...cipherView, - id: "testId", - }, - encryptedFor: userId, - }); const errorMessage = "fetch error"; createWithServerSpy.mockImplementation(() => { throw new Error(errorMessage); @@ -1256,7 +1240,6 @@ describe("NotificationBackground", () => { sendMockExtensionMessage(message, sender); await flushPromises(); - expect(cipherEncryptSpy).toHaveBeenCalledWith(cipherView, "testId"); expect(createWithServerSpy).toThrow(errorMessage); expect(tabSendMessageSpy).not.toHaveBeenCalledWith(sender.tab, { command: "addedCipher", diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 1cbf915b06a..f8459cf8f23 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -866,13 +866,11 @@ export default class NotificationBackground { return; } - const encrypted = await this.cipherService.encrypt(newCipher, activeUserId); - const { cipher } = encrypted; try { - await this.cipherService.createWithServer(encrypted); + const resultCipher = await this.cipherService.createWithServer(newCipher, activeUserId); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { itemName: newCipher?.name && String(newCipher?.name), - cipherId: cipher?.id && String(cipher?.id), + cipherId: resultCipher?.id && String(resultCipher?.id), }); await BrowserApi.tabSendMessage(tab, { command: "addedCipher" }); } catch (error) { @@ -910,7 +908,6 @@ export default class NotificationBackground { await BrowserApi.tabSendMessage(tab, { command: "editedCipher" }); return; } - const cipher = await this.cipherService.encrypt(cipherView, userId); try { if (!cipherView.edit) { @@ -939,7 +936,7 @@ export default class NotificationBackground { return; } - await this.cipherService.updateWithServer(cipher); + await this.cipherService.updateWithServer(cipherView, userId); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { itemName: cipherView?.name && String(cipherView?.name), diff --git a/apps/browser/src/autofill/popup/fido2/fido2.component.ts b/apps/browser/src/autofill/popup/fido2/fido2.component.ts index c1982d27d24..5720419f909 100644 --- a/apps/browser/src/autofill/popup/fido2/fido2.component.ts +++ b/apps/browser/src/autofill/popup/fido2/fido2.component.ts @@ -444,10 +444,9 @@ export class Fido2Component implements OnInit, OnDestroy { ); this.buildCipher(name, username); - const encrypted = await this.cipherService.encrypt(this.cipher, activeUserId); try { - await this.cipherService.createWithServer(encrypted); - this.cipher.id = encrypted.cipher.id; + const result = await this.cipherService.createWithServer(this.cipher, activeUserId); + this.cipher.id = result.id; } catch (e) { this.logService.error(e); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 58a7eb99ec6..660fcb97bcf 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -194,6 +194,7 @@ import { SendService } from "@bitwarden/common/tools/send/services/send.service" import { InternalSendService as InternalSendServiceAbstraction } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.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"; @@ -211,6 +212,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 { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service"; @@ -367,6 +369,7 @@ export default class MainBackground { apiService: ApiServiceAbstraction; hibpApiService: HibpApiService; environmentService: BrowserEnvironmentService; + cipherSdkService: CipherSdkService; cipherService: CipherServiceAbstraction; folderService: InternalFolderServiceAbstraction; userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction; @@ -973,6 +976,8 @@ export default class MainBackground { this.logService, ); + this.cipherSdkService = new DefaultCipherSdkService(this.sdkService, this.logService); + this.cipherService = new CipherService( this.keyService, this.domainSettingsService, @@ -988,6 +993,7 @@ export default class MainBackground { this.logService, this.cipherEncryptionService, this.messagingService, + this.cipherSdkService, ); this.folderService = new FolderService( this.keyService, diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.html b/apps/browser/src/tools/popup/send-v2/send-v2.component.html index 47ecd7564dc..48295fda35d 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.html +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.html @@ -1,4 +1,4 @@ - + diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts index dfbfabf8d5e..dc4b935c6c8 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts @@ -11,7 +11,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -110,7 +109,6 @@ describe("SendV2Component", () => { provide: BillingAccountProfileStateService, useValue: { hasPremiumFromAnySource$: of(false) }, }, - { provide: ConfigService, useValue: mock() }, { provide: EnvironmentService, useValue: mock() }, { provide: LogService, useValue: mock() }, { provide: PlatformUtilsService, useValue: mock() }, diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.ts b/apps/browser/src/tools/popup/send-v2/send-v2.component.ts index f36a475a805..8c1edee79dc 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.ts +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.ts @@ -11,8 +11,6 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; @@ -84,30 +82,17 @@ export class SendV2Component implements OnDestroy { protected listState: SendState | null = null; protected sends$ = this.sendItemsService.filteredAndSortedSends$; - private skeletonFeatureFlag$ = this.configService.getFeatureFlag$( - FeatureFlag.VaultLoadingSkeletons, - ); protected sendsLoading$ = this.sendItemsService.loading$.pipe( distinctUntilChanged(), shareReplay({ bufferSize: 1, refCount: true }), ); - /** Spinner Loading State */ - protected showSpinnerLoaders$ = combineLatest([ - this.sendsLoading$, - this.skeletonFeatureFlag$, - ]).pipe(map(([loading, skeletonsEnabled]) => loading && !skeletonsEnabled)); - /** Skeleton Loading State */ protected showSkeletonsLoaders$ = combineLatest([ this.sendsLoading$, this.searchService.isSendSearching$, - this.skeletonFeatureFlag$, ]).pipe( - map( - ([loading, cipherSearching, skeletonsEnabled]) => - (loading || cipherSearching) && skeletonsEnabled, - ), + map(([loading, cipherSearching]) => loading || cipherSearching), distinctUntilChanged(), skeletonLoadingDelay(), ); @@ -128,7 +113,6 @@ export class SendV2Component implements OnDestroy { protected sendListFiltersService: SendListFiltersService, private policyService: PolicyService, private accountService: AccountService, - private configService: ConfigService, private searchService: SearchService, ) { combineLatest([ diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index f881b07282b..d7de51ad20f 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -277,8 +277,7 @@ export class ItemMoreOptionsComponent { this.accountService.activeAccount$.pipe(map((a) => a?.id)), )) as UserId; - const encryptedCipher = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(encryptedCipher); + await this.cipherService.updateWithServer(cipher, activeUserId); this.toastService.showToast({ variant: "success", message: this.i18nService.t( diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts index 37c4804e600..ca73a7332ee 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts @@ -4,7 +4,6 @@ import { FormsModule } from "@angular/forms"; import { BehaviorSubject } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SearchTextDebounceInterval } from "@bitwarden/common/vault/services/search.service"; import { SearchModule } from "@bitwarden/components"; @@ -20,7 +19,6 @@ describe("VaultV2SearchComponent", () => { const searchText$ = new BehaviorSubject(""); const loading$ = new BehaviorSubject(false); - const featureFlag$ = new BehaviorSubject(true); const applyFilter = jest.fn(); const createComponent = () => { @@ -31,7 +29,6 @@ describe("VaultV2SearchComponent", () => { beforeEach(async () => { applyFilter.mockClear(); - featureFlag$.next(true); await TestBed.configureTestingModule({ imports: [VaultV2SearchComponent, CommonModule, SearchModule, JslibModule, FormsModule], @@ -49,12 +46,6 @@ describe("VaultV2SearchComponent", () => { loading$, }, }, - { - provide: ConfigService, - useValue: { - getFeatureFlag$: jest.fn(() => featureFlag$), - }, - }, { provide: I18nService, useValue: { t: (key: string) => key } }, ], }).compileComponents(); @@ -70,91 +61,55 @@ describe("VaultV2SearchComponent", () => { }); describe("debouncing behavior", () => { - describe("when feature flag is enabled", () => { - beforeEach(() => { - featureFlag$.next(true); - createComponent(); - }); - - it("debounces search text changes when not loading", fakeAsync(() => { - loading$.next(false); - - component.searchText = "test"; - component.onSearchTextChanged(); - - expect(applyFilter).not.toHaveBeenCalled(); - - tick(SearchTextDebounceInterval); - - expect(applyFilter).toHaveBeenCalledWith("test"); - expect(applyFilter).toHaveBeenCalledTimes(1); - })); - - it("should not debounce search text changes when loading", fakeAsync(() => { - loading$.next(true); - - component.searchText = "test"; - component.onSearchTextChanged(); - - tick(0); - - expect(applyFilter).toHaveBeenCalledWith("test"); - expect(applyFilter).toHaveBeenCalledTimes(1); - })); - - it("cancels previous debounce when new text is entered", fakeAsync(() => { - loading$.next(false); - - component.searchText = "test"; - component.onSearchTextChanged(); - - tick(SearchTextDebounceInterval / 2); - - component.searchText = "test2"; - component.onSearchTextChanged(); - - tick(SearchTextDebounceInterval / 2); - - expect(applyFilter).not.toHaveBeenCalled(); - - tick(SearchTextDebounceInterval / 2); - - expect(applyFilter).toHaveBeenCalledWith("test2"); - expect(applyFilter).toHaveBeenCalledTimes(1); - })); + beforeEach(() => { + createComponent(); }); - describe("when feature flag is disabled", () => { - beforeEach(() => { - featureFlag$.next(false); - createComponent(); - }); + it("debounces search text changes when not loading", fakeAsync(() => { + loading$.next(false); - it("debounces search text changes", fakeAsync(() => { - component.searchText = "test"; - component.onSearchTextChanged(); + component.searchText = "test"; + component.onSearchTextChanged(); - expect(applyFilter).not.toHaveBeenCalled(); + expect(applyFilter).not.toHaveBeenCalled(); - tick(SearchTextDebounceInterval); + tick(SearchTextDebounceInterval); - expect(applyFilter).toHaveBeenCalledWith("test"); - expect(applyFilter).toHaveBeenCalledTimes(1); - })); + expect(applyFilter).toHaveBeenCalledWith("test"); + expect(applyFilter).toHaveBeenCalledTimes(1); + })); - it("ignores loading state and always debounces", fakeAsync(() => { - loading$.next(true); + it("should not debounce search text changes when loading", fakeAsync(() => { + loading$.next(true); - component.searchText = "test"; - component.onSearchTextChanged(); + component.searchText = "test"; + component.onSearchTextChanged(); - expect(applyFilter).not.toHaveBeenCalled(); + tick(0); - tick(SearchTextDebounceInterval); + expect(applyFilter).toHaveBeenCalledWith("test"); + expect(applyFilter).toHaveBeenCalledTimes(1); + })); - expect(applyFilter).toHaveBeenCalledWith("test"); - expect(applyFilter).toHaveBeenCalledTimes(1); - })); - }); + it("cancels previous debounce when new text is entered", fakeAsync(() => { + loading$.next(false); + + component.searchText = "test"; + component.onSearchTextChanged(); + + tick(SearchTextDebounceInterval / 2); + + component.searchText = "test2"; + component.onSearchTextChanged(); + + tick(SearchTextDebounceInterval / 2); + + expect(applyFilter).not.toHaveBeenCalled(); + + tick(SearchTextDebounceInterval / 2); + + expect(applyFilter).toHaveBeenCalledWith("test2"); + expect(applyFilter).toHaveBeenCalledTimes(1); + })); }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts index 154cd49c5a3..3419bd30ea0 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts @@ -7,17 +7,13 @@ import { Subscription, combineLatest, debounce, - debounceTime, distinctUntilChanged, filter, map, - switchMap, timer, } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SearchTextDebounceInterval } from "@bitwarden/common/vault/services/search.service"; import { SearchModule } from "@bitwarden/components"; @@ -40,7 +36,6 @@ export class VaultV2SearchComponent { constructor( private vaultPopupItemsService: VaultPopupItemsService, private vaultPopupLoadingService: VaultPopupLoadingService, - private configService: ConfigService, private ngZone: NgZone, ) { this.subscribeToLatestSearchText(); @@ -63,31 +58,19 @@ export class VaultV2SearchComponent { } subscribeToApplyFilter(): void { - this.configService - .getFeatureFlag$(FeatureFlag.VaultLoadingSkeletons) + combineLatest([this.searchText$, this.loading$]) .pipe( - switchMap((enabled) => { - if (!enabled) { - return this.searchText$.pipe( - debounceTime(SearchTextDebounceInterval), - distinctUntilChanged(), - ); - } - - return combineLatest([this.searchText$, this.loading$]).pipe( - debounce(([_, isLoading]) => { - // If loading apply immediately to avoid stale searches. - // After loading completes, debounce to avoid excessive searches. - const delayTime = isLoading ? 0 : SearchTextDebounceInterval; - return timer(delayTime); - }), - distinctUntilChanged( - ([prevText, prevLoading], [newText, newLoading]) => - prevText === newText && prevLoading === newLoading, - ), - map(([text, _]) => text), - ); + debounce(([_, isLoading]) => { + // If loading apply immediately to avoid stale searches. + // After loading completes, debounce to avoid excessive searches. + const delayTime = isLoading ? 0 : SearchTextDebounceInterval; + return timer(delayTime); }), + distinctUntilChanged( + ([prevText, prevLoading], [newText, newLoading]) => + prevText === newText && prevLoading === newLoading, + ), + map(([text, _]) => text), takeUntilDestroyed(), ) .subscribe((text) => { diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html index 34454371f21..20871b4b134 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html @@ -1,4 +1,4 @@ - + @@ -8,37 +8,28 @@ - -
- - {{ "yourVaultIsEmpty" | i18n }} - -

- {{ "emptyVaultDescription" | i18n }} -

- - - {{ "newLogin" | i18n }} - - -
- - - @if (skeletonFeatureFlag$ | async) { - - + @if (vaultState === VaultStateEnum.Empty) { + +
+ + {{ "yourVaultIsEmpty" | i18n }} + +

+ {{ "emptyVaultDescription" | i18n }} +

+ + + {{ "newLogin" | i18n }} + + +
- } @else { - } - - - - - - - - - @if (skeletonFeatureFlag$ | async) { - - + @if (vaultState === null) { + + @if (!(loading$ | async)) { + + + + } - } @else { - } diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts index 2c94d9c226b..e3b72c3319f 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts @@ -1,6 +1,7 @@ import { ChangeDetectionStrategy, Component, input, NO_ERRORS_SCHEMA } from "@angular/core"; import { TestBed, fakeAsync, flush, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; +import { provideNoopAnimations } from "@angular/platform-browser/animations"; import { ActivatedRoute, Router } from "@angular/router"; import { RouterTestingModule } from "@angular/router/testing"; import { mock } from "jest-mock-extended"; @@ -243,6 +244,7 @@ describe("VaultV2Component", () => { await TestBed.configureTestingModule({ imports: [VaultV2Component, RouterTestingModule], providers: [ + provideNoopAnimations(), { provide: VaultPopupItemsService, useValue: itemsSvc }, { provide: VaultPopupListFiltersService, useValue: filtersSvc }, { provide: VaultPopupScrollPositionService, useValue: scrollSvc }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 4678e2733eb..c58b7b20d2f 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -158,10 +158,6 @@ export class VaultV2Component implements OnInit, OnDestroy { }), ); - protected skeletonFeatureFlag$ = this.configService.getFeatureFlag$( - FeatureFlag.VaultLoadingSkeletons, - ); - protected premiumSpotlightFeatureFlag$ = this.configService.getFeatureFlag$( FeatureFlag.BrowserPremiumSpotlight, ); @@ -216,20 +212,14 @@ export class VaultV2Component implements OnInit, OnDestroy { PremiumUpgradeDialogComponent.open(this.dialogService); } - /** When true, show spinner loading state */ - protected showSpinnerLoaders$ = combineLatest([this.loading$, this.skeletonFeatureFlag$]).pipe( - map(([loading, skeletonsEnabled]) => loading && !skeletonsEnabled), - ); - /** When true, show skeleton loading state with debouncing to prevent flicker */ protected showSkeletonsLoaders$ = combineLatest([ this.loading$, this.searchService.isCipherSearching$, this.vaultItemsTransferService.transferInProgress$, - this.skeletonFeatureFlag$, ]).pipe( - map(([loading, cipherSearching, transferInProgress, skeletonsEnabled]) => { - return (loading || cipherSearching || transferInProgress) && skeletonsEnabled; + map(([loading, cipherSearching, transferInProgress]) => { + return loading || cipherSearching || transferInProgress; }), distinctUntilChanged(), skeletonLoadingDelay(), diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts index 5818c6e32ff..94542009a89 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts @@ -378,8 +378,7 @@ describe("VaultPopupAutofillService", () => { expect(result).toBe(true); expect(mockCipher.login.uris).toHaveLength(1); expect(mockCipher.login.uris[0].uri).toBe(mockCurrentTab.url); - expect(mockCipherService.encrypt).toHaveBeenCalledWith(mockCipher, mockUserId); - expect(mockCipherService.updateWithServer).toHaveBeenCalledWith(mockEncryptedCipher); + expect(mockCipherService.updateWithServer).toHaveBeenCalledWith(mockCipher, mockUserId); }); it("should add a URI to the cipher when there are no existing URIs", async () => { diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts index 6feeec29efc..025088e029e 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.ts @@ -426,8 +426,7 @@ export class VaultPopupAutofillService { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); - const encCipher = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(encCipher); + await this.cipherService.updateWithServer(cipher, activeUserId); this.messagingService.send("editedCipher"); return true; } catch { diff --git a/apps/cli/package.json b/apps/cli/package.json index a19c811b4bf..c80f79feff8 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -81,7 +81,7 @@ "lowdb": "1.0.0", "lunr": "2.3.9", "multer": "2.0.2", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "open": "11.0.0", "papaparse": "5.5.3", diff --git a/apps/cli/src/commands/edit.command.ts b/apps/cli/src/commands/edit.command.ts index d95e8333dca..dbcb0489187 100644 --- a/apps/cli/src/commands/edit.command.ts +++ b/apps/cli/src/commands/edit.command.ts @@ -138,10 +138,8 @@ export class EditCommand { ); } - const encCipher = await this.cipherService.encrypt(cipherView, activeUserId); try { - const updatedCipher = await this.cipherService.updateWithServer(encCipher); - const decCipher = await this.cipherService.decrypt(updatedCipher, activeUserId); + const decCipher = await this.cipherService.updateWithServer(cipherView, activeUserId); const res = new CipherResponse(decCipher); return Response.success(res); } catch (e) { diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index bc3d3153b13..7bb8da27040 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -147,11 +147,13 @@ import { SendService } from "@bitwarden/common/tools/send/services/send.service" import { UserId } from "@bitwarden/common/types/guid"; import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; +import { CipherSdkService } from "@bitwarden/common/vault/abstractions/cipher-sdk.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; 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"; @@ -254,6 +256,7 @@ export class ServiceContainer { twoFactorApiService: TwoFactorApiService; hibpApiService: HibpApiService; environmentService: EnvironmentService; + cipherSdkService: CipherSdkService; cipherService: CipherService; folderService: InternalFolderService; organizationUserApiService: OrganizationUserApiService; @@ -794,6 +797,8 @@ export class ServiceContainer { this.logService, ); + this.cipherSdkService = new DefaultCipherSdkService(this.sdkService, this.logService); + this.cipherService = new CipherService( this.keyService, this.domainSettingsService, @@ -809,6 +814,7 @@ export class ServiceContainer { this.logService, this.cipherEncryptionService, this.messagingService, + this.cipherSdkService, ); this.cipherArchiveService = new DefaultCipherArchiveService( diff --git a/apps/cli/src/vault/create.command.ts b/apps/cli/src/vault/create.command.ts index d826766dc65..e1a91966afd 100644 --- a/apps/cli/src/vault/create.command.ts +++ b/apps/cli/src/vault/create.command.ts @@ -103,10 +103,11 @@ export class CreateCommand { return Response.error("Creating this item type is restricted by organizational policy."); } - const cipher = await this.cipherService.encrypt(CipherExport.toView(req), activeUserId); - const newCipher = await this.cipherService.createWithServer(cipher); - const decCipher = await this.cipherService.decrypt(newCipher, activeUserId); - const res = new CipherResponse(decCipher); + const newCipher = await this.cipherService.createWithServer( + CipherExport.toView(req), + activeUserId, + ); + const res = new CipherResponse(newCipher); return Response.success(res); } catch (e) { return Response.error(e); diff --git a/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts b/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts index cf29370840d..432448faba3 100644 --- a/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts +++ b/apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts @@ -299,12 +299,11 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi throw new Error("No active user ID found!"); } - const encCipher = await this.cipherService.encrypt(cipher, activeUserId); - try { - const createdCipher = await this.cipherService.createWithServer(encCipher); + const createdCipher = await this.cipherService.createWithServer(cipher, activeUserId); + const encryptedCreatedCipher = await this.cipherService.encrypt(createdCipher, activeUserId); - return createdCipher; + return encryptedCreatedCipher.cipher; } catch { throw new Error("Unable to create cipher"); } @@ -316,8 +315,7 @@ export class DesktopFido2UserInterfaceSession implements Fido2UserInterfaceSessi this.accountService.activeAccount$.pipe( map(async (a) => { if (a) { - const encCipher = await this.cipherService.encrypt(cipher, a.id); - await this.cipherService.updateWithServer(encCipher); + await this.cipherService.updateWithServer(cipher, a.id); } }), ), diff --git a/apps/desktop/src/services/encrypted-message-handler.service.ts b/apps/desktop/src/services/encrypted-message-handler.service.ts index 366a144c021..ccbc7c539d0 100644 --- a/apps/desktop/src/services/encrypted-message-handler.service.ts +++ b/apps/desktop/src/services/encrypted-message-handler.service.ts @@ -166,8 +166,7 @@ export class EncryptedMessageHandlerService { try { const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - const encrypted = await this.cipherService.encrypt(cipherView, activeUserId); - await this.cipherService.createWithServer(encrypted); + await this.cipherService.createWithServer(cipherView, activeUserId); // Notify other clients of new login await this.messagingService.send("addedCipher"); @@ -212,9 +211,8 @@ export class EncryptedMessageHandlerService { cipherView.login.password = credentialUpdatePayload.password; cipherView.login.username = credentialUpdatePayload.userName; cipherView.login.uris[0].uri = credentialUpdatePayload.uri; - const encrypted = await this.cipherService.encrypt(cipherView, activeUserId); - await this.cipherService.updateWithServer(encrypted); + await this.cipherService.updateWithServer(cipherView, activeUserId); // Notify other clients of update await this.messagingService.send("editedCipher"); diff --git a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts index d8519b86094..f775ed84ede 100644 --- a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts @@ -193,6 +193,7 @@ export abstract class CipherReportComponent implements OnDestroy { formConfig, activeCollectionId, disableForm, + isAdminConsoleAction: true, }); const result = await lastValueFrom(this.vaultItemDialogRef.closed); diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html index 059347709f0..ec06c740f24 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html @@ -3,7 +3,7 @@ {{ title }} - @if (isCipherArchived) { + @if (isCipherArchived && !params.isAdminConsoleAction) { {{ "archived" | i18n }} } diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts index 63b5071d1f5..9a048b7a8b3 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts @@ -303,6 +303,25 @@ describe("VaultItemDialogComponent", () => { }); }); + describe("archive badge", () => { + it('should show "archived" badge when the item is archived and not an admin console action', () => { + component.setTestCipher({ isArchived: true }); + component.setTestParams({ mode: "view" }); + fixture.detectChanges(); + const archivedBadge = fixture.debugElement.query(By.css("span[bitBadge]")); + expect(archivedBadge).toBeTruthy(); + expect(archivedBadge.nativeElement.textContent.trim()).toBe("archived"); + }); + + it('should not show "archived" badge when the item is archived and is an admin console action', () => { + component.setTestCipher({ isArchived: true }); + component.setTestParams({ mode: "view", isAdminConsoleAction: true }); + fixture.detectChanges(); + const archivedBadge = fixture.debugElement.query(By.css("span[bitBadge]")); + expect(archivedBadge).toBeFalsy(); + }); + }); + describe("submitButtonText$", () => { it("should return 'unArchiveAndSave' when premium is false and cipher is archived", (done) => { jest.spyOn(component as any, "userHasPremium$", "get").mockReturnValue(of(false)); diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 5ca3a11d5ab..532757852a3 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -1536,8 +1536,7 @@ export class VaultComponent implements OnInit, OnDestr const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const cipherFullView = await this.cipherService.getFullCipherView(cipher); cipherFullView.favorite = !cipherFullView.favorite; - const encryptedCipher = await this.cipherService.encrypt(cipherFullView, activeUserId); - await this.cipherService.updateWithServer(encryptedCipher); + await this.cipherService.updateWithServer(cipherFullView, activeUserId); this.toastService.showToast({ variant: "success", diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 7b504548ff5..1ecf7fe3e3d 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,6 +614,7 @@ const safeProviders: SafeProvider[] = [ logService: LogService, cipherEncryptionService: CipherEncryptionService, messagingService: MessagingServiceAbstraction, + cipherSdkService: CipherSdkService, ) => new CipherService( keyService, @@ -623,6 +631,7 @@ const safeProviders: SafeProvider[] = [ logService, cipherEncryptionService, messagingService, + cipherSdkService, ), deps: [ KeyService, @@ -639,6 +648,7 @@ const safeProviders: SafeProvider[] = [ LogService, CipherEncryptionService, MessagingServiceAbstraction, + CipherSdkService, ], }), safeProvider({ diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 86fd0927f28..ba6927c39b9 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -64,10 +64,9 @@ export enum FeatureFlag { PM22134SdkCipherListView = "pm-22134-sdk-cipher-list-view", PM22136_SdkCipherEncryption = "pm-22136-sdk-cipher-encryption", CipherKeyEncryption = "cipher-key-encryption", - RiskInsightsForPremium = "pm-23904-risk-insights-for-premium", - VaultLoadingSkeletons = "pm-25081-vault-skeleton-loaders", BrowserPremiumSpotlight = "pm-23384-browser-premium-spotlight", MigrateMyVaultToMyItems = "pm-20558-migrate-myvault-to-myitems", + PM27632_SdkCipherCrudOperations = "pm-27632-cipher-crud-operations-to-sdk", /* Platform */ IpcChannelFramework = "ipc-channel-framework", @@ -127,9 +126,8 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM19941MigrateCipherDomainToSdk]: FALSE, [FeatureFlag.PM22134SdkCipherListView]: FALSE, [FeatureFlag.PM22136_SdkCipherEncryption]: FALSE, - [FeatureFlag.RiskInsightsForPremium]: FALSE, - [FeatureFlag.VaultLoadingSkeletons]: FALSE, [FeatureFlag.BrowserPremiumSpotlight]: FALSE, + [FeatureFlag.PM27632_SdkCipherCrudOperations]: FALSE, [FeatureFlag.MigrateMyVaultToMyItems]: FALSE, /* Auth */ diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts index 9c50bd1ab65..6223e4274bf 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts @@ -254,17 +254,17 @@ describe("FidoAuthenticatorService", () => { } it("should save credential to vault if request confirmed by user", async () => { - const encryptedCipher = Symbol(); userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: existingCipher.id, userVerified: false, }); - cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as EncryptionContext); await authenticator.makeCredential(params, windowReference); - const saved = cipherService.encrypt.mock.lastCall?.[0]; - expect(saved).toEqual( + const savedCipher = cipherService.updateWithServer.mock.lastCall?.[0]; + const actualUserId = cipherService.updateWithServer.mock.lastCall?.[1]; + expect(actualUserId).toEqual(userId); + expect(savedCipher).toEqual( expect.objectContaining({ type: CipherType.Login, name: existingCipher.name, @@ -288,7 +288,6 @@ describe("FidoAuthenticatorService", () => { }), }), ); - expect(cipherService.updateWithServer).toHaveBeenCalledWith(encryptedCipher); }); /** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */ @@ -361,17 +360,14 @@ describe("FidoAuthenticatorService", () => { cipherService.getAllDecrypted.mockResolvedValue([await cipher]); cipherService.decrypt.mockResolvedValue(cipher); - cipherService.encrypt.mockImplementation(async (cipher) => { - cipher.login.fido2Credentials[0].credentialId = credentialId; // Replace id for testability - return { cipher: {} as any as Cipher, encryptedFor: userId }; - }); - cipherService.createWithServer.mockImplementation(async ({ cipher }) => { - cipher.id = cipherId; + cipherService.createWithServer.mockImplementation(async (cipherView, _userId) => { + cipherView.id = cipherId; return cipher; }); - cipherService.updateWithServer.mockImplementation(async ({ cipher }) => { - cipher.id = cipherId; - return cipher; + cipherService.updateWithServer.mockImplementation(async (cipherView, _userId) => { + cipherView.id = cipherId; + cipherView.login.fido2Credentials[0].credentialId = credentialId; // Replace id for testability + return cipherView; }); }); @@ -701,14 +697,11 @@ describe("FidoAuthenticatorService", () => { /** Spec: Increment the credential associated signature counter */ it("should increment counter and save to server when stored counter is larger than zero", async () => { - const encrypted = Symbol(); - cipherService.encrypt.mockResolvedValue(encrypted as any); ciphers[0].login.fido2Credentials[0].counter = 9000; await authenticator.getAssertion(params, windowReference); - expect(cipherService.updateWithServer).toHaveBeenCalledWith(encrypted); - expect(cipherService.encrypt).toHaveBeenCalledWith( + expect(cipherService.updateWithServer).toHaveBeenCalledWith( expect.objectContaining({ id: ciphers[0].id, login: expect.objectContaining({ @@ -725,8 +718,6 @@ describe("FidoAuthenticatorService", () => { /** Spec: Authenticators that do not implement a signature counter leave the signCount in the authenticator data constant at zero. */ it("should not save to server when stored counter is zero", async () => { - const encrypted = Symbol(); - cipherService.encrypt.mockResolvedValue(encrypted as any); ciphers[0].login.fido2Credentials[0].counter = 0; await authenticator.getAssertion(params, windowReference); diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts index d1081e9f7b2..1b150207290 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts @@ -187,8 +187,7 @@ export class Fido2AuthenticatorService< if (Utils.isNullOrEmpty(cipher.login.username)) { cipher.login.username = fido2Credential.userName; } - const reencrypted = await this.cipherService.encrypt(cipher, activeUserId); - await this.cipherService.updateWithServer(reencrypted); + await this.cipherService.updateWithServer(cipher, activeUserId); await this.cipherService.clearCache(activeUserId); credentialId = fido2Credential.credentialId; } catch (error) { @@ -328,8 +327,7 @@ export class Fido2AuthenticatorService< const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ); - const encrypted = await this.cipherService.encrypt(selectedCipher, activeUserId); - await this.cipherService.updateWithServer(encrypted); + await this.cipherService.updateWithServer(selectedCipher, activeUserId); await this.cipherService.clearCache(activeUserId); } diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.ts b/libs/common/src/platform/services/sdk/default-sdk.service.ts index 5084f5f5f18..e2c9c77e204 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.ts @@ -80,7 +80,7 @@ export class DefaultSdkService implements SdkService { client$ = this.environmentService.environment$.pipe( concatMap(async (env) => { await SdkLoadService.Ready; - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService), settings, @@ -210,7 +210,7 @@ export class DefaultSdkService implements SdkService { return undefined; } - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService, userId), settings, @@ -322,11 +322,12 @@ export class DefaultSdkService implements SdkService { client.platform().load_flags(featureFlagMap); } - private toSettings(env: Environment): ClientSettings { + private async toSettings(env: Environment): Promise { return { apiUrl: env.getApiUrl(), identityUrl: env.getIdentityUrl(), deviceType: toSdkDevice(this.platformUtilsService.getDevice()), + bitwardenClientVersion: await this.platformUtilsService.getApplicationVersionNumber(), userAgent: this.userAgent ?? navigator.userAgent, }; } diff --git a/libs/common/src/platform/services/sdk/register-sdk.service.ts b/libs/common/src/platform/services/sdk/register-sdk.service.ts index a222807640f..073c5c0560c 100644 --- a/libs/common/src/platform/services/sdk/register-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/register-sdk.service.ts @@ -62,7 +62,7 @@ export class DefaultRegisterSdkService implements RegisterSdkService { client$ = this.environmentService.environment$.pipe( concatMap(async (env) => { await SdkLoadService.Ready; - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService), settings, @@ -137,7 +137,7 @@ export class DefaultRegisterSdkService implements RegisterSdkService { return undefined; } - const settings = this.toSettings(env); + const settings = await this.toSettings(env); const client = await this.sdkClientFactory.createSdkClient( new JsTokenProvider(this.apiService, userId), settings, @@ -185,12 +185,13 @@ export class DefaultRegisterSdkService implements RegisterSdkService { client.platform().load_flags(featureFlagMap); } - private toSettings(env: Environment): ClientSettings { + private async toSettings(env: Environment): Promise { return { apiUrl: env.getApiUrl(), identityUrl: env.getIdentityUrl(), deviceType: toSdkDevice(this.platformUtilsService.getDevice()), userAgent: this.userAgent ?? navigator.userAgent, + bitwardenClientVersion: await this.platformUtilsService.getApplicationVersionNumber(), }; } } 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..1037bfc2b92 --- /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/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 203984075f7..1db5f8d38a7 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -119,9 +119,11 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + ): Promise; + /** * Update a cipher with the server * @param cipher The cipher to update @@ -131,10 +133,11 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + ): Promise; /** * Move a cipher to an organization by re-encrypting its keys with the organization's key. diff --git a/libs/common/src/vault/models/view/cipher.view.spec.ts b/libs/common/src/vault/models/view/cipher.view.spec.ts index 475fe9e23f3..1c7017d5d89 100644 --- a/libs/common/src/vault/models/view/cipher.view.spec.ts +++ b/libs/common/src/vault/models/view/cipher.view.spec.ts @@ -353,4 +353,366 @@ describe("CipherView", () => { }); }); }); + + // Note: These tests use jest.requireActual() because the file has jest.mock() calls + // at the top that mock LoginView, FieldView, etc. Those mocks are needed for other tests + // but interfere with these tests which need the real implementations. + describe("toSdkCreateCipherRequest", () => { + it("maps all properties correctly for a login cipher", () => { + const { FieldView: RealFieldView } = jest.requireActual("./field.view"); + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.organizationId = "000f2a6e-da5e-4726-87ed-1c5c77322c3c"; + cipherView.folderId = "41b22db4-8e2a-4ed2-b568-f1186c72922f"; + cipherView.collectionIds = ["b0473506-3c3c-4260-a734-dfaaf833ab6f"]; + cipherView.name = "Test Login"; + cipherView.notes = "Test notes"; + cipherView.type = CipherType.Login; + cipherView.favorite = true; + cipherView.reprompt = CipherRepromptType.Password; + + const field = new RealFieldView(); + field.name = "testField"; + field.value = "testValue"; + field.type = SdkFieldType.Text; + cipherView.fields = [field]; + + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + cipherView.login.password = "testpass"; + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.organizationId).toEqual(asUuid("000f2a6e-da5e-4726-87ed-1c5c77322c3c")); + expect(result.folderId).toEqual(asUuid("41b22db4-8e2a-4ed2-b568-f1186c72922f")); + expect(result.collectionIds).toEqual([asUuid("b0473506-3c3c-4260-a734-dfaaf833ab6f")]); + expect(result.name).toBe("Test Login"); + expect(result.notes).toBe("Test notes"); + expect(result.favorite).toBe(true); + expect(result.reprompt).toBe(CipherRepromptType.Password); + expect(result.fields).toHaveLength(1); + expect(result.fields![0]).toMatchObject({ + name: "testField", + value: "testValue", + type: SdkFieldType.Text, + }); + expect(result.type).toHaveProperty("login"); + expect((result.type as any).login).toMatchObject({ + username: "testuser", + password: "testpass", + }); + }); + + it("handles undefined organizationId and folderId", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.organizationId).toBeUndefined(); + expect(result.folderId).toBeUndefined(); + expect(result.name).toBe("Test Cipher"); + }); + + it("handles empty collectionIds array", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.collectionIds = []; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.collectionIds).toEqual([]); + }); + + it("defaults favorite to false when undefined", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.favorite = undefined as any; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.favorite).toBe(false); + }); + + it("defaults reprompt to None when undefined", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + cipherView.reprompt = undefined as any; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const result = cipherView.toSdkCreateCipherRequest(); + + expect(result.reprompt).toBe(CipherRepromptType.None); + }); + + test.each([ + ["Login", CipherType.Login, "login.view", "LoginView"], + ["Card", CipherType.Card, "card.view", "CardView"], + ["Identity", CipherType.Identity, "identity.view", "IdentityView"], + ["SecureNote", CipherType.SecureNote, "secure-note.view", "SecureNoteView"], + ["SshKey", CipherType.SshKey, "ssh-key.view", "SshKeyView"], + ])( + "creates correct type property for %s cipher", + (typeName: string, cipherType: CipherType, moduleName: string, className: string) => { + const module = jest.requireActual(`./${moduleName}`); + const ViewClass = module[className]; + + const cipherView = new CipherView(); + cipherView.name = `Test ${typeName}`; + cipherView.type = cipherType; + + // Set the appropriate view property + const viewPropertyName = typeName.charAt(0).toLowerCase() + typeName.slice(1); + (cipherView as any)[viewPropertyName] = new ViewClass(); + + const result = cipherView.toSdkCreateCipherRequest(); + + const typeKey = typeName.charAt(0).toLowerCase() + typeName.slice(1); + expect(result.type).toHaveProperty(typeKey); + }, + ); + }); + + describe("toSdkUpdateCipherRequest", () => { + it("maps all properties correctly for an update request", () => { + const { FieldView: RealFieldView } = jest.requireActual("./field.view"); + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.organizationId = "000f2a6e-da5e-4726-87ed-1c5c77322c3c"; + cipherView.folderId = "41b22db4-8e2a-4ed2-b568-f1186c72922f"; + cipherView.name = "Updated Login"; + cipherView.notes = "Updated notes"; + cipherView.type = CipherType.Login; + cipherView.favorite = true; + cipherView.reprompt = CipherRepromptType.Password; + cipherView.revisionDate = new Date("2022-01-02T12:00:00.000Z"); + cipherView.archivedDate = new Date("2022-01-03T12:00:00.000Z"); + cipherView.key = new EncString("cipher-key"); + + const mockField = new RealFieldView(); + mockField.name = "testField"; + mockField.value = "testValue"; + cipherView.fields = [mockField]; + + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.id).toEqual(asUuid("0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602")); + expect(result.organizationId).toEqual(asUuid("000f2a6e-da5e-4726-87ed-1c5c77322c3c")); + expect(result.folderId).toEqual(asUuid("41b22db4-8e2a-4ed2-b568-f1186c72922f")); + expect(result.name).toBe("Updated Login"); + expect(result.notes).toBe("Updated notes"); + expect(result.favorite).toBe(true); + expect(result.reprompt).toBe(CipherRepromptType.Password); + expect(result.revisionDate).toBe("2022-01-02T12:00:00.000Z"); + expect(result.archivedDate).toBe("2022-01-03T12:00:00.000Z"); + expect(result.fields).toHaveLength(1); + expect(result.fields![0]).toMatchObject({ + name: "testField", + value: "testValue", + }); + expect(result.type).toHaveProperty("login"); + expect((result.type as any).login).toMatchObject({ + username: "testuser", + }); + expect(result.key).toBeDefined(); + }); + + it("handles undefined optional properties", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + cipherView.revisionDate = new Date("2022-01-02T12:00:00.000Z"); + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.organizationId).toBeUndefined(); + expect(result.folderId).toBeUndefined(); + expect(result.archivedDate).toBeUndefined(); + expect(result.key).toBeUndefined(); + }); + + it("converts dates to ISO strings", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + cipherView.revisionDate = new Date("2022-05-15T10:30:00.000Z"); + cipherView.archivedDate = new Date("2022-06-20T14:45:00.000Z"); + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.revisionDate).toBe("2022-05-15T10:30:00.000Z"); + expect(result.archivedDate).toBe("2022-06-20T14:45:00.000Z"); + }); + + it("includes attachments when present", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + const { AttachmentView: RealAttachmentView } = jest.requireActual("./attachment.view"); + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = "Test Cipher"; + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + + const attachment1 = new RealAttachmentView(); + attachment1.id = "attachment-id-1"; + attachment1.fileName = "file1.txt"; + + const attachment2 = new RealAttachmentView(); + attachment2.id = "attachment-id-2"; + attachment2.fileName = "file2.pdf"; + + cipherView.attachments = [attachment1, attachment2]; + + const result = cipherView.toSdkUpdateCipherRequest(); + + expect(result.attachments).toHaveLength(2); + }); + + test.each([ + ["Login", CipherType.Login, "login.view", "LoginView"], + ["Card", CipherType.Card, "card.view", "CardView"], + ["Identity", CipherType.Identity, "identity.view", "IdentityView"], + ["SecureNote", CipherType.SecureNote, "secure-note.view", "SecureNoteView"], + ["SshKey", CipherType.SshKey, "ssh-key.view", "SshKeyView"], + ])( + "creates correct type property for %s cipher", + (typeName: string, cipherType: CipherType, moduleName: string, className: string) => { + const module = jest.requireActual(`./${moduleName}`); + const ViewClass = module[className]; + + const cipherView = new CipherView(); + cipherView.id = "0a54d80c-14aa-4ef8-8c3a-7ea99ce5b602"; + cipherView.name = `Test ${typeName}`; + cipherView.type = cipherType; + + // Set the appropriate view property + const viewPropertyName = typeName.charAt(0).toLowerCase() + typeName.slice(1); + (cipherView as any)[viewPropertyName] = new ViewClass(); + + const result = cipherView.toSdkUpdateCipherRequest(); + + const typeKey = typeName.charAt(0).toLowerCase() + typeName.slice(1); + expect(result.type).toHaveProperty(typeKey); + }, + ); + }); + + describe("getSdkCipherViewType", () => { + it("returns login type for Login cipher", () => { + const { LoginView: RealLoginView } = jest.requireActual("./login.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Login; + cipherView.login = new RealLoginView(); + cipherView.login.username = "testuser"; + cipherView.login.password = "testpass"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("login"); + expect((result as any).login).toMatchObject({ + username: "testuser", + password: "testpass", + }); + }); + + it("returns card type for Card cipher", () => { + const { CardView: RealCardView } = jest.requireActual("./card.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Card; + cipherView.card = new RealCardView(); + cipherView.card.cardholderName = "John Doe"; + cipherView.card.number = "4111111111111111"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("card"); + expect((result as any).card.cardholderName).toBe("John Doe"); + expect((result as any).card.number).toBe("4111111111111111"); + }); + + it("returns identity type for Identity cipher", () => { + const { IdentityView: RealIdentityView } = jest.requireActual("./identity.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.Identity; + cipherView.identity = new RealIdentityView(); + cipherView.identity.firstName = "John"; + cipherView.identity.lastName = "Doe"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("identity"); + expect((result as any).identity.firstName).toBe("John"); + expect((result as any).identity.lastName).toBe("Doe"); + }); + + it("returns secureNote type for SecureNote cipher", () => { + const { SecureNoteView: RealSecureNoteView } = jest.requireActual("./secure-note.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.SecureNote; + cipherView.secureNote = new RealSecureNoteView(); + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("secureNote"); + }); + + it("returns sshKey type for SshKey cipher", () => { + const { SshKeyView: RealSshKeyView } = jest.requireActual("./ssh-key.view"); + + const cipherView = new CipherView(); + cipherView.type = CipherType.SshKey; + cipherView.sshKey = new RealSshKeyView(); + cipherView.sshKey.privateKey = "privateKeyData"; + cipherView.sshKey.publicKey = "publicKeyData"; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("sshKey"); + expect((result as any).sshKey.privateKey).toBe("privateKeyData"); + expect((result as any).sshKey.publicKey).toBe("publicKeyData"); + }); + + it("defaults to empty login for unknown cipher type", () => { + const cipherView = new CipherView(); + cipherView.type = 999 as CipherType; + + const result = cipherView.getSdkCipherViewType(); + + expect(result).toHaveProperty("login"); + }); + }); }); diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 89f59665681..0909d0bda80 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -1,7 +1,12 @@ import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { asUuid, uuidAsString } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { ItemView } from "@bitwarden/common/vault/models/view/item.view"; -import { CipherView as SdkCipherView } from "@bitwarden/sdk-internal"; +import { + CipherCreateRequest, + CipherEditRequest, + CipherViewType, + CipherView as SdkCipherView, +} from "@bitwarden/sdk-internal"; import { View } from "../../../models/view/view"; import { InitializerMetadata } from "../../../platform/interfaces/initializer-metadata.interface"; @@ -332,6 +337,85 @@ export class CipherView implements View, InitializerMetadata { return cipherView; } + /** + * Maps CipherView to an SDK CipherCreateRequest + * + * @returns {CipherCreateRequest} The SDK cipher create request object + */ + toSdkCreateCipherRequest(): CipherCreateRequest { + const sdkCipherCreateRequest: CipherCreateRequest = { + organizationId: this.organizationId ? asUuid(this.organizationId) : undefined, + collectionIds: this.collectionIds ? this.collectionIds.map((i) => asUuid(i)) : [], + folderId: this.folderId ? asUuid(this.folderId) : undefined, + name: this.name ?? "", + notes: this.notes, + favorite: this.favorite ?? false, + reprompt: this.reprompt ?? CipherRepromptType.None, + fields: this.fields?.map((f) => f.toSdkFieldView()), + type: this.getSdkCipherViewType(), + }; + + return sdkCipherCreateRequest; + } + + /** + * Maps CipherView to an SDK CipherEditRequest + * + * @returns {CipherEditRequest} The SDK cipher edit request object + */ + toSdkUpdateCipherRequest(): CipherEditRequest { + const sdkCipherEditRequest: CipherEditRequest = { + id: asUuid(this.id), + organizationId: this.organizationId ? asUuid(this.organizationId) : undefined, + folderId: this.folderId ? asUuid(this.folderId) : undefined, + name: this.name ?? "", + notes: this.notes, + favorite: this.favorite ?? false, + reprompt: this.reprompt ?? CipherRepromptType.None, + fields: this.fields?.map((f) => f.toSdkFieldView()), + type: this.getSdkCipherViewType(), + revisionDate: this.revisionDate?.toISOString(), + archivedDate: this.archivedDate?.toISOString(), + attachments: this.attachments?.map((a) => a.toSdkAttachmentView()), + key: this.key?.toSdk(), + }; + + return sdkCipherEditRequest; + } + + /** + * Returns the SDK CipherViewType object for the cipher. + * + * @returns {CipherViewType} The SDK CipherViewType for the cipher.t + */ + getSdkCipherViewType(): CipherViewType { + let viewType: CipherViewType; + switch (this.type) { + case CipherType.Card: + viewType = { card: this.card?.toSdkCardView() }; + break; + case CipherType.Identity: + viewType = { identity: this.identity?.toSdkIdentityView() }; + break; + case CipherType.Login: + viewType = { login: this.login?.toSdkLoginView() }; + break; + case CipherType.SecureNote: + viewType = { secureNote: this.secureNote?.toSdkSecureNoteView() }; + break; + case CipherType.SshKey: + viewType = { sshKey: this.sshKey?.toSdkSshKeyView() }; + break; + default: + viewType = { + // Default to empty login - should not be valid code path. + login: new LoginView().toSdkLoginView(), + }; + break; + } + return viewType; + } + /** * Maps CipherView to SdkCipherView * 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..bd3feb4619e --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.spec.ts @@ -0,0 +1,246 @@ +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 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.createWithServer(cipherView, userId)).rejects.toThrow(); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to create cipher"), + ); + }); + + it("should throw error and log when SDK throws an error", async () => { + const cipherView = new CipherView(); + cipherView.name = "Test Cipher"; + + mockCiphersSdk.create.mockRejectedValue(new Error("SDK error")); + + await expect(cipherSdkService.createWithServer(cipherView, userId)).rejects.toThrow(); + 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, + }), + expect.anything(), // Empty CipherView - timestamps vary so we just verify it was called + ); + 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..06f5d3eb961 --- /dev/null +++ b/libs/common/src/vault/services/cipher-sdk.service.ts @@ -0,0 +1,82 @@ +import { firstValueFrom, switchMap, catchError } 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}`); + throw error; + }), + ), + ); + } + + 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() || new CipherView().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 153bb01403c..4f98ba62a1c 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -28,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"; @@ -54,9 +55,9 @@ function encryptText(clearText: string | Uint8Array) { const ENCRYPTED_BYTES = mock(); const cipherData: CipherData = { - id: "id", - organizationId: "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b2" as OrganizationId, - folderId: "folderId", + id: "5ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b22" as CipherId, + organizationId: "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId, + folderId: "6ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b23", edit: true, viewPassword: true, organizationUseTotp: true, @@ -109,9 +110,10 @@ describe("Cipher Service", () => { const stateProvider = new FakeStateProvider(accountService); const cipherEncryptionService = mock(); const messageSender = mock(); + const cipherSdkService = mock(); const userId = "TestUserId" as UserId; - const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b2" as OrganizationId; + const orgId = "4ff8c0b2-1d3e-4f8c-9b2d-1d3e4f8c0b21" as OrganizationId; let cipherService: CipherService; let encryptionContext: EncryptionContext; @@ -145,6 +147,7 @@ describe("Cipher Service", () => { logService, cipherEncryptionService, messageSender, + cipherSdkService, ); encryptionContext = { cipher: new Cipher(cipherData), encryptedFor: userId }; @@ -207,11 +210,22 @@ describe("Cipher Service", () => { }); describe("createWithServer()", () => { + beforeEach(() => { + jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); + jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { + return new CipherView(cipher); + }); + }); + it("should call apiService.postCipherAdmin when orgAdmin param is true and the cipher orgId != null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); const spy = jest .spyOn(apiService, "postCipherAdmin") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext, true); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId, true); const expectedObj = new CipherCreateRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -219,11 +233,15 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipher when orgAdmin param is true and the cipher orgId is null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.organizationId = null!; const spy = jest .spyOn(apiService, "postCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext, true); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId, true); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -231,11 +249,15 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipherCreate if collectionsIds != null", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.collectionIds = ["123"]; const spy = jest .spyOn(apiService, "postCipherCreate") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId); const expectedObj = new CipherCreateRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -243,35 +265,86 @@ describe("Cipher Service", () => { }); it("should call apiService.postCipher when orgAdmin and collectionIds logic is false", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); const spy = jest .spyOn(apiService, "postCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.createWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.createWithServer(cipherView, userId); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(expectedObj); }); + + it("should delegate to cipherSdkService when feature flag is enabled", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(true); + + const cipherView = new CipherView(encryptionContext.cipher); + const expectedResult = new CipherView(encryptionContext.cipher); + + 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(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); }); describe("updateWithServer()", () => { + beforeEach(() => { + jest.spyOn(cipherService, "encrypt").mockResolvedValue(encryptionContext); + jest.spyOn(cipherService, "decrypt").mockImplementation(async (cipher) => { + return new CipherView(cipher); + }); + jest.spyOn(cipherService, "upsert").mockResolvedValue({ + [cipherData.id as CipherId]: cipherData, + }); + }); + it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); + + const testCipher = new Cipher(cipherData); + testCipher.organizationId = orgId; + const testContext = { cipher: testCipher, encryptedFor: userId }; + jest.spyOn(cipherService, "encrypt").mockResolvedValue(testContext); + const spy = jest .spyOn(apiService, "putCipherAdmin") - .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext, true); - const expectedObj = new CipherRequest(encryptionContext); + .mockImplementation(() => Promise.resolve(testCipher.toCipherData())); + const cipherView = new CipherView(testCipher); + await cipherService.updateWithServer(cipherView, userId, undefined, true); + const expectedObj = new CipherRequest(testContext); expect(spy).toHaveBeenCalled(); - expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj); + expect(spy).toHaveBeenCalledWith(testCipher.id, expectedObj); }); it("should call apiService.putCipher if cipher.edit is true", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.edit = true; const spy = jest .spyOn(apiService, "putCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.updateWithServer(cipherView, userId); const expectedObj = new CipherRequest(encryptionContext); expect(spy).toHaveBeenCalled(); @@ -279,16 +352,79 @@ describe("Cipher Service", () => { }); it("should call apiService.putPartialCipher when orgAdmin, and edit are false", async () => { + configService.getFeatureFlag + .calledWith(FeatureFlag.PM27632_SdkCipherCrudOperations) + .mockResolvedValue(false); encryptionContext.cipher.edit = false; const spy = jest .spyOn(apiService, "putPartialCipher") .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData())); - await cipherService.updateWithServer(encryptionContext); + const cipherView = new CipherView(encryptionContext.cipher); + await cipherService.updateWithServer(cipherView, userId); const expectedObj = new CipherPartialRequest(encryptionContext.cipher); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj); }); + + 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 expectedResult = new CipherView(testCipher); + + 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(cipherSdkServiceSpy).toHaveBeenCalledWith(cipherView, userId, undefined, undefined); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); + + it("should delegate to cipherSdkService with orgAdmin 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 originalCipherView = new CipherView(testCipher); + 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"); + + const result = await cipherService.updateWithServer( + cipherView, + userId, + originalCipherView, + true, + ); + + expect(cipherSdkServiceSpy).toHaveBeenCalledWith( + cipherView, + userId, + originalCipherView, + true, + ); + expect(apiSpy).not.toHaveBeenCalled(); + expect(clearCacheSpy).toHaveBeenCalledWith(userId); + expect(result).toBeInstanceOf(CipherView); + }); }); describe("encrypt", () => { diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 2e0adc892e3..53d7666e304 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -42,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, @@ -120,6 +121,7 @@ export class CipherService implements CipherServiceAbstraction { private logService: LogService, private cipherEncryptionService: CipherEncryptionService, private messageSender: MessageSender, + private cipherSdkService: CipherSdkService, ) {} localData$(userId: UserId): Observable> { @@ -903,6 +905,40 @@ export class CipherService implements CipherServiceAbstraction { } async createWithServer( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + const useSdk = await this.configService.getFeatureFlag( + FeatureFlag.PM27632_SdkCipherCrudOperations, + ); + + if (useSdk) { + return ( + (await this.createWithServerUsingSdk(cipherView, userId, orgAdmin)) || new CipherView() + ); + } + + const encrypted = await this.encrypt(cipherView, userId); + const result = await this.createWithServer_legacy(encrypted, orgAdmin); + return await this.decrypt(result, userId); + } + + private async createWithServerUsingSdk( + cipherView: CipherView, + userId: UserId, + orgAdmin?: boolean, + ): Promise { + const resultCipherView = await this.cipherSdkService.createWithServer( + cipherView, + userId, + orgAdmin, + ); + await this.clearCache(userId); + return resultCipherView; + } + + private async createWithServer_legacy( { cipher, encryptedFor }: EncryptionContext, orgAdmin?: boolean, ): Promise { @@ -929,6 +965,42 @@ export class CipherService implements CipherServiceAbstraction { } async updateWithServer( + cipherView: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + const useSdk = await this.configService.getFeatureFlag( + FeatureFlag.PM27632_SdkCipherCrudOperations, + ); + + if (useSdk) { + return await this.updateWithServerUsingSdk(cipherView, userId, originalCipherView, orgAdmin); + } + + const encrypted = await this.encrypt(cipherView, userId); + const updatedCipher = await this.updateWithServer_legacy(encrypted, orgAdmin); + const updatedCipherView = await this.decrypt(updatedCipher, userId); + return updatedCipherView; + } + + async updateWithServerUsingSdk( + cipher: CipherView, + userId: UserId, + originalCipherView?: CipherView, + orgAdmin?: boolean, + ): Promise { + const resultCipherView = await this.cipherSdkService.updateWithServer( + cipher, + userId, + originalCipherView, + orgAdmin, + ); + await this.clearCache(userId); + return resultCipherView; + } + + async updateWithServer_legacy( { cipher, encryptedFor }: EncryptionContext, orgAdmin?: boolean, ): Promise { @@ -1119,8 +1191,7 @@ export class CipherService implements CipherServiceAbstraction { //in order to keep item and it's attachments with the same encryption level if (cipher.key != null && !cipherKeyEncryptionEnabled) { const model = await this.decrypt(cipher, userId); - const reEncrypted = await this.encrypt(model, userId); - await this.updateWithServer(reEncrypted); + await this.updateWithServer(model, userId); } const encFileName = await this.encryptService.encryptString(filename, cipherEncKey); diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts index 59c583f980b..8566e51d74f 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts @@ -37,14 +37,13 @@ export class DefaultCipherFormService implements CipherFormService { // Creating a new cipher if (cipher.id == null || cipher.id === "") { - const encrypted = await this.cipherService.encrypt(cipher, activeUserId); - savedCipher = await this.cipherService.createWithServer(encrypted, config.admin); - return await this.cipherService.decrypt(savedCipher, activeUserId); + return await this.cipherService.createWithServer(cipher, activeUserId, config.admin); } if (config.originalCipher == null) { throw new Error("Original cipher is required for updating an existing cipher"); } + const originalCipherView = await this.decryptCipher(config.originalCipher); // Updating an existing cipher @@ -66,35 +65,31 @@ export class DefaultCipherFormService implements CipherFormService { ); // If the collectionIds are the same, update the cipher normally } else if (isSetEqual(originalCollectionIds, newCollectionIds)) { - const encrypted = await this.cipherService.encrypt( + const savedCipherView = await this.cipherService.updateWithServer( cipher, activeUserId, - null, - null, - config.originalCipher, + originalCipherView, + config.admin, ); - savedCipher = await this.cipherService.updateWithServer(encrypted, config.admin); + savedCipher = await this.cipherService + .encrypt(savedCipherView, activeUserId) + .then((res) => res.cipher); } else { - const encrypted = await this.cipherService.encrypt( - cipher, - activeUserId, - null, - null, - config.originalCipher, - ); - const encryptedCipher = encrypted.cipher; - // Updating a cipher with collection changes is not supported with a single request currently // First update the cipher with the original collectionIds - encryptedCipher.collectionIds = config.originalCipher.collectionIds; - await this.cipherService.updateWithServer( - encrypted, + cipher.collectionIds = config.originalCipher.collectionIds; + const newCipher = await this.cipherService.updateWithServer( + cipher, + activeUserId, + originalCipherView, config.admin || originalCollectionIds.size === 0, ); // Then save the new collection changes separately - encryptedCipher.collectionIds = cipher.collectionIds; + newCipher.collectionIds = cipher.collectionIds; + // TODO: Remove after migrating all SDK ops + const { cipher: encryptedCipher } = await this.cipherService.encrypt(newCipher, activeUserId); if (config.admin || originalCollectionIds.size === 0) { // When using an admin config or the cipher was unassigned, update collections as an admin savedCipher = await this.cipherService.saveCollectionsWithServerAdmin(encryptedCipher); diff --git a/libs/vault/src/cipher-view/cipher-view.component.spec.ts b/libs/vault/src/cipher-view/cipher-view.component.spec.ts index 18a5132781b..2300565035e 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.spec.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.spec.ts @@ -8,7 +8,6 @@ import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService, Account } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -42,11 +41,9 @@ describe("CipherViewComponent", () => { let mockLogService: LogService; let mockCipherRiskService: CipherRiskService; let mockBillingAccountProfileStateService: BillingAccountProfileStateService; - let mockConfigService: ConfigService; // Mock data let mockCipherView: CipherView; - let featureFlagEnabled$: BehaviorSubject; let hasPremiumFromAnySource$: BehaviorSubject; let activeAccount$: BehaviorSubject; @@ -57,7 +54,6 @@ describe("CipherViewComponent", () => { email: "test@example.com", } as Account); - featureFlagEnabled$ = new BehaviorSubject(false); hasPremiumFromAnySource$ = new BehaviorSubject(true); // Create service mocks @@ -83,9 +79,6 @@ describe("CipherViewComponent", () => { .fn() .mockReturnValue(hasPremiumFromAnySource$); - mockConfigService = mock(); - mockConfigService.getFeatureFlag$ = jest.fn().mockReturnValue(featureFlagEnabled$); - // Setup mock cipher view mockCipherView = new CipherView(); mockCipherView.id = "cipher-id"; @@ -110,7 +103,6 @@ describe("CipherViewComponent", () => { provide: BillingAccountProfileStateService, useValue: mockBillingAccountProfileStateService, }, - { provide: ConfigService, useValue: mockConfigService }, ], schemas: [NO_ERRORS_SCHEMA], }) @@ -145,7 +137,6 @@ describe("CipherViewComponent", () => { beforeEach(() => { // Reset observables to default values for this test suite - featureFlagEnabled$.next(true); hasPremiumFromAnySource$.next(true); // Setup default mock for computeCipherRiskForUser (individual tests can override) @@ -162,18 +153,6 @@ describe("CipherViewComponent", () => { component = fixture.componentInstance; }); - it("returns false when feature flag is disabled", fakeAsync(() => { - featureFlagEnabled$.next(false); - - const cipher = createLoginCipherView(); - fixture.componentRef.setInput("cipher", cipher); - fixture.detectChanges(); - tick(); - - expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); - expect(component.passwordIsAtRisk()).toBe(false); - })); - it("returns false when cipher has no login password", fakeAsync(() => { const cipher = createLoginCipherView(); cipher.login = {} as any; // No password diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index b5c063df51e..26e3f18b542 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -13,8 +13,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { isCardExpired } from "@bitwarden/common/autofill/utils"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { getByIds } from "@bitwarden/common/platform/misc"; @@ -113,7 +111,6 @@ export class CipherViewComponent { private logService: LogService, private cipherRiskService: CipherRiskService, private billingAccountService: BillingAccountProfileStateService, - private configService: ConfigService, ) {} readonly resolvedCollections = toSignal( @@ -248,19 +245,9 @@ export class CipherViewComponent { * The password is only evaluated when the user is premium and has edit access to the cipher. */ readonly passwordIsAtRisk = toSignal( - combineLatest([ - this.activeUserId$, - this.cipher$, - this.configService.getFeatureFlag$(FeatureFlag.RiskInsightsForPremium), - ]).pipe( - switchMap(([userId, cipher, featureEnabled]) => { - if ( - !featureEnabled || - !cipher.hasLoginPassword || - !cipher.edit || - cipher.organizationId || - cipher.isDeleted - ) { + combineLatest([this.activeUserId$, this.cipher$]).pipe( + switchMap(([userId, cipher]) => { + if (!cipher.hasLoginPassword || !cipher.edit || cipher.organizationId || cipher.isDeleted) { return of(false); } return this.switchPremium$( diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.html b/libs/vault/src/components/download-attachment/download-attachment.component.html index 9d80f36818a..c6665c5d569 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.html +++ b/libs/vault/src/components/download-attachment/download-attachment.component.html @@ -5,6 +5,6 @@ buttonType="main" size="small" type="button" - [label]="'downloadAttachmentName' | i18n: attachment().fileName" + [label]="'downloadAttachmentLabel' | i18n" > } diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts b/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts index 3bbc375fdfc..a46ce28fca8 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts +++ b/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts @@ -108,7 +108,7 @@ describe("DownloadAttachmentComponent", () => { it("renders delete button", () => { const deleteButton = fixture.debugElement.query(By.css("button")); - expect(deleteButton.attributes["aria-label"]).toBe("downloadAttachmentName"); + expect(deleteButton.attributes["aria-label"]).toBe("downloadAttachmentLabel"); }); describe("download attachment", () => { diff --git a/package-lock.json b/package-lock.json index 42206a1b46c..2cd18e11adc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,7 +52,7 @@ "lunr": "2.3.9", "multer": "2.0.2", "ngx-toastr": "19.1.0", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "oidc-client-ts": "2.4.1", "open": "11.0.0", @@ -110,7 +110,7 @@ "@types/lowdb": "1.0.15", "@types/lunr": "2.3.7", "@types/node": "22.19.3", - "@types/node-fetch": "2.6.4", + "@types/node-fetch": "2.6.13", "@types/node-forge": "1.3.14", "@types/papaparse": "5.5.0", "@types/proper-lockfile": "4.1.4", @@ -217,7 +217,7 @@ "lowdb": "1.0.0", "lunr": "2.3.9", "multer": "2.0.2", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "open": "11.0.0", "papaparse": "5.5.3", @@ -15842,53 +15842,14 @@ } }, "node_modules/@types/node-fetch": { - "version": "2.6.4", - "resolved": "https://registry.npmjs.org/@types/node-fetch/-/node-fetch-2.6.4.tgz", - "integrity": "sha512-1ZX9fcN4Rvkvgv4E6PAY5WXUFWFcRWxZa3EW83UjycOB9ljJCedb2CupIP4RZMEwF/M3eTcCihbBRgwtGbg5Rg==", + "version": "2.6.13", + "resolved": "https://registry.npmjs.org/@types/node-fetch/-/node-fetch-2.6.13.tgz", + "integrity": "sha512-QGpRVpzSaUs30JBSGPjOg4Uveu384erbHBoT1zeONvyCfwQxIkUshLAOqN/k9EjGviPRmWTTe6aH2qySWKTVSw==", "dev": true, "license": "MIT", "dependencies": { "@types/node": "*", - "form-data": "^3.0.0" - } - }, - "node_modules/@types/node-fetch/node_modules/form-data": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.3.tgz", - "integrity": "sha512-q5YBMeWy6E2Un0nMGWMgI65MAKtaylxfNJGJxpGh45YDciZB4epbWpaAfImil6CPAPTYB4sh0URQNDRIZG5F2w==", - "dev": true, - "license": "MIT", - "dependencies": { - "asynckit": "^0.4.0", - "combined-stream": "^1.0.8", - "es-set-tostringtag": "^2.1.0", - "mime-types": "^2.1.35" - }, - "engines": { - "node": ">= 6" - } - }, - "node_modules/@types/node-fetch/node_modules/mime-db": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", - "integrity": "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/@types/node-fetch/node_modules/mime-types": { - "version": "2.1.35", - "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.35.tgz", - "integrity": "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==", - "dev": true, - "license": "MIT", - "dependencies": { - "mime-db": "1.52.0" - }, - "engines": { - "node": ">= 0.6" + "form-data": "^4.0.4" } }, "node_modules/@types/node-forge": { @@ -32816,9 +32777,9 @@ } }, "node_modules/node-fetch": { - "version": "2.6.12", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.12.tgz", - "integrity": "sha512-C/fGU2E8ToujUivIO0H+tpQ6HWo4eEmchoPIoXtxCrVghxdKq+QOHqEZW7tuP3KlV3bC8FRMO5nMCC7Zm1VP6g==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "license": "MIT", "dependencies": { "whatwg-url": "^5.0.0" diff --git a/package.json b/package.json index 829dc91370a..8455d97c87c 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "@types/lowdb": "1.0.15", "@types/lunr": "2.3.7", "@types/node": "22.19.3", - "@types/node-fetch": "2.6.4", + "@types/node-fetch": "2.6.13", "@types/node-forge": "1.3.14", "@types/papaparse": "5.5.0", "@types/proper-lockfile": "4.1.4", @@ -191,7 +191,7 @@ "lunr": "2.3.9", "multer": "2.0.2", "ngx-toastr": "19.1.0", - "node-fetch": "2.6.12", + "node-fetch": "2.7.0", "node-forge": "1.3.2", "oidc-client-ts": "2.4.1", "open": "11.0.0",