1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 00:03:56 +00:00

[PM-12281] [PM-12301] [PM-12306] [PM-12334] Move delete item permission to Can Manage (#11289)

* Added inputs to the view and edit component to disable or remove the delete button when a user does not have manage rights

* Refactored editByCipherId to receive cipherview object

* Fixed issue where adding an item on the individual vault throws a null reference

* Fixed issue where adding an item on the AC vault throws a null reference

* Allow delete in unassigned collection

* created reusable service to check if a user has delete permission on an item

* Registered service

* Used authorizationservice on the browser and desktop

Only display the delete button when a user has delete permission

* Added comments to the service

* Passed active collectionId to add edit component

renamed constructor parameter

* restored input property used by the web

* Fixed dependency issue

* Fixed dependency issue

* Fixed dependency issue

* Modified service to cater for org vault

* Updated to include new dependency

* Updated components to use the observable

* Added check on the cli to know if user has rights to delete an item

* Renamed abstraction and renamed implementation to include Default

Fixed permission issues

* Fixed test to reflect changes in implementation

* Modified base classes to use new naming

Passed new parameters for the canDeleteCipher

* Modified base classes to use new naming

Made changes from base class

* Desktop changes

Updated reference naming

* cli changes

Updated reference naming

Passed new parameters for the canDeleteCipher$

* Updated references

* browser changes

Updated reference naming

Passed new parameters for the canDeleteCipher$

* Modified cipher form dialog to take in active collection id

used canDeleteCipher$ on the vault item dialog to disable the delete button when user does not have the required permissions

* Fix number of arguments issue

* Added active collection id

* Updated canDeleteCipher$ arguments

* Updated to pass the cipher object

* Fixed up refrences and comments

* Updated dependency

* updated check to canEditUnassignedCiphers

* Fixed unit tests

* Removed activeCollectionId from cipher form

* Fixed issue where bulk delete option shows for can edit users

* Fix null reference when checking if a cipher belongs to the unassigned collection

* Fixed bug where allowedCollection passed is undefined

* Modified cipher by adding a isAdminConsoleAction argument to tell when a reuqest comes from the admin console

* Passed isAdminConsoleAction as true when request is from the admin console
This commit is contained in:
SmithThe4th
2024-10-22 15:15:15 +02:00
committed by GitHub
parent 470ddf79ab
commit 4a30782939
39 changed files with 551 additions and 58 deletions

View File

@@ -0,0 +1,200 @@
import { mock } from "jest-mock-extended";
import { of } from "rxjs";
import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { CollectionId } from "@bitwarden/common/types/guid";
import { CipherView } from "../models/view/cipher.view";
import {
CipherAuthorizationService,
DefaultCipherAuthorizationService,
} from "./cipher-authorization.service";
describe("CipherAuthorizationService", () => {
let cipherAuthorizationService: CipherAuthorizationService;
const mockCollectionService = mock<CollectionService>();
const mockOrganizationService = mock<OrganizationService>();
// Mock factories
const createMockCipher = (
organizationId: string | null,
collectionIds: string[],
edit: boolean = true,
) => ({
organizationId,
collectionIds,
edit,
});
const createMockCollection = (id: string, manage: boolean) => ({
id,
manage,
});
const createMockOrganization = ({
allowAdminAccessToAllCollectionItems = false,
canEditAllCiphers = false,
canEditUnassignedCiphers = false,
} = {}) => ({
allowAdminAccessToAllCollectionItems,
canEditAllCiphers,
canEditUnassignedCiphers,
});
beforeEach(() => {
jest.clearAllMocks();
cipherAuthorizationService = new DefaultCipherAuthorizationService(
mockCollectionService,
mockOrganizationService,
);
});
describe("canDeleteCipher$", () => {
it("should return true if cipher has no organizationId", (done) => {
const cipher = createMockCipher(null, []) as CipherView;
cipherAuthorizationService.canDeleteCipher$(cipher).subscribe((result) => {
expect(result).toBe(true);
done();
});
});
it("should return true if isAdminConsoleAction is true and cipher is unassigned", (done) => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization({ canEditUnassignedCiphers: true });
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(true);
done();
});
});
it("should return true if isAdminConsoleAction is true and user can edit all ciphers in the org", (done) => {
const cipher = createMockCipher("org1", ["col1"]) as CipherView;
const organization = createMockOrganization({ canEditAllCiphers: true });
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(true);
expect(mockOrganizationService.get$).toHaveBeenCalledWith("org1");
done();
});
});
it("should return false if isAdminConsoleAction is true but user does not have permission to edit unassigned ciphers", (done) => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization({ canEditUnassignedCiphers: false });
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(false);
done();
});
});
it("should return true if activeCollectionId is provided and has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const activeCollectionId = "col1" as CollectionId;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const allCollections = [
createMockCollection("col1", true),
createMockCollection("col2", false),
];
mockCollectionService.decryptedCollectionViews$.mockReturnValue(
of(allCollections as CollectionView[]),
);
cipherAuthorizationService
.canDeleteCipher$(cipher, [activeCollectionId])
.subscribe((result) => {
expect(result).toBe(true);
expect(mockCollectionService.decryptedCollectionViews$).toHaveBeenCalledWith([
"col1",
"col2",
] as CollectionId[]);
done();
});
});
it("should return false if activeCollectionId is provided and manage permission is not present", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const activeCollectionId = "col1" as CollectionId;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const allCollections = [
createMockCollection("col1", false),
createMockCollection("col2", true),
];
mockCollectionService.decryptedCollectionViews$.mockReturnValue(
of(allCollections as CollectionView[]),
);
cipherAuthorizationService
.canDeleteCipher$(cipher, [activeCollectionId])
.subscribe((result) => {
expect(result).toBe(false);
expect(mockCollectionService.decryptedCollectionViews$).toHaveBeenCalledWith([
"col1",
"col2",
] as CollectionId[]);
done();
});
});
it("should return true if any collection has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2", "col3"]) as CipherView;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const allCollections = [
createMockCollection("col1", false),
createMockCollection("col2", true),
createMockCollection("col3", false),
];
mockCollectionService.decryptedCollectionViews$.mockReturnValue(
of(allCollections as CollectionView[]),
);
cipherAuthorizationService.canDeleteCipher$(cipher).subscribe((result) => {
expect(result).toBe(true);
expect(mockCollectionService.decryptedCollectionViews$).toHaveBeenCalledWith([
"col1",
"col2",
"col3",
] as CollectionId[]);
done();
});
});
it("should return false if no collection has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const allCollections = [
createMockCollection("col1", false),
createMockCollection("col2", false),
];
mockCollectionService.decryptedCollectionViews$.mockReturnValue(
of(allCollections as CollectionView[]),
);
cipherAuthorizationService.canDeleteCipher$(cipher).subscribe((result) => {
expect(result).toBe(false);
expect(mockCollectionService.decryptedCollectionViews$).toHaveBeenCalledWith([
"col1",
"col2",
] as CollectionId[]);
done();
});
});
});
});

