Restructure the `provider-permissions` guard to be Angular 17+ compliant (#9609)

* Document the `provider-permissions` guard in code

* Restructure the `provider-permissions` guard to be Angular 17+ compliant
This commit is contained in:
Addison Beck 2024-07-02 11:35:51 -04:00 committed by GitHub
parent b7e3f5bc68
commit 72355d26e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 88 additions and 61 deletions

View File

@ -1,4 +1,5 @@
import { ActivatedRouteSnapshot, Router } from "@angular/router";
import { TestBed } from "@angular/core/testing";
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/router";
import { mock, MockProxy } from "jest-mock-extended";
import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service";
@ -7,7 +8,7 @@ import { Provider } from "@bitwarden/common/admin-console/models/domain/provider
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { ProviderPermissionsGuard } from "./provider-permissions.guard";
import { providerPermissionsGuard } from "./provider-permissions.guard";
const providerFactory = (props: Partial<Provider> = {}) =>
Object.assign(
@ -21,14 +22,11 @@ const providerFactory = (props: Partial<Provider> = {}) =>
);
describe("Provider Permissions Guard", () => {
let router: MockProxy<Router>;
let providerService: MockProxy<ProviderService>;
let route: MockProxy<ActivatedRouteSnapshot>;
let providerPermissionsGuard: ProviderPermissionsGuard;
let state: MockProxy<RouterStateSnapshot>;
beforeEach(() => {
router = mock<Router>();
providerService = mock<ProviderService>();
route = mock<ActivatedRouteSnapshot>({
params: {
@ -38,19 +36,22 @@ describe("Provider Permissions Guard", () => {
providerPermissions: null,
},
});
providerPermissionsGuard = new ProviderPermissionsGuard(
providerService,
router,
mock<PlatformUtilsService>(),
mock<I18nService>(),
);
TestBed.configureTestingModule({
providers: [
{ provide: ProviderService, useValue: providerService },
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: I18nService, useValue: mock<I18nService>() },
{ provide: Router, useValue: mock<Router>() },
],
});
});
it("blocks navigation if provider does not exist", async () => {
providerService.get.mockResolvedValue(null);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard()(route, state),
);
expect(actual).not.toBe(true);
});
@ -59,22 +60,23 @@ describe("Provider Permissions Guard", () => {
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard()(route, state),
);
expect(actual).toBe(true);
});
it("permits navigation if the user has permissions", async () => {
const permissionsCallback = jest.fn();
permissionsCallback.mockImplementation((provider) => true);
route.data = {
providerPermissions: permissionsCallback,
};
permissionsCallback.mockImplementation((_provider) => true);
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard(permissionsCallback)(route, state),
);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).toBe(true);
@ -82,15 +84,13 @@ describe("Provider Permissions Guard", () => {
it("blocks navigation if the user does not have permissions", async () => {
const permissionsCallback = jest.fn();
permissionsCallback.mockImplementation((org) => false);
route.data = {
providerPermissions: permissionsCallback,
};
permissionsCallback.mockImplementation((_org) => false);
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard(permissionsCallback)(route, state),
);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).not.toBe(true);
@ -104,7 +104,9 @@ describe("Provider Permissions Guard", () => {
});
providerService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard()(route, state),
);
expect(actual).not.toBe(true);
});
@ -116,7 +118,9 @@ describe("Provider Permissions Guard", () => {
});
providerService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await providerPermissionsGuard.canActivate(route);
const actual = await TestBed.runInInjectionContext(
async () => await providerPermissionsGuard()(route, state),
);
expect(actual).toBe(true);
});

View File

