Skip to content

Commit 25dbd61

Browse files
cshfangcshfang
authored
chore(auth): Refactor to eliminate circular dependencies (#14173)
* chore(auth): Refactor to eliminate circular dependencies * Bump size limit
1 parent bbdd2dc commit 25dbd61

24 files changed

+550
-420
lines changed

‎packages/auth/__tests__/client/flows/shared/handlePasswordSRP.test.ts

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import { createCognitoUserPoolEndpointResolver } from '../../../../src/providers
66
import { getAuthenticationHelper } from '../../../../src/providers/cognito/utils/srp';
77
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
88
import { handlePasswordSRP } from '../../../../src/client/flows/shared/handlePasswordSRP';
9-
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
9+
import { handlePasswordVerifierChallenge } from '../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge';
10+
import { retryOnResourceNotFoundException } from '../../../../src/providers/cognito/utils/retryOnResourceNotFoundException';
11+
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';
1012

1113
// Mock dependencies
1214
jest.mock(
@@ -15,14 +17,13 @@ jest.mock(
1517
jest.mock('../../../../src/providers/cognito/factories');
1618
jest.mock('../../../../src/providers/cognito/utils/srp');
1719
jest.mock('../../../../src/providers/cognito/utils/userContextData');
18-
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
19-
...jest.requireActual(
20-
'../../../../src/providers/cognito/utils/signInHelpers',
21-
),
22-
setActiveSignInUsername: jest.fn(),
23-
handlePasswordVerifierChallenge: jest.fn(),
24-
retryOnResourceNotFoundException: jest.fn(),
25-
}));
20+
jest.mock(
21+
'../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge',
22+
);
23+
jest.mock(
24+
'../../../../src/providers/cognito/utils/retryOnResourceNotFoundException',
25+
);
26+
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');
2627

