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-2161] update cipher collections org vault modal #8027

Merged
merged 24 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1de7b08
updated collections component and vault component to disable readOnly…
Jingo88 Feb 20, 2024
03491e4
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Feb 20, 2024
69b3ce4
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Feb 23, 2024
8ee593b
update collections component to check if org allows Owners up manage …
Jingo88 Feb 26, 2024
d559a12
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Feb 28, 2024
29d2e53
updated canEditItems logic
Jingo88 Feb 28, 2024
3a960a4
added a canEditItems method to check for flex collection flag, assign…
Jingo88 Feb 28, 2024
f299244
add logic to pass organization so canEditItems does not return false …
Jingo88 Feb 29, 2024
0907d7e
updated other collections components with org service import
Jingo88 Feb 29, 2024
325df09
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 7, 2024
f274391
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 11, 2024
4038744
add logic for unassigned items in saveCollections of org vault collec…
Jingo88 Mar 12, 2024
86457a6
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 12, 2024
cab9613
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 13, 2024
9f955c7
update loadCipher method in collections component org vault to target…
Jingo88 Mar 13, 2024
6f8f2e3
add canEditAllCiphers logic to submit in collections component so we …
Jingo88 Mar 13, 2024
da4264f
cleanup excess code in collectiosn component. remove filter in vaultE…
Jingo88 Mar 14, 2024
e1ba2de
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 15, 2024
18263dd
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 18, 2024
b4d0f09
change collections value in editCipherCollections for FC Flag off so …
Jingo88 Mar 18, 2024
7b538cb
check for unassigned ciphers added to org vault load method
Jingo88 Mar 19, 2024
db42343
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 19, 2024
36f5c43
latest main. update collections component with missing await
Jingo88 Mar 19, 2024
a744ade
Merge branch 'main' into AC-2161-Update-Cipher-Collections
Jingo88 Mar 21, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { first } from "rxjs/operators";

import { CollectionsComponent as BaseCollectionsComponent } from "@bitwarden/angular/admin-console/components/collections.component";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";

Check warning on line 7 in apps/browser/src/vault/popup/components/vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/components/vault/collections.component.ts#L7

Added line #L7 was not covered by tests
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -21,11 +22,19 @@
platformUtilsService: PlatformUtilsService,
i18nService: I18nService,
cipherService: CipherService,
organizationService: OrganizationService,
private route: ActivatedRoute,
private location: Location,
logService: LogService,
) {
super(collectionService, platformUtilsService, i18nService, cipherService, logService);
super(

Check warning on line 30 in apps/browser/src/vault/popup/components/vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/components/vault/collections.component.ts#L30

Added line #L30 was not covered by tests
collectionService,
platformUtilsService,
i18nService,
cipherService,
organizationService,
logService,
);
}

async ngOnInit() {
Expand Down
11 changes: 10 additions & 1 deletion apps/desktop/src/vault/app/vault/collections.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Component } from "@angular/core";

import { CollectionsComponent as BaseCollectionsComponent } from "@bitwarden/angular/admin-console/components/collections.component";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";

Check warning on line 4 in apps/desktop/src/vault/app/vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/vault/app/vault/collections.component.ts#L4

Added line #L4 was not covered by tests
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -17,8 +18,16 @@
i18nService: I18nService,
collectionService: CollectionService,
platformUtilsService: PlatformUtilsService,
organizationService: OrganizationService,
logService: LogService,
) {
super(collectionService, platformUtilsService, i18nService, cipherService, logService);
super(
collectionService,

Check warning on line 25 in apps/desktop/src/vault/app/vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/vault/app/vault/collections.component.ts#L24-L25

Added lines #L24 - L25 were not covered by tests
platformUtilsService,
i18nService,
cipherService,
organizationService,
logService,
);

Check notice on line 31 in apps/desktop/src/vault/app/vault/collections.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Excess Number of Function Arguments

CollectionsComponent.constructor increases from 5 to 6 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ <h3>{{ "collections" | i18n }}</h3>
[(ngModel)]="$any(c).checked"
name="Collection[{{ i }}].Checked"
appStopProp
[disabled]="!c.canEditItems(this.organization, this.flexibleCollectionsV1Enabled)"
/>
</td>
<td>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Component, OnDestroy } from "@angular/core";

import { CollectionsComponent as BaseCollectionsComponent } from "@bitwarden/angular/admin-console/components/collections.component";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -18,16 +19,27 @@
platformUtilsService: PlatformUtilsService,
i18nService: I18nService,
cipherService: CipherService,
organizationSerivce: OrganizationService,
logService: LogService,
) {
super(collectionService, platformUtilsService, i18nService, cipherService, logService);
super(
collectionService,
platformUtilsService,
i18nService,
cipherService,
organizationSerivce,
logService,
);

Check notice on line 32 in apps/web/src/app/vault/individual-vault/collections.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Excess Number of Function Arguments

CollectionsComponent.constructor increases from 5 to 6 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
}

