Upstream
/
bitwarden-browser
mirror of https://github.com/bitwarden/browser.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[AC-2499] Add permission checks on bulk actions menu (#8912) 

* Add permission checks for org vault bulk actions

* Show checkboxes for all collections except Unassigned

* Separate individual and admin logic between CollectionView
  and CollectionAdminView

* Remove heading for error toasts per design feedback
This commit is contained in:
Thomas Rittson 2024-05-15 08:29:54 +10:00 committed by GitHub
parent 3eeafc098a
commit 6ab7336c21
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 221 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html
View File

@ -3,7 +3,7 @@
type="checkbox"
bitCheckbox
appStopProp
*ngIf="canDeleteCollection"
*ngIf="showCheckbox"
[disabled]="disabled"
[checked]="checked"
(change)="$event ? this.checkedToggled.next() : null"

8
apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts
View File

@ -90,4 +90,12 @@ export class VaultCollectionRowComponent {
protected deleteCollection() {
this.onEvent.next({ type: "delete", items: [{ collection: this.collection }] });
}
protected get showCheckbox() {
if (this.flexibleCollectionsV1Enabled) {
return this.collection?.id !== Unassigned;
}
return this.canDeleteCollection;
}
}

22
apps/web/src/app/vault/components/vault-items/vault-items.component.ts
View File

@ -245,11 +245,23 @@ export class VaultItemsComponent {
const items: VaultItem[] = [].concat(collections).concat(ciphers);
this.selection.clear();
this.editableItems = items.filter(
(item) =>
item.cipher !== undefined ||
(item.collection !== undefined && this.canDeleteCollection(item.collection)),
);
if (this.flexibleCollectionsV1Enabled) {
// Every item except for the Unassigned collection is selectable, individual bulk actions check the user's permission
this.editableItems = items.filter(
(item) =>
item.cipher !== undefined ||
(item.collection !== undefined && item.collection.id !== Unassigned),
);
} else {
// only collections the user can delete are selectable
this.editableItems = items.filter(
(item) =>
item.cipher !== undefined ||
(item.collection !== undefined && this.canDeleteCollection(item.collection)),
);
}
this.dataSource.data = items;
}

20
apps/web/src/app/vault/core/views/collection-admin.view.ts
View File

