feat(cognito): allow retrieval of UserPoolClient generated client secret #21262

Dzhuneyt · 2022-07-21T09:55:58Z

Consumers of the cognito.UserPoolClient construct currently have a way to retrieve useful information and pass that information to other pieces of infrastructure (e.g. Lambdas, ECS, Parameter Store, CfnOutput), e.g. the Client ID, which is useful for certain operations against the Cognito user pool that require a Cognito client.

However, if consumers decide to configure the Client, with a pre-generated (random) Client Secret for security reasons, by passing the generateSecret: true prop to cognito.UserPoolClient, most operations to the Cognito UserPool now require the Client ID AND Client Secret, otherwise they generate the Unable to verify secret hash for client <client-id> error (see AWS support article for more info).

Currently, the construct exposes no way to retrieve the Client Secret, forcing consumers to work around this by using custom resources or custom AWS SDK calls to retrieve it.

This PR addresses that, by exposing a new getter method of the UserPoolClient class, allowing consumers to read it on demand and pass it to other pieces of infrastructure, using code like:

const userPoolClient = new cognito.UserPoolClient(this, 'UserPoolClient', {
  userPool,
  generateSecret: true,
});

// Allows you to pass the generated secret to other pieces of infrastructure
const secret = userPoolClient.userPoolClientSecret;

Closes #7225

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

