1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-19 19:04:01 +00:00

[PM-31685] Removing email hashes (#18744)

* [PM-31685] Removing email hashes

* [PM-31685] fixing tests, which are now passing

* [PM-31685] removing anon access emails field and reusing emails field

* [PM-31685] fixing missed tests

* [PM-31685] fixing missed tests

* [PM-31685] code review changes

* [PM-31685] do not encrypt emails by use of domain functionality

* [PM-31685] test fixes
This commit is contained in:
Alex Dragovich
2026-02-09 12:59:17 -08:00
committed by jaasen-livefront
parent e3563b8f7c
commit 3c3ac1d39c
11 changed files with 29 additions and 333 deletions

View File

@@ -1008,7 +1008,6 @@ export default class MainBackground {
this.keyGenerationService,
this.sendStateProvider,
this.encryptService,
this.cryptoFunctionService,
this.configService,
);
this.sendApiService = new SendApiService(

View File

@@ -615,7 +615,6 @@ export class ServiceContainer {
this.keyGenerationService,
this.sendStateProvider,
this.encryptService,
this.cryptoFunctionService,
this.configService,
);

View File

@@ -859,7 +859,6 @@ const safeProviders: SafeProvider[] = [
KeyGenerationService,
SendStateProviderAbstraction,
EncryptService,
CryptoFunctionServiceAbstraction,
ConfigService,
],
}),

View File

