diff --git a/src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.spec.ts b/src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.spec.ts index c6d99aff5a..4251c1772e 100644 --- a/src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.spec.ts +++ b/src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.spec.ts @@ -8,16 +8,25 @@ import { MockedTestingModule } from "../../../../utils/mocked-testing.module"; import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; import { MatDialog } from "@angular/material/dialog"; import { of } from "rxjs"; +import { EntityAbility } from "../../../permissions/ability/entity-ability"; describe("EnumDropdownComponent", () => { let component: EnumDropdownComponent; let fixture: ComponentFixture; let mockDialog: jasmine.SpyObj; + let mockAbility: jasmine.SpyObj; + beforeEach(async () => { mockDialog = jasmine.createSpyObj(["open"]); + mockAbility = jasmine.createSpyObj(["can"]); + mockAbility.can.and.returnValue(true); + await TestBed.configureTestingModule({ imports: [EnumDropdownComponent, MockedTestingModule.withState()], - providers: [{ provide: MatDialog, useValue: mockDialog }], + providers: [ + { provide: MatDialog, useValue: mockDialog }, + { provide: EntityAbility, useValue: mockAbility }, + ], }).compileComponents(); fixture = TestBed.createComponent(EnumDropdownComponent); diff --git a/src/app/core/config/config-fix.ts b/src/app/core/config/config-fix.ts index 8a9c2b99e2..1355efea65 100644 --- a/src/app/core/config/config-fix.ts +++ b/src/app/core/config/config-fix.ts @@ -174,6 +174,7 @@ export const defaultJsonConfig = { "view:note": { "component": "NotesManager", "config": { + "entity": "Note", "title": $localize`:Title for notes overview:Notes & Reports`, "includeEventNotes": false, "showEventNotesToggle": true, @@ -279,6 +280,7 @@ export const defaultJsonConfig = { } ] }, + "requiredPermissionOperation": "update", "permittedUserRoles": ["admin_app"] }, "view:admin": { @@ -411,6 +413,7 @@ export const defaultJsonConfig = { "view:child": { "component": "ChildrenList", "config": { + "entity": "Child", "columns": [ { "viewComponent": "ChildBlock", diff --git a/src/app/core/config/dynamic-routing/router.service.spec.ts b/src/app/core/config/dynamic-routing/router.service.spec.ts index d4b4dcca37..349c7184c7 100644 --- a/src/app/core/config/dynamic-routing/router.service.spec.ts +++ b/src/app/core/config/dynamic-routing/router.service.spec.ts @@ -13,6 +13,7 @@ import { NotFoundComponent } from "./not-found/not-found.component"; import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { AuthGuard } from "../../session/auth.guard"; import { RoutedViewComponent } from "../../ui/routed-view/routed-view.component"; +import { EntityPermissionGuard } from "../../permissions/permission-guard/entity-permission.guard"; class TestComponent extends Component {} @@ -66,20 +67,20 @@ describe("RouterService", () => { component: RoutedViewComponent, data: { component: "ChildrenList" }, canDeactivate: [jasmine.any(Function)], - canActivate: [AuthGuard], + canActivate: [AuthGuard, EntityPermissionGuard], }, { path: "child/:id", component: RoutedViewComponent, data: { component: "EntityDetails", config: testViewConfig }, canDeactivate: [jasmine.any(Function)], - canActivate: [AuthGuard], + canActivate: [AuthGuard, EntityPermissionGuard], }, { path: "list", component: RoutedViewComponent, data: { component: "EntityList", permittedUserRoles: ["user_app"] }, - canActivate: [AuthGuard, UserRoleGuard], + canActivate: [AuthGuard, EntityPermissionGuard, UserRoleGuard], canDeactivate: [jasmine.any(Function)], }, ]; @@ -108,7 +109,7 @@ describe("RouterService", () => { { path: "other", component: TestComponent, - canActivate: [AuthGuard, UserRoleGuard], + canActivate: [AuthGuard, EntityPermissionGuard, UserRoleGuard], canDeactivate: [jasmine.any(Function)], data: { permittedUserRoles: ["admin_app"] }, }, @@ -166,7 +167,7 @@ describe("RouterService", () => { path: "list", component: RoutedViewComponent, data: { component: "EntityList", permittedUserRoles: ["admin"] }, - canActivate: [AuthGuard, UserRoleGuard], + canActivate: [AuthGuard, EntityPermissionGuard, UserRoleGuard], canDeactivate: [jasmine.any(Function)], }, ]; diff --git a/src/app/core/config/dynamic-routing/router.service.ts b/src/app/core/config/dynamic-routing/router.service.ts index f087dfd363..238d3d8581 100644 --- a/src/app/core/config/dynamic-routing/router.service.ts +++ b/src/app/core/config/dynamic-routing/router.service.ts @@ -8,6 +8,7 @@ import { NotFoundComponent } from "./not-found/not-found.component"; import { AuthGuard } from "../../session/auth.guard"; import { UnsavedChangesService } from "../../entity-details/form/unsaved-changes.service"; import { RoutedViewComponent } from "../../ui/routed-view/routed-view.component"; +import { EntityPermissionGuard } from "../../permissions/permission-guard/entity-permission.guard"; /** * The RouterService dynamically sets up Angular routing from config loaded through the {@link ConfigService}. @@ -86,7 +87,7 @@ export class RouterService { private generateRouteFromConfig(view: ViewConfig, route: Route): Route { route.data = route.data ?? {}; - route.canActivate = [AuthGuard]; + route.canActivate = [AuthGuard, EntityPermissionGuard]; route.canDeactivate = [ () => inject(UnsavedChangesService).checkUnsavedChanges(), ]; diff --git a/src/app/core/demo-data/demo-data.module.ts b/src/app/core/demo-data/demo-data.module.ts index 0fa13022ab..e04fec7a98 100644 --- a/src/app/core/demo-data/demo-data.module.ts +++ b/src/app/core/demo-data/demo-data.module.ts @@ -42,8 +42,8 @@ import { DemoSiteSettingsGeneratorService } from "../site-settings/demo-site-set import { DemoReportConfigGeneratorService } from "../../features/reporting/demo-report-config-generator.service"; const demoDataGeneratorProviders = [ - ...DemoConfigGeneratorService.provider(), ...DemoPermissionGeneratorService.provider(), + ...DemoConfigGeneratorService.provider(), ...DemoSiteSettingsGeneratorService.provider(), ...DemoPublicFormGeneratorService.provider(), ...DemoUserGeneratorService.provider(), diff --git a/src/app/core/entity-details/entity-details/entity-details.component.spec.ts b/src/app/core/entity-details/entity-details/entity-details.component.spec.ts index f324442a7b..d8c54400cc 100644 --- a/src/app/core/entity-details/entity-details/entity-details.component.spec.ts +++ b/src/app/core/entity-details/entity-details/entity-details.component.spec.ts @@ -55,8 +55,10 @@ describe("EntityDetailsComponent", () => { mockEntityRemoveService = jasmine.createSpyObj(["remove"]); mockChildrenService.queryRelationsOf.and.resolveTo([]); mockChildrenService.getAserResultsOfChild.and.resolveTo([]); - mockAbility = jasmine.createSpyObj(["cannot", "update"]); + mockAbility = jasmine.createSpyObj(["cannot", "update", "on"]); mockAbility.cannot.and.returnValue(false); + mockAbility.on.and.returnValue(() => true); + TestBed.configureTestingModule({ imports: [EntityDetailsComponent, MockedTestingModule.withState()], providers: [ diff --git a/src/app/core/entity-details/form/form.component.spec.ts b/src/app/core/entity-details/form/form.component.spec.ts index a6816a2b6a..ecdf23af5d 100644 --- a/src/app/core/entity-details/form/form.component.spec.ts +++ b/src/app/core/entity-details/form/form.component.spec.ts @@ -42,12 +42,18 @@ describe("FormComponent", () => { }); it("calls router once a new child is saved", async () => { + const entityFormService = TestBed.inject(EntityFormService); + spyOn(entityFormService, "saveChanges").and.resolveTo(); + const testChild = new Child(); const router = fixture.debugElement.injector.get(Router); spyOn(router, "navigate"); + component.creatingNew = true; component.entity = testChild; await component.saveClicked(); + + expect(entityFormService.saveChanges).toHaveBeenCalled(); expect(router.navigate).toHaveBeenCalledWith(["", testChild.getId()]); }); diff --git a/src/app/core/entity/latest-entity-loader.ts b/src/app/core/entity/latest-entity-loader.ts index 18755a262f..296121fe59 100644 --- a/src/app/core/entity/latest-entity-loader.ts +++ b/src/app/core/entity/latest-entity-loader.ts @@ -25,28 +25,35 @@ export abstract class LatestEntityLoader { * Initialize the loader to make the entity available and emit continuous updates * through the `entityUpdated` property */ - startLoading() { - this.loadOnce(); + async startLoading() { + const initialValue = await this.loadOnce(); this.entityMapper .receiveUpdates(this.entityCtor) .pipe(filter(({ entity }) => entity.getId() === this.entityID)) - .subscribe(({ entity }) => this.entityUpdated.next(entity)); + .subscribe(({ entity }) => { + this.entityUpdated.next(entity); + }); + return initialValue; } /** * Do an initial load of the entity to be available through the `entityUpdated` property * (without watching for continuous updates). */ - loadOnce() { + loadOnce(): Promise { return this.entityMapper .load(this.entityCtor, this.entityID) - .then((entity) => this.entityUpdated.next(entity)) + .then((entity) => { + this.entityUpdated.next(entity); + return entity; + }) .catch((err) => { if (err?.status !== HttpStatusCode.NotFound) { this.logger.error( `Loading entity "${this.entityCtor.ENTITY_TYPE}:${this.entityID}" failed: ${err} `, ); } + return undefined; }); } } diff --git a/src/app/core/permissions/ability/ability.service.spec.ts b/src/app/core/permissions/ability/ability.service.spec.ts index ba8d066a01..eebbe67852 100644 --- a/src/app/core/permissions/ability/ability.service.spec.ts +++ b/src/app/core/permissions/ability/ability.service.spec.ts @@ -1,7 +1,7 @@ import { fakeAsync, TestBed, tick, waitForAsync } from "@angular/core/testing"; import { AbilityService } from "./ability.service"; -import { Subject } from "rxjs"; +import { BehaviorSubject, Subject } from "rxjs"; import { Child } from "../../../child-dev-project/children/model/child"; import { Note } from "../../../child-dev-project/notes/model/note"; import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service"; @@ -12,10 +12,10 @@ import { EntityAbility } from "./entity-ability"; import { DatabaseRule, DatabaseRules } from "../permission-types"; import { Config } from "../../config/config"; import { LoggingService } from "../../logging/logging.service"; -import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { UpdatedEntity } from "../../entity/model/entity-update"; import { mockEntityMapper } from "../../entity/entity-mapper/mock-entity-mapper-service"; import { TEST_USER } from "../../../utils/mock-local-session"; +import { CoreTestingModule } from "../../../utils/core-testing.module"; describe("AbilityService", () => { let service: AbilityService; @@ -35,9 +35,25 @@ describe("AbilityService", () => { entityMapper = mockEntityMapper() as any; spyOn(entityMapper, "receiveUpdates").and.returnValue(entityUpdates); spyOn(entityMapper, "load").and.callThrough(); + TestBed.configureTestingModule({ - imports: [MockedTestingModule.withState()], - providers: [{ provide: EntityMapperService, useValue: entityMapper }], + imports: [CoreTestingModule], + providers: [ + AbilityService, + EntityAbility, + { + provide: CurrentUserSubject, + useValue: new BehaviorSubject({ + name: TEST_USER, + roles: ["user_app"], + }), + }, + { + provide: PermissionEnforcerService, + useValue: jasmine.createSpyObj(["enforcePermissionsOnLocalData"]), + }, + { provide: EntityMapperService, useValue: entityMapper }, + ], }); service = TestBed.inject(AbilityService); ability = TestBed.inject(EntityAbility); @@ -52,15 +68,31 @@ describe("AbilityService", () => { }); it("should fetch the rules object from the database", () => { + service.initializeRules(); expect(entityMapper.load).toHaveBeenCalledWith( Config, Config.PERMISSION_KEY, ); }); - it("should update the rules when a change is published", () => { + it("should give all permissions if no rules object can be fetched but not until checking for rules object", fakeAsync(() => { entityMapper.load.and.rejectWith("no initial config"); + // no permissions until rules object has been attempted to fetch initially + expect(ability.rules).toEqual([]); + + service.initializeRules(); + tick(); // initial attempt to load rules resolved now + + // Request failed, sync not started - offline without cached rules object + expect(ability.rules).toEqual([{ action: "manage", subject: "all" }]); + })); + + it("should update the rules when a change is published", fakeAsync(() => { + entityMapper.load.and.rejectWith("no initial config"); + service.initializeRules(); + tick(); + expect(entityMapper.load).toHaveBeenCalled(); // Default rule expect(ability.rules).toEqual([{ action: "manage", subject: "all" }]); @@ -69,21 +101,29 @@ describe("AbilityService", () => { entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); + tick(); expect(ability.rules).toEqual(rules["user_app"]); - }); + })); + + it("should update the ability with the received rules for the logged in user", fakeAsync(() => { + service.initializeRules(); + tick(); - it("should update the ability with the received rules for the logged in user", () => { spyOn(ability, "update"); entityUpdates.next({ entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); + tick(); expect(ability.update).toHaveBeenCalledWith(rules.user_app); - }); + })); + + it("should update the ability with rules for all roles the logged in user has", fakeAsync(() => { + service.initializeRules(); + tick(); - it("should update the ability with rules for all roles the logged in user has", () => { spyOn(ability, "update"); TestBed.inject(CurrentUserSubject).next({ name: "testAdmin", @@ -94,17 +134,22 @@ describe("AbilityService", () => { entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); + tick(); expect(ability.update).toHaveBeenCalledWith( rules.user_app.concat(rules.admin_app), ); - }); + })); + + it("should create an ability that correctly uses the defined rules", fakeAsync(() => { + service.initializeRules(); + tick(); - it("should create an ability that correctly uses the defined rules", () => { entityUpdates.next({ entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); + tick(); expect(ability.can("read", Child)).toBeTrue(); expect(ability.can("create", Child)).toBeFalse(); @@ -123,12 +168,13 @@ describe("AbilityService", () => { const updatedConfig = new Config(Config.PERMISSION_KEY, rules); updatedConfig._rev = "update"; entityUpdates.next({ entity: updatedConfig, type: "update" }); + tick(); expect(ability.can("manage", Child)).toBeTrue(); expect(ability.can("manage", new Child())).toBeTrue(); expect(ability.can("manage", Note)).toBeTrue(); expect(ability.can("manage", new Note())).toBeTrue(); - }); + })); it("should throw an error when checking permissions on a object that is not a Entity", () => { entityUpdates.next({ @@ -141,40 +187,25 @@ describe("AbilityService", () => { expect(() => ability.can("read", new TestClass() as any)).toThrowError(); }); - it("should give all permissions if no rules object can be fetched", () => { - // Request failed, sync not started - offline without cached rules object - expect(ability.can("read", Child)).toBeTrue(); - expect(ability.can("update", Child)).toBeTrue(); - expect(ability.can("manage", new Note())).toBeTrue(); - }); - - it("should notify when the rules are updated", (done) => { - spyOn(ability, "update"); - service.abilityUpdated.subscribe(() => { - expect(ability.update).toHaveBeenCalled(); - done(); - }); + it("should call the ability enforcer after updating the rules", fakeAsync(() => { + service.initializeRules(); + tick(); entityUpdates.next({ entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); - }); + tick(); - it("should call the ability enforcer after updating the rules", () => { - const enforceSpy = spyOn( - TestBed.inject(PermissionEnforcerService), - "enforcePermissionsOnLocalData", - ); - entityUpdates.next({ - entity: new Config(Config.PERMISSION_KEY, rules), - type: "update", - }); + expect( + TestBed.inject(PermissionEnforcerService).enforcePermissionsOnLocalData, + ).toHaveBeenCalled(); + })); - expect(enforceSpy).toHaveBeenCalled(); - }); + it("should allow to access user properties in the rules", fakeAsync(() => { + service.initializeRules(); + tick(); - it("should allow to access user properties in the rules", () => { const config = new Config(Config.PERMISSION_KEY, { user_app: [ { @@ -185,6 +216,7 @@ describe("AbilityService", () => { ], }); entityUpdates.next({ entity: config, type: "update" }); + tick(); const userEntity = new User(); userEntity.name = TEST_USER; @@ -192,9 +224,12 @@ describe("AbilityService", () => { const anotherUser = new User(); anotherUser.name = "another user"; expect(ability.cannot("manage", anotherUser)).toBeTrue(); - }); + })); it("should allow to check conditions with complex data types", fakeAsync(() => { + service.initializeRules(); + tick(); + const classInteraction = defaultInteractionTypes.find( (type) => type.id === "SCHOOL_CLASS", ); @@ -208,6 +243,7 @@ describe("AbilityService", () => { ], }); entityUpdates.next({ entity: config, type: "update" }); + tick(); const note = new Note(); expect(ability.can("read", note)).toBeFalse(); @@ -216,7 +252,10 @@ describe("AbilityService", () => { expect(ability.can("read", note)).toBeTrue(); })); - it("should log a warning if no rules are found for a user", () => { + it("should log a warning if no rules are found for a user", fakeAsync(() => { + service.initializeRules(); + tick(); + TestBed.inject(CurrentUserSubject).next({ name: "new-user", roles: ["invalid_role"], @@ -226,11 +265,14 @@ describe("AbilityService", () => { entity: new Config(Config.PERMISSION_KEY, rules), type: "update", }); + tick(); expect(warnSpy).toHaveBeenCalled(); - }); + })); - it("should prepend default rules to all users", () => { + it("should prepend default rules to all users", fakeAsync(() => { + service.initializeRules(); + tick(); const defaultRules: DatabaseRule[] = [ { subject: "Config", action: "read" }, { subject: "ProgressDashboardConfig", action: "manage" }, @@ -241,6 +283,7 @@ describe("AbilityService", () => { ); entityUpdates.next({ entity: config, type: "update" }); + tick(); expect(ability.rules).toEqual(defaultRules.concat(...rules.user_app)); @@ -251,16 +294,20 @@ describe("AbilityService", () => { config._rev = "update"; entityUpdates.next({ entity: config, type: "update" }); + tick(); expect(ability.rules).toEqual( defaultRules.concat(...rules.user_app, ...rules.admin_app), ); - }); + })); - it("should allow everything if permission doc has been deleted", () => { + it("should allow everything if permission doc has been deleted", fakeAsync(() => { + service.initializeRules(); + tick(); entityUpdates.next({ type: "new", entity: new Config(Config.PERMISSION_KEY, rules), }); + tick(); expect(ability.rules).toEqual(rules["user_app"]); @@ -268,7 +315,8 @@ describe("AbilityService", () => { type: "remove", entity: new Config(Config.PERMISSION_KEY), }); + tick(); expect(ability.rules).toEqual([{ subject: "all", action: "manage" }]); - }); + })); }); diff --git a/src/app/core/permissions/ability/ability.service.ts b/src/app/core/permissions/ability/ability.service.ts index 8d210b5f1f..37d11b2cd9 100644 --- a/src/app/core/permissions/ability/ability.service.ts +++ b/src/app/core/permissions/ability/ability.service.ts @@ -1,5 +1,4 @@ import { Injectable } from "@angular/core"; -import { shareReplay } from "rxjs/operators"; import { DatabaseRule, DatabaseRules } from "../permission-types"; import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service"; import { PermissionEnforcerService } from "../permission-enforcer/permission-enforcer.service"; @@ -13,15 +12,12 @@ import { CurrentUserSubject } from "../../user/user"; /** * This service sets up the `EntityAbility` injectable with the JSON defined rules for the currently logged in user. + * + * To get notified whenever the permissions of the current user are updated, use EntityAbility.on("updated", callback): + * https://casl.js.org/v6/en/api/casl-ability#on */ @Injectable() export class AbilityService extends LatestEntityLoader> { - /** - * Get notified whenever the permissions of the current user are updated. - * Use this to re-evaluate the permissions of the currently logged-in user. - */ - abilityUpdated = this.entityUpdated.pipe(shareReplay(1)); - constructor( private ability: EntityAbility, private currentUser: CurrentUserSubject, @@ -32,10 +28,15 @@ export class AbilityService extends LatestEntityLoader> { super(Config, Config.PERMISSION_KEY, entityMapper, logger); } - initializeRules() { - // Initially allow everything until permission document could be fetched - this.ability.update([{ action: "manage", subject: "all" }]); - super.startLoading(); + async initializeRules() { + const initialPermissions = await super.startLoading(); + if (initialPermissions) { + await this.updateAbilityWithUserRules(initialPermissions.data); + } else { + // as default fallback if no permission object is defined: allow everything + this.ability.update([{ action: "manage", subject: "all" }]); + } + this.entityUpdated.subscribe((config) => this.updateAbilityWithUserRules(config.data), ); diff --git a/src/app/core/permissions/permission-directive/disable-entity-operation.directive.spec.ts b/src/app/core/permissions/permission-directive/disable-entity-operation.directive.spec.ts index 1619782356..0ba6162fe8 100644 --- a/src/app/core/permissions/permission-directive/disable-entity-operation.directive.spec.ts +++ b/src/app/core/permissions/permission-directive/disable-entity-operation.directive.spec.ts @@ -3,30 +3,20 @@ import { Component, ElementRef, ViewChild } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { Entity } from "../../entity/model/entity"; import { Child } from "../../../child-dev-project/children/model/child"; -import { Subject } from "rxjs"; -import { AbilityService } from "../ability/ability.service"; import { EntityAbility } from "../ability/entity-ability"; describe("DisableEntityOperationDirective", () => { let testComponent: ComponentFixture; - let mockAbility: jasmine.SpyObj; - let mockAbilityService: jasmine.SpyObj; - let mockUpdateNotifier: Subject; + let mockAbility: EntityAbility; beforeEach(() => { - mockAbility = jasmine.createSpyObj(["cannot"]); - mockUpdateNotifier = new Subject(); - mockAbilityService = jasmine.createSpyObj([], { - abilityUpdated: mockUpdateNotifier, - }); + mockAbility = new EntityAbility(null); + spyOn(mockAbility, "cannot"); TestBed.configureTestingModule({ declarations: [TestComponent], imports: [DisableEntityOperationDirective], - providers: [ - { provide: EntityAbility, useValue: mockAbility }, - { provide: AbilityService, useValue: mockAbilityService }, - ], + providers: [{ provide: EntityAbility, useValue: mockAbility }], }); }); @@ -58,7 +48,7 @@ describe("DisableEntityOperationDirective", () => { testComponent.componentInstance.buttonRef.nativeElement.disabled, ).toBeFalse(); - mockAbility.cannot.and.returnValue(true); + (mockAbility.cannot as jasmine.Spy).and.returnValue(true); testComponent.componentInstance.entityConstructor = Child; testComponent.detectChanges(); @@ -74,8 +64,8 @@ describe("DisableEntityOperationDirective", () => { testComponent.componentInstance.buttonRef.nativeElement.disabled, ).toBeTrue(); - mockAbility.cannot.and.returnValue(false); - mockUpdateNotifier.next(); + (mockAbility.cannot as jasmine.Spy).and.returnValue(false); + mockAbility.update([{ action: "manage", subject: "all" }]); testComponent.detectChanges(); expect( @@ -84,7 +74,7 @@ describe("DisableEntityOperationDirective", () => { }); function createComponent(disabled: boolean = true) { - mockAbility.cannot.and.returnValue(disabled); + (mockAbility.cannot as jasmine.Spy).and.returnValue(disabled); testComponent = TestBed.createComponent(TestComponent); testComponent.detectChanges(); } diff --git a/src/app/core/permissions/permission-directive/disable-entity-operation.directive.ts b/src/app/core/permissions/permission-directive/disable-entity-operation.directive.ts index efdfe821be..2548287941 100644 --- a/src/app/core/permissions/permission-directive/disable-entity-operation.directive.ts +++ b/src/app/core/permissions/permission-directive/disable-entity-operation.directive.ts @@ -3,26 +3,27 @@ import { Directive, Input, OnChanges, + OnDestroy, OnInit, TemplateRef, ViewContainerRef, } from "@angular/core"; import { DisabledWrapperComponent } from "./disabled-wrapper.component"; import { EntityAction, EntitySubject } from "../permission-types"; -import { AbilityService } from "../ability/ability.service"; import { EntityAbility } from "../ability/entity-ability"; -import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { Unsubscribe } from "@casl/ability"; /** * This directive can be used to disable a element (e.g. button) based on the current users permissions. * Additionally, a little popup will be shown when the user hovers the disabled element. */ -@UntilDestroy() @Directive({ selector: "[appDisabledEntityOperation]", standalone: true, }) -export class DisableEntityOperationDirective implements OnInit, OnChanges { +export class DisableEntityOperationDirective + implements OnInit, OnChanges, OnDestroy +{ /** * 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 @@ -36,15 +37,20 @@ export class DisableEntityOperationDirective implements OnInit, OnChanges { private wrapperComponent: ComponentRef; private text: string = $localize`:Missing permission:Your account does not have the required permission for this action.`; + private readonly unsubscribeAbilityUpdates: Unsubscribe; + constructor( private templateRef: TemplateRef, private viewContainerRef: ViewContainerRef, private ability: EntityAbility, - private abilityService: AbilityService, ) { - this.abilityService.abilityUpdated - .pipe(untilDestroyed(this)) - .subscribe(() => this.applyPermissions()); + this.unsubscribeAbilityUpdates = this.ability.on("updated", () => + this.applyPermissions(), + ); + } + + ngOnDestroy(): void { + this.unsubscribeAbilityUpdates(); } ngOnInit() { diff --git a/src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts b/src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts new file mode 100644 index 0000000000..ec2fd3049a --- /dev/null +++ b/src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts @@ -0,0 +1,91 @@ +import { TestBed } from "@angular/core/testing"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ActivatedRouteSnapshot, Router } from "@angular/router"; +import { EntityPermissionGuard } from "./entity-permission.guard"; +import { EntityAbility } from "../ability/entity-ability"; + +describe("EntityPermissionGuard", () => { + let guard: EntityPermissionGuard; + let mockAbility: jasmine.SpyObj; + + beforeEach(() => { + mockAbility = jasmine.createSpyObj(["can"]); + + TestBed.configureTestingModule({ + imports: [RouterTestingModule], + providers: [ + EntityPermissionGuard, + { provide: EntityAbility, useValue: mockAbility }, + ], + }); + guard = TestBed.inject(EntityPermissionGuard); + }); + + it("should be created", () => { + expect(guard).toBeTruthy(); + }); + + it("should use ability to check if current user is allowed", () => { + mockAbility.can.and.returnValue(true); + + const result = guard.canActivate({ + routeConfig: { path: "url" }, + data: { + requiredPermissionOperation: "update", + config: { entityType: "TestEntity" }, + }, + } as Partial as ActivatedRouteSnapshot); + + expect(result).toBeTrue(); + expect(mockAbility.can).toHaveBeenCalledWith("update", "TestEntity"); + }); + + it("should navigate to 404 for real navigation requests without permissions", () => { + const router = TestBed.inject(Router); + spyOn(router, "navigate"); + const route = new ActivatedRouteSnapshot(); + Object.assign(route, { + routeConfig: { path: "url" }, + data: { requiredPermissionOperation: "update", entityType: "Child" }, + }); + mockAbility.can.and.returnValue(false); + + guard.canActivate(route); + + expect(router.navigate).toHaveBeenCalledWith(["/404"]); + }); + + it("should check 'read' permissions if only entity and no operation is set", () => { + mockAbility.can.and.returnValue(true); + const result = guard.canActivate({ + routeConfig: { path: "default-operation-config" }, + data: { entity: "Child" }, + } as Partial as ActivatedRouteSnapshot); + + expect(result).toBeTrue(); + expect(mockAbility.can).toHaveBeenCalledWith("read", "Child"); + }); + + it("should return true as default if no entity or operation are configured", () => { + const result = guard.canActivate({ + routeConfig: { path: "no-permission-config" }, + } as any); + + expect(result).toBeTrue(); + expect(mockAbility.can).not.toHaveBeenCalled(); + }); + + it("should evaluate correct route permissions on 'pre-check' (checkRoutePermissions)", () => { + mockAbility.can.and.returnValue(true); + + TestBed.inject(Router).config.push({ + path: "check-route/:id", + data: { requiredPermissionOperation: "update", entityType: "Child" }, + }); + + const result = guard.checkRoutePermissions("check-route/1"); + + expect(result).toBeTrue(); + expect(mockAbility.can).toHaveBeenCalledWith("update", "Child"); + }); +}); diff --git a/src/app/core/permissions/permission-guard/entity-permission.guard.ts b/src/app/core/permissions/permission-guard/entity-permission.guard.ts new file mode 100644 index 0000000000..78f2ba0966 --- /dev/null +++ b/src/app/core/permissions/permission-guard/entity-permission.guard.ts @@ -0,0 +1,64 @@ +import { Injectable } from "@angular/core"; +import { ActivatedRouteSnapshot, CanActivate, Router } from "@angular/router"; +import { RouteData } from "../../config/dynamic-routing/view-config.interface"; +import { EntityAbility } from "../ability/entity-ability"; + +/** + * A guard that checks the current users permission to interact with the entity of the route. + * Define `requiredPermissionOperation` in the route data / config, to enable a check that will find the relevant entity from config. + */ +@Injectable() +export class EntityPermissionGuard implements CanActivate { + constructor( + private router: Router, + private ability: EntityAbility, + ) {} + + canActivate(route: ActivatedRouteSnapshot): boolean { + const routeData: RouteData = route.data; + if (this.canAccessRoute(routeData)) { + return true; + } else { + if (route instanceof ActivatedRouteSnapshot) { + // Route should only change if this is a "real" navigation check (not the check in the NavigationComponent) + this.router.navigate(["/404"]); + } + return false; + } + } + + private canAccessRoute(routeData: RouteData) { + const operation = routeData?.["requiredPermissionOperation"] ?? "read"; + const primaryEntity = + routeData?.["entityType"] ?? + routeData?.["entity"] ?? + routeData?.["config"]?.["entityType"] ?? + routeData?.["config"]?.["entity"]; + + if (!primaryEntity) { + // No relevant config set => all users are allowed + return true; + } + + return this.ability.can(operation, primaryEntity); + } + + public checkRoutePermissions(path: string) { + // removing leading slash + path = path.replace(/^\//, ""); + + function isPathMatch(genericPath: string, path: string) { + const routeRegex = genericPath + .split("/") + // replace params with wildcard regex + .map((part) => (part.startsWith(":") ? "[^/]*" : part)) + .join("/"); + return path.match("^" + routeRegex + "$"); + } + + const routeData = this.router.config.find((r) => isPathMatch(r.path, path)) + ?.data; + + return this.canAccessRoute(routeData); + } +} diff --git a/src/app/core/permissions/permissions.module.ts b/src/app/core/permissions/permissions.module.ts index 3322c78309..4831e6a718 100644 --- a/src/app/core/permissions/permissions.module.ts +++ b/src/app/core/permissions/permissions.module.ts @@ -3,10 +3,12 @@ import { UserRoleGuard } from "./permission-guard/user-role.guard"; import { PureAbility } from "@casl/ability"; import { EntityAbility } from "./ability/entity-ability"; import { AbilityService } from "./ability/ability.service"; +import { EntityPermissionGuard } from "./permission-guard/entity-permission.guard"; @NgModule({ providers: [ UserRoleGuard, + EntityPermissionGuard, AbilityService, EntityAbility, { diff --git a/src/app/core/site-settings/site-settings.service.spec.ts b/src/app/core/site-settings/site-settings.service.spec.ts index 2eb01aff29..5fd959a1e1 100644 --- a/src/app/core/site-settings/site-settings.service.spec.ts +++ b/src/app/core/site-settings/site-settings.service.spec.ts @@ -1,4 +1,4 @@ -import { fakeAsync, TestBed, tick } from "@angular/core/testing"; +import { fakeAsync, TestBed, tick, waitForAsync } from "@angular/core/testing"; import { SiteSettingsService } from "./site-settings.service"; import { FileService } from "../../features/file/file.service"; @@ -24,7 +24,7 @@ describe("SiteSettingsService", () => { let entityMapper: MockEntityMapperService; let mockFileService: jasmine.SpyObj; - beforeEach(() => { + beforeEach(waitForAsync(() => { localStorage.clear(); entityMapper = mockEntityMapper(); mockFileService = jasmine.createSpyObj(["loadFile"]); @@ -37,7 +37,7 @@ describe("SiteSettingsService", () => { ], }); service = TestBed.inject(SiteSettingsService); - }); + })); afterEach(() => { localStorage.removeItem(service.SITE_SETTINGS_LOCAL_STORAGE_KEY); diff --git a/src/app/core/ui/navigation/navigation/navigation.component.spec.ts b/src/app/core/ui/navigation/navigation/navigation.component.spec.ts index 3abd3e6c29..902625e341 100644 --- a/src/app/core/ui/navigation/navigation/navigation.component.spec.ts +++ b/src/app/core/ui/navigation/navigation/navigation.component.spec.ts @@ -25,6 +25,7 @@ import { Config } from "../../../config/config"; import { UserRoleGuard } from "../../../permissions/permission-guard/user-role.guard"; import { Event, NavigationEnd, Router } from "@angular/router"; import { MockedTestingModule } from "../../../../utils/mocked-testing.module"; +import { EntityPermissionGuard } from "../../../permissions/permission-guard/entity-permission.guard"; describe("NavigationComponent", () => { let component: NavigationComponent; @@ -32,7 +33,7 @@ describe("NavigationComponent", () => { let mockConfigService: jasmine.SpyObj; let mockConfigUpdated: BehaviorSubject; - let mockUserRoleGuard: jasmine.SpyObj; + let mockGuard: jasmine.SpyObj; beforeEach(waitForAsync(() => { mockConfigUpdated = new BehaviorSubject(null); @@ -41,13 +42,14 @@ describe("NavigationComponent", () => { }); mockConfigService.getConfig.and.returnValue({ items: [] }); mockConfigService.getAllConfigs.and.returnValue([]); - mockUserRoleGuard = jasmine.createSpyObj(["checkRoutePermissions"]); - mockUserRoleGuard.checkRoutePermissions.and.returnValue(true); + mockGuard = jasmine.createSpyObj(["checkRoutePermissions"]); + mockGuard.checkRoutePermissions.and.returnValue(true); TestBed.configureTestingModule({ imports: [NavigationComponent, MockedTestingModule.withState()], providers: [ - { provide: UserRoleGuard, useValue: mockUserRoleGuard }, + { provide: UserRoleGuard, useValue: mockGuard }, + { provide: EntityPermissionGuard, useValue: mockGuard }, { provide: ConfigService, useValue: mockConfigService }, ], }).compileComponents(); @@ -88,7 +90,7 @@ describe("NavigationComponent", () => { ], }; mockConfigService.getConfig.and.returnValue(testConfig); - mockUserRoleGuard.checkRoutePermissions.and.callFake((route: string) => { + mockGuard.checkRoutePermissions.and.callFake((route: string) => { switch (route) { case "/dashboard": return false; diff --git a/src/app/core/ui/navigation/navigation/navigation.component.ts b/src/app/core/ui/navigation/navigation/navigation.component.ts index 78a26315c2..d4bc498ea6 100644 --- a/src/app/core/ui/navigation/navigation/navigation.component.ts +++ b/src/app/core/ui/navigation/navigation/navigation.component.ts @@ -27,6 +27,7 @@ import { MatListModule } from "@angular/material/list"; import { NgForOf } from "@angular/common"; import { Angulartics2Module } from "angulartics2"; import { FaDynamicIconComponent } from "../../../common-components/fa-dynamic-icon/fa-dynamic-icon.component"; +import { EntityPermissionGuard } from "../../../permissions/permission-guard/entity-permission.guard"; /** * Main app menu listing. @@ -54,9 +55,10 @@ export class NavigationComponent { public menuItems: MenuItem[] = []; constructor( - private userRoleGuard: UserRoleGuard, private configService: ConfigService, private router: Router, + private userRoleGuard: UserRoleGuard, + private entityPermissionGuard: EntityPermissionGuard, ) { this.configService.configUpdates .pipe(untilDestroyed(this)) @@ -116,7 +118,14 @@ export class NavigationComponent { const config: NavigationMenuConfig = this.configService.getConfig(this.CONFIG_ID); this.menuItems = config.items - .filter(({ link }) => this.userRoleGuard.checkRoutePermissions(link)) + .filter(({ link }) => this.isAccessibleRouteForUser(link)) .map(({ name, icon, link }) => new MenuItem(name, icon, link)); } + + private isAccessibleRouteForUser(path: string) { + return ( + this.userRoleGuard.checkRoutePermissions(path) && + this.entityPermissionGuard.checkRoutePermissions(path) + ); + } } diff --git a/src/app/features/todos/todo-details/todo-details.component.spec.ts b/src/app/features/todos/todo-details/todo-details.component.spec.ts index 8ddad1846f..42c90bec3f 100644 --- a/src/app/features/todos/todo-details/todo-details.component.spec.ts +++ b/src/app/features/todos/todo-details/todo-details.component.spec.ts @@ -9,6 +9,7 @@ import { MockedTestingModule } from "../../../utils/mocked-testing.module"; import { LoginState } from "../../../core/session/session-states/login-state.enum"; import { EntityMapperService } from "../../../core/entity/entity-mapper/entity-mapper.service"; import { NEVER } from "rxjs"; +import { EntityAbility } from "../../../core/permissions/ability/entity-ability"; describe("TodoDetailsComponent", () => { let component: TodoDetailsComponent; @@ -51,6 +52,8 @@ describe("TodoDetailsComponent", () => { }); it("should save entity with all changes when completing todo", async () => { + spyOn(TestBed.inject(EntityAbility), "can").and.returnValue(true); + const editedEntityProp = "subject"; component.formColumns = [{ fields: [editedEntityProp] }]; component.ngOnInit(); diff --git a/src/app/utils/core-testing.module.ts b/src/app/utils/core-testing.module.ts index 647ddaaa4f..084f3b4d73 100644 --- a/src/app/utils/core-testing.module.ts +++ b/src/app/utils/core-testing.module.ts @@ -9,13 +9,14 @@ import { mockEntityMapper } from "../core/entity/entity-mapper/mock-entity-mappe import { ConfigurableEnumService } from "../core/basic-datatypes/configurable-enum/configurable-enum.service"; import { ComponentRegistry } from "../dynamic-components"; import { EntityActionsService } from "../core/entity/entity-actions/entity-actions.service"; +import { ConfigurableEnumModule } from "../core/basic-datatypes/configurable-enum/configurable-enum.module"; /** * A basic module that can be imported in unit tests to provide default datatypes. * In contrast to MockedTestingModule this imports a much more limited set of modules. */ @NgModule({ - imports: [CoreModule], + imports: [CoreModule, ConfigurableEnumModule], providers: [ { provide: EntityRegistry, useValue: entityRegistry }, { provide: EntityMapperService, useValue: mockEntityMapper() },