diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts index e212264756..7f49c42fb8 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.spec.ts @@ -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 = {}) => Object.assign( @@ -21,14 +22,11 @@ const providerFactory = (props: Partial = {}) => ); describe("Provider Permissions Guard", () => { - let router: MockProxy; let providerService: MockProxy; let route: MockProxy; - - let providerPermissionsGuard: ProviderPermissionsGuard; + let state: MockProxy; beforeEach(() => { - router = mock(); providerService = mock(); route = mock({ params: { @@ -38,19 +36,22 @@ describe("Provider Permissions Guard", () => { providerPermissions: null, }, }); - - providerPermissionsGuard = new ProviderPermissionsGuard( - providerService, - router, - mock(), - mock(), - ); + TestBed.configureTestingModule({ + providers: [ + { provide: ProviderService, useValue: providerService }, + { provide: PlatformUtilsService, useValue: mock() }, + { provide: I18nService, useValue: mock() }, + { provide: Router, useValue: mock() }, + ], + }); }); 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); }); diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts index 3e05d44b5f..6dcaf60466 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/guards/provider-permissions.guard.ts @@ -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; - } + }; } diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-routing.module.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-routing.module.ts index 29f37d2b58..797b7b06f1 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-routing.module.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-routing.module.ts @@ -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, }, }, ], diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts index 110796ea5c..46adf44d5a 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts @@ -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 {}