1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-19 01:33:33 +00:00

[EC-416] Refactor organization permission checks (#3252)

* Replace Permissions enum and helper methods with callbacks

* Remove scim feature flag

* Check if org has feature enabled as part of canManage checks

* Pin jest-mock-extended at v2.0.6 to fix compilation error
This commit is contained in:
Thomas Rittson
2022-08-16 00:08:06 +10:00
committed by GitHub
parent 96d5f50c7f
commit d30701ada7
32 changed files with 474 additions and 282 deletions

View File

@@ -2,12 +2,12 @@ import { NgModule } from "@angular/core";
import { RouterModule, Routes } from "@angular/router";
import { AuthGuard } from "@bitwarden/angular/guards/auth.guard";
import { Permissions } from "@bitwarden/common/enums/permissions";
import { Organization } from "@bitwarden/common/models/domain/organization";
import { PermissionsGuard } from "src/app/organizations/guards/permissions.guard";
import { OrganizationPermissionsGuard } from "src/app/organizations/guards/org-permissions.guard";
import { OrganizationLayoutComponent } from "src/app/organizations/layouts/organization-layout.component";
import { ManageComponent } from "src/app/organizations/manage/manage.component";
import { NavigationPermissionsService } from "src/app/organizations/services/navigation-permissions.service";
import { canAccessManageTab } from "src/app/organizations/navigation-permissions";
import { ScimComponent } from "./manage/scim.component";
import { SsoComponent } from "./manage/sso.component";
@@ -16,30 +16,30 @@ const routes: Routes = [
{
path: "organizations/:organizationId",
component: OrganizationLayoutComponent,
canActivate: [AuthGuard, PermissionsGuard],
canActivate: [AuthGuard, OrganizationPermissionsGuard],
children: [
{
path: "manage",
component: ManageComponent,
canActivate: [PermissionsGuard],
canActivate: [OrganizationPermissionsGuard],
data: {
permissions: NavigationPermissionsService.getPermissions("manage"),
organizationPermissions: canAccessManageTab,
},
children: [
{
path: "sso",
component: SsoComponent,
canActivate: [PermissionsGuard],
canActivate: [OrganizationPermissionsGuard],
data: {
permissions: [Permissions.ManageSso],
organizationPermissions: (org: Organization) => org.canManageSso,
},
},
{
path: "scim",
component: ScimComponent,
canActivate: [PermissionsGuard],
canActivate: [OrganizationPermissionsGuard],
data: {
permissions: [Permissions.ManageScim],
organizationPermissions: (org: Organization) => org.canManageScim,
},
},
],

View File

@@ -0,0 +1,124 @@
import { ActivatedRouteSnapshot, Router } from "@angular/router";
import { mock, MockProxy } from "jest-mock-extended";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
import { ProviderService } from "@bitwarden/common/abstractions/provider.service";
import { ProviderUserType } from "@bitwarden/common/enums/providerUserType";
import { Provider } from "@bitwarden/common/models/domain/provider";
import { ProviderPermissionsGuard } from "./provider-permissions.guard";
const providerFactory = (props: Partial<Provider> = {}) =>
Object.assign(
new Provider(),
{
id: "myProviderId",
enabled: true,
type: ProviderUserType.ServiceUser,
},
props
);
describe("Provider Permissions Guard", () => {
let router: MockProxy<Router>;
let providerService: MockProxy<ProviderService>;
let route: MockProxy<ActivatedRouteSnapshot>;
let providerPermissionsGuard: ProviderPermissionsGuard;
beforeEach(() => {
router = mock<Router>();
providerService = mock<ProviderService>();
route = mock<ActivatedRouteSnapshot>({
params: {
providerId: providerFactory().id,
},
data: {
providerPermissions: null,
},
});
providerPermissionsGuard = new ProviderPermissionsGuard(
providerService,
router,
mock<PlatformUtilsService>(),
mock<I18nService>()
);
});
it("blocks navigation if provider does not exist", async () => {
providerService.get.mockResolvedValue(null);
const actual = await providerPermissionsGuard.canActivate(route);
expect(actual).not.toBe(true);
});
it("permits navigation if no permissions are specified", async () => {
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
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,
};
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).toBe(true);
});
it("blocks navigation if the user does not have permissions", async () => {
const permissionsCallback = jest.fn();
permissionsCallback.mockImplementation((org) => false);
route.data = {
providerPermissions: permissionsCallback,
};
const provider = providerFactory();
providerService.get.calledWith(provider.id).mockResolvedValue(provider);
const actual = await providerPermissionsGuard.canActivate(route);
expect(permissionsCallback).toHaveBeenCalled();
expect(actual).not.toBe(true);
});
describe("given a disabled organization", () => {
it("blocks navigation if user is not an admin", async () => {
const org = providerFactory({
type: ProviderUserType.ServiceUser,
enabled: false,
});
providerService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await providerPermissionsGuard.canActivate(route);
expect(actual).not.toBe(true);
});
it("permits navigation if user is an admin", async () => {
const org = providerFactory({
type: ProviderUserType.ProviderAdmin,
enabled: false,
});
providerService.get.calledWith(org.id).mockResolvedValue(org);
const actual = await providerPermissionsGuard.canActivate(route);
expect(actual).toBe(true);
});
});
});

View File

@@ -4,26 +4,34 @@ import { ActivatedRouteSnapshot, CanActivate, Router } from "@angular/router";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
import { ProviderService } from "@bitwarden/common/abstractions/provider.service";
import { Provider } from "@bitwarden/common/models/domain/provider";
@Injectable()
export class ProviderGuard implements CanActivate {
export class ProviderPermissionsGuard implements CanActivate {
constructor(
private providerService: ProviderService,
private router: Router,
private platformUtilsService: PlatformUtilsService,
private i18nService: I18nService,
private providerService: ProviderService
private i18nService: I18nService
) {}
async canActivate(route: ActivatedRouteSnapshot) {
const provider = await this.providerService.get(route.params.providerId);
if (provider == null) {
this.router.navigate(["/"]);
return false;
return this.router.createUrlTree(["/"]);
}
if (!provider.isProviderAdmin && !provider.enabled) {
this.platformUtilsService.showToast("error", null, this.i18nService.t("providerIsDisabled"));
this.router.navigate(["/"]);
return false;
return this.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]);
}
return true;

View File

@@ -1,26 +0,0 @@
import { Injectable } from "@angular/core";
import { ActivatedRouteSnapshot, CanActivate, Router } from "@angular/router";
import { ProviderService } from "@bitwarden/common/abstractions/provider.service";
import { Permissions } from "@bitwarden/common/enums/permissions";
@Injectable()
export class PermissionsGuard implements CanActivate {
constructor(private providerService: ProviderService, private router: Router) {}
async canActivate(route: ActivatedRouteSnapshot) {
const provider = await this.providerService.get(route.params.providerId);
const permissions = route.data == null ? null : (route.data.permissions as Permissions[]);
if (
(permissions.indexOf(Permissions.AccessEventLogs) !== -1 && provider.canAccessEventLogs) ||
(permissions.indexOf(Permissions.ManageProvider) !== -1 && provider.isProviderAdmin) ||
(permissions.indexOf(Permissions.ManageUsers) !== -1 && provider.canManageUsers)
) {
return true;
}
this.router.navigate(["/providers", provider.id]);
return false;
}
}

View File

@@ -2,15 +2,14 @@ import { NgModule } from "@angular/core";
import { RouterModule, Routes } from "@angular/router";
import { AuthGuard } from "@bitwarden/angular/guards/auth.guard";
import { Permissions } from "@bitwarden/common/enums/permissions";
import { Provider } from "@bitwarden/common/models/domain/provider";
import { FrontendLayoutComponent } from "src/app/layouts/frontend-layout.component";
import { ProvidersComponent } from "src/app/providers/providers.component";
import { ClientsComponent } from "./clients/clients.component";
import { CreateOrganizationComponent } from "./clients/create-organization.component";
import { PermissionsGuard } from "./guards/provider-type.guard";
import { ProviderGuard } from "./guards/provider.guard";
import { ProviderPermissionsGuard } from "./guards/provider-permissions.guard";
import { AcceptProviderComponent } from "./manage/accept-provider.component";
import { EventsComponent } from "./manage/events.component";
import { ManageComponent } from "./manage/manage.component";
@@ -54,7 +53,7 @@ const routes: Routes = [
{
path: ":providerId",
component: ProvidersLayoutComponent,
canActivate: [ProviderGuard],
canActivate: [ProviderPermissionsGuard],
children: [
{ path: "", pathMatch: "full", redirectTo: "clients" },
{ path: "clients/create", component: CreateOrganizationComponent },
@@ -71,19 +70,19 @@ const routes: Routes = [
{
path: "people",
component: PeopleComponent,
canActivate: [PermissionsGuard],
canActivate: [ProviderPermissionsGuard],
data: {
titleId: "people",
permissions: [Permissions.ManageUsers],
providerPermissions: (provider: Provider) => provider.canManageUsers,
},
},
{
path: "events",
component: EventsComponent,
canActivate: [PermissionsGuard],
canActivate: [ProviderPermissionsGuard],
data: {
titleId: "eventLogs",
permissions: [Permissions.AccessEventLogs],
providerPermissions: (provider: Provider) => provider.canAccessEventLogs,
},
},
],
@@ -100,10 +99,10 @@ const routes: Routes = [
{
path: "account",
component: AccountComponent,
canActivate: [PermissionsGuard],
canActivate: [ProviderPermissionsGuard],
data: {
titleId: "myProvider",
permissions: [Permissions.ManageProvider],
providerPermissions: (provider: Provider) => provider.isProviderAdmin,
},
},
],

View File

@@ -10,8 +10,7 @@ import { OssModule } from "src/app/oss.module";
import { AddOrganizationComponent } from "./clients/add-organization.component";
import { ClientsComponent } from "./clients/clients.component";
import { CreateOrganizationComponent } from "./clients/create-organization.component";
import { PermissionsGuard } from "./guards/provider-type.guard";
import { ProviderGuard } from "./guards/provider.guard";
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";
@@ -46,7 +45,7 @@ import { SetupComponent } from "./setup/setup.component";
SetupProviderComponent,
UserAddEditComponent,
],
providers: [WebProviderService, ProviderGuard, PermissionsGuard],
providers: [WebProviderService, ProviderPermissionsGuard],
})
export class ProvidersModule {
constructor(modalService: ModalService, componentFactoryResolver: ComponentFactoryResolver) {