From 9e9f977eb3b2f662080b5b446fe5fca883f9001e Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 22 Nov 2024 07:58:03 +1000 Subject: [PATCH] [PM-11360] Remove export permission for providers (#12062) * Split organization.canAccessImportExport * Fix import permission to include CanCreateNewCollections * Remove provider export permission (feature flagged) --- apps/cli/src/tools/import.command.ts | 2 +- .../organization-layout.component.html | 4 +- .../layouts/organization-layout.component.ts | 40 ++++++++----------- .../organization-settings-routing.module.ts | 32 ++++++++++++--- .../navigation-switcher.stories.ts | 24 +++++++++-- .../product-switcher.stories.ts | 24 +++++++++-- .../shared/product-switcher.service.spec.ts | 23 +++++++++-- .../organization.service.abstraction.ts | 31 +------------- .../vnext.organization.service.ts | 19 +-------- .../models/domain/organization.ts | 23 ++++++++++- libs/common/src/enums/feature-flag.enum.ts | 2 + .../src/components/import.component.ts | 15 +++---- 12 files changed, 142 insertions(+), 97 deletions(-) diff --git a/apps/cli/src/tools/import.command.ts b/apps/cli/src/tools/import.command.ts index 1cb3ac19f6..879b2313ba 100644 --- a/apps/cli/src/tools/import.command.ts +++ b/apps/cli/src/tools/import.command.ts @@ -27,7 +27,7 @@ export class ImportCommand { ); } - if (!organization.canAccessImportExport) { + if (!organization.canAccessImport) { return Response.badRequest( "You are not authorized to import into the provided organization.", ); diff --git a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.html b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.html index aaba492dff..1e811b6c29 100644 --- a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.html +++ b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.html @@ -84,12 +84,12 @@ canAccessOrgAdmin(org); organization$: Observable; + canAccessExport$: Observable; showPaymentAndHistory$: Observable; hideNewOrgButton$: Observable; organizationIsUnmanaged$: Observable; isAccessIntelligenceFeatureEnabled = false; - private _destroy = new Subject(); - constructor( private route: ActivatedRoute, private organizationService: OrganizationService, @@ -71,23 +70,23 @@ export class OrganizationLayoutComponent implements OnInit, OnDestroy { FeatureFlag.AccessIntelligence, ); - this.organization$ = this.route.params - .pipe(takeUntil(this._destroy)) - .pipe(map((p) => p.organizationId)) - .pipe( - mergeMap((id) => { - return this.organizationService.organizations$ - .pipe(takeUntil(this._destroy)) - .pipe(getOrganizationById(id)); - }), - ); + this.organization$ = this.route.params.pipe( + map((p) => p.organizationId), + switchMap((id) => this.organizationService.organizations$.pipe(getById(id))), + filter((org) => org != null), + ); + + this.canAccessExport$ = combineLatest([ + this.organization$, + this.configService.getFeatureFlag$(FeatureFlag.PM11360RemoveProviderExportPermission), + ]).pipe(map(([org, removeProviderExport]) => org.canAccessExport(removeProviderExport))); this.showPaymentAndHistory$ = this.organization$.pipe( map( (org) => !this.platformUtilsService.isSelfHost() && - org?.canViewBillingHistory && - org?.canEditPaymentMethods, + org.canViewBillingHistory && + org.canEditPaymentMethods, ), ); @@ -107,11 +106,6 @@ export class OrganizationLayoutComponent implements OnInit, OnDestroy { ); } - ngOnDestroy() { - this._destroy.next(); - this._destroy.complete(); - } - canShowVaultTab(organization: Organization): boolean { return canAccessVaultTab(organization); } diff --git a/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts b/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts index 4e9180ef12..d7fad9fded 100644 --- a/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts +++ b/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts @@ -1,8 +1,11 @@ -import { NgModule } from "@angular/core"; -import { RouterModule, Routes } from "@angular/router"; +import { inject, NgModule } from "@angular/core"; +import { CanMatchFn, RouterModule, Routes } from "@angular/router"; +import { map } from "rxjs"; import { canAccessSettingsTab } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { organizationPermissionsGuard } from "../../organizations/guards/org-permissions.guard"; import { organizationRedirectGuard } from "../../organizations/guards/org-redirect.guard"; @@ -11,6 +14,11 @@ import { PoliciesComponent } from "../../organizations/policies"; import { AccountComponent } from "./account.component"; import { TwoFactorSetupComponent } from "./two-factor-setup.component"; +const removeProviderExportPermission$: CanMatchFn = () => + inject(ConfigService) + .getFeatureFlag$(FeatureFlag.PM11360RemoveProviderExportPermission) + .pipe(map((removeProviderExport) => removeProviderExport === true)); + const routes: Routes = [ { path: "", @@ -53,18 +61,32 @@ const routes: Routes = [ path: "import", loadComponent: () => import("./org-import.component").then((mod) => mod.OrgImportComponent), - canActivate: [organizationPermissionsGuard((org) => org.canAccessImportExport)], + canActivate: [organizationPermissionsGuard((org) => org.canAccessImport)], data: { titleId: "importData", }, }, + + // Export routing is temporarily duplicated to set the flag value passed into org.canAccessExport + { + path: "export", + loadComponent: () => + import("../tools/vault-export/org-vault-export.component").then( + (mod) => mod.OrganizationVaultExportComponent, + ), + canMatch: [removeProviderExportPermission$], // if this matches, the flag is ON + canActivate: [organizationPermissionsGuard((org) => org.canAccessExport(true))], + data: { + titleId: "exportVault", + }, + }, { path: "export", loadComponent: () => import("../tools/vault-export/org-vault-export.component").then( (mod) => mod.OrganizationVaultExportComponent, ), - canActivate: [organizationPermissionsGuard((org) => org.canAccessImportExport)], + canActivate: [organizationPermissionsGuard((org) => org.canAccessExport(false))], data: { titleId: "exportVault", }, @@ -82,7 +104,7 @@ function getSettingsRoute(organization: Organization) { if (organization.canManagePolicies) { return "policies"; } - if (organization.canAccessImportExport) { + if (organization.canAccessImport) { return ["tools", "import"]; } if (organization.canManageSso) { diff --git a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts index cd1a77a9ec..a7ff50b426 100644 --- a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts @@ -152,7 +152,13 @@ export const SMAvailable: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: false, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: false, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [], }, @@ -162,7 +168,13 @@ export const SMAndACAvailable: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: true, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: true, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [], }, @@ -172,7 +184,13 @@ export const WithAllOptions: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: true, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: true, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [{ id: "provider-a" }] as Provider[], }, diff --git a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts index b9d1d39492..b53d0243f6 100644 --- a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts @@ -171,7 +171,13 @@ export const WithSM: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: false, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: false, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [], }, @@ -181,7 +187,13 @@ export const WithSMAndAC: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: true, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: true, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [], }, @@ -191,7 +203,13 @@ export const WithAllOptions: Story = { ...Template, args: { mockOrgs: [ - { id: "org-a", canManageUsers: true, canAccessSecretsManager: true, enabled: true }, + { + id: "org-a", + canManageUsers: true, + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => false, + }, ] as Organization[], mockProviders: [{ id: "provider-a" }] as Provider[], }, diff --git a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts index 07a41e7c94..7c53cd86d3 100644 --- a/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts +++ b/apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts @@ -110,7 +110,12 @@ describe("ProductSwitcherService", () => { it("is included in bento when there is an organization with SM", async () => { organizationService.organizations$ = of([ - { id: "1234", canAccessSecretsManager: true, enabled: true }, + { + id: "1234", + canAccessSecretsManager: true, + enabled: true, + canAccessExport: (_) => true, + }, ] as Organization[]); initiateService(); @@ -220,8 +225,20 @@ describe("ProductSwitcherService", () => { router.url = "/sm/4243"; organizationService.organizations$ = of([ - { id: "23443234", canAccessSecretsManager: true, enabled: true, name: "Org 2" }, - { id: "4243", canAccessSecretsManager: true, enabled: true, name: "Org 32" }, + { + id: "23443234", + canAccessSecretsManager: true, + enabled: true, + name: "Org 2", + canAccessExport: (_) => true, + }, + { + id: "4243", + canAccessSecretsManager: true, + enabled: true, + name: "Org 32", + canAccessExport: (_) => true, + }, ] as Organization[]); initiateService(); diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index a2ea6aa886..e687c0a34a 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -1,7 +1,5 @@ import { map, Observable } from "rxjs"; -import { I18nService } from "../../../platform/abstractions/i18n.service"; -import { Utils } from "../../../platform/misc/utils"; import { UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; @@ -16,7 +14,8 @@ export function canAccessSettingsTab(org: Organization): boolean { org.canManagePolicies || org.canManageSso || org.canManageScim || - org.canAccessImportExport || + org.canAccessImport || + org.canAccessExport(false) || // Feature flag value doesn't matter here, providers will have access to this group anyway org.canManageDeviceApprovals ); } @@ -56,32 +55,6 @@ export function getOrganizationById(id: string) { return map((orgs) => orgs.find((o) => o.id === id)); } -export function canAccessAdmin(i18nService: I18nService) { - return map((orgs) => - orgs.filter(canAccessOrgAdmin).sort(Utils.getSortFunction(i18nService, "name")), - ); -} - -/** - * @deprecated - * To be removed after Flexible Collections. - **/ -export function canAccessImportExport(i18nService: I18nService) { - return map((orgs) => - orgs - .filter((org) => org.canAccessImportExport) - .sort(Utils.getSortFunction(i18nService, "name")), - ); -} - -export function canAccessImport(i18nService: I18nService) { - return map((orgs) => - orgs - .filter((org) => org.canAccessImportExport || org.canCreateNewCollections) - .sort(Utils.getSortFunction(i18nService, "name")), - ); -} - /** * Returns `true` if a user is a member of an organization (rather than only being a ProviderUser) * @deprecated Use organizationService.organizations$ with a filter instead diff --git a/libs/common/src/admin-console/abstractions/organization/vnext.organization.service.ts b/libs/common/src/admin-console/abstractions/organization/vnext.organization.service.ts index 45c00e3fd4..68c61e298a 100644 --- a/libs/common/src/admin-console/abstractions/organization/vnext.organization.service.ts +++ b/libs/common/src/admin-console/abstractions/organization/vnext.organization.service.ts @@ -1,7 +1,5 @@ import { map, Observable } from "rxjs"; -import { I18nService } from "../../../platform/abstractions/i18n.service"; -import { Utils } from "../../../platform/misc/utils"; import { UserId } from "../../../types/guid"; import { OrganizationData } from "../../models/data/organization.data"; import { Organization } from "../../models/domain/organization"; @@ -16,7 +14,8 @@ export function canAccessSettingsTab(org: Organization): boolean { org.canManagePolicies || org.canManageSso || org.canManageScim || - org.canAccessImportExport || + org.canAccessImport || + org.canAccessExport(false) || // Feature flag value doesn't matter here, providers will have access to this group anyway org.canManageDeviceApprovals ); } @@ -56,20 +55,6 @@ export function getOrganizationById(id: string) { return map((orgs) => orgs.find((o) => o.id === id)); } -export function canAccessAdmin(i18nService: I18nService) { - return map((orgs) => - orgs.filter(canAccessOrgAdmin).sort(Utils.getSortFunction(i18nService, "name")), - ); -} - -export function canAccessImport(i18nService: I18nService) { - return map((orgs) => - orgs - .filter((org) => org.canAccessImportExport || org.canCreateNewCollections) - .sort(Utils.getSortFunction(i18nService, "name")), - ); -} - /** * Publishes an observable stream of organizations. This service is meant to * be used widely across Bitwarden as the primary way of fetching organizations. diff --git a/libs/common/src/admin-console/models/domain/organization.ts b/libs/common/src/admin-console/models/domain/organization.ts index 070617b9e5..2ff17ca119 100644 --- a/libs/common/src/admin-console/models/domain/organization.ts +++ b/libs/common/src/admin-console/models/domain/organization.ts @@ -168,8 +168,27 @@ export class Organization { return (this.isAdmin || this.permissions.accessEventLogs) && this.useEvents; } - get canAccessImportExport() { - return this.isAdmin || this.permissions.accessImportExport; + get canAccessImport() { + return ( + this.isProviderUser || + this.type === OrganizationUserType.Owner || + this.type === OrganizationUserType.Admin || + this.permissions.accessImportExport || + this.canCreateNewCollections // To allow users to create collections and then import into them + ); + } + + canAccessExport(removeProviderExport: boolean) { + if (!removeProviderExport && this.isProviderUser) { + return true; + } + + return ( + this.isMember && + (this.type === OrganizationUserType.Owner || + this.type === OrganizationUserType.Admin || + this.permissions.accessImportExport) + ); } get canAccessReports() { diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index ec5a8e47d7..6dafe8fc7f 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -40,6 +40,7 @@ export enum FeatureFlag { NewDeviceVerificationTemporaryDismiss = "new-device-temporary-dismiss", NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss", DisableFreeFamiliesSponsorship = "PM-12274-disable-free-families-sponsorship", + PM11360RemoveProviderExportPermission = "pm-11360-remove-provider-export-permission", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -90,6 +91,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.NewDeviceVerificationTemporaryDismiss]: FALSE, [FeatureFlag.NewDeviceVerificationPermanentDismiss]: FALSE, [FeatureFlag.DisableFreeFamiliesSponsorship]: FALSE, + [FeatureFlag.PM11360RemoveProviderExportPermission]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; diff --git a/libs/importer/src/components/import.component.ts b/libs/importer/src/components/import.component.ts index 1ffe2728b0..1ab300fc37 100644 --- a/libs/importer/src/components/import.component.ts +++ b/libs/importer/src/components/import.component.ts @@ -21,10 +21,7 @@ import { JslibModule } from "@bitwarden/angular/jslib.module"; import { safeProvider, SafeProvider } from "@bitwarden/angular/platform/utils/safe-provider"; import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { - canAccessImport, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -226,7 +223,7 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { this.setImportOptions(); await this.initializeOrganizations(); - if (this.organizationId && (await this.canAccessImportExport(this.organizationId))) { + if (this.organizationId && (await this.canAccessImport(this.organizationId))) { this.handleOrganizationImportInit(); } else { this.handleImportInit(); @@ -289,7 +286,7 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { private async initializeOrganizations() { this.organizations$ = concat( this.organizationService.memberOrganizations$.pipe( - canAccessImport(this.i18nService), + map((orgs) => orgs.filter((org) => org.canAccessImport)), map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name"))), ), ); @@ -375,7 +372,7 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { importContents, this.organizationId, this.formGroup.controls.targetSelector.value, - (await this.canAccessImportExport(this.organizationId)) && this.isFromAC, + (await this.canAccessImport(this.organizationId)) && this.isFromAC, ); //No errors, display success message @@ -395,11 +392,11 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { } } - private async canAccessImportExport(organizationId?: string): Promise { + private async canAccessImport(organizationId?: string): Promise { if (!organizationId) { return false; } - return (await this.organizationService.get(this.organizationId))?.canAccessImportExport; + return (await this.organizationService.get(this.organizationId))?.canAccessImport; } getFormatInstructionTitle() {