2728
describe('handlePasswordSRP', () => {
2829
const mockConfig = {
@@ -31,6 +32,13 @@ describe('handlePasswordSRP', () => {
3132
userPoolEndpoint: 'test-endpoint',
3233
};
3334

35+
const mockHandlePasswordVerifierChallenge = jest.mocked(
36+
handlePasswordVerifierChallenge,
37+
);
38+
const mockRetryOnResourceNotFoundException = jest.mocked(
39+
retryOnResourceNotFoundException,
40+
);
41+
const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
3442
const mockInitiateAuth = jest.fn();
3543
const mockCreateEndpointResolver = jest.fn();
3644
const mockAuthenticationHelper = {
@@ -53,9 +61,9 @@ describe('handlePasswordSRP', () => {
5361
(getUserContextData as jest.Mock).mockReturnValue({
5462
UserContextData: 'test',
5563
});
56-
(
57-
signInHelpers.retryOnResourceNotFoundException as jest.Mock
58-
).mockImplementation((fn, args) => fn(...args));
64+
mockRetryOnResourceNotFoundException.mockImplementation((fn, args) =>
65+
fn(...args),
66+
);
5967
mockInitiateAuth.mockResolvedValue({
6068
ChallengeParameters: { USERNAME: 'testuser' },
6169
Session: 'test-session',
@@ -173,8 +181,8 @@ describe('handlePasswordSRP', () => {
173181
authFlow: 'USER_AUTH',
174182
});
175183

176-
expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
177-
signInHelpers.handlePasswordVerifierChallenge,
184+
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
185+
mockHandlePasswordVerifierChallenge,
178186
[
179187
password,
180188
challengeParameters,
@@ -208,9 +216,7 @@ describe('handlePasswordSRP', () => {
208216
});
209217

210218
expect(result).toEqual(mockResponse);
211-
expect(
212-
signInHelpers.retryOnResourceNotFoundException,
213-
).not.toHaveBeenCalled();
219+
expect(mockRetryOnResourceNotFoundException).not.toHaveBeenCalled();
214220
});
215221

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

257-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
258-
challengeUsername,
259-
);
263+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
260264
});
261265

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

288-
expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
289-
signInHelpers.handlePasswordVerifierChallenge,
292+
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
293+
mockHandlePasswordVerifierChallenge,
290294
[
291295
password,
292296
challengeParameters,
@@ -341,9 +345,7 @@ describe('handlePasswordSRP', () => {
341345
authFlow: 'USER_AUTH',
342346
});
343347

344-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
345-
username,
346-
);
348+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
347349
});
348350

349351
test('should not add PREFERRED_CHALLENGE for USER_AUTH when preferredChallenge is undefined', async () => {

‎packages/auth/__tests__/client/flows/userAuth/handleSelectChallengeWithPassword.test.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,15 @@ import { createRespondToAuthChallengeClient } from '../../../../src/foundation/f
55
import { createCognitoUserPoolEndpointResolver } from '../../../../src/providers/cognito/factories';
66
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
77
import { handleSelectChallengeWithPassword } from '../../../../src/client/flows/userAuth/handleSelectChallengeWithPassword';
8-
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
8+
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';
99

1010
// Mock dependencies
1111
jest.mock(
1212
'../../../../src/foundation/factories/serviceClients/cognitoIdentityProvider',
1313
);
1414
jest.mock('../../../../src/providers/cognito/factories');
1515
jest.mock('../../../../src/providers/cognito/utils/userContextData');
16-
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
17-
...jest.requireActual(
18-
'../../../../src/providers/cognito/utils/signInHelpers',
19-
),
20-
setActiveSignInUsername: jest.fn(),
21-
}));
16+
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');
2217

2318
describe('handlePasswordChallenge', () => {
2419
const mockConfig = {
@@ -27,6 +22,7 @@ describe('handlePasswordChallenge', () => {
2722
userPoolEndpoint: 'test-endpoint',
2823
};
2924

25+
const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
3026
const mockRespondToAuthChallenge = jest.fn();
3127
const mockCreateEndpointResolver = jest.fn();
3228

@@ -124,9 +120,7 @@ describe('handlePasswordChallenge', () => {
124120
session,
125121
);
126122

127-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
128-
challengeUsername,
129-
);
123+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
130124
});
131125

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

151-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
152-
username,
153-
);
145+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
154146
});
155147

156148
test('should throw error when respondToAuthChallenge fails', async () => {

‎packages/auth/__tests__/client/flows/userAuth/handleSelectChallengeWithPasswordSRP.test.ts

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import { createRespondToAuthChallengeClient } from '../../../../src/foundation/f
55
import { getAuthenticationHelper } from '../../../../src/providers/cognito/utils/srp';
66
import { getUserContextData } from '../../../../src/providers/cognito/utils/userContextData';
77
import { handleSelectChallengeWithPasswordSRP } from '../../../../src/client/flows/userAuth/handleSelectChallengeWithPasswordSRP';
8-
import * as signInHelpers from '../../../../src/providers/cognito/utils/signInHelpers';
8+
import { handlePasswordVerifierChallenge } from '../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge';
9+
import { retryOnResourceNotFoundException } from '../../../../src/providers/cognito/utils/retryOnResourceNotFoundException';
10+
import { setActiveSignInUsername } from '../../../../src/providers/cognito/utils/setActiveSignInUsername';
911

1012
// Mock dependencies
1113
jest.mock(
@@ -14,14 +16,13 @@ jest.mock(
1416
jest.mock('../../../../src/providers/cognito/factories');
1517
jest.mock('../../../../src/providers/cognito/utils/srp');
1618
jest.mock('../../../../src/providers/cognito/utils/userContextData');
17-
jest.mock('../../../../src/providers/cognito/utils/signInHelpers', () => ({
18-
...jest.requireActual(
19-
'../../../../src/providers/cognito/utils/signInHelpers',
20-
),
21-
setActiveSignInUsername: jest.fn(),
22-
handlePasswordVerifierChallenge: jest.fn(),
23-
retryOnResourceNotFoundException: jest.fn(),
24-
}));
19+
jest.mock(
20+
'../../../../src/providers/cognito/utils/handlePasswordVerifierChallenge',
21+
);
22+
jest.mock(
23+
'../../../../src/providers/cognito/utils/retryOnResourceNotFoundException',
24+
);
25+
jest.mock('../../../../src/providers/cognito/utils/setActiveSignInUsername');
2526

2627
describe('handleSelectChallengeWithPasswordSRP', () => {
2728
const mockConfig = {
@@ -35,6 +36,13 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
3536
clearDeviceMetadata: jest.fn(),
3637
} as any;
3738

39+
const mockHandlePasswordVerifierChallenge = jest.mocked(
40+
handlePasswordVerifierChallenge,
41+
);
42+
const mockRetryOnResourceNotFoundException = jest.mocked(
43+
retryOnResourceNotFoundException,
44+
);
45+
const mockSetActiveSignInUsername = jest.mocked(setActiveSignInUsername);
3846
const mockRespondToAuthChallenge = jest.fn();
3947
const mockAuthenticationHelper = {
4048
A: { toString: () => '123456' },
@@ -108,13 +116,12 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
108116
};
109117

110118
mockRespondToAuthChallenge.mockResolvedValueOnce(verifierResponse);
111-
(
112-
signInHelpers.retryOnResourceNotFoundException as jest.Mock
113-
).mockImplementation((fn, args) => fn(...args));
114-
(
115-
signInHelpers.handlePasswordVerifierChallenge as jest.Mock
116-
).mockResolvedValue({
119+
mockRetryOnResourceNotFoundException.mockImplementation((fn, args) =>
120+
fn(...args),
121+
);
122+
mockHandlePasswordVerifierChallenge.mockResolvedValue({
117123
AuthenticationResult: { AccessToken: 'token' },
124+
$metadata: {},
118125
});
119126

120127
await handleSelectChallengeWithPasswordSRP(
@@ -126,8 +133,8 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
126133
mockTokenOrchestrator,
127134
);
128135

129-
expect(signInHelpers.retryOnResourceNotFoundException).toHaveBeenCalledWith(
130-
signInHelpers.handlePasswordVerifierChallenge,
136+
expect(mockRetryOnResourceNotFoundException).toHaveBeenCalledWith(
137+
mockHandlePasswordVerifierChallenge,
131138
[
132139
password,
133140
verifierResponse.ChallengeParameters,
@@ -188,9 +195,7 @@ describe('handleSelectChallengeWithPasswordSRP', () => {
188195
mockTokenOrchestrator,
189196
);
190197

191-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
192-
challengeUsername,
193-
);
198+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(challengeUsername);
194199
});
195200

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

217222
// Verify it falls back to the original username
218-
expect(signInHelpers.setActiveSignInUsername).toHaveBeenCalledWith(
219-
username,
220-
);
223+
expect(mockSetActiveSignInUsername).toHaveBeenCalledWith(username);
221224
});
222225

223226
test('should handle userPoolId without second part after underscore', async () => {

‎packages/auth/__tests__/providers/cognito/getNewDeviceMetadata.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Amplify } from '@aws-amplify/core';
55

66
import { AuthError } from '../../../src/errors/AuthError';
77
import { ConfirmDeviceException } from '../../../src/providers/cognito/types/errors';
8-
import { getNewDeviceMetadata } from '../../../src/providers/cognito/utils/signInHelpers';
8+
import { getNewDeviceMetadata } from '../../../src/providers/cognito/utils/getNewDeviceMetadata';
99
import { createCognitoUserPoolEndpointResolver } from '../../../src/providers/cognito/factories';
1010
import { createConfirmDeviceClient } from '../../../src/foundation/factories/serviceClients/cognitoIdentityProvider';
1111

‎packages/auth/__tests__/providers/cognito/utils/signInHelpers/handleWebAuthnSignInResult.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ import {
1919
assertCredentialIsPkcWithAuthenticatorAssertionResponse,
2020
assertCredentialIsPkcWithAuthenticatorAttestationResponse,
2121
} from '../../../../../src/client/utils/passkey/types';
22+
import { AuthSignInOutput } from '../../../../../src/types';
23+
import {
24+
ChallengeName,
25+
ChallengeParameters,
26+
} from '../../../../../src/foundation/factories/serviceClients/cognitoIdentityProvider/types';
2227

2328
jest.mock('@aws-amplify/core', () => ({
2429
...(jest.createMockFromModule('@aws-amplify/core') as object),
@@ -101,6 +106,7 @@ describe('handleWebAuthnSignInResult', () => {
101106
expect(error.name).toBe(AuthErrorCodes.SignInException);
102107
}
103108
});
109+
104110
it('should throw an error when CREDENTIAL_REQUEST_OPTIONS is empty', async () => {
105111
expect.assertions(2);
106112
try {
@@ -166,9 +172,51 @@ describe('handleWebAuthnSignInResult', () => {
166172
mockCacheCognitoTokens.mockResolvedValue(undefined);
167173
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);
168174

169-
const result = await handleWebAuthnSignInResult(challengeParameters);
175+
const result = (await handleWebAuthnSignInResult(
176+
challengeParameters,
177+
)) as AuthSignInOutput;
170178

171179
expect(result.isSignedIn).toBe(true);
172180
expect(result.nextStep.signInStep).toBe('DONE');
173181
});
182+
183+
it('should return the next challenge', async () => {
184+
mockStoreGetState.mockReturnValue({
185+
username,
186+
challengeName,
187+
signInSession,
188+
});
189+
mockRespondToAuthChallenge.mockResolvedValue(
190+
authAPITestParams.CustomChallengeResponse,
191+
);
192+
mockCacheCognitoTokens.mockResolvedValue(undefined);
193+
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);
194+
195+
const result = (await handleWebAuthnSignInResult(challengeParameters)) as {
196+
challengeName: ChallengeName;
197+
challengeParameters: ChallengeParameters;
198+
};
199+
200+
expect(result.challengeName).toBe(
201+
authAPITestParams.CustomChallengeResponse.ChallengeName,
202+
);
203+
});
204+
205+
it('should throw an error if next challenge is WEB_AUTHN', async () => {
206+
mockStoreGetState.mockReturnValue({
207+
username,
208+
challengeName,
209+
signInSession,
210+
});
211+
mockRespondToAuthChallenge.mockResolvedValue({
212+
ChallengeName: 'WEB_AUTHN',
213+
Session: 'Session',
214+
});
215+
mockCacheCognitoTokens.mockResolvedValue(undefined);
216+
mockDispatchSignedInHubEvent.mockResolvedValue(undefined);
217+
218+
await expect(
219+
handleWebAuthnSignInResult(challengeParameters),
220+
).rejects.toThrow('Sequential WEB_AUTHN challenges returned');
221+
});
174222
});

‎packages/auth/src/client/flows/shared/handlePasswordSRP.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,14 @@ import {
1313
RespondToAuthChallengeCommandOutput,
1414
} from '../../../foundation/factories/serviceClients/cognitoIdentityProvider/types';
1515
import { getAuthenticationHelper } from '../../../providers/cognito/utils/srp';
16-
import {
17-
handlePasswordVerifierChallenge,
18-
retryOnResourceNotFoundException,
19-
setActiveSignInUsername,
20-
} from '../../../providers/cognito/utils/signInHelpers';
2116
import { createInitiateAuthClient } from '../../../foundation/factories/serviceClients/cognitoIdentityProvider';
2217
import { createCognitoUserPoolEndpointResolver } from '../../../providers/cognito/factories';
2318
import { getRegionFromUserPoolId } from '../../../foundation/parsers';
2419
import { getAuthUserAgentValue } from '../../../utils';
2520
import { AuthFactorType } from '../../../providers/cognito/types/models';
21+
import { handlePasswordVerifierChallenge } from '../../../providers/cognito/utils/handlePasswordVerifierChallenge';
22+
import { retryOnResourceNotFoundException } from '../../../providers/cognito/utils/retryOnResourceNotFoundException';
23+
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';
2624

2725
interface HandlePasswordSRPInput {
2826
username: string;

‎packages/auth/src/client/flows/userAuth/handleSelectChallengeWithPassword.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { getRegionFromUserPoolId } from '../../../foundation/parsers';
1111
import { getAuthUserAgentValue } from '../../../utils';
1212
import { getUserContextData } from '../../../providers/cognito/utils/userContextData';
1313
import { RespondToAuthChallengeCommandOutput } from '../../../foundation/factories/serviceClients/cognitoIdentityProvider/types';
14-
import { setActiveSignInUsername } from '../../../providers/cognito/utils/signInHelpers';
14+
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';
1515

1616
/**
1717
* Handles the SELECT_CHALLENGE response specifically for Password authentication.

‎packages/auth/src/client/flows/userAuth/handleSelectChallengeWithPasswordSRP.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@ import { getRegionFromUserPoolId } from '../../../foundation/parsers';
1616
import { getAuthUserAgentValue } from '../../../utils';
1717
import { getAuthenticationHelper } from '../../../providers/cognito/utils/srp';
1818
import { getUserContextData } from '../../../providers/cognito/utils/userContextData';
19-
import {
20-
handlePasswordVerifierChallenge,
21-
retryOnResourceNotFoundException,
22-
setActiveSignInUsername,
23-
} from '../../../providers/cognito/utils/signInHelpers';
19+
import { setActiveSignInUsername } from '../../../providers/cognito/utils/setActiveSignInUsername';
20+
import { retryOnResourceNotFoundException } from '../../../providers/cognito/utils/retryOnResourceNotFoundException';
21+
import { handlePasswordVerifierChallenge } from '../../../providers/cognito/utils/handlePasswordVerifierChallenge';
2422

2523
/**
2624
* Handles the SELECT_CHALLENGE response specifically for Password SRP authentication.

0 commit comments

Comments
 (0)