From 305fd3987152cb2cdcc31d510120f88ac71bbcc6 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Mon, 29 Jan 2024 15:11:19 +0000 Subject: [PATCH] [AC-1782] Flexible collections import behavior in Password Manager and Admin Console (#6888) * Added logic to only return organisations where user has import permissions and collections that he manages on Import * Changed the UnassignedCollections validation logic * Added validation to check if the user is coming from AdminConsole on the import component * Added import collection service abstraction to allow get admin collections * Corrected feature flag reads on import component * Refactor import component methods ngOnInit and performImport to improve codescene Using FeatureFlag Observable * Modified validation to allow import if user has organizations to import into * Using the new organization flexiblecollections property on import * Created collection-admin-import.service to return all the org collections to the import on Admin Console * Small changes on import flexible collections * Fix linting issues * changed canAccessImport rules and deprecated canAccessImportExport * Validating if user canAccessImportExport instead of admin before calling the handleOrganizationImportInit. * AC-2095 - Corrected getAllAdminCollections from ImportCollectionAdminService to properly get all the collections on AdminConsole * Reverting AC-2095 --------- Co-authored-by: Daniel James Smith --- .../organization-settings-routing.module.ts | 4 +- .../tools/import/admin-import.component.ts | 24 ++ .../import/import-collection-admin.service.ts | 14 ++ .../organization.service.abstraction.ts | 15 ++ .../src/components/import.component.ts | 211 ++++++++++-------- .../import-collection.service.abstraction.ts | 5 + libs/importer/src/services/import.service.ts | 5 +- libs/importer/src/services/index.ts | 2 + 8 files changed, 184 insertions(+), 96 deletions(-) create mode 100644 apps/web/src/app/tools/import/admin-import.component.ts create mode 100644 apps/web/src/app/tools/import/import-collection-admin.service.ts create mode 100644 libs/importer/src/services/import-collection.service.abstraction.ts diff --git a/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts b/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts index 76f4a30bd4..e287066965 100644 --- a/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts +++ b/apps/web/src/app/admin-console/organizations/settings/organization-settings-routing.module.ts @@ -49,8 +49,8 @@ const routes: Routes = [ { path: "import", loadComponent: () => - import("../../../tools/import/import-web.component").then( - (mod) => mod.ImportWebComponent, + import("../../../tools/import/admin-import.component").then( + (mod) => mod.AdminImportComponent, ), canActivate: [OrganizationPermissionsGuard], data: { diff --git a/apps/web/src/app/tools/import/admin-import.component.ts b/apps/web/src/app/tools/import/admin-import.component.ts new file mode 100644 index 0000000000..8577f1e2d3 --- /dev/null +++ b/apps/web/src/app/tools/import/admin-import.component.ts @@ -0,0 +1,24 @@ +import { Component } from "@angular/core"; + +import { ImportCollectionServiceAbstraction } from "@bitwarden/importer/core"; +import { ImportComponent } from "@bitwarden/importer/ui"; + +import { SharedModule } from "../../shared"; +import { CollectionAdminService } from "../../vault/core/collection-admin.service"; + +import { ImportCollectionAdminService } from "./import-collection-admin.service"; +import { ImportWebComponent } from "./import-web.component"; + +@Component({ + templateUrl: "import-web.component.html", + standalone: true, + imports: [SharedModule, ImportComponent], + providers: [ + { + provide: ImportCollectionServiceAbstraction, + useClass: ImportCollectionAdminService, + deps: [CollectionAdminService], + }, + ], +}) +export class AdminImportComponent extends ImportWebComponent {} diff --git a/apps/web/src/app/tools/import/import-collection-admin.service.ts b/apps/web/src/app/tools/import/import-collection-admin.service.ts new file mode 100644 index 0000000000..e48f9b27ce --- /dev/null +++ b/apps/web/src/app/tools/import/import-collection-admin.service.ts @@ -0,0 +1,14 @@ +import { Injectable } from "@angular/core"; + +import { ImportCollectionServiceAbstraction } from "../../../../../../libs/importer/src/services/import-collection.service.abstraction"; +import { CollectionAdminService } from "../../vault/core/collection-admin.service"; +import { CollectionAdminView } from "../../vault/core/views/collection-admin.view"; + +@Injectable() +export class ImportCollectionAdminService implements ImportCollectionServiceAbstraction { + constructor(private collectionAdminService: CollectionAdminService) {} + + async getAllAdminCollections(organizationId: string): Promise { + return await this.collectionAdminService.getAll(organizationId); + } +} diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 603ef54b6b..06fe8efc87 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -57,6 +57,10 @@ export function canAccessAdmin(i18nService: I18nService) { ); } +/** + * @deprecated + * To be removed after Flexible Collections. + **/ export function canAccessImportExport(i18nService: I18nService) { return map((orgs) => orgs @@ -65,6 +69,17 @@ export function canAccessImportExport(i18nService: I18nService) { ); } +export function canAccessImport(i18nService: I18nService) { + return map((orgs) => + orgs + .filter( + (org) => + org.canAccessImportExport || (org.canCreateNewCollections && org.flexibleCollections), + ) + .sort(Utils.getSortFunction(i18nService, "name")), + ); +} + /** * Returns `true` if a user is a member of an organization (rather than only being a ProviderUser) * @deprecated Use organizationService.memberOrganizations$ instead diff --git a/libs/importer/src/components/import.component.ts b/libs/importer/src/components/import.component.ts index 4efedbfcfd..ceb21fd516 100644 --- a/libs/importer/src/components/import.component.ts +++ b/libs/importer/src/components/import.component.ts @@ -2,9 +2,11 @@ import { CommonModule } from "@angular/common"; import { Component, EventEmitter, + Inject, Input, OnDestroy, OnInit, + Optional, Output, ViewChild, } from "@angular/core"; @@ -16,7 +18,7 @@ import { filter, map, takeUntil } from "rxjs/operators"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { - canAccessImportExport, + canAccessImport, OrganizationService, } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -50,6 +52,7 @@ import { ImportOption, ImportResult, ImportType } from "../models"; import { ImportApiService, ImportApiServiceAbstraction, + ImportCollectionServiceAbstraction, ImportService, ImportServiceAbstraction, } from "../services"; @@ -129,6 +132,7 @@ export class ImportComponent implements OnInit, OnDestroy { protected destroy$ = new Subject(); private _importBlockedByPolicy = false; + private _isFromAC = false; formGroup = this.formBuilder.group({ vaultSelector: [ @@ -176,9 +180,12 @@ export class ImportComponent implements OnInit, OnDestroy { protected syncService: SyncService, protected dialogService: DialogService, protected folderService: FolderService, - protected collectionService: CollectionService, protected organizationService: OrganizationService, + protected collectionService: CollectionService, protected formBuilder: FormBuilder, + @Inject(ImportCollectionServiceAbstraction) + @Optional() + protected importCollectionService: ImportCollectionServiceAbstraction, ) {} protected get importBlockedByPolicy(): boolean { @@ -200,41 +207,12 @@ export class ImportComponent implements OnInit, OnDestroy { this.setImportOptions(); await this.initializeOrganizations(); - - if (this.organizationId) { - this.formGroup.controls.vaultSelector.patchValue(this.organizationId); - this.formGroup.controls.vaultSelector.disable(); - - this.collections$ = Utils.asyncToObservable(() => - this.collectionService - .getAllDecrypted() - .then((c) => c.filter((c2) => c2.organizationId === this.organizationId)), - ); + if (this.organizationId && this.canAccessImportExport(this.organizationId)) { + this.handleOrganizationImportInit(); } else { - // Filter out the `no folder`-item from folderViews$ - this.folders$ = this.folderService.folderViews$.pipe( - map((folders) => folders.filter((f) => f.id != null)), - ); - this.formGroup.controls.targetSelector.disable(); - - this.formGroup.controls.vaultSelector.valueChanges - .pipe(takeUntil(this.destroy$)) - .subscribe((value) => { - this.organizationId = value != "myVault" ? value : undefined; - if (!this._importBlockedByPolicy) { - this.formGroup.controls.targetSelector.enable(); - } - if (value) { - this.collections$ = Utils.asyncToObservable(() => - this.collectionService - .getAllDecrypted() - .then((c) => c.filter((c2) => c2.organizationId === value)), - ); - } - }); - - this.formGroup.controls.vaultSelector.setValue("myVault"); + this.handleImportInit(); } + this.formGroup.controls.format.valueChanges .pipe(takeUntil(this.destroy$)) .subscribe((value) => { @@ -244,10 +222,58 @@ export class ImportComponent implements OnInit, OnDestroy { await this.handlePolicies(); } + private handleOrganizationImportInit() { + this.formGroup.controls.vaultSelector.patchValue(this.organizationId); + this.formGroup.controls.vaultSelector.disable(); + + this.collections$ = Utils.asyncToObservable(() => + this.importCollectionService + .getAllAdminCollections(this.organizationId) + .then((collections) => collections.sort(Utils.getSortFunction(this.i18nService, "name"))), + ); + + this._isFromAC = true; + } + + private handleImportInit() { + // Filter out the no folder-item from folderViews$ + this.folders$ = this.folderService.folderViews$.pipe( + map((folders) => folders.filter((f) => f.id != null)), + ); + + this.formGroup.controls.targetSelector.disable(); + + combineLatest([this.formGroup.controls.vaultSelector.valueChanges, this.organizations$]) + .pipe(takeUntil(this.destroy$)) + .subscribe(([value, organizations]) => { + this.organizationId = value !== "myVault" ? value : undefined; + + if (!this._importBlockedByPolicy) { + this.formGroup.controls.targetSelector.enable(); + } + const flexCollectionEnabled = + organizations.find((x) => x.id == this.organizationId)?.flexibleCollections ?? false; + if (value) { + this.collections$ = Utils.asyncToObservable(() => + this.collectionService + .getAllDecrypted() + .then((decryptedCollections) => + decryptedCollections + .filter( + (c2) => c2.organizationId === value && (!flexCollectionEnabled || c2.manage), + ) + .sort(Utils.getSortFunction(this.i18nService, "name")), + ), + ); + } + }); + this.formGroup.controls.vaultSelector.setValue("myVault"); + } + private async initializeOrganizations() { this.organizations$ = concat( this.organizationService.memberOrganizations$.pipe( - canAccessImportExport(this.i18nService), + canAccessImport(this.i18nService), map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name"))), ), ); @@ -293,24 +319,7 @@ export class ImportComponent implements OnInit, OnDestroy { } protected async performImport() { - if (this.organization) { - const confirmed = await this.dialogService.openSimpleDialog({ - title: { key: "warning" }, - content: { key: "importWarning", placeholders: [this.organization.name] }, - type: "warning", - }); - - if (!confirmed) { - return; - } - } - - if (this.importBlockedByPolicy && this.organizationId == null) { - this.platformUtilsService.showToast( - "error", - null, - this.i18nService.t("personalOwnershipPolicyInEffectImports"), - ); + if (!(await this.validateImport())) { return; } @@ -333,10 +342,9 @@ export class ImportComponent implements OnInit, OnDestroy { return; } - const fileEl = document.getElementById("import_input_file") as HTMLInputElement; - const files = fileEl.files; - let fileContents = this.formGroup.controls.fileContents.value; - if ((files == null || files.length === 0) && (fileContents == null || fileContents === "")) { + const importContents = await this.setImportContents(); + + if (importContents == null || importContents === "") { this.platformUtilsService.showToast( "error", this.i18nService.t("errorOccurred"), @@ -345,37 +353,13 @@ export class ImportComponent implements OnInit, OnDestroy { return; } - if (files != null && files.length > 0) { - try { - const content = await this.getFileContents(files[0]); - if (content != null) { - fileContents = content; - } - } catch (e) { - this.logService.error(e); - } - } - - if (fileContents == null || fileContents === "") { - this.platformUtilsService.showToast( - "error", - this.i18nService.t("errorOccurred"), - this.i18nService.t("selectFile"), - ); - return; - } - - if (this.organizationId) { - await this.organizationService.get(this.organizationId)?.isAdmin; - } - try { const result = await this.importService.import( importer, - fileContents, + importContents, this.organizationId, this.formGroup.controls.targetSelector.value, - this.canAccessImportExport(this.organizationId), + this.canAccessImportExport(this.organizationId) && this._isFromAC, ); //No errors, display success message @@ -393,13 +377,6 @@ export class ImportComponent implements OnInit, OnDestroy { } } - private isUserAdmin(organizationId?: string): boolean { - if (!organizationId) { - return false; - } - return this.organizationService.get(this.organizationId)?.isAdmin; - } - private canAccessImportExport(organizationId?: string): boolean { if (!organizationId) { return false; @@ -507,6 +484,58 @@ export class ImportComponent implements OnInit, OnDestroy { return await lastValueFrom(dialog.closed); } + private async validateImport(): Promise { + if (this.organization) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "warning" }, + content: { key: "importWarning", placeholders: [this.organization.name] }, + type: "warning", + }); + + if (!confirmed) { + return false; + } + } + + if (this.importBlockedByPolicy && this.organizationId == null) { + this.platformUtilsService.showToast( + "error", + null, + this.i18nService.t("personalOwnershipPolicyInEffectImports"), + ); + return false; + } + + return true; + } + + private async setImportContents(): Promise { + const fileEl = document.getElementById("import_input_file") as HTMLInputElement; + const files = fileEl.files; + let fileContents = this.formGroup.controls.fileContents.value; + if ((files == null || files.length === 0) && (fileContents == null || fileContents === "")) { + this.platformUtilsService.showToast( + "error", + this.i18nService.t("errorOccurred"), + this.i18nService.t("selectFile"), + ); + return; + } + + if (files != null && files.length > 0) { + try { + const content = await this.getFileContents(files[0]); + if (content != null) { + fileContents = content; + } + } catch (e) { + this.logService.error(e); + } + } + + return fileContents; + } + ngOnDestroy(): void { this.destroy$.next(); this.destroy$.complete(); diff --git a/libs/importer/src/services/import-collection.service.abstraction.ts b/libs/importer/src/services/import-collection.service.abstraction.ts new file mode 100644 index 0000000000..68b7e5f192 --- /dev/null +++ b/libs/importer/src/services/import-collection.service.abstraction.ts @@ -0,0 +1,5 @@ +import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; + +export abstract class ImportCollectionServiceAbstraction { + getAllAdminCollections: (organizationId: string) => Promise; +} diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index 25aae1a58c..e26b768ab6 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -152,9 +152,8 @@ export class ImportService implements ImportServiceAbstraction { Utils.isNullOrWhitespace(selectedImportTarget) && !canAccessImportExport ) { - const hasUnassignedCollections = importResult.ciphers.some( - (c) => !Array.isArray(c.collectionIds) || c.collectionIds.length == 0, - ); + const hasUnassignedCollections = + importResult.collectionRelationships.length < importResult.ciphers.length; if (hasUnassignedCollections) { throw new Error(this.i18nService.t("importUnassignedItemsError")); } diff --git a/libs/importer/src/services/index.ts b/libs/importer/src/services/index.ts index 7b1244867f..13568f886c 100644 --- a/libs/importer/src/services/index.ts +++ b/libs/importer/src/services/index.ts @@ -3,3 +3,5 @@ export { ImportApiService } from "./import-api.service"; export { ImportServiceAbstraction } from "./import.service.abstraction"; export { ImportService } from "./import.service"; + +export { ImportCollectionServiceAbstraction } from "./import-collection.service.abstraction";