Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(auth): Refactor to eliminate circular dependencies #14173

Merged
merged 4 commits into from
Jan 30, 2025
Merged

chore(auth): Refactor to eliminate circular dependencies #14173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb28c72
chore(auth): Refactor to eliminate circular dependencies
cshfang Jan 29, 2025
869245e
Merge branch 'main' into chore/eliminate-circular-dependencies
cshfang Jan 30, 2025
255949f
Bump size limit
cshfang Jan 30, 2025
0a9f66a
Merge branch 'main' into chore/eliminate-circular-dependencies
cshfang Jan 30, 2025
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
52 changes: 27 additions & 25 deletions packages/auth/__tests__/client/flows/shared/handlePasswordSRP.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { createCognitoUserPoolEndpointResolver } from '../../../../src/providers
import { getAuthenticationHelper } from '../../../../src/providers/cognito/utils/srp';
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
import { handlePasswordSRP } from '../../../../src/client/flows/shared/handlePasswordSRP';
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
import { handlePasswordVerifierChallenge } from '../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge';
import { retryOnResourceNotFoundException } from '../../../../src/providers/cognito/utils/retryOnResourceNotFoundException';
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';

// Mock dependencies
jest.mock(
Expand All @@ -15,14 +17,13 @@ jest.mock(
jest.mock('../../../../src/providers/cognito/factories');
jest.mock('../../../../src/providers/cognito/utils/srp');
jest.mock('../../../../src/providers/cognito/utils/userContextData');
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
...jest.requireActual(
'../../../../src/providers/cognito/utils/signInHelpers',
),
setActiveSignInUsername: jest.fn(),
handlePasswordVerifierChallenge: jest.fn(),
retryOnResourceNotFoundException: jest.fn(),
}));
jest.mock(
'../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge',
);
jest.mock(
'../../../../src/providers/cognito/utils/retryOnResourceNotFoundException',
);
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');