@ -62,13 +62,23 @@ export class CollectionAdminView extends CollectionView {
}
/**
* Whether the current user can edit the collection, including user and group access
* Returns true if the user can edit a collection (including user and group access) from the Admin Console.
*/
override canEdit(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
return org?.flexibleCollections
? org?.canEditAnyCollection(flexibleCollectionsV1Enabled) || this.manage
: org?.canEditAnyCollection(flexibleCollectionsV1Enabled) ||
(org?.canEditAssignedCollections && this.assigned);
return (
org?.canEditAnyCollection(flexibleCollectionsV1Enabled) ||
super.canEdit(org, flexibleCollectionsV1Enabled)
);
}
/**
* Returns true if the user can delete a collection from the Admin Console.
*/
override canDelete(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
return (
org?.canDeleteAnyCollection(flexibleCollectionsV1Enabled) ||
super.canDelete(org, flexibleCollectionsV1Enabled)
);
}
/**

101
apps/web/src/app/vault/individual-vault/vault.component.ts
View File

@ -47,7 +47,6 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { KdfType, PBKDF2_ITERATIONS } from "@bitwarden/common/platform/enums";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
@ -61,7 +60,7 @@ import { CollectionDetailsResponse } from "@bitwarden/common/vault/models/respon
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { ServiceUtils } from "@bitwarden/common/vault/service-utils";
import { DialogService, Icons } from "@bitwarden/components";
import { DialogService, Icons, ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import {
@ -167,7 +166,6 @@ export class VaultComponent implements OnInit, OnDestroy {
private platformUtilsService: PlatformUtilsService,
private broadcasterService: BroadcasterService,
private ngZone: NgZone,
private stateService: StateService,
private organizationService: OrganizationService,
private vaultFilterService: VaultFilterService,
private routedVaultFilterService: RoutedVaultFilterService,
@ -184,6 +182,7 @@ export class VaultComponent implements OnInit, OnDestroy {
private apiService: ApiService,
private userVerificationService: UserVerificationService,
private billingAccountProfileStateService: BillingAccountProfileStateService,
private toastService: ToastService,
protected kdfConfigService: KdfConfigService,
) {}
@ -347,7 +346,7 @@ export class VaultComponent implements OnInit, OnDestroy {
} else {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
null,
this.i18nService.t("unknownCipher"),
);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
@ -551,6 +550,12 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async shareCipher(cipher: CipherView) {
if ((await this.flexibleCollectionsV1Enabled()) && cipher.organizationId != null) {
// You cannot move ciphers between organizations
this.showMissingPermissionsError();
return;
}
if (cipher?.reprompt !== 0 && !(await this.passwordRepromptService.showPasswordPrompt())) {
this.go({ cipherId: null, itemId: null });
return;
@ -700,11 +705,7 @@ export class VaultComponent implements OnInit, OnDestroy {
const organization = await this.organizationService.get(collection.organizationId);
const flexibleCollectionsV1Enabled = await firstValueFrom(this.flexibleCollectionsV1Enabled$);
if (!collection.canDelete(organization, flexibleCollectionsV1Enabled)) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("missingPermissions"),
);
this.showMissingPermissionsError();
return;
}
const confirmed = await this.dialogService.openSimpleDialog({
@ -755,11 +756,16 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async restore(c: CipherView): Promise<boolean> {
if (!(await this.repromptCipher([c]))) {
if (!c.isDeleted) {
return;
}
if (!c.isDeleted) {
if ((await this.flexibleCollectionsV1Enabled()) && !c.edit) {
this.showMissingPermissionsError();
return;
}
if (!(await this.repromptCipher([c]))) {
return;
}
@ -773,17 +779,18 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async bulkRestore(ciphers: CipherView[]) {
if ((await this.flexibleCollectionsV1Enabled()) && ciphers.some((c) => !c.edit)) {
this.showMissingPermissionsError();
return;
}
if (!(await this.repromptCipher(ciphers))) {
return;
}
const selectedCipherIds = ciphers.map((cipher) => cipher.id);
if (selectedCipherIds.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
@ -817,6 +824,11 @@ export class VaultComponent implements OnInit, OnDestroy {
return;
}
if ((await this.flexibleCollectionsV1Enabled()) && !c.edit) {
this.showMissingPermissionsError();
return;
}
const permanent = c.isDeleted;
const confirmed = await this.dialogService.openSimpleDialog({
@ -852,13 +864,27 @@ export class VaultComponent implements OnInit, OnDestroy {
}
if (ciphers.length === 0 && collections.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
const flexibleCollectionsV1Enabled = await this.flexibleCollectionsV1Enabled();
const canDeleteCollections =
collections == null ||
collections.every((c) =>
c.canDelete(
organizations.find((o) => o.id == c.organizationId),
flexibleCollectionsV1Enabled,
),
);
const canDeleteCiphers = ciphers == null || ciphers.every((c) => c.edit);
if (flexibleCollectionsV1Enabled && (!canDeleteCollections || !canDeleteCiphers)) {
this.showMissingPermissionsError();
return;
}
const dialog = openBulkDeleteDialog(this.dialogService, {
data: {
permanent: this.filter.type === "trash",
@ -881,11 +907,7 @@ export class VaultComponent implements OnInit, OnDestroy {
const selectedCipherIds = ciphers.map((cipher) => cipher.id);
if (selectedCipherIds.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
@ -955,12 +977,17 @@ export class VaultComponent implements OnInit, OnDestroy {
return;
}
if (
(await this.flexibleCollectionsV1Enabled()) &&
ciphers.some((c) => c.organizationId != null)
) {
// You cannot move ciphers between organizations
this.showMissingPermissionsError();
return;
}
if (ciphers.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
@ -1016,6 +1043,18 @@ export class VaultComponent implements OnInit, OnDestroy {
replaceUrl: true,
});
}
private showMissingPermissionsError() {
this.toastService.showToast({
variant: "error",
title: null,
message: this.i18nService.t("missingPermissions"),
});
}
private flexibleCollectionsV1Enabled() {
return firstValueFrom(this.flexibleCollectionsV1Enabled$);
}
}
/**

14
apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts
View File

@ -70,6 +70,13 @@ export class BulkCollectionAssignmentDialogComponent implements OnDestroy, OnIni
) {}
async ngOnInit() {
// If no ciphers are passed in, close the dialog
if (this.params.ciphers == null || this.params.ciphers.length < 1) {
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
this.dialogRef.close(BulkCollectionAssignmentDialogResult.Canceled);
return;
}
const v1FCEnabled = await this.configService.getFeatureFlag(FeatureFlag.FlexibleCollectionsV1);
const restrictProviderAccess = await this.configService.getFeatureFlag(
FeatureFlag.RestrictProviderAccess,
@ -86,12 +93,9 @@ export class BulkCollectionAssignmentDialogComponent implements OnDestroy, OnIni
// If no ciphers are editable, close the dialog
if (this.editableItemCount == 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("missingPermissions"));
this.dialogRef.close(BulkCollectionAssignmentDialogResult.Canceled);
return;
}
this.totalItemCount = this.params.ciphers.length;

120
apps/web/src/app/vault/org-vault/vault.component.ts
View File

@ -59,7 +59,7 @@ import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
import { ServiceUtils } from "@bitwarden/common/vault/service-utils";
import { DialogService, Icons } from "@bitwarden/components";
import { DialogService, Icons, ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault";
import { GroupService, GroupView } from "../../admin-console/organizations/core";
@ -152,7 +152,7 @@ export class VaultComponent implements OnInit, OnDestroy {
* A list of collections that the user can assign items to and edit those items within.
* @protected
*/
protected editableCollections$: Observable<CollectionView[]>;
protected editableCollections$: Observable<CollectionAdminView[]>;
protected allCollectionsWithoutUnassigned$: Observable<CollectionAdminView[]>;
private _flexibleCollectionsV1FlagEnabled: boolean;
@ -200,6 +200,7 @@ export class VaultComponent implements OnInit, OnDestroy {
private collectionService: CollectionService,
private organizationUserService: OrganizationUserService,
protected configService: ConfigService,
private toastService: ToastService,
) {}
async ngOnInit() {
@ -567,11 +568,7 @@ export class VaultComponent implements OnInit, OnDestroy {
if (canEditCipher) {
await this.editCipherId(cipherId);
} else {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("unknownCipher"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("unknownCipher"));
await this.router.navigate([], {
queryParams: { cipherId: null, itemId: null },
queryParamsHandling: "merge",
@ -596,11 +593,7 @@ export class VaultComponent implements OnInit, OnDestroy {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.viewEvents(cipher);
} else {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("unknownCipher"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("unknownCipher"));
// 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([], {
@ -765,7 +758,7 @@ export class VaultComponent implements OnInit, OnDestroy {
} else if (event.type === "viewCollectionAccess") {
await this.editCollection(event.item, CollectionDialogTabType.Access, event.readonly);
} else if (event.type === "bulkEditCollectionAccess") {
await this.bulkEditCollectionAccess(event.items);
await this.bulkEditCollectionAccess(event.items, this.organization);
} else if (event.type === "assignToCollections") {
await this.bulkAssignToCollections(event.items);
} else if (event.type === "viewEvents") {
@ -817,7 +810,7 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async editCipherCollections(cipher: CipherView) {
let collections: CollectionView[] = [];
let collections: CollectionAdminView[] = [];
if (this.flexibleCollectionsV1Enabled) {
// V1 limits admins to only adding items to collections they have access to.
@ -978,11 +971,20 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async restore(c: CipherView): Promise<boolean> {
if (!(await this.repromptCipher([c]))) {
if (!c.isDeleted) {
return;
}
if (!c.isDeleted) {
if (
this.flexibleCollectionsV1Enabled &&
!c.edit &&
!this.organization.allowAdminAccessToAllCollectionItems
) {
this.showMissingPermissionsError();
return;
}
if (!(await this.repromptCipher([c]))) {
return;
}
@ -997,17 +999,21 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async bulkRestore(ciphers: CipherView[]) {
if (
this.flexibleCollectionsV1Enabled &&
ciphers.some((c) => !c.edit && !this.organization.allowAdminAccessToAllCollectionItems)
) {
this.showMissingPermissionsError();
return;
}
if (!(await this.repromptCipher(ciphers))) {
return;
}
const selectedCipherIds = ciphers.map((cipher) => cipher.id);
if (selectedCipherIds.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
@ -1017,6 +1023,15 @@ export class VaultComponent implements OnInit, OnDestroy {
}
async deleteCipher(c: CipherView): Promise<boolean> {
if (
this.flexibleCollectionsV1Enabled &&
!c.edit &&
!this.organization.allowAdminAccessToAllCollectionItems
) {
this.showMissingPermissionsError();
return;
}
if (!(await this.repromptCipher([c]))) {
return;
}
@ -1048,11 +1063,7 @@ export class VaultComponent implements OnInit, OnDestroy {
async deleteCollection(collection: CollectionView): Promise<void> {
if (!collection.canDelete(this.organization, this.flexibleCollectionsV1Enabled)) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("missingPermissions"),
);
this.showMissingPermissionsError();
return;
}
const confirmed = await this.dialogService.openSimpleDialog({
@ -1097,13 +1108,23 @@ export class VaultComponent implements OnInit, OnDestroy {
}
if (ciphers.length === 0 && collections.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
const canDeleteCollections =
collections == null ||
collections.every((c) => c.canDelete(organization, this.flexibleCollectionsV1Enabled));
const canDeleteCiphers =
ciphers == null ||
this.organization.allowAdminAccessToAllCollectionItems ||
ciphers.every((c) => c.edit);
if (this.flexibleCollectionsV1Enabled && (!canDeleteCiphers || !canDeleteCollections)) {
this.showMissingPermissionsError();
return;
}
const dialog = openBulkDeleteDialog(this.dialogService, {
data: {
permanent: this.filter.type === "trash",
@ -1228,13 +1249,24 @@ export class VaultComponent implements OnInit, OnDestroy {
}
}
async bulkEditCollectionAccess(collections: CollectionView[]): Promise<void> {
async bulkEditCollectionAccess(
collections: CollectionView[],
organization: Organization,
): Promise<void> {
if (collections.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.toastService.showToast({
variant: "error",
title: null,
message: this.i18nService.t("noCollectionsSelected"),
});
return;
}
if (
this.flexibleCollectionsV1Enabled &&
collections.some((c) => !c.canEdit(organization, this.flexibleCollectionsV1Enabled))
) {
this.showMissingPermissionsError();
return;
}
@ -1253,11 +1285,7 @@ export class VaultComponent implements OnInit, OnDestroy {
async bulkAssignToCollections(items: CipherView[]) {
if (items.length === 0) {
this.platformUtilsService.showToast(
"error",
this.i18nService.t("errorOccurred"),
this.i18nService.t("nothingSelected"),
);
this.platformUtilsService.showToast("error", null, this.i18nService.t("nothingSelected"));
return;
}
@ -1338,6 +1366,14 @@ export class VaultComponent implements OnInit, OnDestroy {
}
protected readonly CollectionDialogTabType = CollectionDialogTabType;
private showMissingPermissionsError() {
this.toastService.showToast({
variant: "error",
title: null,
message: this.i18nService.t("missingPermissions"),
});
}
}
/**

3
apps/web/src/locales/en/messages.json
View File

@ -8187,6 +8187,9 @@
"viewAccess": {
"message": "View access"
},
"noCollectionsSelected": {
"message": "You have not selected any collections."
},
"updateName": {
"message": "Update name"
},

25
libs/common/src/vault/models/view/collection.view.ts
View File

@ -61,7 +61,10 @@ export class CollectionView implements View, ITreeNodeObject {
return org?.canEditAnyCollection(false) || (org?.canEditAssignedCollections && this.assigned);
}
// For editing collection details, not the items within it.
/**
* Returns true if the user can edit a collection (including user and group access) from the individual vault.
* After FCv1, does not include admin permissions - see {@link CollectionAdminView.canEdit}.
*/
canEdit(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
if (org != null && org.id !== this.organizationId) {
throw new Error(
@ -69,12 +72,18 @@ export class CollectionView implements View, ITreeNodeObject {
);
}
return org?.flexibleCollections
? org?.canEditAnyCollection(flexibleCollectionsV1Enabled) || this.manage
: org?.canEditAnyCollection(flexibleCollectionsV1Enabled) || org?.canEditAssignedCollections;
if (flexibleCollectionsV1Enabled) {
// Only use individual permissions, not admin permissions
return this.manage;
}
return org?.canEditAnyCollection(flexibleCollectionsV1Enabled) || this.manage;
}
// For deleting a collection, not the items within it.
/**
* Returns true if the user can delete a collection from the individual vault.
* After FCv1, does not include admin permissions - see {@link CollectionAdminView.canDelete}.
*/
canDelete(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
if (org != null && org.id !== this.organizationId) {
throw new Error(
@ -83,6 +92,12 @@ export class CollectionView implements View, ITreeNodeObject {
}
const canDeleteManagedCollections = !org?.limitCollectionCreationDeletion || org.isAdmin;
if (flexibleCollectionsV1Enabled) {
// Only use individual permissions, not admin permissions
return canDeleteManagedCollections && this.manage;
}
return (
org?.canDeleteAnyCollection(flexibleCollectionsV1Enabled) ||
(canDeleteManagedCollections && this.manage)