From 1763324a89459264bbcf37cad550dac7e3083993 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 20 Jun 2024 12:43:41 -0500 Subject: [PATCH] [SM-654] Individual secret permissions (#9527) * update secret service * Add new method to access policy selector service * Add individual secret permission management to secret dialog * add secret.service tests --- apps/web/src/locales/en/messages.json | 18 + .../dialog/secret-dialog.component.html | 131 +++-- .../secrets/dialog/secret-dialog.component.ts | 373 ++++++++---- .../secrets/requests/secret.request.ts | 3 + .../secrets/secret.service.spec.ts | 125 +++++ .../secrets-manager/secrets/secret.service.ts | 31 +- .../access-policy-selector.service.spec.ts | 531 ++++++++++++++---- .../access-policy-selector.service.ts | 60 +- .../access-policies/access-policy.service.ts | 15 + .../secret-access-policies.request.ts | 7 + 10 files changed, 1005 insertions(+), 289 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.spec.ts create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/models/requests/secret-access-policies.request.ts diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 82aee1e320..540eb22625 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -8429,5 +8429,23 @@ }, "providerReinstate":{ "message": " Contact Customer Support to reinstate your subscription." + }, + "secretPeopleDescription": { + "message": "Grant groups or people access to this secret. Permissions set for people will override permissions set by groups." + }, + "secretPeopleEmptyMessage": { + "message": "Add people or groups to share access to this secret" + }, + "secretMachineAccountsDescription": { + "message": "Grant machine accounts access to this secret." + }, + "secretMachineAccountsEmptyMessage": { + "message": "Add machine accounts to grant access to this secret" + }, + "smAccessRemovalWarningSecretTitle": { + "message": "Remove access to this secret" + }, + "smAccessRemovalSecretMessage": { + "message": "This action will remove your access to this secret." } } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.html index 62692511e2..24168d0b02 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.html @@ -1,71 +1,102 @@
- - {{ title | i18n }} +
-
- -
-
- - {{ "name" | i18n }} - - - - {{ "value" | i18n }} - - -
- - {{ "notes" | i18n }} - - + + +
+ + {{ "name" | i18n }} + + + + {{ "value" | i18n }} + + +
+ + {{ "notes" | i18n }} + + -
+
- - {{ "project" | i18n }} - - - + {{ "project" | i18n }} + + + + + + + + + {{ "projectName" | i18n }} + + + + + +

+ {{ "secretPeopleDescription" | i18n }} +

+ - - - + + - - {{ "projectName" | i18n }} - - + +

+ {{ "secretMachineAccountsDescription" | i18n }} +

+ + + +
+ - - diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts index 0287cdd425..a6cf19a001 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts @@ -1,5 +1,5 @@ import { DialogRef, DIALOG_DATA } from "@angular/cdk/dialog"; -import { Component, Inject, OnInit } from "@angular/core"; +import { ChangeDetectorRef, Component, Inject, OnInit } from "@angular/core"; import { FormControl, FormGroup, Validators } from "@angular/forms"; import { lastValueFrom, Subject, takeUntil } from "rxjs"; @@ -9,12 +9,25 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogService, BitValidators } from "@bitwarden/components"; +import { SecretAccessPoliciesView } from "../../models/view/access-policies/secret-access-policies.view"; import { ProjectListView } from "../../models/view/project-list.view"; import { ProjectView } from "../../models/view/project.view"; import { SecretListView } from "../../models/view/secret-list.view"; import { SecretProjectView } from "../../models/view/secret-project.view"; import { SecretView } from "../../models/view/secret.view"; import { ProjectService } from "../../projects/project.service"; +import { AccessPolicySelectorService } from "../../shared/access-policies/access-policy-selector/access-policy-selector.service"; +import { + ApItemValueType, + convertToSecretAccessPoliciesView, +} from "../../shared/access-policies/access-policy-selector/models/ap-item-value.type"; +import { + ApItemViewType, + convertPotentialGranteesToApItemViewType, + convertSecretAccessPoliciesToApItemViews, +} from "../../shared/access-policies/access-policy-selector/models/ap-item-view.type"; +import { ApItemEnum } from "../../shared/access-policies/access-policy-selector/models/enums/ap-item.enum"; +import { AccessPolicyService } from "../../shared/access-policies/access-policy.service"; import { SecretService } from "../secret.service"; import { SecretDeleteDialogComponent, SecretDeleteOperation } from "./secret-delete.component"; @@ -24,6 +37,12 @@ export enum OperationType { Edit, } +export enum SecretDialogTabType { + NameValuePair = 0, + People = 1, + ServiceAccounts = 2, +} + export interface SecretOperation { organizationId: string; operation: OperationType; @@ -36,6 +55,12 @@ export interface SecretOperation { templateUrl: "./secret-dialog.component.html", }) export class SecretDialogComponent implements OnInit { + loading = true; + projects: ProjectListView[]; + addNewProject = false; + newProjectGuid = Utils.newGuid(); + tabIndex: SecretDialogTabType = SecretDialogTabType.NameValuePair; + protected formGroup = new FormGroup({ name: new FormControl("", { validators: [Validators.required, Validators.maxLength(500), BitValidators.trimValidator], @@ -51,77 +76,60 @@ export class SecretDialogComponent implements OnInit { validators: [Validators.maxLength(500), BitValidators.trimValidator], updateOn: "submit", }), + peopleAccessPolicies: new FormControl([] as ApItemValueType[]), + serviceAccountAccessPolicies: new FormControl([] as ApItemValueType[]), }); + protected peopleAccessPolicyItems: ApItemViewType[]; + protected serviceAccountAccessPolicyItems: ApItemViewType[]; private destroy$ = new Subject(); - private loading = true; - projects: ProjectListView[]; - addNewProject = false; - newProjectGuid = Utils.newGuid(); + private currentPeopleAccessPolicies: ApItemViewType[]; constructor( public dialogRef: DialogRef, @Inject(DIALOG_DATA) private data: SecretOperation, private secretService: SecretService, + private changeDetectorRef: ChangeDetectorRef, private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, private projectService: ProjectService, private dialogService: DialogService, private organizationService: OrganizationService, + private accessPolicyService: AccessPolicyService, + private accessPolicySelectorService: AccessPolicySelectorService, ) {} + get title() { + return this.data.operation === OperationType.Add ? "newSecret" : "editSecret"; + } + + get subtitle(): string | undefined { + if (this.data.operation === OperationType.Edit) { + return this.formGroup.get("name").value; + } + } + + get deleteButtonIsVisible(): boolean { + return this.data.operation === OperationType.Edit; + } + async ngOnInit() { + this.loading = true; if (this.data.operation === OperationType.Edit && this.data.secretId) { - await this.loadData(); + await this.loadEditDialog(); } else if (this.data.operation !== OperationType.Add) { this.dialogRef.close(); throw new Error(`The secret dialog was not called with the appropriate operation values.`); - } else if (this.data.operation == OperationType.Add) { - await this.loadProjects(true); - if (this.data.projectId == null || this.data.projectId == "") { - this.addNewProjectOptionToProjectsDropDown(); - } - } - - if (this.data.projectId) { - this.formGroup.get("project").setValue(this.data.projectId); + } else if (this.data.operation === OperationType.Add) { + await this.loadAddDialog(); } if ((await this.organizationService.get(this.data.organizationId))?.isAdmin) { this.formGroup.get("project").removeValidators(Validators.required); this.formGroup.get("project").updateValueAndValidity(); } - } - - async loadData() { - this.formGroup.disable(); - const secret: SecretView = await this.secretService.getBySecretId(this.data.secretId); - - await this.loadProjects(secret.write); - - this.formGroup.setValue({ - name: secret.name, - value: secret.value, - notes: secret.note, - project: secret.projects[0]?.id ?? "", - newProjectName: "", - }); this.loading = false; - - if (secret.write) { - this.formGroup.enable(); - } - } - - async loadProjects(filterByPermission: boolean) { - this.projects = await this.projectService - .getProjects(this.data.organizationId) - .then((projects) => projects.sort((a, b) => a.name.localeCompare(b.name))); - - if (filterByPermission) { - this.projects = this.projects.filter((p) => p.write); - } } ngOnDestroy(): void { @@ -129,6 +137,152 @@ export class SecretDialogComponent implements OnInit { this.destroy$.complete(); } + submit = async () => { + if (!this.data.organizationEnabled) { + this.platformUtilsService.showToast("error", null, this.i18nService.t("secretsCannotCreate")); + return; + } + + if (this.isFormInvalid()) { + return; + } + + const secretView = this.getSecretView(); + const secretAccessPoliciesView = convertToSecretAccessPoliciesView([ + ...this.formGroup.value.peopleAccessPolicies, + ...this.formGroup.value.serviceAccountAccessPolicies, + ]); + + const showAccessRemovalWarning = + this.data.operation === OperationType.Edit && + (await this.accessPolicySelectorService.showSecretAccessRemovalWarning( + this.data.organizationId, + this.currentPeopleAccessPolicies, + this.formGroup.value.peopleAccessPolicies, + )); + + if (showAccessRemovalWarning) { + const confirmed = await this.showWarning(); + if (!confirmed) { + return; + } + } + + if (this.addNewProject) { + const newProject = await this.createProject(this.getNewProjectView()); + secretView.projects = [newProject]; + } + + if (this.data.operation === OperationType.Add) { + await this.createSecret(secretView, secretAccessPoliciesView); + } else { + secretView.id = this.data.secretId; + await this.updateSecret(secretView, secretAccessPoliciesView); + } + this.dialogRef.close(); + }; + + delete = async () => { + const secretListView: SecretListView[] = this.getSecretListView(); + + const dialogRef = this.dialogService.open( + SecretDeleteDialogComponent, + { + data: { + secrets: secretListView, + }, + }, + ); + + await lastValueFrom(dialogRef.closed).then( + (closeData) => closeData !== undefined && this.dialogRef.close(), + ); + }; + + private async loadEditDialog() { + const secret = await this.secretService.getBySecretId(this.data.secretId); + await this.loadProjects(secret.projects); + + const currentAccessPolicies = await this.getCurrentAccessPolicies( + this.data.organizationId, + this.data.secretId, + ); + this.currentPeopleAccessPolicies = currentAccessPolicies.filter( + (p) => p.type === ApItemEnum.User || p.type === ApItemEnum.Group, + ); + const currentServiceAccountPolicies = currentAccessPolicies.filter( + (p) => p.type === ApItemEnum.ServiceAccount, + ); + + this.peopleAccessPolicyItems = await this.getPeoplePotentialGrantees(this.data.organizationId); + this.serviceAccountAccessPolicyItems = await this.getServiceAccountItems( + this.data.organizationId, + currentServiceAccountPolicies, + ); + + // Must detect changes so that AccessSelector @Inputs() are aware of the latest + // potentialGrantees, otherwise no selected values will be patched below + this.changeDetectorRef.detectChanges(); + + this.formGroup.patchValue({ + name: secret.name, + value: secret.value, + notes: secret.note, + project: secret.projects[0]?.id ?? "", + newProjectName: "", + peopleAccessPolicies: this.currentPeopleAccessPolicies.map((m) => ({ + type: m.type, + id: m.id, + permission: m.permission, + currentUser: m.type === ApItemEnum.User ? m.currentUser : null, + currentUserInGroup: m.type === ApItemEnum.Group ? m.currentUserInGroup : null, + })), + serviceAccountAccessPolicies: currentServiceAccountPolicies.map((m) => ({ + type: m.type, + id: m.id, + permission: m.permission, + })), + }); + } + + private async loadAddDialog() { + await this.loadProjects(); + this.peopleAccessPolicyItems = await this.getPeoplePotentialGrantees(this.data.organizationId); + this.serviceAccountAccessPolicyItems = await this.getServiceAccountItems( + this.data.organizationId, + ); + + if ( + this.data.projectId === null || + this.data.projectId === "" || + this.data.projectId === undefined + ) { + this.addNewProjectOptionToProjectsDropDown(); + } + + if (this.data.projectId) { + this.formGroup.get("project").setValue(this.data.projectId); + } + } + + private async loadProjects(currentProjects?: SecretProjectView[]) { + this.projects = await this.projectService + .getProjects(this.data.organizationId) + .then((projects) => projects.filter((p) => p.write)); + + if (currentProjects?.length > 0) { + const currentProject = currentProjects?.[0]; + if (this.projects.find((p) => p.id === currentProject.id) === undefined) { + const listView = new ProjectListView(); + listView.id = currentProject.id; + listView.name = currentProject.name; + this.projects.push(listView); + } + } + + this.projects = this.projects.sort((a, b) => a.name.localeCompare(b.name)); + } + private addNewProjectOptionToProjectsDropDown() { this.formGroup .get("project") @@ -155,70 +309,15 @@ export class SecretDialogComponent implements OnInit { this.formGroup.get("newProjectName").updateValueAndValidity(); } - get title() { - return this.data.operation === OperationType.Add ? "newSecret" : "editSecret"; - } - - get showSpinner() { - return this.data.operation === OperationType.Edit && this.loading; - } - - submit = async () => { - if (!this.data.organizationEnabled) { - this.platformUtilsService.showToast("error", null, this.i18nService.t("secretsCannotCreate")); - return; - } - - this.formGroup.markAllAsTouched(); - - if (this.formGroup.invalid) { - return; - } - - const secretView = this.getSecretView(); - - if (this.addNewProject) { - const newProject = await this.createProject(this.getNewProjectView()); - secretView.projects = [newProject]; - } - - if (this.data.operation === OperationType.Add) { - await this.createSecret(secretView); - } else { - secretView.id = this.data.secretId; - await this.updateSecret(secretView); - } - this.dialogRef.close(); - }; - - get deleteButtonIsVisible(): boolean { - return this.data.operation === OperationType.Edit; - } - private async createProject(projectView: ProjectView) { return await this.projectService.create(this.data.organizationId, projectView); } - protected async openDeleteSecretDialog() { - const secretListView: SecretListView[] = this.getSecretListView(); - - const dialogRef = this.dialogService.open( - SecretDeleteDialogComponent, - { - data: { - secrets: secretListView, - }, - }, - ); - - // If the secret is deleted, chain close this dialog after the delete dialog - await lastValueFrom(dialogRef.closed).then( - (closeData) => closeData !== undefined && this.dialogRef.close(), - ); - } - - private async createSecret(secretView: SecretView) { - await this.secretService.create(this.data.organizationId, secretView); + private async createSecret( + secretView: SecretView, + secretAccessPoliciesView: SecretAccessPoliciesView, + ) { + await this.secretService.create(this.data.organizationId, secretView, secretAccessPoliciesView); this.platformUtilsService.showToast("success", null, this.i18nService.t("secretCreated")); } @@ -229,8 +328,11 @@ export class SecretDialogComponent implements OnInit { return projectView; } - private async updateSecret(secretView: SecretView) { - await this.secretService.update(this.data.organizationId, secretView); + private async updateSecret( + secretView: SecretView, + secretAccessPoliciesView: SecretAccessPoliciesView, + ) { + await this.secretService.update(this.data.organizationId, secretView, secretAccessPoliciesView); this.platformUtilsService.showToast("success", null, this.i18nService.t("secretEdited")); } @@ -265,4 +367,63 @@ export class SecretDialogComponent implements OnInit { secretListViews.push(secretListView); return secretListViews; } + + private async getCurrentAccessPolicies( + organizationId: string, + secretId: string, + ): Promise { + return convertSecretAccessPoliciesToApItemViews( + await this.accessPolicyService.getSecretAccessPolicies(organizationId, secretId), + ); + } + + private async getPeoplePotentialGrantees(organizationId: string): Promise { + return convertPotentialGranteesToApItemViewType( + await this.accessPolicyService.getPeoplePotentialGrantees(organizationId), + ); + } + + private async getServiceAccountItems( + organizationId: string, + currentAccessPolicies?: ApItemViewType[], + ): Promise { + const potentialGrantees = convertPotentialGranteesToApItemViewType( + await this.accessPolicyService.getServiceAccountsPotentialGrantees(organizationId), + ); + const items = [...potentialGrantees]; + if (currentAccessPolicies) { + for (const policy of currentAccessPolicies) { + const exists = potentialGrantees.some((grantee) => grantee.id === policy.id); + if (!exists) { + items.push(policy); + } + } + } + return items; + } + + private async showWarning(): Promise { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "smAccessRemovalWarningSecretTitle" }, + content: { key: "smAccessRemovalSecretMessage" }, + acceptButtonText: { key: "removeAccess" }, + cancelButtonText: { key: "cancel" }, + type: "warning", + }); + return confirmed; + } + + private isFormInvalid(): boolean { + this.formGroup.markAllAsTouched(); + + if (this.formGroup.invalid && this.tabIndex !== SecretDialogTabType.NameValuePair) { + this.platformUtilsService.showToast( + "error", + null, + this.i18nService.t("fieldOnTabRequiresAttention", this.i18nService.t("nameValuePair")), + ); + } + + return this.formGroup.invalid; + } } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/requests/secret.request.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/requests/secret.request.ts index 9fdd7567f1..9723bc9ed6 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/requests/secret.request.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/requests/secret.request.ts @@ -1,6 +1,9 @@ +import { SecretAccessPoliciesRequest } from "../../shared/access-policies/models/requests/secret-access-policies.request"; + export class SecretRequest { key: string; value: string; note: string; projectIds?: string[]; + accessPoliciesRequests: SecretAccessPoliciesRequest; } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.spec.ts new file mode 100644 index 0000000000..f2a77553fb --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.spec.ts @@ -0,0 +1,125 @@ +import { mock } from "jest-mock-extended"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; +import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; + +import { SecretAccessPoliciesView } from "../models/view/access-policies/secret-access-policies.view"; +import { SecretView } from "../models/view/secret.view"; +import { AccessPolicyService } from "../shared/access-policies/access-policy.service"; + +import { SecretService } from "./secret.service"; + +describe("SecretService", () => { + let sut: SecretService; + + const cryptoService = mock(); + const apiService = mock(); + const encryptService = mock(); + const accessPolicyService = mock(); + + beforeEach(() => { + jest.resetAllMocks(); + + sut = new SecretService(cryptoService, apiService, encryptService, accessPolicyService); + + encryptService.encrypt.mockResolvedValue({ + encryptedString: "mockEncryptedString", + } as EncString); + encryptService.decryptToUtf8.mockResolvedValue(mockUnencryptedData); + }); + + it("instantiates", () => { + expect(sut).not.toBeFalsy(); + }); + + describe("create", () => { + it("emits the secret created", async () => { + apiService.send.mockResolvedValue(mockedSecretResponse); + + sut.secret$.subscribe((secret) => { + expect(secret).toBeDefined(); + expect(secret).toEqual(expectedSecretView); + }); + + await sut.create("organizationId", secretView, secretAccessPoliciesView); + }); + }); + + describe("update", () => { + it("emits the secret updated", async () => { + apiService.send.mockResolvedValue(mockedSecretResponse); + + sut.secret$.subscribe((secret) => { + expect(secret).toBeDefined(); + expect(secret).toEqual(expectedSecretView); + }); + + await sut.update("organizationId", secretView, secretAccessPoliciesView); + }); + }); +}); + +const mockedSecretResponse: any = { + id: "001f835c-aa41-4f25-bfbf-b18d0103a1db", + organizationId: "da0eea55-8604-4307-8a24-b187015e3071", + key: "mockEncryptedString", + value: "mockEncryptedString", + note: "mockEncryptedString", + creationDate: "2024-07-12T15:45:17.49823Z", + revisionDate: "2024-07-12T15:45:17.49823Z", + projects: [ + { + id: "502d93ae-a084-490a-8a64-b187015eb69c", + name: "mockEncryptedString", + }, + ], + read: true, + write: true, + object: "secret", +}; + +const secretView: SecretView = { + id: "001f835c-aa41-4f25-bfbf-b18d0103a1db", + organizationId: "da0eea55-8604-4307-8a24-b187015e3071", + name: "key", + value: "value", + note: "note", + creationDate: "2024-06-12T15:45:17.49823Z", + revisionDate: "2024-06-12T15:45:17.49823Z", + projects: [ + { + id: "502d93ae-a084-490a-8a64-b187015eb69c", + name: "project-name", + }, + ], + read: true, + write: true, +}; + +const secretAccessPoliciesView: SecretAccessPoliciesView = { + userAccessPolicies: [], + groupAccessPolicies: [], + serviceAccountAccessPolicies: [], +}; + +const mockUnencryptedData = "mockUnEncryptedString"; + +const expectedSecretView: SecretView = { + id: "001f835c-aa41-4f25-bfbf-b18d0103a1db", + organizationId: "da0eea55-8604-4307-8a24-b187015e3071", + name: mockUnencryptedData, + value: mockUnencryptedData, + note: mockUnencryptedData, + creationDate: "2024-07-12T15:45:17.49823Z", + revisionDate: "2024-07-12T15:45:17.49823Z", + projects: [ + { + id: "502d93ae-a084-490a-8a64-b187015eb69c", + name: mockUnencryptedData, + }, + ], + read: true, + write: true, +}; diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.ts index f9e9011e2f..0848ab9e7a 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secret.service.ts @@ -7,9 +7,11 @@ import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt. import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { SecretAccessPoliciesView } from "../models/view/access-policies/secret-access-policies.view"; import { SecretListView } from "../models/view/secret-list.view"; import { SecretProjectView } from "../models/view/secret-project.view"; import { SecretView } from "../models/view/secret.view"; +import { AccessPolicyService } from "../shared/access-policies/access-policy.service"; import { BulkOperationStatus } from "../shared/dialogs/bulk-status-dialog.component"; import { SecretRequest } from "./requests/secret.request"; @@ -30,6 +32,7 @@ export class SecretService { private cryptoService: CryptoService, private apiService: ApiService, private encryptService: EncryptService, + private accessPolicyService: AccessPolicyService, ) {} async getBySecretId(secretId: string): Promise { @@ -65,8 +68,16 @@ export class SecretService { return await this.createSecretsListView(organizationId, results); } - async create(organizationId: string, secretView: SecretView) { - const request = await this.getSecretRequest(organizationId, secretView); + async create( + organizationId: string, + secretView: SecretView, + secretAccessPoliciesView: SecretAccessPoliciesView, + ) { + const request = await this.getSecretRequest( + organizationId, + secretView, + secretAccessPoliciesView, + ); const r = await this.apiService.send( "POST", "/organizations/" + organizationId + "/secrets", @@ -77,8 +88,16 @@ export class SecretService { this._secret.next(await this.createSecretView(new SecretResponse(r))); } - async update(organizationId: string, secretView: SecretView) { - const request = await this.getSecretRequest(organizationId, secretView); + async update( + organizationId: string, + secretView: SecretView, + secretAccessPoliciesView: SecretAccessPoliciesView, + ) { + const request = await this.getSecretRequest( + organizationId, + secretView, + secretAccessPoliciesView, + ); const r = await this.apiService.send("PUT", "/secrets/" + secretView.id, request, true, true); this._secret.next(await this.createSecretView(new SecretResponse(r))); } @@ -140,6 +159,7 @@ export class SecretService { private async getSecretRequest( organizationId: string, secretView: SecretView, + secretAccessPoliciesView: SecretAccessPoliciesView, ): Promise { const orgKey = await this.getOrganizationKey(organizationId); const request = new SecretRequest(); @@ -155,6 +175,9 @@ export class SecretService { secretView.projects?.forEach((e) => request.projectIds.push(e.id)); + request.accessPoliciesRequests = + this.accessPolicyService.getSecretAccessPoliciesRequest(secretAccessPoliciesView); + return request; } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts index 324406e766..05b677d490 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.spec.ts @@ -64,10 +64,12 @@ describe("AccessPolicySelectorService", () => { const selectedPolicyValues: ApItemValueType[] = []; selectedPolicyValues.push( - createApItemValueType({ - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemValueType( + { + permission: ApPermissionEnum.CanRead, + }, + true, + ), ); const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -80,15 +82,17 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - permission: ApPermissionEnum.CanReadWrite, - currentUser: true, - }), + createApItemValueType( + { + permission: ApPermissionEnum.CanReadWrite, + }, + true, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); - expect(result).toBe(true); + expect(result).toBe(false); }); it("returns true when current user isn't owner/admin and a group Read policy is submitted that the user is a member of", async () => { @@ -96,12 +100,15 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanRead, - currentUserInGroup: true, - }), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanRead, + }, + false, + true, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -114,12 +121,15 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanReadWrite, - currentUserInGroup: true, - }), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + true, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -132,12 +142,15 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanReadWrite, - currentUserInGroup: false, - }), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + false, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -150,16 +163,21 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanReadWrite, - currentUserInGroup: true, - }), + createApItemValueType( + { + permission: ApPermissionEnum.CanRead, + }, + true, + ), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + true, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -172,16 +190,21 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanReadWrite, - currentUserInGroup: false, - }), + createApItemValueType( + { + permission: ApPermissionEnum.CanRead, + }, + true, + ), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + false, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -194,16 +217,21 @@ describe("AccessPolicySelectorService", () => { organizationService.get.calledWith(org.id).mockResolvedValue(org); const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), - createApItemValueType({ - id: "groupId", - type: ApItemEnum.Group, - permission: ApPermissionEnum.CanRead, - currentUserInGroup: true, - }), + createApItemValueType( + { + permission: ApPermissionEnum.CanRead, + }, + true, + ), + createApItemValueType( + { + id: "groupId", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanRead, + }, + false, + true, + ), ]; const result = await sut.showAccessRemovalWarning(org.id, selectedPolicyValues); @@ -211,6 +239,246 @@ describe("AccessPolicySelectorService", () => { expect(result).toBe(true); }); }); + describe("showSecretAccessRemovalWarning", () => { + it("returns false when there are no current access policies", async () => { + const org = orgFactory(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = []; + const selectedPolicyValues: ApItemValueType[] = []; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns false when current user is admin", async () => { + const org = orgFactory(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = []; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns false when current user is owner", async () => { + const org = orgFactory(); + org.type = OrganizationUserType.Owner; + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = []; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns false when current non-admin user doesn't have Read, Write access with current access policies -- user policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = []; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns false when current non-admin user doesn't have Read, Write access with current access policies -- group policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanRead, + }, + false, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = []; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns true when current non-admin user has Read, Write access with current access policies and doesn't with selected -- user policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanReadWrite, + }, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = [ + createApItemValueType({ + type: ApItemEnum.User, + permission: ApPermissionEnum.CanRead, + currentUser: true, + }), + ]; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(true); + }); + it("returns true when current non-admin user has Read, Write access with current access policies and doesn't with selected -- group policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = [ + createApItemValueType( + { + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanRead, + }, + false, + true, + ), + ]; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(true); + }); + it("returns false when current non-admin user has Read, Write access with current access policies and does with selected -- user policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanReadWrite, + }, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = [ + createApItemValueType( + { + type: ApItemEnum.User, + permission: ApPermissionEnum.CanReadWrite, + }, + true, + ), + ]; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + it("returns false when current non-admin user has Read, Write access with current access policies and does with selected -- group policy", async () => { + const org = setupUserOrg(); + organizationService.get.calledWith(org.id).mockResolvedValue(org); + + const currentAccessPolicies: ApItemViewType[] = [ + createApItemViewType( + { + id: "example", + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + true, + ), + ]; + const selectedPolicyValues: ApItemValueType[] = [ + createApItemValueType( + { + type: ApItemEnum.Group, + permission: ApPermissionEnum.CanReadWrite, + }, + false, + true, + ), + ]; + + const result = await sut.showSecretAccessRemovalWarning( + org.id, + currentAccessPolicies, + selectedPolicyValues, + ); + + expect(result).toBe(false); + }); + }); describe("isAccessRemoval", () => { it("returns false when there are no previous policies and no selected policies", async () => { const currentAccessPolicies: ApItemViewType[] = []; @@ -223,11 +491,13 @@ describe("AccessPolicySelectorService", () => { it("returns false when there are no previous policies", async () => { const currentAccessPolicies: ApItemViewType[] = []; const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemValueType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const result = sut.isAccessRemoval(currentAccessPolicies, selectedPolicyValues); @@ -236,18 +506,22 @@ describe("AccessPolicySelectorService", () => { }); it("returns false when previous policies and selected policies are the same", async () => { const currentAccessPolicies: ApItemViewType[] = [ - createApItemViewType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemValueType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const result = sut.isAccessRemoval(currentAccessPolicies, selectedPolicyValues); @@ -256,23 +530,29 @@ describe("AccessPolicySelectorService", () => { }); it("returns false when previous policies are still selected", async () => { const currentAccessPolicies: ApItemViewType[] = [ - createApItemViewType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), - createApItemValueType({ - id: "example-2", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemValueType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), + createApItemValueType( + { + id: "example-2", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const result = sut.isAccessRemoval(currentAccessPolicies, selectedPolicyValues); @@ -281,23 +561,29 @@ describe("AccessPolicySelectorService", () => { }); it("returns true when previous policies are not selected", async () => { const currentAccessPolicies: ApItemViewType[] = [ - createApItemViewType({ - id: "example", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemViewType( + { + id: "example", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const selectedPolicyValues: ApItemValueType[] = [ - createApItemValueType({ - id: "test", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), - createApItemValueType({ - id: "example-2", - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemValueType( + { + id: "test", + permission: ApPermissionEnum.CanRead, + }, + true, + ), + createApItemValueType( + { + id: "example-2", + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const result = sut.isAccessRemoval(currentAccessPolicies, selectedPolicyValues); @@ -306,10 +592,12 @@ describe("AccessPolicySelectorService", () => { }); it("returns true when there are previous policies and nothing was selected", async () => { const currentAccessPolicies: ApItemViewType[] = [ - createApItemViewType({ - permission: ApPermissionEnum.CanRead, - currentUser: true, - }), + createApItemViewType( + { + permission: ApPermissionEnum.CanRead, + }, + true, + ), ]; const selectedPolicyValues: ApItemValueType[] = []; @@ -331,17 +619,31 @@ const orgFactory = (props: Partial = {}) => props, ); -function createApItemValueType(options: Partial = {}) { - return { +function createApItemValueType( + options: Partial = {}, + currentUser = false, + currentUserInGroup = false, +) { + const item: ApItemValueType = { id: options?.id ?? "test", type: options?.type ?? ApItemEnum.User, permission: options?.permission ?? ApPermissionEnum.CanRead, - currentUserInGroup: options?.currentUserInGroup ?? false, }; + if (item.type === ApItemEnum.User) { + item.currentUser = currentUser; + } + if (item.type === ApItemEnum.Group) { + item.currentUserInGroup = currentUserInGroup; + } + return item; } -function createApItemViewType(options: Partial = {}) { - return { +function createApItemViewType( + options: Partial = {}, + currentUser = false, + currentUserInGroup = false, +) { + const item: ApItemViewType = { id: options?.id ?? "test", listName: options?.listName ?? "test", labelName: options?.labelName ?? "test", @@ -349,6 +651,13 @@ function createApItemViewType(options: Partial = {}) { permission: options?.permission ?? ApPermissionEnum.CanRead, readOnly: options?.readOnly ?? false, }; + if (item.type === ApItemEnum.User) { + item.currentUser = currentUser; + } + if (item.type === ApItemEnum.Group) { + item.currentUserInGroup = currentUserInGroup; + } + return item; } function setupUserOrg() { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts index b219bfd33d..a4d1c3cd03 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.service.ts @@ -22,26 +22,29 @@ export class AccessPolicySelectorService { return false; } - const selectedUserReadWritePolicy = selectedPoliciesValues.find( - (s) => - s.type === ApItemEnum.User && - s.currentUser && - s.permission === ApPermissionEnum.CanReadWrite, - ); + if (!this.userHasReadWriteAccess(selectedPoliciesValues)) { + return true; + } - const selectedGroupReadWritePolicies = selectedPoliciesValues.filter( - (s) => - s.type === ApItemEnum.Group && - s.permission == ApPermissionEnum.CanReadWrite && - s.currentUserInGroup, - ); + return false; + } - if (selectedGroupReadWritePolicies == null || selectedGroupReadWritePolicies.length == 0) { - if (selectedUserReadWritePolicy == null) { - return true; - } else { - return false; - } + async showSecretAccessRemovalWarning( + organizationId: string, + current: ApItemViewType[], + selectedPoliciesValues: ApItemValueType[], + ): Promise { + if (current.length === 0) { + return false; + } + + const organization = await this.organizationService.get(organizationId); + if (organization.isOwner || organization.isAdmin || !this.userHasReadWriteAccess(current)) { + return false; + } + + if (!this.userHasReadWriteAccess(selectedPoliciesValues)) { + return true; } return false; @@ -67,4 +70,25 @@ export class AccessPolicySelectorService { const selectedIds = selected.map((x) => x.id); return !currentIds.every((id) => selectedIds.includes(id)); } + + private userHasReadWriteAccess(policies: ApItemValueType[] | ApItemViewType[]): boolean { + const userReadWritePolicy = (policies as Array).find( + (s) => + s.type === ApItemEnum.User && + s.currentUser && + s.permission === ApPermissionEnum.CanReadWrite, + ); + + const groupReadWritePolicies = (policies as Array).filter( + (s) => + s.type === ApItemEnum.Group && + s.permission === ApPermissionEnum.CanReadWrite && + s.currentUserInGroup, + ); + + if (groupReadWritePolicies.length > 0 || userReadWritePolicy !== undefined) { + return true; + } + return false; + } } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy.service.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy.service.ts index 67fb9b19bc..db0c932e83 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy.service.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy.service.ts @@ -27,6 +27,7 @@ import { ServiceAccountGrantedPoliciesRequest } from "../access-policies/models/ import { AccessPolicyRequest } from "./models/requests/access-policy.request"; import { ProjectServiceAccountsAccessPoliciesRequest } from "./models/requests/project-service-accounts-access-policies.request"; +import { SecretAccessPoliciesRequest } from "./models/requests/secret-access-policies.request"; import { GroupAccessPolicyResponse, UserAccessPolicyResponse, @@ -233,6 +234,20 @@ export class AccessPolicyService { return await this.createPotentialGranteeViews(organizationId, results.data); } + getSecretAccessPoliciesRequest(view: SecretAccessPoliciesView): SecretAccessPoliciesRequest { + return { + userAccessPolicyRequests: view.userAccessPolicies.map((ap) => { + return this.getAccessPolicyRequest(ap.organizationUserId, ap); + }), + groupAccessPolicyRequests: view.groupAccessPolicies.map((ap) => { + return this.getAccessPolicyRequest(ap.groupId, ap); + }), + serviceAccountAccessPolicyRequests: view.serviceAccountAccessPolicies.map((ap) => { + return this.getAccessPolicyRequest(ap.serviceAccountId, ap); + }), + }; + } + private async getOrganizationKey(organizationId: string): Promise { return await this.cryptoService.getOrgKey(organizationId); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/models/requests/secret-access-policies.request.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/models/requests/secret-access-policies.request.ts new file mode 100644 index 0000000000..ac49dd4974 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/models/requests/secret-access-policies.request.ts @@ -0,0 +1,7 @@ +import { AccessPolicyRequest } from "./access-policy.request"; + +export class SecretAccessPoliciesRequest { + userAccessPolicyRequests: AccessPolicyRequest[]; + groupAccessPolicyRequests: AccessPolicyRequest[]; + serviceAccountAccessPolicyRequests: AccessPolicyRequest[]; +}