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.

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

UserPoolClient - Retrieve the client secret #7225

Closed
nija-at opened this issue Apr 7, 2020 · 16 comments · Fixed by #21262
Closed

UserPoolClient - Retrieve the client secret #7225

nija-at opened this issue Apr 7, 2020 · 16 comments · Fixed by #21262
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p1

Comments

@nija-at
Copy link
Contributor

nija-at commented Apr 7, 2020

Forking off #3037

I'm not sure if this is the right place, but in my use case, I'd like to authenticate with cognito from an application load balancer action using a secret generated via a UserPoolClient or CfnUserPoolClient.

It doesn't seem clear how the oidc client secret can be gotten from the UserPoolClient and given to the application load balancer rule actions, as I seem to get a nonsense value from from the UserPoolClient.userPoolClientClientSecret property.

Apparently there was a ClientSecret attribute documented on UserPoolClient resources at one point. I'm not sure what happened.

awsdocs/aws-cloudformation-user-guide#72

Originally posted by @misterjoshua in #3037 (comment)

@nija-at nija-at added @aws-cdk/aws-cognito Related to Amazon Cognito feature-request A feature should be added or improved. labels Apr 7, 2020
@nija-at nija-at self-assigned this Apr 7, 2020
@nija-at
Copy link
Contributor Author

nija-at commented Apr 7, 2020

Originally posted by @dveijck in #3037 (comment)

Hi @misterjoshua,

I believe there is no convenient way to automate this at the moment. The client secret is not available through CloudFormation and thus there is no way to use it from CDK simply by referencing the attribute.

That said, what you might be able to do is the following*:

  • Within the UserPoolClient stack create a CloudFormation custom resource backed by an AWS Lambda.
  • The Lambda function (with the appropriate roles) could be used to get the client secret
  • Then create a parameter in either SSM or SecretsManager.
  • As SSM and SecretsManager secret parameters are supported as input by CloudFormation/CDK you might be able to reference those within your ALB stack and use that in your ALB rule actions.

*Not sure if this plays well if all resources are defined within the same stack, because secrets needs to be available at the moment the CloudFormation template is being rolled out. But if you use different stacks for your Cognito UserPoolClient and ALB resources I don't see why it wouldn't work.

I haven't tried this myself, so that's how far my warranty goes ;-), but it might be worth a try.

By the way have you considered posting your question on Stackoverflow? Maybe other people might have a potential solution for this as well.

@nija-at
Copy link
Contributor Author

nija-at commented Apr 7, 2020

Originally posted by @0xdevalias in #3037 (comment)

As a followup to @dveijck's post above replying to @misterjoshua; CDK has a really short/convenient syntax for custom resources that just need to call AWS SDK functions:

A basic example (untested for this use case exactly) derived from some similar code I wrote recently:

const describeCognitoUserPoolClient = new cr.AwsCustomResource(
      this,
      'DescribeCognitoUserPoolClient',
      {
        resourceType: 'Custom::DescribeCognitoUserPoolClient',
        onCreate: {
          region: 'us-east-1',
          service: 'CognitoIdentityServiceProvider',
          action: 'describeUserPoolClient',
          parameters: {
            UserPoolId: userPool.userPoolId,
            ClientId: userPoolClient.userPoolClientId,
          },
          physicalResourceId: cr.PhysicalResourceId.of(userPoolClient.userPoolClientId),
        },
        // TODO: can we restrict this policy more?
        policy: cr.AwsCustomResourcePolicy.fromSdkCalls({
          resources: cr.AwsCustomResourcePolicy.ANY_RESOURCE,
        }),
      }
    )

    const userPoolClientSecret = describeCognitoUserPoolClient.getResponseField(
      'UserPoolClient.ClientSecret'
    )
    new cdk.CfnOutput(this, 'UserPoolClientSecret', {
      value: userPoolClientSecret,
    })

@nija-at nija-at added the effort/medium Medium work item – several days of effort label Apr 7, 2020
@misterjoshua
Copy link
Contributor

I've discovered since originally posting my message that I can add an ActionProperty to my ListenerRule of type authenticate-cognito and the application load balancer appears to handle the secret for me, so my original use case isn't a big deal anymore.

@nija-at nija-at added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 labels Aug 18, 2020
@guzmonne
Copy link

guzmonne commented Sep 7, 2021

Originally posted by @0xdevalias in #3037 (comment)

As a followup to @dveijck's post above replying to @misterjoshua; CDK has a really short/convenient syntax for custom resources that just need to call AWS SDK functions:

A basic example (untested for this use case exactly) derived from some similar code I wrote recently:

const describeCognitoUserPoolClient = new cr.AwsCustomResource(
      this,
      'DescribeCognitoUserPoolClient',
      {
        resourceType: 'Custom::DescribeCognitoUserPoolClient',
        onCreate: {
          region: 'us-east-1',
          service: 'CognitoIdentityServiceProvider',
          action: 'describeUserPoolClient',
          parameters: {
            UserPoolId: userPool.userPoolId,
            ClientId: userPoolClient.userPoolClientId,
          },
          physicalResourceId: cr.PhysicalResourceId.of(userPoolClient.userPoolClientId),
        },
        // TODO: can we restrict this policy more?
        policy: cr.AwsCustomResourcePolicy.fromSdkCalls({
          resources: cr.AwsCustomResourcePolicy.ANY_RESOURCE,
        }),
      }
    )

    const userPoolClientSecret = describeCognitoUserPoolClient.getResponseField(
      'UserPoolClient.ClientSecret'
    )
    new cdk.CfnOutput(this, 'UserPoolClientSecret', {
      value: userPoolClientSecret,
    })

