aws/aws-cdk

[cognito] ALB dns name has upper case that cognito does not accept as a callback url

jolo-dev opened this issue · 15 comments

By using the aws-elasticloadbalancingv2-actions, I noticed that the Cognito construct wants to have the callback URL in all lower case. Because it is not the case, the callback to the loadbalancer is not reached.

Reproduction Steps

Basically, I took it from here

    const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', {
      vpc,
      internetFacing: true,
    });

    const userPool = new cognito.UserPool(this, 'UserPool');
    const userPoolClient = new cognito.UserPoolClient(this, 'Client', {
      userPool,

      // Required minimal configuration for use with an ELB
      generateSecret: true,
      authFlows: {
        userPassword: true,
      },
      oAuth: {
        flows: {
          authorizationCodeGrant: true,
        },
        scopes: [cognito.OAuthScope.EMAIL],
        callbackUrls: [
          `https://${lb.loadBalancerDnsName}/oauth2/idpresponse`,
        ],
      },
    });
    const cfnClient = userPoolClient.node.defaultChild as cognito.CfnUserPoolClient;
    cfnClient.addPropertyOverride('RefreshTokenValidity', 1);
    cfnClient.addPropertyOverride('SupportedIdentityProviders', ['COGNITO']);

    const userPoolDomain = new cognito.UserPoolDomain(this, 'Domain', {
      userPool,
      cognitoDomain: {
        domainPrefix: 'test-cdk-prefix',
      },
    });

    lb.addListener('Listener', {
      port: 443,
      certificates: [certificate],
      defaultAction: new actions.AuthenticateCognitoAction({
        userPool,
        userPoolClient,
        userPoolDomain,
        next: elbv2.ListenerAction.fixedResponse(200, {
          contentType: 'text/plain',
          messageBody: 'Authenticated',
        }),
      }),
    });

    new CfnOutput(this, 'DNS', {
      value: lb.loadBalancerDnsName,
    });
  }
}

const app = new App();
new CognitoStack(app, 'integ-cognito');
app.synth();

What did you expect to happen?

A redirect to the loadbalancer.

What actually happened?

The cognito domain appends an error.

Environment

  • CLI Version : 1.70.0
  • Framework Version: 1.70.0
  • Node.js Version: 12.18.3
  • OS : Ubuntu
  • Language (Version): Python 3.8

Other


This is 🐛 Bug Report

I think a solution is to enforce the callback-url always to lower case.
https://stackoverflow.com/questions/55279710/aws-cognito-and-application-load-balancer-error-in-saml-response-processing-red

Currently, it is manual work after a deployment since a Token is generated.

Unfortunately, there is not an easy way for the CDK to do anything here. Since the DNS name is generated as part of CloudFormation deployment, there is no way for the CDK to know/manipulate the DNS name.
This needs to be taken up with the Cognito service team to accept upper case domain names.

The workaround is to add a lambda backed custom resource that does this conversion.

On the CDK end, we could introduce a loadBalacerDnsNameLower property in the aws-elasticloadbalancerv2 package which automatically creates this custom resource.

@rix0rrr @njlynch - what do you think?

@nija-at Thanks for taking a look into this.
I was also wondering if that is not an ELB thing because I don't think a need for having not all lowercase of the loadBalacerDnsName.

Internal ref w/ Cognito service: t.corp/D18522969

@nija-at has loadBalancerDnsNameLower been introduced? This is a real PITA because many tools expect a lower-case hostname but the CDK does not provide any easy way to lower-case a string (a custom resource or a lambda is not "easy").

Hello, any updates on this issue?

Amazing that this issue still exists. Definitely should be fixed by the Cognito team. For non-CDK users, an intrinsic ToLowerCase function for CFN templates would also help. If CDK team isn't going to do anything to help, can you guys please forward issue as appropriate and then close?

You can use the custom resource below to get the DNS OnCreate
But this is still a token so you cannot lowercase the result directly.

const describeLoadBalancerRole = new Role(
  this,
  "DescribeLoadBalancersRole",
  {
    assumedBy: new CompositePrincipal(
      new ServicePrincipal("ec2.amazonaws.com"),
      new ServicePrincipal("elasticloadbalancing.amazonaws.com"),
      new ServicePrincipal("lambda.amazonaws.com")
    )
  }
);
describeLoadBalancerRole.addToPolicy(
  new PolicyStatement({
    resources: ["*"],
    actions: ["elasticloadbalancing:Describe*"]
  })
);
describeLoadBalancerRole.addToPolicy(
  new PolicyStatement({
    resources: ["*"],
    actions: [
      "ec2:DescribeInstances",
      "ec2:DescribeClassicLinkInstances",
      "ec2:DescribeSecurityGroups"
    ]
  })
);

const describeLoadBalancer = new AwsCustomResource(
  this,
  "DescribeLoadBalancers",
  {
    resourceType: "Custom::DescribeLoadBalancers",
    onCreate: {
      service: "ELBv2",
      action: "describeLoadBalancers",
      parameters: {
        LoadBalancerArns: [loadBalancer.loadBalancerArn]
      },
      physicalResourceId: PhysicalResourceId.of(
        `${id}-AwsSdk-${loadBalancer.loadBalancerFullName}`
      )
    },
    policy: AwsCustomResourcePolicy.fromSdkCalls({
      resources: AwsCustomResourcePolicy.ANY_RESOURCE
    }),
    role: describeLoadBalancerRole,
    logRetention: RetentionDays.FIVE_DAYS
  }
);

const loadBalancerTrueDnsName = describeLoadBalancer.getResponseField(
  "LoadBalancers.0.DNSName"
);
new CfnOutput(this, "loadbalancer-address", {
  value: loadBalancerTrueDnsName
});

If you don't mind using an additional lambda function, you can try this in your cdk:

  1. Create a lambda function that use aws-sdk's ELBv2 to get the dns you want
  2. Create a custom resource to invoke the lambda function

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

I would say this should stay open.

Keep this open

This has been open for almost 3 and a half years, any update?

I have a similar issue. Impossible to retrieve the load balancer dns name using CfnOutput. It always takes a value like ${token[token.XXX]}. Any update on it? Thanks

+1 here