diff --git a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html index 4f9e65454b..48db50e202 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html @@ -3,7 +3,7 @@ type="checkbox" bitCheckbox appStopProp - *ngIf="canDeleteCollection" + *ngIf="showCheckbox" [disabled]="disabled" [checked]="checked" (change)="$event ? this.checkedToggled.next() : null" diff --git a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts index db9648f37f..7dcb463548 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts @@ -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; + } } 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 9d82ee2ef9..6443d8de3d 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 @@ -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; } diff --git a/apps/web/src/app/vault/core/views/collection-admin.view.ts b/apps/web/src/app/vault/core/views/collection-admin.view.ts index 40638268e1..96a8150b0a 100644 --- a/apps/web/src/app/vault/core/views/collection-admin.view.ts +++ b/apps/web/src/app/vault/core/views/collection-admin.view.ts @@ -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) + ); } /** diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index f203fee063..474e9045d0 100644 --- 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,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 { - 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$); + } } /** diff --git a/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts b/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts index e13ef49fc3..8998629b66 100644 --- a/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts +++ b/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts @@ -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; diff --git a/apps/web/src/app/vault/org-vault/vault.component.ts b/apps/web/src/app/vault/org-vault/vault.component.ts index 0b16c1c78f..95dd9cab16 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.ts +++ b/apps/web/src/app/vault/org-vault/vault.component.ts @@ -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; + protected editableCollections$: Observable; protected allCollectionsWithoutUnassigned$: Observable; 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 { - 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 { + 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 { 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 { + async bulkEditCollectionAccess( + collections: CollectionView[], + organization: Organization, + ): Promise { 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"), + }); + } } /** diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index cd3147532d..9e5ad73a03 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -8187,6 +8187,9 @@ "viewAccess": { "message": "View access" }, + "noCollectionsSelected": { + "message": "You have not selected any collections." + }, "updateName": { "message": "Update name" }, diff --git a/libs/common/src/vault/models/view/collection.view.ts b/libs/common/src/vault/models/view/collection.view.ts index d77af747f8..f5f9cb60b7 100644 --- a/libs/common/src/vault/models/view/collection.view.ts +++ b/libs/common/src/vault/models/view/collection.view.ts @@ -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)