diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts index 95288f6b411..77a50ea35d9 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts @@ -1,14 +1,22 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; import { ActivatedRoute, Router } from "@angular/router"; -import { mock } from "jest-mock-extended"; +import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject } from "rxjs"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CipherType } from "@bitwarden/common/vault/enums"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { CipherFormConfig, CipherFormConfigService, CipherFormMode } from "@bitwarden/vault"; +import { AddEditCipherInfo } from "@bitwarden/common/vault/types/add-edit-cipher-info"; +import { + CipherFormConfig, + CipherFormConfigService, + CipherFormMode, + OptionalInitialValues, +} from "@bitwarden/vault"; import { BrowserFido2UserInterfaceSession } from "../../../../../autofill/fido2/services/browser-fido2-user-interface.service"; import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; @@ -25,6 +33,8 @@ jest.mock("qrcode-parser", () => {}); describe("AddEditV2Component", () => { let component: AddEditV2Component; let fixture: ComponentFixture; + let addEditCipherInfo$: BehaviorSubject; + let cipherServiceMock: MockProxy; const buildConfigResponse = { originalCipher: {} } as CipherFormConfig; const buildConfig = jest.fn((mode: CipherFormMode) => @@ -41,6 +51,10 @@ describe("AddEditV2Component", () => { navigate.mockClear(); back.mockClear(); + addEditCipherInfo$ = new BehaviorSubject(null); + cipherServiceMock = mock(); + cipherServiceMock.addEditCipherInfo$ = addEditCipherInfo$.asObservable(); + await TestBed.configureTestingModule({ imports: [AddEditV2Component], providers: [ @@ -51,6 +65,7 @@ describe("AddEditV2Component", () => { { provide: Router, useValue: { navigate } }, { provide: ActivatedRoute, useValue: { queryParams: queryParams$ } }, { provide: I18nService, useValue: { t: (key: string) => key } }, + { provide: CipherService, useValue: cipherServiceMock }, ], }) .overrideProvider(CipherFormConfigService, { @@ -107,6 +122,72 @@ describe("AddEditV2Component", () => { }); }); + describe("addEditCipherInfo initialization", () => { + it("populates config.initialValues with `addEditCipherInfo` values", fakeAsync(() => { + const addEditCipherInfo = { + cipher: { + name: "test", + folderId: "folder1", + organizationId: "org1", + type: CipherType.Login, + login: { + password: "password", + username: "username", + uris: [{ uri: "https://example.com" }], + }, + }, + collectionIds: ["col1", "col2"], + } as AddEditCipherInfo; + addEditCipherInfo$.next(addEditCipherInfo); + queryParams$.next({}); + + tick(); + + expect(component.config.initialValues).toEqual({ + name: "test", + folderId: "folder1", + organizationId: "org1", + password: "password", + username: "username", + loginUri: "https://example.com", + collectionIds: ["col1", "col2"], + } as OptionalInitialValues); + })); + + it("populates config.initialValues.username when `addEditCipherInfo` is an Identity", fakeAsync(() => { + addEditCipherInfo$.next({ + cipher: { type: CipherType.Identity, identity: { username: "identity-username" } }, + } as AddEditCipherInfo); + queryParams$.next({}); + + tick(); + + expect(component.config.initialValues.username).toBe("identity-username"); + })); + + it("overrides query params with `addEditCipherInfo` values", fakeAsync(() => { + addEditCipherInfo$.next({ + cipher: { name: "AddEditCipherName" }, + } as AddEditCipherInfo); + queryParams$.next({ + name: "QueryParamName", + }); + + tick(); + + expect(component.config.initialValues.name).toBe("AddEditCipherName"); + })); + + it("clears `addEditCipherInfo` after initialization", fakeAsync(() => { + addEditCipherInfo$.next({ cipher: { name: "test" } } as AddEditCipherInfo); + queryParams$.next({}); + + tick(); + + expect(cipherServiceMock.setAddEditCipherInfo).toHaveBeenCalledTimes(1); + })); + }); + describe("onCipherSaved", () => { it("disables warning when in popout", async () => { jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValueOnce(true); diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index b830ae75048..b1e95afb535 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -8,8 +8,10 @@ import { firstValueFrom, map, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherId, CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { AddEditCipherInfo } from "@bitwarden/common/vault/types/add-edit-cipher-info"; import { AsyncActionsModule, ButtonModule, SearchModule } from "@bitwarden/components"; import { CipherFormConfig, @@ -18,6 +20,7 @@ import { CipherFormMode, CipherFormModule, DefaultCipherFormConfigService, + OptionalInitialValues, TotpCaptureService, } from "@bitwarden/vault"; @@ -156,6 +159,7 @@ export class AddEditV2Component implements OnInit { private popupCloseWarningService: PopupCloseWarningService, private popupRouterCacheService: PopupRouterCacheService, private router: Router, + private cipherService: CipherService, ) { this.subscribeToParams(); } @@ -255,7 +259,21 @@ export class AddEditV2Component implements OnInit { config.mode = "partial-edit"; } - this.setInitialValuesFromParams(params, config); + config.initialValues = this.setInitialValuesFromParams(params); + + // The browser notification bar and overlay use addEditCipherInfo$ to pass modified cipher details to the form + // Attempt to fetch them here and overwrite the initialValues if present + const cachedCipherInfo = await firstValueFrom(this.cipherService.addEditCipherInfo$); + + if (cachedCipherInfo != null) { + // Cached cipher info has priority over queryParams + config.initialValues = { + ...config.initialValues, + ...mapAddEditCipherInfoToInitialValues(cachedCipherInfo), + }; + // Be sure to clear the "cached" cipher info, so it doesn't get used again + await this.cipherService.setAddEditCipherInfo(null); + } return config; }), @@ -266,26 +284,27 @@ export class AddEditV2Component implements OnInit { }); } - setInitialValuesFromParams(params: QueryParams, config: CipherFormConfig) { - config.initialValues = {}; + setInitialValuesFromParams(params: QueryParams) { + const initialValues = {} as OptionalInitialValues; if (params.folderId) { - config.initialValues.folderId = params.folderId; + initialValues.folderId = params.folderId; } if (params.organizationId) { - config.initialValues.organizationId = params.organizationId; + initialValues.organizationId = params.organizationId; } if (params.collectionId) { - config.initialValues.collectionIds = [params.collectionId]; + initialValues.collectionIds = [params.collectionId]; } if (params.uri) { - config.initialValues.loginUri = params.uri; + initialValues.loginUri = params.uri; } if (params.username) { - config.initialValues.username = params.username; + initialValues.username = params.username; } if (params.name) { - config.initialValues.name = params.name; + initialValues.name = params.name; } + return initialValues; } setHeader(mode: CipherFormMode, type: CipherType) { @@ -303,3 +322,63 @@ export class AddEditV2Component implements OnInit { } } } + +/** + * Helper to map the old AddEditCipherInfo to the new OptionalInitialValues type used by the CipherForm + * @param cipherInfo + */ +const mapAddEditCipherInfoToInitialValues = ( + cipherInfo: AddEditCipherInfo | null, +): OptionalInitialValues => { + const initialValues: OptionalInitialValues = {}; + + if (cipherInfo == null) { + return initialValues; + } + + if (cipherInfo.collectionIds != null) { + initialValues.collectionIds = cipherInfo.collectionIds as CollectionId[]; + } + + if (cipherInfo.cipher == null) { + return initialValues; + } + + const cipher = cipherInfo.cipher; + + if (cipher.folderId != null) { + initialValues.folderId = cipher.folderId; + } + + if (cipher.organizationId != null) { + initialValues.organizationId = cipher.organizationId as OrganizationId; + } + + if (cipher.name != null) { + initialValues.name = cipher.name; + } + + if (cipher.type === CipherType.Login) { + const login = cipher.login; + + if (login != null) { + if (login.uris != null && login.uris.length > 0) { + initialValues.loginUri = login.uris[0].uri; + } + + if (login.username != null) { + initialValues.username = login.username; + } + + if (login.password != null) { + initialValues.password = login.password; + } + } + } + + if (cipher.type === CipherType.Identity && cipher.identity?.username != null) { + initialValues.username = cipher.identity.username; + } + + return initialValues; +}; diff --git a/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts b/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts index 837c9d11ede..25c0e411243 100644 --- a/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts +++ b/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts @@ -23,6 +23,7 @@ export type OptionalInitialValues = { collectionIds?: CollectionId[]; loginUri?: string; username?: string; + password?: string; name?: string; }; @@ -58,7 +59,8 @@ type BaseCipherFormConfig = { originalCipher?: Cipher; /** - * Optional initial values for the form when creating a new cipher. Useful when creating a cipher in a filtered view. + * Optional initial values for the form when opening the cipher form. + * Useful when creating a new cipher in a filtered view or modifying a cipher with values from another source (e.g. the notification bar in Browser) */ initialValues?: OptionalInitialValues; 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 601380f98a1..e4dba11525b 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 @@ -128,6 +128,47 @@ describe("AutofillOptionsComponent", () => { expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(null); }); + it("initializes 'autoFillOptionsForm' with initialValues when editing an existing cipher", () => { + cipherFormContainer.config.initialValues = { loginUri: "https://new-website.com" }; + const existingLogin = new LoginUriView(); + existingLogin.uri = "https://example.com"; + existingLogin.match = UriMatchStrategy.Exact; + + (cipherFormContainer.originalCipherView as CipherView) = new CipherView(); + cipherFormContainer.originalCipherView.login = { + autofillOnPageLoad: true, + uris: [existingLogin], + } as LoginView; + + fixture.detectChanges(); + + expect(component.autofillOptionsForm.value.uris).toEqual([ + { uri: "https://example.com", matchDetection: UriMatchStrategy.Exact }, + { uri: "https://new-website.com", matchDetection: null }, + ]); + expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(true); + }); + + it("initializes 'autoFillOptionsForm' with initialValues without duplicating an existing URI", () => { + cipherFormContainer.config.initialValues = { loginUri: "https://example.com" }; + const existingLogin = new LoginUriView(); + existingLogin.uri = "https://example.com"; + existingLogin.match = UriMatchStrategy.Exact; + + (cipherFormContainer.originalCipherView as CipherView) = new CipherView(); + cipherFormContainer.originalCipherView.login = { + autofillOnPageLoad: true, + uris: [existingLogin], + } as LoginView; + + fixture.detectChanges(); + + expect(component.autofillOptionsForm.value.uris).toEqual([ + { uri: "https://example.com", matchDetection: UriMatchStrategy.Exact }, + ]); + expect(component.autofillOptionsForm.value.autofillOnPageLoad).toEqual(true); + }); + it("initializes 'autoFillOptionsForm' with an empty URI when creating a new cipher", () => { cipherFormContainer.config.initialValues = null; 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 80de50c4421..eb5767b534f 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 @@ -143,6 +143,20 @@ export class AutofillOptionsComponent implements OnInit { this.autofillOptionsForm.patchValue({ autofillOnPageLoad: existingLogin.autofillOnPageLoad, }); + + if (this.cipherFormContainer.config.initialValues?.loginUri) { + // Avoid adding the same uri again if it already exists + if ( + existingLogin.uris?.findIndex( + (uri) => uri.uri === this.cipherFormContainer.config.initialValues.loginUri, + ) === -1 + ) { + this.addUri({ + uri: this.cipherFormContainer.config.initialValues.loginUri, + matchDetection: null, + }); + } + } } private initNewCipher() { 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 ae712b915b3..d8f938f4ae7 100644 --- a/libs/vault/src/cipher-form/components/identity/identity.component.ts +++ b/libs/vault/src/cipher-form/components/identity/identity.component.ts @@ -127,7 +127,7 @@ export class IdentitySectionComponent implements OnInit { firstName: identity.firstName, middleName: identity.middleName, lastName: identity.lastName, - username: identity.username, + username: this.cipherFormContainer.config.initialValues?.username ?? identity.username, company: identity.company, ssn: identity.ssn, passportNumber: identity.passportNumber, 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 a0a1b4e83f7..d7678aa596a 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 @@ -5,6 +5,7 @@ import { mock, MockProxy } from "jest-mock-extended"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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 { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; @@ -104,6 +105,43 @@ 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" } as CollectionView, + { id: "col2", name: "Collection 2", organizationId: "org1" } 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]; 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 99ecd84cd29..b0716218b59 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 @@ -190,9 +190,9 @@ export class ItemDetailsSectionComponent implements OnInit { private async initFromExistingCipher() { this.itemDetailsForm.setValue({ - name: this.originalCipherView.name, - organizationId: this.originalCipherView.organizationId, - folderId: this.originalCipherView.folderId, + 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, collectionIds: [], favorite: this.originalCipherView.favorite, }); @@ -208,7 +208,10 @@ export class ItemDetailsSectionComponent implements OnInit { } } - await this.updateCollectionOptions(this.originalCipherView.collectionIds as CollectionId[]); + await this.updateCollectionOptions( + this.initialValues?.collectionIds ?? + (this.originalCipherView.collectionIds as CollectionId[]), + ); 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 06f325d0534..f50f8598b94 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 @@ -125,6 +125,29 @@ describe("LoginDetailsSectionComponent", () => { }); }); + it("initializes 'loginDetailsForm' with initialValues that override any original cipher view values", async () => { + (cipherFormContainer.originalCipherView as CipherView) = { + 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(); + + expect(component.loginDetailsForm.value).toEqual({ + username: "new-username", + password: "new-password", + totp: "original-totp", + }); + }); + describe("viewHiddenFields", () => { beforeEach(() => { (cipherFormContainer.originalCipherView as CipherView) = { 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 020c2d18bd8..0186b0820c3 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 @@ -95,6 +95,10 @@ export class LoginDetailsSectionComponent implements OnInit { return true; } + get initialValues() { + return this.cipherFormContainer.config.initialValues; + } + constructor( private cipherFormContainer: CipherFormContainer, private formBuilder: FormBuilder, @@ -139,8 +143,8 @@ export class LoginDetailsSectionComponent implements OnInit { private initFromExistingCipher(existingLogin: LoginView) { this.loginDetailsForm.patchValue({ - username: existingLogin.username, - password: existingLogin.password, + username: this.initialValues?.username ?? existingLogin.username, + password: this.initialValues?.password ?? existingLogin.password, totp: existingLogin.totp, }); @@ -154,8 +158,8 @@ export class LoginDetailsSectionComponent implements OnInit { private async initNewCipher() { this.loginDetailsForm.patchValue({ - username: this.cipherFormContainer.config.initialValues?.username || "", - password: "", + username: this.initialValues?.username || "", + password: this.initialValues?.password || "", }); }