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.

Sign up for GitHub

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

Jump to bottom

[AC-1139] Flexible collections: deprecate Manage/Edit/Delete Assigned Collections custom permissions #6906

Merged
merged 30 commits into from
Dec 8, 2023
Merged

[AC-1139] Flexible collections: deprecate Manage/Edit/Delete Assigned Collections custom permissions #6906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ca1ef99
[AC-1139] Add new layout for MemberDialogComponent when FC feature fl…
r-tome Nov 16, 2023
7ce712a
[AC-1139] Deprecated Organization canEditAssignedCollections, canDele…
r-tome Nov 16, 2023
8e5dc60
[AC-1139] Checking if FC feature flag is enabled when using canDelete…
r-tome Nov 17, 2023
8d2e529
[AC-1139] Added missing parameter to customRedirect
r-tome Nov 17, 2023
4e28fc1
[AC-1139] Fixed canEdit permission
r-tome Nov 17, 2023
12a76a1
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
r-tome Nov 20, 2023
572e6e7
[AC-1139] Fixed CanDelete logic
r-tome Nov 21, 2023
9cac254
[AC-1139] Changed canAccessVaultTab function to receive configService
r-tome Nov 22, 2023
f0c25a6
Override deprecated values on sync
eliykat Nov 23, 2023
ed9e035
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
r-tome Nov 27, 2023
975d38b
[AC-1139] Reverted change that introduced ConfigService as a paramete…
r-tome Nov 27, 2023
e54e614
Merge remote-tracking branch 'origin/ac/ac-1139/deprecate-custom-coll…
r-tome Nov 27, 2023
6484420
[AC-1139] Fixed circular dependency
r-tome Nov 27, 2023
c6d8067
[AC-1139] Moved overriding of deprecated values to syncService
r-tome Nov 27, 2023
108de73
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
r-tome Nov 28, 2023
5114eac
Revert "[AC-1139] Fixed circular dependency"
r-tome Nov 28, 2023
8be75ef
Revert "Override deprecated values on sync"
r-tome Nov 28, 2023
33b6930
[AC-1139] Added back the deprecation of methods canEditAssignedCollec…
r-tome Nov 28, 2023
04fb7c0
[AC-1139] Reverted change on syncService
r-tome Nov 28, 2023
9cfe891
[AC-1139] Override deprecated values on sync
r-tome Nov 28, 2023
59d1fe6
[AC-1139] Fix canDelete logic in
r-tome Dec 4, 2023
1c4e73b
[AC-1139] Moved override logic from syncService to organizationService
r-tome Dec 4, 2023
efb3195
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
r-tome Dec 5, 2023
a97f44a
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
r-tome Dec 5, 2023
9ede0fc
[AC-1139] Add ability to have titlecase titles on nested-checkbox.com…
r-tome Dec 5, 2023
7622c99
Revert "[AC-1139] Add ability to have titlecase titles on nested-chec…
eliykat Dec 6, 2023
561aaf0
[AC-1139] Fix bulk delete functionality
r-tome Dec 6, 2023
b9ca4f0
[AC-1139] Refactor canEdit and canDelete to use ternary operator
r-tome Dec 6, 2023
73dcf08
[AC-1139] Fix canDelete condition in VaultComponent
r-tome Dec 7, 2023
8896d81
Merge branch 'master' into ac/ac-1139/deprecate-custom-collection-perm
eliykat Dec 8, 2023
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: 1 addition & 0 deletions apps/browser/src/background/main.background.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ export default class MainBackground {
this.folderApiService,
this.organizationService,
this.sendApiService,
this.configService,
logoutCallback,
);
this.eventUploadService = new EventUploadService(
Expand Down
1 change: 1 addition & 0 deletions apps/cli/src/bw.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@
this.folderApiService,
this.organizationService,
this.sendApiService,
this.configService,

Check warning on line 446 in apps/cli/src/bw.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Large Method 

Main.constructor increases from 271 to 272 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.
async (expired: boolean) => await this.logout(),
);

Expand Down
257 changes: 183 additions & 74 deletions ...admin-console/organizations/members/components/member-dialog/member-dialog.component.html
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The labels for each custom permission checkbox in Figma all use title case. e.g. Edit Any Collection, not Edit any collection.