It worked perfectly for me.

@rix0rrr rix0rrr added p1 and removed p2 labels Mar 16, 2022
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Jun 21, 2022

Should we implement this custom resource solution in CDK? I didn't find any issues related to this on the coverage roadmap.

@faemmi
Copy link

faemmi commented Jul 13, 2022

@nija-at solutions worked like a charm for me. I would also have loved to this as a feature of the CDK since I think its something that might not be too inconvenient, e.g. if people have to build their custom authentication solutions for AWS Cognito.

@sdelamo
Copy link

sdelamo commented Jul 21, 2022

Currently, it is possible to get the ClientID (UserPoolClient:.getUserPoolClientId) but not the secret. I think the UserPoolClient api should expose a method Optional<String> getUserPoolClientSecret(). Even if the proposed workaround works.

@Dzhuneyt
Copy link
Contributor

I'll create a PR that brings the proposed workaround to the CDK core and leave it to the maintainers to decide whether or not it should be merged. Just FYI.

@sdelamo
Copy link

sdelamo commented Jul 21, 2022

I tried to use the workaround in Java without success:

 private String getUserPoolClientSecret(UserPool userPool, UserPoolClient userPoolClient) {
        AwsCustomResource describeCognitoUserPoolClient = AwsCustomResource.Builder.create(this, "DescribeCognitoUserPoolClient")
                .resourceType("Custom::DescribeCognitoUserPoolClient")
                .onCreate(AwsSdkCall.builder()
                        .region("us-east-1")
                        .service("CognitoIdentityServiceProvider")
                        .action("describeUserPoolClient")
                        .parameters(Map.of("UserPoolId", userPool.getUserPoolId(),
                                        "ClientId", userPoolClient.getUserPoolClientId()))
                        .physicalResourceId(PhysicalResourceId.of(userPoolClient.getUserPoolClientId()))
                        .build())
                .policy(AwsCustomResourcePolicy.fromSdkCalls(SdkCallsPolicyOptions.builder()
                        .resources(ANY_RESOURCE)
                        .build()))
                .build();
        return describeCognitoUserPoolClient.getResponseField("UserPoolClient.ClientSecret");
}

using this fails with CustomResource attribute error: Vendor response doesn't contain UserPoolClient.ClientSecret key in object.

I am probably missing something.

@Dzhuneyt
Copy link
Contributor

Dzhuneyt commented Jul 27, 2022

@nija-at @rix0rrr @jogold I've created a PR to address this feature (which is already approved by a few other contributors) but not sure what the next step is to get it merged: #21262

It is pending approval by the "CDK team" and I'm not sure what this means and if there's anything else to be done on my side to bring this to the attention of that team. Figured one of you fellas might be a part of this team.

@TassoKarkanisAGMT
Copy link

TassoKarkanisAGMT commented Aug 2, 2022

Here is @0xdevalias's solution again, this time in Python:

    def get_user_pool_client_secret(self, pool, client):
        # Maybe a method on the user pool client will be supported soon:
        #
        #  https://github.com/aws/aws-cdk/issues/7225
        
        client_cr = cr.AwsCustomResource(
            self, "CognitoUserPoolDescribeClient",
            function_name="DescribeCognitoUserPoolClient",
            resource_type="Custom::DescribeCognitoUserPoolClient",
            on_create=cr.AwsSdkCall(
                region="us-east-2",
                service="CognitoIdentityServiceProvider",
                action="describeUserPoolClient",
                parameters={
                    "UserPoolId": pool.user_pool_id,
                    "ClientId": client.user_pool_client_id,
                },
                physical_resource_id=cr.PhysicalResourceId.of(client.user_pool_client_id),
            ),
            policy=cr.AwsCustomResourcePolicy.from_sdk_calls(
                resources=cr.AwsCustomResourcePolicy.ANY_RESOURCE
            )
        )

        return client_cr.get_response_field("UserPoolClient.ClientSecret")

A method would be great.

@mergify mergify bot closed this as completed in #21262 Aug 5, 2022
mergify bot pushed a commit that referenced this issue Aug 5, 2022
…ret (#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 #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*
@github-actions
Copy link

github-actions bot commented Aug 5, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@Dzhuneyt
Copy link
Contributor

Dzhuneyt commented Aug 5, 2022

@TassoKarkanisAGMT a recent PR just got merged that exposes this as a propery.

After next week's release, you should be able to retrieve the user pool client secret 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;

@NickDiz
Copy link

NickDiz commented Aug 9, 2022

@Dzhuneyt Do you have a day next week this is planning to release? Our team is very much looking forward to using the exposed secret and not having to implement the tedious work arounds

@Dzhuneyt
Copy link
Contributor

Dzhuneyt commented Aug 10, 2022

@NickDiz sorry, I'm not familiar with the exact day of week but from memory - the CDK was buffering and releasing new PRs once per week. Which day - not sure.

@RichardInnocent
Copy link

@NickDiz This is part v2.37.0, released a couple of hours after your question.

Thanks Dzhuneyt, your work is much appreciated.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.