fix: first granted permission doesn't save (#261)

* Fix incorrect setStorageKey() signature

* After writing to profile sync, fetch the stored data to ensure that it is retrievable before returning ot the caller. Do not skip permissions that fail to serialize.

* Remove unnecessary calls to getAccessToken()
Plus minor changes:
- Improve a comment relating to maximum permissions size
- Remove unnecessary type assertion in test.

* Fixes from review:
- Remove size limitation
- Add performance metrics to profilesync store operations
- Decompose validation of stored permission into shared function with improved error messages

* Remove auth from profileSyncManager factory ctor

* Fix linting

Author: jeffsmale90

packages/gator-permissions-snap/src/profileSync/options.ts

@@ -50,11 +50,11 @@ export const createProfileSyncOptions = (
    * Uses snap storage to persist persist storage key
    */
   const keyStorageOptions: StorageOptions = {
-    getStorageKey: async () => {
+    getStorageKey: async (_message: `metamask:${string}`) => {
       const state = await stateManager.getState();
       return state.profileSyncUserStorageKey;
     },
-    setStorageKey: async (val: string) => {
+    setStorageKey: async (_message: `metamask:${string}`, val: string) => {
       const state = await stateManager.getState();
       await stateManager.setState({
         ...state,

packages/gator-permissions-snap/src/profileSync/profileSync.ts

@@ -12,12 +12,11 @@ import { hashDelegation, decodeDelegations } from '@metamask/delegation-core';
 import type { Hex } from '@metamask/delegation-core';
 import type {
   UserStorageGenericPathWithFeatureAndKey,
-  JwtBearerAuth,
   UserStorage,
 } from '@metamask/profile-sync-controller/sdk';
 import {
+  InternalError,
   InvalidInputError,
-  LimitExceededError,
   ParseError,
   UnsupportedMethodError,
 } from '@metamask/snaps-sdk';
@@ -30,9 +29,6 @@ export type RevocationMetadata = {
   recordedAt: number;
 };
 
-// Constants for validation
-const MAX_STORAGE_SIZE_BYTES = 400 * 1024; // 400kb limit as documented
-
 // Zod schema for runtime validation of StoredGrantedPermission
 const zStoredGrantedPermission = z.object({
   permissionResponse: zPermissionResponse,
@@ -83,10 +79,9 @@ function safeDeserializeStoredGrantedPermission(
 }
 
 /**
- * Safely serializes a StoredGrantedPermission object with size validation.
+ * Safely serializes a StoredGrantedPermission object.
  * @param permission - The permission object to serialize.
  * @returns The JSON string.
- * @throws LimitExceededError if size limit exceeded.
  */
 function safeSerializeStoredGrantedPermission(
   permission: StoredGrantedPermission,
@@ -98,19 +93,12 @@ function safeSerializeStoredGrantedPermission(
     // Serialize to JSON
     const jsonString = JSON.stringify(validated);
 
-    // Check size limit
-    const sizeBytes = new TextEncoder().encode(jsonString).length;
-    if (sizeBytes > MAX_STORAGE_SIZE_BYTES) {
-      throw new LimitExceededError(
-        `Permission data exceeds size limit: ${sizeBytes} bytes > ${MAX_STORAGE_SIZE_BYTES} bytes`,
-      );
-    }
-
     return jsonString;
   } catch (error) {
     if (error instanceof z.ZodError) {
       throw new InvalidInputError(extractZodError(error.errors));
     }
+
     throw error;
   }
 }
@@ -153,7 +141,6 @@ export function generateObjectKey(permissionContext: Hex): Hex {
 }
 
 export type ProfileSyncManagerConfig = {
-  auth: JwtBearerAuth;
   userStorage: UserStorage;
   isFeatureEnabled: boolean;
   snapsMetricsService?: SnapsMetricsService;
@@ -168,7 +155,7 @@ export function createProfileSyncManager(
   config: ProfileSyncManagerConfig,
 ): ProfileSyncManager {
   const FEATURE = 'gator_7715_permissions';
-  const { auth, userStorage, isFeatureEnabled, snapsMetricsService } = config;
+  const { userStorage, isFeatureEnabled, snapsMetricsService } = config;
   const unConfiguredProfileSyncManager = {
     getAllGrantedPermissions: async (): Promise<StoredGrantedPermission[]> => {
       logger.debug('unConfiguredProfileSyncManager.getAllGrantedPermissions()');
@@ -201,19 +188,6 @@ export function createProfileSyncManager(
     },
   };
 
-  /**
-   * Authenticates the user with profile sync.
-   *
-   */
-  async function authenticate(): Promise<void> {
-    try {
-      await auth.getAccessToken();
-    } catch (error) {
-      logger.error('Error fetching access token:', error);
-      throw error;
-    }
-  }
-
   /**
    * Retrieve all granted permission items under the "7715_permissions" feature will result in GET /api/v1/userstorage/7715_permissions
    * VALUES: decrypted("JSONstringifyPermission1", storage_key), decrypted("JSONstringifyPermission2", storage_key).
@@ -223,8 +197,6 @@ export function createProfileSyncManager(
     StoredGrantedPermission[]
   > {
     try {
-      await authenticate();
-
       const items = await userStorage.getAllFeatureItems(FEATURE);
       if (!items) {
         await snapsMetricsService?.trackProfileSync({
@@ -271,8 +243,6 @@ export function createProfileSyncManager(
     permissionContext: Hex,
   ): Promise<StoredGrantedPermission | null> {
     try {
-      await authenticate();
-
       const path: UserStorageGenericPathWithFeatureAndKey = `${FEATURE}.${generateObjectKey(permissionContext)}`;
       const permission = await userStorage.getItem(path);
 
@@ -287,6 +257,36 @@ export function createProfileSyncManager(
     }
   }
 
+  /**
+   * Validates that the stored permission value for the given key matches the expected value.
+   *
+   * Retrieves the item from user storage using the provided key and compares it to the expected value.
+   * Throws an InternalError if the item does not exist or if the stored value does not match the expected value.
+   *
+   * @param key - The key for the item in user storage.
+   * @param expectedValue - The expected serialized value to compare against the retrieved value.
+   * @throws {InternalError} If the stored item is missing or does not match the expected value.
+   */
+  async function validatePermissionStored(
+    key: string,
+    expectedValue: string,
+  ): Promise<void> {
+    const path: UserStorageGenericPathWithFeatureAndKey = `${FEATURE}.${key}`;
+    const retrievedValue = await userStorage.getItem(path);
+
+    if (!retrievedValue) {
+      throw new InternalError(
+        `Unable to retrieve stored item with key: ${key}`,
+      );
+    }
+
+    if (expectedValue !== retrievedValue) {
+      throw new InternalError(
+        `Retrieved stored item with key: ${key} but does not match expected value`,
+      );
+    }
+  }
+
   /**
    * Store the granted permission in profile sync.
    *
@@ -300,27 +300,44 @@ export function createProfileSyncManager(
   async function storeGrantedPermission(
     storedGrantedPermission: StoredGrantedPermission,
   ): Promise<void> {
+    const performanceData = {
+      permissionCount: 1,
+      permissionsStoredElapsedMs: -1,
+      storedPermissionsValidatedElapsedMs: -1,
+    };
     try {
-      await authenticate();
-
+      const startTime = Date.now();
       // Validate and serialize with size check
       const serializedPermission = safeSerializeStoredGrantedPermission(
         storedGrantedPermission,
       );
 
-      const path: UserStorageGenericPathWithFeatureAndKey = `${FEATURE}.${generateObjectKey(storedGrantedPermission.permissionResponse.context)}`;
+      const key = generateObjectKey(
+        storedGrantedPermission.permissionResponse.context,
+      );
+
+      const path: UserStorageGenericPathWithFeatureAndKey = `${FEATURE}.${key}`;
       await userStorage.setItem(path, serializedPermission);
 
+      performanceData.permissionsStoredElapsedMs = Date.now() - startTime;
+
+      await validatePermissionStored(key, serializedPermission);
+
+      performanceData.storedPermissionsValidatedElapsedMs =
+        Date.now() - startTime;
+
       await snapsMetricsService?.trackProfileSync({
         operation: 'store',
         success: true,
+        performanceData,
       });
     } catch (error) {
       logger.error('Error storing granted permission');
       await snapsMetricsService?.trackProfileSync({
         operation: 'store',
         success: false,
         errorMessage: (error as Error).message,
+        performanceData,
       });
       throw error;
     }
@@ -339,23 +356,25 @@ export function createProfileSyncManager(
   async function storeGrantedPermissionBatch(
     storedGrantedPermissions: StoredGrantedPermission[],
   ): Promise<void> {
-    try {
-      await authenticate();
+    const performanceData = {
+      permissionCount: storedGrantedPermissions.length,
+      permissionsStoredElapsedMs: -1,
+      storedPermissionsValidatedElapsedMs: -1,
+    };
 
+    try {
+      const startTime = Date.now();
       // Validate and serialize all permissions with size checks
       const validatedItems: [string, string][] = [];
 
       for (const permission of storedGrantedPermissions) {
-        try {
-          const serializedPermission =
-            safeSerializeStoredGrantedPermission(permission);
-          validatedItems.push([
-            generateObjectKey(permission.permissionResponse.context), // key
-            serializedPermission, // value
-          ]);
-        } catch {
-          logger.warn('Skipping invalid permission in batch');
-        }
+        const serializedPermission =
+          safeSerializeStoredGrantedPermission(permission);
+
+        validatedItems.push([
+          generateObjectKey(permission.permissionResponse.context), // key
+          serializedPermission, // value
+        ]);
       }
 
       if (validatedItems.length === 0) {
@@ -366,16 +385,30 @@ export function createProfileSyncManager(
 
       await userStorage.batchSetItems(FEATURE, validatedItems);
 
+      performanceData.permissionsStoredElapsedMs = Date.now() - startTime;
+
+      // ensure that all items were stored correctly
+      const fetchedItems = validatedItems.map(async ([key, value]) =>
+        validatePermissionStored(key, value),
+      );
+
+      await Promise.all(fetchedItems);
+
+      performanceData.storedPermissionsValidatedElapsedMs =
+        Date.now() - startTime;
+
       await snapsMetricsService?.trackProfileSync({
         operation: 'batch_store',
         success: true,
+        performanceData,
       });
     } catch (error) {
       logger.error('Error storing granted permission batch');
       await snapsMetricsService?.trackProfileSync({
         operation: 'batch_store',
         success: false,
         errorMessage: (error as Error).message,
+        performanceData,
       });
       throw error;
     }
@@ -411,8 +444,6 @@ export function createProfileSyncManager(
         revocationMetadata,
       });
 
-      await authenticate();
-
       const updatedPermission: StoredGrantedPermission = {
         ...existingPermission,
         revocationMetadata,

packages/gator-permissions-snap/src/services/snapsMetricsService.ts

@@ -95,6 +95,8 @@ export type TrackProfileSyncParams = {
   success: boolean;
   /** Optional error message if operation failed */
   errorMessage?: string;
+  /** Optional performance data for the operation */
+  performanceData?: Record<string, number>;
 };
 
 /**
@@ -254,13 +256,18 @@ export class SnapsMetricsService {
    * @param params - The tracking parameters.
    */
   async trackProfileSync(params: TrackProfileSyncParams): Promise<void> {
+    const { operation, success, errorMessage, performanceData } = params;
+
+    const message = success
+      ? `Profile sync ${operation} successful`
+      : `Profile sync ${operation} failed`;
+
     await this.#trackEvent('Profile Sync', {
-      message: params.success
-        ? `Profile sync ${params.operation} successful`
-        : `Profile sync ${params.operation} failed`,
-      operation: params.operation,
-      success: params.success,
-      ...(params.errorMessage ? { error_message: params.errorMessage } : {}),
+      message,
+      operation,
+      success,
+      ...(performanceData ? { performance_data: performanceData } : {}),
+      ...(errorMessage ? { error_message: errorMessage } : {}),
     });
   }