From c07973e5a3c2e3711b416cbab0ede8482e208e31 Mon Sep 17 00:00:00 2001 From: ManojNB Date: Tue, 11 Feb 2025 17:17:26 -0800 Subject: [PATCH 1/5] fix: associate unauth identityId to auth identityId --- .../credentialsProvider.test.ts | 7 ++- .../identityIdProvider.test.ts | 7 ++- .../credentialsProvider/IdentityIdProvider.ts | 50 ++++++++----------- .../credentialsProvider/IdentityIdStore.ts | 13 ++++- .../credentialsProvider.ts | 16 +++--- 5 files changed, 45 insertions(+), 48 deletions(-) rename packages/auth/__tests__/providers/cognito/{ => credentialsProvider}/credentialsProvider.test.ts (98%) rename packages/auth/__tests__/providers/cognito/{ => credentialsProvider}/identityIdProvider.test.ts (93%) diff --git a/packages/auth/__tests__/providers/cognito/credentialsProvider.test.ts b/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts similarity index 98% rename from packages/auth/__tests__/providers/cognito/credentialsProvider.test.ts rename to packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts index 3390e14a052..c76048881c0 100644 --- a/packages/auth/__tests__/providers/cognito/credentialsProvider.test.ts +++ b/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts @@ -11,10 +11,9 @@ import { import { CognitoAWSCredentialsAndIdentityIdProvider, DefaultIdentityIdStore, -} from '../../../src/providers/cognito'; -import { AuthError } from '../../../src/errors/AuthError'; - -import { authAPITestParams } from './testUtils/authApiTestParams'; +} from '../../../../src/providers/cognito'; +import { AuthError } from '../../../../src/errors/AuthError'; +import { authAPITestParams } from '../testUtils/authApiTestParams'; jest.mock('@aws-amplify/core', () => ({ ...jest.requireActual('@aws-amplify/core'), diff --git a/packages/auth/__tests__/providers/cognito/identityIdProvider.test.ts b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts similarity index 93% rename from packages/auth/__tests__/providers/cognito/identityIdProvider.test.ts rename to packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts index 2b8620c680d..a7b09e772b4 100644 --- a/packages/auth/__tests__/providers/cognito/identityIdProvider.test.ts +++ b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts @@ -8,10 +8,9 @@ import { } from '@aws-amplify/core/internals/aws-clients/cognitoIdentity'; import { CognitoIdentityPoolConfig } from '@aws-amplify/core/internals/utils'; -import { DefaultIdentityIdStore } from '../../../src/providers/cognito/credentialsProvider/IdentityIdStore'; -import { cognitoIdentityIdProvider } from '../../../src/providers/cognito/credentialsProvider/IdentityIdProvider'; - -import { authAPITestParams } from './testUtils/authApiTestParams'; +import { DefaultIdentityIdStore } from '../../../../src/providers/cognito/credentialsProvider/IdentityIdStore'; +import { cognitoIdentityIdProvider } from '../../../../src/providers/cognito/credentialsProvider/IdentityIdProvider'; +import { authAPITestParams } from '../testUtils/authApiTestParams'; jest.mock('@aws-amplify/core', () => ({ ...jest.requireActual('@aws-amplify/core'), diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts index b96adf08fbc..dbebeb5264b 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts @@ -33,46 +33,38 @@ export async function cognitoIdentityIdProvider({ identityIdStore.setAuthConfig({ Cognito: authConfig }); // will return null only if there is no identityId cached or if there is an error retrieving it - let identityId: Identity | null = await identityIdStore.loadIdentityId(); + const identityId: Identity | null = await identityIdStore.loadIdentityId(); - // Tokens are available so return primary identityId - if (tokens) { - // If there is existing primary identityId in-memory return that - if (identityId && identityId.type === 'primary') { - return identityId.id; - } else { + if (identityId) { + logger.debug('Cached identityId found.'); + + return identityId.id; + } else { + logger.debug('Generating a new identityId as it was not found in cache.'); + + let generatedIdentityId; + if (tokens) { const logins = tokens.idToken ? formLoginsMap(tokens.idToken.toString()) : {}; - const generatedIdentityId = await generateIdentityId(logins, authConfig); - - if (identityId && identityId.id === generatedIdentityId) { - logger.debug( - `The guest identity ${identityId.id} has become the primary identity.`, - ); - } - identityId = { + generatedIdentityId = await generateIdentityId(logins, authConfig); + // Store generated identityId + identityIdStore.storeIdentityId({ id: generatedIdentityId, type: 'primary', - }; - } - } else { - // If there is existing guest identityId cached return that - if (identityId && identityId.type === 'guest') { - return identityId.id; + }); } else { - identityId = { - id: await generateIdentityId({}, authConfig), + generatedIdentityId = await generateIdentityId({}, authConfig); + // Store generated identityId + identityIdStore.storeIdentityId({ + id: generatedIdentityId, type: 'guest', - }; + }); } - } - // Store in-memory or local storage depending on guest or primary identityId - identityIdStore.storeIdentityId(identityId); - - return identityId.id; + return generatedIdentityId; + } } async function generateIdentityId( diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts index 283b0e9cd3f..f1e113553e9 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { + Amplify, AuthConfig, ConsoleLogger, Identity, @@ -89,7 +90,17 @@ export class DefaultIdentityIdStore implements IdentityIdStore { async clearIdentityId(): Promise { this._primaryIdentityId = undefined; - await this.keyValueStorage.removeItem(this._authKeys.identityId); + // Re-generate the authKeys in case of tab/app is refreshed. + const authConfig = + this.authConfig?.Cognito ?? Amplify.getConfig().Auth?.Cognito; + if (authConfig?.identityPoolId) { + logger.debug('No auth keys present, so generating it.'); + this._authKeys = createKeysForAuthStorage( + 'Cognito', + authConfig.identityPoolId, + ); + await this.keyValueStorage.removeItem(this._authKeys.identityId); + } } } diff --git a/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts index 6356ff0fd09..883b13dbc8c 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts @@ -136,13 +136,11 @@ export class CognitoAWSCredentialsAndIdentityIdProvider sessionToken: clientResult.Credentials.SessionToken, expiration: clientResult.Credentials.Expiration, }, - identityId, + identityId: clientResult.IdentityId, }; - const identityIdRes = clientResult.IdentityId; - if (identityIdRes) { - res.identityId = identityIdRes; + if (res.identityId) { this._identityIdStore.storeIdentityId({ - id: identityIdRes, + id: res.identityId, type: 'guest', }); } @@ -206,7 +204,7 @@ export class CognitoAWSCredentialsAndIdentityIdProvider sessionToken: clientResult.Credentials.SessionToken, expiration: clientResult.Credentials.Expiration, }, - identityId, + identityId: clientResult.IdentityId, }; // Store the credentials in-memory along with the expiration this._credentialsAndIdentityId = { @@ -216,11 +214,9 @@ export class CognitoAWSCredentialsAndIdentityIdProvider }; this._nextCredentialsRefresh = new Date().getTime() + CREDENTIALS_TTL; - const identityIdRes = clientResult.IdentityId; - if (identityIdRes) { - res.identityId = identityIdRes; + if (res.identityId) { this._identityIdStore.storeIdentityId({ - id: identityIdRes, + id: res.identityId, type: 'primary', }); } From 5341c3cd4fb1eab67fc0569a3d921113a04116a0 Mon Sep 17 00:00:00 2001 From: ManojNB Date: Wed, 12 Feb 2025 12:41:44 -0800 Subject: [PATCH 2/5] chore:remove signout fix changes --- .../cognito/credentialsProvider/IdentityIdStore.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts index f1e113553e9..283b0e9cd3f 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdStore.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import { - Amplify, AuthConfig, ConsoleLogger, Identity, @@ -90,17 +89,7 @@ export class DefaultIdentityIdStore implements IdentityIdStore { async clearIdentityId(): Promise { this._primaryIdentityId = undefined; - // Re-generate the authKeys in case of tab/app is refreshed. - const authConfig = - this.authConfig?.Cognito ?? Amplify.getConfig().Auth?.Cognito; - if (authConfig?.identityPoolId) { - logger.debug('No auth keys present, so generating it.'); - this._authKeys = createKeysForAuthStorage( - 'Cognito', - authConfig.identityPoolId, - ); - await this.keyValueStorage.removeItem(this._authKeys.identityId); - } + await this.keyValueStorage.removeItem(this._authKeys.identityId); } } From b06548fcbbc0498a27c3fffa8dfe30ec1d62a322 Mon Sep 17 00:00:00 2001 From: ManojNB Date: Wed, 12 Feb 2025 16:33:19 -0800 Subject: [PATCH 3/5] chore: address comments and update tests path --- .../credentialsProvider.test.ts | 2 +- .../identityIdProvider.test.ts | 4 ++- .../credentialsProvider/IdentityIdProvider.ts | 30 ++++++------------- .../credentialsProvider.ts | 14 +++++---- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts b/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts index c76048881c0..c36894725d3 100644 --- a/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts +++ b/packages/auth/__tests__/providers/cognito/credentialsProvider/credentialsProvider.test.ts @@ -21,7 +21,7 @@ jest.mock('@aws-amplify/core', () => ({ })); jest.mock( - './../../../src/providers/cognito/credentialsProvider/IdentityIdProvider', + './../../../../src/providers/cognito/credentialsProvider/IdentityIdProvider', () => ({ cognitoIdentityIdProvider: jest .fn() diff --git a/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts index a7b09e772b4..d20e7a81550 100644 --- a/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts +++ b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts @@ -17,7 +17,9 @@ jest.mock('@aws-amplify/core', () => ({ getId: jest.fn(), })); jest.mock('@aws-amplify/core/internals/aws-clients/cognitoIdentity'); -jest.mock('../../../src/providers/cognito/credentialsProvider/IdentityIdStore'); +jest.mock( + '../../../../src/providers/cognito/credentialsProvider/IdentityIdStore', +); const ampConfig: ResourcesConfig = { Auth: { diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts index dbebeb5264b..ebb01c38c20 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts @@ -41,27 +41,15 @@ export async function cognitoIdentityIdProvider({ return identityId.id; } else { logger.debug('Generating a new identityId as it was not found in cache.'); - - let generatedIdentityId; - if (tokens) { - const logins = tokens.idToken - ? formLoginsMap(tokens.idToken.toString()) - : {}; - - generatedIdentityId = await generateIdentityId(logins, authConfig); - // Store generated identityId - identityIdStore.storeIdentityId({ - id: generatedIdentityId, - type: 'primary', - }); - } else { - generatedIdentityId = await generateIdentityId({}, authConfig); - // Store generated identityId - identityIdStore.storeIdentityId({ - id: generatedIdentityId, - type: 'guest', - }); - } + const logins = tokens?.idToken + ? formLoginsMap(tokens.idToken.toString()) + : {}; + const generatedIdentityId = await generateIdentityId(logins, authConfig); + // Store generated identityId + identityIdStore.storeIdentityId({ + id: generatedIdentityId, + type: tokens ? 'primary' : 'guest', + }); return generatedIdentityId; } diff --git a/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts index 883b13dbc8c..e6e0c91079e 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/credentialsProvider.ts @@ -136,11 +136,12 @@ export class CognitoAWSCredentialsAndIdentityIdProvider sessionToken: clientResult.Credentials.SessionToken, expiration: clientResult.Credentials.Expiration, }, - identityId: clientResult.IdentityId, + identityId, }; - if (res.identityId) { + if (clientResult.IdentityId) { + res.identityId = clientResult.IdentityId; this._identityIdStore.storeIdentityId({ - id: res.identityId, + id: clientResult.IdentityId, type: 'guest', }); } @@ -204,7 +205,7 @@ export class CognitoAWSCredentialsAndIdentityIdProvider sessionToken: clientResult.Credentials.SessionToken, expiration: clientResult.Credentials.Expiration, }, - identityId: clientResult.IdentityId, + identityId, }; // Store the credentials in-memory along with the expiration this._credentialsAndIdentityId = { @@ -214,9 +215,10 @@ export class CognitoAWSCredentialsAndIdentityIdProvider }; this._nextCredentialsRefresh = new Date().getTime() + CREDENTIALS_TTL; - if (res.identityId) { + if (clientResult.IdentityId) { + res.identityId = clientResult.IdentityId; this._identityIdStore.storeIdentityId({ - id: res.identityId, + id: clientResult.IdentityId, type: 'primary', }); } From 33c19294c188c2c6015e7b1af9cfd3cbb7a220fa Mon Sep 17 00:00:00 2001 From: ManojNB Date: Wed, 19 Feb 2025 17:06:19 -0800 Subject: [PATCH 4/5] chore: update debug log and add some unit tests --- .../identityIdProvider.test.ts | 55 +++++++++++++++++++ .../credentialsProvider/IdentityIdProvider.ts | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts index d20e7a81550..5c254bc6cf0 100644 --- a/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts +++ b/packages/auth/__tests__/providers/cognito/credentialsProvider/identityIdProvider.test.ts @@ -141,4 +141,59 @@ describe('Cognito IdentityId Provider Happy Path Cases:', () => { ).toBe(authAPITestParams.PrimaryIdentityId.id); expect(mockGetId).toHaveBeenCalledTimes(1); }); + test('Should return the identityId irresspective of the type if present', async () => { + mockDefaultIdentityIdStoreInstance.loadIdentityId.mockImplementationOnce( + async () => { + return authAPITestParams.PrimaryIdentityId as Identity; + }, + ); + expect( + await cognitoIdentityIdProvider({ + tokens: authAPITestParams.ValidAuthTokens, + authConfig: { + identityPoolId: 'XXXXXXXXXXXXXXXXX', + }, + identityIdStore: mockDefaultIdentityIdStoreInstance, + }), + ).toBe(authAPITestParams.PrimaryIdentityId.id); + + mockDefaultIdentityIdStoreInstance.loadIdentityId.mockImplementationOnce( + async () => { + return authAPITestParams.GuestIdentityId as Identity; + }, + ); + expect( + await cognitoIdentityIdProvider({ + tokens: authAPITestParams.ValidAuthTokens, + authConfig: { + identityPoolId: 'XXXXXXXXXXXXXXXXX', + }, + identityIdStore: mockDefaultIdentityIdStoreInstance, + }), + ).toBe(authAPITestParams.GuestIdentityId.id); + expect(mockGetId).toHaveBeenCalledTimes(0); + }); + test('Should fetch from Cognito when there is no identityId cached', async () => { + mockDefaultIdentityIdStoreInstance.loadIdentityId.mockImplementationOnce( + async () => { + return undefined; + }, + ); + mockDefaultIdentityIdStoreInstance.storeIdentityId.mockImplementationOnce( + async (identity: Identity) => { + expect(identity.id).toBe(authAPITestParams.PrimaryIdentityId.id); + expect(identity.type).toBe(authAPITestParams.PrimaryIdentityId.type); + }, + ); + expect( + await cognitoIdentityIdProvider({ + tokens: authAPITestParams.ValidAuthTokens, + authConfig: { + identityPoolId: 'us-east-1:test-id', + }, + identityIdStore: mockDefaultIdentityIdStoreInstance, + }), + ).toBe(authAPITestParams.PrimaryIdentityId.id); + expect(mockGetId).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts index ebb01c38c20..8e0b4171e17 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts @@ -40,7 +40,7 @@ export async function cognitoIdentityIdProvider({ return identityId.id; } else { - logger.debug('Generating a new identityId as it was not found in cache.'); + logger.debug('IdentityId not found in cache, fetching it from Cognito.'); const logins = tokens?.idToken ? formLoginsMap(tokens.idToken.toString()) : {}; From 50e562ecbeb741d3f4fea2b69f4847ebeecff0d3 Mon Sep 17 00:00:00 2001 From: ManojNB Date: Mon, 24 Feb 2025 10:52:39 -0800 Subject: [PATCH 5/5] chore: address nits --- .../credentialsProvider/IdentityIdProvider.ts | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts index 8e0b4171e17..5949e785a73 100644 --- a/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts +++ b/packages/auth/src/providers/cognito/credentialsProvider/IdentityIdProvider.ts @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { AuthTokens, ConsoleLogger, Identity, getId } from '@aws-amplify/core'; +import { AuthTokens, Identity, getId } from '@aws-amplify/core'; import { CognitoIdentityPoolConfig } from '@aws-amplify/core/internals/utils'; import { AuthError } from '../../../errors/AuthError'; @@ -11,7 +11,6 @@ import { GetIdException } from '../types/errors'; import { IdentityIdStore } from './types'; import { formLoginsMap } from './utils'; -const logger = new ConsoleLogger('CognitoIdentityIdProvider'); /** * Provides a Cognito identityId * @@ -36,23 +35,19 @@ export async function cognitoIdentityIdProvider({ const identityId: Identity | null = await identityIdStore.loadIdentityId(); if (identityId) { - logger.debug('Cached identityId found.'); - return identityId.id; - } else { - logger.debug('IdentityId not found in cache, fetching it from Cognito.'); - const logins = tokens?.idToken - ? formLoginsMap(tokens.idToken.toString()) - : {}; - const generatedIdentityId = await generateIdentityId(logins, authConfig); - // Store generated identityId - identityIdStore.storeIdentityId({ - id: generatedIdentityId, - type: tokens ? 'primary' : 'guest', - }); - - return generatedIdentityId; } + const logins = tokens?.idToken + ? formLoginsMap(tokens.idToken.toString()) + : {}; + const generatedIdentityId = await generateIdentityId(logins, authConfig); + // Store generated identityId + identityIdStore.storeIdentityId({ + id: generatedIdentityId, + type: tokens ? 'primary' : 'guest', + }); + + return generatedIdentityId; } async function generateIdentityId(