[PM-12281] [PM-12301] [PM-12306] [PM-12334] Move delete item permissi…

…on to Can Manage (#11289) * Added inputs to the view and edit component to disable or remove the delete button when a user does not have manage rights * Refactored editByCipherId to receive cipherview object * Fixed issue where adding an item on the individual vault throws a null reference * Fixed issue where adding an item on the AC vault throws a null reference * Allow delete in unassigned collection * created reusable service to check if a user has delete permission on an item * Registered service * Used authorizationservice on the browser and desktop Only display the delete button when a user has delete permission * Added comments to the service * Passed active collectionId to add edit component renamed constructor parameter * restored input property used by the web * Fixed dependency issue * Fixed dependency issue * Fixed dependency issue * Modified service to cater for org vault * Updated to include new dependency * Updated components to use the observable * Added check on the cli to know if user has rights to delete an item * Renamed abstraction and renamed implementation to include Default Fixed permission issues * Fixed test to reflect changes in implementation * Modified base classes to use new naming Passed new parameters for the canDeleteCipher * Modified base classes to use new naming Made changes from base class * Desktop changes Updated reference naming * cli changes Updated reference naming Passed new parameters for the canDeleteCipher$ * Updated references * browser changes Updated reference naming Passed new parameters for the canDeleteCipher$ * Modified cipher form dialog to take in active collection id used canDeleteCipher$ on the vault item dialog to disable the delete button when user does not have the required permissions * Fix number of arguments issue * Added active collection id * Updated canDeleteCipher$ arguments * Updated to pass the cipher object * Fixed up refrences and comments * Updated dependency * updated check to canEditUnassignedCiphers * Fixed unit tests * Removed activeCollectionId from cipher form * Fixed issue where bulk delete option shows for can edit users * Fix null reference when checking if a cipher belongs to the unassigned collection * Fixed bug where allowedCollection passed is undefined * Modified cipher by adding a isAdminConsoleAction argument to tell when a reuqest comes from the admin console * Passed isAdminConsoleAction as true when request is from the admin console
bitwarden · Oct 22, 2024 · 4a30782 · 4a30782
1 parent 470ddf7
commit 4a30782
Show file tree

Hide file tree

Showing 39 changed files with 551 additions and 58 deletions.
diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts
@@ -177,6 +177,10 @@ import { InternalFolderService as InternalFolderServiceAbstraction } from "@bitw
 import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service";
 import { VaultSettingsService as VaultSettingsServiceAbstraction } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import {
+  CipherAuthorizationService,
+  DefaultCipherAuthorizationService,
+} from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { CipherService } from "@bitwarden/common/vault/services/cipher.service";
 import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service";
 import { FolderApiService } from "@bitwarden/common/vault/services/folder/folder-api.service";
@@ -369,6 +373,7 @@ export default class MainBackground {
   themeStateService: DefaultThemeStateService;
   autoSubmitLoginBackground: AutoSubmitLoginBackground;
   sdkService: SdkService;
+  cipherAuthorizationService: CipherAuthorizationService;
 
   onUpdatedRan: boolean;
   onReplacedRan: boolean;
@@ -1265,6 +1270,11 @@ export default class MainBackground {
     }
 
     this.userAutoUnlockKeyService = new UserAutoUnlockKeyService(this.cryptoService);
+
+    this.cipherAuthorizationService = new DefaultCipherAuthorizationService(
+      this.collectionService,
+      this.organizationService,
+    );
   }
 
   async bootstrap() {

diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html
@@ -28,7 +28,7 @@
 
     <button
       slot="end"
-      *ngIf="cipher.edit"
+      *ngIf="canDeleteCipher$ | async"
       [bitAction]="delete"
       type="button"
       buttonType="danger"

diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts
@@ -15,6 +15,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp
 import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherType } from "@bitwarden/common/vault/enums";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 
 import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
 
@@ -81,6 +82,12 @@ describe("ViewV2Component", () => {
           provide: AccountService,
           useValue: accountService,
         },
+        {
+          provide: CipherAuthorizationService,
+          useValue: {
+            canDeleteCipher$: jest.fn().mockReturnValue(true),
+          },
+        },
       ],
     }).compileComponents();
 

diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts
@@ -19,6 +19,7 @@ import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import {
   AsyncActionsModule,
   ButtonModule,
@@ -68,6 +69,7 @@ export class ViewV2Component {
   cipher: CipherView;
   organization$: Observable<Organization>;
   folder$: Observable<FolderView>;
+  canDeleteCipher$: Observable<boolean>;
   collections$: Observable<CollectionView[]>;
   loadAction: typeof AUTOFILL_ID | typeof SHOW_AUTOFILL_BUTTON;
 
@@ -83,6 +85,7 @@ export class ViewV2Component {
     private accountService: AccountService,
     private eventCollectionService: EventCollectionService,
     private popupRouterCacheService: PopupRouterCacheService,
+    protected cipherAuthorizationService: CipherAuthorizationService,
   ) {
     this.subscribeToParams();
   }
@@ -101,6 +104,8 @@ export class ViewV2Component {
             await this.vaultPopupAutofillService.doAutofill(this.cipher);
           }
 
+          this.canDeleteCipher$ = this.cipherAuthorizationService.canDeleteCipher$(cipher);
+
           await this.eventCollectionService.collect(
             EventType.Cipher_ClientViewed,
             cipher.id,

diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.html b/apps/browser/src/vault/popup/components/vault/add-edit.component.html
@@ -779,7 +779,7 @@ <h2 class="box-header">
         </div>
       </div>
     </div>
-    <div class="box list" *ngIf="editMode && !cloneMode && !(!cipher.edit && editMode)">
+    <div class="box list" *ngIf="editMode && !cloneMode && (canDeleteCipher$ | async)">
       <div class="box-content single-line">
         <button
           type="button"

diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts
@@ -24,6 +24,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService } from "@bitwarden/components";
 import { PasswordRepromptService } from "@bitwarden/vault";
 
@@ -72,6 +73,7 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit {
     datePipe: DatePipe,
     configService: ConfigService,
     private fido2UserVerificationService: Fido2UserVerificationService,
+    cipherAuthorizationService: CipherAuthorizationService,
   ) {
     super(
       cipherService,
@@ -92,6 +94,7 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit {
       window,
       datePipe,
       configService,
+      cipherAuthorizationService,
     );
   }
 
@@ -107,6 +110,7 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit {
         this.folderId = params.folderId;
       }
       if (params.collectionId) {
+        this.collectionId = params.collectionId;
         const collection = this.writeableCollections.find((c) => c.id === params.collectionId);
         if (collection != null) {
           this.collectionIds = [collection.id];

diff --git a/apps/browser/src/vault/popup/components/vault/vault-items.component.ts b/apps/browser/src/vault/popup/components/vault/vault-items.component.ts
@@ -198,7 +198,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnIn
         // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
         // eslint-disable-next-line @typescript-eslint/no-floating-promises
         this.router.navigate(["/view-cipher"], {
-          queryParams: { cipherId: cipher.id },
+          queryParams: { cipherId: cipher.id, collectionId: this.collectionId },
         });
       }
       this.preventSelected = false;

diff --git a/apps/browser/src/vault/popup/components/vault/view.component.html b/apps/browser/src/vault/popup/components/vault/view.component.html
@@ -644,7 +644,7 @@ <h2 class="box-header">
         class="box-content-row"
         appStopClick
         (click)="delete()"
-        *ngIf="cipher.edit"
+        *ngIf="canDeleteCipher$ | async"
       >
         <div class="row-main text-danger">
           <div class="icon text-danger" aria-hidden="true">

diff --git a/apps/browser/src/vault/popup/components/vault/view.component.ts b/apps/browser/src/vault/popup/components/vault/view.component.ts
@@ -26,6 +26,7 @@ import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/a
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
 import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService } from "@bitwarden/components";
 import { PasswordRepromptService } from "@bitwarden/vault";
 
@@ -102,6 +103,7 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
     datePipe: DatePipe,
     accountService: AccountService,
     billingAccountProfileStateService: BillingAccountProfileStateService,
+    cipherAuthorizationService: CipherAuthorizationService,
   ) {
     super(
       cipherService,
@@ -127,6 +129,7 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
       datePipe,
       accountService,
       billingAccountProfileStateService,
+      cipherAuthorizationService,
     );
   }
 
@@ -143,7 +146,13 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
     this.route.queryParams.pipe(first()).subscribe(async (params) => {
       if (params.cipherId) {
         this.cipherId = params.cipherId;
-      } else {
+      }
+
+      if (params.collectionId) {
+        this.collectionId = params.collectionId;
+      }
+
+      if (!params.cipherId) {
         // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
         // eslint-disable-next-line @typescript-eslint/no-floating-promises
         this.close();
@@ -197,7 +206,12 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
     // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     this.router.navigate(["/edit-cipher"], {
-      queryParams: { cipherId: this.cipher.id, type: this.cipher.type, isNew: false },
+      queryParams: {
+        cipherId: this.cipher.id,
+        type: this.cipher.type,
+        isNew: false,
+        collectionId: this.collectionId,
+      },
     });
     return true;
   }

diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts
@@ -113,6 +113,7 @@ export class OssServeConfigurator {
       this.serviceContainer.apiService,
       this.serviceContainer.folderApiService,
       this.serviceContainer.billingAccountProfileStateService,
+      this.serviceContainer.cipherAuthorizationService,
     );
     this.confirmCommand = new ConfirmCommand(
       this.serviceContainer.apiService,

diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts
@@ -129,6 +129,10 @@ import { SendStateProvider } from "@bitwarden/common/tools/send/services/send-st
 import { SendService } from "@bitwarden/common/tools/send/services/send.service";
 import { VaultTimeoutStringType } from "@bitwarden/common/types/vault-timeout.type";
 import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
+import {
+  CipherAuthorizationService,
+  DefaultCipherAuthorizationService,
+} from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { CipherService } from "@bitwarden/common/vault/services/cipher.service";
 import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service";
 import { FolderApiService } from "@bitwarden/common/vault/services/folder/folder-api.service";
@@ -255,6 +259,7 @@ export class ServiceContainer {
   kdfConfigService: KdfConfigServiceAbstraction;
   taskSchedulerService: TaskSchedulerService;
   sdkService: SdkService;
+  cipherAuthorizationService: CipherAuthorizationService;
 
   constructor() {
     let p = null;
@@ -805,6 +810,11 @@ export class ServiceContainer {
       this.apiService,
       this.configService,
     );
+
+    this.cipherAuthorizationService = new DefaultCipherAuthorizationService(
+      this.collectionService,
+      this.organizationService,
+    );
   }
 
   async logout() {

diff --git a/apps/cli/src/vault.program.ts b/apps/cli/src/vault.program.ts
@@ -319,6 +319,7 @@ export class VaultProgram extends BaseProgram {
           this.serviceContainer.apiService,
           this.serviceContainer.folderApiService,
           this.serviceContainer.billingAccountProfileStateService,
+          this.serviceContainer.cipherAuthorizationService,
         );
         const response = await command.run(object, id, cmd);
         this.processResponse(response);

diff --git a/apps/cli/src/vault/delete.command.ts b/apps/cli/src/vault/delete.command.ts
@@ -6,6 +6,7 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 
 import { Response } from "../models/response";
 import { CliUtils } from "../utils";
@@ -17,6 +18,7 @@ export class DeleteCommand {
     private apiService: ApiService,
     private folderApiService: FolderApiServiceAbstraction,
     private accountProfileService: BillingAccountProfileStateService,
+    private cipherAuthorizationService: CipherAuthorizationService,
   ) {}
 
   async run(object: string, id: string, cmdOptions: Record<string, any>): Promise<Response> {
@@ -45,6 +47,14 @@ export class DeleteCommand {
       return Response.notFound();
     }
 
+    const canDeleteCipher = await firstValueFrom(
+      this.cipherAuthorizationService.canDeleteCipher$(cipher),
+    );
+
+    if (!canDeleteCipher) {
+      return Response.error("You do not have permission to delete this item.");
+    }
+
     try {
       if (options.permanent) {
         await this.cipherService.deleteWithServer(id);

diff --git a/apps/desktop/src/vault/app/vault/add-edit.component.html b/apps/desktop/src/vault/app/vault/add-edit.component.html
@@ -710,7 +710,7 @@ <h2 class="box-header">
         (click)="delete()"
         class="danger"
         appA11yTitle="{{ 'delete' | i18n }}"
-        *ngIf="editMode && !cloneMode"
+        *ngIf="editMode && !cloneMode && (canDeleteCipher$ | async)"
         [disabled]="$any(deleteBtn).loading"
         [appApiAction]="deletePromise"
       >

diff --git a/apps/desktop/src/vault/app/vault/add-edit.component.ts b/apps/desktop/src/vault/app/vault/add-edit.component.ts
@@ -18,6 +18,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
 import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService } from "@bitwarden/components";
 import { PasswordRepromptService } from "@bitwarden/vault";
 
@@ -50,6 +51,7 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit, On
     dialogService: DialogService,
     datePipe: DatePipe,
     configService: ConfigService,
+    cipherAuthorizationService: CipherAuthorizationService,
   ) {
     super(
       cipherService,
@@ -70,6 +72,7 @@ export class AddEditComponent extends BaseAddEditComponent implements OnInit, On
       window,
       datePipe,
       configService,
+      cipherAuthorizationService,
     );
   }
 

diff --git a/apps/desktop/src/vault/app/vault/vault.component.html b/apps/desktop/src/vault/app/vault/vault.component.html
@@ -14,6 +14,7 @@
     class="details"
     *ngIf="cipherId && action === 'view'"
     [cipherId]="cipherId"
+    [collectionId]="activeFilter?.selectedCollectionId"
     (onCloneCipher)="cloneCipherWithoutPasswordPrompt($event)"
     (onEditCipher)="editCipherWithoutPasswordPrompt($event)"
     (onViewCipherPasswordHistory)="viewCipherPasswordHistory($event)"
@@ -29,6 +30,7 @@
     [folderId]="action === 'add' && folderId !== 'none' ? folderId : null"
     [organizationId]="action === 'add' ? addOrganizationId : null"
     [collectionIds]="action === 'add' ? addCollectionIds : null"
+    [collectionId]="activeFilter?.selectedCollectionId"
     [type]="action === 'add' ? (addType ? addType : type) : null"
     [cipherId]="action === 'edit' || action === 'clone' ? cipherId : null"
     (onSavedCipher)="savedCipher($event)"

diff --git a/apps/desktop/src/vault/app/vault/view.component.html b/apps/desktop/src/vault/app/vault/view.component.html
@@ -566,7 +566,7 @@ <h2 class="box-header">
   >
     <i class="bwi bwi-files bwi-fw bwi-lg" aria-hidden="true"></i>
   </button>
-  <div class="right" *ngIf="cipher.edit">
+  <div class="right" *ngIf="canDeleteCipher$ | async">
     <button
       type="button"
       (click)="delete()"

diff --git a/apps/desktop/src/vault/app/vault/view.component.ts b/apps/desktop/src/vault/app/vault/view.component.ts
@@ -30,6 +30,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService } from "@bitwarden/components";
 import { PasswordRepromptService } from "@bitwarden/vault";
 
@@ -66,6 +67,7 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
     datePipe: DatePipe,
     billingAccountProfileStateService: BillingAccountProfileStateService,
     accountService: AccountService,
+    cipherAuthorizationService: CipherAuthorizationService,
   ) {
     super(
       cipherService,
@@ -91,6 +93,7 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
       datePipe,
       accountService,
       billingAccountProfileStateService,
+      cipherAuthorizationService,
     );
   }
   ngOnInit() {

diff --git a/apps/web/src/app/auth/settings/emergency-access/view/emergency-add-edit-cipher.component.ts b/apps/web/src/app/auth/settings/emergency-access/view/emergency-add-edit-cipher.component.ts
@@ -18,6 +18,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
 import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
+import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService } from "@bitwarden/components";
 import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";
 import { PasswordRepromptService } from "@bitwarden/vault";
@@ -54,6 +55,7 @@ export class EmergencyAddEditCipherComponent extends BaseAddEditComponent {
     datePipe: DatePipe,
     configService: ConfigService,
     billingAccountProfileStateService: BillingAccountProfileStateService,
+    cipherAuthorizationService: CipherAuthorizationService,
   ) {
     super(
       cipherService,
@@ -76,6 +78,7 @@ export class EmergencyAddEditCipherComponent extends BaseAddEditComponent {
       datePipe,
       configService,
       billingAccountProfileStateService,
+      cipherAuthorizationService,
     );
   }
 

diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html
@@ -72,7 +72,7 @@
         buttonType="danger"
         [appA11yTitle]="'delete' | i18n"
         [bitAction]="delete"
-        [disabled]="!canDelete"
+        [disabled]="!(canDeleteCipher$ | async)"
         data-testid="delete-cipher-btn"
       ></button>
     </div>