mirror of
https://github.com/bitwarden/browser
synced 2025-12-11 22:03:36 +00:00
[PM-19357] - [Defect] Unauthorised access allows limited access user to change custom hidden field of Items (#14068)
* update tests * finish tests * only disallow hidden fields for hiddenPassword users * fix failing tests * fix story * only disable hidden field option when editing
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
// FIXME: Update this file to be type safe and remove this and next line
|
// FIXME: Update this file to be type safe and remove this and next line
|
||||||
// @ts-strict-ignore
|
// @ts-strict-ignore
|
||||||
import { importProvidersFrom, signal } from "@angular/core";
|
import { importProvidersFrom, signal } from "@angular/core";
|
||||||
|
import { ActivatedRoute } from "@angular/router";
|
||||||
import { action } from "@storybook/addon-actions";
|
import { action } from "@storybook/addon-actions";
|
||||||
import {
|
import {
|
||||||
applicationConfig,
|
applicationConfig,
|
||||||
@@ -225,6 +226,14 @@ export default {
|
|||||||
getFeatureFlag: () => Promise.resolve(false),
|
getFeatureFlag: () => Promise.resolve(false),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
provide: ActivatedRoute,
|
||||||
|
useValue: {
|
||||||
|
snapshot: {
|
||||||
|
queryParams: {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
],
|
],
|
||||||
}),
|
}),
|
||||||
componentWrapperDecorator(
|
componentWrapperDecorator(
|
||||||
|
|||||||
@@ -83,4 +83,24 @@ describe("AddEditCustomFieldDialogComponent", () => {
|
|||||||
expect.objectContaining({ value: FieldType.Linked }),
|
expect.objectContaining({ value: FieldType.Linked }),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not filter out 'Hidden' field type when 'disallowHiddenField' is false", () => {
|
||||||
|
dialogData.disallowHiddenField = false;
|
||||||
|
fixture = TestBed.createComponent(AddEditCustomFieldDialogComponent);
|
||||||
|
component = fixture.componentInstance;
|
||||||
|
|
||||||
|
expect(component.fieldTypeOptions).toContainEqual(
|
||||||
|
expect.objectContaining({ value: FieldType.Hidden }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("filers out 'Hidden' field type when 'disallowHiddenField' is true", () => {
|
||||||
|
dialogData.disallowHiddenField = true;
|
||||||
|
fixture = TestBed.createComponent(AddEditCustomFieldDialogComponent);
|
||||||
|
component = fixture.componentInstance;
|
||||||
|
|
||||||
|
expect(component.fieldTypeOptions).not.toContainEqual(
|
||||||
|
expect.objectContaining({ value: FieldType.Hidden }),
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ export type AddEditCustomFieldDialogData = {
|
|||||||
cipherType: CipherType;
|
cipherType: CipherType;
|
||||||
/** When provided, dialog will display edit label variants */
|
/** When provided, dialog will display edit label variants */
|
||||||
editLabelConfig?: { index: number; label: string };
|
editLabelConfig?: { index: number; label: string };
|
||||||
|
disallowHiddenField?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
@Component({
|
@Component({
|
||||||
@@ -68,6 +69,9 @@ export class AddEditCustomFieldDialogComponent {
|
|||||||
this.variant = data.editLabelConfig ? "edit" : "add";
|
this.variant = data.editLabelConfig ? "edit" : "add";
|
||||||
|
|
||||||
this.fieldTypeOptions = this.fieldTypeOptions.filter((option) => {
|
this.fieldTypeOptions = this.fieldTypeOptions.filter((option) => {
|
||||||
|
if (this.data.disallowHiddenField && option.value === FieldType.Hidden) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
// Filter out the Linked field type for Secure Notes
|
// Filter out the Linked field type for Secure Notes
|
||||||
if (this.data.cipherType === CipherType.SecureNote) {
|
if (this.data.cipherType === CipherType.SecureNote) {
|
||||||
return option.value !== FieldType.Linked;
|
return option.value !== FieldType.Linked;
|
||||||
|
|||||||
@@ -89,7 +89,7 @@
|
|||||||
bitIconButton="bwi-pencil-square"
|
bitIconButton="bwi-pencil-square"
|
||||||
class="tw-self-center tw-mt-2"
|
class="tw-self-center tw-mt-2"
|
||||||
data-testid="edit-custom-field-button"
|
data-testid="edit-custom-field-button"
|
||||||
*ngIf="!isPartialEdit"
|
*ngIf="canEdit(field.value.type)"
|
||||||
></button>
|
></button>
|
||||||
|
|
||||||
<button
|
<button
|
||||||
@@ -100,7 +100,7 @@
|
|||||||
[appA11yTitle]="'reorderToggleButton' | i18n: field.value.name"
|
[appA11yTitle]="'reorderToggleButton' | i18n: field.value.name"
|
||||||
(keydown)="handleKeyDown($event, field.value.name, i)"
|
(keydown)="handleKeyDown($event, field.value.name, i)"
|
||||||
data-testid="reorder-toggle-button"
|
data-testid="reorder-toggle-button"
|
||||||
*ngIf="!isPartialEdit"
|
*ngIf="canEdit(field.value.type)"
|
||||||
></button>
|
></button>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -45,7 +45,9 @@ describe("CustomFieldsComponent", () => {
|
|||||||
announce = jest.fn().mockResolvedValue(null);
|
announce = jest.fn().mockResolvedValue(null);
|
||||||
patchCipher = jest.fn();
|
patchCipher = jest.fn();
|
||||||
originalCipherView = new CipherView();
|
originalCipherView = new CipherView();
|
||||||
config = {} as CipherFormConfig;
|
config = {
|
||||||
|
collections: [],
|
||||||
|
} as CipherFormConfig;
|
||||||
|
|
||||||
await TestBed.configureTestingModule({
|
await TestBed.configureTestingModule({
|
||||||
imports: [CustomFieldsComponent],
|
imports: [CustomFieldsComponent],
|
||||||
@@ -463,5 +465,91 @@ describe("CustomFieldsComponent", () => {
|
|||||||
// "reorder boolean label to position 4 of 4"
|
// "reorder boolean label to position 4 of 4"
|
||||||
expect(announce).toHaveBeenCalledWith("reorderFieldDown boolean label 4 4", "assertive");
|
expect(announce).toHaveBeenCalledWith("reorderFieldDown boolean label 4 4", "assertive");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("hides reorder buttons when in partial edit mode", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
config.mode = "partial-edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
toggleItems = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="reorder-toggle-button"]'),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(toggleItems).toHaveLength(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows all reorders button when in edit mode and viewPassword is true", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
originalCipherView.viewPassword = true;
|
||||||
|
config.mode = "edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const toggleItems = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="reorder-toggle-button"]'),
|
||||||
|
);
|
||||||
|
expect(toggleItems).toHaveLength(4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows all reorder buttons except for hidden fields when in edit mode and viewPassword is false", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
originalCipherView.viewPassword = false;
|
||||||
|
config.mode = "edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const toggleItems = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="reorder-toggle-button"]'),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(toggleItems).toHaveLength(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("edit button", () => {
|
||||||
|
it("hides the edit button when in partial-edit mode", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
config.mode = "partial-edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const editButtons = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="edit-custom-field-button"]'),
|
||||||
|
);
|
||||||
|
expect(editButtons).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows all the edit buttons when in edit mode and viewPassword is true", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
originalCipherView.viewPassword = true;
|
||||||
|
config.mode = "edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const editButtons = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="edit-custom-field-button"]'),
|
||||||
|
);
|
||||||
|
expect(editButtons).toHaveLength(4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows all the edit buttons except for hidden fields when in edit mode and viewPassword is false", () => {
|
||||||
|
originalCipherView.fields = mockFieldViews;
|
||||||
|
originalCipherView.viewPassword = false;
|
||||||
|
config.mode = "edit";
|
||||||
|
|
||||||
|
component.ngOnInit();
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const editButtons = fixture.debugElement.queryAll(
|
||||||
|
By.css('button[data-testid="edit-custom-field-button"]'),
|
||||||
|
);
|
||||||
|
expect(editButtons).toHaveLength(3);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -116,6 +116,8 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit {
|
|||||||
/** Emits when a new custom field should be focused */
|
/** Emits when a new custom field should be focused */
|
||||||
private focusOnNewInput$ = new Subject<void>();
|
private focusOnNewInput$ = new Subject<void>();
|
||||||
|
|
||||||
|
disallowHiddenField?: boolean;
|
||||||
|
|
||||||
destroyed$: DestroyRef;
|
destroyed$: DestroyRef;
|
||||||
FieldType = FieldType;
|
FieldType = FieldType;
|
||||||
|
|
||||||
@@ -141,6 +143,13 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit {
|
|||||||
return this.customFieldsForm.controls.fields as FormArray;
|
return this.customFieldsForm.controls.fields as FormArray;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
canEdit(type: FieldType): boolean {
|
||||||
|
return (
|
||||||
|
!this.isPartialEdit &&
|
||||||
|
(type !== FieldType.Hidden || this.cipherFormContainer.originalCipherView?.viewPassword)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
ngOnInit() {
|
ngOnInit() {
|
||||||
const linkedFieldsOptionsForCipher = this.getLinkedFieldsOptionsForCipher();
|
const linkedFieldsOptionsForCipher = this.getLinkedFieldsOptionsForCipher();
|
||||||
const optionsArray = Array.from(linkedFieldsOptionsForCipher?.entries() ?? []);
|
const optionsArray = Array.from(linkedFieldsOptionsForCipher?.entries() ?? []);
|
||||||
@@ -210,6 +219,7 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit {
|
|||||||
|
|
||||||
/** Opens the add/edit custom field dialog */
|
/** Opens the add/edit custom field dialog */
|
||||||
openAddEditCustomFieldDialog(editLabelConfig?: AddEditCustomFieldDialogData["editLabelConfig"]) {
|
openAddEditCustomFieldDialog(editLabelConfig?: AddEditCustomFieldDialogData["editLabelConfig"]) {
|
||||||
|
const { cipherType, mode, originalCipher } = this.cipherFormContainer.config;
|
||||||
this.dialogRef = this.dialogService.open<unknown, AddEditCustomFieldDialogData>(
|
this.dialogRef = this.dialogService.open<unknown, AddEditCustomFieldDialogData>(
|
||||||
AddEditCustomFieldDialogComponent,
|
AddEditCustomFieldDialogComponent,
|
||||||
{
|
{
|
||||||
@@ -217,8 +227,9 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit {
|
|||||||
addField: this.addField.bind(this),
|
addField: this.addField.bind(this),
|
||||||
updateLabel: this.updateLabel.bind(this),
|
updateLabel: this.updateLabel.bind(this),
|
||||||
removeField: this.removeField.bind(this),
|
removeField: this.removeField.bind(this),
|
||||||
cipherType: this.cipherFormContainer.config.cipherType,
|
cipherType,
|
||||||
editLabelConfig,
|
editLabelConfig,
|
||||||
|
disallowHiddenField: mode === "edit" && !originalCipher.viewPassword,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in New Issue
Block a user