Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-21 16:18:28 +01:00
Code Issues Projects Releases Wiki Activity

[PM-13839][PM-13840] Admin Console Collections (#11649)

Browse Source 
* allow admin console to see all collections when viewing a cipher

- When "manage all" option is selected all collections should be editable

* update cipher form service to use admin endpoints

* when saving a cipher, choose to move to collections first before saving any other edits

- This handles the case where a cipher is moving from unassigned to assigned and needs to have a collection to save any other edits

* set admin flag when the original cipher has zero collections

- handling the case where the user  un-assigns themselves from a cipher

* add check for the users ability to edit items within the collection

* save cipher edit first to handle when the user unassigns themselves from the cipher

* update filter order of collections

* use cipher returned from the collections endpoint rather than re-fetching it

* fix unit tests by adding canEditItems

* re-enable collection control when orgId is present

* fetch the updated cipher from the respective service for editing a cipher
This commit is contained in:
Nick Krantz 2024-11-07 10:22:35 -06:00 committed by GitHub
parent 05a79d58bb
commit b42741f313
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 240 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts
View File

@ -6,6 +6,7 @@ import { firstValueFrom, Observable, Subject } from "rxjs";
import { map } from "rxjs/operators";
import { CollectionView } from "@bitwarden/admin-console/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions";
@ -17,6 +18,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service";
import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions/view-password-history.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
import {
@ -231,6 +234,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy {
private billingAccountProfileStateService: BillingAccountProfileStateService,
private premiumUpgradeService: PremiumUpgradePromptService,
private cipherAuthorizationService: CipherAuthorizationService,
private apiService: ApiService,
) {
this.updateTitle();
}
@ -278,7 +282,20 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy {
if (this._originalFormMode === "add" || this._originalFormMode === "clone") {
this.formConfig.mode = "edit";
}
this.formConfig.originalCipher = await this.cipherService.get(cipherView.id);
let cipher: Cipher;
// When the form config is used within the Admin Console, retrieve the cipher from the admin endpoint
if (this.formConfig.isAdminConsole) {
const cipherResponse = await this.apiService.getCipherAdmin(cipherView.id);
const cipherData = new CipherData(cipherResponse);
cipher = new Cipher(cipherData);
} else {
cipher = await this.cipherService.get(cipherView.id);
}
// Store the updated cipher so any following edits use the most up to date cipher
this.formConfig.originalCipher = cipher;
this._cipherModified = true;
await this.changeMode("view");
}

64
apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts
View File

@ -1,14 +1,13 @@
import { TestBed } from "@angular/core/testing";
import { BehaviorSubject } from "rxjs";
import { CollectionAdminService } from "@bitwarden/admin-console/common";
import { CollectionAdminService, CollectionAdminView } from "@bitwarden/admin-console/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { CipherId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/services/routed-vault-filter.service";
@ -35,27 +34,41 @@ describe("AdminConsoleCipherFormConfigService", () => {
status: OrganizationUserStatusType.Confirmed,
};
const policyAppliesToActiveUser$ = new BehaviorSubject<boolean>(true);
const collection = {
id: "12345-5555",
organizationId: "234534-34334",
name: "Test Collection 1",
assigned: false,
readOnly: true,
} as CollectionAdminView;
const collection2 = {
id: "12345-6666",
organizationId: "22222-2222",
name: "Test Collection 2",
assigned: true,
readOnly: false,
} as CollectionAdminView;
const organization$ = new BehaviorSubject<Organization>(testOrg as Organization);
const organizations$ = new BehaviorSubject<Organization[]>([testOrg, testOrg2] as Organization[]);
const getCipherAdmin = jest.fn().mockResolvedValue(null);
const getCipher = jest.fn().mockResolvedValue(null);
beforeEach(async () => {
getCipherAdmin.mockClear();
getCipher.mockClear();
getCipher.mockResolvedValue({ id: cipherId, name: "Test Cipher - (non-admin)" });
getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
await TestBed.configureTestingModule({
providers: [
AdminConsoleCipherFormConfigService,
{ provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } },
{
provide: CollectionAdminService,
useValue: { getAll: () => Promise.resolve([collection, collection2]) },
},
{
provide: PolicyService,
useValue: { policyAppliesToActiveUser$: () => policyAppliesToActiveUser$ },
},
{ provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } },
{ provide: CipherService, useValue: { get: getCipher } },
{ provide: CollectionAdminService, useValue: { getAll: () => Promise.resolve([]) } },
{
provide: RoutedVaultFilterService,
useValue: { filter$: new BehaviorSubject({ organizationId: testOrg.id }) },
@ -86,6 +99,12 @@ describe("AdminConsoleCipherFormConfigService", () => {
expect(mode).toBe("edit");
});
it("returns all collections", async () => {
const { collections } = await adminConsoleConfigService.buildConfig("edit", cipherId);
expect(collections).toEqual([collection, collection2]);
});
it("sets admin flag based on `canEditAllCiphers`", async () => {
// Disable edit all ciphers on org
testOrg.canEditAllCiphers = false;
@ -153,33 +172,14 @@ describe("AdminConsoleCipherFormConfigService", () => {
expect(result.organizations).toEqual([testOrg, testOrg2]);
});
describe("getCipher", () => {
it("retrieves the cipher from the cipher service", async () => {
testOrg.canEditAllCiphers = false;
it("retrieves the cipher from the admin service", async () => {
getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
const result = await adminConsoleConfigService.buildConfig("clone", cipherId);
await adminConsoleConfigService.buildConfig("add", cipherId);
expect(getCipher).toHaveBeenCalledWith(cipherId);
expect(result.originalCipher.name).toBe("Test Cipher - (non-admin)");
// Admin service not needed when cipher service can return the cipher
expect(getCipherAdmin).not.toHaveBeenCalled();
});
it("retrieves the cipher from the admin service", async () => {
getCipher.mockResolvedValueOnce(null);
getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" });
adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService);
await adminConsoleConfigService.buildConfig("add", cipherId);
expect(getCipherAdmin).toHaveBeenCalledWith(cipherId);
expect(getCipher).toHaveBeenCalledWith(cipherId);
});
expect(getCipherAdmin).toHaveBeenCalledWith(cipherId);
});
});
});

39
apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts
View File

@ -6,9 +6,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType, OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { CipherId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
@ -25,7 +23,6 @@ import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/se
export class AdminConsoleCipherFormConfigService implements CipherFormConfigService {
private policyService: PolicyService = inject(PolicyService);
private organizationService: OrganizationService = inject(OrganizationService);
private cipherService: CipherService = inject(CipherService);
private routedVaultFilterService: RoutedVaultFilterService = inject(RoutedVaultFilterService);
private collectionAdminService: CollectionAdminService = inject(CollectionAdminService);
private apiService: ApiService = inject(ApiService);
@ -51,20 +48,8 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
map(([orgs, orgId]) => orgs.find((o) => o.id === orgId)),
);
private editableCollections$ = this.organization$.pipe(
switchMap(async (org) => {
if (!org) {
return [];
}
const collections = await this.collectionAdminService.getAll(org.id);
// Users that can edit all ciphers can implicitly add to / edit within any collection
if (org.canEditAllCiphers) {
return collections;
}
// The user is only allowed to add/edit items to assigned collections that are not readonly
return collections.filter((c) => c.assigned && !c.readOnly);
}),
private allCollections$ = this.organization$.pipe(
switchMap(async (org) => await this.collectionAdminService.getAll(org.id)),
);
async buildConfig(
@ -72,21 +57,17 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
cipherId?: CipherId,
cipherType?: CipherType,
): Promise<CipherFormConfig> {
const cipher = await this.getCipher(cipherId);
const [organization, allowPersonalOwnership, allOrganizations, allCollections] =
await firstValueFrom(
combineLatest([
this.organization$,
this.allowPersonalOwnership$,
this.allOrganizations$,
this.editableCollections$,
this.allCollections$,
]),
);
const cipher = await this.getCipher(organization, cipherId);
const collections = allCollections.filter(
(c) => c.organizationId === organization.id && c.assigned && !c.readOnly,
);
// When cloning from within the Admin Console, all organizations should be available.
// Otherwise only the one in context should be
const organizations = mode === "clone" ? allOrganizations : [organization];
@ -100,7 +81,7 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
admin: organization.canEditAllCiphers ?? false,
allowPersonalOwnership: allowPersonalOwnershipOnlyForClone,
originalCipher: cipher,
collections,
collections: allCollections,
organizations,
folders: [], // folders not applicable in the admin console
hideIndividualVaultFields: true,
@ -108,19 +89,11 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ
};
}
private async getCipher(organization: Organization, id?: CipherId): Promise<Cipher | null> {
private async getCipher(id?: CipherId): Promise<Cipher | null> {
if (id == null) {
return Promise.resolve(null);
}
// Check to see if the user has direct access to the cipher
const cipherFromCipherService = await this.cipherService.get(id);
// If the organization doesn't allow admin/owners to edit all ciphers return the cipher
if (!organization.canEditAllCiphers && cipherFromCipherService != null) {
return cipherFromCipherService;
}
// Retrieve the cipher through the means of an admin
const cipherResponse = await this.apiService.getCipherAdmin(id);
cipherResponse.edit = true;

2
libs/common/src/services/api.service.ts
View File

@ -584,7 +584,7 @@ export class ApiService implements ApiServiceAbstraction {
}
putCipherCollectionsAdmin(id: string, request: CipherCollectionsRequest): Promise<any> {
return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, false);
return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, true);
}
postPurgeCiphers(

2
libs/common/src/vault/abstractions/cipher.service.ts
View File

@ -119,7 +119,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider<Ciphe
* Used for Unassigned ciphers or when the user only has admin access to the cipher (not assigned normally).
* @param cipher
*/
saveCollectionsWithServerAdmin: (cipher: Cipher) => Promise<void>;
saveCollectionsWithServerAdmin: (cipher: Cipher) => Promise<Cipher>;
/**
* Bulk update collections for many ciphers with the server
* @param orgId

6
libs/common/src/vault/services/cipher.service.ts
View File

@ -880,9 +880,11 @@ export class CipherService implements CipherServiceAbstraction {
return new Cipher(updated[cipher.id as CipherId], cipher.localData);
}
async saveCollectionsWithServerAdmin(cipher: Cipher): Promise<void> {
async saveCollectionsWithServerAdmin(cipher: Cipher): Promise<Cipher> {
const request = new CipherCollectionsRequest(cipher.collectionIds);
await this.apiService.putCipherCollectionsAdmin(cipher.id, request);
const response = await this.apiService.putCipherCollectionsAdmin(cipher.id, request);
const data = new CipherData(response);
return new Cipher(data);
}
/**

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

@ -87,7 +87,12 @@ describe("ItemDetailsSectionComponent", () => {
component.config.allowPersonalOwnership = true;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{ id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView,
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
];
component.originalCipherView = {
name: "cipher1",
@ -116,8 +121,18 @@ describe("ItemDetailsSectionComponent", () => {
component.config.allowPersonalOwnership = true;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{ id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView,
{ id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView,
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
canEditItems: (_org) => false,
} as CollectionView,
{
id: "col2",
name: "Collection 2",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
];
component.originalCipherView = {
name: "cipher1",
@ -367,9 +382,24 @@ describe("ItemDetailsSectionComponent", () => {
} as CipherView;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{ id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView,
{ id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView,
{ id: "col3", name: "Collection 3", organizationId: "org1" } as CollectionView,
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
{
id: "col2",
name: "Collection 2",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
{
id: "col3",
name: "Collection 3",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
];
fixture.detectChanges();
@ -387,7 +417,12 @@ describe("ItemDetailsSectionComponent", () => {
component.config.allowPersonalOwnership = true;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{ id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView,
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
];
fixture.detectChanges();
@ -414,13 +449,24 @@ describe("ItemDetailsSectionComponent", () => {
} as CipherView;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{ id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView,
{ id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView,
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
{
id: "col2",
name: "Collection 2",
organizationId: "org1",
canEditItems: (_org) => true,
} as CollectionView,
{
id: "col3",
name: "Collection 3",
organizationId: "org1",
readOnly: true,
canEditItems: (_org) => true,
} as CollectionView,
];
@ -433,5 +479,94 @@ describe("ItemDetailsSectionComponent", () => {
expect(collectionHint).not.toBeNull();
});
it("should allow all collections to be altered when `config.admin` is true", async () => {
component.config.admin = true;
component.config.allowPersonalOwnership = true;
component.config.organizations = [{ id: "org1" } as Organization];
component.config.collections = [
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
readOnly: true,
canEditItems: (_org) => false,
} as CollectionView,
{
id: "col2",
name: "Collection 2",
organizationId: "org1",
readOnly: true,
canEditItems: (_org) => false,
} as CollectionView,
{
id: "col3",
name: "Collection 3",
organizationId: "org1",
readOnly: false,
canEditItems: (_org) => false,
} as CollectionView,
];
fixture.detectChanges();
await fixture.whenStable();
component.itemDetailsForm.controls.organizationId.setValue("org1");
expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]);
});
});
describe("readonlyCollections", () => {
beforeEach(() => {
component.config.mode = "edit";
component.config.admin = true;
component.config.collections = [
{
id: "col1",
name: "Collection 1",
organizationId: "org1",
readOnly: true,
canEditItems: (_org) => false,
} as CollectionView,
{
id: "col2",
name: "Collection 2",
organizationId: "org1",
canEditItems: (_org) => false,
} as CollectionView,
{
id: "col3",
name: "Collection 3",
organizationId: "org1",
readOnly: true,
canEditItems: (_org) => false,
} as CollectionView,
];
component.originalCipherView = {
name: "cipher1",
organizationId: "org1",
folderId: "folder1",
collectionIds: ["col1", "col2", "col3"],
favorite: true,
} as CipherView;
component.config.organizations = [{ id: "org1" } as Organization];
});
it("should not show collections as readonly when `config.admin` is true", async () => {
await component.ngOnInit();
fixture.detectChanges();
// Filters out all collections
expect(component["readOnlyCollections"]).toEqual([]);
// Non-admin, keep readonly collections
component.config.admin = false;
await component.ngOnInit();
fixture.detectChanges();
expect(component["readOnlyCollections"]).toEqual(["Collection 1", "Collection 3"]);
});
});
});

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

@ -240,7 +240,11 @@ export class ItemDetailsSectionComponent implements OnInit {
} else if (this.config.mode === "edit") {
this.readOnlyCollections = this.collections
.filter(
(c) => c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId),
// When the configuration is set up for admins, they can alter read only collections
(c) =>
c.readOnly &&
!this.config.admin &&
this.originalCipherView.collectionIds.includes(c.id as CollectionId),
)
.map((c) => c.name);
}
@ -262,12 +266,24 @@ export class ItemDetailsSectionComponent implements OnInit {
collectionsControl.disable();
this.showCollectionsControl = false;
return;
} else {
collectionsControl.enable();
this.showCollectionsControl = true;
}
const organization = this.organizations.find((o) => o.id === orgId);
this.collectionOptions = this.collections
.filter((c) => {
// If partial edit mode, show all org collections because the control is disabled.
return c.organizationId === orgId && (this.partialEdit || !c.readOnly);
// Filter criteria:
// - The collection belongs to the organization
// - When in partial edit mode, show all org collections because the control is disabled.
// - The user can edit items within the collection
// - When viewing as an admin, all collections should be shown, even readonly. When non-admin, filter out readonly collections
return (
c.organizationId === orgId &&
(this.partialEdit || c.canEditItems(organization) || this.config.admin)
);
})
.map((c) => ({
id: c.id,

16
libs/vault/src/cipher-form/services/default-cipher-form.service.ts
View File

@ -1,6 +1,7 @@
import { inject, Injectable } from "@angular/core";
import { firstValueFrom, map } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
@ -17,6 +18,7 @@ function isSetEqual(a: Set<string>, b: Set<string>) {
export class DefaultCipherFormService implements CipherFormService {
private cipherService: CipherService = inject(CipherService);
private accountService: AccountService = inject(AccountService);
private apiService: ApiService = inject(ApiService);
async decryptCipher(cipher: Cipher): Promise<CipherView> {
const activeUserId = await firstValueFrom(
@ -66,11 +68,21 @@ export class DefaultCipherFormService implements CipherFormService {
// Updating a cipher with collection changes is not supported with a single request currently
// First update the cipher with the original collectionIds
encryptedCipher.collectionIds = config.originalCipher.collectionIds;
await this.cipherService.updateWithServer(encryptedCipher, config.admin);
await this.cipherService.updateWithServer(
encryptedCipher,
config.admin || originalCollectionIds.size === 0,
config.mode !== "clone",
);
// Then save the new collection changes separately
encryptedCipher.collectionIds = cipher.collectionIds;
savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher);
if (config.admin || originalCollectionIds.size === 0) {
// When using an admin config or the cipher was unassigned, update collections as an admin
savedCipher = await this.cipherService.saveCollectionsWithServerAdmin(encryptedCipher);
} else {
savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher);
}
}
// Its possible the cipher was made no longer available due to collection assignment changes

1
libs/vault/src/cipher-view/cipher-view.component.ts
View File

@ -98,6 +98,7 @@ export class CipherViewComponent implements OnChanges, OnDestroy {
async loadCipherData() {
// Load collections if not provided and the cipher has collectionIds
if (
this.cipher.collectionIds &&
this.cipher.collectionIds.length > 0 &&
(!this.collections || this.collections.length === 0)
) {