Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@
"count": 2
},
"@typescript-eslint/prefer-nullish-coalescing": {
"count": 2
"count": 1
}
},
"packages/profile-sync-controller/src/controllers/authentication/__fixtures__/mockServices.ts": {
Expand Down Expand Up @@ -1624,11 +1624,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
Expand Down
4 changes: 4 additions & 0 deletions packages/profile-metrics-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Bump `@metamask/transaction-controller` from `^62.17.0` to `^62.20.0` ([#7996](https://github.com/MetaMask/core/pull/7996), [#8005](https://github.com/MetaMask/core/pull/8005), [#8031](https://github.com/MetaMask/core/pull/8031) [#8104](https://github.com/MetaMask/core/pull/8104))
- Bump `@metamask/controller-utils` from `^11.18.0` to `^11.19.0` ([#7995](https://github.com/MetaMask/core/pull/7995))

### 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.1]

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
9 changes: 9 additions & 0 deletions packages/profile-sync-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- 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
- Add client-side JWT `exp` claim validation to prevent stale cached tokens from being returned ([#8144](https://github.com/MetaMask/core/pull/8144))
- `#getAuthSession` in both `SRPJwtBearerAuth` and `SIWEJwtBearerAuth` now decodes the JWT `exp` claim and rejects tokens that have actually expired, regardless of client-side TTL tracking (`obtainedAt`/`expiresIn`)
- Update `getUserProfileLineage` to accept an optional `entropySourceId` parameter ([#8144](https://github.com/MetaMask/core/pull/8144))

## [27.1.0]

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,63 @@ 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);
});
});

describe('getSessionProfile', () => {
Expand Down Expand Up @@ -657,7 +714,7 @@ describe('metadata', () => {
"profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
},
"token": {
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"expiresIn": 1000,
"obtainedAt": 0,
},
Expand All @@ -669,7 +726,7 @@ describe('metadata', () => {
"profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
},
"token": {
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"expiresIn": 1000,
"obtainedAt": 0,
},
Expand Down Expand Up @@ -704,7 +761,7 @@ describe('metadata', () => {
"profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
},
"token": {
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"expiresIn": 1000,
"obtainedAt": 0,
},
Expand All @@ -716,7 +773,7 @@ describe('metadata', () => {
"profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad",
},
"token": {
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
"expiresIn": 1000,
"obtainedAt": 0,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ export class AuthenticationController extends BaseController<

#isUnlocked = false;

#cachedPrimaryEntropySourceId?: string;

readonly #keyringController = {
setupLockedStateSubscriptions: () => {
const { isUnlocked } = this.messenger.call('KeyringController:getState');
Expand Down Expand Up @@ -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,
},
};
});
}

Expand All @@ -265,6 +257,15 @@ export class AuthenticationController extends BaseController<
}
}

async #getPrimaryEntropySourceId(): Promise<string> {
if (this.#cachedPrimaryEntropySourceId) {
return this.#cachedPrimaryEntropySourceId;
}
const allPublicKeys = await this.#snapGetAllPublicKeys();
this.#cachedPrimaryEntropySourceId = allPublicKeys[0][0];
return this.#cachedPrimaryEntropySourceId;
}

public async performSignIn(): Promise<string[]> {
this.#assertIsUnlocked('performSignIn');

Expand All @@ -282,6 +283,7 @@ export class AuthenticationController extends BaseController<
}

public performSignOut(): void {
this.#cachedPrimaryEntropySourceId = undefined;
this.update((state) => {
state.isSignedIn = false;
state.srpSessionData = undefined;
Expand All @@ -297,7 +299,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);
}

/**
Expand All @@ -312,12 +316,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export class SIWEJwtBearerAuth implements IBaseAuth {
return this.#signer.address;
}

async getUserProfileLineage(): Promise<UserProfileLineage> {
async getUserProfileLineage(
_entropySourceId?: string,
): Promise<UserProfileLineage> {
const accessToken = await this.getAccessToken();
return await getUserProfileLineage(this.#config.env, accessToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,46 @@ describe('SRPJwtBearerAuth rate limit handling', () => {
// Should NOT apply any delay
expect(mockDelay).not.toHaveBeenCalled();
});

it('triggers a fresh login when the cached JWT exp claim is in the past', async () => {
const expiredExp = Math.floor(Date.now() / 1000) - 3600;
const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' }));
const payload = btoa(JSON.stringify({ exp: expiredExp }));
const expiredJwt = `${header}.${payload}.fake-sig`;

const { auth, store } = createAuth();
store.value = {
profile: MOCK_PROFILE,
token: {
accessToken: expiredJwt,
expiresIn: 86400,
obtainedAt: Date.now(),
},
};

const token = await auth.getAccessToken();
expect(token).toBe('access');
expect(mockGetNonce).toHaveBeenCalledTimes(1);
});

it('returns the cached token when JWT exp claim is still in the future', async () => {
const futureExp = Math.floor(Date.now() / 1000) + 3600;
const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' }));
const payload = btoa(JSON.stringify({ exp: futureExp }));
const validJwt = `${header}.${payload}.fake-sig`;

const { auth, store } = createAuth();
store.value = {
profile: MOCK_PROFILE,
token: {
accessToken: validJwt,
expiresIn: 86400,
obtainedAt: Date.now(),
},
};

const token = await auth.getAccessToken();
expect(token).toBe(validJwt);
expect(mockGetNonce).not.toHaveBeenCalled();
});
});
Loading
Loading