rehanvdm/serverless-website-analytics

Bug: wrong A record for Cognito domain if using a subdomain

m-radzikowski opened this issue · 6 comments

I have Hosted Zone example.com. With this config:

new Swa(this, "swa-example-com", {
	...
	auth: {
		cognito: {
			loginSubDomain: "login",
			users: [],
		},
	},
	domain: {
		name: 'swa.example.com',
		usEast1Certificate: props.certificate,
		hostedZone: route53.HostedZone.fromHostedZoneAttributes(this, "HostedZone", {
			hostedZoneId: 'Z01234',
			zoneName: 'example.com',
		}),
	},
});

What's created is:

  • Hosted Zone swa.example.com A record for the website ✅
  • Cognito Custom Domain login.swa.example.com
  • Hosted Zone login.example.com A record for the Cognito Custom Domain ❌ -> should be login.swa.example.com

I'm pretty sure the fix is to change

recordName: props.auth.cognito.loginSubDomain,

to

recordName: cognitoDomain.domainName,

The route53.ARecord Construct accepts either subdomain name or fully qualified domain name: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_route53.ARecord.html#recordname
Theoretically, the fully qualified domain name should end with . so we can do recordName: cognitoDomain.domainName + '.' but in fact CDK auto-fixes it 🤷‍♂️

Thanks for the detailed explanation.

Hosted Zone login.example.com A record for the Cognito Custom Domain ❌ -> should be login.swa.example.com

It will never create that record, it will create it on the domain/subdomain you specified with domain.name. So this is the intended behavior.

The code you referred to is to create the Alias record for the Cognito Custom Domain, so that login.swa.example.com points to the auto-generated Cognito Cloudfront Hosted UI which is in the format of https://yourdomain.us-east-1.amazoncognito.com. That code created this record - Cognito Custom Domain login.swa.example.com ✅

I have it documented in the JSDoc properties here

/**
* Optional, if not specified then no DNS records will be created. You will have to create the DNS records yourself.
*
* The Route53 hosted zone to add DNS records for CloudFront and Cognito. Cognito will be added as a subdomain of
* the domain, for example: `{auth.cognito.loginSubDomain}.{domain.name}`.
*/
readonly hostedZone?: route53.IHostedZone;
and
/**
* The unique domain prefix for the Cognito Hosted UI login page. If no `domain` is specified, this will be used to
* prefix the AWS provided domain for Cognito Hosted UI login. If `domain` is specified, then it will create the
* Cognito Hosted UI login page at `{loginSubDomain}.${domain}`.
*/
readonly loginSubDomain: string;

Let me know if I understand the problem correctly? There is nothing in the README that mentions this explicitly as in the code, which is also certainly not perfect. So it might just need to be called out explicitly.

Thanks for the response.

The issue is:

  • in Cognito, we create a subdomain to the given domain.name
  • in Route53, we create a subdomain to the Hosted Zone name, not the domain.name.

In my example:

  • Hosted Zone name: example.com (the top-level Hosted Zone domain)
  • domain.name: swa.example.com -> subdomain of the hosted zone domain!
  • cognito.loginSubDomain: login

As a result, in Cognito we have login.swa.example.com and in Route53 we have login.example.com.

The mismatch between using the domain.name and Hosted Zone top-level name for the Cognito domain is here:

