rehanvdm / serverless-website-analytics

A CDK construct that consists of a serverless backend, frontend and client side code to track website analytics
GNU General Public License v2.0
162 stars 11 forks source link

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

Closed m-radzikowski closed 3 months ago

m-radzikowski commented 3 months ago

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:

I'm pretty sure the fix is to change

https://github.com/rehanvdm/serverless-website-analytics/blob/1a1ac09eea9398db6dcced17002af74f06bb618c/infra/src/frontend.ts#L222

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 🤷‍♂️

rehanvdm commented 3 months ago

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 ofhttps://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 https://github.com/rehanvdm/serverless-website-analytics/blob/2f2326d15307155ea273bd5c966e9aede56f3a1d/src/index.ts#L96-L102 and https://github.com/rehanvdm/serverless-website-analytics/blob/2f2326d15307155ea273bd5c966e9aede56f3a1d/src/index.ts#L33-L38

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.

m-radzikowski commented 3 months ago

Thanks for the response.

The issue is:

In my example:

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:

https://github.com/rehanvdm/serverless-website-analytics/blob/2f2326d15307155ea273bd5c966e9aede56f3a1d/src/frontend.ts#L204-L212

and here:

https://github.com/rehanvdm/serverless-website-analytics/blob/1a1ac09eea9398db6dcced17002af74f06bb618c/infra/src/frontend.ts#L219-L230

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?

rehanvdm commented 3 months ago

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?

rehanvdm commented 3 months ago

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.

rehanvdm commented 3 months ago

Should be working in v1.7.0

m-radzikowski commented 3 months ago

Yes. Thank you.