diff --git a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html index 6345c3ea4e1..2650345e94b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/attachments/open-attachments/open-attachments.component.html @@ -1,5 +1,10 @@ - @@ -104,6 +105,7 @@ [label]="'reorderToggleButton' | i18n: field.value.name" (keydown)="handleKeyDown($event, field.value.name, i)" data-testid="reorder-toggle-button" + [disabled]="parentFormDisabled" *ngIf="canEdit(field.value.type)" > @@ -113,7 +115,8 @@ bitLink linkType="primary" (click)="openAddEditCustomFieldDialog()" - *ngIf="!isPartialEdit" + data-testid="add-field-button" + *ngIf="!isPartialEdit && !parentFormDisabled" > {{ "addField" | i18n }} 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 ced8763f895..1d1bcfa1ee0 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 @@ -4,6 +4,7 @@ import { DebugElement } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { mock } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -16,7 +17,12 @@ import { } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; -import { DialogRef, BitPasswordInputToggleDirective, DialogService } from "@bitwarden/components"; +import { + DialogRef, + BitPasswordInputToggleDirective, + DialogService, + BitIconButtonComponent, +} from "@bitwarden/components"; import { CipherFormConfig } from "../../abstractions/cipher-form-config.service"; import { CipherFormContainer } from "../../cipher-form-container"; @@ -39,6 +45,7 @@ describe("CustomFieldsComponent", () => { let announce: jest.Mock; let patchCipher: jest.Mock; let config: CipherFormConfig; + const formStatusChange$ = new BehaviorSubject<"disabled" | "enabled">("enabled"); beforeEach(async () => { open = jest.fn(); @@ -65,6 +72,7 @@ describe("CustomFieldsComponent", () => { registerChildForm: jest.fn(), config, getInitialCipherView: jest.fn(() => originalCipherView), + formStatusChange$, }, }, { @@ -552,4 +560,54 @@ describe("CustomFieldsComponent", () => { expect(editButtons).toHaveLength(3); }); }); + + describe("parent form disabled", () => { + beforeEach(() => { + originalCipherView!.fields = mockFieldViews; + formStatusChange$.next("disabled"); + component.ngOnInit(); + + fixture.detectChanges(); + }); + + afterEach(() => { + formStatusChange$.next("enabled"); + fixture.detectChanges(); + }); + + it("disables edit and reorder buttons", () => { + const reorderButtonQuery = By.directive(BitIconButtonComponent); + const editButtonQuery = By.directive(BitIconButtonComponent); + + let reorderButton = fixture.debugElement.query(reorderButtonQuery); + let editButton = fixture.debugElement.query(editButtonQuery); + + expect(reorderButton.componentInstance.disabled()).toBe(true); + expect(editButton.componentInstance.disabled()).toBe(true); + + formStatusChange$.next("enabled"); + fixture.detectChanges(); + + reorderButton = fixture.debugElement.query(reorderButtonQuery); + editButton = fixture.debugElement.query(editButtonQuery); + + expect(reorderButton.componentInstance.disabled()).toBe(false); + expect(editButton.componentInstance.disabled()).toBe(false); + }); + + it("hides add field button", () => { + const query = By.css('button[data-testid="add-field-button"]'); + + let addFieldButton = fixture.debugElement.query(query); + + expect(addFieldButton).toBeNull(); + + formStatusChange$.next("enabled"); + fixture.detectChanges(); + + addFieldButton = fixture.debugElement.query(query); + + expect(addFieldButton).not.toBeNull(); + }); + }); }); 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 c8edba6c9fd..e3612e75a1b 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 @@ -113,6 +113,9 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit { /** Emits when a new custom field should be focused */ private focusOnNewInput$ = new Subject(); + /** Tracks the disabled status of the edit cipher form */ + protected parentFormDisabled: boolean = false; + disallowHiddenField?: boolean; destroyed$: DestroyRef; @@ -133,6 +136,10 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit { // getRawValue ensures disabled fields are included this.updateCipher(this.fields.getRawValue()); }); + + this.cipherFormContainer.formStatusChange$.pipe(takeUntilDestroyed()).subscribe((status) => { + this.parentFormDisabled = status === "disabled"; + }); } /** Fields form array, referenced via a getter to avoid type-casting in multiple places */ diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index fc208bd9b92..9bf6dc32758 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -11,6 +11,7 @@ [attr.aria-checked]="itemDetailsForm.value.favorite" [label]="'favorite' | i18n" (click)="toggleFavorite()" + [disabled]="favoriteButtonDisabled" > 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 4fd999ae601..bc5e7c43d12 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 @@ -82,6 +82,8 @@ export class ItemDetailsSectionComponent implements OnInit { protected userId: UserId; + protected favoriteButtonDisabled = false; + @Input({ required: true }) config: CipherFormConfig; @@ -241,15 +243,19 @@ export class ItemDetailsSectionComponent implements OnInit { /** * When the cipher does not belong to an organization but the user's organization * requires all ciphers to be owned by an organization, disable the entire form - * until the user selects an organization. + * until the user selects an organization. Once the organization is set, enable the form. + * Ensure to properly set the collections control state when the form is enabled. */ private setFormState() { if (this.config.originalCipher && !this.allowPersonalOwnership) { if (this.itemDetailsForm.controls.organizationId.value === null) { this.cipherFormContainer.disableFormFields(); this.itemDetailsForm.controls.organizationId.enable(); + this.favoriteButtonDisabled = true; } else { this.cipherFormContainer.enableFormFields(); + this.favoriteButtonDisabled = false; + this.setCollectionControlState(); } } } @@ -305,7 +311,6 @@ export class ItemDetailsSectionComponent implements OnInit { }); const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; - const organization = this.organizations.find((o) => o.id === orgId); const initializedWithCachedCipher = this.cipherFormContainer.initializedWithCachedCipher(); // Configure form for clone mode. @@ -327,9 +332,7 @@ export class ItemDetailsSectionComponent implements OnInit { await this.updateCollectionOptions(prefillCollections); - if (!organization?.canEditAllCiphers && !prefillCipher.canAssignToCollections) { - this.itemDetailsForm.controls.collectionIds.disable(); - } + this.setCollectionControlState(); if (this.partialEdit) { this.itemDetailsForm.disable(); @@ -344,22 +347,34 @@ export class ItemDetailsSectionComponent implements OnInit { c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId), ); - - // When Owners/Admins access setting is turned on. - // Disable Collections Options if Owner/Admin does not have Edit/Manage permissions on item - // Disable Collections Options if Custom user does not have Edit/Manage permissions on item - if ( - (organization?.allowAdminAccessToAllCollectionItems && - (!this.originalCipherView.viewPassword || !this.originalCipherView.edit)) || - (organization?.type === OrganizationUserType.Custom && - !this.originalCipherView.viewPassword) - ) { - this.itemDetailsForm.controls.collectionIds.disable(); - } } } } + private setCollectionControlState() { + const initialCipherView = this.cipherFormContainer.getInitialCipherView(); + const orgId = this.itemDetailsForm.controls.organizationId.value as OrganizationId; + const organization = this.organizations.find((o) => o.id === orgId); + if (!organization || !initialCipherView) { + return; + } + // Disable the collection control if either of the following apply: + // 1. The organization does not allow editing all ciphers and the existing cipher cannot be assigned to + // collections + // 2. When Owners/Admins access setting is turned on. + // AND either: + // a. Disable Collections Options if Owner/Admin does not have Edit/Manage permissions on item + // b. Disable Collections Options if Custom user does not have Edit/Manage permissions on item + if ( + (!organization.canEditAllCiphers && !initialCipherView.canAssignToCollections) || + (organization.allowAdminAccessToAllCollectionItems && + (!initialCipherView.viewPassword || !initialCipherView.edit)) || + (organization.type === OrganizationUserType.Custom && !initialCipherView.viewPassword) + ) { + this.itemDetailsForm.controls.collectionIds.disable(); + } + } + /** * Updates the collection options based on the selected organization. * @param startingSelection - Optional starting selection of collectionIds to be automatically selected. 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 c5b1fc7897b..b07a50fd383 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 @@ -3,6 +3,7 @@ import { Component } from "@angular/core"; import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -47,6 +48,7 @@ describe("LoginDetailsSectionComponent", () => { getInitialCipherView.mockClear(); cipherFormContainer = mock({ getInitialCipherView, + formStatusChange$: new BehaviorSubject<"enabled" | "disabled">("enabled"), website: "example.com", }); 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 e74d9915cdb..061a8c4abf4 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 @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { DatePipe, NgIf } from "@angular/common"; -import { Component, inject, OnInit, Optional } from "@angular/core"; +import { Component, DestroyRef, inject, OnInit, Optional } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, ReactiveFormsModule } from "@angular/forms"; import { map } from "rxjs"; @@ -81,6 +81,8 @@ export class LoginDetailsSectionComponent implements OnInit { */ private existingFido2Credentials?: Fido2CredentialView[]; + private destroyRef = inject(DestroyRef); + get hasPasskey(): boolean { return this.existingFido2Credentials != null && this.existingFido2Credentials.length > 0; } @@ -148,6 +150,19 @@ export class LoginDetailsSectionComponent implements OnInit { if (this.cipherFormContainer.config.mode === "partial-edit") { this.loginDetailsForm.disable(); } + + // If the form is enabled, ensure to disable password or TOTP + // for hidden password users + this.cipherFormContainer.formStatusChange$ + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((status) => { + if (status === "enabled") { + if (!this.viewHiddenFields) { + this.loginDetailsForm.controls.password.disable(); + this.loginDetailsForm.controls.totp.disable(); + } + } + }); } private initFromExistingCipher(existingLogin: LoginView) { diff --git a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts index f83f93267c9..f92c4420d03 100644 --- a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts +++ b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts @@ -98,9 +98,13 @@ export class SshKeySectionComponent implements OnInit { // Disable the form if the cipher form container is enabled // to prevent user interaction - this.cipherFormContainer.formEnabled$ + this.cipherFormContainer.formStatusChange$ .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => this.sshKeyForm.disable()); + .subscribe((status) => { + if (status === "enabled") { + this.sshKeyForm.disable(); + } + }); } /** Set form initial form values from the current cipher */