From f75e2c4175c81ca52926c3ce21a3c7f7dbbe6126 Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 19 Apr 2022 11:31:49 +0200 Subject: [PATCH] Proposed refactor of folder domain model --- .../components/folder-add-edit.component.ts | 2 +- common/spec/domain/folder.spec.ts | 36 +---- common/spec/services/folder.service.spec.ts | 130 ++++++++++++++++++ common/src/abstractions/folder.service.ts | 5 +- common/src/importers/bitwardenJsonImporter.ts | 11 +- .../bitwardenPasswordProtectedImporter.ts | 11 +- common/src/models/data/folderData.ts | 23 +++- common/src/models/domain/folder.ts | 35 +---- common/src/models/export/folder.ts | 2 +- common/src/models/response/folderResponse.ts | 11 ++ common/src/models/view/folderView.ts | 10 -- common/src/services/folder.service.ts | 67 ++++----- common/src/services/import.service.ts | 3 +- common/src/services/sync.service.ts | 9 +- 14 files changed, 218 insertions(+), 137 deletions(-) create mode 100644 common/spec/services/folder.service.spec.ts diff --git a/angular/src/components/folder-add-edit.component.ts b/angular/src/components/folder-add-edit.component.ts index ef1fa111..34b19abc 100644 --- a/angular/src/components/folder-add-edit.component.ts +++ b/angular/src/components/folder-add-edit.component.ts @@ -88,7 +88,7 @@ export class FolderAddEditComponent implements OnInit { this.editMode = true; this.title = this.i18nService.t("editFolder"); const folder = await this.folderService.get(this.folderId); - this.folder = await folder.decrypt(); + this.folder = await this.folderService.decrypt(folder); } else { this.title = this.i18nService.t("addFolder"); } diff --git a/common/spec/domain/folder.spec.ts b/common/spec/domain/folder.spec.ts index 64ee40b9..fad01ad9 100644 --- a/common/spec/domain/folder.spec.ts +++ b/common/spec/domain/folder.spec.ts @@ -1,22 +1,13 @@ import { FolderData } from "jslib-common/models/data/folderData"; -import { Folder } from "jslib-common/models/domain/folder"; - -import { mockEnc } from "../utils"; describe("Folder", () => { - let data: FolderData; - - beforeEach(() => { - data = { - id: "id", - userId: "userId", - name: "encName", - revisionDate: "2022-01-31T12:00:00.000Z", - }; - }); - it("Convert", () => { - const field = new Folder(data); + const data = new FolderData(); + data.id = "id"; + data.name = "encName"; + data.revisionDate = "2022-01-31T12:00:00.000Z"; + + const field = data.toFolder(); expect(field).toEqual({ id: "id", @@ -24,19 +15,4 @@ describe("Folder", () => { revisionDate: new Date("2022-01-31T12:00:00.000Z"), }); }); - - it("Decrypt", async () => { - const folder = new Folder(); - folder.id = "id"; - folder.name = mockEnc("encName"); - folder.revisionDate = new Date("2022-01-31T12:00:00.000Z"); - - const view = await folder.decrypt(); - - expect(view).toEqual({ - id: "id", - name: "encName", - revisionDate: new Date("2022-01-31T12:00:00.000Z"), - }); - }); }); diff --git a/common/spec/services/folder.service.spec.ts b/common/spec/services/folder.service.spec.ts new file mode 100644 index 00000000..c401c605 --- /dev/null +++ b/common/spec/services/folder.service.spec.ts @@ -0,0 +1,130 @@ +import Substitute, { SubstituteOf } from "@fluffy-spoon/substitute"; + +import { ApiService } from "jslib-common/abstractions/api.service"; +import { CipherService } from "jslib-common/abstractions/cipher.service"; +import { CryptoService } from "jslib-common/abstractions/crypto.service"; +import { I18nService } from "jslib-common/abstractions/i18n.service"; +import { StateService } from "jslib-common/abstractions/state.service"; +import { FolderData } from "jslib-common/models/data/folderData"; +import { Folder } from "jslib-common/models/domain/folder"; +import { FolderService } from "jslib-common/services/folder.service"; + +import { mockEnc } from "../utils"; + +describe("FolderService", () => { + let stateService: SubstituteOf; + + let folderService: FolderService; + + beforeEach(() => { + const cryptoService = Substitute.for(); + const apiService = Substitute.for(); + const i18nService = Substitute.for(); + const cipherService = Substitute.for(); + stateService = Substitute.for(); + + folderService = new FolderService( + cryptoService, + apiService, + i18nService, + cipherService, + stateService + ); + }); + + describe("clearCache", () => { + it("without userId", async () => { + await folderService.clearCache(null); + + stateService.received(1).setDecryptedFolders(null, { userId: null }); + }); + + it("with userId", async () => { + await folderService.clearCache("userId"); + + stateService.received(1).setDecryptedFolders(null, { userId: "userId" }); + }); + }); + + describe("get", () => { + it("retrieves folder", async () => { + const f = new FolderData(); + f.id = "id"; + f.name = "encName"; + f.revisionDate = "2022-01-31T12:00:00.000Z"; + + stateService.getEncryptedFolders().resolves({ id: f }); + const folder = await folderService.get("id"); + + expect(folder).toEqual({ + id: "id", + name: { encryptedString: "encName", encryptionType: 0 }, + revisionDate: new Date("2022-01-31T12:00:00.000Z"), + }); + }); + + describe("not exist", () => { + it("folders are null", async () => { + stateService.getEncryptedFolders().resolves(null); + const folder = await folderService.get("id"); + + expect(folder).toBeNull(); + }); + + it("folder does not exist", async () => { + const f = new FolderData(); + f.id = "id"; + f.name = "encName"; + f.revisionDate = "2022-01-31T12:00:00.000Z"; + + stateService.getEncryptedFolders().resolves({ id: f }); + const folder = await folderService.get("id2"); + + expect(folder).toBeNull(); + }); + }); + }); + + describe("getAll", () => { + it("retrieves folder", async () => { + const f = new FolderData(); + f.id = "id"; + f.name = "encName"; + f.revisionDate = "2022-01-31T12:00:00.000Z"; + + stateService.getEncryptedFolders().resolves({ id: f }); + const folder = await folderService.getAll(); + + expect(folder).toEqual([ + { + id: "id", + name: { encryptedString: "encName", encryptionType: 0 }, + revisionDate: new Date("2022-01-31T12:00:00.000Z"), + }, + ]); + }); + + it("null", async () => { + stateService.getEncryptedFolders().resolves(null); + const folders = await folderService.getAll(); + + expect(folders).toHaveLength(0); + }); + }); + + it("Decrypt", async () => { + const folder: Folder = { + id: "id", + name: mockEnc("encName"), + revisionDate: new Date("2022-01-31T12:00:00.000Z"), + }; + + const view = await folderService.decrypt(folder); + + expect(view).toEqual({ + id: "id", + name: "encName", + revisionDate: new Date("2022-01-31T12:00:00.000Z"), + }); + }); +}); diff --git a/common/src/abstractions/folder.service.ts b/common/src/abstractions/folder.service.ts index 574cebfc..abd59b66 100644 --- a/common/src/abstractions/folder.service.ts +++ b/common/src/abstractions/folder.service.ts @@ -7,15 +7,16 @@ import { FolderView } from "../models/view/folderView"; export abstract class FolderService { clearCache: (userId?: string) => Promise; encrypt: (model: FolderView, key?: SymmetricCryptoKey) => Promise; + decrypt: (model: Folder, orgId?: string, key?: SymmetricCryptoKey) => Promise; get: (id: string) => Promise; getAll: () => Promise; getAllDecrypted: () => Promise; getAllNested: () => Promise[]>; getNested: (id: string) => Promise>; saveWithServer: (folder: Folder) => Promise; - upsert: (folder: FolderData | FolderData[]) => Promise; + upsert: (folder: FolderData) => Promise; replace: (folders: { [id: string]: FolderData }) => Promise; clear: (userId: string) => Promise; - delete: (id: string | string[]) => Promise; + delete: (id: string) => Promise; deleteWithServer: (id: string) => Promise; } diff --git a/common/src/importers/bitwardenJsonImporter.ts b/common/src/importers/bitwardenJsonImporter.ts index 6caf185e..e2d582a6 100644 --- a/common/src/importers/bitwardenJsonImporter.ts +++ b/common/src/importers/bitwardenJsonImporter.ts @@ -1,3 +1,6 @@ +import { FolderService } from "jslib-common/abstractions/folder.service"; +import { FolderView } from "jslib-common/models/view/folderView"; + import { CryptoService } from "../abstractions/crypto.service"; import { I18nService } from "../abstractions/i18n.service"; import { EncString } from "../models/domain/encString"; @@ -13,7 +16,11 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { private results: any; private result: ImportResult; - constructor(protected cryptoService: CryptoService, protected i18nService: I18nService) { + constructor( + protected cryptoService: CryptoService, + protected i18nService: I18nService, + private folderService: FolderService + ) { super(); } @@ -74,7 +81,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { const folder = FolderWithId.toDomain(f); if (folder != null) { folder.id = null; - const view = await folder.decrypt(); + const view = await this.folderService.decrypt(folder); groupingsMap.set(f.id, this.result.folders.length); this.result.folders.push(view); } diff --git a/common/src/importers/bitwardenPasswordProtectedImporter.ts b/common/src/importers/bitwardenPasswordProtectedImporter.ts index 54da71b5..10055e62 100644 --- a/common/src/importers/bitwardenPasswordProtectedImporter.ts +++ b/common/src/importers/bitwardenPasswordProtectedImporter.ts @@ -1,3 +1,5 @@ +import { FolderService } from "jslib-common/abstractions/folder.service"; + import { CryptoService } from "../abstractions/crypto.service"; import { I18nService } from "../abstractions/i18n.service"; import { KdfType } from "../enums/kdfType"; @@ -21,8 +23,13 @@ interface BitwardenPasswordProtectedFileFormat { export class BitwardenPasswordProtectedImporter extends BitwardenJsonImporter implements Importer { private key: SymmetricCryptoKey; - constructor(cryptoService: CryptoService, i18nService: I18nService, private password: string) { - super(cryptoService, i18nService); + constructor( + cryptoService: CryptoService, + i18nService: I18nService, + folderService: FolderService, + private password: string + ) { + super(cryptoService, i18nService, folderService); } async parse(data: string): Promise { diff --git a/common/src/models/data/folderData.ts b/common/src/models/data/folderData.ts index 459ba4de..8044064b 100644 --- a/common/src/models/data/folderData.ts +++ b/common/src/models/data/folderData.ts @@ -1,15 +1,24 @@ -import { FolderResponse } from "../response/folderResponse"; +import { EncString } from "../domain/encString"; +import { Folder } from "../domain/folder"; export class FolderData { id: string; - userId: string; name: string; revisionDate: string; - constructor(response: FolderResponse, userId: string) { - this.userId = userId; - this.name = response.name; - this.id = response.id; - this.revisionDate = response.revisionDate; + constructor(f?: Folder) { + if (f) { + this.id = f.id; + this.name = f.name.encryptedString; + this.revisionDate = f.revisionDate.toISOString(); + } + } + + toFolder(): Folder { + return { + id: this.id, + name: new EncString(this.name), + revisionDate: new Date(this.revisionDate), + }; } } diff --git a/common/src/models/domain/folder.ts b/common/src/models/domain/folder.ts index 8596e94d..7e3c7127 100644 --- a/common/src/models/domain/folder.ts +++ b/common/src/models/domain/folder.ts @@ -1,40 +1,7 @@ -import { FolderData } from "../data/folderData"; -import { FolderView } from "../view/folderView"; - -import Domain from "./domainBase"; import { EncString } from "./encString"; -export class Folder extends Domain { +export interface Folder { id: string; name: EncString; revisionDate: Date; - - constructor(obj?: FolderData) { - super(); - if (obj == null) { - return; - } - - this.buildDomainModel( - this, - obj, - { - id: null, - name: null, - }, - ["id"] - ); - - this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null; - } - - decrypt(): Promise { - return this.decryptObj( - new FolderView(this), - { - name: null, - }, - null - ); - } } diff --git a/common/src/models/export/folder.ts b/common/src/models/export/folder.ts index e770f326..ef739997 100644 --- a/common/src/models/export/folder.ts +++ b/common/src/models/export/folder.ts @@ -14,7 +14,7 @@ export class Folder { return view; } - static toDomain(req: Folder, domain = new FolderDomain()) { + static toDomain(req: Folder, domain: FolderDomain = {} as FolderDomain) { domain.name = req.name != null ? new EncString(req.name) : null; return domain; } diff --git a/common/src/models/response/folderResponse.ts b/common/src/models/response/folderResponse.ts index 70d4897f..6aa3d96a 100644 --- a/common/src/models/response/folderResponse.ts +++ b/common/src/models/response/folderResponse.ts @@ -1,3 +1,6 @@ +import { EncString } from "../domain/encString"; +import { Folder } from "../domain/folder"; + import { BaseResponse } from "./baseResponse"; export class FolderResponse extends BaseResponse { @@ -11,4 +14,12 @@ export class FolderResponse extends BaseResponse { this.name = this.getResponseProperty("Name"); this.revisionDate = this.getResponseProperty("RevisionDate"); } + + toFolder(): Folder { + return { + id: this.id, + name: new EncString(this.name), + revisionDate: new Date(this.revisionDate), + }; + } } diff --git a/common/src/models/view/folderView.ts b/common/src/models/view/folderView.ts index 731acffb..207332db 100644 --- a/common/src/models/view/folderView.ts +++ b/common/src/models/view/folderView.ts @@ -1,4 +1,3 @@ -import { Folder } from "../domain/folder"; import { ITreeNodeObject } from "../domain/treeNode"; import { View } from "./view"; @@ -7,13 +6,4 @@ export class FolderView implements View, ITreeNodeObject { id: string = null; name: string = null; revisionDate: Date = null; - - constructor(f?: Folder) { - if (!f) { - return; - } - - this.id = f.id; - this.revisionDate = f.revisionDate; - } } diff --git a/common/src/services/folder.service.ts b/common/src/services/folder.service.ts index 666ca6d2..235d711d 100644 --- a/common/src/services/folder.service.ts +++ b/common/src/services/folder.service.ts @@ -31,32 +31,32 @@ export class FolderService implements FolderServiceAbstraction { } async encrypt(model: FolderView, key?: SymmetricCryptoKey): Promise { - const folder = new Folder(); - folder.id = model.id; - folder.name = await this.cryptoService.encrypt(model.name, key); - return folder; + return { + id: model.id, + name: await this.cryptoService.encrypt(model.name, key), + revisionDate: null, + }; + } + + async decrypt(model: Folder, orgId?: string, key?: SymmetricCryptoKey): Promise { + const view = new FolderView(); + view.id = model.id; + view.name = await model.name.decrypt(orgId, key); + view.revisionDate = model.revisionDate; + + return view; } async get(id: string): Promise { const folders = await this.stateService.getEncryptedFolders(); - // eslint-disable-next-line - if (folders == null || !folders.hasOwnProperty(id)) { - return null; - } - return new Folder(folders[id]); + return folders?.[id]?.toFolder() ?? null; } async getAll(): Promise { - const folders = await this.stateService.getEncryptedFolders(); - const response: Folder[] = []; - for (const id in folders) { - // eslint-disable-next-line - if (folders.hasOwnProperty(id)) { - response.push(new Folder(folders[id])); - } - } - return response; + const folders = (await this.stateService.getEncryptedFolders()) ?? {}; + + return Object.values(folders).map((f) => f.toFolder()); } async getAllDecrypted(): Promise { @@ -74,7 +74,7 @@ export class FolderService implements FolderServiceAbstraction { const promises: Promise[] = []; const folders = await this.getAll(); folders.forEach((folder) => { - promises.push(folder.decrypt().then((f) => decFolders.push(f))); + promises.push(this.decrypt(folder).then((f) => decFolders.push(f))); }); await Promise.all(promises); @@ -117,25 +117,17 @@ export class FolderService implements FolderServiceAbstraction { response = await this.apiService.putFolder(folder.id, request); } - const userId = await this.stateService.getUserId(); - const data = new FolderData(response, userId); + const data = new FolderData(response.toFolder()); await this.upsert(data); } - async upsert(folder: FolderData | FolderData[]): Promise { + async upsert(folder: FolderData): Promise { let folders = await this.stateService.getEncryptedFolders(); if (folders == null) { folders = {}; } - if (folder instanceof FolderData) { - const f = folder as FolderData; - folders[f.id] = f; - } else { - (folder as FolderData[]).forEach((f) => { - folders[f.id] = f; - }); - } + folders[folder.id] = folder; await this.stateService.setDecryptedFolders(null); await this.stateService.setEncryptedFolders(folders); @@ -151,22 +143,13 @@ export class FolderService implements FolderServiceAbstraction { await this.stateService.setEncryptedFolders(null, { userId: userId }); } - async delete(id: string | string[]): Promise { + async delete(id: string): Promise { const folders = await this.stateService.getEncryptedFolders(); - if (folders == null) { + if (folders?.[id] == null) { return; } - if (typeof id === "string") { - if (folders[id] == null) { - return; - } - delete folders[id]; - } else { - (id as string[]).forEach((i) => { - delete folders[i]; - }); - } + delete folders[id]; await this.stateService.setDecryptedFolders(null); await this.stateService.setEncryptedFolders(folders); diff --git a/common/src/services/import.service.ts b/common/src/services/import.service.ts index 86226d7d..618fbda9 100644 --- a/common/src/services/import.service.ts +++ b/common/src/services/import.service.ts @@ -163,11 +163,12 @@ export class ImportService implements ImportServiceAbstraction { case "bitwardencsv": return new BitwardenCsvImporter(); case "bitwardenjson": - return new BitwardenJsonImporter(this.cryptoService, this.i18nService); + return new BitwardenJsonImporter(this.cryptoService, this.i18nService, this.folderService); case "bitwardenpasswordprotected": return new BitwardenPasswordProtectedImporter( this.cryptoService, this.i18nService, + this.folderService, password ); case "lastpasscsv": diff --git a/common/src/services/sync.service.ts b/common/src/services/sync.service.ts index 93611238..d733cb05 100644 --- a/common/src/services/sync.service.ts +++ b/common/src/services/sync.service.ts @@ -101,7 +101,7 @@ export class SyncService implements SyncServiceAbstraction { const response = await this.apiService.getSync(); await this.syncProfile(response.profile); - await this.syncFolders(userId, response.folders); + await this.syncFolders(response.folders); await this.syncCollections(response.collections); await this.syncCiphers(userId, response.ciphers); await this.syncSends(userId, response.sends); @@ -130,8 +130,7 @@ export class SyncService implements SyncServiceAbstraction { ) { const remoteFolder = await this.apiService.getFolder(notification.id); if (remoteFolder != null) { - const userId = await this.stateService.getUserId(); - await this.folderService.upsert(new FolderData(remoteFolder, userId)); + await this.folderService.upsert(new FolderData(remoteFolder.toFolder())); this.messagingService.send("syncedUpsertedFolder", { folderId: notification.id }); return this.syncCompleted(true); } @@ -339,10 +338,10 @@ export class SyncService implements SyncServiceAbstraction { } } - private async syncFolders(userId: string, response: FolderResponse[]) { + private async syncFolders(response: FolderResponse[]) { const folders: { [id: string]: FolderData } = {}; response.forEach((f) => { - folders[f.id] = new FolderData(f, userId); + folders[f.id] = new FolderData(f.toFolder()); }); return await this.folderService.replace(folders); }