1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-22 16:29:09 +01:00

Restructure the org-permissions guard to be Angular 17+ compliant (#9631)

* Document the `org-permissions` guard in code

* Restructure the `org-permissions` guard to be Angular 17+ compliant

* Update the `org-permissions` guard to use `ToastService`

* Simplify callback function sigantures

* Remove unused test object

* Fix updated route from merge
This commit is contained in:
Addison Beck 2024-07-02 10:53:06 -04:00 committed by GitHub
parent 804cbe6da1
commit b7e3f5bc68
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 129 additions and 123 deletions

View File

@ -1,3 +1,4 @@
import { TestBed } from "@angular/core/testing";
import {
ActivatedRouteSnapshot,
convertToParamMap,
@ -10,10 +11,10 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { ToastService } from "@bitwarden/components";
import { OrganizationPermissionsGuard } from "./org-permissions.guard";
import { organizationPermissionsGuard } from "./org-permissions.guard";
const orgFactory = (props: Partial<Organization> = {}) =>
Object.assign(
@ -32,8 +33,6 @@ describe("Organization Permissions Guard", () => {
let state: MockProxy<RouterStateSnapshot>;
let route: MockProxy<ActivatedRouteSnapshot>;
let organizationPermissionsGuard: OrganizationPermissionsGuard;
beforeEach(() => {
router = mock<Router>();
organizationService = mock<OrganizationService>();
@ -42,24 +41,25 @@ describe("Organization Permissions Guard", () => {
params: {
organizationId: orgFactory().id,
},
data: {
organizationPermissions: null,
},
});
organizationPermissionsGuard = new OrganizationPermissionsGuard(
router,
organizationService,
mock<PlatformUtilsService>(),
mock<I18nService>(),
mock<SyncService>(),
);
TestBed.configureTestingModule({
providers: [
{ provide: Router, useValue: router },
{ provide: OrganizationService, useValue: organizationService },
{ provide: ToastService, useValue: mock<ToastService>() },
{ provide: I18nService, useValue: mock<I18nService>() },
{ provide: SyncService, useValue: mock<SyncService>() },
],
});
});
it("blocks navigation if organization does not exist", async () => {
organizationService.get.mockReturnValue(null);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard()(route, state),
);
expect(actual).not.toBe(true);
});
@ -68,22 +68,22 @@ describe("Organization Permissions Guard", () => {
const org = orgFactory();
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(async () =>
organizationPermissionsGuard()(route, state),
);
expect(actual).toBe(true);
});
it("permits navigation if the user has permissions", async () => {
const permissionsCallback = jest.fn();
permissionsCallback.mockImplementation((org) => true);
route.data = {
organizationPermissions: permissionsCallback,
};
permissionsCallback.mockImplementation((_org) => true);
const org = orgFactory();
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard(permissionsCallback)(route, state),
);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).toBe(true);
@ -92,10 +92,7 @@ describe("Organization Permissions Guard", () => {
describe("if the user does not have permissions", () => {
it("and there is no Item ID, block navigation", async () => {
const permissionsCallback = jest.fn();
permissionsCallback.mockImplementation((org) => false);
route.data = {
organizationPermissions: permissionsCallback,
};
permissionsCallback.mockImplementation((_org) => false);
state = mock<RouterStateSnapshot>({
root: mock<ActivatedRouteSnapshot>({
@ -106,16 +103,15 @@ describe("Organization Permissions Guard", () => {
const org = orgFactory();
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard(permissionsCallback)(route, state),
);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).not.toBe(true);
});
it("and there is an Item ID, redirect to the item in the individual vault", async () => {
route.data = {
organizationPermissions: (org: Organization) => false,
};
state = mock<RouterStateSnapshot>({
root: mock<ActivatedRouteSnapshot>({
queryParamMap: convertToParamMap({
@ -126,7 +122,9 @@ describe("Organization Permissions Guard", () => {
const org = orgFactory();
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard((_org: Organization) => false)(route, state),
);
expect(router.createUrlTree).toHaveBeenCalledWith(["/vault"], {
queryParams: { itemId: "myItemId" },
@ -143,7 +141,9 @@ describe("Organization Permissions Guard", () => {
});
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard()(route, state),
);
expect(actual).not.toBe(true);
});
@ -155,7 +155,9 @@ describe("Organization Permissions Guard", () => {
});
organizationService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await organizationPermissionsGuard.canActivate(route, state);
const actual = await TestBed.runInInjectionContext(
async () => await organizationPermissionsGuard()(route, state),
);
expect(actual).toBe(true);
});

View File

@ -1,5 +1,10 @@
import { Injectable } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from "@angular/router";
import { inject } from "@angular/core";
import {
ActivatedRouteSnapshot,
CanActivateFn,
Router,
RouterStateSnapshot,
} from "@angular/router";
import {
canAccessOrgAdmin,
@ -7,43 +12,58 @@ import {
} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { ToastService } from "@bitwarden/components";
@Injectable({
providedIn: "root",
})
export class OrganizationPermissionsGuard implements CanActivate {
constructor(
private router: Router,
private organizationService: OrganizationService,
private platformUtilsService: PlatformUtilsService,
private i18nService: I18nService,
private syncService: SyncService,
) {}
/**
* `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 organization in the URL parameters, they
* are redirected to the home screen.
* 2. If the organization 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 callback one of the following
* will happen:
*
* 1. If the logged in user does not have the required permissions they are
* redirected to `/organizations/{id}` or `/` based on admin console access
* permissions.
* 2. If the logged in user does have the required permissions navigation
* proceeds as expected.
*/
export function organizationPermissionsGuard(
permissionsCallback?: (organization: Organization) => boolean,
): CanActivateFn {
return async (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => {
const router = inject(Router);
const organizationService = inject(OrganizationService);
const toastService = inject(ToastService);
const i18nService = inject(I18nService);
const syncService = inject(SyncService);
async canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
// TODO: We need to fix this issue once and for all.
if ((await this.syncService.getLastSync()) == null) {
await this.syncService.fullSync(false);
// TODO: We need to fix issue once and for all.
if ((await syncService.getLastSync()) == null) {
await syncService.fullSync(false);
}
const org = await this.organizationService.get(route.params.organizationId);
const org = await organizationService.get(route.params.organizationId);
if (org == null) {
return this.router.createUrlTree(["/"]);
return router.createUrlTree(["/"]);
}
if (!org.isOwner && !org.enabled) {
this.platformUtilsService.showToast(
"error",
null,
this.i18nService.t("organizationIsDisabled"),
);
return this.router.createUrlTree(["/"]);
toastService.showToast({
variant: "error",
title: null,
message: i18nService.t("organizationIsDisabled"),
});
return router.createUrlTree(["/"]);
}
const permissionsCallback: (organization: Organization) => boolean =
route.data?.organizationPermissions;
const hasPermissions = permissionsCallback == null || permissionsCallback(org);
if (!hasPermissions) {
@ -52,19 +72,23 @@ export class OrganizationPermissionsGuard implements CanActivate {
const cipherId =
state.root.queryParamMap.get("itemId") || state.root.queryParamMap.get("cipherId");
if (cipherId) {
return this.router.createUrlTree(["/vault"], {
return router.createUrlTree(["/vault"], {
queryParams: {
itemId: cipherId,
},
});
}
this.platformUtilsService.showToast("error", null, this.i18nService.t("accessDenied"));
toastService.showToast({
variant: "error",
title: null,
message: i18nService.t("accessDenied"),
});
return canAccessOrgAdmin(org)
? this.router.createUrlTree(["/organizations", org.id])
: this.router.createUrlTree(["/"]);
? router.createUrlTree(["/organizations", org.id])
: router.createUrlTree(["/"]);
}
return true;
}
};
}

View File

@ -3,7 +3,7 @@ import { RouterModule, Routes } from "@angular/router";
import { canAccessMembersTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { OrganizationPermissionsGuard } from "../guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../guards/org-permissions.guard";
import { MembersComponent } from "./members.component";
@ -11,10 +11,9 @@ const routes: Routes = [
{
path: "",
component: MembersComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard(canAccessMembersTab)],
data: {
titleId: "members",
organizationPermissions: canAccessMembersTab,
},
},
];

View File

@ -12,7 +12,7 @@ import {
} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationRedirectGuard } from "../../admin-console/organizations/guards/org-redirect.guard";
import { OrganizationLayoutComponent } from "../../admin-console/organizations/layouts/organization-layout.component";
import { GroupsComponent } from "../../admin-console/organizations/manage/groups.component";
@ -23,10 +23,7 @@ const routes: Routes = [
{
path: ":organizationId",
component: OrganizationLayoutComponent,
canActivate: [deepLinkGuard(), AuthGuard, OrganizationPermissionsGuard],
data: {
organizationPermissions: canAccessOrgAdmin,
},
canActivate: [deepLinkGuard(), AuthGuard, organizationPermissionsGuard(canAccessOrgAdmin)],
children: [
{
path: "",
@ -52,10 +49,9 @@ const routes: Routes = [
{
path: "groups",
component: GroupsComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard(canAccessGroupsTab)],
data: {
titleId: "groups",
organizationPermissions: canAccessGroupsTab,
},
},
{

View File

@ -10,7 +10,7 @@ import { ReusedPasswordsReportComponent } from "../../../admin-console/organizat
import { UnsecuredWebsitesReportComponent } from "../../../admin-console/organizations/tools/unsecured-websites-report.component";
import { WeakPasswordsReportComponent } from "../../../admin-console/organizations/tools/weak-passwords-report.component";
import { isPaidOrgGuard } from "../guards/is-paid-org.guard";
import { OrganizationPermissionsGuard } from "../guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../guards/org-permissions.guard";
import { organizationRedirectGuard } from "../guards/org-redirect.guard";
import { EventsComponent } from "../manage/events.component";
@ -19,8 +19,7 @@ import { ReportsHomeComponent } from "./reports-home.component";
const routes: Routes = [
{
path: "",
canActivate: [OrganizationPermissionsGuard],
data: { organizationPermissions: canAccessReportingTab },
canActivate: [organizationPermissionsGuard(canAccessReportingTab)],
children: [
{
path: "",
@ -31,7 +30,7 @@ const routes: Routes = [
{
path: "reports",
component: ReportsHomeComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard()],
data: {
titleId: "reports",
},
@ -81,10 +80,9 @@ const routes: Routes = [
{
path: "events",
component: EventsComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canAccessEventLogs)],
data: {
titleId: "eventLogs",
organizationPermissions: (org: Organization) => org.canAccessEventLogs,
},
},
],

View File

@ -4,7 +4,7 @@ import { RouterModule, Routes } from "@angular/router";
import { canAccessSettingsTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationPermissionsGuard } from "../../organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../../organizations/guards/org-permissions.guard";
import { organizationRedirectGuard } from "../../organizations/guards/org-redirect.guard";
import { PoliciesComponent } from "../../organizations/policies";
@ -14,8 +14,7 @@ import { TwoFactorSetupComponent } from "./two-factor-setup.component";
const routes: Routes = [
{
path: "",
canActivate: [OrganizationPermissionsGuard],
data: { organizationPermissions: canAccessSettingsTab },
canActivate: [organizationPermissionsGuard(canAccessSettingsTab)],
children: [
{
path: "",
@ -32,9 +31,8 @@ const routes: Routes = [
{
path: "policies",
component: PoliciesComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canManagePolicies)],
data: {
organizationPermissions: (org: Organization) => org.canManagePolicies,
titleId: "policies",
},
},
@ -45,10 +43,9 @@ const routes: Routes = [
path: "import",
loadComponent: () =>
import("./org-import.component").then((mod) => mod.OrgImportComponent),
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canAccessImportExport)],
data: {
titleId: "importData",
organizationPermissions: (org: Organization) => org.canAccessImportExport,
},
},
{
@ -57,10 +54,9 @@ const routes: Routes = [
import("../tools/vault-export/org-vault-export.component").then(
(mod) => mod.OrganizationVaultExportComponent,
),
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canAccessImportExport)],
data: {
titleId: "exportVault",
organizationPermissions: (org: Organization) => org.canAccessImportExport,
},
},
],

View File

@ -2,9 +2,8 @@ import { NgModule } from "@angular/core";
import { RouterModule, Routes } from "@angular/router";
import { canAccessBillingTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationIsUnmanaged } from "../../billing/guards/organization-is-unmanaged.guard";
import { WebPlatformUtilsService } from "../../core/web-platform-utils.service";
import { PaymentMethodComponent } from "../shared";
@ -16,8 +15,7 @@ import { OrganizationSubscriptionSelfhostComponent } from "./organization-subscr
const routes: Routes = [
{
path: "",
canActivate: [OrganizationPermissionsGuard],
data: { organizationPermissions: canAccessBillingTab },
canActivate: [organizationPermissionsGuard(canAccessBillingTab)],
children: [
{ path: "", pathMatch: "full", redirectTo: "subscription" },
{
@ -30,19 +28,23 @@ const routes: Routes = [
{
path: "payment-method",
component: PaymentMethodComponent,
canActivate: [OrganizationPermissionsGuard, organizationIsUnmanaged],
canActivate: [
organizationPermissionsGuard((org) => org.canEditPaymentMethods),
organizationIsUnmanaged,
],
data: {
titleId: "paymentMethod",
organizationPermissions: (org: Organization) => org.canEditPaymentMethods,
},
},
{
path: "history",
component: OrgBillingHistoryViewComponent,
canActivate: [OrganizationPermissionsGuard, organizationIsUnmanaged],
canActivate: [
organizationPermissionsGuard((org) => org.canViewBillingHistory),
organizationIsUnmanaged,
],
data: {
titleId: "billingHistory",
organizationPermissions: (org: Organization) => org.canViewBillingHistory,
},
},
],

View File

@ -3,15 +3,15 @@ import { RouterModule, Routes } from "@angular/router";
import { canAccessVaultTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { OrganizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard";
import { VaultComponent } from "./vault.component";
const routes: Routes = [
{
path: "",
component: VaultComponent,
canActivate: [OrganizationPermissionsGuard],
data: { titleId: "vaults", organizationPermissions: canAccessVaultTab },
canActivate: [organizationPermissionsGuard(canAccessVaultTab)],
data: { titleId: "vaults" },
},
];
@NgModule({

View File

@ -3,8 +3,7 @@ import { RouterModule, Routes } from "@angular/router";
import { AuthGuard } from "@bitwarden/angular/auth/guards";
import { canAccessSettingsTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationPermissionsGuard } from "@bitwarden/web-vault/app/admin-console/organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "@bitwarden/web-vault/app/admin-console/organizations/guards/org-permissions.guard";
import { OrganizationLayoutComponent } from "@bitwarden/web-vault/app/admin-console/organizations/layouts/organization-layout.component";
import { SsoComponent } from "../../auth/sso/sso.component";
@ -16,40 +15,34 @@ const routes: Routes = [
{
path: "organizations/:organizationId",
component: OrganizationLayoutComponent,
canActivate: [AuthGuard, OrganizationPermissionsGuard],
canActivate: [AuthGuard, organizationPermissionsGuard()],
children: [
{
path: "settings",
canActivate: [OrganizationPermissionsGuard],
data: {
organizationPermissions: canAccessSettingsTab,
},
canActivate: [organizationPermissionsGuard(canAccessSettingsTab)],
children: [
{
path: "domain-verification",
component: DomainVerificationComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canManageDomainVerification)],
data: {
titleId: "domainVerification",
organizationPermissions: (org: Organization) => org.canManageDomainVerification,
},
},
{
path: "sso",
component: SsoComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canManageSso)],
data: {
titleId: "singleSignOn",
organizationPermissions: (org: Organization) => org.canManageSso,
},
},
{
path: "scim",
component: ScimComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canManageScim)],
data: {
titleId: "scim",
organizationPermissions: (org: Organization) => org.canManageScim,
},
},
{
@ -58,9 +51,8 @@ const routes: Routes = [
import("./manage/device-approvals/device-approvals.component").then(
(mod) => mod.DeviceApprovalsComponent,
),
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.canManageDeviceApprovals)],
data: {
organizationPermissions: (org: Organization) => org.canManageDeviceApprovals,
titleId: "deviceApprovals",
},
},

View File

@ -1,8 +1,7 @@
import { NgModule } from "@angular/core";
import { RouterModule, Routes } from "@angular/router";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationPermissionsGuard } from "@bitwarden/web-vault/app/admin-console/organizations/guards/org-permissions.guard";
import { organizationPermissionsGuard } from "@bitwarden/web-vault/app/admin-console/organizations/guards/org-permissions.guard";
import { SecretsManagerExportComponent } from "./porting/sm-export.component";
import { SecretsManagerImportComponent } from "./porting/sm-import.component";
@ -11,19 +10,17 @@ const routes: Routes = [
{
path: "import",
component: SecretsManagerImportComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.isAdmin)],
data: {
titleId: "importData",
organizationPermissions: (org: Organization) => org.isAdmin,
},
},
{
path: "export",
component: SecretsManagerExportComponent,
canActivate: [OrganizationPermissionsGuard],
canActivate: [organizationPermissionsGuard((org) => org.isAdmin)],
data: {
titleId: "exportData",
organizationPermissions: (org: Organization) => org.isAdmin,
},
},
];