Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-1893] Removed logic to downgrade Manager roles and remove Edit/Delete any collection permissions for Flexible Collections #7365

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,6 @@ export default class MainBackground {
this.folderApiService,
this.organizationService,
this.sendApiService,
this.configService,
logoutCallback,
);
this.eventUploadService = new EventUploadService(
Expand Down
1 change: 0 additions & 1 deletion apps/cli/src/bw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,7 @@
this.providerService,
this.folderApiService,
this.organizationService,
this.sendApiService,

Check notice on line 478 in apps/cli/src/bw.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Large Method

Main.constructor decreases from 288 to 287 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
this.configService,
async (expired: boolean) => await this.logout(),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga
import { SecretsManagerSubscribeRequest } from "@bitwarden/common/billing/models/request/sm-subscribe.request";
import { BillingCustomerDiscount } from "@bitwarden/common/billing/models/response/organization-subscription.response";
import { PlanResponse } from "@bitwarden/common/billing/models/response/plan.response";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";

Expand All @@ -35,7 +33,6 @@ export class SecretsManagerSubscribeStandaloneComponent {
private i18nService: I18nService,
private organizationApiService: OrganizationApiServiceAbstraction,
private organizationService: InternalOrganizationServiceAbstraction,
private configService: ConfigServiceAbstraction,
) {}

submit = async () => {
Expand All @@ -55,11 +52,7 @@ export class SecretsManagerSubscribeStandaloneComponent {
isMember: this.organization.isMember,
isProviderUser: this.organization.isProviderUser,
});
const flexibleCollectionsEnabled = await this.configService.getFeatureFlag(
FeatureFlag.FlexibleCollections,
false,
);
await this.organizationService.upsert(organizationData, flexibleCollectionsEnabled);
await this.organizationService.upsert(organizationData);

/*
Because subscribing to Secrets Manager automatically provides access to Secrets Manager for the
Expand Down
1 change: 0 additions & 1 deletion libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ import { ModalService } from "./modal.service";
FolderApiServiceAbstraction,
OrganizationServiceAbstraction,
SendApiServiceAbstraction,
ConfigServiceAbstraction,
LOGOUT_CALLBACK,
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@ export abstract class OrganizationService {
}

export abstract class InternalOrganizationServiceAbstraction extends OrganizationService {
replace: (
organizations: { [id: string]: OrganizationData },
flexibleCollectionsEnabled: boolean,
) => Promise<void>;
upsert: (
OrganizationData: OrganizationData | OrganizationData[],
flexibleCollectionsEnabled: boolean,
) => Promise<void>;
replace: (organizations: { [id: string]: OrganizationData }) => Promise<void>;
upsert: (OrganizationData: OrganizationData | OrganizationData[]) => Promise<void>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe("Organization Service", () => {
});

it("upsert", async () => {
await organizationService.upsert(organizationData("2", "Test 2"), false);
await organizationService.upsert(organizationData("2", "Test 2"));

expect(await firstValueFrom(organizationService.organizations$)).toEqual([
{
Expand Down Expand Up @@ -146,15 +146,15 @@ describe("Organization Service", () => {

describe("delete", () => {
it("exists", async () => {
await organizationService.delete("1", false);
await organizationService.delete("1");

expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);

expect(stateService.setOrganizations).toHaveBeenCalledTimes(1);
});

it("does not exist", async () => {
organizationService.delete("1", false);
organizationService.delete("1");

expect(stateService.getOrganizations).toHaveBeenCalledTimes(2);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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";

Expand Down Expand Up @@ -52,18 +51,18 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
return organizations.length > 0;
}

async upsert(organization: OrganizationData, flexibleCollectionsEnabled: boolean): Promise<void> {
async upsert(organization: OrganizationData): Promise<void> {
let organizations = await this.stateService.getOrganizations();
if (organizations == null) {
organizations = {};
}

organizations[organization.id] = organization;

await this.replace(organizations, flexibleCollectionsEnabled);
await this.replace(organizations);
}

async delete(id: string, flexibleCollectionsEnabled: boolean): Promise<void> {
async delete(id: string): Promise<void> {
const organizations = await this.stateService.getOrganizations();
if (organizations == null) {
return;
Expand All @@ -74,7 +73,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
}

delete organizations[id];
await this.replace(organizations, flexibleCollectionsEnabled);
await this.replace(organizations);
}

get(id: string): Organization {
Expand Down Expand Up @@ -103,24 +102,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
return organizations.find((organization) => organization.identifier === identifier);
}

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;
}
});
}

async replace(organizations: { [id: string]: OrganizationData }) {
await this.stateService.setOrganizations(organizations);
this.updateObservables(organizations);
}
Expand Down
16 changes: 3 additions & 13 deletions libs/common/src/vault/services/sync/sync.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ 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,
SyncFolderNotification,
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";
Expand Down Expand Up @@ -61,7 +59,6 @@ export class SyncService implements SyncServiceAbstraction {
private folderApiService: FolderApiServiceAbstraction,
private organizationService: InternalOrganizationServiceAbstraction,
private sendApiService: SendApiService,
private configService: ConfigServiceAbstraction,
private logoutCallback: (expired: boolean) => Promise<void>,
) {}

Expand Down Expand Up @@ -321,11 +318,7 @@ export class SyncService implements SyncServiceAbstraction {

await this.setForceSetPasswordReasonIfNeeded(response);

const flexibleCollectionsEnabled = await this.configService.getFeatureFlag(
FeatureFlag.FlexibleCollections,
false,
);
await this.syncProfileOrganizations(response, flexibleCollectionsEnabled);
await this.syncProfileOrganizations(response);

const providers: { [id: string]: ProviderData } = {};
response.providers.forEach((p) => {
Expand Down Expand Up @@ -393,10 +386,7 @@ export class SyncService implements SyncServiceAbstraction {
}
}

private async syncProfileOrganizations(
response: ProfileResponse,
flexibleCollectionsEnabled: boolean,
) {
private async syncProfileOrganizations(response: ProfileResponse) {
const organizations: { [id: string]: OrganizationData } = {};
response.organizations.forEach((o) => {
organizations[o.id] = new OrganizationData(o, {
Expand All @@ -416,7 +406,7 @@ export class SyncService implements SyncServiceAbstraction {
}
});

await this.organizationService.replace(organizations, flexibleCollectionsEnabled);
await this.organizationService.replace(organizations);
}

private async syncFolders(response: FolderResponse[]) {
Expand Down
Loading