You may just be able to use the TitleCasePipe to adjust this without updating messages.json, which would require re-translations and CrowdIn sync etc.

Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,128 @@
</div>
</fieldset>
<ng-container *ngIf="customUserTypeSelected">
<h3 class="mt-4 d-flex tw-font-semibold">
{{ "permissions" | i18n }}
</h3>
<div class="row" [formGroup]="permissionsGroup">
<div class="col-6">
<div class="mb-3">
<label class="tw-font-semibold">{{ "managerPermissions" | i18n }}</label>
<hr class="tw-mb-2 tw-mr-2 tw-mt-0" />
<app-nested-checkbox
parentId="manageAssignedCollections"
[checkboxes]="permissionsGroup.controls.manageAssignedCollectionsGroup"
>
</app-nested-checkbox>
<ng-container *ngIf="!(flexibleCollectionsEnabled$ | async); else customPermissionsFC">
<h3 class="mt-4 d-flex tw-font-semibold">
shane-melton marked this conversation as resolved.
Show resolved Hide resolved
{{ "permissions" | i18n }}
</h3>
<div class="row" [formGroup]="permissionsGroup">
<div class="col-6">
<div class="mb-3">
<label class="tw-font-semibold">{{ "managerPermissions" | i18n }}</label>
<hr class="tw-mb-2 tw-mr-2 tw-mt-0" />
<app-nested-checkbox
parentId="manageAssignedCollections"
[checkboxes]="permissionsGroup.controls.manageAssignedCollectionsGroup"
>
</app-nested-checkbox>
</div>
</div>
<div class="col-6">
<div class="mb-3">
<label class="tw-font-semibold">{{ "adminPermissions" | i18n }}</label>
<hr class="tw-mb-2 tw-mr-2 tw-mt-0" />
<div>
<input
type="checkbox"
name="accessEventLogs"
id="accessEventLogs"
formControlName="accessEventLogs"
/>
<label class="!tw-font-normal" for="accessEventLogs">
{{ "accessEventLogs" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="accessImportExport"
id="accessImportExport"
formControlName="accessImportExport"
/>
<label class="!tw-font-normal" for="accessImportExport">
{{ "accessImportExport" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="accessReports"
id="accessReports"
formControlName="accessReports"
/>
<label class="!tw-font-normal" for="accessReports">
{{ "accessReports" | i18n }}
</label>
</div>
<app-nested-checkbox
parentId="manageAllCollections"
[checkboxes]="permissionsGroup.controls.manageAllCollectionsGroup"
>
</app-nested-checkbox>
<div>
<input
type="checkbox"
name="manageGroups"
id="manageGroups"
formControlName="manageGroups"
/>
<label class="!tw-font-normal" for="manageGroups">
{{ "manageGroups" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageSso"
id="manageSso"
formControlName="manageSso"
/>
<label class="!tw-font-normal" for="manageSso">
{{ "manageSso" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="managePolicies"
id="managePolicies"
formControlName="managePolicies"
/>
<label class="!tw-font-normal" for="managePolicies">
{{ "managePolicies" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageUsers"
id="manageUsers"
formControlName="manageUsers"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageUsers">
{{ "manageUsers" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageResetPassword"
id="manageResetPassword"
formControlName="manageResetPassword"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageResetPassword">
{{ "manageAccountRecovery" | i18n }}
</label>
</div>
</div>
</div>
</div>
<div class="col-6">
<div class="mb-3">
<label class="tw-font-semibold">{{ "adminPermissions" | i18n }}</label>
<hr class="tw-mb-2 tw-mr-2 tw-mt-0" />
</ng-container>
<ng-template #customPermissionsFC>
<div class="row" [formGroup]="permissionsGroup">
<div class="col-4">
<div>
<input
type="checkbox"
Expand Down Expand Up @@ -190,71 +293,77 @@ <h3 class="mt-4 d-flex tw-font-semibold">
{{ "accessReports" | i18n }}
</label>
</div>
</div>
<div class="col-4">
<app-nested-checkbox
parentId="manageAllCollections"
[checkboxes]="permissionsGroup.controls.manageAllCollectionsGroup"
>
</app-nested-checkbox>
<div>
<input
type="checkbox"
name="manageGroups"
id="manageGroups"
formControlName="manageGroups"
/>
<label class="!tw-font-normal" for="manageGroups">
{{ "manageGroups" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageSso"
id="manageSso"
formControlName="manageSso"
/>
<label class="!tw-font-normal" for="manageSso">
{{ "manageSso" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="managePolicies"
id="managePolicies"
formControlName="managePolicies"
/>
<label class="!tw-font-normal" for="managePolicies">
{{ "managePolicies" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageUsers"
id="manageUsers"
formControlName="manageUsers"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageUsers">
{{ "manageUsers" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageResetPassword"
id="manageResetPassword"
formControlName="manageResetPassword"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageResetPassword">
{{ "manageAccountRecovery" | i18n }}
</label>
</div>
<div class="col-4">
<div class="mb-3">
<div>
<input
type="checkbox"
name="manageGroups"
id="manageGroups"
formControlName="manageGroups"
/>
<label class="!tw-font-normal" for="manageGroups">
{{ "manageGroups" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageSso"
id="manageSso"
formControlName="manageSso"
/>
<label class="!tw-font-normal" for="manageSso">
{{ "manageSso" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="managePolicies"
id="managePolicies"
formControlName="managePolicies"
/>
<label class="!tw-font-normal" for="managePolicies">
{{ "managePolicies" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageUsers"
id="manageUsers"
formControlName="manageUsers"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageUsers">
{{ "manageUsers" | i18n }}
</label>
</div>
<div>
<input
type="checkbox"
name="manageResetPassword"
id="manageResetPassword"
formControlName="manageResetPassword"
(change)="handleDependentPermissions()"
/>
<label class="!tw-font-normal" for="manageResetPassword">
{{ "manageAccountRecovery" | i18n }}
</label>
</div>
</div>
</div>
</div>
</div>
</ng-template>
</ng-container>
<ng-container *ngIf="canUseSecretsManager">
<h3 class="mt-4">
Expand Down
9 changes: 8 additions & 1 deletion apps/web/src/app/billing/organizations/sm-subscribe-standalone.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ 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 @@ -33,6 +35,7 @@ export class SecretsManagerSubscribeStandaloneComponent {
private i18nService: I18nService,
private organizationApiService: OrganizationApiServiceAbstraction,
private organizationService: InternalOrganizationServiceAbstraction,
private configService: ConfigServiceAbstraction,
) {}

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

/*
Because subscribing to Secrets Manager automatically provides access to Secrets Manager for the
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
{{ "cancel" | i18n }}
</button>
<button
*ngIf="editMode && organization?.canDeleteAssignedCollections"
*ngIf="canDelete$ | async"
r-tome marked this conversation as resolved.
Show resolved Hide resolved
type="button"
bitIconButton="bwi-trash"
buttonType="danger"
Expand Down
7 changes: 7 additions & 0 deletions apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,13 @@ export class CollectionDialogComponent implements OnInit, OnDestroy {
this.close(CollectionDialogAction.Deleted, this.collection);
};

protected canDelete$ = this.flexibleCollectionsEnabled$.pipe(
map(
(flexibleCollectionsEnabled) =>
this.editMode && this.collection.canDelete(this.organization, flexibleCollectionsEnabled),
),
);

ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/app/vault/components/vault-items/vault-items.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class VaultItemsComponent {
}

const organization = this.allOrganizations.find((o) => o.id === collection.organizationId);
return collection.canEdit(organization);
return collection.canEdit(organization, this.flexibleCollectionsEnabled);
}

protected canDeleteCollection(collection: CollectionView): boolean {
Expand Down
14 changes: 7 additions & 7 deletions apps/web/src/app/vault/core/views/collection-admin.view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ export class CollectionAdminView extends CollectionView {
this.assigned = response.assigned;
}

override canEdit(org: Organization): boolean {
return org?.canEditAnyCollection || (org?.canEditAssignedCollections && this.assigned);
override canEdit(org: Organization, flexibleCollectionsEnabled: boolean): boolean {
return flexibleCollectionsEnabled
? org?.canEditAnyCollection
: org?.canEditAnyCollection || (org?.canEditAssignedCollections && this.assigned);
}

override canDelete(org: Organization, flexibleCollectionsEnabled: boolean): boolean {
if (flexibleCollectionsEnabled) {
return org?.canDeleteAnyCollection;
} else {
return org?.canDeleteAnyCollection || (org?.canDeleteAssignedCollections && this.assigned);
}
return flexibleCollectionsEnabled
? org?.canDeleteAnyCollection
: org?.canDeleteAnyCollection || (org?.canDeleteAssignedCollections && this.assigned);
}
}
Loading
Loading