bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-12 06:13:38 +00:00
Code Issues Releases Wiki Activity

Add-userid-to-encryption-methods (#14844)

Browse Source 
* Get userId from response if available

This is a small improvement for the Auth team which avoids inspection of the access token, sometimes.

* Initialize sdk clients with a userId

* return both Cipher and encryptedFor when encrypting a cipher

Update cipher api requests to include encryptedFor attribute

* Prefer named types with documentation

* Update sdk to latest

* Fixup types

* Fixup tests

* Revert getting userId from identity token response

---------

Co-authored-by: Shane <smelton@bitwarden.com>
This commit is contained in:
Matt Gibson
2025-05-30 10:50:54 -07:00
committed by GitHub
parent 4e112e2daa
commit 9f9cb0d13d
19 changed files with 212 additions and 227 deletions
Download Patch File Download Diff File Expand all files Collapse all files

168
libs/common/src/vault/services/cipher.service.spec.ts
View File

@@ -27,6 +27,7 @@ import { ContainerService } from "../../platform/services/container.service";
import { CipherId, UserId } from "../../types/guid";
import { CipherKey, OrgKey, UserKey } from "../../types/key";
import { CipherEncryptionService } from "../abstractions/cipher-encryption.service";
import { EncryptionContext } from "../abstractions/cipher.service";
import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service";
import { FieldType } from "../enums";
import { CipherRepromptType } from "../enums/cipher-reprompt-type";
@@ -78,36 +79,12 @@ const cipherData: CipherData = {
},
passwordHistory: [{ password: "EncryptedString", lastUsedDate: "2022-01-31T12:00:00.000Z" }],
attachments: [
{
id: "a1",
url: "url",
size: "1100",
sizeName: "1.1 KB",
fileName: "file",
key: "EncKey",
},
{
id: "a2",
url: "url",
size: "1100",
sizeName: "1.1 KB",
fileName: "file",
key: "EncKey",
},
{ id: "a1", url: "url", size: "1100", sizeName: "1.1 KB", fileName: "file", key: "EncKey" },
{ id: "a2", url: "url", size: "1100", sizeName: "1.1 KB", fileName: "file", key: "EncKey" },
],
fields: [
{
name: "EncryptedString",
value: "EncryptedString",
type: FieldType.Text,
linkedId: null,
},
{
name: "EncryptedString",
value: "EncryptedString",
type: FieldType.Hidden,
linkedId: null,
},
{ name: "EncryptedString", value: "EncryptedString", type: FieldType.Text, linkedId: null },
{ name: "EncryptedString", value: "EncryptedString", type: FieldType.Hidden, linkedId: null },
],
};
const mockUserId = Utils.newGuid() as UserId;
@@ -133,7 +110,7 @@ describe("Cipher Service", () => {
const userId = "TestUserId" as UserId;
let cipherService: CipherService;
let cipherObj: Cipher;
let encryptionContext: EncryptionContext;
beforeEach(() => {
encryptService.encryptFileData.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES));
@@ -159,7 +136,7 @@ describe("Cipher Service", () => {
cipherEncryptionService,
);
cipherObj = new Cipher(cipherData);
encryptionContext = { cipher: new Cipher(cipherData), encryptedFor: userId };
});
afterEach(() => {
@@ -192,33 +169,33 @@ describe("Cipher Service", () => {
it("should call apiService.postCipherAdmin when orgAdmin param is true and the cipher orgId != null", async () => {
const spy = jest
.spyOn(apiService, "postCipherAdmin")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.createWithServer(cipherObj, true);
const expectedObj = new CipherCreateRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.createWithServer(encryptionContext, true);
const expectedObj = new CipherCreateRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
});
it("should call apiService.postCipher when orgAdmin param is true and the cipher orgId is null", async () => {
cipherObj.organizationId = null;
encryptionContext.cipher.organizationId = null!;
const spy = jest
.spyOn(apiService, "postCipher")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.createWithServer(cipherObj, true);
const expectedObj = new CipherRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.createWithServer(encryptionContext, true);
const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
});
it("should call apiService.postCipherCreate if collectionsIds != null", async () => {
cipherObj.collectionIds = ["123"];
encryptionContext.cipher.collectionIds = ["123"];
const spy = jest
.spyOn(apiService, "postCipherCreate")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.createWithServer(cipherObj);
const expectedObj = new CipherCreateRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.createWithServer(encryptionContext);
const expectedObj = new CipherCreateRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
@@ -227,9 +204,9 @@ describe("Cipher Service", () => {
it("should call apiService.postCipher when orgAdmin and collectionIds logic is false", async () => {
const spy = jest
.spyOn(apiService, "postCipher")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.createWithServer(cipherObj);
const expectedObj = new CipherRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.createWithServer(encryptionContext);
const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
@@ -240,36 +217,36 @@ describe("Cipher Service", () => {
it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => {
const spy = jest
.spyOn(apiService, "putCipherAdmin")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.updateWithServer(cipherObj, true);
const expectedObj = new CipherRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.updateWithServer(encryptionContext, true);
const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
it("should call apiService.putCipher if cipher.edit is true", async () => {
cipherObj.edit = true;
encryptionContext.cipher.edit = true;
const spy = jest
.spyOn(apiService, "putCipher")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.updateWithServer(cipherObj);
const expectedObj = new CipherRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.updateWithServer(encryptionContext);
const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
it("should call apiService.putPartialCipher when orgAdmin, and edit are false", async () => {
cipherObj.edit = false;
encryptionContext.cipher.edit = false;
const spy = jest
.spyOn(apiService, "putPartialCipher")
.mockImplementation(() => Promise.resolve<any>(cipherObj.toCipherData()));
await cipherService.updateWithServer(cipherObj);
const expectedObj = new CipherPartialRequest(cipherObj);
.mockImplementation(() => Promise.resolve<any>(encryptionContext.cipher.toCipherData()));
await cipherService.updateWithServer(encryptionContext);
const expectedObj = new CipherPartialRequest(encryptionContext.cipher);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
});
@@ -293,6 +270,15 @@ describe("Cipher Service", () => {
jest.spyOn(cipherService as any, "getAutofillOnPageLoadDefault").mockResolvedValue(true);
});
it("should return the encrypting user id", async () => {
keyService.getOrgKey.mockReturnValue(
Promise.resolve<any>(new SymmetricCryptoKey(new Uint8Array(32)) as OrgKey),
);
const { encryptedFor } = await cipherService.encrypt(cipherView, userId);
expect(encryptedFor).toEqual(userId);
});
describe("login encryption", () => {
it("should add a uri hash to login uris", async () => {
encryptService.hash.mockImplementation((value) => Promise.resolve(`${value} hash`));
@@ -304,9 +290,9 @@ describe("Cipher Service", () => {
Promise.resolve<any>(new SymmetricCryptoKey(new Uint8Array(32)) as OrgKey),
);
const domain = await cipherService.encrypt(cipherView, userId);
const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(domain.login.uris).toEqual([
expect(cipher.login.uris).toEqual([
{
uri: new EncString("uri has been encrypted"),
uriChecksum: new EncString("uri hash has been encrypted"),
@@ -325,7 +311,7 @@ describe("Cipher Service", () => {
it("is null when feature flag is false", async () => {
configService.getFeatureFlag.mockResolvedValue(false);
const cipher = await cipherService.encrypt(cipherView, userId);
const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeNull();
});
@@ -338,7 +324,7 @@ describe("Cipher Service", () => {
it("is null when the cipher is not viewPassword", async () => {
cipherView.viewPassword = false;
const cipher = await cipherService.encrypt(cipherView, userId);
const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeNull();
});
@@ -346,7 +332,7 @@ describe("Cipher Service", () => {
it("is defined when the cipher is viewPassword", async () => {
cipherView.viewPassword = true;
const cipher = await cipherService.encrypt(cipherView, userId);
const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeDefined();
});
@@ -393,7 +379,13 @@ describe("Cipher Service", () => {
it("is called when cipher viewPassword is false and original cipher has a key", async () => {
cipherView.viewPassword = false;
await cipherService.encrypt(cipherView, userId, undefined, undefined, cipherObj);
await cipherService.encrypt(
cipherView,
userId,
undefined,
undefined,
encryptionContext.cipher,
);
expect(cipherService["encryptCipherWithCipherKey"]).toHaveBeenCalled();
});
@@ -416,22 +408,17 @@ describe("Cipher Service", () => {
stateService.getUserId.mockResolvedValue(mockUserId);
const keys = {
userKey: originalUserKey,
} as CipherDecryptionKeys;
const keys = { userKey: originalUserKey } as CipherDecryptionKeys;
keyService.cipherDecryptionKeys$.mockReturnValue(of(keys));
const cipher1 = new CipherView(cipherObj);
cipher1.id = "Cipher 1";
const cipher1 = new CipherView(encryptionContext.cipher);
cipher1.id = "Cipher 1" as CipherId;
cipher1.organizationId = null;
const cipher2 = new CipherView(cipherObj);
cipher2.id = "Cipher 2";
const cipher2 = new CipherView(encryptionContext.cipher);
cipher2.id = "Cipher 2" as CipherId;
cipher2.organizationId = null;
decryptedCiphers = new BehaviorSubject({
Cipher1: cipher1,
Cipher2: cipher2,
});
decryptedCiphers = new BehaviorSubject({ [cipher1.id]: cipher1, [cipher2.id]: cipher2 });
jest
.spyOn(cipherService, "cipherViews$")
.mockImplementation((userId: UserId) =>
@@ -462,19 +449,19 @@ describe("Cipher Service", () => {
});
it("throws if the original user key is null", async () => {
await expect(cipherService.getRotatedData(null, newUserKey, mockUserId)).rejects.toThrow(
await expect(cipherService.getRotatedData(null!, newUserKey, mockUserId)).rejects.toThrow(
"Original user key is required to rotate ciphers",
);
});
it("throws if the new user key is null", async () => {
await expect(cipherService.getRotatedData(originalUserKey, null, mockUserId)).rejects.toThrow(
"New user key is required to rotate ciphers",
);
await expect(
cipherService.getRotatedData(originalUserKey, null!, mockUserId),
).rejects.toThrow("New user key is required to rotate ciphers");
});
it("throws if the user has any failed to decrypt ciphers", async () => {
const badCipher = new CipherView(cipherObj);
const badCipher = new CipherView(encryptionContext.cipher);
badCipher.id = "Cipher 3";
badCipher.organizationId = null;
badCipher.decryptionFailure = true;
@@ -488,12 +475,15 @@ describe("Cipher Service", () => {
describe("decrypt", () => {
it("should call decrypt method of CipherEncryptionService when feature flag is true", async () => {
configService.getFeatureFlag.mockResolvedValue(true);
cipherEncryptionService.decrypt.mockResolvedValue(new CipherView(cipherObj));
cipherEncryptionService.decrypt.mockResolvedValue(new CipherView(encryptionContext.cipher));
const result = await cipherService.decrypt(cipherObj, userId);
const result = await cipherService.decrypt(encryptionContext.cipher, userId);
expect(result).toEqual(new CipherView(cipherObj));
expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(cipherObj, userId);
expect(result).toEqual(new CipherView(encryptionContext.cipher));
expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(
encryptionContext.cipher,
userId,
);
});
it("should call legacy decrypt when feature flag is false", async () => {
@@ -501,12 +491,14 @@ describe("Cipher Service", () => {
configService.getFeatureFlag.mockResolvedValue(false);
cipherService.getKeyForCipherKeyDecryption = jest.fn().mockResolvedValue(mockUserKey);
encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(32));
jest.spyOn(cipherObj, "decrypt").mockResolvedValue(new CipherView(cipherObj));
jest
.spyOn(encryptionContext.cipher, "decrypt")
.mockResolvedValue(new CipherView(encryptionContext.cipher));
const result = await cipherService.decrypt(cipherObj, userId);
const result = await cipherService.decrypt(encryptionContext.cipher, userId);
expect(result).toEqual(new CipherView(cipherObj));
expect(cipherObj.decrypt).toHaveBeenCalledWith(mockUserKey);
expect(result).toEqual(new CipherView(encryptionContext.cipher));
expect(encryptionContext.cipher.decrypt).toHaveBeenCalledWith(mockUserKey);
});
});