1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

[PM-25982] Assign to Collections - My Items (#16591)

* update cipher form to exclude my items collections

* handle default collections for assign to collections and bulk

* account for every returning true for empty arrays
This commit is contained in:
Nick Krantz
2025-09-29 13:06:36 -05:00
committed by GitHub
parent f988d3fd70
commit c4ee2fdae2
4 changed files with 199 additions and 8 deletions

View File

@@ -7,7 +7,7 @@ import { BehaviorSubject, of } from "rxjs";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports // eslint-disable-next-line no-restricted-imports
import { CollectionTypes, CollectionView } from "@bitwarden/admin-console/common"; import { CollectionType, CollectionTypes, CollectionView } from "@bitwarden/admin-console/common";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
@@ -33,6 +33,7 @@ const createMockCollection = (
organizationId: string, organizationId: string,
readOnly = false, readOnly = false,
canEdit = true, canEdit = true,
type: CollectionType = CollectionTypes.DefaultUserCollection,
): CollectionView => { ): CollectionView => {
const cv = new CollectionView({ const cv = new CollectionView({
name, name,
@@ -41,7 +42,7 @@ const createMockCollection = (
}); });
cv.readOnly = readOnly; cv.readOnly = readOnly;
cv.manage = true; cv.manage = true;
cv.type = CollectionTypes.DefaultUserCollection; cv.type = type;
cv.externalId = ""; cv.externalId = "";
cv.hidePasswords = false; cv.hidePasswords = false;
cv.assigned = true; cv.assigned = true;
@@ -519,6 +520,42 @@ describe("ItemDetailsSectionComponent", () => {
expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]); expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]);
}); });
it("should exclude default collections when the cipher is only assigned to shared collections", async () => {
component.config.admin = false;
component.config.organizationDataOwnershipDisabled = true;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = new Array(4)
.fill(null)
.map((_, i) => i + 1)
.map(
(i) =>
createMockCollection(
`col${i}`,
`Collection ${i}`,
"org1",
false,
false,
i < 4 ? CollectionTypes.SharedCollection : CollectionTypes.DefaultUserCollection,
) as CollectionView,
);
component.originalCipherView = {
name: "cipher1",
organizationId: "org1",
folderId: "folder1",
collectionIds: ["col2", "col3"],
favorite: true,
} as CipherView;
fixture.detectChanges();
await fixture.whenStable();
component.itemDetailsForm.controls.organizationId.setValue("org1");
fixture.detectChanges();
await fixture.whenStable();
expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]);
});
}); });
describe("readonlyCollections", () => { describe("readonlyCollections", () => {

View File

@@ -406,6 +406,17 @@ export class ItemDetailsSectionComponent implements OnInit {
this.showCollectionsControl = true; this.showCollectionsControl = true;
} }
/**
* Determine if the the cipher is only assigned to shared collections.
* i.e. The cipher is not assigned to a default collections.
* Note: `.every` will return true for an empty array
*/
const cipherIsOnlyInOrgCollections =
(this.originalCipherView?.collectionIds ?? []).length > 0 &&
this.originalCipherView.collectionIds.every(
(cId) =>
this.collections.find((c) => c.id === cId)?.type === CollectionTypes.SharedCollection,
);
this.collectionOptions = this.collections this.collectionOptions = this.collections
.filter((c) => { .filter((c) => {
// The collection belongs to the organization // The collection belongs to the organization
@@ -423,10 +434,17 @@ export class ItemDetailsSectionComponent implements OnInit {
return true; return true;
} }
// When the cipher is only assigned to shared collections, do not allow a user to
// move it back to a default collection. Exclude the default collection from the list.
if (cipherIsOnlyInOrgCollections && c.type === CollectionTypes.DefaultUserCollection) {
return false;
}
// Non-admins can only select assigned collections that are not read only. (Non-AC) // Non-admins can only select assigned collections that are not read only. (Non-AC)
return c.assigned && !c.readOnly; return c.assigned && !c.readOnly;
}) })
.sort((a, b) => { .sort((a, b) => {
// Show default collection first
const aIsDefaultCollection = a.type === CollectionTypes.DefaultUserCollection ? -1 : 0; const aIsDefaultCollection = a.type === CollectionTypes.DefaultUserCollection ? -1 : 0;
const bIsDefaultCollection = b.type === CollectionTypes.DefaultUserCollection ? -1 : 0; const bIsDefaultCollection = b.type === CollectionTypes.DefaultUserCollection ? -1 : 0;
return aIsDefaultCollection - bIsDefaultCollection; return aIsDefaultCollection - bIsDefaultCollection;

View File

@@ -5,7 +5,11 @@ import { of } from "rxjs";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports // eslint-disable-next-line no-restricted-imports
import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import {
CollectionService,
CollectionTypes,
CollectionView,
} from "@bitwarden/admin-console/common";
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";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -34,7 +38,6 @@ describe("AssignCollectionsComponent", () => {
organizationId: "org-id" as OrganizationId, organizationId: "org-id" as OrganizationId,
name: "Editable Collection", name: "Editable Collection",
}); });
editCollection.readOnly = false; editCollection.readOnly = false;
editCollection.manage = true; editCollection.manage = true;
@@ -52,6 +55,24 @@ describe("AssignCollectionsComponent", () => {
}); });
readOnlyCollection2.readOnly = true; readOnlyCollection2.readOnly = true;
const sharedCollection = new CollectionView({
id: "shared-collection-id" as CollectionId,
organizationId: "org-id" as OrganizationId,
name: "Shared Collection",
});
sharedCollection.readOnly = false;
sharedCollection.assigned = true;
sharedCollection.type = CollectionTypes.SharedCollection;
const defaultCollection = new CollectionView({
id: "default-collection-id" as CollectionId,
organizationId: "org-id" as OrganizationId,
name: "Default Collection",
});
defaultCollection.readOnly = false;
defaultCollection.manage = true;
defaultCollection.type = CollectionTypes.DefaultUserCollection;
const params = { const params = {
organizationId: "org-id" as OrganizationId, organizationId: "org-id" as OrganizationId,
ciphers: [ ciphers: [
@@ -116,4 +137,75 @@ describe("AssignCollectionsComponent", () => {
]); ]);
}); });
}); });
describe("default collections", () => {
const cipher1 = new CipherView();
cipher1.id = "cipher-id-1";
cipher1.collectionIds = [editCollection.id, sharedCollection.id];
cipher1.edit = true;
const cipher2 = new CipherView();
cipher2.id = "cipher-id-2";
cipher2.collectionIds = [defaultCollection.id];
cipher2.edit = true;
const cipher3 = new CipherView();
cipher3.id = "cipher-id-3";
cipher3.collectionIds = [defaultCollection.id];
cipher3.edit = true;
const cipher4 = new CipherView();
cipher4.id = "cipher-id-4";
cipher4.collectionIds = [];
cipher4.edit = true;
it('does not show the "Default Collection" if any cipher is in a shared collection', async () => {
component.params = {
...component.params,
ciphers: [cipher1, cipher2],
availableCollections: [editCollection, sharedCollection, defaultCollection],
};
await component.ngOnInit();
fixture.detectChanges();
expect(component["availableCollections"].map((c) => c.id)).toEqual([
editCollection.id,
sharedCollection.id,
]);
});
it('shows the "Default Collection" if no ciphers are in a shared collection', async () => {
component.params = {
...component.params,
ciphers: [cipher2, cipher3],
availableCollections: [editCollection, sharedCollection, defaultCollection],
};
await component.ngOnInit();
fixture.detectChanges();
expect(component["availableCollections"].map((c) => c.id)).toEqual([
editCollection.id,
sharedCollection.id,
defaultCollection.id,
]);
});
it('shows the "Default Collection" for singular cipher', async () => {
component.params = {
...component.params,
ciphers: [cipher4],
availableCollections: [readOnlyCollection1, sharedCollection, defaultCollection],
};
await component.ngOnInit();
fixture.detectChanges();
expect(component["availableCollections"].map((c) => c.id)).toEqual([
sharedCollection.id,
defaultCollection.id,
]);
});
});
}); });

