feat: add optional validateAgainstSchema option when creating user storage entry paths (#5326)

## Explanation

This PR adds a new `validateAgainstSchema` option when creating user
storage entry paths.
This defaults to true, but can be relaxed to false when using the SDK.
SDK users should use the feature and key names they decide to use, and
Controller users should follow the internal schema.

## References

Related to: https://consensyssoftware.atlassian.net/browse/IDENTITY-31

## Changelog

### `@metamask/profile-sync-controller`

- **ADDED**: Optional `validateAgainstSchema` option when creating user
storage entry paths

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: mathieuartu

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

@@ -5,10 +5,10 @@ import { SHARED_SALT } from '../shared/encryption/constants';
 import type { Env } from '../shared/env';
 import { getEnvUrls } from '../shared/env';
 import type {
-  UserStorageFeatureKeys,
-  UserStorageFeatureNames,
-  UserStoragePathWithFeatureAndKey,
-  UserStoragePathWithFeatureOnly,
+  UserStorageGenericFeatureKey,
+  UserStorageGenericFeatureName,
+  UserStorageGenericPathWithFeatureAndKey,
+  UserStorageGenericPathWithFeatureOnly,
 } from '../shared/storage-schema';
 import { createEntryPath } from '../shared/storage-schema';
 
@@ -54,42 +54,46 @@ export class UserStorage {
   }
 
   async setItem(
-    path: UserStoragePathWithFeatureAndKey,
+    path: UserStorageGenericPathWithFeatureAndKey,
     value: string,
   ): Promise<void> {
     await this.#upsertUserStorage(path, value);
   }
 
-  async batchSetItems<FeatureName extends UserStorageFeatureNames>(
-    path: FeatureName,
-    values: [UserStorageFeatureKeys<FeatureName>, string][],
+  async batchSetItems(
+    path: UserStorageGenericFeatureName,
+    values: [UserStorageGenericFeatureKey, string][],
   ) {
     await this.#batchUpsertUserStorage(path, values);
   }
 
-  async getItem(path: UserStoragePathWithFeatureAndKey): Promise<string> {
+  async getItem(
+    path: UserStorageGenericPathWithFeatureAndKey,
+  ): Promise<string> {
     return this.#getUserStorage(path);
   }
 
   async getAllFeatureItems(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericFeatureName,
   ): Promise<string[] | null> {
     return this.#getUserStorageAllFeatureEntries(path);
   }
 
-  async deleteItem(path: UserStoragePathWithFeatureAndKey): Promise<void> {
+  async deleteItem(
+    path: UserStorageGenericPathWithFeatureAndKey,
+  ): Promise<void> {
     return this.#deleteUserStorage(path);
   }
 
   async deleteAllFeatureItems(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericFeatureName,
   ): Promise<void> {
     return this.#deleteUserStorageAllFeatureEntries(path);
   }
 
   async batchDeleteItems(
-    path: UserStoragePathWithFeatureOnly,
-    values: string[],
+    path: UserStorageGenericFeatureName,
+    values: UserStorageGenericFeatureKey[],
   ) {
     return this.#batchDeleteUserStorage(path, values);
   }
@@ -110,14 +114,16 @@ export class UserStorage {
   }
 
   async #upsertUserStorage(
-    path: UserStoragePathWithFeatureAndKey,
+    path: UserStorageGenericPathWithFeatureAndKey,
     data: string,
   ): Promise<void> {
     try {
       const headers = await this.#getAuthorizationHeader();
       const storageKey = await this.getStorageKey();
       const encryptedData = await encryption.encryptString(data, storageKey);
-      const encryptedPath = createEntryPath(path, storageKey);
+      const encryptedPath = createEntryPath(path, storageKey, {
+        validateAgainstSchema: false,
+      });
 
       const url = new URL(STORAGE_URL(this.env, encryptedPath));
 
@@ -150,7 +156,7 @@ export class UserStorage {
   }
 
   async #batchUpsertUserStorage(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericPathWithFeatureOnly,
     data: [string, string][],
   ): Promise<void> {
     try {
@@ -201,7 +207,7 @@ export class UserStorage {
   }
 
   async #batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericPathWithFeatureOnly,
     encryptedData: [string, string][],
   ): Promise<void> {
     try {
@@ -242,12 +248,14 @@ export class UserStorage {
   }
 
   async #getUserStorage(
-    path: UserStoragePathWithFeatureAndKey,
+    path: UserStorageGenericPathWithFeatureAndKey,
   ): Promise<string> {
     try {
       const headers = await this.#getAuthorizationHeader();
       const storageKey = await this.getStorageKey();
-      const encryptedPath = createEntryPath(path, storageKey);
+      const encryptedPath = createEntryPath(path, storageKey, {
+        validateAgainstSchema: false,
+      });
 
       const url = new URL(STORAGE_URL(this.env, encryptedPath));
 
@@ -300,7 +308,7 @@ export class UserStorage {
   }
 
   async #getUserStorageAllFeatureEntries(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericPathWithFeatureOnly,
   ): Promise<string[] | null> {
     try {
       const headers = await this.#getAuthorizationHeader();
@@ -383,12 +391,14 @@ export class UserStorage {
   }
 
   async #deleteUserStorage(
-    path: UserStoragePathWithFeatureAndKey,
+    path: UserStorageGenericPathWithFeatureAndKey,
   ): Promise<void> {
     try {
       const headers = await this.#getAuthorizationHeader();
       const storageKey = await this.getStorageKey();
-      const encryptedPath = createEntryPath(path, storageKey);
+      const encryptedPath = createEntryPath(path, storageKey, {
+        validateAgainstSchema: false,
+      });
 
       const url = new URL(STORAGE_URL(this.env, encryptedPath));
 
@@ -428,7 +438,7 @@ export class UserStorage {
   }
 
   async #deleteUserStorageAllFeatureEntries(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericPathWithFeatureOnly,
   ): Promise<void> {
     try {
       const headers = await this.#getAuthorizationHeader();
@@ -469,7 +479,7 @@ export class UserStorage {
   }
 
   async #batchDeleteUserStorage(
-    path: UserStoragePathWithFeatureOnly,
+    path: UserStorageGenericPathWithFeatureOnly,
     data: string[],
   ): Promise<void> {
     try {

packages/profile-sync-controller/src/shared/storage-schema.test.ts

@@ -49,6 +49,15 @@ describe('user-storage/schema.ts', () => {
       );
     });
 
+    it('should not throw errors if validateAgainstSchema is false', () => {
+      const path = 'invalid.feature';
+      expect(() =>
+        getFeatureAndKeyFromPath(path, {
+          validateAgainstSchema: false,
+        }),
+      ).not.toThrow();
+    });
+
     it('should return feature and key from path', () => {
       const result = getFeatureAndKeyFromPath(
         `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
@@ -68,5 +77,15 @@ describe('user-storage/schema.ts', () => {
         key: '0x123',
       });
     });
+
+    it('should return feature and key from path with arbitrary feature and key when validateAgainstSchema is false', () => {
+      const result = getFeatureAndKeyFromPath('feature.key', {
+        validateAgainstSchema: false,
+      });
+      expect(result).toStrictEqual({
+        feature: 'feature',
+        key: 'key',
+      });
+    });
   });
 });

packages/profile-sync-controller/src/shared/storage-schema.ts

@@ -41,9 +41,36 @@ export type UserStoragePathWithFeatureAndKey = {
   [K in UserStorageFeatureNames]: `${K}.${UserStorageFeatureKeys<K>}`;
 }[UserStoragePathWithFeatureOnly];
 
-export const getFeatureAndKeyFromPath = (
-  path: UserStoragePathWithFeatureAndKey,
-): UserStorageFeatureAndKey => {
+/**
+ * The below types are mainly used for the SDK.
+ * These exist so that the SDK can be used with arbitrary feature names and keys.
+ *
+ * We only type enforce feature names and keys when using UserStorageController.
+ * This is done so we don't end up with magic strings within the applications.
+ */
+
+export type UserStorageGenericFeatureName = string;
+export type UserStorageGenericFeatureKey = string;
+export type UserStorageGenericPathWithFeatureAndKey =
+  `${UserStorageGenericFeatureName}.${UserStorageGenericFeatureKey}`;
+export type UserStorageGenericPathWithFeatureOnly =
+  UserStorageGenericFeatureName;
+
+type UserStorageGenericFeatureAndKey = {
+  feature: UserStorageGenericFeatureName;
+  key: UserStorageGenericFeatureKey;
+};
+
+export const getFeatureAndKeyFromPath = <T extends boolean>(
+  path: T extends true
+    ? UserStoragePathWithFeatureAndKey
+    : UserStorageGenericPathWithFeatureAndKey,
+  options: {
+    validateAgainstSchema: T;
+  } = { validateAgainstSchema: true as T },
+): T extends true
+  ? UserStorageFeatureAndKey
+  : UserStorageGenericFeatureAndKey => {
   const pathRegex = /^\w+\.\w+$/u;
 
   if (!pathRegex.test(path)) {
@@ -52,29 +79,41 @@ export const getFeatureAndKeyFromPath = (
     );
   }
 
-  const [feature, key] = path.split('.') as [
-    UserStorageFeatureNames,
-    UserStorageFeatureKeys<UserStorageFeatureNames>,
-  ];
-
-  if (!(feature in USER_STORAGE_SCHEMA)) {
-    throw new Error(`user-storage - invalid feature provided: ${feature}`);
-  }
-
-  const validFeature = USER_STORAGE_SCHEMA[feature] as readonly string[];
-
-  if (
-    !validFeature.includes(key) &&
-    !validFeature.includes(ALLOW_ARBITRARY_KEYS)
-  ) {
-    const validKeys = USER_STORAGE_SCHEMA[feature].join(', ');
-
-    throw new Error(
-      `user-storage - invalid key provided for this feature: ${key}. Valid keys: ${validKeys}`,
-    );
+  const [feature, key] = path.split('.');
+
+  if (options.validateAgainstSchema) {
+    const featureToValidate = feature as UserStorageFeatureNames;
+    const keyToValidate = key as UserStorageFeatureKeys<
+      typeof featureToValidate
+    >;
+
+    if (!(featureToValidate in USER_STORAGE_SCHEMA)) {
+      throw new Error(
+        `user-storage - invalid feature provided: ${featureToValidate}. Valid features: ${Object.keys(
+          USER_STORAGE_SCHEMA,
+        ).join(', ')}`,
+      );
+    }
+
+    const validFeature = USER_STORAGE_SCHEMA[
+      featureToValidate
+    ] as readonly string[];
+
+    if (
+      !validFeature.includes(keyToValidate) &&
+      !validFeature.includes(ALLOW_ARBITRARY_KEYS)
+    ) {
+      const validKeys = USER_STORAGE_SCHEMA[featureToValidate].join(', ');
+
+      throw new Error(
+        `user-storage - invalid key provided for this feature: ${keyToValidate}. Valid keys: ${validKeys}`,
+      );
+    }
   }
 
-  return { feature, key };
+  return { feature, key } as T extends true
+    ? UserStorageFeatureAndKey
+    : UserStorageGenericFeatureAndKey;
 };
 
 export const isPathWithFeatureAndKey = (
@@ -92,13 +131,21 @@ export const isPathWithFeatureAndKey = (
  *
  * @param path - string in the form of `${feature}.${key}` that matches schema
  * @param storageKey - users storage key
+ * @param options - options object
+ * @param options.validateAgainstSchema - whether to validate the path against the schema.
+ * This defaults to true, and should only be set to false when using the SDK with arbitrary feature names and keys.
  * @returns path to store entry
  */
-export function createEntryPath(
-  path: UserStoragePathWithFeatureAndKey,
+export function createEntryPath<T extends boolean>(
+  path: T extends true
+    ? UserStoragePathWithFeatureAndKey
+    : UserStorageGenericPathWithFeatureAndKey,
   storageKey: string,
+  options: {
+    validateAgainstSchema: T;
+  } = { validateAgainstSchema: true as T },
 ): string {
-  const { feature, key } = getFeatureAndKeyFromPath(path);
+  const { feature, key } = getFeatureAndKeyFromPath(path, options);
   const hashedKey = createSHA256Hash(key + storageKey);
 
   return `${feature}/${hashedKey}`;