[PM-11343] Browser Refresh - View dialog permissions in AC (#11092)

* [PM-11343] Add param to conditionally disable the edit button * [PM-11343] Cleanup router navigation and move query param handling to callers of the View dialog * [PM-11343] Fix failing test * [PM-11343] Fix missing router after merge * [PM-11343] Add null checks in case the dialog result is undefined (due to closing via the ESC key) * [PM-11343] Add support to provide a list of collections to the cipher view component * [PM-11343] Add collections as an optional view cipher dialog parameter * [PM-11343] Update the org vault to provide collections when opening the View cipher dialog * [PM-11343] Fix import * [PM-11343] Use [replaceUrl] for cipher items to avoid needing double back button
bitwarden · Sep 19, 2024 · 4327fa2 · 4327fa2
1 parent e1e772b
commit 4327fa2
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 106 deletions.
diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html
@@ -21,6 +21,7 @@
       [routerLink]="[]"
       [queryParams]="{ itemId: cipher.id, action: extensionRefreshEnabled ? 'view' : null }"
       queryParamsHandling="merge"
+      [replaceUrl]="extensionRefreshEnabled"
       title="{{ 'editItemWithName' | i18n: cipher.name }}"
       type="button"
       appStopProp

diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts
@@ -47,7 +47,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { SyncService } from "@bitwarden/common/platform/sync";
-import { CipherId, OrganizationId, CollectionId } from "@bitwarden/common/types/guid";
+import { CipherId, CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
 import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
@@ -722,10 +722,6 @@ export class VaultComponent implements OnInit, OnDestroy {
     this.go({ cipherId: null, itemId: null, action: null });
   }
 
-  async navigateToCipher(cipher: CipherView) {
-    this.go({ itemId: cipher?.id });
-  }
-
   async editCipher(cipher: CipherView, cloneMode?: boolean) {
     return this.editCipherId(cipher?.id, cloneMode);
   }
@@ -861,16 +857,19 @@ export class VaultComponent implements OnInit, OnDestroy {
     // Wait for the dialog to close.
     const result: ViewCipherDialogCloseResult = await lastValueFrom(dialogRef.closed);
 
+    // If the dialog was closed by clicking the edit button, navigate to open the edit dialog.
+    if (result?.action === ViewCipherDialogResult.Edited) {
+      this.go({ itemId: cipherView.id, action: "edit" });
+      return;
+    }
+
     // If the dialog was closed by deleting the cipher, refresh the vault.
     if (result?.action === ViewCipherDialogResult.Deleted) {
       this.refresh();
-      this.go({ cipherId: null, itemId: null, action: null });
     }
 
-    // If the dialog was closed by any other action (close button, escape key, etc), navigate back to the vault.
-    if (!result?.action) {
-      this.go({ cipherId: null, itemId: null, action: null });
-    }
+    // Clear the query params when the view dialog closes
+    this.go({ cipherId: null, itemId: null, action: null });
   }
 
   async addCollection() {

diff --git a/apps/web/src/app/vault/individual-vault/view.component.html b/apps/web/src/app/vault/individual-vault/view.component.html
@@ -3,13 +3,19 @@
     {{ cipherTypeString }}
   </span>
   <ng-container bitDialogContent>
-    <app-cipher-view [cipher]="cipher"></app-cipher-view>
+    <app-cipher-view [cipher]="cipher" [collections]="collections"></app-cipher-view>
   </ng-container>
   <ng-container bitDialogFooter>
-    <button bitButton (click)="edit()" buttonType="primary" type="button" [disabled]="!cipher.edit">
+    <button
+      bitButton
+      (click)="edit()"
+      buttonType="primary"
+      type="button"
+      [disabled]="params.disableEdit"
+    >
       {{ "edit" | i18n }}
     </button>
-    <div class="ml-auto">
+    <div class="tw-ml-auto">
       <button
         bitButton
         type="button"

diff --git a/apps/web/src/app/vault/individual-vault/view.component.spec.ts b/apps/web/src/app/vault/individual-vault/view.component.spec.ts
@@ -1,6 +1,5 @@
 import { DIALOG_DATA, DialogRef } from "@angular/cdk/dialog";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
-import { Router } from "@angular/router";
 import { mock } from "jest-mock-extended";
 
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@@ -17,12 +16,11 @@ import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folde
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { DialogService, ToastService } from "@bitwarden/components";
 
-import { ViewComponent, ViewCipherDialogParams, ViewCipherDialogResult } from "./view.component";
+import { ViewCipherDialogParams, ViewCipherDialogResult, ViewComponent } from "./view.component";
 
 describe("ViewComponent", () => {
   let component: ViewComponent;
   let fixture: ComponentFixture<ViewComponent>;
-  let router: Router;
 
   const mockCipher: CipherView = {
     id: "cipher-id",
@@ -56,7 +54,6 @@ describe("ViewComponent", () => {
           provide: OrganizationService,
           useValue: { get: jest.fn().mockResolvedValue(mockOrganization) },
         },
-        { provide: Router, useValue: mock<Router>() },
         { provide: CollectionService, useValue: mock<CollectionService>() },
         { provide: FolderService, useValue: mock<FolderService>() },
         { provide: CryptoService, useValue: mock<CryptoService>() },
@@ -70,7 +67,6 @@ describe("ViewComponent", () => {
 
     fixture = TestBed.createComponent(ViewComponent);
     component = fixture.componentInstance;
-    router = TestBed.inject(Router);
     component.params = mockParams;
     component.cipher = mockCipher;
   });
@@ -85,19 +81,11 @@ describe("ViewComponent", () => {
   });
 
   describe("edit", () => {
-    it("navigates to the edit route and closes the dialog with the proper arguments", async () => {
-      jest.spyOn(router, "navigate").mockResolvedValue(true);
+    it("closes the dialog with the proper arguments", async () => {
       const dialogRefCloseSpy = jest.spyOn(component["dialogRef"], "close");
 
       await component.edit();
 
-      expect(router.navigate).toHaveBeenCalledWith([], {
-        queryParams: {
-          itemId: mockCipher.id,
-          action: "edit",
-          organizationId: mockCipher.organizationId,
-        },
-      });
       expect(dialogRefCloseSpy).toHaveBeenCalledWith({ action: ViewCipherDialogResult.Edited });
     });
   });

diff --git a/apps/web/src/app/vault/individual-vault/view.component.ts b/apps/web/src/app/vault/individual-vault/view.component.ts
@@ -1,8 +1,6 @@
 import { DIALOG_DATA, DialogConfig, DialogRef } from "@angular/cdk/dialog";
 import { CommonModule } from "@angular/common";
-import { Component, Inject, OnInit, EventEmitter, OnDestroy } from "@angular/core";
-import { Router } from "@angular/router";
-import { Subject } from "rxjs";
+import { Component, Inject, OnInit, EventEmitter } from "@angular/core";
 
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@@ -12,6 +10,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
 import {
   AsyncActionsModule,
   DialogModule,
@@ -26,6 +25,17 @@ import { WebVaultPremiumUpgradePromptService } from "../services/web-premium-upg
 
 export interface ViewCipherDialogParams {
   cipher: CipherView;
+
+  /**
+   * Optional list of collections the cipher is assigned to. If none are provided, they will be loaded using the
+   * `CipherService` and the `collectionIds` property of the cipher.
+   */
+  collections?: CollectionView[];
+
+  /**
+   * If true, the edit button will be disabled in the dialog.
+   */
+  disableEdit?: boolean;
 }
 
 export enum ViewCipherDialogResult {
@@ -50,14 +60,13 @@ export interface ViewCipherDialogCloseResult {
     { provide: PremiumUpgradePromptService, useClass: WebVaultPremiumUpgradePromptService },
   ],
 })
-export class ViewComponent implements OnInit, OnDestroy {
+export class ViewComponent implements OnInit {
   cipher: CipherView;
+  collections?: CollectionView[];
   onDeletedCipher = new EventEmitter<CipherView>();
   cipherTypeString: string;
   organization: Organization;
 
-  protected destroy$ = new Subject<void>();
-
   constructor(
     @Inject(DIALOG_DATA) public params: ViewCipherDialogParams,
     private dialogRef: DialogRef<ViewCipherDialogCloseResult>,
@@ -68,28 +77,20 @@ export class ViewComponent implements OnInit, OnDestroy {
     private cipherService: CipherService,
     private toastService: ToastService,
     private organizationService: OrganizationService,
-    private router: Router,
   ) {}
 
   /**
    * Lifecycle hook for component initialization.
    */
   async ngOnInit() {
     this.cipher = this.params.cipher;
+    this.collections = this.params.collections;
     this.cipherTypeString = this.getCipherViewTypeString();
     if (this.cipher.organizationId) {
       this.organization = await this.organizationService.get(this.cipher.organizationId);
     }
   }
 
-  /**
-   * Lifecycle hook for component destruction.
-   */
-  ngOnDestroy(): void {
-    this.destroy$.next();
-    this.destroy$.complete();
-  }
-
   /**
    * Method to handle cipher deletion. Called when a user clicks the delete button.
    */
@@ -124,7 +125,6 @@ export class ViewComponent implements OnInit, OnDestroy {
     }
 
     this.dialogRef.close({ action: ViewCipherDialogResult.Deleted });
-    await this.router.navigate(["/vault"]);
   };
 
   /**
@@ -144,13 +144,6 @@ export class ViewComponent implements OnInit, OnDestroy {
    */
   async edit(): Promise<void> {
     this.dialogRef.close({ action: ViewCipherDialogResult.Edited });
-    await this.router.navigate([], {
-      queryParams: {
-        itemId: this.cipher.id,
-        action: "edit",
-        organizationId: this.cipher.organizationId,
-      },
-    });
   }
 
   /**

diff --git a/apps/web/src/app/vault/org-vault/vault.component.ts b/apps/web/src/app/vault/org-vault/vault.component.ts
@@ -494,20 +494,23 @@ export class VaultComponent implements OnInit, OnDestroy {
 
     firstSetup$
       .pipe(
-        switchMap(() => combineLatest([this.route.queryParams, organization$])),
-        switchMap(async ([qParams, organization]) => {
+        switchMap(() =>
+          combineLatest([this.route.queryParams, allCipherMap$, allCollections$, organization$]),
+        ),
+        switchMap(async ([qParams, allCiphersMap, allCollections]) => {
           const cipherId = getCipherIdFromParams(qParams);
           if (!cipherId) {
             return;
           }
 
-          const canEditCipher =
-            organization.canEditAllCiphers ||
-            (await firstValueFrom(allCipherMap$))[cipherId] != undefined;
+          const cipher = allCiphersMap[cipherId];
+          const cipherCollections = allCollections.filter((c) =>
+            cipher.collectionIds.includes(c.id),
+          );
 
-          if (canEditCipher) {
+          if (cipher) {
             if (qParams.action === "view") {
-              await this.viewCipherById(cipherId);
+              await this.viewCipher(cipher, cipherCollections);
             } else {
               await this.editCipherId(cipherId);
             }
@@ -775,10 +778,6 @@ export class VaultComponent implements OnInit, OnDestroy {
     });
   }
 
-  async navigateToCipher(cipher: CipherView) {
-    this.go({ itemId: cipher?.id });
-  }
-
   async editCipher(
     cipher: CipherView,
     additionalComponentParameters?: (comp: AddEditComponent) => void,
@@ -842,59 +841,46 @@ export class VaultComponent implements OnInit, OnDestroy {
   }
 
   /**
-   * Takes a CipherView and opens a dialog where it can be viewed (wraps viewCipherById).
-   * @param cipher - CipherView
-   * @returns Promise<void>
+   * Takes a cipher and its assigned collections to opens dialog where it can be viewed.
+   * @param cipher - the cipher to view
+   * @param collections - the collections the cipher is assigned to
    */
-  viewCipher(cipher: CipherView) {
-    return this.viewCipherById(cipher.id);
-  }
-
-  /**
-   * Takes a cipher id and opens a dialog where it can be viewed.
-   * @param id - string
-   * @returns Promise<void>
-   */
-  async viewCipherById(id: string) {
-    const cipher = await this.cipherService.get(id);
-    // if cipher exists (cipher is null when new) and MP reprompt
-    // is on for this cipher, then show password reprompt.
-    if (
-      cipher &&
-      cipher.reprompt !== 0 &&
-      !(await this.passwordRepromptService.showPasswordPrompt())
-    ) {
-      // didn't pass password prompt, so don't open add / edit modal.
+  async viewCipher(cipher: CipherView, collections: CollectionView[] = []) {
+    if (!cipher) {
       this.go({ cipherId: null, itemId: null });
       return;
     }
 
-    const activeUserId = await firstValueFrom(
-      this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-    );
-    // Decrypt the cipher.
-    const cipherView = await cipher.decrypt(
-      await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
-    );
+    if (cipher.reprompt !== 0 && !(await this.passwordRepromptService.showPasswordPrompt())) {
+      // didn't pass password prompt, so don't open the dialog
+      this.go({ cipherId: null, itemId: null });
+      return;
+    }
 
-    // Open the dialog.
     const dialogRef = openViewCipherDialog(this.dialogService, {
-      data: { cipher: cipherView },
+      data: {
+        cipher: cipher,
+        collections: collections,
+        disableEdit: !cipher.edit && !this.organization.canEditAllCiphers,
+      },
     });
 
     // Wait for the dialog to close.
     const result: ViewCipherDialogCloseResult = await lastValueFrom(dialogRef.closed);
 
+    // If the dialog was closed by clicking the edit button, navigate to open the edit dialog.
+    if (result?.action === ViewCipherDialogResult.Edited) {
+      this.go({ itemId: cipher.id, action: "edit" });
+      return;
+    }
+
     // If the dialog was closed by deleting the cipher, refresh the vault.
     if (result?.action === ViewCipherDialogResult.Deleted) {
       this.refresh();
-      this.go({ cipherId: null, itemId: null, action: null });
     }
 
-    // If the dialog was closed by any other action (close button, escape key, etc), navigate back to the vault.
-    if (!result?.action) {
-      this.go({ cipherId: null, itemId: null, action: null });
-    }
+    // Clear the query params when the view dialog closes
+    this.go({ cipherId: null, itemId: null, action: null });
   }
 
   async cloneCipher(cipher: CipherView) {

diff --git a/libs/vault/src/cipher-view/cipher-view.component.html b/libs/vault/src/cipher-view/cipher-view.component.html
@@ -16,7 +16,7 @@
   <app-item-details-v2
     [cipher]="cipher"
     [organization]="organization$ | async"
-    [collections]="collections$ | async"
+    [collections]="collections"
     [folder]="folder$ | async"
   >
   </app-item-details-v2>