Skip to content

Commit 46aa386

Browse files
[PM-26681] - ensure initial cipher state is set for empty vaults (#16793)
* don't skip cipher state updates for empty vaults * add specs and comment * do not return ciphers, just return * update spec names
1 parent 2aa0ad6 commit 46aa386

File tree

2 files changed

+88
-2
lines changed

2 files changed

+88
-2
lines changed

libs/common/src/vault/services/cipher.service.spec.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { CipherView } from "../models/view/cipher.view";
4545
import { LoginUriView } from "../models/view/login-uri.view";
4646

4747
import { CipherService } from "./cipher.service";
48+
import { ENCRYPTED_CIPHERS } from "./key-state/ciphers.state";
4849

4950
const ENCRYPTED_TEXT = "This data has been encrypted";
5051
function encryptText(clearText: string | Uint8Array) {
@@ -817,4 +818,87 @@ describe("Cipher Service", () => {
817818
expect(failures).toHaveLength(0);
818819
});
819820
});
821+
822+
describe("replace (no upsert)", () => {
823+
// In order to set up initial state we need to manually update the encrypted state
824+
// which will result in an emission. All tests will have this baseline emission.
825+
const TEST_BASELINE_EMISSIONS = 1;
826+
827+
const makeCipher = (id: string): CipherData =>
828+
({
829+
...cipherData,
830+
id,
831+
name: `Enc ${id}`,
832+
}) as CipherData;
833+
834+
const tick = async () => new Promise((r) => setTimeout(r, 0));
835+
836+
const setEncryptedState = async (data: Record<CipherId, CipherData>, uid = userId) => {
837+
// Directly set the encrypted state, this will result in a single emission
838+
await stateProvider.getUser(uid, ENCRYPTED_CIPHERS).update(() => data);
839+
// match service’s “next tick” behavior so subscribers see it
840+
await tick();
841+
};
842+
843+
it("emits and calls updateEncryptedCipherState when current state is empty and replace({}) is called", async () => {
844+
// Ensure empty state
845+
await setEncryptedState({});
846+
847+
const emissions: Array<Record<CipherId, CipherData>> = [];
848+
const sub = cipherService.ciphers$(userId).subscribe((v) => emissions.push(v));
849+
await tick();
850+
851+
const spy = jest.spyOn<any, any>(cipherService, "updateEncryptedCipherState");
852+
853+
// Calling replace with empty object MUST still update to trigger init emissions
854+
await cipherService.replace({}, userId);
855+
await tick();
856+
857+
expect(spy).toHaveBeenCalledTimes(1);
858+
expect(emissions.length).toBeGreaterThanOrEqual(TEST_BASELINE_EMISSIONS + 1);
859+
860+
sub.unsubscribe();
861+
});
862+
863+
it("does NOT emit or call updateEncryptedCipherState when state is non-empty and identical", async () => {
864+
const A = makeCipher("A");
865+
await setEncryptedState({ [A.id as CipherId]: A });
866+
867+
const emissions: Array<Record<CipherId, CipherData>> = [];
868+
const sub = cipherService.ciphers$(userId).subscribe((v) => emissions.push(v));
869+
await tick();
870+
871+
const spy = jest.spyOn<any, any>(cipherService, "updateEncryptedCipherState");
872+
873+
// identical snapshot → short-circuit path
874+
await cipherService.replace({ [A.id as CipherId]: A }, userId);
875+
await tick();
876+
877+
expect(spy).not.toHaveBeenCalled();
878+
expect(emissions.length).toBe(TEST_BASELINE_EMISSIONS);
879+
880+
sub.unsubscribe();
881+
});
882+
883+
it("emits and calls updateEncryptedCipherState when the provided state differs from current", async () => {
884+
const A = makeCipher("A");
885+
await setEncryptedState({ [A.id as CipherId]: A });
886+
887+
const emissions: Array<Record<CipherId, CipherData>> = [];
888+
const sub = cipherService.ciphers$(userId).subscribe((v) => emissions.push(v));
889+
await tick();
890+
891+
const spy = jest.spyOn<any, any>(cipherService, "updateEncryptedCipherState");
892+
893+
const B = makeCipher("B");
894+
await cipherService.replace({ [B.id as CipherId]: B }, userId);
895+
await tick();
896+
897+
expect(spy).toHaveBeenCalledTimes(1);
898+
899+
expect(emissions.length).toBeGreaterThanOrEqual(TEST_BASELINE_EMISSIONS + 1);
900+
901+
sub.unsubscribe();
902+
});
903+
});
820904
});

libs/common/src/vault/services/cipher.service.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,8 +1169,10 @@ export class CipherService implements CipherServiceAbstraction {
11691169
// If stored and provided data are identical, this event doesn’t fire and the ciphers$
11701170
// observable won’t emit a new value. In this case we can skip the update to avoid calling
11711171
// clearCache and causing an empty state.
1172-
if (JSON.stringify(current) === JSON.stringify(ciphers)) {
1173-
return ciphers;
1172+
// If the current state is empty (eg. for new users), we still want to perform the update to ensure
1173+
// we trigger an emission as many subscribers rely on it during initialization.
1174+
if (Object.keys(current).length > 0 && JSON.stringify(current) === JSON.stringify(ciphers)) {
1175+
return;
11741176
}
11751177

11761178
await this.updateEncryptedCipherState(() => ciphers, userId);

0 commit comments

Comments
 (0)