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-1713] [Flexible collections] Add feature flags to clients #6486

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ <h1 bitTypography="h1" class="tw-mt-16 tw-pb-2.5">{{ "apiKey" | i18n }}</h1>
</button>
</ng-container>
<form
*ngIf="org && !loading"
*ngIf="org && !loading && (showCollectionManagementSettings$ | async)"
[bitSubmit]="submitCollectionManagement"
[formGroup]="collectionManagementFormGroup"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { OrganizationCollectionManagementUpdateRequest } from "@bitwarden/common
import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request";
import { OrganizationUpdateRequest } from "@bitwarden/common/admin-console/models/request/organization-update.request";
import { OrganizationResponse } from "@bitwarden/common/admin-console/models/response/organization.response";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand Down Expand Up @@ -39,6 +41,10 @@ export class AccountComponent {
canUseApi = false;
org: OrganizationResponse;
taxFormPromise: Promise<unknown>;
showCollectionManagementSettings$ = this.configService.getFeatureFlag$(
FeatureFlag.FlexibleCollections,
false
);

// FormGroup validators taken from server Organization domain object
protected formGroup = this.formBuilder.group({
Expand Down Expand Up @@ -78,7 +84,8 @@ export class AccountComponent {
private organizationService: OrganizationService,
private organizationApiService: OrganizationApiServiceAbstraction,
private dialogService: DialogService,
private formBuilder: FormBuilder
private formBuilder: FormBuilder,
private configService: ConfigServiceAbstraction
) {}

async ngOnInit() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { FormsModule, ReactiveFormsModule } from "@angular/forms";
import { MockProxy, mock } from "jest-mock-extended";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import {
OrganizationUserStatusType,
OrganizationUserType,
} from "@bitwarden/common/admin-console/enums";
import { ConfigService } from "@bitwarden/common/platform/services/config/config.service";
import {
AvatarModule,
BadgeModule,
Expand Down Expand Up @@ -50,8 +52,11 @@ class TestableAccessSelectorComponent extends AccessSelectorComponent {
describe("AccessSelectorComponent", () => {
let component: TestableAccessSelectorComponent;
let fixture: ComponentFixture<TestableAccessSelectorComponent>;
let configService: MockProxy<ConfigService>;

beforeEach(() => {
configService = mock();

TestBed.configureTestingModule({
imports: [
ButtonModule,
Expand All @@ -67,7 +72,12 @@ describe("AccessSelectorComponent", () => {
IconButtonModule,
],
declarations: [TestableAccessSelectorComponent, UserTypePipe],
providers: [],
providers: [
{
provide: ConfigService,
useValue: configService,
},
],
}).compileComponents();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Subject, takeUntil } from "rxjs";

import { ControlsOf } from "@bitwarden/angular/types/controls-of";
import { FormSelectionList } from "@bitwarden/angular/utils/form-selection-list";
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 { SelectItemView } from "@bitwarden/components/src/multi-select/models/select-item-view";

Expand Down Expand Up @@ -121,8 +123,11 @@ export class AccessSelectorComponent implements ControlValueAccessor, OnInit, On
{ perm: CollectionPermission.ViewExceptPass, labelId: "canViewExceptPass" },
{ perm: CollectionPermission.Edit, labelId: "canEdit" },
{ perm: CollectionPermission.EditExceptPass, labelId: "canEditExceptPass" },
{ perm: CollectionPermission.Manage, labelId: "canManage" },
];
private canManagePermissionListItem = {
perm: CollectionPermission.Manage,
labelId: "canManage",
};
protected initialPermission = CollectionPermission.View;

disabled: boolean;
Expand Down Expand Up @@ -195,7 +200,10 @@ export class AccessSelectorComponent implements ControlValueAccessor, OnInit, On

constructor(
private readonly formBuilder: FormBuilder,
private readonly i18nService: I18nService
private readonly i18nService: I18nService,

// reminder: remove this dependency from the spec file as well when this feature flag is removed
private readonly configService: ConfigServiceAbstraction
) {}

/** Required for NG_VALUE_ACCESSOR */
Expand Down Expand Up @@ -255,7 +263,7 @@ export class AccessSelectorComponent implements ControlValueAccessor, OnInit, On
this.pauseChangeNotification = false;
}

ngOnInit() {
async ngOnInit() {
// Watch the internal formArray for changes and propagate them
this.selectionList.formArray.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((v) => {
if (!this.notifyOnChange || this.pauseChangeNotification) {
Expand All @@ -269,6 +277,10 @@ export class AccessSelectorComponent implements ControlValueAccessor, OnInit, On
}
this.notifyOnChange(v);
});

if (await this.configService.getFeatureFlag(FeatureFlag.FlexibleCollections)) {
this.permissionList.push(this.canManagePermissionListItem);
}
}

ngOnDestroy() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { AbstractControl, FormBuilder, Validators } from "@angular/forms";
import {
combineLatest,
firstValueFrom,
from,
map,
Observable,
Expand All @@ -16,6 +17,8 @@
import { OrganizationUserService } from "@bitwarden/common/abstractions/organization-user/organization-user.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
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";
import { Utils } from "@bitwarden/common/platform/misc/utils";
Expand Down Expand Up @@ -69,6 +72,10 @@
export class CollectionDialogComponent implements OnInit, OnDestroy {
private destroy$ = new Subject<void>();
protected organizations$: Observable<Organization[]>;
private flexibleCollectionsEnabled$ = this.configService.getFeatureFlag$(
FeatureFlag.FlexibleCollections,
false
);

protected tabIndex: CollectionDialogTabType;
protected loading = true;
Expand All @@ -82,7 +89,7 @@
name: ["", [Validators.required, BitValidators.forbiddenCharacters(["/"])]],
externalId: "",
parent: undefined as string | undefined,
access: [[] as AccessItemValue[], [validateCanManagePermission]],
access: [[] as AccessItemValue[]],
selectedOrg: "",
});
protected PermissionMode = PermissionMode;
Expand All @@ -98,7 +105,8 @@
private platformUtilsService: PlatformUtilsService,
private organizationUserService: OrganizationUserService,
private dialogService: DialogService,
private changeDetectorRef: ChangeDetectorRef
private changeDetectorRef: ChangeDetectorRef,
private configService: ConfigServiceAbstraction
) {
this.tabIndex = params.initialTab ?? CollectionDialogTabType.Info;
}
Expand All @@ -124,6 +132,10 @@
this.formGroup.patchValue({ selectedOrg: this.params.organizationId });
await this.loadOrg(this.params.organizationId, this.params.collectionIds);
}

if (await firstValueFrom(this.flexibleCollectionsEnabled$)) {
this.formGroup.controls.access.addValidators(validateCanManagePermission);
}
}

async loadOrg(orgId: string, collectionIds: string[]) {
Expand All @@ -147,67 +159,72 @@
: of(null),
groups: groups$,
users: this.organizationUserService.getAllUsers(orgId),
flexibleCollections: this.flexibleCollectionsEnabled$,
})
.pipe(takeUntil(this.formGroup.controls.selectedOrg.valueChanges), takeUntil(this.destroy$))
.subscribe(({ organization, collections, collectionDetails, groups, users }) => {
this.organization = organization;
this.accessItems = [].concat(
groups.map(mapGroupToAccessItemView),
users.data.map(mapUserToAccessItemView)
);

// Force change detection to update the access selector's items
this.changeDetectorRef.detectChanges();

if (collectionIds) {
collections = collections.filter((c) => collectionIds.includes(c.id));
}

if (this.params.collectionId) {
this.collection = collections.find((c) => c.id === this.collectionId);
this.nestOptions = collections.filter((c) => c.id !== this.collectionId);

if (!this.collection) {
throw new Error("Could not find collection to edit.");
.subscribe(
({ organization, collections, collectionDetails, groups, users, flexibleCollections }) => {
this.organization = organization;
this.accessItems = [].concat(
groups.map(mapGroupToAccessItemView),
users.data.map(mapUserToAccessItemView)
);

// Force change detection to update the access selector's items
this.changeDetectorRef.detectChanges();

if (collectionIds) {
collections = collections.filter((c) => collectionIds.includes(c.id));
}

const { name, parent } = parseName(this.collection);
if (parent !== undefined && !this.nestOptions.find((c) => c.name === parent)) {
this.deletedParentName = parent;
if (this.params.collectionId) {
this.collection = collections.find((c) => c.id === this.collectionId);
this.nestOptions = collections.filter((c) => c.id !== this.collectionId);

if (!this.collection) {
throw new Error("Could not find collection to edit.");
}

const { name, parent } = parseName(this.collection);
if (parent !== undefined && !this.nestOptions.find((c) => c.name === parent)) {
this.deletedParentName = parent;
}

const accessSelections = mapToAccessSelections(collectionDetails);
this.formGroup.patchValue({
name,
externalId: this.collection.externalId,
parent,
access: accessSelections,
});
} else {
this.nestOptions = collections;
const parent = collections.find((c) => c.id === this.params.parentCollectionId);
const currentOrgUserId = users.data.find(
(u) => u.userId === this.organization?.userId
)?.id;
const initialSelection: AccessItemValue[] =
currentOrgUserId !== undefined
? [
{
id: currentOrgUserId,
type: AccessItemType.Member,
permission: flexibleCollections
? CollectionPermission.Manage
: CollectionPermission.Edit,
},
]
: [];

this.formGroup.patchValue({
parent: parent?.name ?? undefined,
access: initialSelection,
});
}

const accessSelections = mapToAccessSelections(collectionDetails);
this.formGroup.patchValue({
name,
externalId: this.collection.externalId,
parent,
access: accessSelections,
});
} else {
this.nestOptions = collections;
const parent = collections.find((c) => c.id === this.params.parentCollectionId);
const currentOrgUserId = users.data.find(
(u) => u.userId === this.organization?.userId
)?.id;
const initialSelection: AccessItemValue[] =
currentOrgUserId !== undefined
? [
{
id: currentOrgUserId,
type: AccessItemType.Member,
permission: CollectionPermission.Manage,
},
]
: [];

this.formGroup.patchValue({
parent: parent?.name ?? undefined,
access: initialSelection,
});
this.loading = false;
}

this.loading = false;
});
);

Check warning on line 227 in apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (feature/flexible-collections)

❌ Getting worse: Complex Method

CollectionDialogComponent.loadOrg increases in cyclomatic complexity from 13 to 14, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

protected get collectionId() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
{{ "moveSelected" | i18n }}
</button>
<button
*ngIf="showAdminActions"
*ngIf="showAdminActions && showBulkEditCollectionAccess"
type="button"
bitMenuItem
(click)="bulkEditCollectionAccess()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class VaultItemsComponent {
@Input() allOrganizations: Organization[] = [];
@Input() allCollections: CollectionView[] = [];
@Input() allGroups: GroupView[] = [];
@Input() showBulkEditCollectionAccess = false;

private _ciphers?: CipherView[] = [];
@Input() get ciphers(): CipherView[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
[cloneableOrganizationCiphers]="false"
[showAdminActions]="false"
(onEvent)="onVaultItemsEvent($event)"
[showBulkEditCollectionAccess]="showBulkCollectionAccess$ | async"
>
</app-vault-items>
<div
Expand Down
5 changes: 5 additions & 0 deletions apps/web/src/app/vault/individual-vault/vault.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { DEFAULT_PBKDF2_ITERATIONS, EventType, KdfType } from "@bitwarden/common/enums";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ServiceUtils } from "@bitwarden/common/misc/serviceUtils";
import { TreeNode } from "@bitwarden/common/models/domain/tree-node";
import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
Expand Down Expand Up @@ -143,6 +144,10 @@ export class VaultComponent implements OnInit, OnDestroy {
protected selectedCollection: TreeNode<CollectionView> | undefined;
protected canCreateCollections = false;
protected currentSearchText$: Observable<string>;
protected showBulkCollectionAccess$ = this.configService.getFeatureFlag$(
FeatureFlag.BulkCollectionAccess,
false
);

private searchText$ = new Subject<string>();
private refresh$ = new BehaviorSubject<void>(null);
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/app/vault/org-vault/vault.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
[cloneableOrganizationCiphers]="true"
[showAdminActions]="true"
(onEvent)="onVaultItemsEvent($event)"
[showBulkEditCollectionAccess]="showBulkEditCollectionAccess$ | async"
>
</app-vault-items>
<div
Expand Down
Loading
Loading