1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 07:43:35 +00:00

[PM-18089] Update cipher permissions model and consumers (#13606)

* update cipher permissions model and consumers

* add new property to tests

* fix test, add property to toCipherData()

* add missing ConfigService

* fix story

* refactor

* fix error, cleanup

* revert refactor

* refactor

* remove uneeded test

* cleanup

* fix build error

* refactor

* clean up

* add tests

* move validation check to after featrue flagged logic

* iterate on feedback

* feedback
This commit is contained in:
Brandon Treston
2025-03-14 09:51:40 -04:00
committed by GitHub
parent b73e6cf2fe
commit 4d68952ef3
18 changed files with 372 additions and 23 deletions

View File

@@ -0,0 +1,21 @@
import { Jsonify } from "type-fest";
import { BaseResponse } from "../../../models/response/base.response";
export class CipherPermissionsApi extends BaseResponse {
delete: boolean = false;
restore: boolean = false;
constructor(data: any = null) {
super(data);
if (data == null) {
return;
}
this.delete = this.getResponseProperty("Delete");
this.restore = this.getResponseProperty("Restore");
}
static fromJSON(obj: Jsonify<CipherPermissionsApi>) {
return Object.assign(new CipherPermissionsApi(), obj);
}
}

View File

@@ -4,6 +4,7 @@ import { Jsonify } from "type-fest";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherType } from "../../enums/cipher-type";
import { CipherPermissionsApi } from "../api/cipher-permissions.api";
import { CipherResponse } from "../response/cipher.response";
import { AttachmentData } from "./attachment.data";
@@ -21,6 +22,7 @@ export class CipherData {
folderId: string;
edit: boolean;
viewPassword: boolean;
permissions: CipherPermissionsApi;
organizationUseTotp: boolean;
favorite: boolean;
revisionDate: string;
@@ -51,6 +53,7 @@ export class CipherData {
this.folderId = response.folderId;
this.edit = response.edit;
this.viewPassword = response.viewPassword;
this.permissions = response.permissions;
this.organizationUseTotp = response.organizationUseTotp;
this.favorite = response.favorite;
this.revisionDate = response.revisionDate;
@@ -95,6 +98,8 @@ export class CipherData {
}
static fromJSON(obj: Jsonify<CipherData>) {
return Object.assign(new CipherData(), obj);
const result = Object.assign(new CipherData(), obj);
result.permissions = CipherPermissionsApi.fromJSON(obj.permissions);
return result;
}
}

View File

@@ -26,6 +26,7 @@ import { SecureNote } from "../../models/domain/secure-note";
import { CardView } from "../../models/view/card.view";
import { IdentityView } from "../../models/view/identity.view";
import { LoginView } from "../../models/view/login.view";
import { CipherPermissionsApi } from "../api/cipher-permissions.api";
describe("Cipher DTO", () => {
it("Convert from empty CipherData", () => {
@@ -54,6 +55,7 @@ describe("Cipher DTO", () => {
fields: null,
passwordHistory: null,
key: null,
permissions: undefined,
});
});
@@ -75,6 +77,7 @@ describe("Cipher DTO", () => {
notes: "EncryptedString",
creationDate: "2022-01-01T12:00:00.000Z",
deletedDate: null,
permissions: new CipherPermissionsApi(),
reprompt: CipherRepromptType.None,
key: "EncryptedString",
login: {
@@ -149,6 +152,7 @@ describe("Cipher DTO", () => {
localData: null,
creationDate: new Date("2022-01-01T12:00:00.000Z"),
deletedDate: null,
permissions: new CipherPermissionsApi(),
reprompt: 0,
key: { encryptedString: "EncryptedString", encryptionType: 0 },
login: {
@@ -228,6 +232,7 @@ describe("Cipher DTO", () => {
cipher.deletedDate = null;
cipher.reprompt = CipherRepromptType.None;
cipher.key = mockEnc("EncKey");
cipher.permissions = new CipherPermissionsApi();
const loginView = new LoginView();
loginView.username = "username";
@@ -270,6 +275,7 @@ describe("Cipher DTO", () => {
deletedDate: null,
reprompt: 0,
localData: undefined,
permissions: new CipherPermissionsApi(),
});
});
});
@@ -297,6 +303,7 @@ describe("Cipher DTO", () => {
secureNote: {
type: SecureNoteType.Generic,
},
permissions: new CipherPermissionsApi(),
};
});
@@ -326,6 +333,7 @@ describe("Cipher DTO", () => {
fields: null,
passwordHistory: null,
key: { encryptedString: "EncKey", encryptionType: 0 },
permissions: new CipherPermissionsApi(),
});
});
@@ -353,6 +361,7 @@ describe("Cipher DTO", () => {
cipher.secureNote = new SecureNote();
cipher.secureNote.type = SecureNoteType.Generic;
cipher.key = mockEnc("EncKey");
cipher.permissions = new CipherPermissionsApi();
const keyService = mock<KeyService>();
const encryptService = mock<EncryptService>();
@@ -387,6 +396,7 @@ describe("Cipher DTO", () => {
deletedDate: null,
reprompt: 0,
localData: undefined,
permissions: new CipherPermissionsApi(),
});
});
});
@@ -409,6 +419,7 @@ describe("Cipher DTO", () => {
notes: "EncryptedString",
creationDate: "2022-01-01T12:00:00.000Z",
deletedDate: null,
permissions: new CipherPermissionsApi(),
reprompt: CipherRepromptType.None,
card: {
cardholderName: "EncryptedString",
@@ -455,6 +466,7 @@ describe("Cipher DTO", () => {
fields: null,
passwordHistory: null,
key: { encryptedString: "EncKey", encryptionType: 0 },
permissions: new CipherPermissionsApi(),
});
});
@@ -480,6 +492,7 @@ describe("Cipher DTO", () => {
cipher.deletedDate = null;
cipher.reprompt = CipherRepromptType.None;
cipher.key = mockEnc("EncKey");
cipher.permissions = new CipherPermissionsApi();
const cardView = new CardView();
cardView.cardholderName = "cardholderName";
@@ -522,6 +535,7 @@ describe("Cipher DTO", () => {
deletedDate: null,
reprompt: 0,
localData: undefined,
permissions: new CipherPermissionsApi(),
});
});
});
@@ -544,6 +558,7 @@ describe("Cipher DTO", () => {
notes: "EncryptedString",
creationDate: "2022-01-01T12:00:00.000Z",
deletedDate: null,
permissions: new CipherPermissionsApi(),
reprompt: CipherRepromptType.None,
key: "EncKey",
identity: {
@@ -614,6 +629,7 @@ describe("Cipher DTO", () => {
fields: null,
passwordHistory: null,
key: { encryptedString: "EncKey", encryptionType: 0 },
permissions: new CipherPermissionsApi(),
});
});
@@ -639,6 +655,7 @@ describe("Cipher DTO", () => {
cipher.deletedDate = null;
cipher.reprompt = CipherRepromptType.None;
cipher.key = mockEnc("EncKey");
cipher.permissions = new CipherPermissionsApi();
const identityView = new IdentityView();
identityView.firstName = "firstName";
@@ -681,6 +698,7 @@ describe("Cipher DTO", () => {
deletedDate: null,
reprompt: 0,
localData: undefined,
permissions: new CipherPermissionsApi(),
});
});
});

