feat: revamp user storage encryption process

packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts

@@ -58,15 +58,16 @@ export async function createMockGetStorageResponse(
 export async function createMockAllFeatureEntriesResponse(
   dataArr: string[] = [MOCK_STORAGE_DATA],
 ): Promise<GetUserStorageAllFeatureEntriesResponse> {
-  return await Promise.all(
-    dataArr.map(async function (d) {
-      const encryptedData = await MOCK_ENCRYPTED_STORAGE_DATA(d);
-      return {
-        HashedKey: 'HASHED_KEY',
-        Data: encryptedData,
-      };
-    }),
-  );
+  const decryptedData = [];
+
+  for (const data of dataArr) {
+    decryptedData.push({
+      HashedKey: 'HASHED_KEY',
+      Data: await MOCK_ENCRYPTED_STORAGE_DATA(data),
+    });
+  }
+
+  return decryptedData;
 }
 
 /**

packages/profile-sync-controller/src/controllers/user-storage/services.test.ts

@@ -1,6 +1,8 @@
 import encryption, { createSHA256Hash } from '../../shared/encryption';
+import { getIfEntriesHaveDifferentSalts } from '../../shared/encryption/utils';
 import type { UserStorageFeatureKeys } from '../../shared/storage-schema';
 import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema';
+import { createMockGetStorageResponse } from './__fixtures__';
 import {
   mockEndpointGetUserStorage,
   mockEndpointUpsertUserStorage,
@@ -81,6 +83,44 @@ describe('user-storage/services.ts - getUserStorage() tests', () => {
     mockGetUserStorage.done();
     expect(result).toBeNull();
   });
+
+  it('re-encrypts data if received entry was encrypted with a non-empty salt, and saves it back to user storage', async () => {
+    // This corresponds to 'data1'
+    // Encrypted with a non-empty salt
+    const mockResponse = {
+      HashedKey: 'entry1',
+      Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+    };
+
+    const mockGetUserStorage = await mockEndpointGetUserStorage(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+      {
+        status: 200,
+        body: JSON.stringify(mockResponse),
+      },
+    );
+
+    const mockUpsertUserStorage = mockEndpointUpsertUserStorage(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+      undefined,
+      async (requestBody) => {
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const isNewSaltEmpty =
+          encryption.getSalt(requestBody.data).length === 0;
+
+        expect(isNewSaltEmpty).toBe(true);
+      },
+    );
+
+    const result = await actCallGetUserStorage();
+
+    mockGetUserStorage.done();
+    mockUpsertUserStorage.done();
+    expect(result).toBe('data1');
+  });
 });
 
 describe('user-storage/services.ts - getUserStorageAllFeatureEntries() tests', () => {
@@ -103,6 +143,66 @@ describe('user-storage/services.ts - getUserStorageAllFeatureEntries() tests', (
     expect(result).toStrictEqual([MOCK_STORAGE_DATA]);
   });
 
+  it('re-encrypts data if received entries were encrypted with non-empty salts, and saves it back to user storage', async () => {
+    // This corresponds to [['entry1', 'data1'], ['entry2', 'data2'], ['HASHED_KEY', '{ "hello": "world" }']]
+    // Each entry has been encrypted with a non-empty salt, except for the last entry
+    // The last entry is used to test if the function can handle entries with either empty or non-empty salts
+    const mockResponse = [
+      {
+        HashedKey: 'entry1',
+        Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      {
+        HashedKey: 'entry2',
+        Data: '{"v":"1","t":"scrypt","d":"3ioo9bxhjDjTmJWIGQMnOlnfa4ysuUNeLYTTmJ+qrq7gwI6hURH3ooUcBldJkHtvuQ==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      await createMockGetStorageResponse(),
+    ];
+
+    const mockGetUserStorageAllFeatureEntries =
+      await mockEndpointGetUserStorageAllFeatureEntries(
+        USER_STORAGE_FEATURE_NAMES.notifications,
+        {
+          status: 200,
+          body: JSON.stringify(mockResponse),
+        },
+      );
+
+    const mockBatchUpsertUserStorage = mockEndpointBatchUpsertUserStorage(
+      USER_STORAGE_FEATURE_NAMES.notifications,
+      undefined,
+      async (_uri, requestBody) => {
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const doEntriesHaveDifferentSalts = getIfEntriesHaveDifferentSalts(
+          Object.entries(requestBody.data).map((entry) => entry[1] as string),
+        );
+
+        expect(doEntriesHaveDifferentSalts).toBe(false);
+
+        const doEntriesHaveEmptySalts = Object.entries(requestBody.data).every(
+          ([_entryKey, entryValue]) =>
+            encryption.getSalt(entryValue as string).length === 0,
+        );
+
+        expect(doEntriesHaveEmptySalts).toBe(true);
+
+        const wereOnlyNonEmptySaltEntriesUploaded =
+          Object.entries(requestBody.data).length === 2;
+
+        expect(wereOnlyNonEmptySaltEntriesUploaded).toBe(true);
+      },
+    );
+
+    const result = await actCallGetUserStorageAllFeatureEntries();
+
+    mockGetUserStorageAllFeatureEntries.done();
+    mockBatchUpsertUserStorage.done();
+    expect(result).toStrictEqual(['data1', 'data2', MOCK_STORAGE_DATA]);
+  });
+
   it('returns null if endpoint does not have entry', async () => {
     const mockGetUserStorage =
       await mockEndpointGetUserStorageAllFeatureEntries(

packages/profile-sync-controller/src/controllers/user-storage/services.ts

@@ -93,6 +93,12 @@ export async function getUserStorage(
       nativeScryptCrypto,
     );
 
+    // Re-encrypt the entry if the salt is non-empty
+    const salt = encryption.getSalt(encryptedData);
+    if (salt.length) {
+      await upsertUserStorage(decryptedData, opts);
+    }
+
     return decryptedData;
   } catch (e) {
     log.error('Failed to get user storage', e);
@@ -137,6 +143,7 @@ export async function getUserStorageAllFeatureEntries(
     }
 
     const decryptedData: string[] = [];
+    const reEncryptedEntries: [string, string][] = [];
 
     for (const entry of userStorage) {
       /* istanbul ignore if - unreachable if statement, but kept as edge case */
@@ -151,11 +158,29 @@ export async function getUserStorageAllFeatureEntries(
           nativeScryptCrypto,
         );
         decryptedData.push(data);
+
+        // Re-encrypt the entry if the salt is non-empty
+        const salt = encryption.getSalt(entry.Data);
+        if (salt.length) {
+          reEncryptedEntries.push([
+            entry.HashedKey,
+            await encryption.encryptString(
+              data,
+              opts.storageKey,
+              nativeScryptCrypto,
+            ),
+          ]);
+        }
       } catch {
         // do nothing
       }
     }
 
