Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2025-02-01 23:01:28 +01:00
Code Issues Projects Releases Wiki Activity

[PM-9111] Extension: persist add/edit form (#12236)

Browse Source 
* remove todo

* Retrieve cache cipher for add-edit form

* user prefilled cipher for add-edit form

* add listener for clearing view cache

* clear local cache when clearing global state

* track initial value of cache for down stream logic that should only occur on non-cached values

* add feature flag for edit form persistence

* add tests for cipher form cache service

* fix optional initialValues

* add services to cipher form storybook

* fix strict types

* rename variables to be platform agnostic

* use deconstructed collectionIds variable to avoid them be overwritten

* use the originalCipherView for initial values

* add comment about signal equality

* prevent events from being emitted when adding uris to the existing form

- This stops other values from being overwrote in the initialization process

* add check for cached cipher when adding initial uris
This commit is contained in:
Nick Krantz 2025-01-22 10:49:07 -06:00 committed by GitHub
parent 1dfae06856
commit 5c32e5020d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 460 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts
View File

@ -133,6 +133,7 @@ export class PopupViewCacheService implements ViewCacheService {
}
private clearState() {
this._cache = {}; // clear local cache
this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, {});
}
}

4
apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts
View File

@ -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 () => {

5
apps/browser/src/platform/services/popup-view-cache-background.service.ts
View File

@ -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(

2
libs/common/src/enums/feature-flag.enum.ts
View File

@ -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<FeatureFlag, AllowedFeatureFlagTypes>;

9
libs/vault/src/cipher-form/cipher-form-container.ts
View File

@ -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;
}

26
libs/vault/src/cipher-form/cipher-form.stories.ts
View File

@ -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(

13
libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts
View File

@ -27,9 +27,12 @@ describe("AdditionalOptionsSectionComponent", () => {
let passwordRepromptService: MockProxy<PasswordRepromptService>;
let passwordRepromptEnabled$: BehaviorSubject<boolean>;
beforeEach(async () => {
cipherFormProvider = mock<CipherFormContainer>();
const getInitialCipherView = jest.fn(() => null);
beforeEach(async () => {
getInitialCipherView.mockClear();
cipherFormProvider = mock<CipherFormContainer>({ getInitialCipherView });
passwordRepromptService = mock<PasswordRepromptService>();
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();

9
libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts
View File

@ -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,
});
}

22
libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.spec.ts
View File

@ -25,9 +25,11 @@ describe("AutofillOptionsComponent", () => {
let domainSettingsService: MockProxy<DomainSettingsService>;
let autofillSettingsService: MockProxy<AutofillSettingsServiceAbstraction>;
let platformUtilsService: MockProxy<PlatformUtilsService>;
const getInitialCipherView = jest.fn(() => null);
beforeEach(async () => {
cipherFormContainer = mock<CipherFormContainer>();
getInitialCipherView.mockClear();
cipherFormContainer = mock<CipherFormContainer>({ getInitialCipherView });
liveAnnouncer = mock<LiveAnnouncer>();
platformUtilsService = mock<PlatformUtilsService>();
domainSettingsService = mock<DomainSettingsService>();
@ -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([

38
libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts
View File

@ -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();

10
libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.spec.ts
View File

@ -20,8 +20,10 @@ describe("CardDetailsSectionComponent", () => {
let registerChildFormSpy: jest.SpyInstance;
let patchCipherSpy: jest.SpyInstance;
const getInitialCipherView = jest.fn(() => null);
beforeEach(async () => {
cipherFormProvider = mock<CipherFormContainer>();
cipherFormProvider = mock<CipherFormContainer>({ 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();

10
libs/vault/src/cipher-form/components/card-details-section/card-details-section.component.ts
View File

@ -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,

49
libs/vault/src/cipher-form/components/cipher-form.component.ts
View File

@ -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<CipherForm>({});
@ -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,
) {}
/**

8
libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts
View File

@ -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,

6
libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts
View File

@ -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) {

11
libs/vault/src/cipher-form/components/identity/identity.component.ts
View File

@ -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,

110
libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts
View File

@ -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<I18nService>;
const activeAccount$ = new BehaviorSubject<{ email: string }>({ email: "test@example.com" });
const getInitialCipherView = jest.fn(() => null);
const initializedWithCachedCipher = jest.fn(() => false);
beforeEach(async () => {
cipherFormProvider = mock<CipherFormContainer>();
getInitialCipherView.mockClear();
initializedWithCachedCipher.mockClear();
cipherFormProvider = mock<CipherFormContainer>({
getInitialCipherView,
initializedWithCachedCipher,
});
i18nService = mock<I18nService>();
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];
});

39
libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts
View File

@ -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();

45
libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts
View File

@ -45,9 +45,11 @@ describe("LoginDetailsSectionComponent", () => {
let configService: MockProxy<ConfigService>;
const collect = jest.fn().mockResolvedValue(null);
const getInitialCipherView = jest.fn(() => null);
beforeEach(async () => {
cipherFormContainer = mock<CipherFormContainer>();
getInitialCipherView.mockClear();
cipherFormContainer = mock<CipherFormContainer>({ getInitialCipherView });
generationService = mock<CipherFormGenerationService>();
auditService = mock<AuditService>();
@ -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();

12
libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts
View File

@ -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 || "",

97
libs/vault/src/cipher-form/services/default-cipher-form-cache.service.spec.ts Normal file
View File

@ -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<CipherView | null>(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();
});
});
});

73
libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts Normal file
View File

@ -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<CipherView | null>({
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();
}
}