From 172bb6d9779a5c1f7c613b701d26b87fd3f91a28 Mon Sep 17 00:00:00 2001 From: Kishore <42832651+kishore03109@users.noreply.github.com> Date: Tue, 2 Apr 2024 18:59:21 +0800 Subject: [PATCH 1/6] fix(server): server should die if unable to connect to db (#1265) ## Problem something in me wanted to check if we indeed exit if we fail to connect to db, and the answer is... no [node.js](https://nodejs.org/docs/latest-v18.x/api/process.html#processexitcode_1) states that > A number which will be the process exit code, when the process either exits gracefully, or is exited via process.exit() without specifying a code. so it does not actually do the exiting, which leads to silent failures --- src/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.js b/src/server.js index 894dae598..454e56e6f 100644 --- a/src/server.js +++ b/src/server.js @@ -487,5 +487,5 @@ sequelize // If we cannot connect to the db, report an error using status code // And gracefully shut down the application since we can't serve client - process.exitCode = 1 + process.exit(1) }) From 3fe07343cf6ffc5c8950a6c5d34ab105a133cc37 Mon Sep 17 00:00:00 2001 From: Kishore <42832651+kishore03109@users.noreply.github.com> Date: Wed, 3 Apr 2024 10:07:29 +0800 Subject: [PATCH 2/6] fix(dig): dig not working (#1246) ## Problem We do have some checks in the backend to check prior to any site launch that there are AAAA records/CAA records present. In this case, the dig commands to check the AAAA records [failed](https://ogp.datadoghq.com/logs?query=service%3Aisomer%20env%3Aproduction%20%22An%20error%20occurred%20while%20performing%20dig%20for%20domain%3A%22%20&cols=host%2Cservice&fromUser=true&index=%2A&messageDisplay=inline&refresh_mode=sliding&storage=hot&stream_sort=desc&viz=stream&from_ts=1711457521321&to_ts=1711543921321&live=true). The reason for above is that we were using a library called `node-dig-dns`. This called the dig command [directly](https://github.com/StephanGeorg/node-dig-dns/blob/master/src/index.js#L78) at a system level. However, our docker container does not have the dig command out of the box. This resulted in the existence of AAAA records not being caught. We are also codifying a check for CAA records and ensuring that if there exist at least one caa record and it uses our redirection service, it should have letsencrypt as one of the caa record. To prevent accidental commits to live indirection repo during dev, also adding a check to only commit to the indirection repository iff it is in prod environment. ## Solution just use node's dns resolver directly. this way we dont have to depend on an external library's implementation of node and dont have to install unnecessary deps in the docker. remove dep introduced in #1244 **Breaking Changes** - [ ] Yes - this PR contains breaking changes - Details ... - [x] No - this PR is backwards compatible with ALL of the following feature flags in this [doc](https://www.notion.so/opengov/Existing-feature-flags-518ad2cdc325420893a105e88c432be5) **Features**: Screenshot 2024-03-27 at 11 21 44 PM ### Manual test (not to be copied over to deployment notes) - [ ] add these lines of code at the end of server.js ``` const formResponses = [ { submissionId: "", requesterEmail: "kishore@open.gov.sg", repoName: "kishore-test-dev-emil", primaryDomain: "google.com", redirectionDomain: "www.google.com", agencyEmail: "kishore@open.gov.sg", }, ] formsgSiteLaunchRouter.handleSiteLaunchResults(formResponses, "test") ``` - [ ] Assert that the email comes out to --- Dockerfile | 3 +- package-lock.json | 19 +- package.json | 3 +- .../formsg/__tests__/formsgSiteLaunch.spec.ts | 202 ++++++++++++++++++ src/routes/formsg/formsgSiteLaunch.ts | 176 +++++++++------ src/services/identity/ReposService.ts | 6 +- .../utilServices/SendDNSRecordEmailClient.ts | 32 ++- src/types/dig.ts | 27 +-- src/types/node-dig-dns/node-dig-dns.d.ts | 1 - src/types/nodeError.ts | 11 + src/types/siteLaunch.ts | 13 ++ 11 files changed, 369 insertions(+), 124 deletions(-) create mode 100644 src/routes/formsg/__tests__/formsgSiteLaunch.spec.ts delete mode 100644 src/types/node-dig-dns/node-dig-dns.d.ts create mode 100644 src/types/nodeError.ts diff --git a/Dockerfile b/Dockerfile index f68a11d24..8ccca8b30 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,8 +4,7 @@ WORKDIR /opt/isomercms-backend RUN apk update && \ apk add --no-cache bash && \ apk add git && \ - apk add openssh-client && \ - apk add bind-tools + apk add openssh-client RUN adduser -u 900 webapp -D -h /home/webapp -s /bin/sh USER webapp diff --git a/package-lock.json b/package-lock.json index 4896eb4e8..cd3b5f831 100644 --- a/package-lock.json +++ b/package-lock.json @@ -67,7 +67,6 @@ "morgan": "~1.10.0", "neverthrow": "^6.1.0", "nocache": "^3.0.4", - "node-dig-dns": "^0.3.3", "otplib": "^12.0.1", "papaparse": "^5.4.1", "patch-package": "^8.0.0", @@ -2109,6 +2108,7 @@ "version": "7.23.9", "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.23.9.tgz", "integrity": "sha512-0CX6F+BI2s9dkUqr08KFrAIZgNFj75rdBU/DjCyYLIaV/quFjkk6T+EJ2LkZHyZTbEV4L5p97mNkUsHl2wLFAw==", + "dev": true, "dependencies": { "regenerator-runtime": "^0.14.0" }, @@ -2119,7 +2119,8 @@ "node_modules/@babel/runtime/node_modules/regenerator-runtime": { "version": "0.14.1", "resolved": "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.14.1.tgz", - "integrity": "sha512-dYnhHh0nJoMfnkZs6GmmhFknAGRrLznOu5nc9ML+EJxGvrx6H7teuevqVqCuPcPK//3eDrrjQhehXVx9cnkGdw==" + "integrity": "sha512-dYnhHh0nJoMfnkZs6GmmhFknAGRrLznOu5nc9ML+EJxGvrx6H7teuevqVqCuPcPK//3eDrrjQhehXVx9cnkGdw==", + "dev": true }, "node_modules/@babel/template": { "version": "7.23.9", @@ -14607,11 +14608,6 @@ "resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz", "integrity": "sha512-H5ZhCF25riFd9uB5UCkVKo61m3S/xZk1x4wA6yp/L3RFP6Z/eHH1ymQcGLo7J3GMPfm0V/7m1tryHuGVxpqEBQ==" }, - "node_modules/lodash.compact": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/lodash.compact/-/lodash.compact-3.0.1.tgz", - "integrity": "sha512-2ozeiPi+5eBXW1CLtzjk8XQFhQOEMwwfxblqeq6EGyTxZJ1bPATqilY0e6g2SLQpP4KuMeuioBhEnWz5Pr7ICQ==" - }, "node_modules/lodash.find": { "version": "4.6.0", "resolved": "https://registry.npmjs.org/lodash.find/-/lodash.find-4.6.0.tgz", @@ -15394,15 +15390,6 @@ "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-6.1.0.tgz", "integrity": "sha512-+eawOlIgy680F0kBzPUNFhMZGtJ1YmqM6l4+Crf4IkImjYrO/mqPwRMh352g23uIaQKFItcQ64I7KMaJxHgAVA==" }, - "node_modules/node-dig-dns": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/node-dig-dns/-/node-dig-dns-0.3.3.tgz", - "integrity": "sha512-l/57tMh/8D89MRiMF+iIYz3h5p2pGusxY75MM7gjsPegV/WqyVWC266DM9HTOeirmp4CdrQQBMbQssb2W7maAA==", - "dependencies": { - "@babel/runtime": "^7.18.9", - "lodash.compact": "^3.0.1" - } - }, "node_modules/node-fetch": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", diff --git a/package.json b/package.json index 522475441..b9a723bbb 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,9 @@ "postinstall": "patch-package" }, "dependencies": { - "@aws-sdk/client-dynamodb": "^3.521.0", "@aws-sdk/client-amplify": "^3.521.0", "@aws-sdk/client-cloudwatch-logs": "^3.521.0", + "@aws-sdk/client-dynamodb": "^3.521.0", "@aws-sdk/client-secrets-manager": "^3.389.0", "@aws-sdk/lib-dynamodb": "^3.521.0", "@growthbook/growthbook": "^0.34.0", @@ -85,7 +85,6 @@ "morgan": "~1.10.0", "neverthrow": "^6.1.0", "nocache": "^3.0.4", - "node-dig-dns": "^0.3.3", "otplib": "^12.0.1", "papaparse": "^5.4.1", "patch-package": "^8.0.0", diff --git a/src/routes/formsg/__tests__/formsgSiteLaunch.spec.ts b/src/routes/formsg/__tests__/formsgSiteLaunch.spec.ts new file mode 100644 index 000000000..9ad88cbed --- /dev/null +++ b/src/routes/formsg/__tests__/formsgSiteLaunch.spec.ts @@ -0,0 +1,202 @@ +import { CaaRecord } from "node:dns" +import dnsPromises from "node:dns/promises" + +import UsersService from "@root/services/identity/UsersService" +import InfraService from "@root/services/infra/InfraService" +import { DigDNSRecord } from "@root/services/utilServices/SendDNSRecordEmailClient" +import { SiteLaunchResult } from "@root/types/siteLaunch" + +import { FormsgSiteLaunchRouter as _FormsgSiteLaunchRouter } from "../formsgSiteLaunch" + +const MockUsersService = ({ + findById: jest.fn(), + findByGitHubId: jest.fn(), + findByEmail: jest.fn(), +} as any) as UsersService + +const MockInfraService = ({ + getSite: jest.fn(), +} as any) as InfraService + +const FormsgSiteLaunch = new _FormsgSiteLaunchRouter({ + usersService: MockUsersService, + infraService: MockInfraService, +}) + +const mockResult: SiteLaunchResult = ({ + siteName: "testSite", + primaryDomain: "test.com", + primaryDomainSource: "test.com", +} as any) as SiteLaunchResult + +const mockResultWithRedirection: SiteLaunchResult = ({ + siteName: "testSite", + primaryDomain: "test.com", + primaryDomainSource: "test.com", + redirectionDomainSource: "test.com", + redirectionDomainTarget: "www.test.com", +} as any) as SiteLaunchResult + +describe("FormsgSiteLaunchRouter", () => { + it("should add quad A records if any exists", async () => { + // Arrange + jest + .spyOn(dnsPromises, "resolve6") + .mockResolvedValue(["2404:6800:4003:c0f::64", "2404:6800:4003:c0f::8b"]) + const expectedResult = [ + { + domain: "test.com", + value: "2404:6800:4003:c0f::64", + type: "AAAA", + }, + { + domain: "test.com", + value: "2404:6800:4003:c0f::8b", + type: "AAAA", + }, + ] as DigDNSRecord[] + + // Act + const actualResult = await FormsgSiteLaunch.digAAAADomainRecords(mockResult) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should not add quad A records if none exists", async () => { + // Arrange + jest.spyOn(dnsPromises, "resolve6").mockResolvedValue([]) + const expectedResult = [] as DigDNSRecord[] + + // Act + const actualResult = await FormsgSiteLaunch.digAAAADomainRecords(mockResult) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should add AWS CAA records if required", async () => { + // Arrange + const caaResult: CaaRecord[] = [ + { + critical: 0, + issue: "cloudflaressl.com", + }, + ] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: true, + addLetsEncryptCAA: false, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords(mockResult) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should not add AWS CAA if not required", async () => { + // Arrange + const caaResult: CaaRecord[] = [] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: false, + addLetsEncryptCAA: false, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords(mockResult) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should not add AWS CAA if already present", async () => { + // Arrange + const caaResult: CaaRecord[] = [ + { + critical: 0, + issue: "amazon.com", + }, + ] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: false, + addLetsEncryptCAA: false, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords(mockResult) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should add LetsEncrypt CAA records if required", async () => { + // Arrange + const caaResult: CaaRecord[] = [ + { + critical: 0, + issue: "amazon.com", + }, + ] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: false, + addLetsEncryptCAA: true, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords( + mockResultWithRedirection + ) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should not add LetsEncrypt CAA if not required", async () => { + // Arrange + const caaResult: CaaRecord[] = [] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: false, + addLetsEncryptCAA: false, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords( + mockResultWithRedirection + ) + + // Assert + expect(actualResult).toEqual(expectedResult) + }) + + it("should not add LetsEncrypt CAA if already present", async () => { + // Arrange + const caaResult: CaaRecord[] = [ + { + critical: 0, + issue: "letsencrypt.org", + }, + { + critical: 0, + issue: "amazon.com", + }, + ] + jest.spyOn(dnsPromises, "resolveCaa").mockResolvedValue(caaResult) + const expectedResult = { + addAWSACMCertCAA: false, + addLetsEncryptCAA: false, + } + + // Act + const actualResult = await FormsgSiteLaunch.digCAADomainRecords( + mockResultWithRedirection + ) + // Assert + expect(actualResult).toEqual(expectedResult) + }) +}) diff --git a/src/routes/formsg/formsgSiteLaunch.ts b/src/routes/formsg/formsgSiteLaunch.ts index f4906f9fe..261479198 100644 --- a/src/routes/formsg/formsgSiteLaunch.ts +++ b/src/routes/formsg/formsgSiteLaunch.ts @@ -1,9 +1,10 @@ +import { CaaRecord } from "node:dns" +import dnsPromises from "node:dns/promises" + import { DecryptedContentAndAttachments } from "@opengovsg/formsg-sdk/dist/types" import autoBind from "auto-bind" -import axios from "axios" import express, { RequestHandler } from "express" import { err, ok } from "neverthrow" -import dig from "node-dig-dns" import { config } from "@config/config" @@ -17,13 +18,15 @@ import { ISOMER_SUPPORT_EMAIL } from "@root/constants" import { attachFormSGHandler } from "@root/middleware" import { mailer } from "@root/services/utilServices/MailClient" import { + DigDNSRecord, DnsRecordsEmailProps, LaunchFailureEmailProps, getDNSRecordsEmailBody, getErrorEmailBody, } from "@root/services/utilServices/SendDNSRecordEmailClient" import TRUSTED_AMPLIFY_CAA_RECORDS from "@root/types/caaAmplify" -import { DigResponse, DigType } from "@root/types/dig" +import { isErrnoException } from "@root/types/nodeError" +import { SiteLaunchResult } from "@root/types/siteLaunch" import UsersService from "@services/identity/UsersService" import InfraService from "@services/infra/InfraService" @@ -225,24 +228,99 @@ export class FormsgSiteLaunchRouter { await mailer.sendMail(requesterEmail, subject, html) } - digDomainRecords = async ( - domain: string, - digType: DigType - ): Promise => - dig([domain, digType]) - .then((result: DigResponse) => { - logger.info(`Received DIG response: ${JSON.stringify(result)}`) - return result - }) - .catch((err: unknown) => { - logger.error( - `An error occurred while performing dig for domain: ${domain}: ${JSON.stringify( - err - )}` + digAAAADomainRecords = async ( + launchResult: SiteLaunchResult + ): Promise => { + try { + // check for AAAA records + const quadADigResponses = await dnsPromises.resolve6( + launchResult.primaryDomainSource + ) + + if (quadADigResponses.length <= 0) { + return [] + } + return quadADigResponses.map((record) => ({ + domain: launchResult.primaryDomainSource, + type: "AAAA", + value: record, + })) + } catch (e) { + if (isErrnoException(e) && e.code === "ENODATA") { + logger.info( + `Domain ${launchResult.primaryDomainSource} does not have any AAAA records.` ) - return null + } + logger.error( + `Error when trying to get AAAA records for domain ${launchResult.primaryDomainSource}: ${e}` + ) + throw e + } + } + + digCAADomainRecords = async ( + launchResult: SiteLaunchResult + ): Promise<{ + addAWSACMCertCAA: boolean + addLetsEncryptCAA: boolean + }> => { + try { + const caaDigResponses: CaaRecord[] = await dnsPromises.resolveCaa( + launchResult.primaryDomainSource + ) + + if (caaDigResponses.length === 0) { + return { + addAWSACMCertCAA: false, + addLetsEncryptCAA: false, + } + } + + const caaRecords = caaDigResponses + + /** + * NOTE: If there exists more than one CAA Record, we need to + * 1. check if they have whitelisted Amazon CAA && letsencrypt.org CAA (if using redir) + * 2. if not, send email to inform them to whitelist Amazon CAA and letsencrypt.org CAA (if using redir) + */ + const hasAmazonCAAWhitelisted = caaRecords.some((record) => { + const isAmazonCAA = TRUSTED_AMPLIFY_CAA_RECORDS.some( + (trustedCAA) => + trustedCAA === record.issue || trustedCAA === record.issuewild + ) + return isAmazonCAA }) + const isUsingRedirectionService = !!launchResult.redirectionDomainSource + + const needsLetsEncryptCAAWhitelisted = + isUsingRedirectionService && + !caaRecords.some((record) => { + const isLetsEncryptCAA = + record.issue === "letsencrypt.org" || + record.issuewild === "letsencrypt.org" + return isLetsEncryptCAA + }) + + const result = { + addAWSACMCertCAA: !hasAmazonCAAWhitelisted, + addLetsEncryptCAA: needsLetsEncryptCAAWhitelisted, + } + + return result + } catch (e) { + if (isErrnoException(e) && e.code === "ENODATA") { + logger.info( + `Domain ${launchResult.primaryDomainSource} does not have any CAA records.` + ) + } + logger.error( + `Error when trying to get CAA records for domain ${launchResult.primaryDomainSource}: ${e}` + ) + throw e + } + } + private async handleSiteLaunchResults( formResponses: FormResponsesProps[], submissionId: string @@ -254,60 +332,18 @@ export class FormsgSiteLaunchRouter { const successResults: DnsRecordsEmailProps[] = [] for (const launchResult of launchResults) { if (launchResult.isOk()) { - // check for AAAA records - const quadADigResponse = await this.digDomainRecords( - launchResult.value.primaryDomainSource, - "AAAA" - ) - const caaDigResponse = await this.digDomainRecords( - launchResult.value.primaryDomainSource, - "CAA" + const successResult: DnsRecordsEmailProps = launchResult.value + + successResult.quadARecords = await this.digAAAADomainRecords( + launchResult.value ) + const { + addAWSACMCertCAA, + addLetsEncryptCAA, + } = await this.digCAADomainRecords(launchResult.value) - const successResult: DnsRecordsEmailProps = launchResult.value - if (quadADigResponse && quadADigResponse.answer) { - const quadARecords = quadADigResponse.answer - successResult.quadARecords = quadARecords.map((record) => ({ - domain: record.domain, - class: record.class, - type: record.type, - value: record.value, - })) - } else { - logger.info( - `Unable to get dig response for domain: ${launchResult.value.primaryDomainSource}. Skipping check for AAAA records` - ) - } - - if (!caaDigResponse) { - logger.info( - `Unable to get dig response for domain: ${launchResult.value.primaryDomainSource}. Skipping check for CAA records` - ) - } else if (caaDigResponse.answer) { - const caaRecords = caaDigResponse.answer - - /** - * NOTE: If there exists more than one CAA Record, we need to - * 1. check if they have whitelisted Amazon CAA - * 2. if not, send email to inform them to whitelist Amazon CAA - */ - const hasAmazonCAAWhitelisted = caaRecords.some((record) => { - const isAmazonCAA = TRUSTED_AMPLIFY_CAA_RECORDS.some( - (trustedCAA) => trustedCAA === record.value - ) - return isAmazonCAA - }) - if (caaRecords.length > 0 && !hasAmazonCAAWhitelisted) { - successResult.addCAARecord = true - } else { - successResult.addCAARecord = false - } - } else { - logger.info( - `${launchResult.value.primaryDomainSource} Domain does not have any CAA records.` - ) - } - // Create better uptime monitor + successResult.addAWSACMCertCAA = addAWSACMCertCAA + successResult.addLetsEncryptCAA = addLetsEncryptCAA successResults.push(successResult) } } diff --git a/src/services/identity/ReposService.ts b/src/services/identity/ReposService.ts index 2c09de224..966c00e0f 100644 --- a/src/services/identity/ReposService.ts +++ b/src/services/identity/ReposService.ts @@ -315,12 +315,14 @@ export const createRecords = (zoneId: string): Record[] => { return records; }; ` - + const isProd = config.get("env") === "prod" + const branchName = isProd ? "staging" : "develop" return ResultAsync.fromPromise( octokit.repos.getContent({ owner: ISOMER_GITHUB_ORGANIZATION_NAME, repo: DNS_INDIRECTION_REPO, path: `dns/${primaryDomain}.ts`, + ref: branchName, }), () => errAsync(true) ) @@ -346,6 +348,7 @@ export const createRecords = (zoneId: string): Record[] => { path: `dns/${primaryDomain}.ts`, message: `Update ${primaryDomain}.ts`, content: Buffer.from(template).toString("base64"), + branch: branchName, sha, }), (error) => { @@ -369,6 +372,7 @@ export const createRecords = (zoneId: string): Record[] => { path: `dns/${primaryDomain}.ts`, message: `Create ${primaryDomain}.ts`, content: Buffer.from(template).toString("base64"), + branch: branchName, }), (error) => { logger.error( diff --git a/src/services/utilServices/SendDNSRecordEmailClient.ts b/src/services/utilServices/SendDNSRecordEmailClient.ts index 0218b669d..815591988 100644 --- a/src/services/utilServices/SendDNSRecordEmailClient.ts +++ b/src/services/utilServices/SendDNSRecordEmailClient.ts @@ -5,7 +5,6 @@ import { DigType } from "@root/types/dig" export interface DigDNSRecord { domain: string - class: string type: DigType value: string } @@ -21,7 +20,8 @@ export interface DnsRecordsEmailProps { redirectionDomainSource?: string redirectionDomainTarget?: string quadARecords?: DigDNSRecord[] - addCAARecord?: boolean + addLetsEncryptCAA?: boolean + addAWSACMCertCAA?: boolean } export interface LaunchFailureEmailProps { @@ -115,7 +115,7 @@ export function getDNSRecordsEmailBody( Object.keys(groupedDnsRecords).forEach((repoName) => { groupedDnsRecords[repoName].forEach((dnsRecord) => { - if (dnsRecord.addCAARecord) { + if (dnsRecord.addAWSACMCertCAA || dnsRecord.addLetsEncryptCAA) { html += `

Please add CAA records for the following repo: ${repoName}.` html += ` @@ -128,7 +128,10 @@ export function getDNSRecordsEmailBody( - + ` + + if (dnsRecord.addAWSACMCertCAA) { + html += ` @@ -142,7 +145,25 @@ export function getDNSRecordsEmailBody( - + ` + } + if (dnsRecord.addLetsEncryptCAA) { + html += ` + + + + + + + + + + + + + ` + } + html += `
Value
${repoName} ${dnsRecord.primaryDomainSource}issuewild amazontrust.com
${repoName}${dnsRecord.primaryDomainSource}0issueletsencrypt.org
${dnsRecord.primaryDomainSource}0issuewildletsencrypt.org
` } }) @@ -174,7 +195,6 @@ export function getDNSRecordsEmailBody( html += ` ${record.domain} - ${record.class} ${record.type} ${record.value} ` diff --git a/src/types/dig.ts b/src/types/dig.ts index 9cf4f352e..2588febbf 100644 --- a/src/types/dig.ts +++ b/src/types/dig.ts @@ -1,26 +1 @@ -export type DigResponse = { - question: string[][] - answer?: { - domain: string - ttl: string - class: string - type: DigType - value: string - }[] - time?: number - server?: string - datetime?: string - size?: number -} - -export type DigType = - | "A" - | "AAAA" - | "CNAME" - | "MX" - | "NS" - | "PTR" - | "SOA" - | "SRV" - | "TXT" - | "CAA" +export type DigType = "AAAA" | "CAA" diff --git a/src/types/node-dig-dns/node-dig-dns.d.ts b/src/types/node-dig-dns/node-dig-dns.d.ts deleted file mode 100644 index d14bacbf4..000000000 --- a/src/types/node-dig-dns/node-dig-dns.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare module "node-dig-dns" diff --git a/src/types/nodeError.ts b/src/types/nodeError.ts new file mode 100644 index 000000000..fd9765fbb --- /dev/null +++ b/src/types/nodeError.ts @@ -0,0 +1,11 @@ +/* eslint-disable import/prefer-default-export */ +export function isErrnoException(obj: unknown): obj is NodeJS.ErrnoException { + const err = obj as NodeJS.ErrnoException + return ( + err && + (typeof err.errno === "number" || err.errno === undefined) && + (typeof err.code === "string" || err.code === undefined) && + (typeof err.path === "string" || err.path === undefined) && + (typeof err.syscall === "string" || err.syscall === undefined) + ) +} diff --git a/src/types/siteLaunch.ts b/src/types/siteLaunch.ts index 90dc5a9e0..0965e7e8f 100644 --- a/src/types/siteLaunch.ts +++ b/src/types/siteLaunch.ts @@ -40,6 +40,19 @@ export interface SiteLaunchMessage { statusMetadata?: string } +export type SiteLaunchResult = Omit< + SiteLaunchMessage, + | "status" + | "statusMetadata" + | "redirectionDomain" + | "appId" + | "requestorEmail" + | "agencyEmail" +> & { + redirectionDomainSource?: string + redirectionDomainTarget?: string +} + export function isSiteLaunchMessage(obj: unknown): obj is SiteLaunchMessage { if (!obj) { return false From a453b9f7d6ada354a007352526ee636b3baa3d62 Mon Sep 17 00:00:00 2001 From: Timothee Groleau Date: Thu, 4 Apr 2024 11:20:10 +0800 Subject: [PATCH 3/6] fix: remove unecessary join and site retrieval (#1268) --- src/services/identity/NotificationsService.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/services/identity/NotificationsService.ts b/src/services/identity/NotificationsService.ts index fa4261c4c..695e9a810 100644 --- a/src/services/identity/NotificationsService.ts +++ b/src/services/identity/NotificationsService.ts @@ -228,6 +228,7 @@ class NotificationsService { const recentTargetNotification = await this.repository.findOne({ where: { user_id: siteMember.userId, + site_id: siteMember.siteId, type: notificationType, created_at: { [Op.gte]: getNotificationExpiryDate(notificationType), @@ -235,16 +236,6 @@ class NotificationsService { link, source_username: notificationSourceUsername, }, - include: [ - { - model: Site, - as: "site", - required: true, - where: { - id: siteMember.siteId, - }, - }, - ], }) if (recentTargetNotification) { From ccde289f6dfd31542f5f5857fc8431a1bc9ab47b Mon Sep 17 00:00:00 2001 From: Timothee Groleau Date: Thu, 4 Apr 2024 13:04:54 +0800 Subject: [PATCH 4/6] Improve APM spans (no more ) (#1267) * refactor: rename wrapper based on original function, only when original.name exists * feat: utility to name all methods of an object * feat: ensure all route handlers are named * feat: drop the bound prefix in span names * chore: remove unused eslint rule disabling --------- Co-authored-by: Alexander Lee --- src/middleware/routeHandler.ts | 19 ++++++++------ src/routes/formsg/formsgSiteCreation.ts | 2 ++ src/routes/formsg/formsgSiteLaunch.ts | 2 ++ src/routes/v2/authenticated/collaborators.ts | 2 ++ src/routes/v2/authenticated/notifications.ts | 2 ++ src/routes/v2/authenticated/review.ts | 2 ++ src/routes/v2/authenticated/sites.ts | 2 ++ src/routes/v2/authenticated/users.ts | 2 ++ src/routes/v2/authenticatedSites/media.ts | 2 ++ .../v2/authenticatedSites/repoManagement.ts | 2 ++ src/routes/v2/sgidAuth.ts | 2 ++ src/utils/apm-utils.ts | 25 +++++++++++++++++++ 12 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 src/utils/apm-utils.ts diff --git a/src/middleware/routeHandler.ts b/src/middleware/routeHandler.ts index da27016b9..3fdc60025 100644 --- a/src/middleware/routeHandler.ts +++ b/src/middleware/routeHandler.ts @@ -83,11 +83,16 @@ const nameRouteHandlerWrapper = < Params = Record, Locals extends Record = Record >( - func: RequestHandler, - name: string + wrapper: RequestHandler, + original: { name: string } ): RequestHandler => { - Object.defineProperty(func, "name", { value: name, writable: false }) - return func + if (original.name) { + Object.defineProperty(wrapper, "name", { + value: original.name.replace(/^bound /, ""), + writable: false, + }) + } + return wrapper } // Used when there are no write API calls to the repo on GitHub @@ -96,7 +101,7 @@ export const attachReadRouteHandlerWrapper: RouteWrapper = (routeHandler) => Promise.resolve(routeHandler(req, res, next)).catch((err: Error) => { next(err) }) - }, routeHandler.name) + }, routeHandler) // Used when there are write API calls to the repo on GitHub export const attachWriteRouteHandlerWrapper: RouteWrapper<{ @@ -135,7 +140,7 @@ export const attachWriteRouteHandlerWrapper: RouteWrapper<{ next(err) } ) - }, routeHandler.name) + }, routeHandler) export const attachRollbackRouteHandlerWrapper: RouteWrapper< { @@ -340,4 +345,4 @@ export const attachRollbackRouteHandlerWrapper: RouteWrapper< next(err) } ) - }, routeHandler.name) + }, routeHandler) diff --git a/src/routes/formsg/formsgSiteCreation.ts b/src/routes/formsg/formsgSiteCreation.ts index a202a22ef..22252ed5a 100644 --- a/src/routes/formsg/formsgSiteCreation.ts +++ b/src/routes/formsg/formsgSiteCreation.ts @@ -11,6 +11,7 @@ import { BadRequestError } from "@errors/BadRequestError" import { getField } from "@utils/formsg-utils" import { attachFormSGHandler } from "@root/middleware" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import GitFileSystemService from "@services/db/GitFileSystemService" import UsersService from "@services/identity/UsersService" import InfraService from "@services/infra/InfraService" @@ -45,6 +46,7 @@ export class FormsgSiteCreateRouter { this.infraService = infraService this.gitFileSystemService = gitFileSystemService // We need to bind all methods because we don't invoke them from the class directly + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/formsg/formsgSiteLaunch.ts b/src/routes/formsg/formsgSiteLaunch.ts index 261479198..ac0ab6af3 100644 --- a/src/routes/formsg/formsgSiteLaunch.ts +++ b/src/routes/formsg/formsgSiteLaunch.ts @@ -27,6 +27,7 @@ import { import TRUSTED_AMPLIFY_CAA_RECORDS from "@root/types/caaAmplify" import { isErrnoException } from "@root/types/nodeError" import { SiteLaunchResult } from "@root/types/siteLaunch" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import UsersService from "@services/identity/UsersService" import InfraService from "@services/infra/InfraService" @@ -150,6 +151,7 @@ export class FormsgSiteLaunchRouter { this.usersService = usersService this.infraService = infraService // We need to bind all methods because we don't invoke them from the class directly + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticated/collaborators.ts b/src/routes/v2/authenticated/collaborators.ts index 82ffaa9dd..f6a434e90 100644 --- a/src/routes/v2/authenticated/collaborators.ts +++ b/src/routes/v2/authenticated/collaborators.ts @@ -13,6 +13,7 @@ import { attachSiteHandler } from "@root/middleware" import { RouteCheckerMiddleware } from "@root/middleware/routeChecker" import { RequestHandler } from "@root/types" import { UserDto } from "@root/types/dto/review" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { CreateCollaboratorRequestSchema } from "@root/validators/RequestSchema" import CollaboratorsService from "@services/identity/CollaboratorsService" @@ -33,6 +34,7 @@ export class CollaboratorsRouter { }: CollaboratorsRouterProps) { this.collaboratorsService = collaboratorsService this.authorizationMiddleware = authorizationMiddleware + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticated/notifications.ts b/src/routes/v2/authenticated/notifications.ts index 832d54d4c..0d2eac85c 100644 --- a/src/routes/v2/authenticated/notifications.ts +++ b/src/routes/v2/authenticated/notifications.ts @@ -10,6 +10,7 @@ import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { attachSiteHandler } from "@root/middleware" import { AuthorizationMiddleware } from "@root/middleware/authorization" import { RequestHandler } from "@root/types" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import NotificationsService, { NotificationResponse, } from "@services/identity/NotificationsService" @@ -31,6 +32,7 @@ export class NotificationsRouter { }: NotificationsRouterProps) { this.notificationsService = notificationsService this.authorizationMiddleware = authorizationMiddleware + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index e4e448954..31dd975b4 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -31,6 +31,7 @@ import { BlobDiffDto, EditedItemDto, } from "@root/types/dto/review" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { CreateCommentSchema, CreateReviewRequestSchema, @@ -66,6 +67,7 @@ export class ReviewsRouter { this.notificationsService = notificationsService this.githubService = githubService + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticated/sites.ts b/src/routes/v2/authenticated/sites.ts index 8aeafc5df..2a46eb1e2 100644 --- a/src/routes/v2/authenticated/sites.ts +++ b/src/routes/v2/authenticated/sites.ts @@ -21,6 +21,7 @@ import { RepositoryData } from "@root/types/repoInfo" import { BrokenLinkErrorDto } from "@root/types/siteChecker" import { SiteInfo, SiteLaunchDto } from "@root/types/siteInfo" import { StagingBuildStatus } from "@root/types/stagingBuildStatus" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { GetPreviewInfoSchema, LaunchSiteSchema, @@ -60,6 +61,7 @@ export class SitesRouter { this.infraService = infraService this.repoCheckerService = repoCheckerService // We need to bind all methods because we don't invoke them from the class directly + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticated/users.ts b/src/routes/v2/authenticated/users.ts index a77974edb..e8d6c956e 100644 --- a/src/routes/v2/authenticated/users.ts +++ b/src/routes/v2/authenticated/users.ts @@ -13,6 +13,7 @@ import UserSessionData from "@classes/UserSessionData" import DatabaseError from "@root/errors/DatabaseError" import { isError, RequestHandler } from "@root/types" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { VerifyEmailOtpSchema, VerifyMobileNumberOtpSchema, @@ -29,6 +30,7 @@ export class UsersRouter { constructor({ usersService }: UsersRouterProps) { this.usersService = usersService + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticatedSites/media.ts b/src/routes/v2/authenticatedSites/media.ts index 5f20c61e1..12a0890d2 100644 --- a/src/routes/v2/authenticatedSites/media.ts +++ b/src/routes/v2/authenticatedSites/media.ts @@ -17,6 +17,7 @@ import type { MediaFileOutput, RequestHandler, } from "@root/types" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { CreateMediaDirectoryRequestSchema, CreateMediaFileRequestSchema, @@ -44,6 +45,7 @@ export class MediaRouter { this.mediaFileService = mediaFileService this.mediaDirectoryService = mediaDirectoryService // We need to bind all methods because we don't invoke them from the class directly + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/authenticatedSites/repoManagement.ts b/src/routes/v2/authenticatedSites/repoManagement.ts index 294fb5a85..c17e82e0b 100644 --- a/src/routes/v2/authenticatedSites/repoManagement.ts +++ b/src/routes/v2/authenticatedSites/repoManagement.ts @@ -11,6 +11,7 @@ import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import GitFileSystemError from "@root/errors/GitFileSystemError" import { attachSiteHandler } from "@root/middleware" import type { RequestHandler } from "@root/types" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { ResetRepoSchema } from "@root/validators/RequestSchema" import RepoManagementService from "@services/admin/RepoManagementService" @@ -30,6 +31,7 @@ export class RepoManagementRouter { }: RepoManagementRouterProps) { this.repoManagementService = repoManagementService this.authorizationMiddleware = authorizationMiddleware + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/routes/v2/sgidAuth.ts b/src/routes/v2/sgidAuth.ts index a1766794b..620121032 100644 --- a/src/routes/v2/sgidAuth.ts +++ b/src/routes/v2/sgidAuth.ts @@ -19,6 +19,7 @@ import { import { RequestHandler } from "@root/types" import { ResponseErrorBody } from "@root/types/dto/error" import { isPublicOfficerData, PublicOfficerData } from "@root/types/sgid" +import { nameAnonymousMethods } from "@root/utils/apm-utils" import { isSecure } from "@root/utils/auth-utils" import { EmailSchema } from "@root/validators/RequestSchema" import UsersService from "@services/identity/UsersService" @@ -42,6 +43,7 @@ export class SgidAuthRouter { constructor({ usersService, sgidAuthService }: SgidAuthRouterProps) { this.usersService = usersService this.sgidAuthService = sgidAuthService + nameAnonymousMethods(this) autoBind(this) } diff --git a/src/utils/apm-utils.ts b/src/utils/apm-utils.ts new file mode 100644 index 000000000..f26fb8a25 --- /dev/null +++ b/src/utils/apm-utils.ts @@ -0,0 +1,25 @@ +/* eslint-disable import/prefer-default-export */ +/** + * A function to set the name of anonymous methods on an object, all the way into the prototype chain + * This is useful to be reported in APM traces and spans + * This is an unconventional thing to do, so we have to disable a couple of eslint rules + */ +export const nameAnonymousMethods = ( + self: SelfType +): SelfType => { + /* eslint-disable no-restricted-syntax */ + /* eslint-disable no-continue */ + for (const methodName in self) { + if (methodName === "constructor") continue + + const method = self[methodName] + if (typeof method !== "function") continue + if (method.name) continue + + Object.defineProperty(method, "name", { + value: methodName, + writable: false, + }) + } + return self +} From 3043f0fbb9ab8e4228833d78003789e2d0917f92 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 4 Apr 2024 13:30:24 +0800 Subject: [PATCH 5/6] fix: external links in top level nav (#1272) --- src/validators/RequestSchema.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/validators/RequestSchema.js b/src/validators/RequestSchema.js index 2e5476049..9040f2503 100644 --- a/src/validators/RequestSchema.js +++ b/src/validators/RequestSchema.js @@ -382,6 +382,7 @@ const UpdateNavigationRequestSchema = Joi.object().keys({ }) ), false_collection: Joi.boolean(), + external: Joi.boolean(), }) .oxor("url", "collection", "resource_room") .oxor("sublinks", "collection") From ef627d5a2383033cd903d4bf6ebc7beaa6dd23f7 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 4 Apr 2024 13:32:34 +0800 Subject: [PATCH 6/6] chore: bump version to v0.76.0 --- CHANGELOG.md | 12 ++++++++++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 768b4dcdd..2c2ab26cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,23 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.76.0](https://github.com/isomerpages/isomercms-backend/compare/v0.75.0...v0.76.0) + +- fix: external links in top level nav [`#1272`](https://github.com/isomerpages/isomercms-backend/pull/1272) +- Improve APM spans (no more <anonymous>) [`#1267`](https://github.com/isomerpages/isomercms-backend/pull/1267) +- fix: remove unecessary join and site retrieval [`#1268`](https://github.com/isomerpages/isomercms-backend/pull/1268) +- fix(dig): dig not working [`#1246`](https://github.com/isomerpages/isomercms-backend/pull/1246) +- fix(server): server should die if unable to connect to db [`#1265`](https://github.com/isomerpages/isomercms-backend/pull/1265) +- backport v0.75.0 [`#1264`](https://github.com/isomerpages/isomercms-backend/pull/1264) + #### [v0.75.0](https://github.com/isomerpages/isomercms-backend/compare/v0.74.0...v0.75.0) +> 2 April 2024 + - feat: carry the name of the wrapped handler [`#1260`](https://github.com/isomerpages/isomercms-backend/pull/1260) - fix: only release the lock after the handler is done [`#1259`](https://github.com/isomerpages/isomercms-backend/pull/1259) - backport v0.74.0 [`#1258`](https://github.com/isomerpages/isomercms-backend/pull/1258) +- chore: bump version to v0.75.0 [`b138978`](https://github.com/isomerpages/isomercms-backend/commit/b138978086136e15f42d7fc9484118ea835b42cc) #### [v0.74.0](https://github.com/isomerpages/isomercms-backend/compare/v0.73.0...v0.74.0) diff --git a/package-lock.json b/package-lock.json index cd3b5f831..4785e50ae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "isomercms", - "version": "0.75.0", + "version": "0.76.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "isomercms", - "version": "0.75.0", + "version": "0.76.0", "hasInstallScript": true, "dependencies": { "@aws-sdk/client-amplify": "^3.521.0", diff --git a/package.json b/package.json index b9a723bbb..8fc5b966c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isomercms", - "version": "0.75.0", + "version": "0.76.0", "private": true, "scripts": { "build": "tsc -p tsconfig.build.json",