/* Add the custom domain to the Cognito User Pool only AFTER the CloudFront creates the A record, otherwise the
* Cognito domain will fail, this is to say we can create DNS records ourselves */
if (props.auth?.cognito) {
const cognitoDomain = authProps.userPool!.addDomain('custom-domain', {
customDomain: {
domainName: `${props.auth.cognito.loginSubDomain}.${props.domain.name}`,
certificate: props.domain.usEast1Certificate!,
},
});

and here:

/* Add the Cognito A record if we can */
if (props.domain.hostedZone) {
new route53.ARecord(scope, name('custom-domain-dns-record'), {
recordName: props.auth.cognito.loginSubDomain,
target: route53.RecordTarget.fromAlias({
bind: () => ({
dnsName: cognitoDomain.cloudFrontDomainName,
hostedZoneId: 'Z2FDTNDATAQYW2', // CloudFront Zone ID, fixed for everyone
}),
}),
zone: props.domain.hostedZone,
});

All code comments describe the Cognito custom domain as {auth.cognito.loginSubDomain}.{domain.name}, which with auth.cognito.loginSubDomain=login and domain.name=swa.example.com should be login.swa.example.com. And it is, in Cognito, but not in the Hosted Zone.

The fix with providing the fully qualified name to ARecord in the second snippet will solve this. At the same time, it will not affecting existing setups that have domain.name equal to the Hosted Zone name, because for them the output record name will stay the same (like login.example.com will be converted by CDK to login, same as now).

For it to work, the domain.name must be the same as the Hosted Zone name - in other words, it must be the top-level domain of the Hosted Zone. So the SWA requires creating a separate Hosted Zone for custom domains to work out of the box, unless I'm missing something.

Did you try to deploy SWA on a subdomain of the Hosted Zone?

Let's work with an example, my personal site that uses Cognito and that creates these records.

SwaConfig

const hostedZone = route53.HostedZone.fromHostedZoneAttributes(this, 'HostedZone', {
    hostedZoneId: 'Z014907810MCI3U4ADO5T',
    zoneName: 'analytics.rehanvdm.com',
});

new Swa(this, name('swa'), {
     ...
      auth: {
        cognito: {
          loginSubDomain: 'login',
          ...
        }
      },

      domain: {
        name: 'analytics.rehanvdm.com',
        certificate: wildCardCertUsEast1,
        hostedZone: hostedZone,
      },
      
    });

Produces two A Records. The first for the site.

{
"rehananalyticsswacloudfrontrecord2415F54F": {
   "Type": "AWS::Route53::RecordSet",
   "Properties": {
    "Name": "analytics.rehanvdm.com.",
    "Type": "A",
    "AliasTarget": {
     "DNSName": {
      "Fn::GetAtt": [
       "rehananalyticsswawebdist39C4A125",
       "DomainName"
      ]
     },
     "HostedZoneId": {
      "Fn::FindInMap": [
       "AWSCloudFrontPartitionHostedZoneIdMap",
       {
        "Ref": "AWS::Partition"
       },
       "zoneId"
      ]
     }
    },
    "HostedZoneId": "Z014907810MCI3U4ADO5T"
   },
   "Metadata": {
    "aws:cdk:path": "rehan-analytics/rehan-analytics-swa-cloudfront-record/Resource"
   }
  },
}

Produces analytics.rehanvdm.com that points to our frontend CloudFront

image

Then for the login Cognito domain

"rehananalyticsswacustomdomaindnsrecordF02C5275": {
   "Type": "AWS::Route53::RecordSet",
   "Properties": {
    "Name": "login.analytics.rehanvdm.com.",
    "Type": "A",
    "AliasTarget": {
     "DNSName": {
      "Fn::GetAtt": [
       "rehananalyticsswauserpoolcustomdomainCloudFrontDomainName9906C9B6",
       "DomainDescription.CloudFrontDistribution"
      ]
     },
     "HostedZoneId": "Z2FDTNDATAQYW2"
    },
    "HostedZoneId": "Z014907810MCI3U4ADO5T"
   },
   "Metadata": {
    "aws:cdk:path": "rehan-analytics/rehan-analytics-swa-custom-domain-dns-record/Resource"
   }
  },

Produces login.analytics.rehanvdm.com that points to the CloudFront that Cognito created

image

image

So I fail to understand where the bug is? It is by design that if you specify the domain as analytics.rehanvdm.com that your login link will always be login.analytics.rehanvdm.com. If you want to use different domains, do not specify the hostedZone property and create the A records yourself. The Construct provides the values of the two DNS records that you have to create in Outputs.

Example diff of me not specifying hostedZone
image

If we are still missing each other somewhere, do you mind opening a PR with the change you are thinking of?

Okay I think I know what is happening, thanks to the PR ^ above. This happens if your hosted zone and the domain you specify are not the same. I assumed everyone would create a new HostedZone for the sub domain and that its name would be the same as the domain name you specified.

I could recreate your experience by making this change from my previous message.
image

The PR above fixes it. More details here if interested.

Should be working in v1.7.0

Yes. Thank you.