feat(cognito): added verified attribute changes #21180

eriktisme · 2022-07-16T17:10:33Z

Added the configuration to keep the original attributes until they are verified to improve the security of the users.

closes #21179

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-07-16T17:10:40Z

eriktisme · 2022-07-16T19:20:23Z

I was unable to find out how to run the tests locally, can someone support me with it?

mrgrain · 2022-07-17T17:39:32Z

I was unable to find out how to run the tests locally, can someone support me with it?

Sure! It's a bit tricky to find, but this section describes it https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#build

mrgrain

Thanks for your contribution! 😃

There are a few issues to address. But feel free to ignore my style suggestion.

packages/@aws-cdk/aws-cognito/lib/user-pool.ts

packages/@aws-cdk/aws-cognito/test/user-pool.test.ts

eriktisme · 2022-07-18T09:32:43Z

@mrgrain thank you for your review, I have addressed your suggestions by using constants!

Pull request has been modified.

mrgrain

👍🏻 👍🏻

mrgrain · 2022-07-18T10:13:37Z

@eriktisme The test failure is unrelated to your change, I'm looking into this right now.

In the meantime, I think it would be good to add an explanation of this feature to the Readme - I guess it's either under Attributes or Sign In.

Related to that, I'm not super familiar with Cognito. Could you explain your naming choice for keepOriginal to me? On the surface this doesn't match up the what the Cloudformation docs describe.

eriktisme · 2022-07-18T11:39:38Z

@eriktisme The test failure is unrelated to your change, I'm looking into this right now.

In the meantime, I think it would be good to add an explanation of this feature to the Readme - I guess it's either under Attributes or Sign In.

Related to that, I'm not super familiar with Cognito. Could you explain your naming choice for keepOriginal to me? On the surface this doesn't match up the what the Cloudformation docs describe.

I opted for "keepOriginal" because it is to maintain the original value of the attribute until the new value is verified. This can be found in step 4 in the Cognito documentation "Choose Keep original attribute value active when an update is pending" at page https://docs.aws.amazon.com/cognito/latest/developerguide/user-pool-settings-email-phone-verification.html?icmpid=docs_cognito_console_help_panel

Pull request has been modified.

mrgrain · 2022-07-18T13:53:02Z

Thanks for the documentation, it reads great!

I have resolved the issue causing the build to fail. You will have to rebase/merge this branch with latest changes from main.

eriktisme · 2022-07-18T15:42:21Z

@mrgrain do you know how to get the PR Linter to succeed?

mrgrain · 2022-07-18T16:05:38Z

@eriktisme Ah yes, it also needs integration tests to be updated to make sure the resource still deploys.
I reckon@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts is the appropriate one.

You can find details on how to run integration tests here: https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md

Pull request has been modified.

TheRealAmazonKendra · 2022-07-18T18:46:01Z

packages/@aws-cdk/aws-cognito/test/integ.user-pool-explicit-props.ts

