Guards fix

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,
   },

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],
   },
 
   {

src/app/core/permissions/permission-guard/entity-permission.guard.spec.ts

@@ -9,7 +9,7 @@ describe("EntityPermissionGuard", () => {
   let mockAbility: jasmine.SpyObj<EntityAbility>;
 
   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<ActivatedRouteSnapshot> as ActivatedRouteSnapshot);
@@ -66,24 +66,24 @@ 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);
 
     expect(result).toBeTrue();
     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({
       path: "check-route/:id",
       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");

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<boolean> {
     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);
   }
 

src/app/core/ui/navigation/navigation/navigation.component.spec.ts

@@ -15,7 +15,13 @@
  *     along with ndb-core.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-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<ConfigService>;
   let mockConfigUpdated: BehaviorSubject<Config>;
-  let mockGuard: jasmine.SpyObj<UserRoleGuard>;
+  let mockRoleGuard: jasmine.SpyObj<UserRoleGuard>;
+  let mockEntityGuard: jasmine.SpyObj<EntityPermissionGuard>;
 
   beforeEach(waitForAsync(() => {
     mockConfigUpdated = new BehaviorSubject<Config>(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<Event>;

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<NavigationMenuConfig>(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) {