@@ -23,7 +23,6 @@ export class SendData {
deletionDate: string;
password: string;
emails: string;
emailHashes: string;
disabled: boolean;
hideEmail: boolean;
authType: AuthType;
@@ -47,7 +46,6 @@ export class SendData {
this.deletionDate = response.deletionDate;
this.password = response.password;
this.emails = response.emails;
this.emailHashes = "";
this.disabled = response.disable;
this.hideEmail = response.hideEmail;
this.authType = response.authType;

View File

@@ -41,7 +41,6 @@ describe("Send", () => {
deletionDate: "2022-01-31T12:00:00.000Z",
password: "password",
emails: "",
emailHashes: "",
disabled: false,
hideEmail: true,
authType: AuthType.None,
@@ -70,8 +69,7 @@ describe("Send", () => {
expirationDate: null,
deletionDate: null,
password: undefined,
emails: null,
emailHashes: undefined,
emails: undefined,
disabled: undefined,
hideEmail: undefined,
});
@@ -97,8 +95,7 @@ describe("Send", () => {
expirationDate: new Date("2022-01-31T12:00:00.000Z"),
deletionDate: new Date("2022-01-31T12:00:00.000Z"),
password: "password",
emails: null,
emailHashes: "",
emails: "",
disabled: false,
hideEmail: true,
authType: AuthType.None,
@@ -173,7 +170,7 @@ describe("Send", () => {
});
});
describe("Email decryption", () => {
describe("Email parsing", () => {
let encryptService: jest.Mocked<EncryptService>;
let keyService: jest.Mocked<KeyService>;
const userKey = new SymmetricCryptoKey(new Uint8Array(32)) as UserKey;
@@ -188,91 +185,45 @@ describe("Send", () => {
(window as any).bitwardenContainerService = new ContainerService(keyService, encryptService);
});
it("should decrypt and parse single email", async () => {
it("should parse single email", async () => {
const send = new Send();
send.id = "id";
send.type = SendType.Text;
send.name = mockEnc("name");
send.notes = mockEnc("notes");
send.key = mockEnc("key");
send.emails = mockEnc("test@example.com");
send.emails = "test@example.com";
send.text = mock<SendText>();
send.text.decrypt = jest.fn().mockResolvedValue("textView" as any);
encryptService.decryptString.mockImplementation((encString, key) => {
if (encString === send.emails) {
return Promise.resolve("test@example.com");
}
if (encString === send.name) {
return Promise.resolve("name");
}
if (encString === send.notes) {
return Promise.resolve("notes");
}
return Promise.resolve("");
});
const view = await send.decrypt(userId);
expect(encryptService.decryptString).toHaveBeenCalledWith(send.emails, "cryptoKey");
expect(view.emails).toEqual(["test@example.com"]);
});
it("should decrypt and parse multiple emails", async () => {
it("should parse multiple emails", async () => {
const send = new Send();
send.id = "id";
send.type = SendType.Text;
send.name = mockEnc("name");
send.notes = mockEnc("notes");
send.key = mockEnc("key");
send.emails = mockEnc("test@example.com,user@test.com,admin@domain.com");
send.emails = "test@example.com,user@test.com,admin@domain.com";
send.text = mock<SendText>();
send.text.decrypt = jest.fn().mockResolvedValue("textView" as any);
encryptService.decryptString.mockImplementation((encString, key) => {
if (encString === send.emails) {
return Promise.resolve("test@example.com,user@test.com,admin@domain.com");
}
if (encString === send.name) {
return Promise.resolve("name");
}
if (encString === send.notes) {
return Promise.resolve("notes");
}
return Promise.resolve("");
});
const view = await send.decrypt(userId);
expect(view.emails).toEqual(["test@example.com", "user@test.com", "admin@domain.com"]);
});
it("should trim whitespace from decrypted emails", async () => {
it("should trim whitespace from emails", async () => {
const send = new Send();
send.id = "id";
send.type = SendType.Text;
send.name = mockEnc("name");
send.notes = mockEnc("notes");
send.key = mockEnc("key");
send.emails = mockEnc(" test@example.com , user@test.com ");
send.emails = " test@example.com , user@test.com ";
send.text = mock<SendText>();
send.text.decrypt = jest.fn().mockResolvedValue("textView" as any);
encryptService.decryptString.mockImplementation((encString, key) => {
if (encString === send.emails) {
return Promise.resolve(" test@example.com , user@test.com ");
}
if (encString === send.name) {
return Promise.resolve("name");
}
if (encString === send.notes) {
return Promise.resolve("notes");
}
return Promise.resolve("");
});
const view = await send.decrypt(userId);
expect(view.emails).toEqual(["test@example.com", "user@test.com"]);
});
@@ -293,61 +244,17 @@ describe("Send", () => {
expect(encryptService.decryptString).not.toHaveBeenCalledWith(expect.anything(), "cryptoKey");
});
it("should return empty array when decrypted emails is empty string", async () => {
it("should return empty array when emails is empty string", async () => {
const send = new Send();
send.id = "id";
send.type = SendType.Text;
send.name = mockEnc("name");
send.notes = mockEnc("notes");
send.key = mockEnc("key");
send.emails = mockEnc("");
send.emails = "";
send.text = mock<SendText>();
send.text.decrypt = jest.fn().mockResolvedValue("textView" as any);
encryptService.decryptString.mockImplementation((encString, key) => {
if (encString === send.emails) {
return Promise.resolve("");
}
if (encString === send.name) {
return Promise.resolve("name");
}
if (encString === send.notes) {
return Promise.resolve("notes");
}
return Promise.resolve("");
});
const view = await send.decrypt(userId);
expect(view.emails).toEqual([]);
});
it("should return empty array when decrypted emails is null", async () => {
const send = new Send();
send.id = "id";
send.type = SendType.Text;
send.name = mockEnc("name");
send.notes = mockEnc("notes");
send.key = mockEnc("key");
send.emails = mockEnc("something");
send.text = mock<SendText>();
send.text.decrypt = jest.fn().mockResolvedValue("textView" as any);
encryptService.decryptString.mockImplementation((encString, key) => {
if (encString === send.emails) {
return Promise.resolve(null);
}
if (encString === send.name) {
return Promise.resolve("name");
}
if (encString === send.notes) {
return Promise.resolve("notes");
}
return Promise.resolve("");
});
const view = await send.decrypt(userId);
expect(view.emails).toEqual([]);
});
});

View File

@@ -31,8 +31,7 @@ export class Send extends Domain {
expirationDate: Date;
deletionDate: Date;
password: string;
emails: EncString;
emailHashes: string;
emails: string;
disabled: boolean;
hideEmail: boolean;
authType: AuthType;
@@ -52,7 +51,6 @@ export class Send extends Domain {
name: null,
notes: null,
key: null,
emails: null,
},
["id", "accessId"],
);
@@ -62,13 +60,13 @@ export class Send extends Domain {
this.maxAccessCount = obj.maxAccessCount;
this.accessCount = obj.accessCount;
this.password = obj.password;
this.emailHashes = obj.emailHashes;
this.disabled = obj.disabled;
this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null;
this.deletionDate = obj.deletionDate != null ? new Date(obj.deletionDate) : null;
this.expirationDate = obj.expirationDate != null ? new Date(obj.expirationDate) : null;
this.hideEmail = obj.hideEmail;
this.authType = obj.authType;
this.emails = obj.emails;
switch (this.type) {
case SendType.Text:
@@ -100,8 +98,7 @@ export class Send extends Domain {
this.notes != null ? await encryptService.decryptString(this.notes, model.cryptoKey) : null;
if (this.emails != null) {
const decryptedEmails = await encryptService.decryptString(this.emails, model.cryptoKey);
model.emails = decryptedEmails ? decryptedEmails.split(",").map((e) => e.trim()) : [];
model.emails = this.emails ? this.emails.split(",").map((e) => e.trim()) : [];
} else {
model.emails = [];
}
@@ -133,7 +130,7 @@ export class Send extends Domain {
key: EncString.fromJSON(obj.key),
name: EncString.fromJSON(obj.name),
notes: EncString.fromJSON(obj.notes),
emails: EncString.fromJSON(obj.emails),
emails: obj.emails,
text: SendText.fromJSON(obj.text),
file: SendFile.fromJSON(obj.file),
revisionDate,

View File

@@ -8,44 +8,6 @@ import { SendRequest } from "./send.request";
describe("SendRequest", () => {
describe("constructor", () => {
it("should populate emails with encrypted string from Send.emails", () => {
const send = new Send();
send.type = SendType.Text;
send.name = new EncString("encryptedName");
send.notes = new EncString("encryptedNotes");
send.key = new EncString("encryptedKey");
send.emails = new EncString("encryptedEmailList");
send.emailHashes = "HASH1,HASH2,HASH3";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
send.text.text = new EncString("text");
send.text.hidden = false;
const request = new SendRequest(send);
expect(request.emails).toBe("encryptedEmailList");
});
it("should populate emailHashes from Send.emailHashes", () => {
const send = new Send();
send.type = SendType.Text;
send.name = new EncString("encryptedName");
send.notes = new EncString("encryptedNotes");
send.key = new EncString("encryptedKey");
send.emails = new EncString("encryptedEmailList");
send.emailHashes = "HASH1,HASH2,HASH3";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
send.text.text = new EncString("text");
send.text.hidden = false;
const request = new SendRequest(send);
expect(request.emailHashes).toBe("HASH1,HASH2,HASH3");
});
it("should set emails to null when Send.emails is null", () => {
const send = new Send();
send.type = SendType.Text;
@@ -53,7 +15,6 @@ describe("SendRequest", () => {
send.notes = new EncString("encryptedNotes");
send.key = new EncString("encryptedKey");
send.emails = null;
send.emailHashes = "";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
@@ -63,45 +24,6 @@ describe("SendRequest", () => {
const request = new SendRequest(send);
expect(request.emails).toBeNull();
expect(request.emailHashes).toBe("");
});
it("should handle empty emailHashes", () => {
const send = new Send();
send.type = SendType.Text;
send.name = new EncString("encryptedName");
send.key = new EncString("encryptedKey");
send.emails = null;
send.emailHashes = "";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
send.text.text = new EncString("text");
send.text.hidden = false;
const request = new SendRequest(send);
expect(request.emailHashes).toBe("");
});
it("should not expose plaintext emails", () => {
const send = new Send();
send.type = SendType.Text;
send.name = new EncString("encryptedName");
send.key = new EncString("encryptedKey");
send.emails = new EncString("2.encrypted|emaildata|here");
send.emailHashes = "ABC123,DEF456";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
send.text.text = new EncString("text");
send.text.hidden = false;
const request = new SendRequest(send);
// Ensure the request contains the encrypted string format, not plaintext
expect(request.emails).toBe("2.encrypted|emaildata|here");
expect(request.emails).not.toContain("@");
});
it("should handle name being null", () => {
@@ -111,7 +33,6 @@ describe("SendRequest", () => {
send.notes = new EncString("encryptedNotes");
send.key = new EncString("encryptedKey");
send.emails = null;
send.emailHashes = "";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
@@ -130,7 +51,6 @@ describe("SendRequest", () => {
send.notes = null;
send.key = new EncString("encryptedKey");
send.emails = null;
send.emailHashes = "";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
@@ -148,7 +68,6 @@ describe("SendRequest", () => {
send.name = new EncString("encryptedName");
send.key = new EncString("encryptedKey");
send.emails = null;
send.emailHashes = "";
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
@@ -160,33 +79,4 @@ describe("SendRequest", () => {
expect(request.fileLength).toBe(1024);
});
});
describe("Email auth requirements", () => {
it("should create request with encrypted emails and plaintext emailHashes", () => {
// Setup: A Send with encrypted emails and computed hashes
const send = new Send();
send.type = SendType.Text;
send.name = new EncString("encryptedName");
send.key = new EncString("encryptedKey");
send.emails = new EncString("2.encryptedEmailString|data");
send.emailHashes = "A1B2C3D4,E5F6G7H8"; // Plaintext hashes
send.disabled = false;
send.hideEmail = false;
send.text = new SendText();
send.text.text = new EncString("text");
send.text.hidden = false;
// Act: Create the request
const request = new SendRequest(send);
// emails field contains encrypted value
expect(request.emails).toBe("2.encryptedEmailString|data");
expect(request.emails).toContain("encrypted");
//emailHashes field contains plaintext comma-separated hashes
expect(request.emailHashes).toBe("A1B2C3D4,E5F6G7H8");
expect(request.emailHashes).not.toContain("encrypted");
expect(request.emailHashes.split(",")).toHaveLength(2);
});
});
});

View File

@@ -18,7 +18,6 @@ export class SendRequest {
file: SendFileApi;
password: string;
emails: string;
emailHashes: string;
disabled: boolean;
hideEmail: boolean;
@@ -32,8 +31,7 @@ export class SendRequest {
this.deletionDate = send.deletionDate != null ? send.deletionDate.toISOString() : null;
this.key = send.key != null ? send.key.encryptedString : null;
this.password = send.password;
this.emails = send.emails ? send.emails.encryptedString : null;
this.emailHashes = send.emailHashes;
this.emails = send.emails;
this.disabled = send.disabled;
this.hideEmail = send.hideEmail;

View File

@@ -1,7 +1,6 @@
import { mock } from "jest-mock-extended";
import { firstValueFrom, of } from "rxjs";
import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { KeyService } from "@bitwarden/key-management";
@@ -51,7 +50,6 @@ describe("SendService", () => {
const keyGenerationService = mock<KeyGenerationService>();
const encryptService = mock<EncryptService>();
const environmentService = mock<EnvironmentService>();
const cryptoFunctionService = mock<CryptoFunctionService>();
const configService = mock<ConfigService>();
let sendStateProvider: SendStateProvider;
let sendService: SendService;
@@ -98,7 +96,6 @@ describe("SendService", () => {
keyGenerationService,
sendStateProvider,
encryptService,
cryptoFunctionService,
configService,
);
});
@@ -612,111 +609,50 @@ describe("SendService", () => {
describe("when SendEmailOTP feature flag is ON", () => {
beforeEach(() => {
configService.getFeatureFlag.mockResolvedValue(true);
cryptoFunctionService.hash.mockClear();
});
describe("email encryption", () => {
it("should encrypt emails when email list is provided", async () => {
describe("email processing", () => {
it("should create a comma separated string when an email list is provided", async () => {
sendView.emails = ["test@example.com", "user@test.com"];
cryptoFunctionService.hash.mockResolvedValue(new Uint8Array([0xab, 0xcd]));
const [send] = await sendService.encrypt(sendView, null, null);
expect(encryptService.encryptString).toHaveBeenCalledWith(
"test@example.com,user@test.com",
mockCryptoKey,
);
expect(send.emails).toEqual({ encryptedString: "encrypted" });
expect(send.emails).toEqual("test@example.com,user@test.com");
expect(send.password).toBeNull();
});
it("should set emails to null when email list is empty", async () => {
sendView.emails = [];
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
});
it("should set emails to null when email list is null", async () => {
sendView.emails = null;
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
});
it("should set emails to null when email list is undefined", async () => {
sendView.emails = undefined;
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
});
});
describe("email hashing", () => {
it("should hash emails using SHA-256 and return uppercase hex", async () => {
sendView.emails = ["test@example.com"];
const mockHash = new Uint8Array([0xab, 0xcd, 0xef]);
cryptoFunctionService.hash.mockResolvedValue(mockHash);
const [send] = await sendService.encrypt(sendView, null, null);
expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256");
expect(send.emailHashes).toBe("ABCDEF");
});
it("should hash multiple emails and return comma-separated hashes", async () => {
it("should process multiple emails and return comma-separated string", async () => {
sendView.emails = ["test@example.com", "user@test.com"];
const mockHash1 = new Uint8Array([0xab, 0xcd]);
const mockHash2 = new Uint8Array([0x12, 0x34]);
cryptoFunctionService.hash
.mockResolvedValueOnce(mockHash1)
.mockResolvedValueOnce(mockHash2);
const [send] = await sendService.encrypt(sendView, null, null);
expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256");
expect(cryptoFunctionService.hash).toHaveBeenCalledWith("user@test.com", "sha256");
expect(send.emailHashes).toBe("ABCD,1234");
expect(send.emails).toBe("test@example.com,user@test.com");
});
it("should trim and lowercase emails before hashing", async () => {
it("should trim and lowercase emails", async () => {
sendView.emails = [" Test@Example.COM ", "USER@test.com"];
const mockHash = new Uint8Array([0xff]);
cryptoFunctionService.hash.mockResolvedValue(mockHash);
await sendService.encrypt(sendView, null, null);
expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256");
expect(cryptoFunctionService.hash).toHaveBeenCalledWith("user@test.com", "sha256");
});
it("should set emailHashes to empty string when no emails", async () => {
sendView.emails = [];
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emailHashes).toBe("");
expect(cryptoFunctionService.hash).not.toHaveBeenCalled();
expect(send.emails).toBe("test@example.com,user@test.com");
});
it("should handle single email correctly", async () => {
sendView.emails = ["single@test.com"];
const mockHash = new Uint8Array([0xa1, 0xb2, 0xc3]);
cryptoFunctionService.hash.mockResolvedValue(mockHash);
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emailHashes).toBe("A1B2C3");
expect(send.emails).toBe("single@test.com");
});
});
@@ -747,7 +683,6 @@ describe("SendService", () => {
describe("when SendEmailOTP feature flag is OFF", () => {
beforeEach(() => {
configService.getFeatureFlag.mockResolvedValue(false);
cryptoFunctionService.hash.mockClear();
});
it("should NOT encrypt emails even when provided", async () => {
@@ -756,8 +691,6 @@ describe("SendService", () => {
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
expect(cryptoFunctionService.hash).not.toHaveBeenCalled();
});
it("should use password when provided and flag is OFF", async () => {
@@ -769,7 +702,6 @@ describe("SendService", () => {
const [send] = await sendService.encrypt(sendView, null, "password123");
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
expect(send.password).toBe("hashedPassword");
});
@@ -782,9 +714,7 @@ describe("SendService", () => {
const [send] = await sendService.encrypt(sendView, null, "password123");
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
expect(send.password).toBe("hashedPassword");
expect(cryptoFunctionService.hash).not.toHaveBeenCalled();
});
it("should set emails and password to null when neither provided", async () => {
@@ -793,7 +723,6 @@ describe("SendService", () => {
const [send] = await sendService.encrypt(sendView, null, null);
expect(send.emails).toBeNull();
expect(send.emailHashes).toBe("");
expect(send.password).toBeUndefined();
});
});

View File

@@ -9,7 +9,6 @@ import { PBKDF2KdfConfig, KeyService } from "@bitwarden/key-management";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { KeyGenerationService } from "../../../key-management/crypto";
import { CryptoFunctionService } from "../../../key-management/crypto/abstractions/crypto-function.service";
import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service";
import { EncString } from "../../../key-management/crypto/models/enc-string";
import { ConfigService } from "../../../platform/abstractions/config/config.service";
@@ -54,7 +53,6 @@ export class SendService implements InternalSendServiceAbstraction {
private keyGenerationService: KeyGenerationService,
private stateProvider: SendStateProvider,
private encryptService: EncryptService,
private cryptoFunctionService: CryptoFunctionService,
private configService: ConfigService,
) {}
@@ -91,13 +89,13 @@ export class SendService implements InternalSendServiceAbstraction {
const hasEmails = (model.emails?.length ?? 0) > 0;
if (sendEmailOTPEnabled && hasEmails) {
const plaintextEmails = model.emails.join(",");
send.emails = await this.encryptService.encryptString(plaintextEmails, model.cryptoKey);
send.emailHashes = await this.hashEmails(plaintextEmails);
send.emails = model.emails
.map((e) => e.trim())
.join(",")
.toLocaleLowerCase();
send.password = null;
} else {
send.emails = null;
send.emailHashes = "";
if (password != null) {
// Note: Despite being called key, the passwordKey is not used for encryption.
@@ -393,19 +391,4 @@ export class SendService implements InternalSendServiceAbstraction {
decryptedSends.sort(Utils.getSortFunction(this.i18nService, "name"));
return decryptedSends;
}
private async hashEmails(emails: string): Promise<string> {
if (!emails) {
return "";
}
const emailArray = emails.split(",").map((e) => e.trim().toLowerCase());
const hashPromises = emailArray.map(async (email) => {
const hash: Uint8Array = await this.cryptoFunctionService.hash(email, "sha256");
return Utils.fromBufferToHex(hash).toUpperCase();
});
const hashes = await Promise.all(hashPromises);
return hashes.join(",");
}
}

View File

@@ -41,7 +41,6 @@ export function createSendData(value: Partial<SendData> = {}) {
deletionDate: "2024-09-04",
password: "password",
emails: "",
emailHashes: "",
disabled: false,
hideEmail: false,
};
@@ -66,7 +65,6 @@ export function testSendData(id: string, name: string) {
data.notes = "Notes!!";
data.key = null;
data.emails = "";
data.emailHashes = "";
return data;
}
@@ -82,7 +80,6 @@ export function testSend(id: string, name: string) {
data.deletionDate = null;
data.notes = new EncString("Notes!!");
data.key = null;
data.emails = null;
data.emailHashes = "";
data.emails = "";
return data;
}