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 4 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
2 changes: 1 addition & 1 deletion apps/cli/src/commands/create.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class CreateCommand {
try {
await this.folderApiService.save(folder);
const newFolder = await this.folderService.get(folder.id);
const decFolder = await this.cryptoService.decryptView(FolderView, newFolder);
const decFolder = await this.cryptoService.decryptDomain(FolderView, newFolder);
const res = new FolderResponse(decFolder);
return Response.success(res);
} catch (e) {
Expand Down
4 changes: 2 additions & 2 deletions apps/cli/src/commands/edit.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ export class EditCommand {
return Response.notFound();
}

let folderView = await this.cryptoService.decryptView(FolderView, folder);
let folderView = await this.cryptoService.decryptDomain(FolderView, folder);
folderView = FolderExport.toView(req, folderView);
const encFolder = await this.cryptoService.encryptView(folderView);
try {
await this.folderApiService.save(encFolder);
Hinton marked this conversation as resolved.
Show resolved Hide resolved
const updatedFolder = await this.folderService.get(folder.id);
const decFolder = await this.cryptoService.decryptView(FolderView, updatedFolder);
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 this.cryptoService.decryptView(FolderView, folder);
decFolder = await this.cryptoService.decryptDomain(FolderView, folder);
}
} else if (id.trim() !== "") {
let folders = await this.folderService.getAllDecryptedFromState();
Expand Down
8 changes: 4 additions & 4 deletions libs/common/src/abstractions/encrypt.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Encryptable,
EncryptableDomain,
} from "../interfaces/crypto.interface";
import { OldDecryptable } from "../interfaces/decryptable.interface";
import { InitializerMetadata } from "../interfaces/initializer-metadata.interface";
import { EncArrayBuffer } from "../models/domain/enc-array-buffer";
import { EncString } from "../models/domain/enc-string";
Expand All @@ -19,11 +20,10 @@ export abstract class EncryptService {
abstract decryptToUtf8: (encString: EncString, key: SymmetricCryptoKey) => Promise<string>;
abstract decryptToBytes: (encThing: IEncrypted, key: SymmetricCryptoKey) => Promise<ArrayBuffer>;
abstract resolveLegacyKey: (key: SymmetricCryptoKey, encThing: IEncrypted) => SymmetricCryptoKey;
abstract decryptDomains: <V, D extends DecryptableDomain & InitializerMetadata>(
view: Decryptable<V, D> & InitializerMetadata,
items: D[],
abstract decryptItems: <T extends InitializerMetadata>(
items: OldDecryptable<T>[],
key: SymmetricCryptoKey
) => Promise<V[]>;
) => Promise<T[]>;

abstract encryptView: <V extends Encryptable<EncryptableDomain<V>>>(
view: V,
Expand Down
40 changes: 23 additions & 17 deletions libs/common/src/models/domain/cipher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Jsonify } from "type-fest";

import { CipherRepromptType } from "../../enums/cipherRepromptType";
import { CipherType } from "../../enums/cipherType";
import { DecryptableDomain, nullableFactory } from "../../interfaces/crypto.interface";
import { OldDecryptable } from "../../interfaces/decryptable.interface";
import { InitializerKey } from "../../services/cryptography/initializer-key";
import { CipherData } from "../data/cipher.data";
import { LocalData } from "../data/local.data";
Expand All @@ -19,11 +19,11 @@ import { Password } from "./password";
import { SecureNote } from "./secure-note";
import { SymmetricCryptoKey } from "./symmetric-crypto-key";

export class Cipher extends Domain implements DecryptableDomain {
export class Cipher extends Domain implements OldDecryptable<CipherView> {
readonly initializerKey = InitializerKey.Cipher;

id: string;
organizationId?: string;
organizationId: string;
folderId: string;
name: EncString;
notes: EncString;
Expand Down Expand Up @@ -52,11 +52,19 @@ export class Cipher extends Domain implements DecryptableDomain {
return;
}

this.id = obj.id;
this.organizationId = obj.organizationId;
this.folderId = obj.folderId;
this.name = nullableFactory(EncString, obj.name);
this.notes = nullableFactory(EncString, obj.notes);
this.buildDomainModel(
this,
obj,
{
id: null,
organizationId: null,
folderId: null,
name: null,
notes: null,
},
["id", "organizationId", "folderId"]
);

this.type = obj.type;
this.favorite = obj.favorite;
this.organizationUseTotp = obj.organizationUseTotp;
Expand All @@ -66,11 +74,11 @@ export class Cipher extends Domain implements DecryptableDomain {
} else {
this.viewPassword = true; // Default for already synced Ciphers without viewPassword
}
this.revisionDate = nullableFactory(Date, obj.revisionDate);
this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null;
this.collectionIds = obj.collectionIds;
this.localData = localData;
this.creationDate = nullableFactory(Date, obj.creationDate);
this.deletedDate = nullableFactory(Date, obj.deletedDate);
this.creationDate = obj.creationDate != null ? new Date(obj.creationDate) : null;
this.deletedDate = obj.deletedDate != null ? new Date(obj.deletedDate) : null;
this.reprompt = obj.reprompt;

switch (this.type) {
Expand Down Expand Up @@ -109,10 +117,6 @@ export class Cipher extends Domain implements DecryptableDomain {
}
}

keyIdentifier(): string {
return this.organizationId;
}

async decrypt(encKey?: SymmetricCryptoKey): Promise<CipherView> {
const model = new CipherView(this);

Expand Down Expand Up @@ -206,8 +210,10 @@ export class Cipher extends Domain implements DecryptableDomain {
c.deletedDate = this.deletedDate != null ? this.deletedDate.toISOString() : null;
c.reprompt = this.reprompt;

c.name = this.name?.encryptedString;
c.notes = this.notes?.encryptedString;
this.buildDataModel(this, c, {
name: null,
notes: null,
});

switch (c.type) {
case CipherType.Login:
Expand Down
6 changes: 4 additions & 2 deletions libs/common/src/models/domain/folder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export class Folder implements DecryptableDomain {
}

static fromJSON(obj: Jsonify<Folder>) {
const revisionDate = nullableFactory(Date, obj.revisionDate);
return Object.assign(new Folder(), obj, { name: EncString.fromJSON(obj.name), revisionDate });
return Object.assign(new Folder(), obj, {
Hinton marked this conversation as resolved.
Show resolved Hide resolved
name: nullableFactory(EncString, obj.name),
revisionDate: nullableFactory(Date, obj.revisionDate),
});
}
}
43 changes: 4 additions & 39 deletions libs/common/src/models/view/cipher.view.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { Jsonify } from "type-fest";

import { EncryptService } from "../../abstractions/encrypt.service";
import { CipherRepromptType } from "../../enums/cipherRepromptType";
import { CipherType } from "../../enums/cipherType";
import { LinkedIdType } from "../../enums/linkedIdType";
import { Encryptable } from "../../interfaces/crypto.interface";
import { InitializerMetadata } from "../../interfaces/initializer-metadata.interface";
import { InitializerKey } from "../../services/cryptography/initializer-key";
import { LocalData } from "../data/local.data";
import { Cipher } from "../domain/cipher";
import { SymmetricCryptoKey } from "../domain/symmetric-crypto-key";

import { AttachmentView } from "./attachment.view";
import { CardView } from "./card.view";
Expand All @@ -18,13 +15,13 @@ import { IdentityView } from "./identity.view";
import { LoginView } from "./login.view";
import { PasswordHistoryView } from "./password-history.view";
import { SecureNoteView } from "./secure-note.view";
import { View } from "./view";

export class CipherView implements Encryptable<Cipher>, InitializerMetadata {
export class CipherView implements View, InitializerMetadata {
readonly initializerKey = InitializerKey.CipherView;
static readonly initializerKey = InitializerKey.CipherView;

id: string = null;
organizationId?: string = null;
organizationId: string = null;
folderId: string = null;
name: string = null;
notes: string = null;
Expand Down Expand Up @@ -52,7 +49,6 @@ export class CipherView implements Encryptable<Cipher>, InitializerMetadata {
return;
}

// TODO: Remove this
this.id = c.id;
this.organizationId = c.organizationId;
this.folderId = c.folderId;
Expand All @@ -66,39 +62,8 @@ export class CipherView implements Encryptable<Cipher>, InitializerMetadata {
this.revisionDate = c.revisionDate;
this.creationDate = c.creationDate;
this.deletedDate = c.deletedDate;
}

encrypt(encryptService: EncryptService, key: SymmetricCryptoKey): Promise<Cipher> {
throw new Error("Method not implemented.");
}

keyIdentifier(): string {
return this.organizationId;
}

static async decrypt(encryptService: EncryptService, key: SymmetricCryptoKey, model: Cipher) {
const view = new CipherView(model);

view.id = model.id;
view.organizationId = model.organizationId;
view.folderId = model.folderId;
view.favorite = model.favorite;
view.organizationUseTotp = model.organizationUseTotp;
view.edit = model.edit;
view.viewPassword = model.viewPassword;
view.type = model.type;
view.localData = model.localData;
view.collectionIds = model.collectionIds;
view.revisionDate = model.revisionDate;
view.creationDate = model.creationDate;
view.deletedDate = model.deletedDate;
// Old locally stored ciphers might have reprompt == null. If so set it to None.
view.reprompt = model.reprompt ?? CipherRepromptType.None;

view.name = await model.name?.decryptWithEncryptService(encryptService, key);
// TODO: Add remaining fields

return view;
this.reprompt = c.reprompt ?? CipherRepromptType.None;
}

private get item() {
Expand Down
7 changes: 4 additions & 3 deletions libs/common/src/models/view/folder.view.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Jsonify } from "type-fest";

import { EncryptService } from "../../abstractions/encrypt.service";
import { Encryptable } from "../../interfaces/crypto.interface";
import { Encryptable, nullableFactory } from "../../interfaces/crypto.interface";
import { Folder } from "../domain/folder";
import { SymmetricCryptoKey } from "../domain/symmetric-crypto-key";
import { ITreeNodeObject } from "../domain/tree-node";
Expand Down Expand Up @@ -36,7 +36,8 @@ export class FolderView implements ITreeNodeObject, Encryptable<Folder> {
}

static fromJSON(obj: Jsonify<FolderView>) {
const revisionDate = obj.revisionDate == null ? null : new Date(obj.revisionDate);
return Object.assign(new FolderView(), obj, { revisionDate });
return Object.assign(new FolderView(), obj, {
Hinton marked this conversation as resolved.
Show resolved Hide resolved
revisionDate: nullableFactory(Date, obj.revisionDate),
});
}
}
17 changes: 7 additions & 10 deletions libs/common/src/services/cipher.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,20 +345,17 @@ export class CipherService implements CipherServiceAbstraction {
const orgKeys = await this.cryptoService.getOrgKeys();
const userKey = await this.cryptoService.getKeyForUserEncryption();

// Group ciphers by keyIdentifier
// Group ciphers by orgId or under 'null' for the user's ciphers
const grouped = ciphers.reduce((agg, c) => {
agg[c.keyIdentifier()] ??= [];
agg[c.keyIdentifier()].push(c);
agg[c.organizationId] ??= [];
agg[c.organizationId].push(c);
return agg;
}, {} as Record<string, Cipher[]>);

const decCiphers = (
await Promise.all(
Object.entries(grouped).map(([keyIdentifier, groupedCiphers]) =>
this.encryptService.decryptDomains(
CipherView,
groupedCiphers,
orgKeys.get(keyIdentifier) ?? userKey
)
Object.entries(grouped).map(([orgId, groupedCiphers]) =>
this.encryptService.decryptItems(groupedCiphers, orgKeys.get(orgId) ?? userKey)
)
)
)
Expand Down Expand Up @@ -515,7 +512,7 @@ export class CipherService implements CipherServiceAbstraction {

const ciphers = response.data.map((cr) => new Cipher(new CipherData(cr)));
const key = await this.cryptoService.getOrgKey(organizationId);
const decCiphers = await this.encryptService.decryptDomains(CipherView, ciphers, key);
const decCiphers = await this.encryptService.decryptItems(ciphers, key);

decCiphers.sort(this.getLocaleSortingFunction());
return decCiphers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Encryptable,
EncryptableDomain,
} from "../../interfaces/crypto.interface";
import { OldDecryptable } from "../../interfaces/decryptable.interface";
import { InitializerMetadata } from "../../interfaces/initializer-metadata.interface";
import { Utils } from "../../misc/utils";
import { EncArrayBuffer } from "../../models/domain/enc-array-buffer";
Expand Down Expand Up @@ -155,16 +156,15 @@ export class EncryptServiceImplementation implements EncryptService {
return result ?? null;
}

async decryptDomains<V, D extends DecryptableDomain & InitializerMetadata>(
view: Decryptable<V, D> & InitializerMetadata,
items: D[],
async decryptItems<T extends InitializerMetadata>(
items: OldDecryptable<T>[],
key: SymmetricCryptoKey
): Promise<V[]> {
): Promise<T[]> {
if (items == null || items.length < 1) {
return [];
}

return await Promise.all(items.map((item) => view.decrypt(this, key, item)));
return await Promise.all(items.map((item) => item.decrypt(key)));
}

async decryptDomain<V, D extends DecryptableDomain>(
Expand Down
15 changes: 5 additions & 10 deletions libs/common/src/services/cryptography/encrypt.worker.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { Jsonify } from "type-fest";

import { InitializerMetadata } from "../../interfaces/initializer-metadata.interface";
import { OldDecryptable } from "../../interfaces/decryptable.interface";
import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key";
import { ConsoleLogService } from "../../services/consoleLog.service";
import { ContainerService } from "../../services/container.service";
import { WebCryptoFunctionService } from "../../services/webCryptoFunction.service";

import { EncryptServiceImplementation } from "./encrypt.service.implementation";
import { getClass, getClassInitializer } from "./get-class-initializer";
import { InitializerKey } from "./initializer-key";
import { getClassInitializer } from "./get-class-initializer";

const workerApi: Worker = self as any;

Expand Down Expand Up @@ -39,20 +38,16 @@ workerApi.addEventListener("message", async (event: { data: string }) => {

const request: {
id: string;
view: InitializerKey;
items: Jsonify<InitializerMetadata>[];
items: Jsonify<OldDecryptable<any>>[];
key: Jsonify<SymmetricCryptoKey>;
} = JSON.parse(event.data);

const key = SymmetricCryptoKey.fromJSON(request.key);
const items = request.items.map((jsonItem) => {
const initializer = getClassInitializer(jsonItem.initializerKey);
const initializer = getClassInitializer<OldDecryptable<any>>(jsonItem.initializerKey);
return initializer(jsonItem);
});

const view = getClass<any>(request.view);

const result = await encryptService.decryptDomains(view as any, items as any, key);
const result = await encryptService.decryptItems(items, key);

workerApi.postMessage({
id: request.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,8 @@ const classInitializers: Record<InitializerKey, (obj: any) => any> = {
[InitializerKey.CipherView]: CipherView.fromJSON,
};

const classes: Partial<Record<InitializerKey, any>> = {
[InitializerKey.CipherView]: CipherView,
};

export function getClassInitializer<T extends InitializerMetadata>(
className: InitializerKey
): (obj: Jsonify<T>) => T {
return classInitializers[className];
}

export function getClass<T extends InitializerMetadata>(className: InitializerKey): (a: any) => T {
return classes[className];
}
Loading