diff --git a/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts b/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts index a717786ae52..69a0fa7d9d1 100644 --- a/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts +++ b/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts @@ -133,6 +133,7 @@ export class PopupViewCacheService implements ViewCacheService { } private clearState() { + this._cache = {}; // clear local cache this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, {}); } } diff --git a/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts b/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts index fbe94bece8c..72cfd39cd62 100644 --- a/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts +++ b/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts @@ -196,13 +196,15 @@ describe("popup view cache", () => { }); it("should clear on 2nd navigation", async () => { - await initServiceWithState({}); + await initServiceWithState({ temp: "state" }); await router.navigate(["a"]); expect(messageSenderMock.send).toHaveBeenCalledTimes(0); + expect(service["_cache"]).toEqual({ temp: "state" }); await router.navigate(["b"]); expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {}); + expect(service["_cache"]).toEqual({}); }); it("should ignore cached values when feature flag is off", async () => { diff --git a/apps/browser/src/platform/services/popup-view-cache-background.service.ts b/apps/browser/src/platform/services/popup-view-cache-background.service.ts index f6f5e45f093..e739a3677f1 100644 --- a/apps/browser/src/platform/services/popup-view-cache-background.service.ts +++ b/apps/browser/src/platform/services/popup-view-cache-background.service.ts @@ -60,6 +60,11 @@ export class PopupViewCacheBackgroundService { ) .subscribe(); + this.messageListener + .messages$(ClEAR_VIEW_CACHE_COMMAND) + .pipe(concatMap(() => this.popupViewCacheState.update(() => null))) + .subscribe(); + merge( // on tab changed, excluding extension tabs fromChromeEvent(chrome.tabs.onActivated).pipe( diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 84f28e1890d..b321199d5e5 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -44,6 +44,7 @@ export enum FeatureFlag { NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss", DisableFreeFamiliesSponsorship = "PM-12274-disable-free-families-sponsorship", MacOsNativeCredentialSync = "macos-native-credential-sync", + PM9111ExtensionPersistAddEditForm = "pm-9111-extension-persist-add-edit-form", PrivateKeyRegeneration = "pm-12241-private-key-regeneration", ResellerManagedOrgAlert = "PM-15814-alert-owners-of-reseller-managed-orgs", } @@ -100,6 +101,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.NewDeviceVerificationPermanentDismiss]: FALSE, [FeatureFlag.DisableFreeFamiliesSponsorship]: FALSE, [FeatureFlag.MacOsNativeCredentialSync]: FALSE, + [FeatureFlag.PM9111ExtensionPersistAddEditForm]: FALSE, [FeatureFlag.PrivateKeyRegeneration]: FALSE, [FeatureFlag.ResellerManagedOrgAlert]: FALSE, } satisfies Record; diff --git a/libs/vault/src/cipher-form/cipher-form-container.ts b/libs/vault/src/cipher-form/cipher-form-container.ts index 54f194598e5..a5f2289b0c6 100644 --- a/libs/vault/src/cipher-form/cipher-form-container.ts +++ b/libs/vault/src/cipher-form/cipher-form-container.ts @@ -14,7 +14,6 @@ import { SshKeySectionComponent } from "./components/sshkey-section/sshkey-secti /** * The complete form for a cipher. Includes all the sub-forms from their respective section components. - * TODO: Add additional form sections as they are implemented. */ export type CipherForm = { itemDetails?: ItemDetailsSectionComponent["itemDetailsForm"]; @@ -57,4 +56,12 @@ export abstract class CipherFormContainer { * @param updateFn - A function that takes the current cipherView and returns the updated cipherView */ abstract patchCipher(updateFn: (current: CipherView) => CipherView): void; + + /** + * Returns initial values for the CipherView, either from the config or the cached cipher + */ + abstract getInitialCipherView(): CipherView | null; + + /** Returns true when the `CipherFormContainer` was initialized with a cached cipher available. */ + abstract initializedWithCachedCipher(): boolean; } diff --git a/libs/vault/src/cipher-form/cipher-form.stories.ts b/libs/vault/src/cipher-form/cipher-form.stories.ts index f7146776b73..1af73b5a8b8 100644 --- a/libs/vault/src/cipher-form/cipher-form.stories.ts +++ b/libs/vault/src/cipher-form/cipher-form.stories.ts @@ -1,4 +1,6 @@ -import { importProvidersFrom } from "@angular/core"; +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore +import { importProvidersFrom, signal } from "@angular/core"; import { action } from "@storybook/addon-actions"; import { applicationConfig, @@ -10,6 +12,7 @@ import { import { BehaviorSubject } from "rxjs"; import { CollectionView } from "@bitwarden/admin-console/common"; +import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -18,6 +21,7 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; import { ClientType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -39,6 +43,7 @@ import { CipherFormService } from "./abstractions/cipher-form.service"; import { TotpCaptureService } from "./abstractions/totp-capture.service"; import { CipherFormModule } from "./cipher-form.module"; import { CipherFormComponent } from "./components/cipher-form.component"; +import { CipherFormCacheService } from "./services/default-cipher-form-cache.service"; const defaultConfig: CipherFormConfig = { mode: "add", @@ -192,6 +197,25 @@ export default { activeAccount$: new BehaviorSubject({ email: "test@example.com" }), }, }, + { + provide: CipherFormCacheService, + useValue: { + getCachedCipherView: (): null => null, + initializedWithValue: false, + }, + }, + { + provide: ViewCacheService, + useValue: { + signal: () => signal(null), + }, + }, + { + provide: ConfigService, + useValue: { + getFeatureFlag: () => Promise.resolve(false), + }, + }, ], }), componentWrapperDecorator( diff --git a/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts b/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts index 15784f1ca06..705c170933a 100644 --- a/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts @@ -27,9 +27,12 @@ describe("AdditionalOptionsSectionComponent", () => { let passwordRepromptService: MockProxy; let passwordRepromptEnabled$: BehaviorSubject; + const getInitialCipherView = jest.fn(() => null); + beforeEach(async () => { - cipherFormProvider = mock(); + getInitialCipherView.mockClear(); + cipherFormProvider = mock({ getInitialCipherView }); passwordRepromptService = mock(); passwordRepromptEnabled$ = new BehaviorSubject(true); passwordRepromptService.enabled$ = passwordRepromptEnabled$; @@ -94,11 +97,11 @@ describe("AdditionalOptionsSectionComponent", () => { expect(component.additionalOptionsForm.disabled).toBe(true); }); - it("initializes 'additionalOptionsForm' with original cipher view values", () => { - (cipherFormProvider.originalCipherView as any) = { + it("initializes 'additionalOptionsForm' from `getInitialCipherValue`", () => { + getInitialCipherView.mockReturnValueOnce({ notes: "original notes", reprompt: 1, - } as CipherView; + } as CipherView); component.ngOnInit(); diff --git a/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts b/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts index 04291a3e083..9c619ca2f84 100644 --- a/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts +++ b/libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts @@ -77,11 +77,12 @@ export class AdditionalOptionsSectionComponent implements OnInit { } ngOnInit() { - if (this.cipherFormContainer.originalCipherView) { + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + if (prefillCipher) { this.additionalOptionsForm.patchValue({ - notes: this.cipherFormContainer.originalCipherView.notes, - reprompt: - this.cipherFormContainer.originalCipherView.reprompt === CipherRepromptType.Password, + notes: prefillCipher.notes, + reprompt: prefillCipher.reprompt === CipherRepromptType.Password, }); } diff --git a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts index f1e5af2868a..ea756d17c40 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts +++ b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts @@ -25,9 +25,11 @@ describe("AutofillOptionsComponent", () => { let domainSettingsService: MockProxy; let autofillSettingsService: MockProxy; let platformUtilsService: MockProxy; + const getInitialCipherView = jest.fn(() => null); beforeEach(async () => { - cipherFormContainer = mock(); + getInitialCipherView.mockClear(); + cipherFormContainer = mock({ getInitialCipherView }); liveAnnouncer = mock(); platformUtilsService = mock(); domainSettingsService = mock(); @@ -107,12 +109,14 @@ describe("AutofillOptionsComponent", () => { existingLogin.uri = "https://example.com"; existingLogin.match = UriMatchStrategy.Exact; - (cipherFormContainer.originalCipherView as CipherView) = new CipherView(); - cipherFormContainer.originalCipherView.login = { + const cipher = new CipherView(); + cipher.login = { autofillOnPageLoad: true, uris: [existingLogin], } as LoginView; + getInitialCipherView.mockReturnValueOnce(cipher); + fixture.detectChanges(); expect(component.autofillOptionsForm.value.uris).toEqual([ @@ -138,12 +142,14 @@ describe("AutofillOptionsComponent", () => { existingLogin.uri = "https://example.com"; existingLogin.match = UriMatchStrategy.Exact; - (cipherFormContainer.originalCipherView as CipherView) = new CipherView(); - cipherFormContainer.originalCipherView.login = { + const cipher = new CipherView(); + cipher.login = { autofillOnPageLoad: true, uris: [existingLogin], } as LoginView; + getInitialCipherView.mockReturnValueOnce(cipher); + fixture.detectChanges(); expect(component.autofillOptionsForm.value.uris).toEqual([ @@ -159,12 +165,14 @@ describe("AutofillOptionsComponent", () => { existingLogin.uri = "https://example.com"; existingLogin.match = UriMatchStrategy.Exact; - (cipherFormContainer.originalCipherView as CipherView) = new CipherView(); - cipherFormContainer.originalCipherView.login = { + const cipher = new CipherView(); + cipher.login = { autofillOnPageLoad: true, uris: [existingLogin], } as LoginView; + getInitialCipherView.mockReturnValueOnce(cipher); + fixture.detectChanges(); expect(component.autofillOptionsForm.value.uris).toEqual([ diff --git a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts index cf73f3dbed4..c3b2a0fb9f9 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts +++ b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts @@ -130,8 +130,9 @@ export class AutofillOptionsComponent implements OnInit { } ngOnInit() { - if (this.cipherFormContainer.originalCipherView?.login) { - this.initFromExistingCipher(this.cipherFormContainer.originalCipherView.login); + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + if (prefillCipher) { + this.initFromExistingCipher(prefillCipher.login); } else { this.initNewCipher(); } @@ -142,17 +143,29 @@ export class AutofillOptionsComponent implements OnInit { } private initFromExistingCipher(existingLogin: LoginView) { + // The `uris` control is a FormArray which needs to dynamically + // add controls to the form. Doing this will trigger the `valueChanges` observable on the form + // and overwrite the `autofillOnPageLoad` value before it is set in the following `patchValue` call. + // Pass `false` to `addUri` to stop events from emitting when adding the URIs. existingLogin.uris?.forEach((uri) => { - this.addUri({ - uri: uri.uri, - matchDetection: uri.match, - }); + this.addUri( + { + uri: uri.uri, + matchDetection: uri.match, + }, + false, + false, + ); }); this.autofillOptionsForm.patchValue({ autofillOnPageLoad: existingLogin.autofillOnPageLoad, }); - if (this.cipherFormContainer.config.initialValues?.loginUri) { + // Only add the initial value when the cipher was not initialized from a cached state + if ( + this.cipherFormContainer.config.initialValues?.loginUri && + !this.cipherFormContainer.initializedWithCachedCipher() + ) { // Avoid adding the same uri again if it already exists if ( existingLogin.uris?.findIndex( @@ -197,9 +210,16 @@ export class AutofillOptionsComponent implements OnInit { * Adds a new URI input to the form. * @param uriFieldValue The initial value for the new URI input. * @param focusNewInput If true, the new URI input will be focused after being added. + * @param emitEvent When false, prevents the `valueChanges` & `statusChanges` observables from firing. */ - addUri(uriFieldValue: UriField = { uri: null, matchDetection: null }, focusNewInput = false) { - this.autofillOptionsForm.controls.uris.push(this.formBuilder.control(uriFieldValue)); + addUri( + uriFieldValue: UriField = { uri: null, matchDetection: null }, + focusNewInput = false, + emitEvent = true, + ) { + this.autofillOptionsForm.controls.uris.push(this.formBuilder.control(uriFieldValue), { + emitEvent, + }); if (focusNewInput) { this.focusOnNewInput$.next(); diff --git a/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.spec.ts index e9581859b2e..39a59192985 100644 --- a/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.spec.ts @@ -20,8 +20,10 @@ describe("CardDetailsSectionComponent", () => { let registerChildFormSpy: jest.SpyInstance; let patchCipherSpy: jest.SpyInstance; + const getInitialCipherView = jest.fn(() => null); + beforeEach(async () => { - cipherFormProvider = mock(); + cipherFormProvider = mock({ getInitialCipherView }); registerChildFormSpy = jest.spyOn(cipherFormProvider, "registerChildForm"); patchCipherSpy = jest.spyOn(cipherFormProvider, "patchCipher"); @@ -94,7 +96,7 @@ describe("CardDetailsSectionComponent", () => { expect(component.cardDetailsForm.disabled).toBe(true); }); - it("initializes `cardDetailsForm` with current values", () => { + it("initializes `cardDetailsForm` from `getInitialCipherValue`", () => { const cardholderName = "Ron Burgundy"; const number = "4242 4242 4242 4242"; const code = "619"; @@ -105,9 +107,7 @@ describe("CardDetailsSectionComponent", () => { cardView.code = code; cardView.brand = "Visa"; - component.originalCipherView = { - card: cardView, - } as CipherView; + getInitialCipherView.mockReturnValueOnce({ card: cardView }); component.ngOnInit(); diff --git a/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.ts b/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.ts index 63c5906b570..c2b3ebb59aa 100644 --- a/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.ts @@ -136,8 +136,10 @@ export class CardDetailsSectionComponent implements OnInit { } ngOnInit() { - if (this.originalCipherView?.card) { - this.setInitialValues(); + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + if (prefillCipher) { + this.setInitialValues(prefillCipher); } if (this.disabled) { @@ -172,8 +174,8 @@ export class CardDetailsSectionComponent implements OnInit { } /** Set form initial form values from the current cipher */ - private setInitialValues() { - const { cardholderName, number, brand, expMonth, expYear, code } = this.originalCipherView.card; + private setInitialValues(cipherView: CipherView) { + const { cardholderName, number, brand, expMonth, expYear, code } = cipherView.card; this.cardDetailsForm.setValue({ cardholderName: cardholderName, diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index 3d3e04864e8..a4f8f8f61e0 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -38,6 +38,7 @@ import { import { CipherFormConfig } from "../abstractions/cipher-form-config.service"; import { CipherFormService } from "../abstractions/cipher-form.service"; import { CipherForm, CipherFormContainer } from "../cipher-form-container"; +import { CipherFormCacheService } from "../services/default-cipher-form-cache.service"; import { AdditionalOptionsSectionComponent } from "./additional-options/additional-options-section.component"; import { CardDetailsSectionComponent } from "./card-details-section/card-details-section.component"; @@ -55,6 +56,9 @@ import { SshKeySectionComponent } from "./sshkey-section/sshkey-section.componen provide: CipherFormContainer, useExisting: forwardRef(() => CipherFormComponent), }, + { + provide: CipherFormCacheService, + }, ], imports: [ AsyncActionsModule, @@ -164,6 +168,26 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci */ patchCipher(updateFn: (current: CipherView) => CipherView): void { this.updatedCipherView = updateFn(this.updatedCipherView); + // Cache the updated cipher + this.cipherFormCacheService.cacheCipherView(this.updatedCipherView); + } + + /** + * Return initial values for given keys of a cipher + */ + getInitialCipherView(): CipherView { + const cachedCipherView = this.cipherFormCacheService.getCachedCipherView(); + + if (cachedCipherView) { + return cachedCipherView; + } + + return this.originalCipherView; + } + + /** */ + initializedWithCachedCipher(): boolean { + return this.cipherFormCacheService.initializedWithValue; } /** @@ -187,6 +211,8 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci // Force change detection so that all child components are destroyed and re-created this.changeDetectorRef.detectChanges(); + await this.cipherFormCacheService.init(); + this.updatedCipherView = new CipherView(); this.originalCipherView = null; this.cipherForm = this.formBuilder.group({}); @@ -220,16 +246,39 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } } + this.setInitialCipherFromCache(); + this.loading = false; this.formReadySubject.next(); } + /** + * Updates `updatedCipherView` based on the value from the cache. + */ + setInitialCipherFromCache() { + const cachedCipher = this.cipherFormCacheService.getCachedCipherView(); + if (cachedCipher === null) { + return; + } + + // Use the cached cipher when it matches the cipher being edited + if (this.updatedCipherView.id === cachedCipher.id) { + this.updatedCipherView = cachedCipher; + } + + // `id` is null when a cipher is being added + if (this.updatedCipherView.id === null) { + this.updatedCipherView = cachedCipher; + } + } + constructor( private formBuilder: FormBuilder, private addEditFormService: CipherFormService, private toastService: ToastService, private i18nService: I18nService, private changeDetectorRef: ChangeDetectorRef, + private cipherFormCacheService: CipherFormCacheService, ) {} /** diff --git a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts index b523d9ef933..945f56821d2 100644 --- a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts +++ b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts @@ -61,7 +61,13 @@ describe("CustomFieldsComponent", () => { }, { provide: CipherFormContainer, - useValue: { patchCipher, originalCipherView, registerChildForm: jest.fn(), config }, + useValue: { + patchCipher, + originalCipherView, + registerChildForm: jest.fn(), + config, + getInitialCipherView: jest.fn(() => originalCipherView), + }, }, { provide: LiveAnnouncer, diff --git a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts index cd75fe8fba1..f17432a993b 100644 --- a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts +++ b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts @@ -148,8 +148,10 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit { value: id, })); - // Populate the form with the existing fields - this.cipherFormContainer.originalCipherView?.fields?.forEach((field) => { + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + // When available, populate the form with the existing fields + prefillCipher.fields?.forEach((field) => { let value: string | boolean = field.value; if (field.type === FieldType.Boolean) { diff --git a/libs/vault/src/cipher-form/components/identity/identity.component.ts b/libs/vault/src/cipher-form/components/identity/identity.component.ts index b1403231ecf..2d0608bf38a 100644 --- a/libs/vault/src/cipher-form/components/identity/identity.component.ts +++ b/libs/vault/src/cipher-form/components/identity/identity.component.ts @@ -113,8 +113,10 @@ export class IdentitySectionComponent implements OnInit { this.identityForm.disable(); } - if (this.originalCipherView && this.originalCipherView.id) { - this.populateFormData(); + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + if (prefillCipher) { + this.populateFormData(prefillCipher); } else { this.identityForm.patchValue({ username: this.cipherFormContainer.config.initialValues?.username || "", @@ -122,8 +124,9 @@ export class IdentitySectionComponent implements OnInit { } } - populateFormData() { - const { identity } = this.originalCipherView; + populateFormData(cipherView: CipherView) { + const { identity } = cipherView; + this.identityForm.setValue({ title: identity.title, firstName: identity.firstName, diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts index 32c1e7417e4..ee1f27b712d 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts @@ -9,7 +9,6 @@ import { CollectionView } from "@bitwarden/admin-console/common"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { SelectComponent } from "@bitwarden/components"; @@ -25,9 +24,17 @@ describe("ItemDetailsSectionComponent", () => { let i18nService: MockProxy; const activeAccount$ = new BehaviorSubject<{ email: string }>({ email: "test@example.com" }); + const getInitialCipherView = jest.fn(() => null); + const initializedWithCachedCipher = jest.fn(() => false); beforeEach(async () => { - cipherFormProvider = mock(); + getInitialCipherView.mockClear(); + initializedWithCachedCipher.mockClear(); + + cipherFormProvider = mock({ + getInitialCipherView, + initializedWithCachedCipher, + }); i18nService = mock(); await TestBed.configureTestingModule({ @@ -95,13 +102,14 @@ describe("ItemDetailsSectionComponent", () => { readOnly: false, } as CollectionView, ]; - component.originalCipherView = { + + getInitialCipherView.mockReturnValueOnce({ name: "cipher1", organizationId: "org1", folderId: "folder1", collectionIds: ["col1"], favorite: true, - } as CipherView; + }); await component.ngOnInit(); tick(); @@ -118,55 +126,6 @@ describe("ItemDetailsSectionComponent", () => { expect(updatedCipher.favorite).toBe(true); })); - it("should prioritize initialValues when editing an existing cipher ", fakeAsync(async () => { - component.config.allowPersonalOwnership = true; - component.config.organizations = [{ id: "org1" } as Organization]; - component.config.collections = [ - { - id: "col1", - name: "Collection 1", - organizationId: "org1", - assigned: true, - readOnly: true, - } as CollectionView, - { - id: "col2", - name: "Collection 2", - organizationId: "org1", - assigned: true, - readOnly: false, - } as CollectionView, - ]; - component.originalCipherView = { - name: "cipher1", - organizationId: "org1", - folderId: "folder1", - collectionIds: ["col1"], - favorite: true, - } as CipherView; - - component.config.initialValues = { - name: "new-name", - folderId: "new-folder", - organizationId: "bad-org" as OrganizationId, // Should not be set in edit mode - collectionIds: ["col2" as CollectionId], - }; - - await component.ngOnInit(); - tick(); - - expect(cipherFormProvider.patchCipher).toHaveBeenCalled(); - const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0]; - - const updatedCipher = patchFn(new CipherView()); - - expect(updatedCipher.name).toBe("new-name"); - expect(updatedCipher.organizationId).toBe("org1"); - expect(updatedCipher.folderId).toBe("new-folder"); - expect(updatedCipher.collectionIds).toEqual(["col2"]); - expect(updatedCipher.favorite).toBe(true); - })); - it("should disable organizationId control if ownership change is not allowed", async () => { component.config.allowPersonalOwnership = false; component.config.organizations = [{ id: "org1" } as Organization]; @@ -294,10 +253,13 @@ describe("ItemDetailsSectionComponent", () => { }); describe("cloneMode", () => { - it("should append '- Clone' to the title if in clone mode", async () => { + beforeEach(() => { component.config.mode = "clone"; + }); + + it("should append '- Clone' to the title if in clone mode", async () => { component.config.allowPersonalOwnership = true; - component.originalCipherView = { + const cipher = { name: "cipher1", organizationId: null, folderId: null, @@ -305,6 +267,8 @@ describe("ItemDetailsSectionComponent", () => { favorite: false, } as CipherView; + getInitialCipherView.mockReturnValueOnce(cipher); + i18nService.t.calledWith("clone").mockReturnValue("Clone"); await component.ngOnInit(); @@ -312,8 +276,28 @@ describe("ItemDetailsSectionComponent", () => { expect(component.itemDetailsForm.controls.name.value).toBe("cipher1 - Clone"); }); + it("does not append clone when the cipher was populated from the cache", async () => { + component.config.allowPersonalOwnership = true; + const cipher = { + name: "from cache cipher", + organizationId: null, + folderId: null, + collectionIds: null, + favorite: false, + } as CipherView; + + getInitialCipherView.mockReturnValueOnce(cipher); + + initializedWithCachedCipher.mockReturnValueOnce(true); + + i18nService.t.calledWith("clone").mockReturnValue("Clone"); + + await component.ngOnInit(); + + expect(component.itemDetailsForm.controls.name.value).toBe("from cache cipher"); + }); + it("should select the first organization if personal ownership is not allowed", async () => { - component.config.mode = "clone"; component.config.allowPersonalOwnership = false; component.config.organizations = [ { id: "org1" } as Organization, @@ -376,13 +360,13 @@ describe("ItemDetailsSectionComponent", () => { it("should set collectionIds to originalCipher collections on first load", async () => { component.config.mode = "clone"; - component.originalCipherView = { + getInitialCipherView.mockReturnValueOnce({ name: "cipher1", organizationId: "org1", folderId: "folder1", collectionIds: ["col1", "col2"], favorite: true, - } as CipherView; + }); component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ { @@ -447,6 +431,13 @@ describe("ItemDetailsSectionComponent", () => { it("should show readonly hint if readonly collections are present", async () => { component.config.mode = "edit"; + getInitialCipherView.mockReturnValueOnce({ + name: "cipher1", + organizationId: "org1", + folderId: "folder1", + collectionIds: ["col1", "col2", "col3"], + favorite: true, + }); component.originalCipherView = { name: "cipher1", organizationId: "org1", @@ -559,6 +550,9 @@ describe("ItemDetailsSectionComponent", () => { collectionIds: ["col1", "col2", "col3"], favorite: true, } as CipherView; + + getInitialCipherView.mockReturnValue(component.originalCipherView); + component.config.organizations = [{ id: "org1" } as Organization]; }); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index f7fd228232e..a01d25f0600 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -183,8 +183,10 @@ export class ItemDetailsSectionComponent implements OnInit { throw new Error("No organizations available for ownership."); } - if (this.originalCipherView) { - await this.initFromExistingCipher(); + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + if (prefillCipher) { + await this.initFromExistingCipher(prefillCipher); } else { this.itemDetailsForm.setValue({ name: this.initialValues?.name || "", @@ -210,30 +212,37 @@ export class ItemDetailsSectionComponent implements OnInit { .subscribe(); } - private async initFromExistingCipher() { + private async initFromExistingCipher(prefillCipher: CipherView) { + const { name, folderId, collectionIds } = prefillCipher; + this.itemDetailsForm.setValue({ - name: this.initialValues?.name ?? this.originalCipherView.name, - organizationId: this.originalCipherView.organizationId, // We do not allow changing ownership of an existing cipher. - folderId: this.initialValues?.folderId ?? this.originalCipherView.folderId, + name: name ? name : (this.initialValues?.name ?? ""), + organizationId: prefillCipher.organizationId, // We do not allow changing ownership of an existing cipher. + folderId: folderId ? folderId : (this.initialValues?.folderId ?? null), collectionIds: [], - favorite: this.originalCipherView.favorite, + favorite: prefillCipher.favorite, }); + const initializedWithCachedCipher = this.cipherFormContainer.initializedWithCachedCipher(); + // Configure form for clone mode. if (this.config.mode === "clone") { - this.itemDetailsForm.controls.name.setValue( - this.originalCipherView.name + " - " + this.i18nService.t("clone"), - ); + if (!initializedWithCachedCipher) { + this.itemDetailsForm.controls.name.setValue( + prefillCipher.name + " - " + this.i18nService.t("clone"), + ); + } - if (!this.allowPersonalOwnership && this.originalCipherView.organizationId == null) { + if (!this.allowPersonalOwnership && prefillCipher.organizationId == null) { this.itemDetailsForm.controls.organizationId.setValue(this.defaultOwner); } } - await this.updateCollectionOptions( - this.initialValues?.collectionIds ?? - (this.originalCipherView.collectionIds as CollectionId[]), - ); + const prefillCollections = collectionIds?.length + ? (collectionIds as CollectionId[]) + : (this.initialValues?.collectionIds ?? []); + + await this.updateCollectionOptions(prefillCollections); if (this.partialEdit) { this.itemDetailsForm.disable(); diff --git a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts index 69cfd419742..453b0dce42d 100644 --- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts @@ -45,9 +45,11 @@ describe("LoginDetailsSectionComponent", () => { let configService: MockProxy; const collect = jest.fn().mockResolvedValue(null); + const getInitialCipherView = jest.fn(() => null); beforeEach(async () => { - cipherFormContainer = mock(); + getInitialCipherView.mockClear(); + cipherFormContainer = mock({ getInitialCipherView }); generationService = mock(); auditService = mock(); @@ -122,18 +124,18 @@ describe("LoginDetailsSectionComponent", () => { }); it("initializes 'loginDetailsForm' with original cipher view values", async () => { - (cipherFormContainer.originalCipherView as CipherView) = { + getInitialCipherView.mockReturnValueOnce({ viewPassword: true, login: { password: "original-password", username: "original-username", totp: "original-totp", - } as LoginView, - } as CipherView; + }, + }); - await component.ngOnInit(); + component.ngOnInit(); - expect(component.loginDetailsForm.value).toEqual({ + expect(component.loginDetailsForm.getRawValue()).toEqual({ username: "original-username", password: "original-password", totp: "original-totp", @@ -141,22 +143,23 @@ describe("LoginDetailsSectionComponent", () => { }); it("initializes 'loginDetailsForm' with initialValues that override any original cipher view values", async () => { - (cipherFormContainer.originalCipherView as CipherView) = { + getInitialCipherView.mockReturnValueOnce({ viewPassword: true, login: { password: "original-password", username: "original-username", totp: "original-totp", - } as LoginView, - } as CipherView; + }, + }); + cipherFormContainer.config.initialValues = { username: "new-username", password: "new-password", }; - await component.ngOnInit(); + component.ngOnInit(); - expect(component.loginDetailsForm.value).toEqual({ + expect(component.loginDetailsForm.getRawValue()).toEqual({ username: "new-username", password: "new-password", totp: "original-totp", @@ -165,12 +168,12 @@ describe("LoginDetailsSectionComponent", () => { describe("viewHiddenFields", () => { beforeEach(() => { - (cipherFormContainer.originalCipherView as CipherView) = { + getInitialCipherView.mockReturnValue({ viewPassword: false, login: { password: "original-password", - } as LoginView, - } as CipherView; + }, + }); }); it("returns value of originalCipher.viewPassword", () => { @@ -251,6 +254,10 @@ describe("LoginDetailsSectionComponent", () => { }); describe("password", () => { + beforeEach(() => { + getInitialCipherView.mockReturnValue(null); + }); + const getGeneratePasswordBtn = () => fixture.nativeElement.querySelector("button[data-testid='generate-password-button']"); @@ -520,11 +527,11 @@ describe("LoginDetailsSectionComponent", () => { fixture.nativeElement.querySelector("input[data-testid='passkey-field']"); beforeEach(() => { - (cipherFormContainer.originalCipherView as CipherView) = { + getInitialCipherView.mockReturnValue({ login: Object.assign(new LoginView(), { fido2Credentials: [{ creationDate: passkeyDate } as Fido2CredentialView], }), - } as CipherView; + }); fixture = TestBed.createComponent(LoginDetailsSectionComponent); component = fixture.componentInstance; @@ -567,7 +574,11 @@ describe("LoginDetailsSectionComponent", () => { }); it("hides the passkey field when missing a passkey", () => { - (cipherFormContainer.originalCipherView as CipherView).login.fido2Credentials = []; + getInitialCipherView.mockReturnValueOnce({ + login: Object.assign(new LoginView(), { + fido2Credentials: [], + }), + }); fixture.detectChanges(); diff --git a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts index 2296932aca3..92def705852 100644 --- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts @@ -139,11 +139,13 @@ export class LoginDetailsSectionComponent implements OnInit { }); } - async ngOnInit() { - if (this.cipherFormContainer.originalCipherView?.login) { - this.initFromExistingCipher(this.cipherFormContainer.originalCipherView.login); + ngOnInit() { + const prefillCipher = this.cipherFormContainer.getInitialCipherView(); + + if (prefillCipher) { + this.initFromExistingCipher(prefillCipher.login); } else { - await this.initNewCipher(); + this.initNewCipher(); } if (this.cipherFormContainer.config.mode === "partial-edit") { @@ -166,7 +168,7 @@ export class LoginDetailsSectionComponent implements OnInit { } } - private async initNewCipher() { + private initNewCipher() { this.loginDetailsForm.patchValue({ username: this.initialValues?.username || "", password: this.initialValues?.password || "", diff --git a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.spec.ts b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.spec.ts new file mode 100644 index 00000000000..6236e2d3dac --- /dev/null +++ b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.spec.ts @@ -0,0 +1,97 @@ +import { signal } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; + +import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +import { CipherFormCacheService } from "./default-cipher-form-cache.service"; + +describe("CipherFormCacheService", () => { + let service: CipherFormCacheService; + let testBed: TestBed; + const cacheSignal = signal(null); + const getCacheSignal = jest.fn().mockReturnValue(cacheSignal); + const getFeatureFlag = jest.fn().mockResolvedValue(false); + const cacheSetMock = jest.spyOn(cacheSignal, "set"); + + beforeEach(() => { + getCacheSignal.mockClear(); + getFeatureFlag.mockClear(); + cacheSetMock.mockClear(); + + testBed = TestBed.configureTestingModule({ + providers: [ + { provide: ViewCacheService, useValue: { signal: getCacheSignal } }, + { provide: ConfigService, useValue: { getFeatureFlag } }, + CipherFormCacheService, + ], + }); + }); + + describe("feature enabled", () => { + beforeEach(async () => { + getFeatureFlag.mockResolvedValue(true); + }); + + it("`getCachedCipherView` returns the cipher", async () => { + cacheSignal.set({ id: "cipher-4" } as CipherView); + service = testBed.inject(CipherFormCacheService); + await service.init(); + + expect(service.getCachedCipherView()).toEqual({ id: "cipher-4" }); + }); + + it("updates the signal value", async () => { + service = testBed.inject(CipherFormCacheService); + await service.init(); + + service.cacheCipherView({ id: "cipher-5" } as CipherView); + + expect(cacheSignal.set).toHaveBeenCalledWith({ id: "cipher-5" }); + }); + + describe("initializedWithValue", () => { + it("sets `initializedWithValue` to true when there is a cached cipher", async () => { + cacheSignal.set({ id: "cipher-3" } as CipherView); + service = testBed.inject(CipherFormCacheService); + await service.init(); + + expect(service.initializedWithValue).toBe(true); + }); + + it("sets `initializedWithValue` to false when there is not a cached cipher", async () => { + cacheSignal.set(null); + service = testBed.inject(CipherFormCacheService); + await service.init(); + + expect(service.initializedWithValue).toBe(false); + }); + }); + }); + + describe("featured disabled", () => { + beforeEach(async () => { + cacheSignal.set({ id: "cipher-1" } as CipherView); + getFeatureFlag.mockResolvedValue(false); + cacheSetMock.mockClear(); + + service = testBed.inject(CipherFormCacheService); + await service.init(); + }); + + it("sets `initializedWithValue` to false", () => { + expect(service.initializedWithValue).toBe(false); + }); + + it("`getCachedCipherView` returns null", () => { + expect(service.getCachedCipherView()).toBeNull(); + }); + + it("does not update the signal value", () => { + service.cacheCipherView({ id: "cipher-2" } as CipherView); + + expect(cacheSignal.set).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts new file mode 100644 index 00000000000..268b2db306b --- /dev/null +++ b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts @@ -0,0 +1,73 @@ +import { inject, Injectable } from "@angular/core"; + +import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +const CIPHER_FORM_CACHE_KEY = "cipher-form-cache"; + +@Injectable() +export class CipherFormCacheService { + private viewCacheService: ViewCacheService = inject(ViewCacheService); + private configService: ConfigService = inject(ConfigService); + + /** True when the `PM9111ExtensionPersistAddEditForm` flag is enabled */ + private featureEnabled: boolean = false; + + /** + * When true the `CipherFormCacheService` a cipher was stored in cache when the service was initialized. + * Otherwise false, when the cache was empty. + * + * This is helpful to know the initial state of the cache as it can be populated quickly after initialization. + */ + initializedWithValue: boolean; + + private cipherCache = this.viewCacheService.signal({ + key: CIPHER_FORM_CACHE_KEY, + initialValue: null, + deserializer: CipherView.fromJSON, + }); + + constructor() { + this.initializedWithValue = !!this.cipherCache(); + } + + /** + * Must be called once before interacting with the cached cipher, otherwise methods will be noop. + */ + async init() { + this.featureEnabled = await this.configService.getFeatureFlag( + FeatureFlag.PM9111ExtensionPersistAddEditForm, + ); + + if (!this.featureEnabled) { + this.initializedWithValue = false; + } + } + + /** + * Update the cache with the new CipherView. + */ + cacheCipherView(cipherView: CipherView): void { + if (!this.featureEnabled) { + return; + } + + // Create a new shallow reference to force the signal to update + // By default, signals use `Object.is` to determine equality + // Docs: https://angular.dev/guide/signals#signal-equality-functions + this.cipherCache.set({ ...cipherView } as CipherView); + } + + /** + * Returns the cached CipherView when available. + */ + getCachedCipherView(): CipherView | null { + if (!this.featureEnabled) { + return null; + } + + return this.cipherCache(); + } +}