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-elasticloadbalancingv2: ALB loadBalancerDnsName token does not resolve #26103

Closed
csmcallister opened this issue Jun 23, 2023 · 4 comments
Closed
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. duplicate This issue is a duplicate. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@csmcallister
Copy link

csmcallister commented Jun 23, 2023

Describe the bug

Start with an ALB:

const alb = new elbv2.ApplicationLoadBalancer(this, "Alb", {
      vpc,
});

Now try to use that ALB's loadBalancerDnsName attribute in a Cognito User Pool Client's callback urls:

const userPoolClient = new cognito.UserPoolClient(this, "UserPoolClient", {
        ...
        callbackUrls: [
          `https://${alb.loadBalancerDnsName}/oauth2/idpresponse`.toLowerCase(),
          `https://${alb.loadBalancerDnsName}/`.toLowerCase(),
        ],
        logoutUrls: [`https://${alb.loadBalancerDnsName}/`.toLowerCase()]
      },
});

With the above, I expect the token for the alb.loadBalancerDnsName to resolve much the same as an S3 bucket's name would resolve when setting it as an environment variable for a lambda function (which is shown in the docs for Tokens here).

The issue is that the tokens do not resolve. If I look at the user pool client in the AWS console, you'll see:

image

Naturally this results in a redirect_mismatch error when trying to authenticate after visiting the ALB DNS:

image

Expected Behavior

I expected the tokens for the load balancer's DNS name to resolve.

Current Behavior

The token values do not resolve.

Reproduction Steps

Here's a stack to reproduce:

import * as cdk from 'aws-cdk-lib';
import * as cognito from "aws-cdk-lib/aws-cognito";
import * as ec2 from "aws-cdk-lib/aws-ec2";
import * as elbv2 from "aws-cdk-lib/aws-elasticloadbalancingv2";
import { Construct } from 'constructs';

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

    const vpc = new ec2.Vpc(this, "vpc", {
      cidr: "10.30.0.0/16",
      natGateways: 1,
      maxAzs: 2,
      subnetConfiguration: [
        {
          cidrMask: 24,
          name: 'public',
          subnetType: ec2.SubnetType.PUBLIC,
        },
        {
          cidrMask: 24,
          name: 'private',
          subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS,
        },
     ]
    });

    const albSg = new ec2.SecurityGroup(this, "SecurityGroupAlb", {
      vpc,
      allowAllOutbound: true,
    });
    albSg.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(443));

    const alb = new elbv2.ApplicationLoadBalancer(this, "Alb", {
      vpc,
      internetFacing: true,
      securityGroup: albSg,
    });

    const userPool = new cognito.UserPool(this, "UserPool");
    new cognito.UserPoolClient(this, "UserPoolClient", {
      userPool,
      userPoolClientName: "UserPoolClient",
      generateSecret: true,
      oAuth: {
        flows: {
          authorizationCodeGrant: true,
        },
        scopes: [cognito.OAuthScope.OPENID],
        callbackUrls: [
          `https://${alb.loadBalancerDnsName}/oauth2/idpresponse`.toLowerCase(),
          `https://${alb.loadBalancerDnsName}/`.toLowerCase(),
        ],
        logoutUrls: [`https://${alb.loadBalancerDnsName}/`.toLowerCase()]
      },
      refreshTokenValidity: cdk.Duration.days(30),
      supportedIdentityProviders: [cognito.UserPoolClientIdentityProvider.COGNITO],
    });

    new cdk.CfnOutput(this, "LoadBalancerDNS", {
      value: `https://${alb.loadBalancerDnsName}`,
    });
  }
}

Possible Solution

No idea. Current workaround is deploying the ALB first, grabbing the DNS as a CfnOutput, and then hardcoding. Not very ideal from an automation standpoint.

Additional Information/Context

I've tried using Token.asString to generate a string encoding and calling .toString() on the token object, but the result is the same. However, that was merely out of desperation since the alb.loadBalancerDnsName is supposed to already be a String.

Additionally, the token does resolve when I use it to set an environment variable in an ECS container:

const container = taskDefinition.addContainer("UiContainer", {
     ...
      environment: {
        ALB_DNS: alb.loadBalancerDnsName,
      },
    });

CDK CLI Version

2.85.0 (build 4e0d726)

Framework Version

No response

Node.js Version

16.20

OS

WSL (Ubuntu 22.04.2)

Language

Typescript

Language Version

4.9.4

Other information

No response

@csmcallister csmcallister added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 23, 2023
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Jun 23, 2023
@peterwoodworth
Copy link
Contributor

Using runtime logic (in this case, toLowerCase()) can break tokens. The generated template looks like this:

      CallbackURLs:
        - https://${token[token.744]}/oauth2/idpresponse
        - https://${token[token.744]}/
      ClientName: UserPoolClient
      GenerateSecret: true
      LogoutURLs:
        - https://${token[token.744]}/

You can't use synth time logic on something not known until deploy time. If you remove the toLowerCase() this will create a good template.

      CallbackURLs:
        - Fn::Join:
            - ""
            - - https://
              - Fn::GetAtt:
                  - Alb16C2F182
                  - DNSName
              - /oauth2/idpresponse
        - Fn::Join:
            - ""
            - - https://
              - Fn::GetAtt:
                  - Alb16C2F182
                  - DNSName
              - /
      ClientName: UserPoolClient
      GenerateSecret: true
      LogoutURLs:
        - Fn::Join:
            - ""
            - - https://
              - Fn::GetAtt:
                  - Alb16C2F182
                  - DNSName
              - /

Though, if you absolutely need to transform the name to lowercase, there will need to be a bit of a workaround.

@peterwoodworth peterwoodworth added 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 Jun 23, 2023
@csmcallister
Copy link
Author

csmcallister commented Jun 23, 2023

From the docs, "The callback URL in the app client settings must use all lowercase letters."

When I omit toLowerCase(), the token resolves, but capital case letters in the callback url break the redirect and the OAuth flow won't work (c.f. this or this).

Also, this is related to #11171. That suggests using a custom resource, which I suppose I could try although it seems to be an unconfirmed fix.

@peterwoodworth
Copy link
Contributor

Yes, I was going to suggest using a custom resource as well. If you have it take in the loadBalancerDnsName as input, you will be able to run logic on it as it will be known at deploy time, and you will be able to pass the output of the custom resource to the callbackUrls.

It looks like these are the same issues, so I'm going to close this in favor of that one.

@peterwoodworth peterwoodworth added the duplicate This issue is a duplicate. label Jun 23, 2023
@github-actions
Copy link

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. duplicate This issue is a duplicate. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants