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

[ADR-16] Refactor Folders #3732

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
53574c4
Random test
Hinton Oct 8, 2022
d93b6b9
Showcase how we can fetch either the user or organization key
Hinton Oct 8, 2022
2922aa4
WIP encrypt
Hinton Oct 14, 2022
1ecc318
Typescript madness
Hinton Oct 14, 2022
e2cb8fd
Update other clients
Hinton Oct 14, 2022
9e53267
Refactor with discussed changes
Hinton Oct 31, 2022
3920edb
Merge branch 'master' of github.com:bitwarden/clients into feature/de…
Hinton Oct 31, 2022
3e22060
Refactor
Hinton Oct 31, 2022
0df5f41
Rename
Hinton Oct 31, 2022
ceba8a9
Fix encrypt/decrypt tests
Hinton Oct 31, 2022
d312a54
Fix last test
Hinton Oct 31, 2022
cfa927a
Remove constructor
Hinton Nov 4, 2022
b0283ad
Merge branch 'master' of github.com:bitwarden/clients into feature/de…
Hinton Nov 18, 2022
750aa42
Add nullable factory
Hinton Nov 18, 2022
d0bf436
Resolve review feedback
Hinton Nov 18, 2022
08e7166
Undo change to property
Hinton Nov 18, 2022
dc8833e
return null
Hinton Nov 18, 2022
96784fd
Fix null error
Hinton Nov 20, 2022
eeb5799
Merge branch 'master' of github.com:bitwarden/clients into feature/de…
Hinton Nov 22, 2022
3298d41
Move logic to encrypt service
Hinton Nov 22, 2022
264b7c2
Fix tests
Hinton Nov 22, 2022
7e7714b
Fix tests
Hinton Nov 28, 2022
cbc08fc
Fix cli
Hinton Nov 28, 2022
bd86146
Resolve comments
Hinton Dec 1, 2022
e298cb1
Showcase multithreaded decryption
Hinton Dec 1, 2022
30ddb42
Remove patch
Hinton Dec 1, 2022
0b58d3d
Revert "Showcase multithreaded decryption"
Hinton Dec 2, 2022
4677176
Fix cli
Hinton Dec 2, 2022
bb46b75
Swap to nullableFactory
Hinton Dec 2, 2022
3eec7ba
Resolve review feedback
Hinton Dec 10, 2022
43850ad
Merge branch 'master' of github.com:bitwarden/clients into feature/de…
Hinton Jan 12, 2023
84ff880
Resolve review feedback
Hinton Jan 12, 2023
ae4cc31
Fix test
Hinton Jan 12, 2023
d73f480
Fix cli build error
Hinton Jan 12, 2023
e69c8dc
Fix browser
Hinton Jan 12, 2023
4fc30c6
Merge branch 'master' of github.com:bitwarden/clients into feature/de…
Hinton Jan 27, 2023
ca685a6
Move specs to live next to source
Hinton Jan 27, 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: 0 additions & 1 deletion .github/whitelist-capital-letters.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
./libs/common/spec/shared/interceptConsole.ts
./libs/common/spec/models/view/passwordHistoryView.spec.ts
./libs/common/spec/models/view/cipherView.spec.ts
./libs/common/spec/models/view/folderView.spec.ts
./libs/common/spec/models/view/attachmentView.spec.ts
./libs/common/spec/models/view/loginView.spec.ts
./libs/common/spec/models/domain/loginUri.spec.ts
Expand Down
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"cSpell.words": ["Popout", "Reprompt", "takeuntil"]
"cSpell.words": ["Decryptable", "Encryptable", "Popout", "Reprompt", "takeuntil"]
}
6 changes: 5 additions & 1 deletion apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,11 @@ export default class MainBackground {
this.cipherService,
this.stateService
);
this.folderApiService = new FolderApiService(this.folderService, this.apiService);
this.folderApiService = new FolderApiService(
this.folderService,
this.cryptoService,
this.apiService
);
this.collectionService = new CollectionService(
this.cryptoService,
this.i18nService,
Expand Down
13 changes: 11 additions & 2 deletions apps/browser/src/popup/settings/folder-add-edit.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ActivatedRoute, Router } from "@angular/router";
import { first } from "rxjs/operators";

import { FolderAddEditComponent as BaseFolderAddEditComponent } from "@bitwarden/angular/components/folder-add-edit.component";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { FolderApiServiceAbstraction } from "@bitwarden/common/abstractions/folder/folder-api.service.abstraction";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
Expand All @@ -22,9 +23,17 @@ export class FolderAddEditComponent extends BaseFolderAddEditComponent {
platformUtilsService: PlatformUtilsService,
private router: Router,
private route: ActivatedRoute,
logService: LogService
logService: LogService,
cryptoService: CryptoService
) {
super(folderService, folderApiService, i18nService, platformUtilsService, logService);
super(
folderService,
folderApiService,
i18nService,
platformUtilsService,
logService,
cryptoService
);
}

async ngOnInit() {
Expand Down
6 changes: 5 additions & 1 deletion apps/cli/src/bw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,11 @@ export class Main {
this.stateService
);

this.folderApiService = new FolderApiService(this.folderService, this.apiService);
this.folderApiService = new FolderApiService(
this.folderService,
this.cryptoService,
this.apiService
);

this.collectionService = new CollectionService(
this.cryptoService,
Expand Down
9 changes: 5 additions & 4 deletions apps/cli/src/commands/create.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { CollectionExport } from "@bitwarden/common/models/export/collection.exp
import { FolderExport } from "@bitwarden/common/models/export/folder.export";
import { CollectionRequest } from "@bitwarden/common/models/request/collection.request";
import { SelectionReadOnlyRequest } from "@bitwarden/common/models/request/selection-read-only.request";
import { FolderView } from "@bitwarden/common/models/view/folder.view";

import { OrganizationCollectionRequest } from "../models/request/organization-collection.request";
import { Response } from "../models/response";
Expand Down Expand Up @@ -148,11 +149,11 @@ export class CreateCommand {
}

private async createFolder(req: FolderExport) {
const folder = await this.folderService.encrypt(FolderExport.toView(req));
const folderView = FolderExport.toView(req);
try {
await this.folderApiService.save(folder);
const newFolder = await this.folderService.get(folder.id);
const decFolder = await newFolder.decrypt();
await this.folderApiService.save(folderView);
const newFolder = await this.folderService.get(folderView.id);
const decFolder = await this.cryptoService.decryptDomain(FolderView, newFolder);
const res = new FolderResponse(decFolder);
return Response.success(res);
} catch (e) {
Expand Down
8 changes: 4 additions & 4 deletions apps/cli/src/commands/edit.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { CollectionExport } from "@bitwarden/common/models/export/collection.exp
import { FolderExport } from "@bitwarden/common/models/export/folder.export";
import { CollectionRequest } from "@bitwarden/common/models/request/collection.request";
import { SelectionReadOnlyRequest } from "@bitwarden/common/models/request/selection-read-only.request";
import { FolderView } from "@bitwarden/common/models/view/folder.view";

import { OrganizationCollectionRequest } from "../models/request/organization-collection.request";
import { Response } from "../models/response";
Expand Down Expand Up @@ -123,13 +124,12 @@ export class EditCommand {
return Response.notFound();
}

let folderView = await folder.decrypt();
let folderView = await this.cryptoService.decryptDomain(FolderView, folder);
folderView = FolderExport.toView(req, folderView);
const encFolder = await this.folderService.encrypt(folderView);
try {
await this.folderApiService.save(encFolder);
await this.folderApiService.save(folderView);
const updatedFolder = await this.folderService.get(folder.id);
const decFolder = await updatedFolder.decrypt();
const decFolder = await this.cryptoService.decryptDomain(FolderView, updatedFolder);
const res = new FolderResponse(decFolder);
return Response.success(res);
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion apps/cli/src/commands/get.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ export class GetCommand extends DownloadCommand {
if (Utils.isGuid(id)) {
const folder = await this.folderService.getFromState(id);
if (folder != null) {
decFolder = await folder.decrypt();
decFolder = await this.cryptoService.decryptDomain(FolderView, folder);
}
} else if (id.trim() !== "") {
let folders = await this.folderService.getAllDecryptedFromState();
Expand Down
13 changes: 11 additions & 2 deletions apps/desktop/src/app/vault/folder-add-edit.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Component } from "@angular/core";

import { FolderAddEditComponent as BaseFolderAddEditComponent } from "@bitwarden/angular/components/folder-add-edit.component";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { FolderApiServiceAbstraction } from "@bitwarden/common/abstractions/folder/folder-api.service.abstraction";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
Expand All @@ -17,8 +18,16 @@ export class FolderAddEditComponent extends BaseFolderAddEditComponent {
folderApiService: FolderApiServiceAbstraction,
i18nService: I18nService,
platformUtilsService: PlatformUtilsService,
logService: LogService
logService: LogService,
cryptoService: CryptoService
) {
super(folderService, folderApiService, i18nService, platformUtilsService, logService);
super(
folderService,
folderApiService,
i18nService,
platformUtilsService,
logService,
cryptoService
);
}
}
6 changes: 4 additions & 2 deletions apps/web/src/app/settings/change-password.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ChangePasswordComponent as BaseChangePasswordComponent } from "@bitward
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { EncryptService } from "@bitwarden/common/abstractions/encrypt.service";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { KeyConnectorService } from "@bitwarden/common/abstractions/keyConnector.service";
Expand Down Expand Up @@ -57,7 +58,8 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent {
private keyConnectorService: KeyConnectorService,
private router: Router,
private organizationApiService: OrganizationApiServiceAbstraction,
private organizationUserService: OrganizationUserService
private organizationUserService: OrganizationUserService,
private encryptService: EncryptService
) {
super(
i18nService,
Expand Down Expand Up @@ -206,7 +208,7 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent {
if (folders[i].id == null) {
continue;
}
const folder = await this.folderService.encrypt(folders[i], encKey[0]);
const folder = await folders[i].encrypt(this.encryptService, encKey[0]);
request.folders.push(new FolderWithIdRequest(folder));
}

Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/app/settings/update-key.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { firstValueFrom } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { EncryptService } from "@bitwarden/common/abstractions/encrypt.service";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/abstractions/log.service";
Expand Down Expand Up @@ -32,7 +33,8 @@ export class UpdateKeyComponent {
private syncService: SyncService,
private folderService: FolderService,
private cipherService: CipherService,
private logService: LogService
private logService: LogService,
private encryptService: EncryptService
) {}

async submit() {
Expand Down Expand Up @@ -87,7 +89,7 @@ export class UpdateKeyComponent {
if (folders[i].id == null) {
continue;
}
const folder = await this.folderService.encrypt(folders[i], encKey[0]);
const folder = await folders[i].encrypt(this.encryptService, encKey[0]);
request.folders.push(new FolderWithIdRequest(folder));
}

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

import { FolderAddEditComponent as BaseFolderAddEditComponent } from "@bitwarden/angular/components/folder-add-edit.component";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { FolderApiServiceAbstraction } from "@bitwarden/common/abstractions/folder/folder-api.service.abstraction";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
Expand All @@ -18,8 +19,16 @@ export class FolderAddEditComponent extends BaseFolderAddEditComponent {
folderApiService: FolderApiServiceAbstraction,
i18nService: I18nService,
platformUtilsService: PlatformUtilsService,
logService: LogService
logService: LogService,
cryptoService: CryptoService
) {
super(folderService, folderApiService, i18nService, platformUtilsService, logService);
super(
folderService,
folderApiService,
i18nService,
platformUtilsService,
logService,
cryptoService
);
}
}
9 changes: 5 additions & 4 deletions libs/angular/src/components/folder-add-edit.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Directive, EventEmitter, Input, OnInit, Output } from "@angular/core";

import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { FolderApiServiceAbstraction } from "@bitwarden/common/abstractions/folder/folder-api.service.abstraction";
import { FolderService } from "@bitwarden/common/abstractions/folder/folder.service.abstraction";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
Expand All @@ -25,7 +26,8 @@ export class FolderAddEditComponent implements OnInit {
protected folderApiService: FolderApiServiceAbstraction,
protected i18nService: I18nService,
protected platformUtilsService: PlatformUtilsService,
private logService: LogService
private logService: LogService,
private cryptoService: CryptoService
) {}

async ngOnInit() {
Expand All @@ -43,8 +45,7 @@ export class FolderAddEditComponent implements OnInit {
}

try {
const folder = await this.folderService.encrypt(this.folder);
this.formPromise = this.folderApiService.save(folder);
this.formPromise = this.folderApiService.save(this.folder);
await this.formPromise;
this.platformUtilsService.showToast(
"success",
Expand Down Expand Up @@ -93,7 +94,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.cryptoService.decryptDomain(FolderView, folder);
} else {
this.title = this.i18nService.t("addFolder");
}
Expand Down
22 changes: 0 additions & 22 deletions libs/common/spec/models/view/folderView.spec.ts

This file was deleted.

40 changes: 12 additions & 28 deletions libs/common/spec/services/folder.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { EncryptService } from "@bitwarden/common/abstractions/encrypt.service";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { FolderData } from "@bitwarden/common/models/data/folder.data";
import { EncString } from "@bitwarden/common/models/domain/enc-string";
import { Folder } from "@bitwarden/common/models/domain/folder";
import { FolderView } from "@bitwarden/common/models/view/folder.view";
import { ContainerService } from "@bitwarden/common/services/container.service";
import { FolderService } from "@bitwarden/common/services/folder/folder.service";
Expand All @@ -33,6 +33,13 @@ describe("Folder Service", () => {
activeAccount = new BehaviorSubject("123");
activeAccountUnlocked = new BehaviorSubject(true);

cryptoService.decryptDomain(Arg.any(), Arg.any()).mimicks((view: any, model: Folder) => {
const v = new FolderView();
v.id = model.id;
v.revisionDate = model.revisionDate;
return Promise.resolve(v);
});

stateService.getEncryptedFolders().resolves({
"1": folderData("1", "test"),
});
Expand All @@ -43,33 +50,13 @@ describe("Folder Service", () => {
folderService = new FolderService(cryptoService, i18nService, cipherService, stateService);
});

it("encrypt", async () => {
const model = new FolderView();
model.id = "2";
model.name = "Test Folder";

cryptoService.encrypt(Arg.any()).resolves(new EncString("ENC"));
cryptoService.decryptToUtf8(Arg.any()).resolves("DEC");

const result = await folderService.encrypt(model);

expect(result).toEqual({
id: "2",
name: {
encryptedString: "ENC",
encryptionType: 0,
},
});
});

describe("get", () => {
it("exists", async () => {
const result = await folderService.get("1");

expect(result).toEqual({
id: "1",
name: {
decryptedValue: [],
encryptedString: "test",
encryptionType: 0,
},
Expand All @@ -91,7 +78,6 @@ describe("Folder Service", () => {
{
id: "1",
name: {
decryptedValue: [],
encryptedString: "test",
encryptionType: 0,
},
Expand All @@ -100,7 +86,6 @@ describe("Folder Service", () => {
{
id: "2",
name: {
decryptedValue: [],
encryptedString: "test 2",
encryptionType: 0,
},
Expand All @@ -109,8 +94,8 @@ describe("Folder Service", () => {
]);

expect(await firstValueFrom(folderService.folderViews$)).toEqual([
{ id: "1", name: [], revisionDate: null },
{ id: "2", name: [], revisionDate: null },
{ id: "1", name: null, revisionDate: null },
{ id: "2", name: null, revisionDate: null },
{ id: null, name: [], revisionDate: null },
]);
});
Expand All @@ -122,7 +107,6 @@ describe("Folder Service", () => {
{
id: "2",
name: {
decryptedValue: [],
encryptedString: "test 2",
encryptionType: 0,
},
Expand All @@ -131,8 +115,8 @@ describe("Folder Service", () => {
]);

expect(await firstValueFrom(folderService.folderViews$)).toEqual([
{ id: "2", name: [], revisionDate: null },
{ id: null, name: [], revisionDate: null },
{ id: "2", name: null, revisionDate: null },
{ id: null, name: null, revisionDate: null },
]);
});

Expand Down
Loading