View File

@@ -26,7 +26,11 @@ import {
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports // eslint-disable-next-line no-restricted-imports
import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import {
CollectionService,
CollectionTypes,
CollectionView,
} from "@bitwarden/admin-console/common";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { import {
getOrganizationById, getOrganizationById,
@@ -311,9 +315,19 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
await this.setReadOnlyCollectionNames(); await this.setReadOnlyCollectionNames();
const canAccessDefaultCollection = this.canAccessDefaultCollection(
this.params.availableCollections,
);
this.availableCollections = this.params.availableCollections this.availableCollections = this.params.availableCollections
.filter((collection) => { .filter((collection) => {
return collection.canEditItems(org); if (canAccessDefaultCollection) {
return collection.canEditItems(org);
}
return (
collection.canEditItems(org) && collection.type !== CollectionTypes.DefaultUserCollection
);
}) })
.map((c) => ({ .map((c) => ({
icon: "bwi-collection-shared", icon: "bwi-collection-shared",
@@ -447,8 +461,16 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
const org = organizations.find((o) => o.id === orgId); const org = organizations.find((o) => o.id === orgId);
this.orgName = org.name; this.orgName = org.name;
return collections.filter((c) => { const orgCollections = collections.filter((c) => c.organizationId === orgId);
return c.organizationId === orgId && !c.readOnly;
const canAccessDefaultCollection = this.canAccessDefaultCollection(collections);
return orgCollections.filter((c) => {
if (canAccessDefaultCollection) {
return !c.readOnly;
}
return !c.readOnly && c.type !== CollectionTypes.DefaultUserCollection;
}); });
}), }),
shareReplay({ refCount: true, bufferSize: 1 }), shareReplay({ refCount: true, bufferSize: 1 }),
@@ -536,4 +558,26 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
}) })
.map((c) => c.name); .map((c) => c.name);
} }
/**
* Determines if the ciphers to be assigned can be assigned to the Default Collection.
* When false, the Default Collections should be excluded from the list of available collections.
*/
private canAccessDefaultCollection(collections: CollectionView[]): boolean {
const collectionsObject = Object.fromEntries(collections.map((c) => [c.id, c]));
const allCiphersUnassignedOrInDefault = this.params.ciphers.every(
(cipher) =>
!cipher.collectionIds.length ||
cipher.collectionIds.some(
(cId) => collectionsObject[cId]?.type === CollectionTypes.DefaultUserCollection,
),
);
// When all ciphers are either:
// - unassigned
// - already in a Default Collection
// then the Default Collection can be shown.
return allCiphersUnassignedOrInDefault;
}
} }