…ion with `userDataCausesReplacement` (#18726) If both `addSignalOnExitCommand` _and_ `userDataCausesReplacement` are used it results in an invalid logicalId being used in the `cfn-signal` call. This is due to `addSignalOnExitCommand` getting the logicalID from `Stack.getLogicalId` which does not take into consideration logicalId overrides which `userDataCausesReplacement` uses. This updates `addSignalOnExitCommand` to use the `logicalId` of the resource which is evaluated lazily and happens after all overrides. fixes #12749 ---- *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-21T09:56:05Z

misterjoshua

I like the idea and this is a great start, but I have a few thoughts.

@pahud Could you please tag a few more community reviewers for this one?

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

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

misterjoshua

This looks really good. I have only a few small nitpicky comments.

I'd like to see more community feedback and perhaps some review by the CDK group, but otherwise, I'm happy with this changeset.

Thanks for your work so far!

packages/@aws-cdk/aws-cognito/test/user-pool-resource-server.integ.snapshot/manifest.json

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

…com/Dzhuneyt/aws-cdk into dzhuneyt/issue-7225-userpool-secret

Pull request has been modified.

Dzhuneyt · 2022-08-01T22:01:32Z

@TheRealAmazonKendra this should be ready for another set of eyes now. Just FYI. Thanks for the good tips above.

TheRealAmazonKendra

Just a few nitpicky things and then this will be ready to go!

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

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

Co-authored-by: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com>

Pull request has been modified.

mergify · 2022-08-02T09:02:54Z

update

✅ Branch has been successfully updated

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

…com/Dzhuneyt/aws-cdk into dzhuneyt/issue-7225-userpool-secret

Pull request has been modified.

TheRealAmazonKendra

I've left suggestions below with the kind of testing I'm looking for. Feel free to just commit my suggestions directly. I've tested them locally and it passes.

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

Co-authored-by: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com>

Pull request has been modified.

mergify · 2022-08-05T06:59:50Z

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

mergify · 2022-08-05T08:13:33Z

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 · 2022-08-05T08:13:42Z

AWS CodeBuild CI Report

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

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

…ret (aws#21262) Consumers of the `cognito.UserPoolClient` construct currently have a way to retrieve useful information and pass that information to other pieces of infrastructure (e.g. Lambdas, ECS, Parameter Store, CfnOutput), e.g. the Client ID, which is useful for certain operations against the Cognito user pool that require a Cognito client. However, if consumers decide to configure the Client, with a pre-generated (random) Client Secret for security reasons, by passing the `generateSecret: true` prop to `cognito.UserPoolClient`, most operations to the Cognito UserPool now require the Client ID AND Client Secret, otherwise they generate the `Unable to verify secret hash for client <client-id>` error (see [AWS support article](https://aws.amazon.com/premiumsupport/knowledge-center/cognito-unable-to-verify-secret-hash/) for more info). Currently, the construct exposes no way to retrieve the Client Secret, forcing consumers to work around this by using custom resources or custom AWS SDK calls to retrieve it. This PR addresses that, by exposing a new getter method of the `UserPoolClient` class, allowing consumers to read it on demand and pass it to other pieces of infrastructure, using code like: ```ts const userPoolClient = new cognito.UserPoolClient(this, 'UserPoolClient', { userPool, generateSecret: true, }); // Allows you to pass the generated secret to other pieces of infrastructure const secret = userPoolClient.userPoolClientSecret; ``` Closes aws#7225 ---- ### 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*

Dzhuneyt and others added 13 commits January 31, 2022 14:49

Merge remote-tracking branch 'remote/master'

fb6de3d

Merge branch 'aws:master' into master

6a35cbd

Merge branch 'aws:master' into master

f2958c6

Merge branch 'aws:master' into master

5531ac1

Merge branch 'aws:master' into master

846ecd3

Merge branch 'aws:master' into master

ee14ff1

Merge branch 'aws:main' into master

2720ad1

Merge branch 'aws:main' into master

ca9bf75

Merge branch 'aws:main' into master

8489ce6

Merge branch 'aws:main' into master

924bbe7

feat: Ability to retrieve Cognito UserPoolClient secret

1360913

chore: Update integ test snapshots

99bcdb9

aws-cdk-automation requested a review from a team July 21, 2022 09:56

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 labels Jul 21, 2022

Dzhuneyt added 3 commits July 21, 2022 10:05

chore: Update README.md

0ec6c57

chore: Update README.md

5aee970

Merge branch 'main' into dzhuneyt/issue-7225-userpool-secret

6d73171

misterjoshua reviewed Jul 21, 2022

View reviewed changes

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

packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts Show resolved Hide resolved

Dzhuneyt added 5 commits July 22, 2022 18:22

Merge branch 'aws:main' into dzhuneyt/issue-7225-userpool-secret

699c248

chore: Address PR reviews

24f98bb

test: Update tests and snapshots

034d997

test: Update integration test

8222327

test: Update integration test

2b59ac4

misterjoshua reviewed Jul 22, 2022

View reviewed changes

packages/@aws-cdk/aws-cognito/test/user-pool-resource-server.integ.snapshot/manifest.json Show resolved Hide resolved

packages/@aws-cdk/aws-cognito/lib/user-pool-client.ts Outdated Show resolved Hide resolved

Dzhuneyt added 2 commits July 22, 2022 20:29

chore: Address PR reviews

678bb71

Merge branch 'main' into dzhuneyt/issue-7225-userpool-secret

8d35a34

Merge branch 'dzhuneyt/issue-7225-userpool-secret' of https://github.…

2fa43da

…com/Dzhuneyt/aws-cdk into dzhuneyt/issue-7225-userpool-secret

Dzhuneyt changed the title ~~feat: Retrieve UserPoolClient generated secret~~ feat(cognito): Retrieve UserPoolClient generated secret Aug 1, 2022

Dzhuneyt changed the title ~~feat(cognito): Retrieve UserPoolClient generated secret~~ feat(cognito): Allow retrieving UserPoolClient generated Client Secret Aug 1, 2022

TheRealAmazonKendra changed the title ~~feat(cognito): Allow retrieving UserPoolClient generated Client Secret~~ feat(cognito): allow retrieval of UserPoolClient generated client secret Aug 2, 2022

TheRealAmazonKendra previously requested changes Aug 2, 2022

View reviewed changes

Apply suggestions from code review

ceeac26

Co-authored-by: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com>

Dzhuneyt and others added 2 commits August 2, 2022 09:01

fix: Failing test

acf40ac

Merge branch 'main' into dzhuneyt/issue-7225-userpool-secret

136624e

TheRealAmazonKendra previously requested changes Aug 2, 2022

View reviewed changes

packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts Show resolved Hide resolved

Dzhuneyt requested a review from TheRealAmazonKendra August 2, 2022 14:34

Dzhuneyt added 3 commits August 3, 2022 11:53

test: Validate that GenerateSecret has correct value in CF template

6f5aa09

Merge branch 'dzhuneyt/issue-7225-userpool-secret' of https://github.…

9c3612e

…com/Dzhuneyt/aws-cdk into dzhuneyt/issue-7225-userpool-secret

Merge branch 'main' into dzhuneyt/issue-7225-userpool-secret

b2190ee

TheRealAmazonKendra previously requested changes Aug 3, 2022

View reviewed changes

packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-cognito/test/user-pool-client.test.ts Outdated Show resolved Hide resolved

Dzhuneyt and others added 2 commits August 5, 2022 09:06

Apply suggestions from code review

d2e1158

Co-authored-by: Kendra Neil <53584728+TheRealAmazonKendra@users.noreply.github.com>

Merge branch 'aws:main' into dzhuneyt/issue-7225-userpool-secret

a1f3b55

TheRealAmazonKendra approved these changes Aug 5, 2022

View reviewed changes

Merge branch 'main' into dzhuneyt/issue-7225-userpool-secret

d6c2cc8

mergify bot merged commit 67a24ba into aws:main Aug 5, 2022

Dzhuneyt deleted the dzhuneyt/issue-7225-userpool-secret branch August 5, 2022 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cognito): allow retrieval of UserPoolClient generated client secret #21262

feat(cognito): allow retrieval of UserPoolClient generated client secret #21262

Dzhuneyt commented Jul 21, 2022 •

edited

Loading

gitpod-io bot commented Jul 21, 2022

misterjoshua left a comment •

edited

Loading

misterjoshua left a comment

Dzhuneyt commented Aug 1, 2022

TheRealAmazonKendra left a comment

mergify bot commented Aug 2, 2022

TheRealAmazonKendra left a comment

mergify bot commented Aug 5, 2022

mergify bot commented Aug 5, 2022

aws-cdk-automation commented Aug 5, 2022

feat(cognito): allow retrieval of UserPoolClient generated client secret #21262

feat(cognito): allow retrieval of UserPoolClient generated client secret #21262

Conversation

Dzhuneyt commented Jul 21, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Jul 21, 2022

misterjoshua left a comment • edited Loading

Choose a reason for hiding this comment

misterjoshua left a comment

Choose a reason for hiding this comment

Dzhuneyt commented Aug 1, 2022

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 2, 2022

✅ Branch has been successfully updated

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 5, 2022

mergify bot commented Aug 5, 2022

aws-cdk-automation commented Aug 5, 2022

AWS CodeBuild CI Report

Dzhuneyt commented Jul 21, 2022 •

edited

Loading

misterjoshua left a comment •

edited

Loading