@ -1,39 +1,61 @@
import { Injectable } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivate, Router } from "@angular/router";
import { inject } from "@angular/core";
import {
ActivatedRouteSnapshot,
CanActivateFn,
Router,
RouterStateSnapshot,
} from "@angular/router";
import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service";
import { Provider } from "@bitwarden/common/admin-console/models/domain/provider";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@Injectable()
export class ProviderPermissionsGuard implements CanActivate {
constructor(
private providerService: ProviderService,
private router: Router,
private platformUtilsService: PlatformUtilsService,
private i18nService: I18nService,
) {}
/**
* `CanActivateFn` that asserts the logged in user has permission to access
* the page being navigated to. Two high-level checks are performed:
*
* 1. If the user is not a member of the provider in the URL parameters, they
* are redirected to the home screen.
* 2. If the provider in the URL parameters is disabled and the user is not
* an admin, they are redirected to the home screen.
*
* In addition to these high level checks the guard accepts a callback
* function as an argument that will be called to check for more granular
* permissions. Based on the return from this callback one of the following
* will happen:
*
* 1. If the logged in user does not have the required permissions they are
* redirected to `/providers`.
* 2. If the logged in user does have the required permissions navigation
* proceeds as expected.
*/
export function providerPermissionsGuard(
permissionsCallback?: (provider: Provider) => boolean,
): CanActivateFn {
return async (route: ActivatedRouteSnapshot, _state: RouterStateSnapshot) => {
const providerService = inject(ProviderService);
const router = inject(Router);
const platformUtilsService = inject(PlatformUtilsService);
const i18nService = inject(I18nService);
async canActivate(route: ActivatedRouteSnapshot) {
const provider = await this.providerService.get(route.params.providerId);
const provider = await providerService.get(route.params.providerId);
if (provider == null) {
return this.router.createUrlTree(["/"]);
return router.createUrlTree(["/"]);
}
if (!provider.isProviderAdmin && !provider.enabled) {
this.platformUtilsService.showToast("error", null, this.i18nService.t("providerIsDisabled"));
return this.router.createUrlTree(["/"]);
platformUtilsService.showToast("error", null, i18nService.t("providerIsDisabled"));
return router.createUrlTree(["/"]);
}
const permissionsCallback: (provider: Provider) => boolean = route.data?.providerPermissions;
const hasSpecifiedPermissions = permissionsCallback == null || permissionsCallback(provider);
if (!hasSpecifiedPermissions) {
this.platformUtilsService.showToast("error", null, this.i18nService.t("accessDenied"));
return this.router.createUrlTree(["/providers", provider.id]);
platformUtilsService.showToast("error", null, i18nService.t("accessDenied"));
return router.createUrlTree(["/providers", provider.id]);
}
return true;
}
};
}

View File

@ -17,7 +17,7 @@ import {
import { ClientsComponent } from "./clients/clients.component";
import { CreateOrganizationComponent } from "./clients/create-organization.component";
import { ProviderPermissionsGuard } from "./guards/provider-permissions.guard";
import { providerPermissionsGuard } from "./guards/provider-permissions.guard";
import { AcceptProviderComponent } from "./manage/accept-provider.component";
import { EventsComponent } from "./manage/events.component";
import { PeopleComponent } from "./manage/people.component";
@ -76,7 +76,7 @@ const routes: Routes = [
{
path: ":providerId",
component: ProvidersLayoutComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [providerPermissionsGuard()],
children: [
{ path: "", pathMatch: "full", redirectTo: "clients" },
{ path: "clients/create", component: CreateOrganizationComponent },
@ -98,27 +98,28 @@ const routes: Routes = [
{
path: "people",
component: PeopleComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [
providerPermissionsGuard((provider: Provider) => provider.canManageUsers),
],
data: {
titleId: "people",
providerPermissions: (provider: Provider) => provider.canManageUsers,
},
},
{
path: "events",
component: EventsComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [
providerPermissionsGuard((provider: Provider) => provider.canAccessEventLogs),
],
data: {
titleId: "eventLogs",
providerPermissions: (provider: Provider) => provider.canAccessEventLogs,
},
},
],
},
{
path: "billing",
canActivate: [ProviderPermissionsGuard, hasConsolidatedBilling],
data: { providerPermissions: (provider: Provider) => provider.isProviderAdmin },
canActivate: [hasConsolidatedBilling],
children: [
{
path: "",
@ -128,7 +129,7 @@ const routes: Routes = [
{
path: "subscription",
component: ProviderSubscriptionComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [providerPermissionsGuard()],
data: {
titleId: "subscription",
},
@ -136,7 +137,7 @@ const routes: Routes = [
{
path: "history",
component: ProviderBillingHistoryComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [providerPermissionsGuard()],
data: {
titleId: "billingHistory",
},
@ -154,10 +155,11 @@ const routes: Routes = [
{
path: "account",
component: AccountComponent,
canActivate: [ProviderPermissionsGuard],
canActivate: [
providerPermissionsGuard((provider: Provider) => provider.isProviderAdmin),
],
data: {
titleId: "myProvider",
providerPermissions: (provider: Provider) => provider.isProviderAdmin,
},
},
],

View File

@ -25,7 +25,6 @@ import { ProviderSubscriptionStatusComponent } from "../../billing/providers/sub
import { AddOrganizationComponent } from "./clients/add-organization.component";
import { ClientsComponent } from "./clients/clients.component";
import { CreateOrganizationComponent } from "./clients/create-organization.component";
import { ProviderPermissionsGuard } from "./guards/provider-permissions.guard";
import { AcceptProviderComponent } from "./manage/accept-provider.component";
import { BulkConfirmComponent } from "./manage/bulk/bulk-confirm.component";
import { BulkRemoveComponent } from "./manage/bulk/bulk-remove.component";
@ -77,6 +76,6 @@ import { SetupComponent } from "./setup/setup.component";
ProviderPaymentMethodComponent,
ProviderSubscriptionStatusComponent,
],
providers: [WebProviderService, ProviderPermissionsGuard],
providers: [WebProviderService],
})
export class ProvidersModule {}