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

Dashboard permissions #2131

Merged
merged 13 commits into from
Dec 19, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export class RoutePermissionsService {
return accessibleRoutes;
}

private isAccessibleRouteForUser(path: string) {
private async isAccessibleRouteForUser(path: string) {
return (
this.roleGuard.checkRoutePermissions(path) &&
this.permissionGuard.checkRoutePermissions(path)
(await this.roleGuard.checkRoutePermissions(path)) &&
(await this.permissionGuard.checkRoutePermissions(path))
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import {
ActivatedRouteSnapshot,
CanActivate,
Route,
Router,
} from "@angular/router";
import { DynamicComponentConfig } from "../../config/dynamic-components/dynamic-component-config.interface";

/**
* Abstract base class with functionality common to all guards that check configurable user permissions or roles.
*/
export abstract class AbstractPermissionGuard implements CanActivate {
constructor(private router: Router) {}

/**
* Check if current navigation is allowed. This is used by Angular Router.
* @param route
*/
async canActivate(route: ActivatedRouteSnapshot): Promise<boolean> {
const routeData: DynamicComponentConfig = route.data;
if (await 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;
}
}

/**
* Implement specific permission checks here, based on the given route data (from config)
* and any required services provided by Angular dependency injection.
*
* @param routeData The route data object defined either in routing code or loaded from config by the RouterService.
* @protected
*/
protected abstract canAccessRoute(
routeData: DynamicComponentConfig,
): Promise<boolean>;

/**
* Pre-check if access to the given route would be allowed.
* This is used by components and services to evaluate permissions without actual navigation.
*
* @param path
*/
public checkRoutePermissions(path: string) {
let routeData = this.getRouteDataFromRouter(path, this.router.config);
return this.canAccessRoute(routeData?.data);
}

/**
* Extract the relevant route from Router, to get a merged route that contains the full trail of `permittedRoles`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean by "full trail of permittedRoles"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this includes the "permittedRoles" from parent routes of nested routing. Not perfect merge of route data, however - for more complex use cases this might potentially break, I think.

* @param path
* @param routes
* @private
*/
private getRouteDataFromRouter(path: string, routes: Route[]) {
// 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 pathSections = path.split("/");
let route = routes.find((r) => isPathMatch(r.path, path));
if (!route && pathSections.length > 1) {
route = routes.find((r) => isPathMatch(r.path, pathSections[0]));
}

if (route?.children) {
const childRoute = this.getRouteDataFromRouter(
pathSections.slice(1).join("/"),
route.children,
);
if (childRoute) {
childRoute.data = { ...route.data, ...childRoute?.data };
route = childRoute;
}
}

return route;
}
}
Original file line number Diff line number Diff line change
@@ -1,33 +1,28 @@
import { Injectable } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivate, Router } from "@angular/router";
import { DynamicComponentConfig } from "../../config/dynamic-components/dynamic-component-config.interface";
import { CanActivate, Router } from "@angular/router";
import { EntityAbility } from "../ability/entity-ability";
import { AbstractPermissionGuard } from "./abstract-permission.guard";
import { DynamicComponentConfig } from "../../config/dynamic-components/dynamic-component-config.interface";

/**
* 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 {
export class EntityPermissionGuard
extends AbstractPermissionGuard
implements CanActivate
{
constructor(
private router: Router,
router: Router,
private ability: EntityAbility,
) {}

async canActivate(route: ActivatedRouteSnapshot): Promise<boolean> {
const routeData: DynamicComponentConfig = route.data;
if (await 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;
}
) {
super(router);
}

private async canAccessRoute(routeData: DynamicComponentConfig) {
protected async canAccessRoute(
routeData: DynamicComponentConfig,
): Promise<boolean> {
const operation = routeData?.["requiredPermissionOperation"] ?? "read";
const primaryEntity =
routeData?.["entityType"] ??
Expand All @@ -47,24 +42,4 @@ export class EntityPermissionGuard implements CanActivate {

return this.ability.can(operation, primaryEntity);
}

public checkRoutePermissions(path: string) {
// removing leading slash
path = path.replace(/^\//, "");

function isPathMatch(genericPath: string, path: string) {
// TODO this does not seem to work with children routes (admin module)
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);
}
}
121 changes: 73 additions & 48 deletions src/app/core/permissions/permission-guard/user-role.guard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { UserRoleGuard } from "./user-role.guard";
import { RouterTestingModule } from "@angular/router/testing";
import { ActivatedRouteSnapshot, Route, Router } from "@angular/router";
import { AuthUser } from "../../session/auth/auth-user";
import { ConfigService } from "../../config/config.service";
import { PREFIX_VIEW_CONFIG } from "../../config/dynamic-routing/view-config.interface";
import { CurrentUserSubject } from "../../user/user";

describe("UserRoleGuard", () => {
Expand All @@ -16,18 +14,11 @@ describe("UserRoleGuard", () => {
name: "admin",
roles: ["admin", "user_app"],
};
let mockConfigService: jasmine.SpyObj<ConfigService>;

beforeEach(() => {
mockConfigService = jasmine.createSpyObj(["getConfig"]);

TestBed.configureTestingModule({
imports: [RouterTestingModule],
providers: [
CurrentUserSubject,
UserRoleGuard,
{ provide: ConfigService, useValue: mockConfigService },
],
providers: [CurrentUserSubject, UserRoleGuard],
});
guard = TestBed.inject(UserRoleGuard);
userSubject = TestBed.inject(CurrentUserSubject);
Expand All @@ -37,23 +28,23 @@ describe("UserRoleGuard", () => {
expect(guard).toBeTruthy();
});

it("should return true if current user is allowed", () => {
it("should return true if current user is allowed", async () => {
userSubject.next(adminUser);

const result = guard.canActivate({
const result = await guard.canActivate({
routeConfig: { path: "url" },
data: { permittedUserRoles: ["admin"] },
} as any);

expect(result).toBeTrue();
});

it("should return false for a user without permissions", () => {
it("should return false for a user without permissions", async () => {
userSubject.next(normalUser);
const router = TestBed.inject(Router);
spyOn(router, "navigate");

const result = guard.canActivate({
const result = await guard.canActivate({
routeConfig: { path: "url" },
data: { permittedUserRoles: ["admin"] },
} as any);
Expand All @@ -62,7 +53,7 @@ describe("UserRoleGuard", () => {
expect(router.navigate).not.toHaveBeenCalled();
});

it("should navigate to 404 for real navigation requests without permissions", () => {
it("should navigate to 404 for real navigation requests without permissions", async () => {
userSubject.next(normalUser);
const router = TestBed.inject(Router);
spyOn(router, "navigate");
Expand All @@ -72,46 +63,64 @@ describe("UserRoleGuard", () => {
data: { permittedUserRoles: ["admin"] },
});

guard.canActivate(route);
await guard.canActivate(route);

expect(router.navigate).toHaveBeenCalledWith(["/404"]);
});

it("should return true if no config is set", () => {
const result = guard.canActivate({ routeConfig: { path: "url" } } as any);
it("should return true if no config is set", async () => {
const result = await guard.canActivate({
routeConfig: { path: "url" },
} as any);

expect(result).toBeTrue();
});

it("should check permissions of a given route (checkRoutePermissions)", () => {
mockConfigService.getConfig.and.callFake((id) => {
switch (id) {
case PREFIX_VIEW_CONFIG + "restricted":
return { permittedUserRoles: ["admin"] } as any;
case PREFIX_VIEW_CONFIG + "pathA":
return {} as any;
case PREFIX_VIEW_CONFIG + "pathA/:id":
// details view restricted
return { permittedUserRoles: ["admin"] } as any;
}
it("should check permissions of a given route (checkRoutePermissions)", async () => {
const router = TestBed.inject(Router);
router.config.push({
path: "restricted",
data: { permittedUserRoles: ["admin"] },
});
router.config.push({ path: "pathA", data: {} });
// details view restricted
router.config.push({
path: "pathA/:id",
data: { permittedUserRoles: ["admin"] },
});

userSubject.next(normalUser);
expect(guard.checkRoutePermissions("free")).toBeTrue();
expect(guard.checkRoutePermissions("/free")).toBeTrue();
expect(guard.checkRoutePermissions("restricted")).toBeFalse();
expect(guard.checkRoutePermissions("pathA")).toBeTrue();
expect(guard.checkRoutePermissions("/pathA")).toBeTrue();
expect(guard.checkRoutePermissions("pathA/1")).toBeFalse();
await expectAsync(guard.checkRoutePermissions("free")).toBeResolvedTo(true);
await expectAsync(guard.checkRoutePermissions("/free")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("restricted")).toBeResolvedTo(
false,
);
await expectAsync(guard.checkRoutePermissions("pathA")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("/pathA")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("pathA/1")).toBeResolvedTo(
false,
);

userSubject.next(adminUser);
expect(guard.checkRoutePermissions("free")).toBeTrue();
expect(guard.checkRoutePermissions("restricted")).toBeTrue();
expect(guard.checkRoutePermissions("pathA")).toBeTrue();
expect(guard.checkRoutePermissions("pathA/1")).toBeTrue();
await expectAsync(guard.checkRoutePermissions("free")).toBeResolvedTo(true);
await expectAsync(guard.checkRoutePermissions("restricted")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("pathA")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("pathA/1")).toBeResolvedTo(
true,
);
});

it("should checkRoutePermissions considering nested child routes", () => {
it("should checkRoutePermissions considering nested child routes", async () => {
const nestedRoute: Route = {
path: "nested",
children: [
Expand All @@ -130,15 +139,31 @@ describe("UserRoleGuard", () => {
router.config.push(onParentRoute);

userSubject.next(normalUser);
expect(guard.checkRoutePermissions("nested")).toBeFalse();
expect(guard.checkRoutePermissions("nested/X")).toBeTrue();
expect(guard.checkRoutePermissions("on-parent")).toBeFalse();
expect(guard.checkRoutePermissions("on-parent/X")).toBeFalse();
await expectAsync(guard.checkRoutePermissions("nested")).toBeResolvedTo(
false,
);
await expectAsync(guard.checkRoutePermissions("nested/X")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("on-parent")).toBeResolvedTo(
false,
);
await expectAsync(
guard.checkRoutePermissions("on-parent/X"),
).toBeResolvedTo(false);

userSubject.next(adminUser);
expect(guard.checkRoutePermissions("nested")).toBeTrue();
expect(guard.checkRoutePermissions("nested/X")).toBeTrue();
expect(guard.checkRoutePermissions("on-parent")).toBeTrue();
expect(guard.checkRoutePermissions("on-parent/X")).toBeTrue();
await expectAsync(guard.checkRoutePermissions("nested")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("nested/X")).toBeResolvedTo(
true,
);
await expectAsync(guard.checkRoutePermissions("on-parent")).toBeResolvedTo(
true,
);
await expectAsync(
guard.checkRoutePermissions("on-parent/X"),
).toBeResolvedTo(true);
});
});
Loading