@@ -27,6 +27,10 @@ const userpool = new UserPool(stack, 'myuserpool', {
    email: true,
    phone: true,
  },
+  keepOriginal: {


Updating this will require an update to the expected output file as well.

Thanks! Updated the snapshot

TheRealAmazonKendra

In addition to my last comment, please review the PR section of our contributing guide and edit the PR title and body accordingly.

Pull request has been modified.

eriktisme · 2022-07-19T07:23:05Z

In addition to my last comment, please review the PR section of our contributing guide and edit the PR title and body accordingly.

Thank you for your feedback! I have updated the title and the body, I hope it is more clear now and follows the guideline

…s values

aws-cdk-automation · 2022-07-19T08:45:51Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: ad7af53
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-07-19T09:31:34Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Added the configuration to keep the original attributes until they are verified to improve the security of the users. closes aws#21179 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions bot added the p2 label Jul 16, 2022

aws-cdk-automation requested a review from a team July 16, 2022 17:10

github-actions bot added the feature-request A feature should be added or improved. label Jul 16, 2022

eriktisme force-pushed the feat/configure-verified-changes branch from 3dd28ed to b9e260a Compare July 16, 2022 18:07

eriktisme force-pushed the feat/configure-verified-changes branch from ac540d4 to 0a12110 Compare July 18, 2022 08:05

mrgrain previously requested changes Jul 18, 2022

View reviewed changes

mrgrain previously approved these changes Jul 18, 2022

View reviewed changes

eriktisme force-pushed the feat/configure-verified-changes branch from 9d4a81f to 2b787bb Compare July 18, 2022 14:04

mrgrain previously approved these changes Jul 18, 2022

View reviewed changes

TheRealAmazonKendra reviewed Jul 18, 2022

View reviewed changes

TheRealAmazonKendra previously requested changes Jul 19, 2022

View reviewed changes

eriktisme changed the title ~~feat(aws-cognito): added verified changes~~ feat(cognito): added verified attribute changes Jul 19, 2022

eriktisme force-pushed the feat/configure-verified-changes branch from c92de19 to 9d4a81f Compare July 19, 2022 07:17

eriktisme added 3 commits July 19, 2022 10:07

feat(aws-cognito): added verified changes

7c8f7b5

perf: improve readability by early retuning

535e9b0

docs: added new section for configuring keeping of original attribute…

fd9ac5b

…s values

eriktisme added 2 commits July 19, 2022 10:07

test: added integration tests

8682ac0

test: updated snapshot

ad7af53

eriktisme force-pushed the feat/configure-verified-changes branch from e21755d to ad7af53 Compare July 19, 2022 08:07

mrgrain approved these changes Jul 19, 2022

View reviewed changes

mergify bot merged commit ad67594 into aws:main Jul 19, 2022

eriktisme deleted the feat/configure-verified-changes branch July 19, 2022 13:14

mrgrain added the effort/medium Medium work item – several days of effort label Jul 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cognito): added verified attribute changes #21180

feat(cognito): added verified attribute changes #21180

eriktisme commented Jul 16, 2022 •

edited

Loading

gitpod-io bot commented Jul 16, 2022

eriktisme commented Jul 16, 2022

mrgrain commented Jul 17, 2022

mrgrain left a comment

eriktisme commented Jul 18, 2022

mrgrain left a comment

mrgrain commented Jul 18, 2022

eriktisme commented Jul 18, 2022

mrgrain commented Jul 18, 2022

eriktisme commented Jul 18, 2022

mrgrain commented Jul 18, 2022

TheRealAmazonKendra Jul 18, 2022

eriktisme Jul 19, 2022

TheRealAmazonKendra left a comment

eriktisme commented Jul 19, 2022

aws-cdk-automation commented Jul 19, 2022

mergify bot commented Jul 19, 2022

feat(cognito): added verified attribute changes #21180

feat(cognito): added verified attribute changes #21180

Conversation

eriktisme commented Jul 16, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Jul 16, 2022

eriktisme commented Jul 16, 2022

mrgrain commented Jul 17, 2022

mrgrain left a comment

Choose a reason for hiding this comment

eriktisme commented Jul 18, 2022

mrgrain left a comment

Choose a reason for hiding this comment

mrgrain commented Jul 18, 2022

eriktisme commented Jul 18, 2022

mrgrain commented Jul 18, 2022

eriktisme commented Jul 18, 2022

mrgrain commented Jul 18, 2022

TheRealAmazonKendra Jul 18, 2022

Choose a reason for hiding this comment

eriktisme Jul 19, 2022

Choose a reason for hiding this comment

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

eriktisme commented Jul 19, 2022

aws-cdk-automation commented Jul 19, 2022

AWS CodeBuild CI Report

mergify bot commented Jul 19, 2022

eriktisme commented Jul 16, 2022 •

edited

Loading