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

fix(auth): Added missing UserContextData to Cognito User Pool operation SignUp #13477

Merged
merged 13 commits into from
Jun 20, 2024
Merged

fix(auth): Added missing UserContextData to Cognito User Pool operation SignUp #13477

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a2dd9bc
Added missing UserContextData to Cognito User Pool operation SignUp
armandsyah134 Jun 6, 2024
df2daba
fix(auth): Added missing UserContextData to Cognito User Pool operati…
armandsyah134 Jun 6, 2024
612bc77
refactor(auth): changed some properties to use the destructured prope…
armandsyah134 Jun 7, 2024
fa85b43
chore: omit yarn.lock change
armandsyah134 Jun 7, 2024
f582f7d
Merge branch 'main' into main
jimblanc Jun 17, 2024
e643d3b
Ran linter to fix formatting issues + moved invocation of getUserCont…
armandsyah134 Jun 18, 2024
ff42865
Merge branch 'auth/fix-adding-usercontextdata-to-signup' of https://g…
armandsyah134 Jun 18, 2024
b890ced
omit yarn.lock file
armandsyah134 Jun 18, 2024
3839009
Merge pull request #1 from armandsyah134/auth/fix-adding-usercontextd…
armandsyah134 Jun 18, 2024
d03c395
Fix unit test for SignUp
armandsyah134 Jun 18, 2024
55f74eb
Merge pull request #2 from armandsyah134/auth/fix-adding-usercontextd…
armandsyah134 Jun 18, 2024
3f87adf
Merge branch 'main' into main
israx Jun 19, 2024
1918826
Merge branch 'main' into main
jimblanc Jun 20, 2024
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
33 changes: 33 additions & 0 deletions packages/auth/__tests__/providers/cognito/signUp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,39 @@
userId,
});
});

it('should send UserContextData', async () => {
window['AmazonCognitoAdvancedSecurityData'] = {

Check failure on line 177 in packages/auth/__tests__/providers/cognito/signUp.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests / Unit Test - @aws-amplify/auth 

["AmazonCognitoAdvancedSecurityData"] is better written in dot notation
getData() {
return 'abcd';
},
};
await signUp({
username: user1.username,
password: user1.password,
options: {
userAttributes: { email: user1.email },
},
});

Check failure on line 189 in packages/auth/__tests__/providers/cognito/signUp.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests / Unit Test - @aws-amplify/auth 

Trailing spaces not allowed

Check failure on line 189 in packages/auth/__tests__/providers/cognito/signUp.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests / Unit Test - @aws-amplify/auth 

Delete `↹`
expect(mockSignUp).toHaveBeenCalledWith(
{
region: 'us-west-2',
userAgentValue: expect.any(String),
},
{
ClientMetadata: undefined,
Password: user1.password,
UserAttributes: [{ Name: 'email', Value: user1.email }],
Username: user1.username,
ValidationData: undefined,
ClientId: '111111-aaaaa-42d8-891d-ee81a1549398',
UserContextData: { EncodedData: 'abcd' },
},
);
expect(mockSignUp).toHaveBeenCalledTimes(1);
window['AmazonCognitoAdvancedSecurityData'] = undefined;

Check failure on line 206 in packages/auth/__tests__/providers/cognito/signUp.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests / Unit Test - @aws-amplify/auth 

["AmazonCognitoAdvancedSecurityData"] is better written in dot notation
});
});

describe('Error Path Cases:', () => {
Expand Down
16 changes: 13 additions & 3 deletions packages/auth/src/providers/cognito/apis/signUp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
setAutoSignInStarted,
setUsernameUsedForAutoSignIn,
} from '../utils/signUpHelpers';
import { getUserContextData } from '../utils/userContextData';
import { getAuthUserAgentValue } from '../../../utils';

import { setAutoSignIn } from './autoSignIn';
Expand Down Expand Up @@ -62,7 +63,6 @@ export async function signUp(input: SignUpInput): Promise<SignUpOutput> {
username,
options: signInServiceOptions,
};

// if the authFlowType is 'CUSTOM_WITHOUT_SRP' then we don't include the password
if (signInServiceOptions?.authFlowType !== 'CUSTOM_WITHOUT_SRP') {
signInInput.password = password;
Expand All @@ -71,9 +71,18 @@ export async function signUp(input: SignUpInput): Promise<SignUpOutput> {
setUsernameUsedForAutoSignIn(username);
setAutoSignInStarted(true);
}

const { userPoolId, userPoolClientId } = authConfig;

const UserContextData = getUserContextData({
username,
userPoolId,
userPoolClientId,
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we move this directly into line 96? I think getUserContextData is expressive enough to convey the context there when used directly. Also prefer not to have the PascalCased variable as it easily gets confused with types/interfaces/classes


const clientOutput = await signUpClient(
{
region: getRegion(authConfig.userPoolId),
region: getRegion(userPoolId),
userAgentValue: getAuthUserAgentValue(AuthAction.SignUp),
},
{
Expand All @@ -83,7 +92,8 @@ export async function signUp(input: SignUpInput): Promise<SignUpOutput> {
options?.userAttributes && toAttributeType(options?.userAttributes),
ClientMetadata: clientMetadata,
ValidationData: validationData && toAttributeType(validationData),
ClientId: authConfig.userPoolClientId,
ClientId: userPoolClientId,
UserContextData,
},
);
const { UserSub, CodeDeliveryDetails } = clientOutput;
Expand Down
Loading