mirror of
https://github.com/bitwarden/browser
synced 2025-12-15 07:43:35 +00:00
[AC-1139] Flexible collections: deprecate Manage/Edit/Delete Assigned Collections custom permissions (#6906)
* [AC-1139] Add new layout for MemberDialogComponent when FC feature flag is enabled * [AC-1139] Deprecated Organization canEditAssignedCollections, canDeleteAssignedCollections, canViewAssignedCollections * [AC-1139] Checking if FC feature flag is enabled when using canDeleteAssignedCollections or canViewAssignedCollections * [AC-1139] Added missing parameter to customRedirect * [AC-1139] Fixed canEdit permission * [AC-1139] Fixed CanDelete logic * [AC-1139] Changed canAccessVaultTab function to receive configService * Override deprecated values on sync * [AC-1139] Reverted change that introduced ConfigService as a parameter to canAccessVaultTab * [AC-1139] Fixed circular dependency * [AC-1139] Moved overriding of deprecated values to syncService * Revert "[AC-1139] Fixed circular dependency" This reverts commit6484420976. * Revert "Override deprecated values on sync" This reverts commitf0c25a6996. * [AC-1139] Added back the deprecation of methods canEditAssignedCollections, canDeleteAssignedCollections, canViewAssignedCollections * [AC-1139] Reverted change on syncService * [AC-1139] Override deprecated values on sync * [AC-1139] Fix canDelete logic in collection-dialog.component.ts and bulk-delete-dialog.component.ts * [AC-1139] Moved override logic from syncService to organizationService * [AC-1139] Add ability to have titlecase titles on nested-checkbox.component checkboxes; use on member-dialog.component * Revert "[AC-1139] Add ability to have titlecase titles on nested-checkbox.component checkboxes; use on member-dialog.component" This reverts commit9ede0fc5ac. * [AC-1139] Fix bulk delete functionality * [AC-1139] Refactor canEdit and canDelete to use ternary operator * [AC-1139] Fix canDelete condition in VaultComponent --------- Co-authored-by: Thomas Rittson <trittson@bitwarden.com> Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
This commit is contained in:
@@ -95,6 +95,12 @@ export abstract class OrganizationService {
|
||||
}
|
||||
|
||||
export abstract class InternalOrganizationServiceAbstraction extends OrganizationService {
|
||||
replace: (organizations: { [id: string]: OrganizationData }) => Promise<void>;
|
||||
upsert: (OrganizationData: OrganizationData | OrganizationData[]) => Promise<void>;
|
||||
replace: (
|
||||
organizations: { [id: string]: OrganizationData },
|
||||
flexibleCollectionsEnabled: boolean,
|
||||
) => Promise<void>;
|
||||
upsert: (
|
||||
OrganizationData: OrganizationData | OrganizationData[],
|
||||
flexibleCollectionsEnabled: boolean,
|
||||
) => Promise<void>;
|
||||
}
|
||||
|
||||
@@ -186,14 +186,29 @@ export class Organization {
|
||||
return this.canEditAnyCollection || this.canDeleteAnyCollection;
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated
|
||||
* This is deprecated with the introduction of Flexible Collections.
|
||||
* This will always return false if FlexibleCollections flag is on.
|
||||
*/
|
||||
get canEditAssignedCollections() {
|
||||
return this.isManager || this.permissions.editAssignedCollections;
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated
|
||||
* This is deprecated with the introduction of Flexible Collections.
|
||||
* This will always return false if FlexibleCollections flag is on.
|
||||
*/
|
||||
get canDeleteAssignedCollections() {
|
||||
return this.isManager || this.permissions.deleteAssignedCollections;
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated
|
||||
* This is deprecated with the introduction of Flexible Collections.
|
||||
* This will always return false if FlexibleCollections flag is on.
|
||||
*/
|
||||
get canViewAssignedCollections() {
|
||||
return this.canDeleteAssignedCollections || this.canEditAssignedCollections;
|
||||
}
|
||||
|
||||
@@ -110,7 +110,7 @@ describe("Organization Service", () => {
|
||||
});
|
||||
|
||||
it("upsert", async () => {
|
||||
await organizationService.upsert(organizationData("2", "Test 2"));
|
||||
await organizationService.upsert(organizationData("2", "Test 2"), false);
|
||||
|
||||
expect(await firstValueFrom(organizationService.organizations$)).toEqual([
|
||||
{
|
||||
@@ -146,7 +146,7 @@ describe("Organization Service", () => {
|
||||
|
||||
describe("delete", () => {
|
||||
it("exists", async () => {
|
||||
await organizationService.delete("1");
|
||||
await organizationService.delete("1", false);
|
||||
|
||||
expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);
|
||||
|
||||
@@ -154,7 +154,7 @@ describe("Organization Service", () => {
|
||||
});
|
||||
|
||||
it("does not exist", async () => {
|
||||
organizationService.delete("1");
|
||||
organizationService.delete("1", false);
|
||||
|
||||
expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
InternalOrganizationServiceAbstraction,
|
||||
isMember,
|
||||
} from "../../abstractions/organization/organization.service.abstraction";
|
||||
import { OrganizationUserType } from "../../enums";
|
||||
import { OrganizationData } from "../../models/data/organization.data";
|
||||
import { Organization } from "../../models/domain/organization";
|
||||
|
||||
@@ -51,7 +52,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
|
||||
return organizations.length > 0;
|
||||
}
|
||||
|
||||
async upsert(organization: OrganizationData): Promise<void> {
|
||||
async upsert(organization: OrganizationData, flexibleCollectionsEnabled: boolean): Promise<void> {
|
||||
let organizations = await this.stateService.getOrganizations();
|
||||
if (organizations == null) {
|
||||
organizations = {};
|
||||
@@ -59,10 +60,10 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
|
||||
|
||||
organizations[organization.id] = organization;
|
||||
|
||||
await this.replace(organizations);
|
||||
await this.replace(organizations, flexibleCollectionsEnabled);
|
||||
}
|
||||
|
||||
async delete(id: string): Promise<void> {
|
||||
async delete(id: string, flexibleCollectionsEnabled: boolean): Promise<void> {
|
||||
const organizations = await this.stateService.getOrganizations();
|
||||
if (organizations == null) {
|
||||
return;
|
||||
@@ -73,7 +74,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
|
||||
}
|
||||
|
||||
delete organizations[id];
|
||||
await this.replace(organizations);
|
||||
await this.replace(organizations, flexibleCollectionsEnabled);
|
||||
}
|
||||
|
||||
get(id: string): Organization {
|
||||
@@ -102,7 +103,24 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
|
||||
return organizations.find((organization) => organization.identifier === identifier);
|
||||
}
|
||||
|
||||
async replace(organizations: { [id: string]: OrganizationData }) {
|
||||
async replace(
|
||||
organizations: { [id: string]: OrganizationData },
|
||||
flexibleCollectionsEnabled: boolean,
|
||||
) {
|
||||
// If Flexible Collections is enabled, treat Managers as Users and ignore deprecated permissions
|
||||
if (flexibleCollectionsEnabled) {
|
||||
Object.values(organizations).forEach((o) => {
|
||||
if (o.type === OrganizationUserType.Manager) {
|
||||
o.type = OrganizationUserType.User;
|
||||
}
|
||||
|
||||
if (o.permissions != null) {
|
||||
o.permissions.editAssignedCollections = false;
|
||||
o.permissions.deleteAssignedCollections = false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
await this.stateService.setOrganizations(organizations);
|
||||
this.updateObservables(organizations);
|
||||
}
|
||||
|
||||
@@ -32,13 +32,16 @@ export class CollectionView implements View, ITreeNodeObject {
|
||||
}
|
||||
|
||||
// For editing collection details, not the items within it.
|
||||
canEdit(org: Organization): boolean {
|
||||
canEdit(org: Organization, flexibleCollectionsEnabled: boolean): boolean {
|
||||
if (org.id !== this.organizationId) {
|
||||
throw new Error(
|
||||
"Id of the organization provided does not match the org id of the collection.",
|
||||
);
|
||||
}
|
||||
return org?.canEditAnyCollection || org?.canEditAssignedCollections;
|
||||
|
||||
return flexibleCollectionsEnabled
|
||||
? org?.canEditAnyCollection || this.manage
|
||||
: org?.canEditAnyCollection || org?.canEditAssignedCollections;
|
||||
}
|
||||
|
||||
// For deleting a collection, not the items within it.
|
||||
@@ -49,10 +52,8 @@ export class CollectionView implements View, ITreeNodeObject {
|
||||
);
|
||||
}
|
||||
|
||||
if (flexibleCollectionsEnabled) {
|
||||
return org?.canDeleteAnyCollection || (!org?.limitCollectionCreationDeletion && this.manage);
|
||||
} else {
|
||||
return org?.canDeleteAnyCollection || org?.canDeleteAssignedCollections;
|
||||
}
|
||||
return flexibleCollectionsEnabled
|
||||
? org?.canDeleteAnyCollection || (!org?.limitCollectionCreationDeletion && this.manage)
|
||||
: org?.canDeleteAnyCollection || org?.canDeleteAssignedCollections;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import { ProviderData } from "../../../admin-console/models/data/provider.data";
|
||||
import { PolicyResponse } from "../../../admin-console/models/response/policy.response";
|
||||
import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service";
|
||||
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
|
||||
import { FeatureFlag } from "../../../enums/feature-flag.enum";
|
||||
import { DomainsResponse } from "../../../models/response/domains.response";
|
||||
import {
|
||||
SyncCipherNotification,
|
||||
@@ -17,6 +18,7 @@ import {
|
||||
SyncSendNotification,
|
||||
} from "../../../models/response/notification.response";
|
||||
import { ProfileResponse } from "../../../models/response/profile.response";
|
||||
import { ConfigServiceAbstraction } from "../../../platform/abstractions/config/config.service.abstraction";
|
||||
import { CryptoService } from "../../../platform/abstractions/crypto.service";
|
||||
import { LogService } from "../../../platform/abstractions/log.service";
|
||||
import { MessagingService } from "../../../platform/abstractions/messaging.service";
|
||||
@@ -59,6 +61,7 @@ export class SyncService implements SyncServiceAbstraction {
|
||||
private folderApiService: FolderApiServiceAbstraction,
|
||||
private organizationService: InternalOrganizationServiceAbstraction,
|
||||
private sendApiService: SendApiService,
|
||||
private configService: ConfigServiceAbstraction,
|
||||
private logoutCallback: (expired: boolean) => Promise<void>,
|
||||
) {}
|
||||
|
||||
@@ -318,7 +321,11 @@ export class SyncService implements SyncServiceAbstraction {
|
||||
|
||||
await this.setForceSetPasswordReasonIfNeeded(response);
|
||||
|
||||
await this.syncProfileOrganizations(response);
|
||||
const flexibleCollectionsEnabled = await this.configService.getFeatureFlag(
|
||||
FeatureFlag.FlexibleCollections,
|
||||
false,
|
||||
);
|
||||
await this.syncProfileOrganizations(response, flexibleCollectionsEnabled);
|
||||
|
||||
const providers: { [id: string]: ProviderData } = {};
|
||||
response.providers.forEach((p) => {
|
||||
@@ -374,7 +381,10 @@ export class SyncService implements SyncServiceAbstraction {
|
||||
}
|
||||
}
|
||||
|
||||
private async syncProfileOrganizations(response: ProfileResponse) {
|
||||
private async syncProfileOrganizations(
|
||||
response: ProfileResponse,
|
||||
flexibleCollectionsEnabled: boolean,
|
||||
) {
|
||||
const organizations: { [id: string]: OrganizationData } = {};
|
||||
response.organizations.forEach((o) => {
|
||||
organizations[o.id] = new OrganizationData(o, {
|
||||
@@ -394,7 +404,7 @@ export class SyncService implements SyncServiceAbstraction {
|
||||
}
|
||||
});
|
||||
|
||||
await this.organizationService.replace(organizations);
|
||||
await this.organizationService.replace(organizations, flexibleCollectionsEnabled);
|
||||
}
|
||||
|
||||
private async syncFolders(response: FolderResponse[]) {
|
||||
|
||||
Reference in New Issue
Block a user