From a0fe4f4ca69ce9e7f679302ebc118a679b0aaee0 Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Thu, 24 Oct 2024 14:12:04 -0700 Subject: [PATCH] [PM-13892] Browser Refresh - Organization item clone permission fix (#11660) * [PM-13892] Introduce canClone$ method on CipherAuthorizationService * [PM-13892] Use new canClone$ method for the 3dot menu in browser extension * [PM-13892] Add todo for vault-items.component.ts --- .../item-more-options.component.html | 2 +- .../item-more-options.component.ts | 6 +- .../vault-items/vault-items.component.ts | 1 + .../cipher-authorization.service.spec.ts | 77 ++++++++++++++++++- .../services/cipher-authorization.service.ts | 38 ++++++++- 5 files changed, 120 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html index cedd7736ba..8b70896e01 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -22,7 +22,7 @@ {{ favoriteText | i18n }} - + {{ "clone" | i18n }} diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index 40dbe5911d..f175c55c82 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -1,7 +1,7 @@ import { CommonModule } from "@angular/common"; import { booleanAttribute, Component, Input, OnInit } from "@angular/core"; import { Router, RouterModule } from "@angular/router"; -import { firstValueFrom, map } from "rxjs"; +import { firstValueFrom, map, Observable } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -10,6 +10,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; import { DialogService, IconButtonModule, @@ -42,6 +43,7 @@ export class ItemMoreOptionsComponent implements OnInit { hideAutofillOptions: boolean; protected autofillAllowed$ = this.vaultPopupAutofillService.autofillAllowed$; + protected canClone$: Observable; /** Boolean dependent on the current user having access to an organization */ protected hasOrganizations = false; @@ -56,10 +58,12 @@ export class ItemMoreOptionsComponent implements OnInit { private vaultPopupAutofillService: VaultPopupAutofillService, private accountService: AccountService, private organizationService: OrganizationService, + private cipherAuthorizationService: CipherAuthorizationService, ) {} async ngOnInit(): Promise { this.hasOrganizations = await this.organizationService.hasOrganizations(); + this.canClone$ = this.cipherAuthorizationService.canCloneCipher$(this.cipher); } get canEdit() { diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts index db1c2d6b56..71a97f1ff4 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts @@ -194,6 +194,7 @@ export class VaultItemsComponent { }); } + // TODO: PM-13944 Refactor to use cipherAuthorizationService.canClone$ instead protected canClone(vaultItem: VaultItem) { if (vaultItem.cipher.organizationId == null) { return true; diff --git a/libs/common/src/vault/services/cipher-authorization.service.spec.ts b/libs/common/src/vault/services/cipher-authorization.service.spec.ts index 3155825d4d..cccd29ad69 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.spec.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.spec.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { of } from "rxjs"; +import { firstValueFrom, of } from "rxjs"; import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -39,10 +39,16 @@ describe("CipherAuthorizationService", () => { allowAdminAccessToAllCollectionItems = false, canEditAllCiphers = false, canEditUnassignedCiphers = false, + isAdmin = false, + editAnyCollection = false, } = {}) => ({ allowAdminAccessToAllCollectionItems, canEditAllCiphers, canEditUnassignedCiphers, + isAdmin, + permissions: { + editAnyCollection, + }, }); beforeEach(() => { @@ -197,4 +203,73 @@ describe("CipherAuthorizationService", () => { }); }); }); + + describe("canCloneCipher$", () => { + it("should return true if cipher has no organizationId", async () => { + const cipher = createMockCipher(null, []) as CipherView; + + const result = await firstValueFrom(cipherAuthorizationService.canCloneCipher$(cipher)); + expect(result).toBe(true); + }); + + describe("isAdminConsoleAction is true", () => { + it("should return true for admin users", async () => { + const cipher = createMockCipher("org1", []) as CipherView; + const organization = createMockOrganization({ isAdmin: true }); + mockOrganizationService.get$.mockReturnValue(of(organization as Organization)); + + const result = await firstValueFrom( + cipherAuthorizationService.canCloneCipher$(cipher, true), + ); + expect(result).toBe(true); + }); + + it("should return true for custom user with canEditAnyCollection", async () => { + const cipher = createMockCipher("org1", []) as CipherView; + const organization = createMockOrganization({ editAnyCollection: true }); + mockOrganizationService.get$.mockReturnValue(of(organization as Organization)); + + const result = await firstValueFrom( + cipherAuthorizationService.canCloneCipher$(cipher, true), + ); + expect(result).toBe(true); + }); + }); + + describe("isAdminConsoleAction is false", () => { + it("should return true if at least one cipher collection has manage permission", async () => { + const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView; + const organization = createMockOrganization(); + mockOrganizationService.get$.mockReturnValue(of(organization as Organization)); + + const allCollections = [ + createMockCollection("col1", true), + createMockCollection("col2", false), + ]; + mockCollectionService.decryptedCollectionViews$.mockReturnValue( + of(allCollections as CollectionView[]), + ); + + const result = await firstValueFrom(cipherAuthorizationService.canCloneCipher$(cipher)); + expect(result).toBe(true); + }); + + it("should return false if no collection has manage permission", async () => { + const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView; + const organization = createMockOrganization(); + mockOrganizationService.get$.mockReturnValue(of(organization as Organization)); + + const allCollections = [ + createMockCollection("col1", false), + createMockCollection("col2", false), + ]; + mockCollectionService.decryptedCollectionViews$.mockReturnValue( + of(allCollections as CollectionView[]), + ); + + const result = await firstValueFrom(cipherAuthorizationService.canCloneCipher$(cipher)); + expect(result).toBe(false); + }); + }); + }); }); diff --git a/libs/common/src/vault/services/cipher-authorization.service.ts b/libs/common/src/vault/services/cipher-authorization.service.ts index 00c7c412d6..eb6211848a 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.ts @@ -1,4 +1,4 @@ -import { map, Observable, of, switchMap } from "rxjs"; +import { map, Observable, of, shareReplay, switchMap } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -30,6 +30,16 @@ export abstract class CipherAuthorizationService { allowedCollections?: CollectionId[], isAdminConsoleAction?: boolean, ) => Observable; + + /** + * Determines if the user can clone the specified cipher. + * + * @param {CipherLike} cipher - The cipher object to evaluate for cloning permissions. + * @param {boolean} isAdminConsoleAction - Optional. A flag indicating if the action is being performed from the admin console. + * + * @returns {Observable} - An observable that emits a boolean value indicating if the user can clone the cipher. + */ + canCloneCipher$: (cipher: CipherLike, isAdminConsoleAction?: boolean) => Observable; } /** @@ -83,4 +93,30 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer }), ); } + + /** + * {@link CipherAuthorizationService.canCloneCipher$} + */ + canCloneCipher$(cipher: CipherLike, isAdminConsoleAction?: boolean): Observable { + if (cipher.organizationId == null) { + return of(true); + } + + return this.organizationService.get$(cipher.organizationId).pipe( + switchMap((organization) => { + // Admins and custom users can always clone when in the Admin Console + if ( + isAdminConsoleAction && + (organization.isAdmin || organization.permissions?.editAnyCollection) + ) { + return of(true); + } + + return this.collectionService + .decryptedCollectionViews$(cipher.collectionIds as CollectionId[]) + .pipe(map((allCollections) => allCollections.some((collection) => collection.manage))); + }), + shareReplay({ bufferSize: 1, refCount: false }), + ); + } }