Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(permissions): filter fields in forms based on read/write permissions #2180

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
420e45f
feat(permissions): filter fields in forms based on read/write permiss…
tomwwinter Jan 15, 2024
0e627bc
fix: reset demo-permission-generator
tomwwinter Jan 15, 2024
ab8a9d6
refactor: disable-wrapper component for better Cognitive Complexity s…
tomwwinter Jan 15, 2024
efa5e04
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 15, 2024
020d3bb
fix: use correct import path in ability.service.ts
tomwwinter Jan 15, 2024
e5301eb
refactor: move filter logic to entity-form component/service
tomwwinter Jan 15, 2024
38f8c81
fix: typo
tomwwinter Jan 15, 2024
41d779c
test: improve test coverage
tomwwinter Jan 15, 2024
ed6181e
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 15, 2024
5248fe0
fix: test syntax
tomwwinter Jan 16, 2024
f99b8d0
fix: test syntax
tomwwinter Jan 16, 2024
754bf3e
fix: pr review feedback
tomwwinter Jan 16, 2024
cf79872
fix: typo
tomwwinter Jan 16, 2024
b1feafc
fix: typo
tomwwinter Jan 16, 2024
c2290c8
fix: pr review feedback
tomwwinter Jan 16, 2024
6e5a4f2
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 18, 2024
b692321
fix: remove unnecessary function call
tomwwinter Jan 18, 2024
8142222
fix: code format
tomwwinter Jan 18, 2024
dd2f5ff
fix: code format
tomwwinter Jan 18, 2024
4988f07
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
TheSlimvReal Jan 22, 2024
baecf55
disabled check also happens on form initialization
TheSlimvReal Jan 22, 2024
14e58a9
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
tomwwinter Jan 24, 2024
429428d
fix: use create permissions when form creates new entity
tomwwinter Jan 24, 2024
28d6bdb
fix: use create permissions when form creates new entity
tomwwinter Jan 24, 2024
587ff54
removed unnecessary variable
TheSlimvReal Jan 25, 2024
017a45b
Merge branch 'master' into tw/feat/1912-filter-fields-if-user-has-no-…
TheSlimvReal Jan 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/app.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ describe("AppComponent", () => {
fixture.detectChanges();
}));

afterEach(() => {
afterEach(waitForAsync(() => {
environment.demo_mode = false;
jasmine.DEFAULT_TIMEOUT_INTERVAL = intervalBefore;
return TestBed.inject(Database).destroy();
});
}));