View File

@@ -0,0 +1,86 @@
import { map, Observable, of, switchMap } from "rxjs";
import { CollectionService } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { CollectionId } from "@bitwarden/common/types/guid";
import { Cipher } from "../models/domain/cipher";
import { CipherView } from "../models/view/cipher.view";
/**
* Represents either a cipher or a cipher view.
*/
type CipherLike = Cipher | CipherView;
/**
* Service for managing user cipher authorization.
*/
export abstract class CipherAuthorizationService {
/**
* Determines if the user can delete the specified cipher.
*
* @param {CipherLike} cipher - The cipher object to evaluate for deletion permissions.
* @param {CollectionId[]} [allowedCollections] - Optional. The selected collection id from the vault filter.
* @param {boolean} isAdminConsoleAction - Optional. A flag indicating if the action is being performed from the admin console.
*
* @returns {Observable<boolean>} - An observable that emits a boolean value indicating if the user can delete the cipher.
*/
canDeleteCipher$: (
cipher: CipherLike,
allowedCollections?: CollectionId[],
isAdminConsoleAction?: boolean,
) => Observable<boolean>;
}
/**
* {@link CipherAuthorizationService}
*/
export class DefaultCipherAuthorizationService implements CipherAuthorizationService {
constructor(
private collectionService: CollectionService,
private organizationService: OrganizationService,
) {}
/**
*
* {@link CipherAuthorizationService.canDeleteCipher$}
*/
canDeleteCipher$(
cipher: CipherLike,
allowedCollections?: CollectionId[],
isAdminConsoleAction?: boolean,
): Observable<boolean> {
if (cipher.organizationId == null) {
return of(true);
}
return this.organizationService.get$(cipher.organizationId).pipe(
switchMap((organization) => {
if (isAdminConsoleAction) {
// If the user is an admin, they can delete an unassigned cipher
if (!cipher.collectionIds || cipher.collectionIds.length === 0) {
return of(organization?.canEditUnassignedCiphers === true);
}
if (organization?.canEditAllCiphers) {
return of(true);
}
}
return this.collectionService
.decryptedCollectionViews$(cipher.collectionIds as CollectionId[])
.pipe(
map((allCollections) => {
const shouldFilter = allowedCollections?.some(Boolean);
const collections = shouldFilter
? allCollections.filter((c) => allowedCollections.includes(c.id as CollectionId))
: allCollections;
return collections.some((collection) => collection.manage);
}),
);
}),
);
}
}