feat(cognito): support UserPoolGroup #31351

mazyu36 · 2024-09-07T11:21:45Z

Issue # (if applicable)

Closes #21026.

Reason for this change

To support UserPool Group L2 Construct.

Description of changes

Add UserPoolGroup class.

Description of how you validated changes

Add unit tests and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

While working on #31351, I discovered. The test case name for `User Pool Domain` was incorrectly set as `User Pool Client`. It's likely that when the code was reused from `user-pool-client.test.ts`, the test case name wasn't updated. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/aws-cdk-lib/aws-cognito/README.md

watany-dev

LGTM!

While working on aws#31351, I discovered. The test case name for `User Pool Domain` was incorrectly set as `User Pool Client`. It's likely that when the code was reused from `user-pool-client.test.ts`, the test case name wasn't updated. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation · 2024-10-21T00:11:54Z

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

…roup-21026

sumupitchayan · 2024-10-29T22:55:34Z

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

+/**
+ * Represents a user pool group.
+ */
+export interface IUserPoolGroup extends IResource {


@mazyu36 thanks for this PR - I'm curious why you added this base interface and if it is necessary?

This follows the design guidelines.
I recognize that Interfaces are essential as they are implemented across almost all resources.

https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#construct-interface

sumupitchayan · 2024-10-29T23:02:04Z

packages/@aws-cdk-testing/framework-integ/test/aws-cognito/test/integ.user-pool-group.ts

+const app = new App();
+const stack = new TestStack(app, 'user-pool-group-stack');
+
+new IntegTest(app, 'integ-user-pool-group', {


Can we add any assertions on the integ test to make sure the UserPoolGroup works as expected?

I tried to implement an assertion to verify that UserGroups are attached to the UserPool.

test.assertions.awsApiCall('cognito-identity-provider', 'ListGroupsCommand', { UserPoolId: stack.userPool.userPoolId }) .expect(ExpectedResult.objectLike({ Groups: [ { GroupName: stack.userPoolGroup.groupName, UserPoolId: stack.userPool.userPoolId, Precedence: 1, Description: 'My user pool group', RoleArn: stack.role.roleArn, }, { GroupName: stack.anotherUserPoolGroup.groupName, UserPoolId: stack.userPool.userPoolId, }, ], }));

However, since the order of items in the result list is non-deterministic, the integration tests sometimes fail.
I think this is problematic.

Based on the assertion guidelines, I think assertion is not mandatory in this case.
What do you think?

Some things you should look for in deciding if the test needs an assertion: - Integrations between services (i.e. integration libraries like `aws-cdk-lib/aws-lambda-destinations`, `aws-cdk-lib/aws-stepfunctions-tasks`, etc). - All custom resources. Must assert the expected behavior of the lambda is correct. - Anything that bundles or deploys custom code (i.e. does a Lambda function bundled with `aws-cdk-lib/aws-lambda-nodejs` still invoke or did we break bundling behavior). - IAM/Networking connections. - This one is a bit of a judgement call. Most things do not need assertions, but sometimes we handle complicated configurations involving IAM permissions or Networking access.

@mazyu36 thank you for the detailed clarification

sumupitchayan · 2024-10-29T23:11:19Z

packages/@aws-cdk-testing/framework-integ/test/aws-cognito/test/integ.user-pool-group.ts

+      userPool: userPool,
+      groupName: 'test-group',
+      description: 'My user pool group',
+      precedence: 0,


Maybe overkill, but can we also add an integ test to confirm that a UserPoolGroup without precedence passed in deploys?

Thank you. I modified AnotherUserPoolGroup to deploy without precedence.

sumupitchayan · 2024-10-30T18:22:23Z

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

+    if (
+      props.groupName !== undefined &&
+      !Token.isUnresolved(props.groupName) &&
+      !/^[\p{L}\p{M}\p{S}\p{N}\p{P}]{1,128}$/u.test(props.groupName)


I'm curious, is this how we normally perform string validations in CDK? Do you have any similar examples that use this?

The following is the similar implementation:

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

Line 434 in 6f106c7

const defaultRedirectUriPattern = /^(?=.{1,1024}$)[\p{L}\p{M}\p{S}\p{N}\p{P}]+$/u;

Regex pattern is quoted from CFn Doucument.
https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-cognito-userpoolgroup.html#cfn-cognito-userpoolgroup-groupname

sumupitchayan · 2024-10-30T18:25:08Z

packages/aws-cdk-lib/aws-cognito/README.md

+
+Support for groups in Amazon Cognito user pools enables you to create and manage groups, add users to groups, and remove users from groups.
+Use groups to create collections of users to manage their permissions or to represent different types of users.


Thanks for this PR @mazyu36 - one followup question, you mention in the README here that user pools enable you to also remove users from groups as well, but I don't see any functions that do this? Can you clarify? (My understanding might be off here)

Thanks.
I've updated README.

Since there is no remove method, I have deleted it.
A remove method is unnecessary because users can be deleted by deleting the add method from implementation.
Other add methods also do not have corresponding remove methods.

Thanks @mazyu36 - overall API design looks good to me.

mergify · 2024-10-31T15:20:06Z

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).

aws-cdk-automation · 2024-10-31T15:53:44Z

AWS CodeBuild CI Report

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

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

mergify · 2024-10-31T15:55:18Z

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).

github-actions · 2024-10-31T15:55:30Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

feat(cognito): support UserPoolGroup

1f53681

github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Sep 7, 2024

aws-cdk-automation requested a review from a team September 7, 2024 11:21

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Sep 7, 2024

aws-cdk-automation previously requested changes Sep 7, 2024

View reviewed changes

add unit tests and README

7818faf

mazyu36 changed the title ~~WIP:feat(cognito): support UserPoolGroup~~ feat(cognito): support UserPoolGroup Sep 7, 2024

mazyu36 marked this pull request as ready for review September 7, 2024 14:06

fix README

10c2477

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 7, 2024

add unit test

4209d5e

mazyu36 mentioned this pull request Sep 8, 2024

chore(cognito): fix test case name for User Pool Domain #31365

Merged

1 task

watany-dev reviewed Sep 9, 2024

View reviewed changes

packages/aws-cdk-lib/aws-cognito/README.md Show resolved Hide resolved

update README

ecdb64c

mazyu36 requested a review from watany-dev September 9, 2024 07:40

watany-dev reviewed Sep 9, 2024

View reviewed changes

github-actions bot mentioned this pull request Sep 9, 2024

Weekly PR metrics report #31370

Closed

mazyu36 added 2 commits October 21, 2024 10:03

Merge remote-tracking branch 'upstream/main' into cognito-user-pool-g…

aa62a3f

…roup-21026

Merge remote-tracking branch 'upstream/main' into cognito-user-pool-g…

b95c54a

…roup-21026

sumupitchayan self-assigned this Oct 29, 2024

sumupitchayan reviewed Oct 29, 2024

View reviewed changes

mazyu36 added 2 commits October 30, 2024 18:59

update integ

c308248

update integ

0168368

mazyu36 requested a review from sumupitchayan October 30, 2024 10:38

sumupitchayan reviewed Oct 30, 2024

View reviewed changes

update README

e455060

mazyu36 requested a review from sumupitchayan October 31, 2024 00:57

sumupitchayan approved these changes Oct 31, 2024

View reviewed changes

Merge branch 'main' into cognito-user-pool-group-21026

4346299

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 31, 2024

mergify bot merged commit 408b20f into aws:main Oct 31, 2024
12 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2024

mazyu36 deleted the cognito-user-pool-group-21026 branch October 31, 2024 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cognito): support UserPoolGroup #31351

feat(cognito): support UserPoolGroup #31351

mazyu36 commented Sep 7, 2024 •

edited

Loading

aws-cdk-automation left a comment

watany-dev left a comment

aws-cdk-automation commented Oct 21, 2024

sumupitchayan Oct 29, 2024

mazyu36 Oct 30, 2024

sumupitchayan Oct 29, 2024

mazyu36 Oct 30, 2024

sumupitchayan Oct 30, 2024

sumupitchayan Oct 29, 2024

mazyu36 Oct 30, 2024

sumupitchayan Oct 30, 2024

mazyu36 Oct 31, 2024

sumupitchayan Oct 30, 2024

mazyu36 Oct 31, 2024

sumupitchayan Oct 31, 2024

mergify bot commented Oct 31, 2024

aws-cdk-automation commented Oct 31, 2024

mergify bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024


		Support for groups in Amazon Cognito user pools enables you to create and manage groups, add users to groups, and remove users from groups.
		Use groups to create collections of users to manage their permissions or to represent different types of users.

feat(cognito): support UserPoolGroup #31351

feat(cognito): support UserPoolGroup #31351

Conversation

mazyu36 commented Sep 7, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

watany-dev left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Oct 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Oct 31, 2024

aws-cdk-automation commented Oct 31, 2024

AWS CodeBuild CI Report

mergify bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

mazyu36 commented Sep 7, 2024 •

edited

Loading