fix: resolve AuthenticationController 401 errors from token caching bugs

eslint-suppressions.json

@@ -1365,7 +1365,7 @@
       "count": 2
     },
     "@typescript-eslint/prefer-nullish-coalescing": {
-      "count": 2
+      "count": 1
     }
   },
   "packages/profile-sync-controller/src/controllers/authentication/__fixtures__/mockServices.ts": {
@@ -1605,11 +1605,6 @@
       "count": 1
     }
   },
-  "packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 1
-    }
-  },
   "packages/profile-sync-controller/src/shared/encryption/cache.ts": {
     "@typescript-eslint/explicit-function-return-type": {
       "count": 1

packages/profile-metrics-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Move bearer token acquisition inside the retry loop in `ProfileMetricsService.submitMetrics` so each retry attempt fetches a fresh token instead of reusing a potentially stale one ([#8144](https://github.com/MetaMask/core/pull/8144))
+
 ## [3.0.2]
 
 ### Changed

packages/profile-metrics-controller/src/ProfileMetricsService.ts

@@ -198,11 +198,11 @@ export class ProfileMetricsService {
    * @returns The response from the API.
    */
   async submitMetrics(data: ProfileMetricsSubmitMetricsRequest): Promise<void> {
-    const authToken = await this.#messenger.call(
-      'AuthenticationController:getBearerToken',
-      data.entropySourceId ?? undefined,
-    );
     await this.#policy.execute(async () => {
+      const authToken = await this.#messenger.call(
+        'AuthenticationController:getBearerToken',
+        data.entropySourceId ?? undefined,
+      );
       const url = new URL(`${this.#baseURL}/profile/accounts`);
       const localResponse = await this.#fetch(url, {
         method: 'PUT',

packages/profile-sync-controller/CHANGELOG.md

@@ -20,10 +20,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING:** Add client-side JWT `exp` claim validation to prevent stale cached tokens from being returned ([#8144](https://github.com/MetaMask/core/pull/8144))
+  - `validateLoginResponse` now decodes the JWT `exp` claim and rejects tokens that have actually expired, regardless of client-side TTL tracking (`obtainedAt`/`expiresIn`)
+  - Non-JWT access tokens are now rejected as invalid. In production this has no effect (access tokens are always JWTs from the OIDC server), but E2E test mocks that use raw identifier strings as access tokens must be updated. `getMockAuthAccessTokenResponse` now wraps identifiers in a JWT; consumers should use `getE2EIdentifierFromJwt` (newly exported) to extract the identifier from the bearer token in mock servers.
 - **BREAKING:** Standardize names of `AuthenticationController` and `UserStorageController` messenger action types ([#7976](https://github.com/MetaMask/core/pull/7976/))
   - All existing types for messenger actions have been renamed so they end in `Action` (e.g. `AuthenticationControllerPerformSignIn` -> `AuthenticationControllerPerformSignInAction`). You will need to update imports appropriately.
   - This change only affects the types. The action type strings themselves have not changed, so you do not need to update the list of actions you pass when initializing `AuthenticationController` and `UserStorageController` messengers.
 
+### Fixed
+
+- Fix `AuthenticationController` silently discarding tokens when `entropySourceId` is `undefined` ([#8144](https://github.com/MetaMask/core/pull/8144))
+  - `getBearerToken`, `getSessionProfile`, and `getUserProfileLineage` now resolve `undefined` `entropySourceId` to the primary SRP entropy source ID via the message-signing snap before delegating to the auth SDK
+  - This also eliminates a login deduplication race condition where `getBearerToken(undefined)` and `getBearerToken("primary-srp-id")` would trigger two independent OIDC logins for the same identity
+- Update `getUserProfileLineage` to accept an optional `entropySourceId` parameter ([#8144](https://github.com/MetaMask/core/pull/8144))
+
 ## [27.1.0]
 
 ### Changed

packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts

@@ -355,6 +355,95 @@ describe('AuthenticationController', () => {
         expect.any(Error),
       );
     });
+
+    it('resolves undefined entropySourceId to primary and stores token', async () => {
+      const metametrics = createMockAuthMetaMetrics();
+      const { messenger, mockSnapGetAllPublicKeys } =
+        createMockAuthenticationMessenger();
+      arrangeAuthAPIs();
+
+      const controller = new AuthenticationController({
+        messenger,
+        metametrics,
+      });
+
+      const result = await controller.getBearerToken();
+      expect(result).toBe(MOCK_OATH_TOKEN_RESPONSE.access_token);
+
+      expect(mockSnapGetAllPublicKeys).toHaveBeenCalled();
+      expect(controller.state.isSignedIn).toBe(true);
+      expect(
+        controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]],
+      ).toBeDefined();
+    });
+
+    it('returns the same token for undefined and explicit primary entropySourceId', async () => {
+      const metametrics = createMockAuthMetaMetrics();
+      const { messenger } = createMockAuthenticationMessenger();
+      const originalState = mockSignedInState();
+      const controller = new AuthenticationController({
+        messenger,
+        state: originalState,
+        metametrics,
+      });
+
+      const resultUndefined = await controller.getBearerToken();
+      const resultExplicit = await controller.getBearerToken(
+        MOCK_ENTROPY_SOURCE_IDS[0],
+      );
+      expect(resultUndefined).toBe(resultExplicit);
+    });
+
+    it('caches the primary entropySourceId resolution across calls', async () => {
+      const metametrics = createMockAuthMetaMetrics();
+      const { messenger, mockSnapGetAllPublicKeys } =
+        createMockAuthenticationMessenger();
+      const originalState = mockSignedInState();
+      const controller = new AuthenticationController({
+        messenger,
+        state: originalState,
+        metametrics,
+      });
+
+      await controller.getBearerToken();
+      await controller.getBearerToken();
+      await controller.getBearerToken();
+
+      // getAllPublicKeys should only be called once despite multiple getBearerToken calls
+      expect(mockSnapGetAllPublicKeys).toHaveBeenCalledTimes(1);
+    });
+
+    it('throws when snap returns no entropy sources', async () => {
+      const metametrics = createMockAuthMetaMetrics();
+      const { messenger, mockSnapGetAllPublicKeys } =
+        createMockAuthenticationMessenger();
+      mockSnapGetAllPublicKeys.mockResolvedValue([]);
+
+      const controller = new AuthenticationController({
+        messenger,
+        metametrics,
+      });
+
+      await expect(controller.getBearerToken()).rejects.toThrow(
+        'No entropy sources found from snap',
+      );
+    });
+
+    it('throws when primary entropy source ID is undefined', async () => {
+      const metametrics = createMockAuthMetaMetrics();
+      const { messenger, mockSnapGetAllPublicKeys } =
+        createMockAuthenticationMessenger();
+      mockSnapGetAllPublicKeys.mockResolvedValue([[undefined, 'MOCK_KEY']]);
+
+      const controller = new AuthenticationController({
+        messenger,
+        metametrics,
+      });
+
+      await expect(controller.getBearerToken()).rejects.toThrow(
+        'Primary entropy source ID is undefined',
+      );
+    });
   });
 
   describe('getSessionProfile', () => {
@@ -657,7 +746,7 @@ describe('metadata', () => {
               "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
             },
             "token": {
-              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
+              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
               "expiresIn": 1000,
               "obtainedAt": 0,
             },
@@ -669,7 +758,7 @@ describe('metadata', () => {
               "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
             },
             "token": {
-              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
+              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
               "expiresIn": 1000,
               "obtainedAt": 0,
             },
@@ -704,7 +793,7 @@ describe('metadata', () => {
               "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
             },
             "token": {
-              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
+              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
               "expiresIn": 1000,
               "obtainedAt": 0,
             },
@@ -716,7 +805,7 @@ describe('metadata', () => {
               "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
             },
             "token": {
-              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
+              "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
               "expiresIn": 1000,
               "obtainedAt": 0,
             },

packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts

@@ -141,6 +141,8 @@ export class AuthenticationController extends BaseController<
 
   #isUnlocked = false;
 
+  #cachedPrimaryEntropySourceId?: string;
+
   readonly #keyringController = {
     setupLockedStateSubscriptions: () => {
       const { isUnlocked } = this.messenger.call('KeyringController:getState');
@@ -219,43 +221,33 @@ export class AuthenticationController extends BaseController<
   async #getLoginResponseFromState(
     entropySourceId?: string,
   ): Promise<LoginResponse | null> {
-    if (entropySourceId) {
-      if (!this.state.srpSessionData?.[entropySourceId]) {
-        return null;
-      }
-      return this.state.srpSessionData[entropySourceId];
-    }
-
-    const primarySrpLoginResponse = Object.values(
-      this.state.srpSessionData || {},
-    )?.[0];
-
-    if (!primarySrpLoginResponse) {
+    const resolvedId =
+      entropySourceId ?? (await this.#getPrimaryEntropySourceId());
+    if (!this.state.srpSessionData?.[resolvedId]) {
       return null;
     }
-
-    return primarySrpLoginResponse;
+    return this.state.srpSessionData[resolvedId];
   }
 
   async #setLoginResponseToState(
     loginResponse: LoginResponse,
     entropySourceId?: string,
   ) {
+    const resolvedId =
+      entropySourceId ?? (await this.#getPrimaryEntropySourceId());
     const metaMetricsId = await this.#metametrics.getMetaMetricsId();
     this.update((state) => {
-      if (entropySourceId) {
-        state.isSignedIn = true;
-        if (!state.srpSessionData) {
-          state.srpSessionData = {};
-        }
-        state.srpSessionData[entropySourceId] = {
-          ...loginResponse,
-          profile: {
-            ...loginResponse.profile,
-            metaMetricsId,
-          },
-        };
+      state.isSignedIn = true;
+      if (!state.srpSessionData) {
+        state.srpSessionData = {};
       }
+      state.srpSessionData[resolvedId] = {
+        ...loginResponse,
+        profile: {
+          ...loginResponse.profile,
+          metaMetricsId,
+        },
+      };
     });
   }
 
@@ -265,6 +257,29 @@ export class AuthenticationController extends BaseController<
     }
   }
 
+  async #getPrimaryEntropySourceId(): Promise<string> {
+    if (this.#cachedPrimaryEntropySourceId) {
+      return this.#cachedPrimaryEntropySourceId;
+    }
+    const allPublicKeys = await this.#snapGetAllPublicKeys();
+
+    if (allPublicKeys.length === 0) {
+      throw new Error(
+        '#getPrimaryEntropySourceId - No entropy sources found from snap',
+      );
+    }
+
+    const primaryId = allPublicKeys[0][0];
+    if (!primaryId) {
+      throw new Error(
+        '#getPrimaryEntropySourceId - Primary entropy source ID is undefined',
+      );
+    }
+
+    this.#cachedPrimaryEntropySourceId = primaryId;
+    return this.#cachedPrimaryEntropySourceId;
+  }
+
   public async performSignIn(): Promise<string[]> {
     this.#assertIsUnlocked('performSignIn');
 
@@ -282,6 +297,7 @@ export class AuthenticationController extends BaseController<
   }
 
   public performSignOut(): void {
+    this.#cachedPrimaryEntropySourceId = undefined;
     this.update((state) => {
       state.isSignedIn = false;
       state.srpSessionData = undefined;
@@ -297,7 +313,9 @@ export class AuthenticationController extends BaseController<
 
   public async getBearerToken(entropySourceId?: string): Promise<string> {
     this.#assertIsUnlocked('getBearerToken');
-    return await this.#auth.getAccessToken(entropySourceId);
+    const resolvedId =
+      entropySourceId ?? (await this.#getPrimaryEntropySourceId());
+    return await this.#auth.getAccessToken(resolvedId);
   }
 
   /**
@@ -312,12 +330,18 @@ export class AuthenticationController extends BaseController<
     entropySourceId?: string,
   ): Promise<UserProfile> {
     this.#assertIsUnlocked('getSessionProfile');
-    return await this.#auth.getUserProfile(entropySourceId);
+    const resolvedId =
+      entropySourceId ?? (await this.#getPrimaryEntropySourceId());
+    return await this.#auth.getUserProfile(resolvedId);
   }
 
-  public async getUserProfileLineage(): Promise<UserProfileLineage> {
+  public async getUserProfileLineage(
+    entropySourceId?: string,
+  ): Promise<UserProfileLineage> {
     this.#assertIsUnlocked('getUserProfileLineage');
-    return await this.#auth.getUserProfileLineage();
+    const resolvedId =
+      entropySourceId ?? (await this.#getPrimaryEntropySourceId());
+    return await this.#auth.getUserProfileLineage(resolvedId);
   }
 
   public isSignedIn(): boolean {