From 7dd2f5f5c0144db9f3f02856ee37c2a314ef206e Mon Sep 17 00:00:00 2001 From: Michael Levin Date: Fri, 13 Sep 2024 10:26:24 -0400 Subject: [PATCH 1/4] [Security] NPM audit fixes --- package-lock.json | 97 ++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 43 deletions(-) diff --git a/package-lock.json b/package-lock.json index 67d5a02..801ce06 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1518,9 +1518,9 @@ } }, "node_modules/body-parser": { - "version": "1.20.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.2.tgz", - "integrity": "sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA==", + "version": "1.20.3", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.3.tgz", + "integrity": "sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g==", "dependencies": { "bytes": "3.1.2", "content-type": "~1.0.5", @@ -1530,7 +1530,7 @@ "http-errors": "2.0.0", "iconv-lite": "0.4.24", "on-finished": "2.4.1", - "qs": "6.11.0", + "qs": "6.13.0", "raw-body": "2.5.2", "type-is": "~1.6.18", "unpipe": "1.0.0" @@ -2310,9 +2310,9 @@ "integrity": "sha512-AKrN98kuwOzMIdAizXGI86UFBoo26CL21UM763y1h/GMSJ4/OHU9k2YlsmBpyScFo/wbLzWQJBMCW4+IO3/+OQ==" }, "node_modules/encodeurl": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-1.0.2.tgz", - "integrity": "sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w==", + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-2.0.0.tgz", + "integrity": "sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg==", "engines": { "node": ">= 0.8" } @@ -2719,36 +2719,36 @@ "optional": true }, "node_modules/express": { - "version": "4.19.2", - "resolved": "https://registry.npmjs.org/express/-/express-4.19.2.tgz", - "integrity": "sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q==", + "version": "4.21.0", + "resolved": "https://registry.npmjs.org/express/-/express-4.21.0.tgz", + "integrity": "sha512-VqcNGcj/Id5ZT1LZ/cfihi3ttTn+NJmkli2eZADigjq29qTlWi/hAQ43t/VLPq8+UX06FCEx3ByOYet6ZFblng==", "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", - "body-parser": "1.20.2", + "body-parser": "1.20.3", "content-disposition": "0.5.4", "content-type": "~1.0.4", "cookie": "0.6.0", "cookie-signature": "1.0.6", "debug": "2.6.9", "depd": "2.0.0", - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "etag": "~1.8.1", - "finalhandler": "1.2.0", + "finalhandler": "1.3.1", "fresh": "0.5.2", "http-errors": "2.0.0", - "merge-descriptors": "1.0.1", + "merge-descriptors": "1.0.3", "methods": "~1.1.2", "on-finished": "2.4.1", "parseurl": "~1.3.3", - "path-to-regexp": "0.1.7", + "path-to-regexp": "0.1.10", "proxy-addr": "~2.0.7", - "qs": "6.11.0", + "qs": "6.13.0", "range-parser": "~1.2.1", "safe-buffer": "5.2.1", - "send": "0.18.0", - "serve-static": "1.15.0", + "send": "0.19.0", + "serve-static": "1.16.2", "setprototypeof": "1.2.0", "statuses": "2.0.1", "type-is": "~1.6.18", @@ -2956,12 +2956,12 @@ } }, "node_modules/finalhandler": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.2.0.tgz", - "integrity": "sha512-5uXcUVftlQMFnWC9qu/svkWv3GTd2PfUhK/3PLkYNAe7FbqJMt3515HaxE6eRL74GdsriiwujiawdaB1BpEISg==", + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", + "integrity": "sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ==", "dependencies": { "debug": "2.6.9", - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "on-finished": "2.4.1", "parseurl": "~1.3.3", @@ -4458,9 +4458,12 @@ } }, "node_modules/merge-descriptors": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", - "integrity": "sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w==" + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.3.tgz", + "integrity": "sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ==", + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } }, "node_modules/methods": { "version": "1.1.2", @@ -4842,9 +4845,9 @@ } }, "node_modules/nise/node_modules/path-to-regexp": { - "version": "6.2.2", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.2.2.tgz", - "integrity": "sha512-GQX3SSMokngb36+whdpRXE+3f9V8UzyAorlYvOGx87ufGHehNTn5lCxrKtLyZ4Yl/wEKnNnr98ZzOwwDZV5ogw==", + "version": "6.3.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-6.3.0.tgz", + "integrity": "sha512-Yhpw4T9C6hPpgPeA28us07OJeqZ5EzQTkbfwuhsUg0c237RomFoETJgmp2sa3F/41gfLE6G5cqcYwznmeEeOlQ==", "dev": true }, "node_modules/node-abi": { @@ -5513,9 +5516,9 @@ } }, "node_modules/path-to-regexp": { - "version": "0.1.7", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.7.tgz", - "integrity": "sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ==" + "version": "0.1.10", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.10.tgz", + "integrity": "sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w==" }, "node_modules/pathval": { "version": "1.1.1", @@ -5923,11 +5926,11 @@ } }, "node_modules/qs": { - "version": "6.11.0", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.11.0.tgz", - "integrity": "sha512-MvjoMCJwEarSbUYk5O+nmoSzSutSsTwF85zcHPQ9OrlFoZOYIjaqBAJIqIXjptyD5vThxGq52Xu/MaJzRkIk4Q==", + "version": "6.13.0", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.13.0.tgz", + "integrity": "sha512-+38qI9SOr8tfZ4QmJNplMUxqjbe7LKvvZgWdExBOmd+egZTtjLB67Gu0HRX3u/XOq7UU2Nx6nsjvS16Z9uwfpg==", "dependencies": { - "side-channel": "^1.0.4" + "side-channel": "^1.0.6" }, "engines": { "node": ">=0.6" @@ -6217,9 +6220,9 @@ } }, "node_modules/send": { - "version": "0.18.0", - "resolved": "https://registry.npmjs.org/send/-/send-0.18.0.tgz", - "integrity": "sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg==", + "version": "0.19.0", + "resolved": "https://registry.npmjs.org/send/-/send-0.19.0.tgz", + "integrity": "sha512-dW41u5VfLXu8SJh5bwRmyYUbAoSB3c9uQh6L8h/KtsFREPWpbX1lrljJo186Jc4nmci/sGUZ9a0a0J2zgfq2hw==", "dependencies": { "debug": "2.6.9", "depd": "2.0.0", @@ -6252,6 +6255,14 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==" }, + "node_modules/send/node_modules/encodeurl": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-1.0.2.tgz", + "integrity": "sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w==", + "engines": { + "node": ">= 0.8" + } + }, "node_modules/send/node_modules/ms": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", @@ -6267,14 +6278,14 @@ } }, "node_modules/serve-static": { - "version": "1.15.0", - "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.15.0.tgz", - "integrity": "sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g==", + "version": "1.16.2", + "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.16.2.tgz", + "integrity": "sha512-VqpjJZKadQB/PEbEwvFdO43Ax5dFBZ2UECszz8bQ7pi7wt//PWe1P6MN7eCnjsatYtBT6EuiClbjSWP2WrIoTw==", "dependencies": { - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "parseurl": "~1.3.3", - "send": "0.18.0" + "send": "0.19.0" }, "engines": { "node": ">= 0.8.0" From 0436acbbcf7f19727dff0b789e51bb9d7040f034 Mon Sep 17 00:00:00 2001 From: Michael Levin Date: Wed, 6 Nov 2024 10:29:35 -0500 Subject: [PATCH 2/4] [Tech Debt] Update supertest, use async/await in tests --- package-lock.json | 43 +++++++------- package.json | 2 +- test/app.test.js | 143 ++++++++++++++++++++-------------------------- test/db.test.js | 139 ++++++++++++++------------------------------ 4 files changed, 126 insertions(+), 201 deletions(-) diff --git a/package-lock.json b/package-lock.json index 801ce06..0fa26a0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,7 +31,7 @@ "nyc": "^15.1.0", "proxyquire": "^2.1.3", "sinon": "^17.0.1", - "supertest": "^6.3.3" + "supertest": "^7.0.0" }, "engines": { "node": "20.x.x" @@ -3106,15 +3106,14 @@ } }, "node_modules/formidable": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/formidable/-/formidable-2.1.2.tgz", - "integrity": "sha512-CM3GuJ57US06mlpQ47YcunuUZ9jpm8Vx+P2CGt2j7HpgkKZO/DJYQ0Bobim8G6PFQmK5lOqOOdUXboU+h73A4g==", + "version": "3.5.2", + "resolved": "https://registry.npmjs.org/formidable/-/formidable-3.5.2.tgz", + "integrity": "sha512-Jqc1btCy3QzRbJaICGwKcBfGWuLADRerLzDqi2NwSt/UkXLsHJw2TVResiaoBufHVHy9aSgClOHCeJsSsFLTbg==", "dev": true, "dependencies": { "dezalgo": "^1.0.4", - "hexoid": "^1.0.0", - "once": "^1.4.0", - "qs": "^6.11.0" + "hexoid": "^2.0.0", + "once": "^1.4.0" }, "funding": { "url": "https://ko-fi.com/tunnckoCore/commissions" @@ -3460,9 +3459,9 @@ } }, "node_modules/hexoid": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/hexoid/-/hexoid-1.0.0.tgz", - "integrity": "sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==", + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/hexoid/-/hexoid-2.0.0.tgz", + "integrity": "sha512-qlspKUK7IlSQv2o+5I7yhUd7TxlOG2Vr5LTa3ve2XSNVKAL/n/u/7KLvKmFNimomDIKvZFXWHv0T12mv7rT8Aw==", "dev": true, "engines": { "node": ">=8" @@ -6638,10 +6637,9 @@ } }, "node_modules/superagent": { - "version": "8.1.2", - "resolved": "https://registry.npmjs.org/superagent/-/superagent-8.1.2.tgz", - "integrity": "sha512-6WTxW1EB6yCxV5VFOIPQruWGHqc3yI7hEmZK6h+pyk69Lk/Ut7rLUY6W/ONF2MjBuGjvmMiIpsrVJ2vjrHlslA==", - "deprecated": "Please upgrade to v9.0.0+ as we have fixed a public vulnerability with formidable dependency. Note that v9.0.0+ requires Node.js v14.18.0+. See https://github.com/ladjs/superagent/pull/1800 for insight. This project is supported and maintained by the team at Forward Email @ https://forwardemail.net", + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/superagent/-/superagent-9.0.2.tgz", + "integrity": "sha512-xuW7dzkUpcJq7QnhOsnNUgtYp3xRwpt2F7abdRYIpCsAt0hhUqia0EdxyXZQQpNmGtsCzYHryaKSV3q3GJnq7w==", "dev": true, "dependencies": { "component-emitter": "^1.3.0", @@ -6649,14 +6647,13 @@ "debug": "^4.3.4", "fast-safe-stringify": "^2.1.1", "form-data": "^4.0.0", - "formidable": "^2.1.2", + "formidable": "^3.5.1", "methods": "^1.1.2", "mime": "2.6.0", - "qs": "^6.11.0", - "semver": "^7.3.8" + "qs": "^6.11.0" }, "engines": { - "node": ">=6.4.0 <13 || >=14" + "node": ">=14.18.0" } }, "node_modules/superagent/node_modules/mime": { @@ -6672,16 +6669,16 @@ } }, "node_modules/supertest": { - "version": "6.3.4", - "resolved": "https://registry.npmjs.org/supertest/-/supertest-6.3.4.tgz", - "integrity": "sha512-erY3HFDG0dPnhw4U+udPfrzXa4xhSG+n4rxfRuZWCUvjFWwKl+OxWf/7zk50s84/fAAs7vf5QAb9uRa0cCykxw==", + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/supertest/-/supertest-7.0.0.tgz", + "integrity": "sha512-qlsr7fIC0lSddmA3tzojvzubYxvlGtzumcdHgPwbFWMISQwL22MhM2Y3LNt+6w9Yyx7559VW5ab70dgphm8qQA==", "dev": true, "dependencies": { "methods": "^1.1.2", - "superagent": "^8.1.2" + "superagent": "^9.0.1" }, "engines": { - "node": ">=6.4.0" + "node": ">=14.18.0" } }, "node_modules/supports-color": { diff --git a/package.json b/package.json index c45084f..1888ccb 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "nyc": "^15.1.0", "proxyquire": "^2.1.3", "sinon": "^17.0.1", - "supertest": "^6.3.3" + "supertest": "^7.0.0" }, "dependencies": { "express": "^4.18.2", diff --git a/test/app.test.js b/test/app.test.js index 9ba28c5..d8d82bb 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -31,7 +31,7 @@ routes.forEach((route) => { db.query = () => Promise.resolve(); }); - it("should pass params from the url to db.query and render the result", (done) => { + it("should pass params from the url to db.query and render the result", async () => { db.query = (params) => { expect(params.reportAgency).to.equal("fake-agency"); expect(params.reportName).to.equal("fake-report"); @@ -46,19 +46,16 @@ routes.forEach((route) => { .get(`/${route}/agencies/fake-agency/reports/fake-report/data`) .expect(200); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfRouteNotice(route, [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ]); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); - it("should not pass the agency param if the request does not specify an agency", (done) => { + it("should not pass the agency param if the request does not specify an agency", async () => { db.query = (params) => { expect(params.reportAgency).to.be.undefined; expect(params.reportName).to.equal("fake-report"); @@ -73,19 +70,16 @@ routes.forEach((route) => { .get(`/${route}/reports/fake-report/data`) .expect(200); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfRouteNotice(route, [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ]); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); - it("should merge the params in the url with query params", (done) => { + it("should merge the params in the url with query params", async () => { db.query = (params) => { expect(params.reportAgency).to.equal("fake-agency"); expect(params.reportName).to.equal("fake-report"); @@ -101,19 +95,16 @@ routes.forEach((route) => { .get(`/${route}/agencies/fake-agency/reports/fake-report/data?limit=50`) .expect(200); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfRouteNotice(route, [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ]); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); - it("should respond with a 400 if db.query rejects", (done) => { + it("should respond with a 400 if db.query rejects", async () => { db.query = () => Promise.reject("This is a test of the emergency broadcast system."); @@ -121,19 +112,16 @@ routes.forEach((route) => { .get(`/${route}/agencies/fake-agency/reports/fake-report/data`) .expect(400); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = { - message: "An error occurred. Please check the application logs.", - status: 400, - }; - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = { + message: "An error occurred. Please check the application logs.", + status: 400, + }; + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); - it("should respond with a 400 if the domain report is not one of the acceptable kinds of reports", (done) => { + it("should respond with a 400 if the domain report is not one of the acceptable kinds of reports", async () => { db.query = (params) => { expect(params.domain).to.equal("fakeiscool.gov"); expect(params.reportName).to.equal("browser"); @@ -155,20 +143,17 @@ routes.forEach((route) => { .get(`/${route}/domain/fakeiscool.gov/reports/browser/data`) .expect(400); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = { - message: - "You are requesting a report that cannot be filtered on domain. Please try one of the following reports: site, domain, download, second-level-domain.", - status: 400, - }; - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "You are requesting a report that cannot be filtered on domain. Please try one of the following reports: site, domain, download, second-level-domain.", + status: 400, + }; + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); - it("should pass params from the url to db.query and render the result for a domain query for a non-download report", (done) => { + it("should pass params from the url to db.query and render the result for a domain query for a non-download report", async () => { db.query = (params) => { expect(params.domain).to.equal("fakeiscool.gov"); expect(params.reportName).to.equal("site"); @@ -187,20 +172,17 @@ routes.forEach((route) => { .get(`/${route}/domain/fakeiscool.gov/reports/site/data`) .expect(200); - dataRequest - .then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { - id: 1, - date: "2017-01-01", - report_name: "site", - domain: "fakeiscool.gov", - }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfRouteNotice(route, [ + { + id: 1, + date: "2017-01-01", + report_name: "site", + domain: "fakeiscool.gov", + }, + ]); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); }); }); }); @@ -210,7 +192,7 @@ describe(`app with unspupported version`, () => { db.query = () => Promise.resolve(); }); - it("should not accept unsupported versions", (done) => { + it("should not accept unsupported versions", async () => { const unsupportedVersion = "v2.x"; db.query = (params) => { @@ -232,15 +214,12 @@ describe(`app with unspupported version`, () => { ) .expect(404); - dataRequest - .then((actualResponse) => { - const expectedResponse = { - _body: expectedErrorMessage, - status: 404, - }; - expect(actualResponse).to.include(expectedResponse); - done(); - }) - .catch(done); + await dataRequest.then((actualResponse) => { + const expectedResponse = { + _body: expectedErrorMessage, + status: 404, + }; + expect(actualResponse).to.include(expectedResponse); + }); }); }); diff --git a/test/db.test.js b/test/db.test.js index 91b08b2..e475c6b 100644 --- a/test/db.test.js +++ b/test/db.test.js @@ -9,19 +9,16 @@ const db = proxyquire("../src/db", { describe("db", () => { const apiVersions = ["v1.1", "v2"]; - before((done) => { + before(async () => { // Setup the test database client - database.createClient().then(() => done()); + await database.createClient(); }); - after((done) => { + after(async () => { // Clean up the test database client and the application database client - database - .destroyClient() - .then(() => { - return db.dbClient.destroy(); - }) - .then(() => done()); + await database.destroyClient().then(() => { + return db.dbClient.destroy(); + }); }); apiVersions.forEach((apiVersion) => { @@ -30,13 +27,13 @@ describe("db", () => { apiVersion === "v1.1" ? "analytics_data" : "analytics_data_ga4"; const queryVersion = apiVersion === `v1.1` ? "1.1" : "2"; - beforeEach((done) => { - database.resetSchema(table).then(() => done()); + beforeEach(async () => { + await database.resetSchema(table); }); describe(".query(params)", () => { - it("should return all rows for the given agency and report", (done) => { - database + it("should return all rows for the given agency and report", async () => { + await database .client(table) .insert([ { report_name: "my-report", report_agency: "my-agency" }, @@ -55,13 +52,11 @@ describe("db", () => { expect(results).to.have.length(1); expect(results[0].report_name).to.equal("my-report"); expect(results[0].report_agency).to.equal("my-agency"); - done(); - }) - .catch(done); + }); }); - it("should return all rows without an agency if no agency name is given", (done) => { - database + it("should return all rows without an agency if no agency name is given", async () => { + await database .client(table) .insert([ { report_name: "my-report", report_agency: "not-my-agency" }, @@ -77,13 +72,11 @@ describe("db", () => { expect(results).to.have.length(1); expect(results[0].report_name).to.equal("my-report"); expect(results[0].report_agency).to.be.null; - done(); - }) - .catch(done); + }); }); - it("should sort the rows according to the date column", (done) => { - database + it("should sort the rows according to the date column", async () => { + await database .client(table) .insert([ { report_name: "report", date: "2017-01-02" }, @@ -100,18 +93,16 @@ describe("db", () => { const expectedDate = `2017-01-0${3 - index}`; expect(resultDate).to.equal(expectedDate); }); - done(); - }) - .catch(done); + }); }); - it("should limit the rows according to the limit param", (done) => { + it("should limit the rows according to the limit param", async () => { const rows = Array(5) .fill(0) .map(() => { return { report_name: "report", date: "2017-01-01" }; }); - database + await database .client(table) .insert(rows) .then(() => { @@ -123,18 +114,16 @@ describe("db", () => { }) .then((results) => { expect(results).to.have.length(4); - done(); - }) - .catch(done); + }); }); - it("should default to a limit of 1000", (done) => { + it("should default to a limit of 1000", async () => { const rows = Array(1001) .fill(0) .map(() => { return { report_name: "report", date: "2017-01-01" }; }); - database + await database .client(table) .insert(rows) .then(() => { @@ -142,18 +131,16 @@ describe("db", () => { }) .then((results) => { expect(results).to.have.length(1000); - done(); - }) - .catch(done); + }); }); - it("should have a maximum limit of 10,000", (done) => { + it("should have a maximum limit of 10,000", async () => { const rows = Array(11000) .fill(0) .map(() => { return { report_name: "report", date: "2017-01-01" }; }); - database + await database .client(table) .insert(rows) .then(() => { @@ -165,20 +152,16 @@ describe("db", () => { }) .then((results) => { expect(results).to.have.length(10000); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should paginate on the page param", (done) => { + it("should paginate on the page param", async () => { const rows = Array(6) .fill(0) .map((val, index) => { return { report_name: "report", date: `2017-01-0${index + 1}` }; }); - database + await database .client(table) .insert(rows) .then(() => { @@ -205,9 +188,7 @@ describe("db", () => { expect(results).to.have.length(3); expect(results[0].date.toISOString()).to.match(/^2017-01-03/); expect(results[2].date.toISOString()).to.match(/^2017-01-01/); - done(); - }) - .catch(done); + }); }); }); @@ -237,8 +218,8 @@ describe("db", () => { }); describe(".queryDomain(params)", () => { - it("should only return 2 results that include site reports from the test.gov domain", (done) => { - database + it("should only return 2 results that include site reports from the test.gov domain", async () => { + await database .client(table) .insert([ { @@ -268,15 +249,11 @@ describe("db", () => { }) .then((results) => { expect(results).to.have.length(2); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 2 results that include site reports from the test.gov domain, when multiple reports", (done) => { - database + it("should only return 2 results that include site reports from the test.gov domain, when multiple reports", async () => { + await database .client(table) .insert([ { @@ -308,15 +285,11 @@ describe("db", () => { expect(results).to.have.length(2); expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 2 results that include site reports from the test.gov domain, when multiple domains", (done) => { - database + it("should only return 2 results that include site reports from the test.gov domain, when multiple domains", async () => { + await database .client(table) .insert([ { @@ -348,14 +321,10 @@ describe("db", () => { expect(results).to.have.length(2); expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 4 results that include download reports from the test.gov domain, when multiple domains", (done) => { + it("should only return 4 results that include download reports from the test.gov domain, when multiple domains", async () => { const testData = [ { report_name: "download", @@ -383,7 +352,7 @@ describe("db", () => { data: { page: "usda.gov" }, }, ]; - database + await database .client(table) .insert(testData) .then(() => { @@ -405,15 +374,11 @@ describe("db", () => { testData[index].data.page, ); }); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 2 results that include site reports from the test.gov domain, when before date parameters are in", (done) => { - database + it("should only return 2 results that include site reports from the test.gov domain, when before date parameters are in", async () => { + await database .client(table) .insert([ { @@ -452,15 +417,11 @@ describe("db", () => { expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); expect(results[0].date.toISOString()).to.match(/^2017-01-02/); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 1 result that include site reports from the test.gov domain, when after date parameters are in", (done) => { - database + it("should only return 1 result that include site reports from the test.gov domain, when after date parameters are in", async () => { + await database .client(table) .insert([ { @@ -499,15 +460,11 @@ describe("db", () => { expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); expect(results[0].date.toISOString()).to.match(/^2018-01-03/); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 2 result that include site reports from the test.gov domain, when after/before date parameters set", (done) => { - database + it("should only return 2 result that include site reports from the test.gov domain, when after/before date parameters set", async () => { + await database .client(table) .insert([ { @@ -557,15 +514,11 @@ describe("db", () => { expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); expect(results[0].date.toISOString()).to.match(/^2017-11-04/); - done(); - }) - .catch((err) => { - done(err); }); }); - it("should only return 2 result that include site reports from the test.gov domain, when after/before date parameters set", (done) => { - database + it("should only return 2 result that include site reports from the test.gov domain, when after/before date parameters set", async () => { + await database .client(table) .insert([ { @@ -615,10 +568,6 @@ describe("db", () => { expect(results[0].report_name).to.equal("site"); expect(results[0].data.domain).to.equal("test.gov"); expect(results[0].date.toISOString()).to.match(/^2018-01-03/); - done(); - }) - .catch((err) => { - done(err); }); }); }); From 108c04881e2774da0b1078848a23dbf2c913c4cc Mon Sep 17 00:00:00 2001 From: Michael Levin Date: Fri, 8 Nov 2024 09:49:44 -0500 Subject: [PATCH 3/4] [Tech Debt] Add request param validation --- package-lock.json | 40 ++- package.json | 3 +- src/app.js | 51 +++- test/app.test.js | 714 ++++++++++++++++++++++++++++++++++------------ 4 files changed, 619 insertions(+), 189 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0fa26a0..8e33a99 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,8 @@ "knex": "^3.0.1", "lodash": ">= 4.17.21", "pg": "^8.11.3", - "winston": "^3.11.0" + "winston": "^3.11.0", + "yup": "^1.4.0" }, "devDependencies": { "@eslint/js": "^8.57.0", @@ -5846,6 +5847,11 @@ "node": ">=10" } }, + "node_modules/property-expr": { + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/property-expr/-/property-expr-2.0.6.tgz", + "integrity": "sha512-SVtmxhRE/CGkn3eZY1T6pC8Nln6Fr/lu1mKSgRud0eC73whjGfoAogbn78LkD8aFL0zz3bAFerKSnOl7NlErBA==" + }, "node_modules/protobufjs": { "version": "7.3.2", "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.3.2.tgz", @@ -6976,6 +6982,11 @@ "node": ">=8" } }, + "node_modules/tiny-case": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/tiny-case/-/tiny-case-1.0.3.tgz", + "integrity": "sha512-Eet/eeMhkO6TX8mnUteS9zgPbUMQa4I6Kkp5ORiBD5476/m+PIRiumP5tmh5ioJpH7k51Kehawy2UDfsnxxY8Q==" + }, "node_modules/to-fast-properties": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/to-fast-properties/-/to-fast-properties-2.0.0.tgz", @@ -7005,6 +7016,11 @@ "node": ">=0.6" } }, + "node_modules/toposort": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/toposort/-/toposort-2.0.2.tgz", + "integrity": "sha512-0a5EOkAUp8D4moMi2W8ZF8jcga7BgZd91O/yabJCFY8az+XSzeGyTKs0Aoo897iV1Nj6guFq8orWDS96z91oGg==" + }, "node_modules/touch": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/touch/-/touch-3.1.1.tgz", @@ -7476,6 +7492,28 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/yup": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/yup/-/yup-1.4.0.tgz", + "integrity": "sha512-wPbgkJRCqIf+OHyiTBQoJiP5PFuAXaWiJK6AmYkzQAh5/c2K9hzSApBZG5wV9KoKSePF7sAxmNSvh/13YHkFDg==", + "dependencies": { + "property-expr": "^2.0.5", + "tiny-case": "^1.0.3", + "toposort": "^2.0.2", + "type-fest": "^2.19.0" + } + }, + "node_modules/yup/node_modules/type-fest": { + "version": "2.19.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-2.19.0.tgz", + "integrity": "sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA==", + "engines": { + "node": ">=12.20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } } } } diff --git a/package.json b/package.json index 1888ccb..2c7f4df 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,8 @@ "knex": "^3.0.1", "lodash": ">= 4.17.21", "pg": "^8.11.3", - "winston": "^3.11.0" + "winston": "^3.11.0", + "yup": "^1.4.0" }, "optionalDependencies": { "newrelic": "^11.3.0" diff --git a/src/app.js b/src/app.js index 0879b3a..3912f8b 100644 --- a/src/app.js +++ b/src/app.js @@ -4,6 +4,7 @@ const db = require("./db"); const logger = require("./logger"); const router = express.Router(); const routesVersioning = require("express-routes-versioning")(); +const yup = require("yup"); const app = express(); @@ -15,6 +16,10 @@ app.use(apiDataGovFilter); app.use(router); app.use(logger.errorLoggingMiddleware()); +/** + * Converts date object to an ISO date string without time and zone. + * @param dataPoint + */ const formatDateForDataPoint = (dataPoint) => { if (dataPoint.date) { return dataPoint.date.toISOString().slice(0, 10); @@ -29,6 +34,17 @@ const acceptableDomainReports = [ "second-level-domain", ]; +/** + * Currently the only regex match in request validation is on date string + * formatting. Hard code that the yup.string().matches() validation returns a + * helpful error message when dates are not formatted correctly. + */ +yup.setLocale({ + string: { + matches: "must be a date in format 'YYYY-MM-DD'", + }, +}); + const checkDomainFilter = (req, res) => { if ( acceptableDomainReports.includes(req.params.reportName) && @@ -45,8 +61,18 @@ const checkDomainFilter = (req, res) => { }; const fetchData = (req, res) => { + try { + validateRequest(req); + } catch (err) { + res.status(400); + return res.json({ + message: `Invalid request params: ${err}`, + status: 400, + }); + } const params = Object.assign(req.query, req.params); - db.query(params) + return db + .query(params) .then((result) => { const response = result.map((dataPoint) => Object.assign( @@ -68,14 +94,33 @@ const fetchData = (req, res) => { }) .catch((err) => { console.error("Unexpected Error:", err); - res.status(400); + res.status(500); return res.json({ message: "An error occurred. Please check the application logs.", - status: 400, + status: 500, }); }); }; +const validateRequest = (req) => { + const isoDateRegex = /^\d{4}-([0][1-9]|1[0-2])-([0][1-9]|[1-2]\d|3[01])$/; + const requestSchema = yup.object({ + query: yup.object({ + before: yup.string().matches(isoDateRegex), + after: yup.string().matches(isoDateRegex), + limit: yup.number().positive().integer().max(10000), + page: yup.number().positive().integer(), + }), + params: yup.object({ + domain: yup.string(), + reportAgency: yup.string(), + reportName: yup.string(), + version: yup.string(), + }), + }); + return requestSchema.validateSync(req); +}; + app.get("/", (req, res) => { res.json({ current_time: new Date(), diff --git a/test/app.test.js b/test/app.test.js index d8d82bb..78f52dc 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -14,8 +14,8 @@ const app = proxyquire("../src/app", { "./db": db, }); -const handleIfRouteNotice = (route, arr) => { - if (route === "v1.1") { +const handleIfApiVersionNotice = (apiVersion, arr) => { + if (apiVersion === "v1.1") { return arr.map((object) => { return { ...object, notice: noticeValue }; }); @@ -23,203 +23,549 @@ const handleIfRouteNotice = (route, arr) => { return arr; }; -const routes = ["v1.1", "v2"]; +const apiVersions = ["v1.1", "v2"]; -routes.forEach((route) => { - describe(`app with ${route}`, () => { - beforeEach(() => { - db.query = () => Promise.resolve(); - }); +const invalidDates = [ + "2020-00-00", + "2024-14-01", + "2025-01-33", + "2020/01/02", + "20202-01-01", + "2020-010-01", + "2020-01-010", + "343542", + "junk", +]; - it("should pass params from the url to db.query and render the result", async () => { - db.query = (params) => { - expect(params.reportAgency).to.equal("fake-agency"); - expect(params.reportName).to.equal("fake-report"); - const arr = handleIfRouteNotice(route, [ - { id: 1, date: new Date("2017-01-01") }, - { id: 2, date: new Date("2017-01-02") }, - ]); - return Promise.resolve(arr); - }; - - const dataRequest = request(app) - .get(`/${route}/agencies/fake-agency/reports/fake-report/data`) - .expect(200); - - await dataRequest.then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - }); - }); +const invalidPositiveIntegers = [-1, 0, 33.33, "foobar", "foo4bar", "4foobar"]; - it("should not pass the agency param if the request does not specify an agency", async () => { - db.query = (params) => { - expect(params.reportAgency).to.be.undefined; - expect(params.reportName).to.equal("fake-report"); - const arr = handleIfRouteNotice(route, [ - { id: 1, date: new Date("2017-01-01") }, - { id: 2, date: new Date("2017-01-02") }, - ]); - return Promise.resolve(arr); - }; - - const dataRequest = request(app) - .get(`/${route}/reports/fake-report/data`) - .expect(200); - - await dataRequest.then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); - }); - }); +const invalidPageNumbers = [...invalidPositiveIntegers, 10001]; + +describe("app", () => { + let url; + + beforeEach(() => { + url = ""; + }); - it("should merge the params in the url with query params", async () => { - db.query = (params) => { - expect(params.reportAgency).to.equal("fake-agency"); - expect(params.reportName).to.equal("fake-report"); - expect(params.limit).to.equal("50"); - const arr = handleIfRouteNotice(route, [ - { id: 1, date: new Date("2017-01-01") }, - { id: 2, date: new Date("2017-01-02") }, - ]); - return Promise.resolve(arr); - }; - - const dataRequest = request(app) - .get(`/${route}/agencies/fake-agency/reports/fake-report/data?limit=50`) - .expect(200); - - await dataRequest.then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { id: 1, date: "2017-01-01" }, - { id: 2, date: "2017-01-02" }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); + apiVersions.forEach((apiVersion) => { + describe(`with api version: ${apiVersion}`, () => { + beforeEach(() => { + db.query = () => Promise.resolve(); }); - }); - it("should respond with a 400 if db.query rejects", async () => { - db.query = () => - Promise.reject("This is a test of the emergency broadcast system."); + describe("and with route: /reports/:reportName/data", () => { + beforeEach(() => { + url = `/${apiVersion}/reports/fake-report/data`; + }); + + describe("when params are valid", () => { + it("should not pass the agency param if the request does not specify an agency", async () => { + db.query = (params) => { + expect(params.reportAgency).to.be.undefined; + expect(params.reportName).to.equal("fake-report"); + const arr = handleIfApiVersionNotice(apiVersion, [ + { id: 1, date: new Date("2017-01-01") }, + { id: 2, date: new Date("2017-01-02") }, + ]); + return Promise.resolve(arr); + }; + + const dataRequest = request(app) + .get(`/${apiVersion}/reports/fake-report/data`) + .expect(200); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfApiVersionNotice( + apiVersion, + [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ], + ); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); + }); + }); + + describe("when params are invalid", () => { + describe("and the before param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?before=${invalidDate}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the after param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?after=${invalidDate}`) + .expect(400); - const dataRequest = request(app) - .get(`/${route}/agencies/fake-agency/reports/fake-report/data`) - .expect(400); + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); - await dataRequest.then((actualResponse) => { - const expectedResponseBody = { - message: "An error occurred. Please check the application logs.", - status: 400, - }; - expect(actualResponse.body).to.deep.equal(expectedResponseBody); + describe("and the page param is not a valid positive integer", () => { + invalidPositiveIntegers.forEach((invalidPositiveInteger) => { + describe(`and page is ${invalidPositiveInteger}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?page=${invalidPositiveInteger}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the limit param is not a valid positive integer with max 10000", () => { + invalidPageNumbers.forEach((invalidPageNumber) => { + describe(`and page is ${invalidPageNumber}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?limit=${invalidPageNumber}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + }); }); - }); - it("should respond with a 400 if the domain report is not one of the acceptable kinds of reports", async () => { - db.query = (params) => { - expect(params.domain).to.equal("fakeiscool.gov"); - expect(params.reportName).to.equal("browser"); - return Promise.resolve([ - { - id: 1, - date: new Date("2017-01-01"), - data: { domain: "fakeiscool.gov" }, - }, - { - id: 2, - date: new Date("2017-01-02"), - data: { domain: "bobtown.gov" }, - }, - ]); - }; - - const dataRequest = request(app) - .get(`/${route}/domain/fakeiscool.gov/reports/browser/data`) - .expect(400); - - await dataRequest.then((actualResponse) => { - const expectedResponseBody = { - message: - "You are requesting a report that cannot be filtered on domain. Please try one of the following reports: site, domain, download, second-level-domain.", - status: 400, - }; - expect(actualResponse.body).to.deep.equal(expectedResponseBody); + describe("and with route: /agencies/:agency/reports/:reportName/data", () => { + beforeEach(() => { + url = `/${apiVersion}/agencies/fake-agency/reports/fake-report/data`; + }); + + describe("and params are valid", () => { + it("should pass params from the url to db.query and render the result", async () => { + db.query = (params) => { + expect(params.reportAgency).to.equal("fake-agency"); + expect(params.reportName).to.equal("fake-report"); + const arr = handleIfApiVersionNotice(apiVersion, [ + { id: 1, date: new Date("2017-01-01") }, + { id: 2, date: new Date("2017-01-02") }, + ]); + return Promise.resolve(arr); + }; + + const dataRequest = request(app).get(url).expect(200); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfApiVersionNotice( + apiVersion, + [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ], + ); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); + }); + + it("should merge the params in the url with query params", async () => { + db.query = (params) => { + expect(params.reportAgency).to.equal("fake-agency"); + expect(params.reportName).to.equal("fake-report"); + expect(params.limit).to.equal("50"); + const arr = handleIfApiVersionNotice(apiVersion, [ + { id: 1, date: new Date("2017-01-01") }, + { id: 2, date: new Date("2017-01-02") }, + ]); + return Promise.resolve(arr); + }; + + const dataRequest = request(app).get(`${url}?limit=50`).expect(200); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfApiVersionNotice( + apiVersion, + [ + { id: 1, date: "2017-01-01" }, + { id: 2, date: "2017-01-02" }, + ], + ); + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); + }); + + it("should respond with a 500 if db.query rejects", async () => { + db.query = () => + Promise.reject( + "This is a test of the emergency broadcast system.", + ); + + const dataRequest = request(app) + .get( + `/${apiVersion}/agencies/fake-agency/reports/fake-report/data`, + ) + .expect(500); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "An error occurred. Please check the application logs.", + status: 500, + }; + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); + }); + }); + + describe("and params are invalid", () => { + describe("and the before param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?before=${invalidDate}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the after param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?after=${invalidDate}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the page param is not a valid positive integer", () => { + invalidPositiveIntegers.forEach((invalidPositiveInteger) => { + describe(`and page is ${invalidPositiveInteger}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?page=${invalidPositiveInteger}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the limit param is not a valid positive integer with max 10000", () => { + invalidPageNumbers.forEach((invalidPageNumber) => { + describe(`and page is ${invalidPageNumber}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?limit=${invalidPageNumber}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + }); }); - }); - it("should pass params from the url to db.query and render the result for a domain query for a non-download report", async () => { - db.query = (params) => { - expect(params.domain).to.equal("fakeiscool.gov"); - expect(params.reportName).to.equal("site"); - const arr = handleIfRouteNotice(route, [ - { - id: 1, - date: new Date("2017-01-01"), - report_name: "site", - data: { domain: "fakeiscool.gov" }, - }, - ]); - return Promise.resolve(arr); - }; - - const dataRequest = request(app) - .get(`/${route}/domain/fakeiscool.gov/reports/site/data`) - .expect(200); - - await dataRequest.then((actualResponse) => { - const expectedResponseBody = handleIfRouteNotice(route, [ - { - id: 1, - date: "2017-01-01", - report_name: "site", - domain: "fakeiscool.gov", - }, - ]); - expect(actualResponse.body).to.deep.equal(expectedResponseBody); + describe("and with route: /domain/:domain/reports/:reportName/data", () => { + const allowedDomainReports = [ + "site", + "domain", + "download", + "second-level-domain", + ]; + + beforeEach(() => { + url = `/${apiVersion}/domain/example.gov/reports/site/data`; + }); + + describe("and params are valid", () => { + allowedDomainReports.forEach((reportName) => { + describe(`and the report name is ${reportName}`, () => { + beforeEach(() => { + url = `/${apiVersion}/domain/example.gov/reports/${reportName}/data`; + + db.query = (params) => { + expect(params.domain).to.equal("example.gov"); + expect(params.reportName).to.equal(reportName); + const arr = handleIfApiVersionNotice(apiVersion, [ + { + id: 1, + date: new Date("2017-01-01"), + report_name: reportName, + data: { domain: "example.gov" }, + }, + ]); + return Promise.resolve(arr); + }; + }); + + it(`should pass params from the url to db.query and render the result`, async () => { + const dataRequest = request(app).get(url).expect(200); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = handleIfApiVersionNotice( + apiVersion, + [ + { + id: 1, + date: "2017-01-01", + report_name: reportName, + domain: "example.gov", + }, + ], + ); + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and params are invalid", () => { + it("should respond with a 400 if the domain report is not one of the acceptable kinds of reports", async () => { + db.query = (params) => { + expect(params.domain).to.equal("fakeiscool.gov"); + expect(params.reportName).to.equal("browser"); + return Promise.resolve([ + { + id: 1, + date: new Date("2017-01-01"), + data: { domain: "fakeiscool.gov" }, + }, + { + id: 2, + date: new Date("2017-01-02"), + data: { domain: "bobtown.gov" }, + }, + ]); + }; + + const dataRequest = request(app) + .get(`/${apiVersion}/domain/fakeiscool.gov/reports/browser/data`) + .expect(400); + + await dataRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "You are requesting a report that cannot be filtered on domain. Please try one of the following reports: site, domain, download, second-level-domain.", + status: 400, + }; + expect(actualResponse.body).to.deep.equal(expectedResponseBody); + }); + }); + + describe("and the before param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?before=${invalidDate}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the after param is not a valid date", () => { + invalidDates.forEach((invalidDate) => { + describe(`and date is ${invalidDate}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?after=${invalidDate}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + message: + "Invalid request params: ValidationError: must be a date in format 'YYYY-MM-DD'", + status: 400, + }; + expect(actualResponse.body).to.deep.equal( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the page param is not a valid positive integer", () => { + invalidPositiveIntegers.forEach((invalidPositiveInteger) => { + describe(`and page is ${invalidPositiveInteger}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?page=${invalidPositiveInteger}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + + describe("and the limit param is not a valid positive integer with max 10000", () => { + invalidPageNumbers.forEach((invalidPageNumber) => { + describe(`and page is ${invalidPageNumber}`, () => { + it("should respond with a 400", async () => { + const apiRequest = request(app) + .get(`${url}?limit=${invalidPageNumber}`) + .expect(400); + + await apiRequest.then((actualResponse) => { + const expectedResponseBody = { + status: 400, + }; + expect(actualResponse.body).to.deep.include( + expectedResponseBody, + ); + }); + }); + }); + }); + }); + }); }); - }); - }); -}); -describe(`app with unspupported version`, () => { - beforeEach(() => { - db.query = () => Promise.resolve(); - }); + describe(`with unsupported version`, () => { + beforeEach(() => { + db.query = () => Promise.resolve(); + }); + + it("should not accept unsupported versions", async () => { + const unsupportedVersion = "v2.x"; - it("should not accept unsupported versions", async () => { - const unsupportedVersion = "v2.x"; - - db.query = (params) => { - expect(params.reportAgency).to.equal("fake-agency"); - expect(params.reportName).to.equal("fake-report"); - const arr = handleIfRouteNotice(unsupportedVersion, [ - { id: 1, date: new Date("2017-01-01") }, - { id: 2, date: new Date("2017-01-02") }, - ]); - return Promise.resolve(arr); - }; - - const expectedErrorMessage = - "Version not found. Visit https://analytics.usa.gov/developer for information on the latest supported version."; - - const dataRequest = request(app) - .get( - `/${unsupportedVersion}/agencies/fake-agency/reports/fake-report/data`, - ) - .expect(404); - - await dataRequest.then((actualResponse) => { - const expectedResponse = { - _body: expectedErrorMessage, - status: 404, - }; - expect(actualResponse).to.include(expectedResponse); + db.query = (params) => { + expect(params.reportAgency).to.equal("fake-agency"); + expect(params.reportName).to.equal("fake-report"); + const arr = handleIfApiVersionNotice(unsupportedVersion, [ + { id: 1, date: new Date("2017-01-01") }, + { id: 2, date: new Date("2017-01-02") }, + ]); + return Promise.resolve(arr); + }; + + const expectedErrorMessage = + "Version not found. Visit https://analytics.usa.gov/developer for information on the latest supported version."; + + const dataRequest = request(app) + .get( + `/${unsupportedVersion}/agencies/fake-agency/reports/fake-report/data`, + ) + .expect(404); + + await dataRequest.then((actualResponse) => { + const expectedResponse = { + _body: expectedErrorMessage, + status: 404, + }; + expect(actualResponse).to.include(expectedResponse); + }); + }); + }); }); }); }); From 3b06022ecbf46646e4ba24cfc15e14340412f819 Mon Sep 17 00:00:00 2001 From: Michael Levin Date: Fri, 8 Nov 2024 09:53:18 -0500 Subject: [PATCH 4/4] [Tech Debt] Add workflow to deploy manually to the dev environment --- .github/workflows/deploy_to_dev_manually.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .github/workflows/deploy_to_dev_manually.yml diff --git a/.github/workflows/deploy_to_dev_manually.yml b/.github/workflows/deploy_to_dev_manually.yml new file mode 100644 index 0000000..64e60f0 --- /dev/null +++ b/.github/workflows/deploy_to_dev_manually.yml @@ -0,0 +1,19 @@ +name: Deploy to dev manually + +on: + workflow_dispatch: + +jobs: + deploy_dev: + uses: 18F/analytics-reporter-api/.github/workflows/deploy.yml@develop + with: + APP_NAME: ${{ vars.APP_NAME_DEV }} + CF_ORGANIZATION_NAME: ${{ vars.CF_ORGANIZATION_NAME }} + CF_SPACE_NAME: ${{ vars.CF_SPACE_NAME_DEV }} + DB_SERVICE_NAME: ${{ vars.DB_SERVICE_NAME_DEV }} + NEW_RELIC_APP_NAME: ${{ vars.NEW_RELIC_APP_NAME_DEV }} + secrets: + API_DATA_GOV_SECRET: ${{ secrets.API_DATA_GOV_SECRET_DEV }} + CF_USERNAME: ${{ secrets.CF_USERNAME_DEV }} + CF_PASSWORD: ${{ secrets.CF_PASSWORD_DEV }} + NEW_RELIC_LICENSE_KEY: ${{ secrets.NEW_RELIC_LICENSE_KEY_DEV }}