bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 05:13:29 +00:00
Code Issues Releases Wiki Activity

[PM-22100] Enforce restrictions based on collection type (#15336)

Browse Source 
* enforce restrictions based on collection type, set default collection type

* fix ts strict errors

* fix default collection enforcement in vault header

* enforce default collection restrictions in vault collection row

* enforce default collection restrictions in AC vault header

* enforce default collection restriction for select all

* fix ts strict error

* switch to signal, fix feature flag

* fix story

* clean up

* remove feature flag, move check for defaultCollecion to CollecitonView

* fix test

* remove unused configService

* fix test: coerce null to undefined for collection Id

* clean up leaky abstraction for default collection

* fix ts-strict error

* fix parens

* rename defaultCollection getter

* clean up
This commit is contained in:
Brandon Treston
2025-07-18 10:53:12 -04:00
committed by GitHub
parent 8811ec41ab
commit 92bbe0a3c2
14 changed files with 182 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts
View File

@@ -237,7 +237,7 @@ export class VaultPopupListFiltersService {
return false;
}
if (filters.collection && !cipher.collectionIds?.includes(filters.collection.id)) {
if (filters.collection && !cipher.collectionIds?.includes(filters.collection.id!)) {
return false;
}

15
apps/web/src/app/admin-console/organizations/collections/vault-header/vault-header.component.html
View File

@@ -25,11 +25,10 @@
</bit-breadcrumbs>
<ng-container slot="title-suffix">
<ng-container
*ngIf="
@if (
collection != null && (canEditCollection || canDeleteCollection || canViewCollectionInfo)
"
>
) {
<ng-container>
<button
bitIconButton="bwi-angle-down"
[bitMenuTriggerFor]="editCollectionMenu"
@@ -73,7 +72,12 @@
{{ "viewAccess" | i18n }}
</button>
</ng-container>
<button type="button" *ngIf="canDeleteCollection" bitMenuItem (click)="deleteCollection()">
<button
type="button"
*ngIf="canDeleteCollection"
bitMenuItem
(click)="deleteCollection()"
>
<span class="tw-text-danger">
<i class="bwi bwi-fw bwi-trash" aria-hidden="true"></i>
{{ "delete" | i18n }}
@@ -81,6 +85,7 @@
</button>
</bit-menu>
</ng-container>
}
<small *ngIf="loading">
<i
class="bwi bwi-spinner bwi-spin tw-text-muted"

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

@@ -1,14 +1,15 @@
<td bitCell [ngClass]="RowHeightClass" class="tw-min-w-fit">
@if (this.canEditCollection || this.canDeleteCollection) {
<input
type="checkbox"
bitCheckbox
appStopProp
*ngIf="showCheckbox"
[disabled]="disabled"
[checked]="checked"
(change)="$event ? this.checkedToggled.next() : null"
[attr.aria-label]="'collectionItemSelect' | i18n"
/>
}
</td>
<td bitCell [ngClass]="RowHeightClass" class="tw-min-w-fit">
<div aria-hidden="true">
@@ -57,8 +58,8 @@
</p>
</td>
<td bitCell [ngClass]="RowHeightClass" class="tw-text-right">
@if (canEditCollection || canDeleteCollection || canViewCollectionInfo) {
<button
*ngIf="canEditCollection || canDeleteCollection || canViewCollectionInfo"
[disabled]="disabled"
[bitMenuTriggerFor]="collectionOptions"
size="small"
@@ -67,6 +68,7 @@
appA11yTitle="{{ 'options' | i18n }}"
appStopProp
></button>
}
<bit-menu #collectionOptions>
<ng-container *ngIf="canEditCollection">
<button type="button" bitMenuItem (click)="edit(false)">

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

@@ -105,12 +105,4 @@ export class VaultCollectionRowComponent<C extends CipherViewLike> {
protected deleteCollection() {
this.onEvent.next({ type: "delete", items: [{ collection: this.collection }] });
}
protected get showCheckbox() {
if (this.collection?.id === Unassigned) {
return false; // Never show checkbox for Unassigned
}
return this.canEditCollection || this.canDeleteCollection;
}
}

11
apps/web/src/app/vault/individual-vault/vault-header/vault-header.component.html
View File

@@ -22,7 +22,8 @@
</bit-breadcrumbs>
<ng-container slot="title-suffix">
<ng-container *ngIf="collection != null && (canEditCollection || canDeleteCollection)">
@if (collection != null && (canEditCollection || canDeleteCollection)) {
<ng-container>
<button
bitIconButton="bwi-angle-down"
[bitMenuTriggerFor]="editCollectionMenu"
@@ -49,7 +50,12 @@
<i class="bwi bwi-fw bwi-users" aria-hidden="true"></i>
{{ "access" | i18n }}
</button>
<button type="button" *ngIf="canDeleteCollection" bitMenuItem (click)="deleteCollection()">
<button
type="button"
*ngIf="canDeleteCollection"
bitMenuItem
(click)="deleteCollection()"
>
<span class="tw-text-danger">
<i class="bwi bwi-fw bwi-trash" aria-hidden="true"></i>
{{ "delete" | i18n }}
@@ -57,6 +63,7 @@
</button>
</bit-menu>
</ng-container>
}
<small *ngIf="loading">
<i
class="bwi bwi-spinner bwi-spin tw-text-muted"

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

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { CommonModule } from "@angular/common";
import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from "@angular/core";
import { Router } from "@angular/router";
@@ -60,10 +58,10 @@ export class VaultHeaderComponent {
* Boolean to determine the loading state of the header.
* Shows a loading spinner if set to true
*/
@Input() loading: boolean;
@Input() loading: boolean = true;
/** Current active filter */
@Input() filter: RoutedVaultFilterModel;
@Input() filter: RoutedVaultFilterModel | undefined;
/** All organizations that can be shown */
@Input() organizations: Organization[] = [];
@@ -72,7 +70,7 @@ export class VaultHeaderComponent {
@Input() collection?: TreeNode<CollectionView>;
/** Whether 'Collection' option is shown in the 'New' dropdown */
@Input() canCreateCollections: boolean;
@Input() canCreateCollections: boolean = false;
/** Emits an event when the new item button is clicked in the header */
@Output() onAddCipher = new EventEmitter<CipherType | undefined>();
@@ -106,7 +104,7 @@ export class VaultHeaderComponent {
return this.collection.node.organizationId;
}
if (this.filter.organizationId !== undefined) {
if (this.filter?.organizationId !== undefined) {
return this.filter.organizationId;
}
@@ -119,10 +117,14 @@ export class VaultHeaderComponent {
}
protected get showBreadcrumbs() {
return this.filter.collectionId !== undefined && this.filter.collectionId !== All;
return this.filter?.collectionId !== undefined && this.filter.collectionId !== All;
}
protected get title() {
if (this.filter === undefined) {
return "";
}
if (this.filter.collectionId === Unassigned) {
return this.i18nService.t("unassigned");
}
@@ -144,7 +146,7 @@ export class VaultHeaderComponent {
}
protected get icon() {
return this.filter.collectionId && this.filter.collectionId !== All
return this.filter?.collectionId && this.filter.collectionId !== All
? "bwi-collection-shared"
: "";
}

28
libs/admin-console/src/common/collections/models/collection-admin.view.ts
View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { CollectionAccessSelectionView } from "./collection-access-selection.view";
@@ -16,12 +14,12 @@ export class CollectionAdminView extends CollectionView {
* Flag indicating the collection has no active user or group assigned to it with CanManage permissions
* In this case, the collection can be managed by admins/owners or custom users with appropriate permissions
*/
unmanaged: boolean;
unmanaged: boolean = false;
/**
* Flag indicating the user has been explicitly assigned to this Collection
*/
assigned: boolean;
assigned: boolean = false;
constructor(response?: CollectionAccessDetailsResponse) {
super(response);
@@ -45,6 +43,10 @@ export class CollectionAdminView extends CollectionView {
* Returns true if the user can edit a collection (including user and group access) from the Admin Console.
*/
override canEdit(org: Organization): boolean {
if (this.isDefaultCollection) {
return false;
}
return (
org?.canEditAnyCollection ||
(this.unmanaged && org?.canEditUnmanagedCollections) ||
@@ -56,6 +58,10 @@ export class CollectionAdminView extends CollectionView {
* Returns true if the user can delete a collection from the Admin Console.
*/
override canDelete(org: Organization): boolean {
if (this.isDefaultCollection) {
return false;
}
return org?.canDeleteAnyCollection || super.canDelete(org);
}
@@ -63,6 +69,10 @@ export class CollectionAdminView extends CollectionView {
* Whether the user can modify user access to this collection
*/
canEditUserAccess(org: Organization): boolean {
if (this.isDefaultCollection) {
return false;
}
return (
(org.permissions.manageUsers && org.allowAdminAccessToAllCollectionItems) || this.canEdit(org)
);
@@ -72,6 +82,10 @@ export class CollectionAdminView extends CollectionView {
* Whether the user can modify group access to this collection
*/
canEditGroupAccess(org: Organization): boolean {
if (this.isDefaultCollection) {
return false;
}
return (
(org.permissions.manageGroups && org.allowAdminAccessToAllCollectionItems) ||
this.canEdit(org)
@@ -82,11 +96,13 @@ export class CollectionAdminView extends CollectionView {
* Returns true if the user can view collection info and access in a read-only state from the Admin Console
*/
override canViewCollectionInfo(org: Organization | undefined): boolean {
if (this.isUnassignedCollection) {
if (this.isUnassignedCollection || this.isDefaultCollection) {
return false;
}
const isAdmin = org?.isAdmin ?? false;
const permissions = org?.permissions.editAnyCollection ?? false;
return this.manage || org?.isAdmin || org?.permissions.editAnyCollection;
return this.manage || isAdmin || permissions;
}
/**

36
libs/admin-console/src/common/collections/models/collection.view.ts
View File

@@ -1,27 +1,25 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Jsonify } from "type-fest";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { View } from "@bitwarden/common/models/view/view";
import { ITreeNodeObject } from "@bitwarden/common/vault/models/domain/tree-node";
import { Collection, CollectionType } from "./collection";
import { Collection, CollectionType, CollectionTypes } from "./collection";
import { CollectionAccessDetailsResponse } from "./collection.response";
export const NestingDelimiter = "/";
export class CollectionView implements View, ITreeNodeObject {
id: string = null;
organizationId: string = null;
name: string = null;
externalId: string = null;
id: string | undefined;
organizationId: string | undefined;
name: string | undefined;
externalId: string | undefined;
// readOnly applies to the items within a collection
readOnly: boolean = null;
hidePasswords: boolean = null;
manage: boolean = null;
assigned: boolean = null;
type: CollectionType = null;
readOnly: boolean = false;
hidePasswords: boolean = false;
manage: boolean = false;
assigned: boolean = false;
type: CollectionType = CollectionTypes.SharedCollection;
constructor(c?: Collection | CollectionAccessDetailsResponse) {
if (!c) {
@@ -57,7 +55,11 @@ export class CollectionView implements View, ITreeNodeObject {
* Returns true if the user can edit a collection (including user and group access) from the individual vault.
* Does not include admin permissions - see {@link CollectionAdminView.canEdit}.
*/
canEdit(org: Organization): boolean {
canEdit(org: Organization | undefined): boolean {
if (this.isDefaultCollection) {
return false;
}
if (org != null && org.id !== this.organizationId) {
throw new Error(
"Id of the organization provided does not match the org id of the collection.",
@@ -71,7 +73,7 @@ export class CollectionView implements View, ITreeNodeObject {
* Returns true if the user can delete a collection from the individual vault.
* Does not include admin permissions - see {@link CollectionAdminView.canDelete}.
*/
canDelete(org: Organization): boolean {
canDelete(org: Organization | undefined): boolean {
if (org != null && org.id !== this.organizationId) {
throw new Error(
"Id of the organization provided does not match the org id of the collection.",
@@ -81,7 +83,7 @@ export class CollectionView implements View, ITreeNodeObject {
const canDeleteManagedCollections = !org?.limitCollectionDeletion || org.isAdmin;
// Only use individual permissions, not admin permissions
return canDeleteManagedCollections && this.manage;
return canDeleteManagedCollections && this.manage && !this.isDefaultCollection;
}
/**
@@ -94,4 +96,8 @@ export class CollectionView implements View, ITreeNodeObject {
static fromJSON(obj: Jsonify<CollectionView>) {
return Object.assign(new CollectionView(new Collection()), obj);
}
get isDefaultCollection() {
return this.type == CollectionTypes.DefaultUserCollection;
}
}

2
libs/angular/src/vault/services/custom-nudges-services/empty-vault-nudge.service.ts
View File

@@ -42,7 +42,7 @@ export class EmptyVaultNudgeService extends DefaultSingleNudgeService {
const orgIds = new Set(orgs.map((org) => org.id));
const canCreateCollections = orgs.some((org) => org.canCreateNewCollections);
const hasManageCollections = collections.some(
(c) => c.manage && orgIds.has(c.organizationId),
(c) => c.manage && orgIds.has(c.organizationId!),
);
// When the user has dismissed the nudge or spotlight, return the nudge status directly

2
libs/angular/src/vault/services/custom-nudges-services/vault-settings-import-nudge.service.ts
View File

@@ -46,7 +46,7 @@ export class VaultSettingsImportNudgeService extends DefaultSingleNudgeService {
const orgIds = new Set(orgs.map((org) => org.id));
const canCreateCollections = orgs.some((org) => org.canCreateNewCollections);
const hasManageCollections = collections.some(
(c) => c.manage && orgIds.has(c.organizationId),
(c) => c.manage && orgIds.has(c.organizationId!),
);
// When the user has dismissed the nudge or spotlight, return the nudge status directly

3
libs/angular/src/vault/vault-filter/services/vault-filter.service.ts
View File

@@ -191,6 +191,9 @@ export function sortDefaultCollections(
.sort((a, b) => {
const aName = orgs.find((o) => o.id === a.organizationId)?.name ?? a.organizationId;
const bName = orgs.find((o) => o.id === b.organizationId)?.name ?? b.organizationId;
if (!aName || !bName) {
throw new Error("Collection does not have an organizationId.");
}
return collator.compare(aName, bName);
});
return [

4
libs/common/src/vault/models/domain/tree-node.ts
View File

@@ -16,6 +16,6 @@ export class TreeNode<T extends ITreeNodeObject> {
}
export interface ITreeNodeObject {
id: string;
name: string;
id: string | undefined;
name: string | undefined;
}

2
libs/importer/src/importers/base-importer.ts
View File

@@ -279,7 +279,7 @@ export abstract class BaseImporter {
result.collections = result.folders.map((f) => {
const collection = new CollectionView();
collection.name = f.name;
collection.id = f.id;
collection.id = f.id ?? undefined; // folder id may be null, which is not suitable for collections.
return collection;
});
result.folderRelationships = [];

3
libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts
View File

@@ -31,7 +31,7 @@ const createMockCollection = (
organizationId: string,
readOnly = false,
canEdit = true,
) => {
): CollectionView => {
return {
id,
name,
@@ -42,6 +42,7 @@ const createMockCollection = (
manage: true,
assigned: true,
type: CollectionTypes.DefaultUserCollection,
isDefaultCollection: true,
canEditItems: jest.fn().mockReturnValue(canEdit),
canEdit: jest.fn(),
canDelete: jest.fn(),