View File

@@ -10,6 +10,7 @@ import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-cr
import { InitializerKey } from "../../../platform/services/cryptography/initializer-key";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherType } from "../../enums/cipher-type";
import { CipherPermissionsApi } from "../api/cipher-permissions.api";
import { CipherData } from "../data/cipher.data";
import { LocalData } from "../data/local.data";
import { AttachmentView } from "../view/attachment.view";
@@ -39,6 +40,7 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
organizationUseTotp: boolean;
edit: boolean;
viewPassword: boolean;
permissions: CipherPermissionsApi;
revisionDate: Date;
localData: LocalData;
login: Login;
@@ -84,6 +86,7 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
} else {
this.viewPassword = true; // Default for already synced Ciphers without viewPassword
}
this.permissions = obj.permissions;
this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null;
this.collectionIds = obj.collectionIds;
this.localData = localData;
@@ -244,6 +247,7 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
c.deletedDate = this.deletedDate != null ? this.deletedDate.toISOString() : null;
c.reprompt = this.reprompt;
c.key = this.key?.encryptedString;
c.permissions = this.permissions;
this.buildDataModel(this, c, {
name: null,

View File

@@ -3,6 +3,7 @@
import { BaseResponse } from "../../../models/response/base.response";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CardApi } from "../api/card.api";
import { CipherPermissionsApi } from "../api/cipher-permissions.api";
import { FieldApi } from "../api/field.api";
import { IdentityApi } from "../api/identity.api";
import { LoginApi } from "../api/login.api";
@@ -28,6 +29,7 @@ export class CipherResponse extends BaseResponse {
favorite: boolean;
edit: boolean;
viewPassword: boolean;
permissions: CipherPermissionsApi;
organizationUseTotp: boolean;
revisionDate: string;
attachments: AttachmentResponse[];
@@ -53,6 +55,7 @@ export class CipherResponse extends BaseResponse {
} else {
this.viewPassword = this.getResponseProperty("ViewPassword");
}
this.permissions = new CipherPermissionsApi(this.getResponseProperty("Permissions"));
this.organizationUseTotp = this.getResponseProperty("OrganizationUseTotp");
this.revisionDate = this.getResponseProperty("RevisionDate");
this.collectionIds = this.getResponseProperty("CollectionIds");

View File

@@ -6,6 +6,7 @@ import { InitializerKey } from "../../../platform/services/cryptography/initiali
import { DeepJsonify } from "../../../types/deep-jsonify";
import { CipherType, LinkedIdType } from "../../enums";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherPermissionsApi } from "../api/cipher-permissions.api";
import { LocalData } from "../data/local.data";
import { Cipher } from "../domain/cipher";
@@ -29,6 +30,7 @@ export class CipherView implements View, InitializerMetadata {
type: CipherType = null;
favorite = false;
organizationUseTotp = false;
permissions: CipherPermissionsApi = new CipherPermissionsApi();
edit = false;
viewPassword = true;
localData: LocalData;
@@ -63,6 +65,7 @@ export class CipherView implements View, InitializerMetadata {
this.organizationUseTotp = c.organizationUseTotp;
this.edit = c.edit;
this.viewPassword = c.viewPassword;
this.permissions = c.permissions;
this.type = c.type;
this.localData = c.localData;
this.collectionIds = c.collectionIds;

View File

@@ -8,6 +8,8 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CollectionId, UserId } from "@bitwarden/common/types/guid";
import { FakeAccountService, mockAccountServiceWith } from "../../../spec";
import { ConfigService } from "../../platform/abstractions/config/config.service";
import { CipherPermissionsApi } from "../models/api/cipher-permissions.api";
import { CipherView } from "../models/view/cipher.view";
import {
@@ -20,6 +22,7 @@ describe("CipherAuthorizationService", () => {
const mockCollectionService = mock<CollectionService>();
const mockOrganizationService = mock<OrganizationService>();
const mockConfigService = mock<ConfigService>();
const mockUserId = Utils.newGuid() as UserId;
let mockAccountService: FakeAccountService;
@@ -28,10 +31,12 @@ describe("CipherAuthorizationService", () => {
organizationId: string | null,
collectionIds: string[],
edit: boolean = true,
permissions: CipherPermissionsApi = new CipherPermissionsApi(),
) => ({
organizationId,
collectionIds,
edit,
permissions,
});
const createMockCollection = (id: string, manage: boolean) => ({
@@ -63,7 +68,78 @@ describe("CipherAuthorizationService", () => {
mockCollectionService,
mockOrganizationService,
mockAccountService,
mockConfigService,
);
mockConfigService.getFeatureFlag$.mockReturnValue(of(false));
});
describe("canRestoreCipher$", () => {
it("should return true if isAdminConsoleAction and cipher is unassigned", (done) => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization({ canEditUnassignedCiphers: true });
mockOrganizationService.organizations$.mockReturnValue(
of([organization]) as Observable<Organization[]>,
);
cipherAuthorizationService.canRestoreCipher$(cipher, true).subscribe((result) => {
expect(result).toBe(true);
done();
});
});
it("should return true if isAdminConsleAction and user can edit all ciphers in the org", (done) => {
const cipher = createMockCipher("org1", ["col1"]) as CipherView;
const organization = createMockOrganization({ canEditAllCiphers: true });
mockOrganizationService.organizations$.mockReturnValue(
of([organization]) as Observable<Organization[]>,
);
cipherAuthorizationService.canRestoreCipher$(cipher, true).subscribe((result) => {
expect(result).toBe(true);
expect(mockOrganizationService.organizations$).toHaveBeenCalledWith(mockUserId);
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.organizations$.mockReturnValue(of([organization] as Organization[]));
cipherAuthorizationService.canRestoreCipher$(cipher, true).subscribe((result) => {
expect(result).toBe(false);
done();
});
});
it("should return false if cipher.permission.restore is false and is not an admin action", (done) => {
const cipher = createMockCipher("org1", [], true, {
restore: false,
} as CipherPermissionsApi) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));
cipherAuthorizationService.canRestoreCipher$(cipher, false).subscribe((result) => {
expect(result).toBe(false);
expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled();
done();
});
});
it("should return true if cipher.permission.restore is true and is not an admin action", (done) => {
const cipher = createMockCipher("org1", [], true, {
restore: true,
} as CipherPermissionsApi) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));
cipherAuthorizationService.canRestoreCipher$(cipher, false).subscribe((result) => {
expect(result).toBe(true);
expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled();
done();
});
});
});
describe("canDeleteCipher$", () => {
@@ -213,6 +289,34 @@ describe("CipherAuthorizationService", () => {
done();
});
});
it("should return true if feature flag enabled and cipher.permissions.delete is true", (done) => {
const cipher = createMockCipher("org1", [], true, {
delete: true,
} as CipherPermissionsApi) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
cipherAuthorizationService.canDeleteCipher$(cipher, [], false).subscribe((result) => {
expect(result).toBe(true);
expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled();
done();
});
});
it("should return false if feature flag enabled and cipher.permissions.delete is false", (done) => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));
mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
cipherAuthorizationService.canDeleteCipher$(cipher, [], false).subscribe((result) => {
expect(result).toBe(false);
expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled();
done();
});
});
});
describe("canCloneCipher$", () => {

View File

@@ -1,12 +1,13 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { map, Observable, of, shareReplay, switchMap } from "rxjs";
import { combineLatest, map, Observable, of, shareReplay, switchMap } from "rxjs";
import { CollectionService } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { CollectionId } from "@bitwarden/common/types/guid";
import { getUserId } from "../../auth/services/account.service";
import { FeatureFlag } from "../../enums/feature-flag.enum";
import { Cipher } from "../models/domain/cipher";
import { CipherView } from "../models/view/cipher.view";
@@ -28,12 +29,25 @@ export abstract class CipherAuthorizationService {
*
* @returns {Observable<boolean>} - An observable that emits a boolean value indicating if the user can delete the cipher.
*/
canDeleteCipher$: (
abstract canDeleteCipher$: (
cipher: CipherLike,
allowedCollections?: CollectionId[],
isAdminConsoleAction?: boolean,
) => Observable<boolean>;
/**
* Determines if the user can restore the specified cipher.
*
* @param {CipherLike} cipher - The cipher object to evaluate for restore permissions.
* @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 restore the cipher.
*/
abstract canRestoreCipher$: (
cipher: CipherLike,
isAdminConsoleAction?: boolean,
) => Observable<boolean>;
/**
* Determines if the user can clone the specified cipher.
*
@@ -42,7 +56,10 @@ export abstract class CipherAuthorizationService {
*
* @returns {Observable<boolean>} - An observable that emits a boolean value indicating if the user can clone the cipher.
*/
canCloneCipher$: (cipher: CipherLike, isAdminConsoleAction?: boolean) => Observable<boolean>;
abstract canCloneCipher$: (
cipher: CipherLike,
isAdminConsoleAction?: boolean,
) => Observable<boolean>;
}
/**
@@ -53,13 +70,16 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
private collectionService: CollectionService,
private organizationService: OrganizationService,
private accountService: AccountService,
private configService: ConfigService,
) {}
private organization$ = (cipher: CipherLike) =>
this.accountService.activeAccount$.pipe(
switchMap((account) => this.organizationService.organizations$(account?.id)),
getUserId,
switchMap((userId) => this.organizationService.organizations$(userId)),
map((orgs) => orgs.find((org) => org.id === cipher.organizationId)),
);
/**
*
* {@link CipherAuthorizationService.canDeleteCipher$}
@@ -69,12 +89,11 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
allowedCollections?: CollectionId[],
isAdminConsoleAction?: boolean,
): Observable<boolean> {
if (cipher.organizationId == null) {
return of(true);
}
return this.organization$(cipher).pipe(
switchMap((organization) => {
return combineLatest([
this.organization$(cipher),
this.configService.getFeatureFlag$(FeatureFlag.LimitItemDeletion),
]).pipe(
switchMap(([organization, featureFlagEnabled]) => {
if (isAdminConsoleAction) {
// If the user is an admin, they can delete an unassigned cipher
if (!cipher.collectionIds || cipher.collectionIds.length === 0) {
@@ -86,6 +105,14 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
}
}
if (featureFlagEnabled) {
return of(cipher.permissions.delete);
}
if (cipher.organizationId == null) {
return of(true);
}
return this.collectionService
.decryptedCollectionViews$(cipher.collectionIds as CollectionId[])
.pipe(
@@ -93,7 +120,7 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
const shouldFilter = allowedCollections?.some(Boolean);
const collections = shouldFilter
? allCollections.filter((c) => allowedCollections.includes(c.id as CollectionId))
? allCollections.filter((c) => allowedCollections?.includes(c.id as CollectionId))
: allCollections;
return collections.some((collection) => collection.manage);
@@ -103,6 +130,29 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
);
}
/**
*
* {@link CipherAuthorizationService.canRestoreCipher$}
*/
canRestoreCipher$(cipher: CipherLike, isAdminConsoleAction?: boolean): Observable<boolean> {
return this.organization$(cipher).pipe(
map((organization) => {
if (isAdminConsoleAction) {
// If the user is an admin, they can restore an unassigned cipher
if (!cipher.collectionIds || cipher.collectionIds.length === 0) {
return organization?.canEditUnassignedCiphers === true;
}
if (organization?.canEditAllCiphers) {
return true;
}
}
return cipher.permissions.restore;
}),
);
}
/**
* {@link CipherAuthorizationService.canCloneCipher$}
*/
@@ -116,6 +166,7 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
// Admins and custom users can always clone when in the Admin Console
if (
isAdminConsoleAction &&
organization &&
(organization.isAdmin || organization.permissions?.editAnyCollection)
) {
return of(true);

View File

@@ -27,6 +27,7 @@ import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file
import { FieldType } from "../enums";
import { CipherRepromptType } from "../enums/cipher-reprompt-type";
import { CipherType } from "../enums/cipher-type";
import { CipherPermissionsApi } from "../models/api/cipher-permissions.api";
import { CipherData } from "../models/data/cipher.data";
import { Cipher } from "../models/domain/cipher";
import { CipherCreateRequest } from "../models/request/cipher-create.request";
@@ -57,6 +58,7 @@ const cipherData: CipherData = {
notes: "EncryptedString",
creationDate: "2022-01-01T12:00:00.000Z",
deletedDate: null,
permissions: new CipherPermissionsApi(),
key: "EncKey",
reprompt: CipherRepromptType.None,
login: {