1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-22 16:29:09 +01:00

[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
This commit is contained in:
Shane Melton 2024-09-19 10:43:28 -07:00 committed by GitHub
parent e1e772b0a2
commit 4327fa21f6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 90 additions and 106 deletions

View File

@ -21,6 +21,7 @@
[routerLink]="[]" [routerLink]="[]"
[queryParams]="{ itemId: cipher.id, action: extensionRefreshEnabled ? 'view' : null }" [queryParams]="{ itemId: cipher.id, action: extensionRefreshEnabled ? 'view' : null }"
queryParamsHandling="merge" queryParamsHandling="merge"
[replaceUrl]="extensionRefreshEnabled"
title="{{ 'editItemWithName' | i18n: cipher.name }}" title="{{ 'editItemWithName' | i18n: cipher.name }}"
type="button" type="button"
appStopProp appStopProp

View File

@ -47,7 +47,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { Utils } from "@bitwarden/common/platform/misc/utils"; import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SyncService } from "@bitwarden/common/platform/sync"; 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 { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
import { TotpService } from "@bitwarden/common/vault/abstractions/totp.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 }); this.go({ cipherId: null, itemId: null, action: null });
} }
async navigateToCipher(cipher: CipherView) {
this.go({ itemId: cipher?.id });
}
async editCipher(cipher: CipherView, cloneMode?: boolean) { async editCipher(cipher: CipherView, cloneMode?: boolean) {
return this.editCipherId(cipher?.id, cloneMode); return this.editCipherId(cipher?.id, cloneMode);
} }
@ -861,17 +857,20 @@ export class VaultComponent implements OnInit, OnDestroy {
// Wait for the dialog to close. // Wait for the dialog to close.
const result: ViewCipherDialogCloseResult = await lastValueFrom(dialogRef.closed); 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 the dialog was closed by deleting the cipher, refresh the vault.
if (result?.action === ViewCipherDialogResult.Deleted) { if (result?.action === ViewCipherDialogResult.Deleted) {
this.refresh(); 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. // Clear the query params when the view dialog closes
if (!result?.action) {
this.go({ cipherId: null, itemId: null, action: null }); this.go({ cipherId: null, itemId: null, action: null });
} }
}
async addCollection() { async addCollection() {
const dialog = openCollectionDialog(this.dialogService, { const dialog = openCollectionDialog(this.dialogService, {

View File

@ -3,13 +3,19 @@
{{ cipherTypeString }} {{ cipherTypeString }}
</span> </span>
<ng-container bitDialogContent> <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>
<ng-container bitDialogFooter> <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 }} {{ "edit" | i18n }}
</button> </button>
<div class="ml-auto"> <div class="tw-ml-auto">
<button <button
bitButton bitButton
type="button" type="button"

View File

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

View File

@ -1,8 +1,6 @@
import { DIALOG_DATA, DialogConfig, DialogRef } from "@angular/cdk/dialog"; import { DIALOG_DATA, DialogConfig, DialogRef } from "@angular/cdk/dialog";
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, Inject, OnInit, EventEmitter, OnDestroy } from "@angular/core"; import { Component, Inject, OnInit, EventEmitter } from "@angular/core";
import { Router } from "@angular/router";
import { Subject } from "rxjs";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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 { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums/cipher-type"; import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { import {
AsyncActionsModule, AsyncActionsModule,
DialogModule, DialogModule,
@ -26,6 +25,17 @@ import { WebVaultPremiumUpgradePromptService } from "../services/web-premium-upg
export interface ViewCipherDialogParams { export interface ViewCipherDialogParams {
cipher: CipherView; 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 { export enum ViewCipherDialogResult {
@ -50,14 +60,13 @@ export interface ViewCipherDialogCloseResult {
{ provide: PremiumUpgradePromptService, useClass: WebVaultPremiumUpgradePromptService }, { provide: PremiumUpgradePromptService, useClass: WebVaultPremiumUpgradePromptService },
], ],
}) })
export class ViewComponent implements OnInit, OnDestroy { export class ViewComponent implements OnInit {
cipher: CipherView; cipher: CipherView;
collections?: CollectionView[];
onDeletedCipher = new EventEmitter<CipherView>(); onDeletedCipher = new EventEmitter<CipherView>();
cipherTypeString: string; cipherTypeString: string;
organization: Organization; organization: Organization;
protected destroy$ = new Subject<void>();
constructor( constructor(
@Inject(DIALOG_DATA) public params: ViewCipherDialogParams, @Inject(DIALOG_DATA) public params: ViewCipherDialogParams,
private dialogRef: DialogRef<ViewCipherDialogCloseResult>, private dialogRef: DialogRef<ViewCipherDialogCloseResult>,
@ -68,7 +77,6 @@ export class ViewComponent implements OnInit, OnDestroy {
private cipherService: CipherService, private cipherService: CipherService,
private toastService: ToastService, private toastService: ToastService,
private organizationService: OrganizationService, private organizationService: OrganizationService,
private router: Router,
) {} ) {}
/** /**
@ -76,20 +84,13 @@ export class ViewComponent implements OnInit, OnDestroy {
*/ */
async ngOnInit() { async ngOnInit() {
this.cipher = this.params.cipher; this.cipher = this.params.cipher;
this.collections = this.params.collections;
this.cipherTypeString = this.getCipherViewTypeString(); this.cipherTypeString = this.getCipherViewTypeString();
if (this.cipher.organizationId) { if (this.cipher.organizationId) {
this.organization = await this.organizationService.get(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. * 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 }); 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> { async edit(): Promise<void> {
this.dialogRef.close({ action: ViewCipherDialogResult.Edited }); this.dialogRef.close({ action: ViewCipherDialogResult.Edited });
await this.router.navigate([], {
queryParams: {
itemId: this.cipher.id,
action: "edit",
organizationId: this.cipher.organizationId,
},
});
} }
/** /**

View File

@ -494,20 +494,23 @@ export class VaultComponent implements OnInit, OnDestroy {
firstSetup$ firstSetup$
.pipe( .pipe(
switchMap(() => combineLatest([this.route.queryParams, organization$])), switchMap(() =>
switchMap(async ([qParams, organization]) => { combineLatest([this.route.queryParams, allCipherMap$, allCollections$, organization$]),
),
switchMap(async ([qParams, allCiphersMap, allCollections]) => {
const cipherId = getCipherIdFromParams(qParams); const cipherId = getCipherIdFromParams(qParams);
if (!cipherId) { if (!cipherId) {
return; return;
} }
const canEditCipher = const cipher = allCiphersMap[cipherId];
organization.canEditAllCiphers || const cipherCollections = allCollections.filter((c) =>
(await firstValueFrom(allCipherMap$))[cipherId] != undefined; cipher.collectionIds.includes(c.id),
);
if (canEditCipher) { if (cipher) {
if (qParams.action === "view") { if (qParams.action === "view") {
await this.viewCipherById(cipherId); await this.viewCipher(cipher, cipherCollections);
} else { } else {
await this.editCipherId(cipherId); 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( async editCipher(
cipher: CipherView, cipher: CipherView,
additionalComponentParameters?: (comp: AddEditComponent) => void, additionalComponentParameters?: (comp: AddEditComponent) => void,
@ -842,60 +841,47 @@ export class VaultComponent implements OnInit, OnDestroy {
} }
/** /**
* Takes a CipherView and opens a dialog where it can be viewed (wraps viewCipherById). * Takes a cipher and its assigned collections to opens dialog where it can be viewed.
* @param cipher - CipherView * @param cipher - the cipher to view
* @returns Promise<void> * @param collections - the collections the cipher is assigned to
*/ */
viewCipher(cipher: CipherView) { async viewCipher(cipher: CipherView, collections: CollectionView[] = []) {
return this.viewCipherById(cipher.id); if (!cipher) {
}
/**
* 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.
this.go({ cipherId: null, itemId: null }); this.go({ cipherId: null, itemId: null });
return; return;
} }
const activeUserId = await firstValueFrom( if (cipher.reprompt !== 0 && !(await this.passwordRepromptService.showPasswordPrompt())) {
this.accountService.activeAccount$.pipe(map((a) => a?.id)), // didn't pass password prompt, so don't open the dialog
); this.go({ cipherId: null, itemId: null });
// Decrypt the cipher. return;
const cipherView = await cipher.decrypt( }
await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
);
// Open the dialog.
const dialogRef = openViewCipherDialog(this.dialogService, { 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. // Wait for the dialog to close.
const result: ViewCipherDialogCloseResult = await lastValueFrom(dialogRef.closed); 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 the dialog was closed by deleting the cipher, refresh the vault.
if (result?.action === ViewCipherDialogResult.Deleted) { if (result?.action === ViewCipherDialogResult.Deleted) {
this.refresh(); 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. // Clear the query params when the view dialog closes
if (!result?.action) {
this.go({ cipherId: null, itemId: null, action: null }); this.go({ cipherId: null, itemId: null, action: null });
} }
}
async cloneCipher(cipher: CipherView) { async cloneCipher(cipher: CipherView) {
if (cipher.login?.hasFido2Credentials) { if (cipher.login?.hasFido2Credentials) {

View File

@ -16,7 +16,7 @@
<app-item-details-v2 <app-item-details-v2
[cipher]="cipher" [cipher]="cipher"
[organization]="organization$ | async" [organization]="organization$ | async"
[collections]="collections$ | async" [collections]="collections"
[folder]="folder$ | async" [folder]="folder$ | async"
> >
</app-item-details-v2> </app-item-details-v2>

View File

@ -1,6 +1,6 @@
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, Input, OnDestroy, OnInit } from "@angular/core"; import { Component, Input, OnDestroy, OnInit } from "@angular/core";
import { Observable, Subject, takeUntil } from "rxjs"; import { firstValueFrom, Observable, Subject, takeUntil } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
@ -12,7 +12,7 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { isCardExpired } from "@bitwarden/common/vault/utils"; import { isCardExpired } from "@bitwarden/common/vault/utils";
import { SearchModule, CalloutModule } from "@bitwarden/components"; import { CalloutModule, SearchModule } from "@bitwarden/components";
import { AdditionalOptionsComponent } from "./additional-options/additional-options.component"; import { AdditionalOptionsComponent } from "./additional-options/additional-options.component";
import { AttachmentsV2ViewComponent } from "./attachments/attachments-v2-view.component"; import { AttachmentsV2ViewComponent } from "./attachments/attachments-v2-view.component";
@ -45,10 +45,15 @@ import { ViewIdentitySectionsComponent } from "./view-identity-sections/view-ide
], ],
}) })
export class CipherViewComponent implements OnInit, OnDestroy { export class CipherViewComponent implements OnInit, OnDestroy {
@Input() cipher: CipherView; @Input({ required: true }) cipher: CipherView;
/**
* Optional list of collections the cipher is assigned to. If none are provided, they will be fetched using the
* `CipherService` and the `collectionIds` property of the cipher.
*/
@Input() collections: CollectionView[];
organization$: Observable<Organization>; organization$: Observable<Organization>;
folder$: Observable<FolderView>; folder$: Observable<FolderView>;
collections$: Observable<CollectionView[]>;
private destroyed$: Subject<void> = new Subject(); private destroyed$: Subject<void> = new Subject();
cardIsExpired: boolean = false; cardIsExpired: boolean = false;
@ -84,10 +89,16 @@ export class CipherViewComponent implements OnInit, OnDestroy {
} }
async loadCipherData() { async loadCipherData() {
if (this.cipher.collectionIds.length > 0) { // Load collections if not provided and the cipher has collectionIds
this.collections$ = this.collectionService if (
.decryptedCollectionViews$(this.cipher.collectionIds as CollectionId[]) this.cipher.collectionIds.length > 0 &&
.pipe(takeUntil(this.destroyed$)); (!this.collections || this.collections.length === 0)
) {
this.collections = await firstValueFrom(
this.collectionService.decryptedCollectionViews$(
this.cipher.collectionIds as CollectionId[],
),
);
} }
if (this.cipher.organizationId) { if (this.cipher.organizationId) {