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

aws-cognito: oAuth.callbackUrls of UserPoolClient should not have http://example.com #28204

Open
jolo-dev opened this issue Nov 30, 2023 · 5 comments
Labels
@aws-cdk/aws-cognito Related to Amazon Cognito bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@jolo-dev
Copy link
Contributor

Describe the bug

The property oAuth.callbackUrls of UserPoolClient should be mandatory when flows.authorizationCodeGrant or flows.implicitCodeGrant istrue otherwise, the oAuth.callbackUrls results into ['http://example.com'].

Expected Behavior

An error is thrown when oAuth.callbackUrls is empty but flows.authorizationCodeGrant or flows.implicitCodeGrant istrue.

Current Behavior

oAuth.callbackUrls results into ['http://example.com']

Reproduction Steps

declare const userPoolIdentityProviderOidc: UserPoolIdentityProviderOidc;
// 👇 code goes through
const userPoolClient = new UserPoolClient(this, 'UserPoolClient', {
      userPool,
      userPoolClientName: `${serviceName}-user-pool-client`,
      generateSecret: true,
      supportedIdentityProviders: [{
        name: userPoolIdentityProviderOidc.providerName,
      }],
      authFlows: {
        userPassword: true,
      },
      oAuth: {
        flows: {
          authorizationCodeGrant: true,
        },
      },
    });

Possible Solution

Either remove the default or put a guardrail or throw an error when flows.authorizationCodeGrant or flows.implicitCodeGrant istrue and oAuth.callbackUrls is not set.

Additional Information/Context

No response

CDK CLI Version

2.110.0

Framework Version

No response

Node.js Version

18.15

OS

macOS 14.1

Language

TypeScript

Language Version

5.2

Other information

No response

@jolo-dev jolo-dev added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2023
@github-actions github-actions bot added the @aws-cdk/aws-cognito Related to Amazon Cognito label Nov 30, 2023
@jolo-dev jolo-dev changed the title aws-cognito: Better Typescript support when using UserPoolClient aws-cognito: oAuth.callbackUrls of UserPoolClient should not have http://example.com Nov 30, 2023
@khushail khushail self-assigned this Nov 30, 2023
@khushail khushail added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2023
@khushail
Copy link
Contributor

thanks @jolo-dev for reporting this. This seems to be bug in the code.

@markmansur
Copy link
Contributor

markmansur commented Dec 3, 2023

I agree that things like example.com should not be the default. The issue here is that default for flows is

flows: {
    authorizationCodeGrant:true, 
    implicityCodeGrant:true
}

and the callbackUrls prop is optional. But trying it out in the AWS console, it looks like it requires you to add at least one callback.

Say we keep the prop as optional and throw an error if it's empty. This means if a user is configuring this construct with the defaults for the first time, they will get an error prompting them for at least one callback url. Not a great experience.

The other option: we only throw an error when authorizationCodeGrant or implicityCodeGrant are explicitly set. Otherwise we will fall back to the default of example.com. I think this is what you meant in your original description, but just clarifying 🙂

@jolo-dev
Copy link
Contributor Author

jolo-dev commented Dec 3, 2023

Hey @markmansur,
Thanks for working on it :)

But trying it out in the AWS console, it looks like it requires you to add at least one callback.

Really? Have you tried to deploy it without a callbackUrls?

The other option: we only throw an error when authorizationCodeGrant or implicityCodeGrant are explicitly set. Otherwise we will fall back to the default of example.com. I think this is what you meant in your original description, but just clarifying 🙂

@markmansur Yes, exactly.

@markmansur
Copy link
Contributor

markmansur commented Dec 3, 2023

Thanks for working on it :)

No worries! 🙂

Really? Have you tried to deploy it without a callbackUrls?

Yeah, see this example

    const pool = new CfnUserPool(this, 'pool')

    new CfnUserPoolClient(this, 'client', {
      allowedOAuthFlows: ['code', 'implicit'],
      userPoolId: this.splitArn(pool.attrArn, cdk.ArnFormat.SLASH_RESOURCE_NAME).resourceName!
    })
9:27:54 a.m. | CREATE_FAILED        | AWS::Cognito::UserPoolClient | client
Resource handler returned message: "CallbackUrls can not be empty when code flow or implicit flow is selected (Service: CognitoIdentityProvider, Status Code: 400, Request ID: d78662fd-5f9c-47be-bd8e-8daa054aae7a)"
(RequestToken: 52599340-35de-5ec5-adef-099ba27a2d2c, HandlerErrorCode: InvalidRequest)

callbackUrls can only be empty when there allowedOAuthFlows is either empty or only contains ['client_credentials']

    // works!
    const pool = new CfnUserPool(this, 'pool')
    new CfnUserPoolClient(this, 'client', {
      userPoolId: this.splitArn(pool.attrArn, cdk.ArnFormat.SLASH_RESOURCE_NAME).resourceName!
    })
    // works!
    const pool = new CfnUserPool(this, 'pool')
    new CfnUserPoolClient(this, 'client', {
      allowedOAuthFlows: ['client_credentials'],
      generateSecret: true,
      userPoolId: this.splitArn(pool.attrArn, cdk.ArnFormat.SLASH_RESOURCE_NAME).resourceName!
    })

@jolo-dev
Copy link
Contributor Author

jolo-dev commented Dec 3, 2023

Yeah, see this example

    const pool = new CfnUserPool(this, 'pool')

    new CfnUserPoolClient(this, 'client', {
      allowedOAuthFlows: ['code', 'implicit'],
      userPoolId: this.splitArn(pool.attrArn, cdk.ArnFormat.SLASH_RESOURCE_NAME).resourceName!
    })
9:27:54 a.m. | CREATE_FAILED        | AWS::Cognito::UserPoolClient | client
Resource handler returned message: "CallbackUrls can not be empty when code flow or implicit flow is selected (Service: CognitoIdentityProvider, Status Code: 400, Request ID: d78662fd-5f9c-47be-bd8e-8daa054aae7a)"
(RequestToken: 52599340-35de-5ec5-adef-099ba27a2d2c, HandlerErrorCode: InvalidRequest)

Oh that's weird.

@markmansur I have added some comments on your PR

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 bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants