Merge pull request #2343 from guardian/pm-limit-userpool-domainprefix

Limit length of cognito user pool prefix to 63

Author: philmcmahon

.changeset/clean-jokes-study.md

@@ -0,0 +1,5 @@
+---
+"@guardian/cdk": patch
+---
+
+Limit the length of the cognito user pool domainPrefix generated by the Ec2App googleAuth functionality to 63 characters

src/patterns/ec2-app/base.ts

@@ -1,5 +1,4 @@
 /* eslint "@guardian/tsdoc-required/tsdoc-required": 2 -- to begin rolling this out for public APIs. */
-import crypto from "crypto";
 import { Duration, SecretValue, Tags } from "aws-cdk-lib";
 import type { BlockDevice } from "aws-cdk-lib/aws-autoscaling";
 import { HealthCheck } from "aws-cdk-lib/aws-autoscaling";
@@ -43,6 +42,7 @@ import {
 import { AppAccess } from "../../types";
 import type { GuAsgCapacity, GuDomainName } from "../../types";
 import type { AmigoProps } from "../../types/amigo";
+import { getUserPoolDomainPrefix } from "../../utils/cognito/cognito";
 
 export interface AccessLoggingProps {
   /**
@@ -573,11 +573,10 @@ export class GuEc2App extends Construct {
       // These help ensure domain is deterministic but also unique. Key
       // assumption is that app/stack/stage combo are unique within Guardian.
       const domainPrefix = `com-gu-${app.toLowerCase()}-${scope.stage.toLowerCase()}`;
-      const suffix = crypto.createHash("md5").update(domainPrefix).digest("hex");
 
       const userPoolDomain = userPool.addDomain("domain", {
         cognitoDomain: {
-          domainPrefix: `${domainPrefix}-${suffix}`,
+          domainPrefix: getUserPoolDomainPrefix(domainPrefix),
         },
       });
 

src/utils/cognito/cognito.test.ts

@@ -0,0 +1,33 @@
+import { getUserPoolDomainPrefix } from "./cognito";
+
+describe("getUserPoolDomainPrefix", () => {
+  it("should use a full length hash where there is space", () => {
+    const shortPrefix = "hello";
+    const domainPrefix = getUserPoolDomainPrefix(shortPrefix);
+
+    expect(domainPrefix.length).toEqual(32 + shortPrefix.length + 1);
+  });
+
+  it("should include the full prefix where there is space for a 5 character hash", () => {
+    const prefix55 = new Array(55 + 1).join("#");
+    const domainPrefix = getUserPoolDomainPrefix(prefix55);
+
+    expect(domainPrefix.includes(prefix55)).toEqual(true);
+    expect(domainPrefix.length).toEqual(63);
+  });
+
+  it("should not include the full prefix where there is not space for a 5 character hash", () => {
+    const prefix56 = new Array(58 + 1).join("#");
+    const domainPrefix = getUserPoolDomainPrefix(prefix56);
+
+    expect(domainPrefix.includes(prefix56)).toEqual(false);
+    expect(domainPrefix.length).toEqual(63);
+  });
+
+  it("should be 63 characters long even with a very long prefix", () => {
+    const prefix = new Array(63 + 1).join("#");
+    const domainPrefix = getUserPoolDomainPrefix(prefix);
+
+    expect(domainPrefix.length).toEqual(63);
+  });
+});

src/utils/cognito/cognito.ts

@@ -0,0 +1,14 @@
+import crypto from "crypto";
+
+export const getUserPoolDomainPrefix = (prefix: string): string => {
+  // the user pool domain prefix has a max length of 63 characters. We want them to be unique so we add a hash to the end
+  // here we hope that a 5 character hash is unique enough to avoid collisions
+  const maxLength = 63;
+  const prefixLengthWithHashSpace = maxLength - 6; // 6 = 5 char hash, 1 hyphen
+  const suffix = crypto.createHash("md5").update(prefix).digest("hex");
+
+  // make space in prefix for hash
+  const prefixTrimmed = prefix.slice(0, prefixLengthWithHashSpace);
+
+  return `${prefixTrimmed}-${suffix}`.slice(0, maxLength);
+};