diff --git a/src/app/app.routing.ts b/src/app/app.routing.ts index 44c1165064..f6be2d0dbb 100644 --- a/src/app/app.routing.ts +++ b/src/app/app.routing.ts @@ -53,6 +53,7 @@ export const allRoutes: Routes = [ }, { path: "admin", + canActivate: [AuthGuard], // add directly without lazy-loading so that Menu can detect permissions for child routes children: AdminModule.routes, }, diff --git a/src/app/core/admin/admin.routing.ts b/src/app/core/admin/admin.routing.ts index 909bfea5c6..4c035d12d0 100644 --- a/src/app/core/admin/admin.routing.ts +++ b/src/app/core/admin/admin.routing.ts @@ -5,7 +5,6 @@ import { ConfigImportComponent } from "../../features/config-setup/config-import import { ConflictResolutionListComponent } from "../../features/conflict-resolution/conflict-resolution-list/conflict-resolution-list.component"; import { UserRoleGuard } from "../permissions/permission-guard/user-role.guard"; import { EntityPermissionGuard } from "../permissions/permission-guard/entity-permission.guard"; -import { AuthGuard } from "../session/auth.guard"; export const adminRoutes: Routes = [ { @@ -24,7 +23,7 @@ export const adminRoutes: Routes = [ entity: "Config", requiredPermissionOperation: "update", }, - canActivate: [AuthGuard, EntityPermissionGuard], + canActivate: [EntityPermissionGuard], }, { 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 index ec2fd3049a..02393e8bb8 100644 --- a/src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts +++ b/src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts @@ -9,7 +9,7 @@ describe("EntityPermissionGuard", () => { let mockAbility: jasmine.SpyObj; beforeEach(() => { - mockAbility = jasmine.createSpyObj(["can"]); + mockAbility = jasmine.createSpyObj(["can"], { rules: [{}] }); TestBed.configureTestingModule({ imports: [RouterTestingModule], @@ -25,10 +25,10 @@ describe("EntityPermissionGuard", () => { expect(guard).toBeTruthy(); }); - it("should use ability to check if current user is allowed", () => { + it("should use ability to check if current user is allowed", async () => { mockAbility.can.and.returnValue(true); - const result = guard.canActivate({ + const result = await guard.canActivate({ routeConfig: { path: "url" }, data: { requiredPermissionOperation: "update", @@ -40,7 +40,7 @@ describe("EntityPermissionGuard", () => { expect(mockAbility.can).toHaveBeenCalledWith("update", "TestEntity"); }); - it("should navigate to 404 for real navigation requests without permissions", () => { + it("should navigate to 404 for real navigation requests without permissions", async () => { const router = TestBed.inject(Router); spyOn(router, "navigate"); const route = new ActivatedRouteSnapshot(); @@ -50,14 +50,14 @@ describe("EntityPermissionGuard", () => { }); mockAbility.can.and.returnValue(false); - guard.canActivate(route); + await guard.canActivate(route); expect(router.navigate).toHaveBeenCalledWith(["/404"]); }); - it("should check 'read' permissions if only entity and no operation is set", () => { + it("should check 'read' permissions if only entity and no operation is set", async () => { mockAbility.can.and.returnValue(true); - const result = guard.canActivate({ + const result = await guard.canActivate({ routeConfig: { path: "default-operation-config" }, data: { entity: "Child" }, } as Partial as ActivatedRouteSnapshot); @@ -66,8 +66,8 @@ describe("EntityPermissionGuard", () => { expect(mockAbility.can).toHaveBeenCalledWith("read", "Child"); }); - it("should return true as default if no entity or operation are configured", () => { - const result = guard.canActivate({ + it("should return true as default if no entity or operation are configured", async () => { + const result = await guard.canActivate({ routeConfig: { path: "no-permission-config" }, } as any); @@ -75,7 +75,7 @@ describe("EntityPermissionGuard", () => { expect(mockAbility.can).not.toHaveBeenCalled(); }); - it("should evaluate correct route permissions on 'pre-check' (checkRoutePermissions)", () => { + it("should evaluate correct route permissions on 'pre-check' (checkRoutePermissions)", async () => { mockAbility.can.and.returnValue(true); TestBed.inject(Router).config.push({ @@ -83,7 +83,7 @@ describe("EntityPermissionGuard", () => { data: { requiredPermissionOperation: "update", entityType: "Child" }, }); - const result = guard.checkRoutePermissions("check-route/1"); + const result = await 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 index 78f2ba0966..c799eb9f3c 100644 --- a/src/app/core/permissions/permission-guard/entity-permission.guard.ts +++ b/src/app/core/permissions/permission-guard/entity-permission.guard.ts @@ -14,9 +14,9 @@ export class EntityPermissionGuard implements CanActivate { private ability: EntityAbility, ) {} - canActivate(route: ActivatedRouteSnapshot): boolean { + async canActivate(route: ActivatedRouteSnapshot): Promise { const routeData: RouteData = route.data; - if (this.canAccessRoute(routeData)) { + if (await this.canAccessRoute(routeData)) { return true; } else { if (route instanceof ActivatedRouteSnapshot) { @@ -27,7 +27,7 @@ export class EntityPermissionGuard implements CanActivate { } } - private canAccessRoute(routeData: RouteData) { + private async canAccessRoute(routeData: RouteData) { const operation = routeData?.["requiredPermissionOperation"] ?? "read"; const primaryEntity = routeData?.["entityType"] ?? @@ -40,6 +40,11 @@ export class EntityPermissionGuard implements CanActivate { return true; } + if (this.ability.rules.length === 0) { + // wait till rules are initialised + await new Promise((res) => this.ability.on("updated", res)); + } + return this.ability.can(operation, primaryEntity); } 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 902625e341..19c82b95a3 100644 --- a/src/app/core/ui/navigation/navigation/navigation.component.spec.ts +++ b/src/app/core/ui/navigation/navigation/navigation.component.spec.ts @@ -15,7 +15,13 @@ * along with ndb-core. If not, see . */ -import { ComponentFixture, TestBed, waitForAsync } from "@angular/core/testing"; +import { + ComponentFixture, + fakeAsync, + TestBed, + tick, + waitForAsync, +} from "@angular/core/testing"; import { NavigationComponent } from "./navigation.component"; import { MenuItem } from "../menu-item"; @@ -33,7 +39,8 @@ describe("NavigationComponent", () => { let mockConfigService: jasmine.SpyObj; let mockConfigUpdated: BehaviorSubject; - let mockGuard: jasmine.SpyObj; + let mockRoleGuard: jasmine.SpyObj; + let mockEntityGuard: jasmine.SpyObj; beforeEach(waitForAsync(() => { mockConfigUpdated = new BehaviorSubject(null); @@ -42,14 +49,16 @@ describe("NavigationComponent", () => { }); mockConfigService.getConfig.and.returnValue({ items: [] }); mockConfigService.getAllConfigs.and.returnValue([]); - mockGuard = jasmine.createSpyObj(["checkRoutePermissions"]); - mockGuard.checkRoutePermissions.and.returnValue(true); + mockRoleGuard = jasmine.createSpyObj(["checkRoutePermissions"]); + mockRoleGuard.checkRoutePermissions.and.returnValue(true); + mockEntityGuard = jasmine.createSpyObj(["checkRoutePermissions"]); + mockEntityGuard.checkRoutePermissions.and.resolveTo(true); TestBed.configureTestingModule({ imports: [NavigationComponent, MockedTestingModule.withState()], providers: [ - { provide: UserRoleGuard, useValue: mockGuard }, - { provide: EntityPermissionGuard, useValue: mockGuard }, + { provide: UserRoleGuard, useValue: mockRoleGuard }, + { provide: EntityPermissionGuard, useValue: mockEntityGuard }, { provide: ConfigService, useValue: mockConfigService }, ], }).compileComponents(); @@ -65,7 +74,7 @@ describe("NavigationComponent", () => { expect(component).toBeTruthy(); }); - it("generates menu items from config", function () { + it("generates menu items from config", fakeAsync(() => { const testConfig = { items: [ { name: "Dashboard", icon: "home", link: "/dashboard" }, @@ -74,23 +83,22 @@ describe("NavigationComponent", () => { }; mockConfigService.getConfig.and.returnValue(testConfig); mockConfigUpdated.next(null); - const items = component.menuItems; + tick(); - expect(items).toEqual([ + expect(component.menuItems).toEqual([ new MenuItem("Dashboard", "home", "/dashboard"), new MenuItem("Children", "child", "/child"), ]); - }); + })); - it("marks items that require admin rights", function () { + it("marks items that require admin rights", fakeAsync(() => { const testConfig = { items: [ { name: "Dashboard", icon: "home", link: "/dashboard" }, { name: "Children", icon: "child", link: "/child" }, ], }; - mockConfigService.getConfig.and.returnValue(testConfig); - mockGuard.checkRoutePermissions.and.callFake((route: string) => { + mockRoleGuard.checkRoutePermissions.and.callFake((route: string) => { switch (route) { case "/dashboard": return false; @@ -103,11 +111,39 @@ describe("NavigationComponent", () => { mockConfigService.getConfig.and.returnValue(testConfig); mockConfigUpdated.next(null); + tick(); expect(component.menuItems).toEqual([ new MenuItem("Children", "child", "/child"), ]); - }); + })); + + it("should not add menu items if entity permissions are missing", fakeAsync(() => { + const testConfig = { + items: [ + { name: "Dashboard", icon: "home", link: "/dashboard" }, + { name: "Children", icon: "child", link: "/child" }, + ], + }; + mockEntityGuard.checkRoutePermissions.and.callFake((route: string) => { + switch (route) { + case "/dashboard": + return Promise.resolve(false); + case "/child": + return Promise.resolve(true); + default: + return Promise.resolve(false); + } + }); + + mockConfigService.getConfig.and.returnValue(testConfig); + mockConfigUpdated.next(null); + tick(); + + expect(component.menuItems).toEqual([ + new MenuItem("Children", "child", "/child"), + ]); + })); it("should highlight active menu item", () => { const routerEvents = TestBed.inject(Router).events as Subject; diff --git a/src/app/core/ui/navigation/navigation/navigation.component.ts b/src/app/core/ui/navigation/navigation/navigation.component.ts index d4bc498ea6..0519f71c3f 100644 --- a/src/app/core/ui/navigation/navigation/navigation.component.ts +++ b/src/app/core/ui/navigation/navigation/navigation.component.ts @@ -114,12 +114,17 @@ export class NavigationComponent { /** * Load menu items from config file */ - private initMenuItemsFromConfig() { + private async initMenuItemsFromConfig() { const config: NavigationMenuConfig = this.configService.getConfig(this.CONFIG_ID); - this.menuItems = config.items - .filter(({ link }) => this.isAccessibleRouteForUser(link)) - .map(({ name, icon, link }) => new MenuItem(name, icon, link)); + const menuItems = []; + for (const item of config.items) { + if (await this.isAccessibleRouteForUser(item.link)) { + menuItems.push(new MenuItem(item.name, item.icon, item.link)); + } + } + this.menuItems = menuItems; + this.activeLink = this.computeActiveLink(location.pathname); } private isAccessibleRouteForUser(path: string) {