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

[cognito] UserPoolClient CallbackURLs specified when disableOAuth is true #10311

Closed
blimmer opened this issue Sep 11, 2020 · 2 comments · Fixed by #10588
Closed

[cognito] UserPoolClient CallbackURLs specified when disableOAuth is true #10311

blimmer opened this issue Sep 11, 2020 · 2 comments · Fixed by #10588
Assignees
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 good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@blimmer
Copy link
Contributor

blimmer commented Sep 11, 2020

When disableOAuth is true, cdk synth still produces an array of CallbackURLs for a UserPoolClient.

Reproduction Steps

import * as cdk from '@aws-cdk/core';
import * as cognito from '@aws-cdk/aws-cognito';

export class CdkStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

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

What did you expect to happen?

I didn't expect any OAuth values to be present in the synth-ed document.

What actually happened?

Resources:
  UserPool6BA7E5F2:
    Type: AWS::Cognito::UserPool
    Properties:
      AccountRecoverySetting:
        RecoveryMechanisms:
          - Name: verified_phone_number
            Priority: 1
          - Name: verified_email
            Priority: 2
      AdminCreateUserConfig:
        AllowAdminCreateUserOnly: true
      EmailVerificationMessage: The verification code to your new account is {####}
      EmailVerificationSubject: Verify your new account
      SmsVerificationMessage: The verification code to your new account is {####}
      VerificationMessageTemplate:
        DefaultEmailOption: CONFIRM_WITH_CODE
        EmailMessage: The verification code to your new account is {####}
        EmailSubject: Verify your new account
        SmsMessage: The verification code to your new account is {####}
    Metadata:
      aws:cdk:path: CdkStack/UserPool/Resource
  UserPoolClient2F5918F7:
    Type: AWS::Cognito::UserPoolClient
    Properties:
      UserPoolId:
        Ref: UserPool6BA7E5F2
      AllowedOAuthFlowsUserPoolClient: false
      CallbackURLs:
        - https://example.com
      SupportedIdentityProviders:
        - COGNITO
    Metadata:
      aws:cdk:path: CdkStack/UserPoolClient/Resource
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Modules: aws-cdk=1.62.0,@aws-cdk/assets=1.62.0,@aws-cdk/aws-applicationautoscaling=1.62.0,@aws-cdk/aws-autoscaling-common=1.62.0,@aws-cdk/aws-cloudwatch=1.62.0,@aws-cdk/aws-codeguruprofiler=1.62.0,@aws-cdk/aws-cognito=1.62.0,@aws-cdk/aws-ec2=1.62.0,@aws-cdk/aws-events=1.62.0,@aws-cdk/aws-iam=1.62.0,@aws-cdk/aws-kms=1.62.0,@aws-cdk/aws-lambda=1.62.0,@aws-cdk/aws-logs=1.62.0,@aws-cdk/aws-s3=1.62.0,@aws-cdk/aws-s3-assets=1.62.0,@aws-cdk/aws-sqs=1.62.0,@aws-cdk/aws-ssm=1.62.0,@aws-cdk/cloud-assembly-schema=1.62.0,@aws-cdk/core=1.62.0,@aws-cdk/custom-resources=1.62.0,@aws-cdk/cx-api=1.62.0,@aws-cdk/region-info=1.62.0,jsii-runtime=node.js/v12.18.3
    Condition: CDKMetadataAvailable

Specifically, I didn't expect this block to be present:

CallbackURLs:
  - https://example.com

Environment

  • CLI Version : 1.62.0 (build 8c2d7fc)
  • Framework Version: 1.62.0
  • Node.js Version: v12.18.3
  • OS : macOS
  • Language (Version):

Other


This is 🐛 Bug Report

@blimmer blimmer added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 11, 2020
@github-actions github-actions bot added the @aws-cdk/aws-cognito Related to Amazon Cognito label Sep 11, 2020
@nija-at
Copy link
Contributor

nija-at commented Sep 17, 2020

It looks like this was missed here. This must likely also be applied to logoutUrls.

callbackUrLs: callbackUrls && callbackUrls.length > 0 ? callbackUrls : undefined,
logoutUrLs: props.oAuth?.logoutUrls,

@blimmer - does this cause any impact to the Cognito client or your application because of this?

@nija-at nija-at added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 17, 2020
@blimmer
Copy link
Contributor Author

blimmer commented Sep 17, 2020

Hey @nija-at - I didn't notice anything weird from this, just the presence of that URL in the cdk diff.

@nija-at nija-at added p2 and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Sep 17, 2020
shivlaks added a commit that referenced this issue Sep 29, 2020
…user pool clients

When the `UserPoolClient` property `disableOAuth` is set, callback URLs should not be
rendered as it represents a list of allowed redirects for identity providers.

Added in a condition that only renders the callback URL default of `https://example.com`
if OAuth is enabled.

Closes #10311
@mergify mergify bot closed this as completed in #10588 Sep 30, 2020
mergify bot pushed a commit that referenced this issue Sep 30, 2020
…user pool clients (#10588)

When the `UserPoolClient` property `disableOAuth` is set, callback URLs should not be
rendered as it represents a list of allowed redirects for identity providers.

Added in a condition that only renders the callback URL default of `https://example.com`
if OAuth is enabled.

Closes #10311


----

*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 bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants