Repository: amplify-education/serverless-domain-manager
Pull request #529 (2022-11-26 01:01:38)

Search SubjectAlternativeNames when finding cert
<!--- Information about referencing Github Issues: https://help.github.com/articles/basic-writing-and-formatting-syntax/#referencing-issues-and-pull-requests --->
Fixes (no ticket)

**Description of Issue Fixed**
- cert alt names are not searched

**Changes proposed in this pull request**:

* search alt names when "searching by domain"
* remove code around expired certs as it's no longer needed (we filter the statuses instead)

<!--- Please remember to allow edits from maintainers: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork --->



Patch:
diff --git a/src/aws/acm-wrapper.ts b/src/aws/acm-wrapper.ts
index 97ad7ba..006f84e 100644
--- a/src/aws/acm-wrapper.ts
+++ b/src/aws/acm-wrapper.ts
@@ -33,9 +33,10 @@ class ACMWrapper {
                 "NextToken",
                 {CertificateStatuses: certStatuses},
             );
-
+            // enhancement idea: weight the choice of cert so longer expiries
+            // and RenewalEligibility = ELIGIBLE is more preferable
             if (certificateName != null) {
-                certificateArn = await this.getCertArnByCertName(certificates, certificateName);
+                certificateArn = this.getCertArnByCertName(certificates, certificateName);
             } else {
                 certificateName = domain.givenDomainName;
                 certificateArn = this.getCertArnByDomainName(certificates, certificateName);
@@ -52,50 +53,42 @@ class ACMWrapper {
     /**
      * * Gets Certificate ARN that most closely matches Cert ARN and not expired
      */
-    private async getCertArnByCertName(certificates, certName): Promise<string> {
-        // note: we only check DomainName, but a future enhancement could
-        // be to also check SubjectAlternativeNames
-        const matches = certificates.filter((certificate) => (certificate.DomainName === certName));
-        for (const certificate of matches) {
-            const certificateArn = certificate.CertificateArn;
-            const details = await throttledCall(
-                this.acm,
-                "describeCertificate",
-                {CertificateArn: certificateArn},
-            );
-            const currNotAfter = details.Certificate.NotAfter;
-            if (Date.now() < currNotAfter) {
-                Globals.logInfo(
-                    `Selecting cert with ARN=${certificateArn} with future expiry (${currNotAfter.toISOString()})`
-                );
-                return certificateArn;
-            }
-            Globals.logInfo(
-                `Ignoring cert with ARN=${certificateArn} that is expired (${currNotAfter.toISOString()})`
-            );
+    private getCertArnByCertName(certificates, certName): string {
+        const found = certificates.find((c) => c.DomainName === certName);
+        if (found) {
+          return found.CertificateArn;
         }
+        return null;
     }
 
     /**
      * * Gets Certificate ARN that most closely matches domain name
      */
-    private getCertArnByDomainName(certificates, domainName): Promise<string> {
+    private getCertArnByDomainName(certificates, domainName): string {
         // The more specific name will be the longest
         let nameLength = 0;
         let certificateArn;
-        certificates.forEach((certificate) => {
-            let certificateListName = certificate.DomainName;
-            // Looks for wild card and takes it out when checking
-            if (certificateListName[0] === "*") {
-                certificateListName = certificateListName.substring(1);
-            }
-            // Looks to see if the name in the list is within the given domain
-            // Also checks if the name is more specific than previous ones
-            if (domainName.includes(certificateListName) && certificateListName.length > nameLength) {
-                nameLength = certificateListName.length;
-                certificateArn = certificate.CertificateArn;
+        for (const currCert of certificates) {
+            const allDomainsForCert = [
+                currCert.DomainName,
+                ...(currCert.SubjectAlternativeNameSummaries || []),
+            ];
+            for (const currCertDomain of allDomainsForCert) {
+                let certificateListName = currCertDomain;
+                // Looks for wild card and take it out when checking
+                if (certificateListName[0] === "*") {
+                    certificateListName = certificateListName.substring(1);
+                }
+                // Looks to see if the name in the list is within the given domain
+                // Also checks if the name is more specific than previous ones
+                if (domainName.includes(certificateListName)
+                      && certificateListName.length > nameLength
+                ) {
+                    nameLength = certificateListName.length;
+                    certificateArn = currCert.CertificateArn;
+                }
             }
-        });
+        }
         return certificateArn;
     }
 }


Test patch:
diff --git a/test/unit-tests/index.test.ts b/test/unit-tests/index.test.ts
index 43559f6..7079ef3 100644
--- a/test/unit-tests/index.test.ts
+++ b/test/unit-tests/index.test.ts
@@ -21,10 +21,6 @@ const certTestData = {
             CertificateArn: "test_arn",
             DomainName: "test_domain",
         },
-        {
-            CertificateArn: "expired_cert_name",
-            DomainName: "cert_name",
-        },
         {
             CertificateArn: "test_given_cert_name",
             DomainName: "cert_name",
@@ -709,30 +705,6 @@ describe("Custom Domain Plugin", () => {
 
         it("Get a given certificate name", async () => {
             AWS.mock("ACM", "listCertificates", certTestData);
-            AWS.mock("ACM", "describeCertificate", (params, callback) => {
-                const fifteenDaysAsMs = 15 * 24 * 60 * 60 * 1000;
-                const fifteenDaysInFuture = new Date(Date.now() + fifteenDaysAsMs);
-                const fifteenDaysInPast = new Date(Date.now() - fifteenDaysAsMs);
-                if (params.CertificateArn === "expired_cert_name") {
-                    callback(null, {
-                        Certificate: {
-                            CertificateArn: "doesnt_matter_wont_be_used",
-                            NotAfter: fifteenDaysInPast,
-                        }
-                    });
-                    return;
-                }
-                if (params.CertificateArn === "test_given_cert_name") {
-                    callback(null, {
-                        Certificate: {
-                            CertificateArn: params.CertificateArn,
-                            NotAfter: fifteenDaysInFuture,
-                        }
-                    });
-                    return;
-                }
-                throw new Error("Programmer error: didn't add new test mock data");
-            });
 
             const plugin = constructPlugin({certificateName: "cert_name"});
             plugin.initializeVariables();
@@ -744,6 +716,91 @@ describe("Custom Domain Plugin", () => {
             expect(result).to.equal("test_given_cert_name");
         });
 
+        it("Get a given certificate by alt name with exact match", async () => {
+            AWS.mock("ACM", "listCertificates", {
+                CertificateSummaryList: [
+                    {
+                        CertificateArn: "test_nomatch",
+                        DomainName: "dontmatch.com",
+                    },
+                    {
+                        CertificateArn: "test_arn",
+                        DomainName: "test.com",
+                        SubjectAlternativeNameSummaries: [
+                          "example.com",
+                        ],
+                    },
+                ],
+            });
+
+            const options = {
+                domainName: "example.com",
+                endpointType: "REGIONAL",
+            };
+            const plugin = constructPlugin(options);
+            plugin.initializeVariables();
+
+            const dc: DomainConfig = new DomainConfig(plugin.serverless.service.custom.customDomain);
+            const acm = new ACMWrapper(dc.endpointType);
+            const result = await acm.getCertArn(dc);
+
+            expect(result).to.equal("test_arn");
+        });
+
+        it("Get a given certificate by alt name with subdomain", async () => {
+            AWS.mock("ACM", "listCertificates", {
+                CertificateSummaryList: [
+                    {
+                        CertificateArn: "test_arn",
+                        DomainName: "test.com",
+                        SubjectAlternativeNameSummaries: [
+                          "example.com",
+                        ],
+                    },
+                ],
+            });
+
+            const options = {
+                domainName: "sub.example.com",
+                endpointType: "REGIONAL",
+            };
+            const plugin = constructPlugin(options);
+            plugin.initializeVariables();
+
+            const dc: DomainConfig = new DomainConfig(plugin.serverless.service.custom.customDomain);
+            const acm = new ACMWrapper(dc.endpointType);
+            const result = await acm.getCertArn(dc);
+
+            expect(result).to.equal("test_arn");
+        });
+
+        it("Get a given certificate by alt name with wildcard", async () => {
+            AWS.mock("ACM", "listCertificates", {
+                CertificateSummaryList: [
+                    {
+                        CertificateArn: "test_arn",
+                        DomainName: "test.com",
+                        SubjectAlternativeNameSummaries: [
+                          "*.example.com",
+                        ],
+                    },
+                ],
+            });
+
+            const options = {
+                domainName: "sub.example.com",
+                endpointType: "REGIONAL",
+            };
+            const plugin = constructPlugin(options);
+            plugin.initializeVariables();
+
+            const dc: DomainConfig = new DomainConfig(plugin.serverless.service.custom.customDomain);
+            const acm = new ACMWrapper(dc.endpointType);
+            const result = await acm.getCertArn(dc);
+
+            expect(result).to.equal("test_arn");
+        });
+
         it("Create a domain name", async () => {
             AWS.mock("APIGateway", "createDomainName", (params, callback) => {
                 callback(null, {distributionDomainName: "foo", securityPolicy: "TLS_1_2"});