ngOnDestroy() {
this.selectAll(false);
}

check(c: CollectionView, select?: boolean) {
if (!c.canEditItems(this.organization, this.flexibleCollectionsV1Enabled)) {
return;

Check warning on line 41 in apps/web/src/app/vault/individual-vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/individual-vault/collections.component.ts#L41

Added line #L41 was not covered by tests
}
(c as any).checked = select == null ? !(c as any).checked : select;
}

Expand Down
13 changes: 11 additions & 2 deletions apps/web/src/app/vault/org-vault/collections.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Component } from "@angular/core";

import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
Expand All @@ -25,10 +26,18 @@
platformUtilsService: PlatformUtilsService,
i18nService: I18nService,
cipherService: CipherService,
organizationService: OrganizationService,
private apiService: ApiService,
logService: LogService,
) {
super(collectionService, platformUtilsService, i18nService, cipherService, logService);
super(

Check warning on line 33 in apps/web/src/app/vault/org-vault/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/collections.component.ts#L33

Added line #L33 was not covered by tests
collectionService,
platformUtilsService,
i18nService,
cipherService,
organizationService,
logService,
);
this.allowSelectNone = true;
}

Expand All @@ -55,7 +64,7 @@
}

protected saveCollections() {
if (this.organization.canEditAnyCollection) {
if (this.organization.canEditAllCiphers(this.flexibleCollectionsV1Enabled)) {
const request = new CipherCollectionsRequest(this.cipherDomain.collectionIds);
return this.apiService.putCipherCollectionsAdmin(this.cipherId, request);
} else {
Expand Down
33 changes: 29 additions & 4 deletions apps/web/src/app/vault/org-vault/vault.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {

Check notice on line 1 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 5.74 to 5.89, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
ChangeDetectorRef,
Component,
NgZone,
Expand Down Expand Up @@ -137,6 +137,7 @@
protected showCollectionAccessRestricted: boolean;
protected currentSearchText$: Observable<string>;
protected editableCollections$: Observable<CollectionView[]>;
protected allCollectionsWithoutUnassigned$: Observable<CollectionAdminView[]>;
protected showBulkEditCollectionAccess$ = this.configService.getFeatureFlag$(
FeatureFlag.BulkCollectionAccess,
false,
Expand Down Expand Up @@ -253,7 +254,7 @@

this.currentSearchText$ = this.route.queryParams.pipe(map((queryParams) => queryParams.search));

const allCollectionsWithoutUnassigned$ = combineLatest([
this.allCollectionsWithoutUnassigned$ = combineLatest([

Check warning on line 257 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L257

Added line #L257 was not covered by tests
organizationId$.pipe(switchMap((orgId) => this.collectionAdminService.getAll(orgId))),
defer(() => this.collectionService.getAllDecrypted()),
]).pipe(
Expand All @@ -276,7 +277,7 @@
shareReplay({ refCount: true, bufferSize: 1 }),
);

this.editableCollections$ = allCollectionsWithoutUnassigned$.pipe(
this.editableCollections$ = this.allCollectionsWithoutUnassigned$.pipe(

Check warning on line 280 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L280

Added line #L280 was not covered by tests
map((collections) => {
// Users that can edit all ciphers can implicitly edit all collections
if (this.organization.canEditAllCiphers(this.flexibleCollectionsV1Enabled)) {
Expand All @@ -287,7 +288,10 @@
shareReplay({ refCount: true, bufferSize: 1 }),
);

const allCollections$ = combineLatest([organizationId$, allCollectionsWithoutUnassigned$]).pipe(
const allCollections$ = combineLatest([

Check warning on line 291 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L291

Added line #L291 was not covered by tests
organizationId$,
this.allCollectionsWithoutUnassigned$,
]).pipe(

Check warning on line 294 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method

VaultComponent.ngOnInit already has high cyclomatic complexity, and now it increases in Lines of Code from 338 to 341. 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.
map(([organizationId, allCollections]) => {
const noneCollection = new CollectionAdminView();
noneCollection.name = this.i18nService.t("unassigned");
Expand Down Expand Up @@ -680,7 +684,27 @@

if (this.flexibleCollectionsV1Enabled) {
// V1 limits admins to only adding items to collections they have access to.
collections = await firstValueFrom(this.editableCollections$);
collections = await firstValueFrom(

Check warning on line 687 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L687

Added line #L687 was not covered by tests
this.allCollectionsWithoutUnassigned$.pipe(
map((c) => {
return c.sort((a, b) => {

Check warning on line 690 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L690

Added line #L690 was not covered by tests
if (
a.canEditItems(this.organization, true) &&
!b.canEditItems(this.organization, true)
) {
return -1;

Check warning on line 695 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L695

Added line #L695 was not covered by tests
} else if (
!a.canEditItems(this.organization, true) &&
b.canEditItems(this.organization, true)
) {
return 1;

Check warning on line 700 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L700

Added line #L700 was not covered by tests
} else {
return a.name.localeCompare(b.name);

Check warning on line 702 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L702

Added line #L702 was not covered by tests
}
});
}),
),
);
} else {
collections = (await firstValueFrom(this.vaultFilterService.filteredCollections$)).filter(
(c) => !c.readOnly && c.id != Unassigned,
shane-melton marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -690,6 +714,7 @@
CollectionsComponent,
this.collectionsModalRef,
(comp) => {
comp.flexibleCollectionsV1Enabled = this.flexibleCollectionsV1Enabled;

Check warning on line 717 in apps/web/src/app/vault/org-vault/vault.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/vault/org-vault/vault.component.ts#L717

Added line #L717 was not covered by tests
comp.collectionIds = cipher.collectionIds;
comp.collections = collections;
comp.organization = this.organization;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Directive, EventEmitter, Input, OnInit, Output } from "@angular/core";

import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";

Check warning on line 3 in libs/angular/src/admin-console/components/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/admin-console/components/collections.component.ts#L3

Added line #L3 was not covered by tests
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -19,6 +21,8 @@
cipher: CipherView;
collectionIds: string[];
collections: CollectionView[] = [];
organization: Organization;
flexibleCollectionsV1Enabled: boolean;

protected cipherDomain: Cipher;

Expand All @@ -27,6 +31,7 @@
protected platformUtilsService: PlatformUtilsService,
protected i18nService: I18nService,
protected cipherService: CipherService,
protected organizationService: OrganizationService,
private logService: LogService,
) {}

Expand All @@ -48,11 +53,24 @@
(c as any).checked = this.collectionIds != null && this.collectionIds.indexOf(c.id) > -1;
});
}

if (this.organization == null) {
this.organization = this.organizationService.get(this.cipher.organizationId);

Check warning on line 58 in libs/angular/src/admin-console/components/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/admin-console/components/collections.component.ts#L58

Added line #L58 was not covered by tests
}
}

async submit() {
const selectedCollectionIds = this.collections
.filter((c) => !!(c as any).checked)
.filter((c) => {
if (
this.organization?.allowAdminAccessToAllCollectionItems &&
this.organization?.canEditAnyCollection
) {
return !!(c as any).checked;

Check warning on line 69 in libs/angular/src/admin-console/components/collections.component.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/admin-console/components/collections.component.ts#L69

Added line #L69 was not covered by tests
} else {
return !!(c as any).checked && c.readOnly == null;
}
})

Check warning on line 73 in libs/angular/src/admin-console/components/collections.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

CollectionsComponent.submit has a cyclomatic complexity of 9, 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.
.map((c) => c.id);
if (!this.allowSelectNone && selectedCollectionIds.length === 0) {
this.platformUtilsService.showToast(
Expand Down
1 change: 1 addition & 0 deletions libs/common/src/vault/models/domain/collection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe("Collection", () => {
organizationId: "orgId",
readOnly: false,
manage: true,
assigned: true,
});
});
});
23 changes: 23 additions & 0 deletions libs/common/src/vault/models/view/collection.view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
readOnly: boolean = null;
hidePasswords: boolean = null;
manage: boolean = null;
assigned: boolean = null;

constructor(c?: Collection | CollectionAccessDetailsResponse) {
if (!c) {
Expand All @@ -30,7 +31,29 @@
this.readOnly = c.readOnly;
this.hidePasswords = c.hidePasswords;
this.manage = c.manage;
this.assigned = true;
}
if (c instanceof CollectionAccessDetailsResponse) {
this.assigned = c.assigned;

Check warning on line 37 in libs/common/src/vault/models/view/collection.view.ts

View check run for this annotation

Codecov / codecov/patch

libs/common/src/vault/models/view/collection.view.ts#L37

Added line #L37 was not covered by tests
}
}

canEditItems(org: Organization, v1FlexibleCollections: boolean): boolean {
if (org != null && org.id !== this.organizationId) {
throw new Error(

Check warning on line 43 in libs/common/src/vault/models/view/collection.view.ts

View check run for this annotation

Codecov / codecov/patch

libs/common/src/vault/models/view/collection.view.ts#L43

Added line #L43 was not covered by tests
"Id of the organization provided does not match the org id of the collection.",
);
}

if (org?.flexibleCollections) {
return (

Check warning on line 49 in libs/common/src/vault/models/view/collection.view.ts

View check run for this annotation

Codecov / codecov/patch

libs/common/src/vault/models/view/collection.view.ts#L49

Added line #L49 was not covered by tests
org?.canEditAllCiphers(v1FlexibleCollections) ||
this.manage ||
(this.assigned && !this.readOnly)
);
}

return org?.canEditAnyCollection || (org?.canEditAssignedCollections && this.assigned);

Check warning on line 56 in libs/common/src/vault/models/view/collection.view.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

CollectionView.canEditItems has a cyclomatic complexity of 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.
}

// For editing collection details, not the items within it.
Expand Down
Loading