+    // Re-upload the re-encrypted entries
+    if (reEncryptedEntries.length) {
+      await batchUpsertUserStorage(reEncryptedEntries, opts);
+    }
+
     return decryptedData;
   } catch (e) {
     log.error('Failed to get user storage', e);

packages/profile-sync-controller/src/sdk/__fixtures__/mock-userstorage.ts

@@ -1,6 +1,6 @@
 import nock from 'nock';
 
-import encryption from '../../shared/encryption';
+import encryption, { createSHA256Hash } from '../../shared/encryption';
 import { Env } from '../../shared/env';
 import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema';
 import { STORAGE_URL } from '../user-storage';
@@ -20,19 +20,19 @@ const MOCK_STORAGE_URL_ALL_FEATURE_ENTRIES = STORAGE_URL(
   USER_STORAGE_FEATURE_NAMES.notifications,
 );
 
-export const MOCK_STORAGE_KEY = 'MOCK_STORAGE_KEY';
+export const MOCK_STORAGE_KEY = createSHA256Hash('mockStorageKey');
 // TODO: Either fix this lint violation or explain why it's necessary to ignore.
 // eslint-disable-next-line @typescript-eslint/naming-convention
-export const MOCK_NOTIFICATIONS_DATA = { is_compact: false };
-export const MOCK_NOTIFICATIONS_DATA_ENCRYPTED = async () =>
+export const MOCK_NOTIFICATIONS_DATA = '{ is_compact: false }';
+export const MOCK_NOTIFICATIONS_DATA_ENCRYPTED = async (data?: string) =>
   await encryption.encryptString(
-    JSON.stringify(MOCK_NOTIFICATIONS_DATA),
+    data ?? MOCK_NOTIFICATIONS_DATA,
     MOCK_STORAGE_KEY,
   );
 
-export const MOCK_STORAGE_RESPONSE = async () => ({
+export const MOCK_STORAGE_RESPONSE = async (data?: string) => ({
   HashedKey: '8485d2c14c333ebca415140a276adaf546619b0efc204586b73a5d400a18a5e2',
-  Data: await MOCK_NOTIFICATIONS_DATA_ENCRYPTED(),
+  Data: await MOCK_NOTIFICATIONS_DATA_ENCRYPTED(data),
 });
 
 export const handleMockUserStorageGet = async (mockReply?: MockReply) => {

packages/profile-sync-controller/src/sdk/user-storage.test.ts

@@ -1,4 +1,5 @@
 import encryption, { createSHA256Hash } from '../shared/encryption';
+import { getIfEntriesHaveDifferentSalts } from '../shared/encryption/utils';
 import { Env } from '../shared/env';
 import type { UserStorageFeatureKeys } from '../shared/storage-schema';
 import { USER_STORAGE_FEATURE_NAMES } from '../shared/storage-schema';
@@ -12,6 +13,7 @@ import {
   handleMockUserStorageDeleteAllFeatureEntries,
   handleMockUserStorageDelete,
   handleMockUserStorageBatchDelete,
+  MOCK_STORAGE_RESPONSE,
 } from './__fixtures__/mock-userstorage';
 import { arrangeAuth, typedMockFn } from './__fixtures__/test-utils';
 import type { IBaseAuth } from './authentication-jwt-bearer/types';
@@ -40,7 +42,7 @@ describe('User Storage', () => {
     const mockGet = await handleMockUserStorageGet();
 
     // Test Set
-    const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
+    const data = MOCK_NOTIFICATIONS_DATA;
     await userStorage.setItem(
       `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
       data,
@@ -71,7 +73,7 @@ describe('User Storage', () => {
     const mockGet = await handleMockUserStorageGet();
 
     // Test Set
-    const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
+    const data = MOCK_NOTIFICATIONS_DATA;
     await userStorage.setItem(
       `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
       data,
@@ -87,20 +89,114 @@ describe('User Storage', () => {
     expect(response).toBe(data);
   });
 
+  it('re-encrypts data if received entry was encrypted with a non-empty salt, and saves it back to user storage', async () => {
+    const { auth } = arrangeAuth('SRP', MOCK_SRP);
+    const { userStorage } = arrangeUserStorage(auth);
+
+    // This corresponds to 'data1'
+    // Encrypted with a non-empty salt
+    const mockResponse = {
+      HashedKey: 'entry1',
+      Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+    };
+
+    const mockGet = await handleMockUserStorageGet({
+      status: 200,
+      body: JSON.stringify(mockResponse),
+    });
+    const mockPut = handleMockUserStoragePut(
+      undefined,
+      async (_, requestBody) => {
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const isNewSaltEmpty =
+          encryption.getSalt(requestBody.data).length === 0;
+
+        expect(isNewSaltEmpty).toBe(true);
+      },
+    );
+
+    await userStorage.getItem(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+    );
+    expect(mockGet.isDone()).toBe(true);
+    expect(mockPut.isDone()).toBe(true);
+  });
+
   it('gets all feature entries', async () => {
     const { auth } = arrangeAuth('SRP', MOCK_SRP);
     const { userStorage } = arrangeUserStorage(auth);
 
     const mockGetAll = await handleMockUserStorageGetAllFeatureEntries();
 
-    const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
+    const data = MOCK_NOTIFICATIONS_DATA;
     const responseAllFeatureEntries = await userStorage.getAllFeatureItems(
       USER_STORAGE_FEATURE_NAMES.notifications,
     );
     expect(mockGetAll.isDone()).toBe(true);
     expect(responseAllFeatureEntries).toStrictEqual([data]);
   });
 
+  it('re-encrypts data if received entries were encrypted with non-empty salts, and saves it back to user storage', async () => {
+    // This corresponds to [['entry1', 'data1'], ['entry2', 'data2'], ['HASHED_KEY', '{ "hello": "world" }']]
+    // Each entry has been encrypted with a non-empty salt, except for the last entry
+    // The last entry is used to test if the function can handle entries with either empty or non-empty salts
+    const mockResponse = [
+      {
+        HashedKey: 'entry1',
+        Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      {
+        HashedKey: 'entry2',
+        Data: '{"v":"1","t":"scrypt","d":"3ioo9bxhjDjTmJWIGQMnOlnfa4ysuUNeLYTTmJ+qrq7gwI6hURH3ooUcBldJkHtvuQ==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      await MOCK_STORAGE_RESPONSE('data3'),
+    ];
+
+    const { auth } = arrangeAuth('SRP', MOCK_SRP);
+    const { userStorage } = arrangeUserStorage(auth);
+
+    const mockGetAll = await handleMockUserStorageGetAllFeatureEntries({
+      status: 200,
+      body: mockResponse,
+    });
+
+    const mockPut = handleMockUserStoragePut(
+      undefined,
+      async (_, requestBody) => {
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const doEntriesHaveDifferentSalts = getIfEntriesHaveDifferentSalts(
+          Object.entries(requestBody.data).map((entry) => entry[1] as string),
+        );
+
+        expect(doEntriesHaveDifferentSalts).toBe(false);
+
+        const doEntriesHaveEmptySalts = Object.entries(requestBody.data).every(
+          ([_entryKey, entryValue]) =>
+            encryption.getSalt(entryValue as string).length === 0,
+        );
+
+        expect(doEntriesHaveEmptySalts).toBe(true);
+
+        const wereOnlyNonEmptySaltEntriesUploaded =
+          Object.entries(requestBody.data).length === 2;
+
+        expect(wereOnlyNonEmptySaltEntriesUploaded).toBe(true);
+      },
+    );
+
+    await userStorage.getAllFeatureItems(
+      USER_STORAGE_FEATURE_NAMES.notifications,
+    );
+    expect(mockGetAll.isDone()).toBe(true);
+    expect(mockPut.isDone()).toBe(true);
+  });
+
   it('batch set items', async () => {
     const dataToStore: [
       UserStorageFeatureKeys<typeof USER_STORAGE_FEATURE_NAMES.accounts>,