it("should be created", () => {
expect(component).toBeTruthy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,46 @@ describe("EntityFormService", () => {
expect(formGroup.valid).toBeTrue();
});

it("should use create permissions to disable fields when creating a new entity", () => {
const formFields = [{ id: "name" }, { id: "dateOfBirth" }];
TestBed.inject(EntityAbility).update([
{ subject: "Child", action: "read", fields: ["name", "dateOfBirth"] },
{ subject: "Child", action: "update", fields: ["name"] },
{ subject: "Child", action: "create", fields: ["dateOfBirth"] },
]);

const formGroup = service.createFormGroup(formFields, new Child());

expect(formGroup.get("name").disabled).toBeTrue();
expect(formGroup.get("dateOfBirth").enabled).toBeTrue();
});

it("should always keep properties disabled if user does not have 'update' permissions for them", () => {
const formFields = [{ id: "name" }, { id: "dateOfBirth" }];
TestBed.inject(EntityAbility).update([
{ subject: "Child", action: "read", fields: ["name", "dateOfBirth"] },
{ subject: "Child", action: "update", fields: ["name"] },
]);

const child = new Child();
child._rev = "foo"; // "not new" state

const formGroup = service.createFormGroup(formFields, child);

expect(formGroup.get("name").enabled).toBeTrue();
expect(formGroup.get("dateOfBirth").disabled).toBeTrue();

formGroup.disable();

expect(formGroup.get("name").disabled).toBeTrue();
expect(formGroup.get("dateOfBirth").disabled).toBeTrue();

formGroup.enable();

expect(formGroup.get("name").enabled).toBeTrue();
expect(formGroup.get("dateOfBirth").disabled).toBeTrue();
});

it("should create a error if form is invalid", () => {
const formFields = [{ id: "schoolId" }, { id: "start" }];
const formGroup = service.createFormGroup(
Expand Down
28 changes: 25 additions & 3 deletions src/app/core/common-components/entity-form/entity-form.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ export class EntityFormService {
* @param formFields
* @param entity
* @param forTable
* @param withPermissionCheck if true, fields without 'update' permissions will stay disabled when enabling form
*/
public createFormGroup<T extends Entity>(
formFields: ColumnConfig[],
entity: T,
forTable = false,
withPermissionCheck = true,
): EntityForm<T> {
const formConfig = {};
const copy = entity.copy();
Expand All @@ -130,10 +132,18 @@ export class EntityFormService {
}
const group = this.fb.group<Partial<T>>(formConfig);

const sub = group.valueChanges.subscribe(
const valueChangesSubscription = group.valueChanges.subscribe(
() => (this.unsavedChanges.pending = group.dirty),
);
this.subscriptions.push(sub);
this.subscriptions.push(valueChangesSubscription);

if (withPermissionCheck) {
this.disableReadOnlyFormControls(group, entity);
const statusChangesSubscription = group.statusChanges
.pipe(filter((status) => status !== "DISABLED"))
.subscribe(() => this.disableReadOnlyFormControls(group, entity));
this.subscriptions.push(statusChangesSubscription);
}

return group;
}
Expand Down Expand Up @@ -194,6 +204,18 @@ export class EntityFormService {
return newVal;
}

private disableReadOnlyFormControls<T extends Entity>(
form: EntityForm<T>,
entity: T,
) {
const action = entity.isNew ? "create" : "update";
Object.keys(form.controls).forEach((fieldId) => {
if (this.ability.cannot(action, entity, fieldId)) {
form.get(fieldId).disable({ onlySelf: true, emitEvent: false });
}
});
}

/**
* This function applies the changes of the formGroup to the entity.
* If the form is invalid or the entity does not pass validation after applying the changes, an error will be thrown.
Expand Down Expand Up @@ -228,7 +250,7 @@ export class EntityFormService {
}

private checkFormValidity<T extends Entity>(form: EntityForm<T>) {
// errors regarding invalid fields wont be displayed unless marked as touched
// errors regarding invalid fields won't be displayed unless marked as touched
form.markAllAsTouched();
if (form.invalid) {
throw new InvalidFormFieldError();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper
import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service";
import { EntityFormService } from "../entity-form.service";
import { DateWithAge } from "../../../basic-datatypes/date-with-age/dateWithAge";
import { EntityAbility } from "../../../permissions/ability/entity-ability";

describe("EntityFormComponent", () => {
let component: EntityFormComponent<Child>;
Expand Down Expand Up @@ -55,6 +56,25 @@ describe("EntityFormComponent", () => {
expect(component).toBeTruthy();
});

it("should remove fields without read permissions", async () => {
component.fieldGroups = [
{ fields: ["foo", "bar"] },
{ fields: ["name"] },
{ fields: ["birthday"] },
];

TestBed.inject(EntityAbility).update([
{ subject: "Child", action: "read", fields: ["foo", "name"] },
]);

component.ngOnChanges({ entity: true, form: true } as any);

expect(component.fieldGroups).toEqual([
{ fields: ["foo"] },
{ fields: ["name"] },
]);
});

it("should not change anything if changed entity has same values as form", () => {
return expectApplyChangesPopup(
"not-shown",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Subscription } from "rxjs";
import moment from "moment";
import { EntityFieldEditComponent } from "../../entity-field-edit/entity-field-edit.component";
import { FieldGroup } from "../../../entity-details/form/field-group";
import { EntityAbility } from "../../../permissions/ability/entity-ability";

/**
* A general purpose form component for displaying and editing entities.
Expand Down Expand Up @@ -60,9 +61,17 @@ export class EntityFormComponent<T extends Entity = Entity>
constructor(
private entityMapper: EntityMapperService,
private confirmationDialog: ConfirmationDialogService,
private ability: EntityAbility,
) {}

ngOnChanges(changes: SimpleChanges) {
if (this.fieldGroups) {
this.fieldGroups = this.filterFieldGroupsByPermissions(
this.fieldGroups,
this.entity,
);
}

if (changes.entity && this.entity) {
this.changesSubscription?.unsubscribe();
this.changesSubscription = this.entityMapper
Expand All @@ -74,6 +83,7 @@ export class EntityFormComponent<T extends Entity = Entity>
)
.subscribe(({ entity }) => this.applyChanges(entity));
}

if (changes.form && this.form) {
this.initialFormValues = this.form.getRawValue();
this.disableForLockedEntity();
Expand Down Expand Up @@ -125,6 +135,24 @@ export class EntityFormComponent<T extends Entity = Entity>
);
}

private filterFieldGroupsByPermissions<T extends Entity = Entity>(
fieldGroups: FieldGroup[],
entity: Entity,
): FieldGroup[] {
return fieldGroups
.map((group) => {
group.fields = group.fields.filter((field) =>
this.ability.can(
"read",
entity,
typeof field === "string" ? field : field.id,
),
);
return group;
})
.filter((group) => group.fields.length > 0);
}

private entityEqualsFormValue(entityValue, formValue) {
return (
(entityValue instanceof Date &&
Expand Down
1 change: 1 addition & 0 deletions src/app/core/entity-details/form/form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class FormComponent<E extends Entity> implements FormConfig, OnInit {
[].concat(...this.fieldGroups.map((group) => group.fields)),
this.entity,
);

if (!this.creatingNew) {
this.form.disable();
}
Expand Down
10 changes: 10 additions & 0 deletions src/app/core/permissions/ability/testing-entity-ability-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { EntitySchemaService } from "../../entity/schema/entity-schema.service";
import { EntityAbility } from "./entity-ability";

export const entityAbilityFactory = (
entitySchemaService: EntitySchemaService,
) => {
let ability = new EntityAbility(entitySchemaService);
ability.update([{ subject: "all", action: "manage" }]);
return ability;
};
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ export class DisableEntityOperationDirective
{
/**
* These arguments are required to check whether the user has permissions to perform the operation.
* The operation property defines to what kind of operation a element belongs, e.g. OperationType.CREATE
* The operation property defines to what kind of operation an element belongs, e.g. OperationType.CREATE
* The entity property defines for which kind of entity the operation will be performed, e.g. Child
*/
@Input("appDisabledEntityOperation") arguments: {
operation: EntityAction;
entity: EntitySubject;
field?: string;
};

private wrapperComponent: ComponentRef<DisabledWrapperComponent>;
Expand Down Expand Up @@ -76,6 +77,7 @@ export class DisableEntityOperationDirective
this.wrapperComponent.instance.elementDisabled = this.ability.cannot(
this.arguments.operation,
this.arguments.entity,
this.arguments.field,
);
this.wrapperComponent.instance.ngAfterViewInit();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,31 @@ export class DisabledWrapperComponent implements AfterViewInit {
constructor(private renderer: Renderer2) {}

ngAfterViewInit() {
if (this.wrapper) {
const buttonElement =
this.wrapper.nativeElement.getElementsByTagName("button")[0];
if (this.elementDisabled) {
this.renderer.addClass(buttonElement, "mat-button-disabled");
this.renderer.setAttribute(buttonElement, "disabled", "true");
} else {
this.renderer.removeAttribute(buttonElement, "disabled");
this.renderer.removeClass(buttonElement, "mat-button-disabled");
}
if (!this.wrapper) {
return;
}

const buttonElement =
this.wrapper.nativeElement.getElementsByTagName("button")[0];

if (!buttonElement) {
return;
}

if (this.elementDisabled) {
this.disable(buttonElement);
} else {
this.enable(buttonElement);
}
}

private enable(buttonElement: HTMLButtonElement) {
this.renderer.removeAttribute(buttonElement, "disabled");
this.renderer.removeClass(buttonElement, "mat-button-disabled");
}

private disable(buttonElement: HTMLButtonElement) {
this.renderer.addClass(buttonElement, "mat-button-disabled");
this.renderer.setAttribute(buttonElement, "disabled", "true");
}
}
9 changes: 9 additions & 0 deletions src/app/utils/mocked-testing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import { BehaviorSubject } from "rxjs";
import { CurrentUserSubject } from "../core/session/current-user-subject";
import { SessionInfo, SessionSubject } from "../core/session/auth/session-info";
import { TEST_USER } from "../core/user/demo-user-generator.service";
import { EntityAbility } from "../core/permissions/ability/entity-ability";
import { EntitySchemaService } from "../core/entity/schema/entity-schema.service";
import { entityAbilityFactory } from "app/core/permissions/ability/testing-entity-ability-factory";

/**
* Utility module that can be imported in test files or stories to have mock implementations of the SessionService
Expand Down Expand Up @@ -72,9 +75,15 @@ export class MockedTestingModule {
): ModuleWithProviders<MockedTestingModule> {
environment.session_type = SessionType.mock;
const mockedEntityMapper = mockEntityMapper([...data]);

return {
ngModule: MockedTestingModule,
providers: [
{
provide: EntityAbility,
useFactory: entityAbilityFactory,
deps: [EntitySchemaService],
},
{ provide: EntityMapperService, useValue: mockedEntityMapper },
{ provide: ConfigService, useValue: createTestingConfigService() },
{
Expand Down
Loading