describe('handlePasswordSRP', () => {
const mockConfig = {
Expand All @@ -31,6 +32,13 @@ describe('handlePasswordSRP', () => {
userPoolEndpoint: 'test-endpoint',
};

const mockHandlePasswordVerifierChallenge = jest.mocked(
handlePasswordVerifierChallenge,
);
const mockRetryOnResourceNotFoundException = jest.mocked(
retryOnResourceNotFoundException,
);
const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
const mockInitiateAuth = jest.fn();
const mockCreateEndpointResolver = jest.fn();
const mockAuthenticationHelper = {
Expand All @@ -53,9 +61,9 @@ describe('handlePasswordSRP', () => {
(getUserContextData as jest.Mock).mockReturnValue({
UserContextData: 'test',
});
(
signInHelpers.retryOnResourceNotFoundException as jest.Mock
).mockImplementation((fn, args) => fn(...args));
mockRetryOnResourceNotFoundException.mockImplementation((fn, args) =>
fn(...args),
);
mockInitiateAuth.mockResolvedValue({
ChallengeParameters: { USERNAME: 'testuser' },
Session: 'test-session',
Expand Down Expand Up @@ -173,8 +181,8 @@ describe('handlePasswordSRP', () => {
authFlow: 'USER_AUTH',
});

expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
signInHelpers.handlePasswordVerifierChallenge,
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
mockHandlePasswordVerifierChallenge,
[
password,
challengeParameters,
Expand Down Expand Up @@ -208,9 +216,7 @@ describe('handlePasswordSRP', () => {
});

expect(result).toEqual(mockResponse);
expect(
signInHelpers.retryOnResourceNotFoundException,
).not.toHaveBeenCalled();
expect(mockRetryOnResourceNotFoundException).not.toHaveBeenCalled();
});

test('should handle client metadata when provided', async () => {
Expand Down Expand Up @@ -254,9 +260,7 @@ describe('handlePasswordSRP', () => {
authFlow: 'USER_SRP_AUTH',
});

expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
challengeUsername,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
});

test('should call handlePasswordVerifierChallenge with correct parameters', async () => {
Expand Down Expand Up @@ -285,8 +289,8 @@ describe('handlePasswordSRP', () => {
authFlow: 'USER_SRP_AUTH',
});

expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
signInHelpers.handlePasswordVerifierChallenge,
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
mockHandlePasswordVerifierChallenge,
[
password,
challengeParameters,
Expand Down Expand Up @@ -341,9 +345,7 @@ describe('handlePasswordSRP', () => {
authFlow: 'USER_AUTH',
});

expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
username,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
});

test('should not add PREFERRED_CHALLENGE for USER_AUTH when preferredChallenge is undefined', async () => {
Expand Down
18 changes: 5 additions & 13 deletions packages/auth/__tests__/client/flows/userAuth/handleSelectChallengeWithPassword.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,15 @@ import { createRespondToAuthChallengeClient } from '../../../../src/foundation/f
import { createCognitoUserPoolEndpointResolver } from '../../../../src/providers/cognito/factories';
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
import { handleSelectChallengeWithPassword } from '../../../../src/client/flows/userAuth/handleSelectChallengeWithPassword';
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';

// Mock dependencies
jest.mock(
'../../../../src/foundation/factories/serviceClients/cognitoIdentityProvider',
);
jest.mock('../../../../src/providers/cognito/factories');
jest.mock('../../../../src/providers/cognito/utils/userContextData');
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
...jest.requireActual(
'../../../../src/providers/cognito/utils/signInHelpers',
),
setActiveSignInUsername: jest.fn(),
}));
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');

describe('handlePasswordChallenge', () => {
const mockConfig = {
Expand All @@ -27,6 +22,7 @@ describe('handlePasswordChallenge', () => {
userPoolEndpoint: 'test-endpoint',
};

const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
const mockRespondToAuthChallenge = jest.fn();
const mockCreateEndpointResolver = jest.fn();

Expand Down Expand Up @@ -124,9 +120,7 @@ describe('handlePasswordChallenge', () => {
session,
);

expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
challengeUsername,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
});

test('should set active username as original username when challenge parameters are missing', async () => {
Expand All @@ -148,9 +142,7 @@ describe('handlePasswordChallenge', () => {
session,
);

expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
username,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
});

test('should throw error when respondToAuthChallenge fails', async () => {
Expand Down
49 changes: 26 additions & 23 deletions packages/auth/__tests__/client/flows/userAuth/handleSelectChallengeWithPasswordSRP.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { createRespondToAuthChallengeClient } from '../../../../src/foundation/f
import { getAuthenticationHelper } from '../../../../src/providers/cognito/utils/srp';
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
import { handleSelectChallengeWithPasswordSRP } from '../../../../src/client/flows/userAuth/handleSelectChallengeWithPasswordSRP';
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
import { handlePasswordVerifierChallenge } from '../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge';
import { retryOnResourceNotFoundException } from '../../../../src/providers/cognito/utils/retryOnResourceNotFoundException';
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';

// Mock dependencies
jest.mock(
Expand All @@ -14,14 +16,13 @@ jest.mock(
jest.mock('../../../../src/providers/cognito/factories');
jest.mock('../../../../src/providers/cognito/utils/srp');
jest.mock('../../../../src/providers/cognito/utils/userContextData');
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
...jest.requireActual(
'../../../../src/providers/cognito/utils/signInHelpers',
),
setActiveSignInUsername: jest.fn(),
handlePasswordVerifierChallenge: jest.fn(),
retryOnResourceNotFoundException: jest.fn(),
}));
jest.mock(
'../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge',
);
jest.mock(
'../../../../src/providers/cognito/utils/retryOnResourceNotFoundException',
);
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');

describe('handleSelectChallengeWithPasswordSRP', () => {
const mockConfig = {
Expand All @@ -35,6 +36,13 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
clearDeviceMetadata: jest.fn(),
} as any;

const mockHandlePasswordVerifierChallenge = jest.mocked(
handlePasswordVerifierChallenge,
);
const mockRetryOnResourceNotFoundException = jest.mocked(
retryOnResourceNotFoundException,
);
const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
const mockRespondToAuthChallenge = jest.fn();
const mockAuthenticationHelper = {
A: { toString: () => '123456' },
Expand Down Expand Up @@ -108,13 +116,12 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
};

mockRespondToAuthChallenge.mockResolvedValueOnce(verifierResponse);
(
signInHelpers.retryOnResourceNotFoundException as jest.Mock
).mockImplementation((fn, args) => fn(...args));
(
signInHelpers.handlePasswordVerifierChallenge as jest.Mock
).mockResolvedValue({
mockRetryOnResourceNotFoundException.mockImplementation((fn, args) =>
fn(...args),
);
mockHandlePasswordVerifierChallenge.mockResolvedValue({
AuthenticationResult: { AccessToken: 'token' },
$metadata: {},
});

await handleSelectChallengeWithPasswordSRP(
Expand All @@ -126,8 +133,8 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
mockTokenOrchestrator,
);

expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
signInHelpers.handlePasswordVerifierChallenge,
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
mockHandlePasswordVerifierChallenge,
[
password,
verifierResponse.ChallengeParameters,
Expand Down Expand Up @@ -188,9 +195,7 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
mockTokenOrchestrator,
);

expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
challengeUsername,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
});

test('should use original username when ChallengeParameters is undefined', async () => {
Expand All @@ -215,9 +220,7 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
);

// Verify it falls back to the original username
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
username,
);
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
});

test('should handle userPoolId without second part after underscore', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/__tests__/providers/cognito/getNewDeviceMetadata.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Amplify } from '@aws-amplify/core';

import { AuthError } from '../../../src/errors/AuthError';
import { ConfirmDeviceException } from '../../../src/providers/cognito/types/errors';
import { getNewDeviceMetadata } from '../../../src/providers/cognito/utils/signInHelpers';
import { getNewDeviceMetadata } from '../../../src/providers/cognito/utils/getNewDeviceMetadata';
import { createCognitoUserPoolEndpointResolver } from '../../../src/providers/cognito/factories';
import { createConfirmDeviceClient } from '../../../src/foundation/factories/serviceClients/cognitoIdentityProvider';

Expand Down
50 changes: 49 additions & 1 deletion ...s/auth/__tests__/providers/cognito/utils/signInHelpers/handleWebAuthnSignInResult.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ import {
assertCredentialIsPkcWithAuthenticatorAssertionResponse,
assertCredentialIsPkcWithAuthenticatorAttestationResponse,
} from '../../../../../src/client/utils/passkey/types';
import { AuthSignInOutput } from '../../../../../src/types';
import {
ChallengeName,
ChallengeParameters,
} from '../../../../../src/foundation/factories/serviceClients/cognitoIdentityProvider/types';

jest.mock('@aws-amplify/core', () => ({
...(jest.createMockFromModule('@aws-amplify/core') as object),
Expand Down Expand Up @@ -101,6 +106,7 @@ describe('handleWebAuthnSignInResult', () => {
expect(error.name).toBe(AuthErrorCodes.SignInException);
}
});

it('should throw an error when CREDENTIAL_REQUEST_OPTIONS is empty', async () => {
expect.assertions(2);
try {
Expand Down Expand Up @@ -166,9 +172,51 @@ describe('handleWebAuthnSignInResult', () => {
mockCacheCognitoTokens.mockResolvedValue(undefined);
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);

const result = await handleWebAuthnSignInResult(challengeParameters);
const result = (await handleWebAuthnSignInResult(
challengeParameters,
)) as AuthSignInOutput;

expect(result.isSignedIn).toBe(true);
expect(result.nextStep.signInStep).toBe('DONE');
});

it('should return the next challenge', async () => {
mockStoreGetState.mockReturnValue({
username,
challengeName,
signInSession,
});
mockRespondToAuthChallenge.mockResolvedValue(
authAPITestParams.CustomChallengeResponse,
);
mockCacheCognitoTokens.mockResolvedValue(undefined);
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);

const result = (await handleWebAuthnSignInResult(challengeParameters)) as {
challengeName: ChallengeName;
challengeParameters: ChallengeParameters;
};

expect(result.challengeName).toBe(
authAPITestParams.CustomChallengeResponse.ChallengeName,
);
});

it('should throw an error if next challenge is WEB_AUTHN', async () => {
mockStoreGetState.mockReturnValue({
username,
challengeName,
signInSession,
});
mockRespondToAuthChallenge.mockResolvedValue({
ChallengeName: 'WEB_AUTHN',
Session: 'Session',
});
mockCacheCognitoTokens.mockResolvedValue(undefined);
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);

await expect(
handleWebAuthnSignInResult(challengeParameters),
).rejects.toThrow('Sequential WEB_AUTHN challenges returned');
});
});
8 changes: 3 additions & 5 deletions packages/auth/src/client/flows/shared/handlePasswordSRP.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,14 @@ import {
RespondToAuthChallengeCommandOutput,
} from '../../../foundation/factories/serviceClients/cognitoIdentityProvider/types';
import { getAuthenticationHelper } from '../../../providers/cognito/utils/srp';
import {
handlePasswordVerifierChallenge,
retryOnResourceNotFoundException,
setActiveSignInUsername,
} from '../../../providers/cognito/utils/signInHelpers';
import { createInitiateAuthClient } from '../../../foundation/factories/serviceClients/cognitoIdentityProvider';
import { createCognitoUserPoolEndpointResolver } from '../../../providers/cognito/factories';
import { getRegionFromUserPoolId } from '../../../foundation/parsers';
import { getAuthUserAgentValue } from '../../../utils';
import { AuthFactorType } from '../../../providers/cognito/types/models';
import { handlePasswordVerifierChallenge } from '../../../providers/cognito/utils/handlePasswordVerifierChallenge';
import { retryOnResourceNotFoundException } from '../../../providers/cognito/utils/retryOnResourceNotFoundException';
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';

interface HandlePasswordSRPInput {
username: string;
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/src/client/flows/userAuth/handleSelectChallengeWithPassword.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { getRegionFromUserPoolId } from '../../../foundation/parsers';
import { getAuthUserAgentValue } from '../../../utils';
import { getUserContextData } from '../../../providers/cognito/utils/userContextData';
import { RespondToAuthChallengeCommandOutput } from '../../../foundation/factories/serviceClients/cognitoIdentityProvider/types';
import { setActiveSignInUsername } from '../../../providers/cognito/utils/signInHelpers';
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';

/**
* Handles the SELECT_CHALLENGE response specifically for Password authentication.
Expand Down
8 changes: 3 additions & 5 deletions packages/auth/src/client/flows/userAuth/handleSelectChallengeWithPasswordSRP.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ import { getRegionFromUserPoolId } from '../../../foundation/parsers';
import { getAuthUserAgentValue } from '../../../utils';
import { getAuthenticationHelper } from '../../../providers/cognito/utils/srp';
import { getUserContextData } from '../../../providers/cognito/utils/userContextData';
import {
handlePasswordVerifierChallenge,
retryOnResourceNotFoundException,
setActiveSignInUsername,
} from '../../../providers/cognito/utils/signInHelpers';
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';
import { retryOnResourceNotFoundException } from '../../../providers/cognito/utils/retryOnResourceNotFoundException';
import { handlePasswordVerifierChallenge } from '../../../providers/cognito/utils/handlePasswordVerifierChallenge';

/**
* Handles the SELECT_CHALLENGE response specifically for Password SRP authentication.
Expand Down
Loading
Loading