diff --git a/.eslintrc b/.eslintrc index 97378f5557..ee25c707ac 100644 --- a/.eslintrc +++ b/.eslintrc @@ -21,7 +21,8 @@ "sourceType": "module", "ecmaFeatures": { "modules": true - } + }, + "project": "./tsconfig.json" }, "plugins": ["@typescript-eslint", "import", "simple-import-sort"], "extends": ["plugin:@typescript-eslint/recommended"], @@ -50,7 +51,8 @@ "import/order": "off", "import/first": "error", "import/newline-after-import": "error", - "import/no-duplicates": "error" + "import/no-duplicates": "error", + "@typescript-eslint/no-floating-promises": 2, } }, { "files": ["*.spec.ts"], "extends": ["plugin:jest/recommended"] } diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 13510810cd..37cfe2af3d 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,12 +1,10 @@ ## Problem - -_What problem are you trying to solve? What issue does this close?_ + Closes [insert issue #] ## Solution - -_How did you solve the problem?_ + **Features**: @@ -23,19 +21,17 @@ _How did you solve the problem?_ ## Before & After Screenshots **BEFORE**: -[insert screenshot here] + **AFTER**: -[insert screenshot here] + ## Tests - -_What tests should be run to confirm functionality?_ + ## Deploy Notes - -_Notes regarding deployment of the contained body of work. These should note any -new dependencies, new scripts, etc._ + + **New environment variables**: diff --git a/.template-env b/.template-env index 2f8dda0b6b..72d755eade 100644 --- a/.template-env +++ b/.template-env @@ -104,4 +104,8 @@ FORMSG_SDK_MODE= # CORPPASS_IDP_ID=https://saml.corppass.gov.sg/FIM/sps/CorpIDPFed/saml20 # IS_SP_MAINTENANCE= -# IS_CP_MAINTENANCE= \ No newline at end of file +# IS_CP_MAINTENANCE= + +## Per-minute, per-IP request limits applied to specific endpoints +# SUBMISSIONS_RATE_LIMIT= +# SEND_AUTH_OTP_RATE_LIMIT= \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index c02fad34f8..32dee9a8d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,49 @@ 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). +#### [v4.39.0](https://github.com/opengovsg/formsg/compare/v4.37.1...v4.39.0) + +- fix: early return when validating empty email string [`#433`](https://github.com/opengovsg/formsg/pull/433) +- fix(deps): bump angular-cookies from 1.8.0 to 1.8.1 [`#419`](https://github.com/opengovsg/formsg/pull/419) +- * /billing [`#426`](https://github.com/opengovsg/formsg/pull/426) +- chore: merge release v4.38.1 into develop [`#430`](https://github.com/opengovsg/formsg/pull/430) +- feat: soft-launch rate-limiting of API endpoints [`#389`](https://github.com/opengovsg/formsg/pull/389) +- refactor: use res.json or sendStatus for objects or empty body [`#424`](https://github.com/opengovsg/formsg/pull/424) +- refactor: migrate CaptchaFactory to Typescript [`#397`](https://github.com/opengovsg/formsg/pull/397) +- chore: key cleanup [`#326`](https://github.com/opengovsg/formsg/pull/326) +- chore(deps-dev): remove eslint-plugin-html [`#402`](https://github.com/opengovsg/formsg/pull/402) +- chore: use comments for PR template guiding questions [`#420`](https://github.com/opengovsg/formsg/pull/420) +- feat: add analytics module to handle /analytics endpoints [`#403`](https://github.com/opengovsg/formsg/pull/403) +- chore(deps-dev): bump axios-mock-adapter from 1.18.1 to 1.18.2 [`#411`](https://github.com/opengovsg/formsg/pull/411) +- feat: add trace to logs [`#405`](https://github.com/opengovsg/formsg/pull/405) +- chore: add lint rule to prevent floating promises [`#404`](https://github.com/opengovsg/formsg/pull/404) +- refactor: migrate SmsFactory to Typescript [`#387`](https://github.com/opengovsg/formsg/pull/387) +- fix: transfer form toastr [`#379`](https://github.com/opengovsg/formsg/pull/379) +- fix(deps): bump @sentry/browser from 5.22.3 to 5.24.2 [`#407`](https://github.com/opengovsg/formsg/pull/407) +- fix: only show exclamation in navbar if sms feature is enabled [`#383`](https://github.com/opengovsg/formsg/pull/383) +- fix(deps): bump nodemailer from 6.4.11 to 6.4.12 [`#399`](https://github.com/opengovsg/formsg/pull/399) +- fix(deps): bump multiparty from 4.2.1 to 4.2.2 [`#406`](https://github.com/opengovsg/formsg/pull/406) +- chore(deps-dev): bump eslint from 7.9.0 to 7.10.0 [`#401`](https://github.com/opengovsg/formsg/pull/401) +- refactor: use validator's isEmail for validating email domains [`#386`](https://github.com/opengovsg/formsg/pull/386) +- style: fix squished styling when user emails are too long [`#382`](https://github.com/opengovsg/formsg/pull/382) +- fix(deps): bump aws-sdk from 2.734.0 to 2.763.0 [`#393`](https://github.com/opengovsg/formsg/pull/393) +- chore(deps-dev): bump @types/express from 4.17.6 to 4.17.8 [`#392`](https://github.com/opengovsg/formsg/pull/392) +- fix(deps): bump helmet from 4.1.0 to 4.1.1 [`#390`](https://github.com/opengovsg/formsg/pull/390) +- chore: merge Release v4.37.1 back into develop [`#391`](https://github.com/opengovsg/formsg/pull/391) +- fix: invalid key reference in retrieving form submissions [`#385`](https://github.com/opengovsg/formsg/pull/385) +- chore: merge release v4.37.0 back into develop [`#384`](https://github.com/opengovsg/formsg/pull/384) +- chore(deps-dev): bump @types/uuid from 8.0.0 to 8.3.0 [`#375`](https://github.com/opengovsg/formsg/pull/375) +- fix(deps): bump twilio from 3.46.0 to 3.49.3 [`#367`](https://github.com/opengovsg/formsg/pull/367) +- chore(deps-dev): bump @types/node from 14.0.13 to 14.11.2 [`#374`](https://github.com/opengovsg/formsg/pull/374) +- test: update tests [`2aa9e24`](https://github.com/opengovsg/formsg/commit/2aa9e24811ceeeb837e60d434b852fcf84a458f2) +- chore: bump version to v4.38.0 [`e940ef9`](https://github.com/opengovsg/formsg/commit/e940ef9f666a74b26944d3111ebccb95babffdd8) +- chore: bump version to 4.38.1 [`118257c`](https://github.com/opengovsg/formsg/commit/118257c5828fefcf101efa1eca032fa021571edc) + #### [v4.37.1](https://github.com/opengovsg/formsg/compare/v4.37.0...v4.37.1) +> 30 September 2020 + +- chore: bump version to v4.37.1 [`ac6389d`](https://github.com/opengovsg/formsg/commit/ac6389dfcc790a6dbccb0f22d505f8daa1100cbe) - fix: correct form header padding when no banner is available [`421a117`](https://github.com/opengovsg/formsg/commit/421a117b096d7053bb93e39091a528a9707bb105) #### [v4.37.0](https://github.com/opengovsg/formsg/compare/v4.36.0...v4.37.0) diff --git a/docker-compose.yml b/docker-compose.yml index 7ca87965b4..699c2b6535 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -30,6 +30,8 @@ services: - AWS_SECRET_ACCESS_KEY=fakeSecret - SESSION_SECRET=thisisasecret - AWS_ENDPOINT=http://localhost:4566 + - SUBMISSIONS_RATE_LIMIT=200 + - SEND_AUTH_OTP_RATE_LIMIT=60 - GA_TRACKING_ID - SENTRY_CONFIG_URL - TWILIO_ACCOUNT_SID diff --git a/docs/DEPLOYMENT_SETUP.md b/docs/DEPLOYMENT_SETUP.md index 6fd821164e..60f3b48beb 100644 --- a/docs/DEPLOYMENT_SETUP.md +++ b/docs/DEPLOYMENT_SETUP.md @@ -212,6 +212,14 @@ SITE_BANNER_CONTENT=hello:This is an invalid banner type, and the full text will | `CHROMIUM_BIN` | Filepath to chromium binary. Required for email autoreply PDF generation with Puppeteer. | | `BOUNCE_LIFE_SPAN` | Time in milliseconds that bounces are tracked for each form. Defaults to 86400000ms or 24 hours. Only relevant if you have set up AWS to send bounce and delivery notifications to the /emailnotifications endpoint. | +#### Rate limits at specific endpoints + +The app applies per-minute, per-IP rate limits at specific API endpoints as a security measure. The limits can be specified with the following environment variables. +| Variable | Description | +| :-------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `SUBMISSIONS_RATE_LIMIT` | Per-minute, per-IP request limit for each submissions endpoint. The limit is applied separately for the email mode and encrypt mode endpoints. | +| `SEND_AUTH_OTP_RATE_LIMIT` | Per-minute, per-IP request limit for the endpoint which requests for new login OTPs for the admin console. | + ### Additional Features The app supports a number of additional features like Captcha protection, Sentry reporting and Google Analytics. Each of these features requires specific environment variables which are detailed below. To deploy a bare bones application without these additional features, one can safely exclude the respective environment variables without any extra configuration. diff --git a/package-lock.json b/package-lock.json index 28b28e5e6e..f9b10447d1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "FormSG", - "version": "4.37.1", + "version": "4.39.0", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -2843,12 +2843,12 @@ }, "dependencies": { "debug": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", - "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", + "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", "dev": true, "requires": { - "ms": "^2.1.1" + "ms": "2.1.2" } }, "globals": { @@ -4158,35 +4158,35 @@ } }, "@sentry/browser": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/browser/-/browser-5.22.3.tgz", - "integrity": "sha512-2TzE/CoBa5ZkvxJizDdi1Iz1ldmXSJpFQ1mL07PIXBjCt0Wxf+WOuFSj5IP4L40XHfJE5gU8wEvSH0VDR8nXtA==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/browser/-/browser-5.24.2.tgz", + "integrity": "sha512-P/uZC/VrLRpU7MVEJnlZK5+AkEmuHu+mns5gC91Z4gjn7GamjR/CaXVedHGw/15ZrsQiAiwoWwuxpv4Ypd/+SA==", "requires": { - "@sentry/core": "5.22.3", - "@sentry/types": "5.22.3", - "@sentry/utils": "5.22.3", + "@sentry/core": "5.24.2", + "@sentry/types": "5.24.2", + "@sentry/utils": "5.24.2", "tslib": "^1.9.3" } }, "@sentry/core": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/core/-/core-5.22.3.tgz", - "integrity": "sha512-eGL5uUarw3o4i9QUb9JoFHnhriPpWCaqeaIBB06HUpdcvhrjoowcKZj1+WPec5lFg5XusE35vez7z/FPzmJUDw==", - "requires": { - "@sentry/hub": "5.22.3", - "@sentry/minimal": "5.22.3", - "@sentry/types": "5.22.3", - "@sentry/utils": "5.22.3", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/core/-/core-5.24.2.tgz", + "integrity": "sha512-nuAwCGU1l9hgMinl5P/8nIQGRXDP2FI9cJnq5h1qiP/XIOvJkJz2yzBR6nTyqr4vBth0tvxQJbIpDNGd7vHJLg==", + "requires": { + "@sentry/hub": "5.24.2", + "@sentry/minimal": "5.24.2", + "@sentry/types": "5.24.2", + "@sentry/utils": "5.24.2", "tslib": "^1.9.3" } }, "@sentry/hub": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/hub/-/hub-5.22.3.tgz", - "integrity": "sha512-INo47m6N5HFEs/7GMP9cqxOIt7rmRxdERunA3H2L37owjcr77MwHVeeJ9yawRS6FMtbWXplgWTyTIWIYOuqVbw==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/hub/-/hub-5.24.2.tgz", + "integrity": "sha512-xmO1Ivvpb5Qr9WgekinuZZlpl9Iw7iPETUe84HQOhUrXf+2gKO+LaUYMMsYSVDwXQEmR6/tTMyOtS6iavldC6w==", "requires": { - "@sentry/types": "5.22.3", - "@sentry/utils": "5.22.3", + "@sentry/types": "5.24.2", + "@sentry/utils": "5.24.2", "tslib": "^1.9.3" } }, @@ -4218,26 +4218,26 @@ } }, "@sentry/minimal": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/minimal/-/minimal-5.22.3.tgz", - "integrity": "sha512-HoINpYnVYCpNjn2XIPIlqH5o4BAITpTljXjtAftOx6Hzj+Opjg8tR8PWliyKDvkXPpc4kXK9D6TpEDw8MO0wZA==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/minimal/-/minimal-5.24.2.tgz", + "integrity": "sha512-biFpux5bI3R8xiD/Zzvrk1kRE6bqPtfWXmZYAHRtaUMCAibprTKSY9Ta8QYHynOAEoJ5Akedy6HUsEkK5DoZfA==", "requires": { - "@sentry/hub": "5.22.3", - "@sentry/types": "5.22.3", + "@sentry/hub": "5.24.2", + "@sentry/types": "5.24.2", "tslib": "^1.9.3" } }, "@sentry/types": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/types/-/types-5.22.3.tgz", - "integrity": "sha512-cv+VWK0YFgCVDvD1/HrrBWOWYG3MLuCUJRBTkV/Opdy7nkdNjhCAJQrEyMM9zX0sac8FKWKOHT0sykNh8KgmYw==" + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/types/-/types-5.24.2.tgz", + "integrity": "sha512-HcOK00R0tQG5vzrIrqQ0jC28+z76jWSgQCzXiessJ5SH/9uc6NzdO7sR7K8vqMP2+nweCHckFohC8G0T1DLzuQ==" }, "@sentry/utils": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/utils/-/utils-5.22.3.tgz", - "integrity": "sha512-AHNryXMBvIkIE+GQxTlmhBXD0Ksh+5w1SwM5qi6AttH+1qjWLvV6WB4+4pvVvEoS8t5F+WaVUZPQLmCCWp6zKw==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/utils/-/utils-5.24.2.tgz", + "integrity": "sha512-oPGde4tNEDHKk0Cg9q2p0qX649jLDUOwzJXHKpd0X65w3A6eJByDevMr8CSzKV9sesjrUpxqAv6f9WWlz185tA==", "requires": { - "@sentry/types": "5.22.3", + "@sentry/types": "5.24.2", "tslib": "^1.9.3" } }, @@ -4494,9 +4494,10 @@ "dev": true }, "@types/express": { - "version": "4.17.6", - "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.6.tgz", - "integrity": "sha512-n/mr9tZI83kd4azlPG5y997C/M4DNABK9yErhFM6hKdym4kkmd9j0vtsJyjFIwfRBxtrxZtAfGZCNRIBMFLK5w==", + "version": "4.17.8", + "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.8.tgz", + "integrity": "sha512-wLhcKh3PMlyA2cNAB9sjM1BntnhPMiM0JOBwPBqttjHev2428MLEB4AYVN+d8s2iyCVZac+o41Pflm/ZH5vLXQ==", + "dev": true, "requires": { "@types/body-parser": "*", "@types/express-serve-static-core": "*", @@ -4504,10 +4505,27 @@ "@types/serve-static": "*" } }, + "@types/express-rate-limit": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/@types/express-rate-limit/-/express-rate-limit-5.1.0.tgz", + "integrity": "sha512-vmg7S3hUnfFmp06V01DrTB41mbQYXMV/F4aF5KKnfCIeSlnizatXaqO9UgR6LvNEEd3eMpuUTLxR6nv3d4hZ6g==", + "dev": true, + "requires": { + "@types/express": "*" + } + }, + "@types/express-request-id": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/@types/express-request-id/-/express-request-id-1.4.1.tgz", + "integrity": "sha512-39g9HGiBJBjsSJZ80vYgkR4yvu7XVL2R/R/KezU/060gnW5e9btRP7d0R8QORzJ3/Uo7Sis36AM58KSQQOfgcw==", + "requires": { + "@types/express-serve-static-core": "*" + } + }, "@types/express-serve-static-core": { - "version": "4.17.7", - "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-4.17.7.tgz", - "integrity": "sha512-EMgTj/DF9qpgLXyc+Btimg+XoH7A2liE8uKul8qSmMTHCeNYzydDKFdsJskDvw42UsesCnhO63dO0Grbj8J4Dw==", + "version": "4.17.13", + "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-4.17.13.tgz", + "integrity": "sha512-RgDi5a4nuzam073lRGKTUIaL3eF2+H7LJvJ8eUnCI0wA6SNjXc44DCmWNiTLs/AZ7QlsFWZiw/gTG3nSQGL0fA==", "requires": { "@types/node": "*", "@types/qs": "*", @@ -4705,9 +4723,9 @@ } }, "@types/node": { - "version": "14.0.13", - "resolved": "https://registry.npmjs.org/@types/node/-/node-14.0.13.tgz", - "integrity": "sha512-rouEWBImiRaSJsVA+ITTFM6ZxibuAlTuNOCyxVbwreu6k6+ujs7DfnU9o+PShFhET78pMBl3eH+AGSI5eOTkPA==" + "version": "14.11.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-14.11.2.tgz", + "integrity": "sha512-jiE3QIxJ8JLNcb1Ps6rDbysDhN4xa8DJJvuC9prr6w+1tIh+QAbYyNF3tyiZNLDBIuBCf4KEcV2UvQm/V60xfA==" }, "@types/nodemailer": { "version": "6.4.0", @@ -4967,9 +4985,9 @@ "dev": true }, "@types/uuid": { - "version": "8.0.0", - "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-8.0.0.tgz", - "integrity": "sha512-xSQfNcvOiE5f9dyd4Kzxbof1aTrLobL278pGLKOZI6esGfZ7ts9Ka16CzIN6Y8hFHE1C7jIBZokULhK1bOgjRw==", + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/@types/uuid/-/uuid-8.3.0.tgz", + "integrity": "sha512-eQ9qFW/fhfGJF8WKHGEHZEyVWfZxrT+6CLIJGBcZPfxUh/+BnEj+UCGYMlr9qZuX/2AltsvwrGqp0LhEW8D0zQ==", "dev": true }, "@types/validator": { @@ -5474,9 +5492,9 @@ "integrity": "sha512-eCQI6EwgY6bYHdzIUfDABHnZjoZ3bNYpCsnceQF4bLfbq1QtZ7raRPNca45sj6C9Pfjde6PNcEDvuLozFPYnrQ==" }, "angular-cookies": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/angular-cookies/-/angular-cookies-1.8.0.tgz", - "integrity": "sha512-gWO3RKF0WMmXhseiN3Aw9aEmQ3mB53wSdAxpeKKHbiDwU7vmK+MBuebyOX9qbwZYubn5nM8LByZVmg7T6jOV1w==" + "version": "1.8.1", + "resolved": "https://registry.npmjs.org/angular-cookies/-/angular-cookies-1.8.1.tgz", + "integrity": "sha512-952lNWW2REfGRwErYB4kLiWCvCE+euaPtIq8iYdZso0T74D6KCz82zSQjO5xnxfIKWMeKp2X2yIMksyH++GZcw==" }, "angular-drag-scroll": { "version": "0.2.2", @@ -5554,9 +5572,9 @@ } }, "ansi-colors": { - "version": "3.2.4", - "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-3.2.4.tgz", - "integrity": "sha512-hHUXGagefjN2iRrID63xckIvotOXOojhQKWIPUZ4mNUZ9nLZW+7FMNoE1lOkEhNWYsx/7ysGIuJYCiMAA9FnrA==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-4.1.1.tgz", + "integrity": "sha512-JoX0apGbHaUJBNl6yF+p6JAFYZ666/hhCGKN5t9QFjbJQKUU/g8MNbFDbvfrgKXvI1QpZplPOnwIo99lX/AAmA==", "dev": true }, "ansi-escapes": { @@ -5920,9 +5938,9 @@ "integrity": "sha512-ZUWFfYy9Mu5ppoDJr3TxY0UWc4peP3tS5sStgnKkRh3urYalQaZWHewhbDbz40I84PO0xhVdBGut1xQtjum3mw==" }, "aws-sdk": { - "version": "2.734.0", - "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.734.0.tgz", - "integrity": "sha512-F5SrcOm6WyaN5j+pybx97vfSCa0SIz/kukJNSeDzMdk5q0xsKUjtUzL/kc0QwtCspCni0WTNmBKiD+firP5JKQ==", + "version": "2.763.0", + "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.763.0.tgz", + "integrity": "sha512-uHb+yfsH21wR8ZInj/GQwxCmWJjrL3sOwqwZ2lf9hDxh1P0lsMKDjf0e+8ICgRBY4x28XNWXs3/O15EID6Rsqw==", "requires": { "buffer": "4.9.2", "events": "1.1.1", @@ -5978,9 +5996,9 @@ } }, "axios-mock-adapter": { - "version": "1.18.1", - "resolved": "https://registry.npmjs.org/axios-mock-adapter/-/axios-mock-adapter-1.18.1.tgz", - "integrity": "sha512-kFBZsG1Ma5yxjRGHq5KuuL55mPb7WzFULhypquEhzPg8SH5CXICb+qwC2CCA5u+GQVpiqGPwKSRkd3mBCs6gdw==", + "version": "1.18.2", + "resolved": "https://registry.npmjs.org/axios-mock-adapter/-/axios-mock-adapter-1.18.2.tgz", + "integrity": "sha512-e5aTsPy2Viov22zNpFTlid76W1Scz82pXeEwwCXdtO85LROhHAF8pHF2qDhiyMONLxKyY3lQ+S4UCsKgrlx8Hw==", "dev": true, "requires": { "fast-deep-equal": "^3.1.1", @@ -9808,9 +9826,9 @@ } }, "dayjs": { - "version": "1.8.28", - "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.8.28.tgz", - "integrity": "sha512-ccnYgKC0/hPSGXxj7Ju6AV/BP4HUkXC2u15mikXT5mX9YorEaoi1bEKOmAqdkJHN4EEkmAf97SpH66Try5Mbeg==" + "version": "1.8.36", + "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.8.36.tgz", + "integrity": "sha512-3VmRXEtw7RZKAf+4Tv1Ym9AGeo8r8+CjDi26x+7SYQil1UqtqdaokhzoEJohqlzt0m5kacJSDhJQkG/LWhpRBw==" }, "debug": { "version": "3.1.0", @@ -10294,26 +10312,6 @@ } } }, - "domhandler": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/domhandler/-/domhandler-3.0.0.tgz", - "integrity": "sha512-eKLdI5v9m67kbXQbJSNn1zjh0SDzvzWVWtX+qEI3eMjZw8daH9k8rlj1FZY9memPwjiskQFbe7vHVVJIAqoEhw==", - "dev": true, - "requires": { - "domelementtype": "^2.0.1" - } - }, - "domutils": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/domutils/-/domutils-2.1.0.tgz", - "integrity": "sha512-CD9M0Dm1iaHfQ1R/TI+z3/JWp/pgub0j4jIQKH89ARR4ATAV2nbaOQS5XxU9maJP5jHaPdDDQSEHuE2UmpUTKg==", - "dev": true, - "requires": { - "dom-serializer": "^0.2.1", - "domelementtype": "^2.0.1", - "domhandler": "^3.0.0" - } - }, "dot-prop": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/dot-prop/-/dot-prop-5.2.0.tgz", @@ -10577,12 +10575,12 @@ } }, "enquirer": { - "version": "2.3.5", - "resolved": "https://registry.npmjs.org/enquirer/-/enquirer-2.3.5.tgz", - "integrity": "sha512-BNT1C08P9XD0vNg3J475yIUG+mVdp9T6towYFHUv897X0KoHBjB1shyrNmhmtHWKP17iSWgo7Gqh7BBuzLZMSA==", + "version": "2.3.6", + "resolved": "https://registry.npmjs.org/enquirer/-/enquirer-2.3.6.tgz", + "integrity": "sha512-yjNnPr315/FjS4zIsUxYguYUPP2e1NK4d7E7ZOLiyYCcbFBiTMyID+2wvm2w6+pZ/odMA7cRkjhsPbltwBOrLg==", "dev": true, "requires": { - "ansi-colors": "^3.2.1" + "ansi-colors": "^4.1.1" } }, "entities": { @@ -10779,9 +10777,9 @@ } }, "eslint": { - "version": "7.9.0", - "resolved": "https://registry.npmjs.org/eslint/-/eslint-7.9.0.tgz", - "integrity": "sha512-V6QyhX21+uXp4T+3nrNfI3hQNBDa/P8ga7LoQOenwrlEFXrEnUEE+ok1dMtaS3b6rmLXhT1TkTIsG75HMLbknA==", + "version": "7.10.0", + "resolved": "https://registry.npmjs.org/eslint/-/eslint-7.10.0.tgz", + "integrity": "sha512-BDVffmqWl7JJXqCjAK6lWtcQThZB/aP1HXSH1JKwGwv0LQEdvpR7qzNrUT487RM39B5goWuboFad5ovMBmD8yA==", "dev": true, "requires": { "@babel/code-frame": "^7.0.0", @@ -10792,7 +10790,7 @@ "debug": "^4.0.1", "doctrine": "^3.0.0", "enquirer": "^2.3.5", - "eslint-scope": "^5.1.0", + "eslint-scope": "^5.1.1", "eslint-utils": "^2.1.0", "eslint-visitor-keys": "^1.3.0", "espree": "^7.3.0", @@ -10876,12 +10874,22 @@ } }, "debug": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", - "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", + "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", "dev": true, "requires": { - "ms": "^2.1.1" + "ms": "2.1.2" + } + }, + "eslint-scope": { + "version": "5.1.1", + "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.1.tgz", + "integrity": "sha512-2NxwbF/hZ0KpepYN0cNbo+FN6XoK7GaHlQhgx/hIZl6Va0bF45RQOOwhLIy8lQDbuCiadSLCBnH2CFYquit5bw==", + "dev": true, + "requires": { + "esrecurse": "^4.3.0", + "estraverse": "^4.1.1" } }, "eslint-visitor-keys": { @@ -10890,6 +10898,23 @@ "integrity": "sha512-6J72N8UNa462wa/KFODt/PJ3IU60SDpC3QXC1Hjc1BXXpfL2C9R5+AU7jhe0F6GREqVMh4Juu+NY7xn+6dipUQ==", "dev": true }, + "esrecurse": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/esrecurse/-/esrecurse-4.3.0.tgz", + "integrity": "sha512-KmfKL3b6G+RXvP8N1vr3Tq1kL/oCFgn2NYXEtqP8/L3pKapUA4G8cFVaoF3SU323CD4XypR/ffioHmkti6/Tag==", + "dev": true, + "requires": { + "estraverse": "^5.2.0" + }, + "dependencies": { + "estraverse": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/estraverse/-/estraverse-5.2.0.tgz", + "integrity": "sha512-BxbNGGNm0RyRYvUdHpIwv9IWzeM9XClbOxwoATuFdOE7ZE6wHL+HQ5T8hoPM+zHvmKzzsEqhgy0GrQ5X13afiQ==", + "dev": true + } + } + }, "globals": { "version": "12.4.0", "resolved": "https://registry.npmjs.org/globals/-/globals-12.4.0.tgz", @@ -11090,15 +11115,6 @@ "integrity": "sha512-OaW5G461C2lIkOG+/bhnBoXB9UQm/r0Dj2Qf9uiIN0/ncvf2Llp30L0q1tqWkN8/CxyBwQKh1v0hpCLLDjaIKQ==", "dev": true }, - "eslint-plugin-html": { - "version": "6.0.2", - "resolved": "https://registry.npmjs.org/eslint-plugin-html/-/eslint-plugin-html-6.0.2.tgz", - "integrity": "sha512-Ik/z32UteKLo8GEfwNqVKcJ/WOz/be4h8N5mbMmxxnZ+9aL9XczOXQFz/bGu+nAGVoRg8CflldxJhONFpqlrxw==", - "dev": true, - "requires": { - "htmlparser2": "^4.1.0" - } - }, "eslint-plugin-import": { "version": "2.22.0", "resolved": "https://registry.npmjs.org/eslint-plugin-import/-/eslint-plugin-import-2.22.0.tgz", @@ -11606,6 +11622,26 @@ "resolved": "https://registry.npmjs.org/express-partials/-/express-partials-0.3.0.tgz", "integrity": "sha1-iLnEAWSv2aVSeGKbKUjmrOe/9F8=" }, + "express-rate-limit": { + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-5.1.3.tgz", + "integrity": "sha512-TINcxve5510pXj4n9/1AMupkj3iWxl3JuZaWhCdYDlZeoCPqweGZrxbrlqTCFb1CT5wli7s8e2SH/Qz2c9GorA==" + }, + "express-request-id": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/express-request-id/-/express-request-id-1.4.1.tgz", + "integrity": "sha512-qpxK6XhDYtdx9FvxwCHkUeZVWtkGbWR87hBAzGECfwYF/QQCPXEwwB2/9NGkOR1tT7/aLs9mma3CT0vjSzuZVw==", + "requires": { + "uuid": "^3.3.2" + }, + "dependencies": { + "uuid": { + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==" + } + } + }, "express-session": { "version": "1.17.1", "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.17.1.tgz", @@ -12933,9 +12969,9 @@ "dev": true }, "helmet": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/helmet/-/helmet-4.1.0.tgz", - "integrity": "sha512-KWy75fYN8hOG2Rhl8e5B3WhOzb0by1boQum85TiddIE9iu6gV+TXbUjVC17wfej0o/ZUpqB9kxM0NFCZRMzf+Q==" + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/helmet/-/helmet-4.1.1.tgz", + "integrity": "sha512-Avg4XxSBrehD94mkRwEljnO+6RZx7AGfk8Wa6K1nxaU+hbXlFOhlOIMgPfFqOYQB/dBCsTpootTGuiOG+CHiQA==" }, "hex-color-regex": { "version": "1.1.0", @@ -13159,18 +13195,6 @@ } } }, - "htmlparser2": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-4.1.0.tgz", - "integrity": "sha512-4zDq1a1zhE4gQso/c5LP1OtrhYTncXNSpvJYtWJBtXAETPlMfi3IFNjGuQbYLuVY4ZR0QMqRVvo4Pdy9KLyP8Q==", - "dev": true, - "requires": { - "domelementtype": "^2.0.1", - "domhandler": "^3.0.0", - "domutils": "^2.0.0", - "entities": "^2.0.0" - } - }, "http-errors": { "version": "1.7.2", "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.7.2.tgz", @@ -19105,20 +19129,31 @@ "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, "multiparty": { - "version": "4.2.1", - "resolved": "https://registry.npmjs.org/multiparty/-/multiparty-4.2.1.tgz", - "integrity": "sha512-AvESCnNoQlZiOfP9R4mxN8M9csy2L16EIbWIkt3l4FuGti9kXBS8QVzlfyg4HEnarJhrzZilgNFlZtqmoiAIIA==", + "version": "4.2.2", + "resolved": "https://registry.npmjs.org/multiparty/-/multiparty-4.2.2.tgz", + "integrity": "sha512-NtZLjlvsjcoGrzojtwQwn/Tm90aWJ6XXtPppYF4WmOk/6ncdwMMKggFY2NlRRN9yiCEIVxpOfPWahVEG2HAG8Q==", "requires": { - "fd-slicer": "1.1.0", - "http-errors": "~1.7.0", - "safe-buffer": "5.1.2", + "http-errors": "~1.8.0", + "safe-buffer": "5.2.1", "uid-safe": "2.1.5" }, "dependencies": { - "safe-buffer": { - "version": "5.1.2", - "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", - "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==" + "http-errors": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.8.0.tgz", + "integrity": "sha512-4I8r0C5JDhT5VkvI47QktDW75rNlGVsUf/8hzjCC/wkWI/jdTRmBb9aI7erSG82r1bjKY3F6k28WnsVxB1C73A==", + "requires": { + "depd": "~1.1.2", + "inherits": "2.0.4", + "setprototypeof": "1.2.0", + "statuses": ">= 1.5.0 < 2", + "toidentifier": "1.0.0" + } + }, + "setprototypeof": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.2.0.tgz", + "integrity": "sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==" } } }, @@ -19514,9 +19549,9 @@ "dev": true }, "nodemailer": { - "version": "6.4.11", - "resolved": "https://registry.npmjs.org/nodemailer/-/nodemailer-6.4.11.tgz", - "integrity": "sha512-BVZBDi+aJV4O38rxsUh164Dk1NCqgh6Cm0rQSb9SK/DHGll/DrCMnycVDD7msJgZCnmVa8ASo8EZzR7jsgTukQ==" + "version": "6.4.12", + "resolved": "https://registry.npmjs.org/nodemailer/-/nodemailer-6.4.12.tgz", + "integrity": "sha512-c/WplZp24Lxc+hn0w/kweNxcYGpaqtH1iecfGKTbXVmp5qx+ILApKsmCAucWWIIQiYKKH4ZA/ffSNOsai6xJGA==" }, "nodemailer-direct-transport": { "version": "3.3.2", @@ -21323,9 +21358,9 @@ "dev": true }, "querystringify": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/querystringify/-/querystringify-2.1.1.tgz", - "integrity": "sha512-w7fLxIRCRT7U8Qu53jQnJyPkYZIaR4n5151KMfcJlO/A9397Wxb1amJvROTK6TOnp7PfoAmg/qXiNHI+08jRfA==" + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/querystringify/-/querystringify-2.2.0.tgz", + "integrity": "sha512-FIqgj2EUvTa7R50u0rGsyTftzjYmv/a3hO345bZNrqabNqjtgiDMgmo4mkUjd+nzU5oF3dClKqFIPUKybUyqoQ==" }, "quick-lru": { "version": "4.0.1", @@ -25305,23 +25340,40 @@ "integrity": "sha512-RKJBIj8lySrShN4w6i/BonWp2Z/uxwC3h4y7xsRrpP59ZboCd0GpEVsOnMDYLMmKBpYhb5TgHzZXy7wTfYFBRw==" }, "twilio": { - "version": "3.46.0", - "resolved": "https://registry.npmjs.org/twilio/-/twilio-3.46.0.tgz", - "integrity": "sha512-A/BLN9Ml0+eQZ/cmeOG2AuTqUcP/berxooVturbDNt6LWr89xko2rUr32o3M/tEmOeWYqDw+VEngDlP3XatvIQ==", + "version": "3.49.3", + "resolved": "https://registry.npmjs.org/twilio/-/twilio-3.49.3.tgz", + "integrity": "sha512-jdstMeMx+mlm8EggoPOxIE3vVPe4doaM2dKB64ib+wJgYtYGKmFztFb0jTVIx4/980/eI3KVGjkzKiZ9c6CJhQ==", "requires": { - "@types/express": "^4.17.3", + "@types/express": "^4.17.7", + "@types/qs": "6.9.4", "axios": "^0.19.2", - "dayjs": "^1.8.21", + "dayjs": "^1.8.29", "jsonwebtoken": "^8.5.1", - "lodash": "^4.17.15", + "lodash": "^4.17.19", "q": "2.0.x", - "qs": "^6.9.1", + "qs": "^6.9.4", "rootpath": "^0.1.2", "scmp": "^2.1.0", "url-parse": "^1.4.7", "xmlbuilder": "^13.0.2" }, "dependencies": { + "@types/express": { + "version": "4.17.8", + "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.8.tgz", + "integrity": "sha512-wLhcKh3PMlyA2cNAB9sjM1BntnhPMiM0JOBwPBqttjHev2428MLEB4AYVN+d8s2iyCVZac+o41Pflm/ZH5vLXQ==", + "requires": { + "@types/body-parser": "*", + "@types/express-serve-static-core": "*", + "@types/qs": "*", + "@types/serve-static": "*" + } + }, + "@types/qs": { + "version": "6.9.4", + "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.4.tgz", + "integrity": "sha512-+wYo+L6ZF6BMoEjtf8zB2esQsqdV6WsjRK/GP9WOgLPrq87PbNWgIxS76dS5uvl/QXtHGakZmwTznIfcPXcKlQ==" + }, "axios": { "version": "0.19.2", "resolved": "https://registry.npmjs.org/axios/-/axios-0.19.2.tgz", diff --git a/package.json b/package.json index c1b377d467..39a200cf9b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "FormSG", "description": "Form Manager for Government", - "version": "4.37.1", + "version": "4.39.0", "homepage": "https://form.gov.sg", "authors": [ "FormSG " @@ -68,14 +68,16 @@ "@opengovsg/myinfo-gov-client": "^1.0.4", "@opengovsg/ng-file-upload": "^12.2.14", "@opengovsg/spcp-auth-client": "^1.3.5", - "@sentry/browser": "^5.22.3", + "@sentry/browser": "^5.24.2", "@sentry/integrations": "^5.24.2", "@stablelib/base64": "^1.0.0", + "@types/express-request-id": "^1.4.1", + "@types/express-serve-static-core": "^4.17.13", "JSONStream": "^1.3.5", "angular": "~1.8.0", "angular-animate": "^1.8.0", "angular-aria": "^1.8.0", - "angular-cookies": "~1.8.0", + "angular-cookies": "~1.8.1", "angular-drag-scroll": "^0.2.1", "angular-messages": "^1.8.0", "angular-moment": "~1.2.0", @@ -88,7 +90,7 @@ "angular-ui-router": "~1.0.22", "await-to-js": "^2.1.1", "aws-info": "^1.1.0", - "aws-sdk": "^2.734.0", + "aws-sdk": "^2.763.0", "axios": "^0.20.0", "bcrypt": "^5.0.0", "bluebird": "^3.5.2", @@ -112,6 +114,8 @@ "ejs": "^3.1.5", "express": "^4.16.4", "express-device": "~0.4.2", + "express-rate-limit": "^5.1.3", + "express-request-id": "^1.4.1", "express-session": "^1.15.6", "express-winston": "^4.0.5", "fetch-readablestream": "^0.2.0", @@ -120,7 +124,7 @@ "font-awesome": "4.7.0", "glob": "^7.1.2", "has-ansi": "^4.0.0", - "helmet": "^4.1.0", + "helmet": "^4.1.1", "http-status-codes": "^2.1.4", "intl-tel-input": "~12.1.6", "json-stringify-deterministic": "^1.0.1", @@ -133,14 +137,14 @@ "moment-timezone": "0.5.31", "mongodb-uri": "^0.9.7", "mongoose": "^5.10.0", - "multiparty": ">=4.1.3", + "multiparty": ">=4.2.2", "neverthrow": "^2.7.1", "ng-infinite-scroll": "^1.3.0", "ng-table": "^3.0.1", "ngclipboard": "^2.0.0", "nocache": "^2.1.0", "node-cache": "^5.1.2", - "nodemailer": "^6.4.11", + "nodemailer": "^6.4.12", "nodemailer-direct-transport": "~3.3.2", "opossum": "^5.0.1", "promise-retry": "^2.0.1", @@ -154,7 +158,7 @@ "toastr": "^2.1.4", "triple-beam": "^1.3.0", "tweetnacl": "^1.0.1", - "twilio": "^3.33.1", + "twilio": "^3.49.3", "ui-select": "^0.19.8", "uid-generator": "^2.0.0", "uuid": "^8.3.0", @@ -174,7 +178,8 @@ "@types/convict": "^5.2.1", "@types/cookie-parser": "^1.4.2", "@types/ejs": "^3.0.4", - "@types/express": "^4.17.6", + "@types/express": "^4.17.8", + "@types/express-rate-limit": "^5.1.0", "@types/express-session": "^1.17.0", "@types/has-ansi": "^3.0.0", "@types/helmet": "0.0.48", @@ -183,7 +188,7 @@ "@types/json-stringify-safe": "^5.0.0", "@types/mongodb-uri": "^0.9.0", "@types/mongoose": "^5.7.36", - "@types/node": "^14.0.13", + "@types/node": "^14.11.2", "@types/nodemailer": "^6.4.0", "@types/nodemailer-direct-transport": "^1.0.31", "@types/promise-retry": "^1.1.3", @@ -191,12 +196,12 @@ "@types/supertest": "^2.0.10", "@types/triple-beam": "^1.3.2", "@types/uid-generator": "^2.0.2", - "@types/uuid": "^8.0.0", + "@types/uuid": "^8.3.0", "@types/validator": "^13.1.0", "@typescript-eslint/eslint-plugin": "^4.0.1", "@typescript-eslint/parser": "^4.0.0", "auto-changelog": "^2.2.1", - "axios-mock-adapter": "^1.18.1", + "axios-mock-adapter": "^1.18.2", "babel-loader": "^8.0.5", "concurrently": "^5.3.0", "copy-webpack-plugin": "^6.0.2", @@ -205,10 +210,9 @@ "css-loader": "^2.1.1", "csv-parse": "^4.12.0", "env-cmd": "^10.1.0", - "eslint": "^7.9.0", + "eslint": "^7.10.0", "eslint-config-prettier": "^6.11.0", "eslint-plugin-angular": "^4.0.1", - "eslint-plugin-html": "^6.0.2", "eslint-plugin-import": "^2.22.0", "eslint-plugin-jest": "^24.0.2", "eslint-plugin-prettier": "^3.1.3", diff --git a/scripts/dangling-key-cleanup/check-keys.js b/scripts/20191212_dangling-key-cleanup/check-keys.js similarity index 100% rename from scripts/dangling-key-cleanup/check-keys.js rename to scripts/20191212_dangling-key-cleanup/check-keys.js diff --git a/scripts/dangling-key-cleanup/compare-screenshots.js b/scripts/20191212_dangling-key-cleanup/compare-screenshots.js similarity index 100% rename from scripts/dangling-key-cleanup/compare-screenshots.js rename to scripts/20191212_dangling-key-cleanup/compare-screenshots.js diff --git a/scripts/dangling-key-cleanup/delete-keys.js b/scripts/20191212_dangling-key-cleanup/delete-keys.js similarity index 100% rename from scripts/dangling-key-cleanup/delete-keys.js rename to scripts/20191212_dangling-key-cleanup/delete-keys.js diff --git a/scripts/dangling-key-cleanup/download-screenshots.js b/scripts/20191212_dangling-key-cleanup/download-screenshots.js similarity index 100% rename from scripts/dangling-key-cleanup/download-screenshots.js rename to scripts/20191212_dangling-key-cleanup/download-screenshots.js diff --git a/scripts/dangling-key-cleanup/formIdsToCheck.js b/scripts/20191212_dangling-key-cleanup/formIdsToCheck.js similarity index 100% rename from scripts/dangling-key-cleanup/formIdsToCheck.js rename to scripts/20191212_dangling-key-cleanup/formIdsToCheck.js diff --git a/scripts/split-isbeta/add-betaFlags.js b/scripts/20200218_split-isbeta/add-betaFlags.js similarity index 100% rename from scripts/split-isbeta/add-betaFlags.js rename to scripts/20200218_split-isbeta/add-betaFlags.js diff --git a/scripts/split-isbeta/backfill-betaFlags.js b/scripts/20200218_split-isbeta/backfill-betaFlags.js similarity index 100% rename from scripts/split-isbeta/backfill-betaFlags.js rename to scripts/20200218_split-isbeta/backfill-betaFlags.js diff --git a/scripts/encryption-versioning/backfill-encryptedVersions.js b/scripts/20200402_encryption-versioning/backfill-encryptedVersions.js similarity index 100% rename from scripts/encryption-versioning/backfill-encryptedVersions.js rename to scripts/20200402_encryption-versioning/backfill-encryptedVersions.js diff --git a/scripts/prevent-submit-logic/backfill-logic-type.js b/scripts/20200504_prevent-submit-logic/backfill-logic-type.js similarity index 100% rename from scripts/prevent-submit-logic/backfill-logic-type.js rename to scripts/20200504_prevent-submit-logic/backfill-logic-type.js diff --git a/scripts/remove-mobile-whatsapp/delete-allow-mobile.js b/scripts/20200609_remove-mobile-whatsapp/delete-allow-mobile.js similarity index 100% rename from scripts/remove-mobile-whatsapp/delete-allow-mobile.js rename to scripts/20200609_remove-mobile-whatsapp/delete-allow-mobile.js diff --git a/scripts/remove-mobile-whatsapp/delete-mobile-wa.js b/scripts/20200609_remove-mobile-whatsapp/delete-mobile-wa.js similarity index 100% rename from scripts/remove-mobile-whatsapp/delete-mobile-wa.js rename to scripts/20200609_remove-mobile-whatsapp/delete-mobile-wa.js diff --git a/scripts/myinfo-phonefields-migration/myinfo-homeno-textfield-migration.js b/scripts/20200611_myinfo-phonefields-migration/myinfo-homeno-textfield-migration.js similarity index 100% rename from scripts/myinfo-phonefields-migration/myinfo-homeno-textfield-migration.js rename to scripts/20200611_myinfo-phonefields-migration/myinfo-homeno-textfield-migration.js diff --git a/scripts/myinfo-phonefields-migration/myinfo-mobile-textfield-migration.js b/scripts/20200611_myinfo-phonefields-migration/myinfo-mobile-textfield-migration.js similarity index 100% rename from scripts/myinfo-phonefields-migration/myinfo-mobile-textfield-migration.js rename to scripts/20200611_myinfo-phonefields-migration/myinfo-mobile-textfield-migration.js diff --git a/scripts/remove-fieldvalue/remove-fieldvalue.js b/scripts/20200618_remove-fieldvalue/remove-fieldvalue.js similarity index 100% rename from scripts/remove-fieldvalue/remove-fieldvalue.js rename to scripts/20200618_remove-fieldvalue/remove-fieldvalue.js diff --git a/scripts/remove-unused-flags/remove-allow-sms.js b/scripts/20200618_remove-unused-flags/remove-allow-sms.js similarity index 100% rename from scripts/remove-unused-flags/remove-allow-sms.js rename to scripts/20200618_remove-unused-flags/remove-allow-sms.js diff --git a/scripts/remove-unused-flags/remove-unused-flags.js b/scripts/20200618_remove-unused-flags/remove-unused-flags.js similarity index 100% rename from scripts/remove-unused-flags/remove-unused-flags.js rename to scripts/20200618_remove-unused-flags/remove-unused-flags.js diff --git a/scripts/date-logic/date-logic.js b/scripts/20200727_date-logic/date-logic.js similarity index 100% rename from scripts/date-logic/date-logic.js rename to scripts/20200727_date-logic/date-logic.js diff --git a/scripts/date-logic/remove-isfutureonly-key.js b/scripts/20200727_date-logic/remove-isfutureonly-key.js similarity index 100% rename from scripts/date-logic/remove-isfutureonly-key.js rename to scripts/20200727_date-logic/remove-isfutureonly-key.js diff --git a/scripts/remove-sms-autoreply/remove-sms-autoreply.js b/scripts/20200729_remove-sms-autoreply/remove-sms-autoreply.js similarity index 100% rename from scripts/remove-sms-autoreply/remove-sms-autoreply.js rename to scripts/20200729_remove-sms-autoreply/remove-sms-autoreply.js diff --git a/scripts/20200923_unused-key-cleanup/delete-agency-key.js b/scripts/20200923_unused-key-cleanup/delete-agency-key.js new file mode 100644 index 0000000000..bca2309bfb --- /dev/null +++ b/scripts/20200923_unused-key-cleanup/delete-agency-key.js @@ -0,0 +1,34 @@ +/* eslint-disable */ + +/* +Delete unused keys for all formfeedback +*/ + +// Check total formfeedback count +db.getCollection('formfeedback').count() + +// Check number of formfeedback with agency flag +db.getCollection('formfeedback') + .find({ 'agency': { $exists: true } }) + .count() + +// !!!! MAIN UPDATE SCRIPT !!!! + +// Delete unused agency key +// ~ number updated should match number which had key +db.getCollection('formfeedback').updateMany({}, { + $unset: { + 'agency': 1, + } +}) + +// !!!! END MAIN UPDATE SCRIPT !!!! + +// Check number of formfeedback with agency flag +// ~ Should be zero +db.getCollection('formfeedback') + .find({ 'agency': { $exists: true } }) + .count() + +// Check total formfeedback count +db.getCollection('formfeedback').count() \ No newline at end of file diff --git a/scripts/20200923_unused-key-cleanup/delete-isBeta-key.js b/scripts/20200923_unused-key-cleanup/delete-isBeta-key.js new file mode 100644 index 0000000000..47ae067d27 --- /dev/null +++ b/scripts/20200923_unused-key-cleanup/delete-isBeta-key.js @@ -0,0 +1,34 @@ +/* eslint-disable */ + +/* +Delete unused keys for all users +*/ + +// Check total user count +db.getCollection('users').count() + +// Check number of users with isBeta flag +db.getCollection('users') + .find({ 'isBeta': { $exists: true } }) + .count() + +// !!!! MAIN UPDATE SCRIPT !!!! + +// Delete unused isBeta key +// ~ number updated should match number which had key +db.getCollection('users').updateMany({}, { + $unset: { + 'isBeta': 1, + } +}) + +// !!!! END MAIN UPDATE SCRIPT !!!! + +// Check number of users with isBeta flag +// ~ Should be zero +db.getCollection('users') + .find({ 'isBeta': { $exists: true } }) + .count() + +// Check total user count +db.getCollection('users').count() \ No newline at end of file diff --git a/scripts/20200923_unused-key-cleanup/delete-isSingPassAuthenticated-key.js b/scripts/20200923_unused-key-cleanup/delete-isSingPassAuthenticated-key.js new file mode 100644 index 0000000000..9778eec7c7 --- /dev/null +++ b/scripts/20200923_unused-key-cleanup/delete-isSingPassAuthenticated-key.js @@ -0,0 +1,34 @@ +/* eslint-disable */ + +/* +Delete unused keys for all submissions +*/ + +// Check total submissions count +db.getCollection('submissions').count() + +// Check number of submissions with isSingPassAuthenticated flag +db.getCollection('submissions') + .find({ 'isSingPassAuthenticated': { $exists: true } }) + .count() + +// !!!! MAIN UPDATE SCRIPT !!!! + +// Delete unused isSingPassAuthenticated key +// ~ number updated should match number which had key +db.getCollection('submissions').updateMany({}, { + $unset: { + 'isSingPassAuthenticated': 1, + } +}) + +// !!!! END MAIN UPDATE SCRIPT !!!! + +// Check number of submissions with isSingPassAuthenticated flag +// ~ Should be zero +db.getCollection('submissions') + .find({ 'isSingPassAuthenticated': { $exists: true } }) + .count() + +// Check total submissions count +db.getCollection('submissions').count() \ No newline at end of file diff --git a/scripts/20200923_unused-key-cleanup/delete-userName-key.js b/scripts/20200923_unused-key-cleanup/delete-userName-key.js new file mode 100644 index 0000000000..24649c1ada --- /dev/null +++ b/scripts/20200923_unused-key-cleanup/delete-userName-key.js @@ -0,0 +1,34 @@ +/* eslint-disable */ + +/* +Delete unused keys for all logins +*/ + +// Check total login count +db.getCollection('logins').count() + +// Check number of logins with userName flag +db.getCollection('logins') + .find({ 'userName': { $exists: true } }) + .count() + +// !!!! MAIN UPDATE SCRIPT !!!! + +// Delete unused userName key +// ~ number updated should match number which had key +db.getCollection('logins').updateMany({}, { + $unset: { + 'userName': 1, + } +}) + +// !!!! END MAIN UPDATE SCRIPT !!!! + +// Check number of logins with userName flag +// ~ Should be zero +db.getCollection('logins') + .find({ 'userName': { $exists: true } }) + .count() + +// Check total login count +db.getCollection('logins').count() \ No newline at end of file diff --git a/scripts/20200923_unused-key-cleanup/transform-emails-key.js b/scripts/20200923_unused-key-cleanup/transform-emails-key.js new file mode 100644 index 0000000000..95b89c7e51 --- /dev/null +++ b/scripts/20200923_unused-key-cleanup/transform-emails-key.js @@ -0,0 +1,100 @@ +/* eslint-disable */ + +/* +Map emails array to proper structure +*/ + +// Check total forms count with responseMode email +db.getCollection('forms').find({ responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails as an array +// ~ Should be the same number as the total number of email mode forms +db.getCollection('forms').find({ responseMode: 'email', emails: { $type: 'array' } }).count() + +// Check total forms count with responseMode email and emails with delimiter ; +db.getCollection('forms').find({ emails: { $regex: /;/ }, responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails with delimiter , +db.getCollection('forms').find({ emails: { $regex: /,/ }, responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails in correct format +let isValidCountBefore = 0 +db.getCollection('forms').find({ responseMode: 'email' }).forEach((form) => { + let isValid = form.emails.every((email) => { + const re = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ + return re.test(email) + }) + isValidCountBefore += (isValid ? 1 : 0) +}) +print('isValidCountBefore', isValidCountBefore) + +// !!!! MAIN UPDATE SCRIPT !!!! + +// Cases +// ['test@hotmail.com'] => ['test@hotmail.com'] ~ unchanged +// ['test@hotmail.com', 'test@gmail.com'] => ['test@hotmail.com', 'test@gmail.com'] ~ unchanged +// ['test@hotmail.com, test@gmail.com'] => ['test@hotmail.com', 'test@gmail.com'] +// ['test@hotmail.com, test@gmail.com', 'test@yahoo.com'] => ['test@hotmail.com', 'test@gmail.com', 'test@yahoo.com'] +// ['test@hotmail.com; test@gmail.com; test@yahoo.com'] => ['test@hotmail.com', 'test@gmail.com', 'test@yahoo.com'] +// ['test@hotmail.com; test@gmail.com; test@yahoo.com;'] => ['test@hotmail.com', 'test@gmail.com', 'test@yahoo.com'] +// ['wee_ching_ni@pa.gov.sg;'] => ['wee_ching_ni@pa.gov.sg'] + +// Update structure of emails key +let requests = [] +db.getCollection('forms').find({ responseMode: 'email' }).forEach((form) => { + let parsedEmails = form.emails + .join(',') + .replace(/;/g, ',') + .replace(/\s/g, ',') + .split(',') + .map(item => item.trim().toLowerCase()) + .filter((email) => email.includes('@')) // remove "" + requests.push({ + updateOne: { + filter: { _id: form._id }, + update: { $set: { emails: parsedEmails } } + } + }) + if (requests.length === 100000) { + //Execute per 100000 operations and re-init + db.getCollection('forms').bulkWrite(requests); + requests = []; + } +}) +if (requests.length > 0) { + db.getCollection('forms').bulkWrite(requests); +} + +// !!!! END MAIN UPDATE SCRIPT !!!! + +// Check total forms count with responseMode email +// ~ should not have changed from before +db.getCollection('forms').find({ responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails as an array +// ~ Should be the same number as the total number of email mode forms +db.getCollection('forms').find({ responseMode: 'email', emails: { $type: 'array' } }).count() + +// Check total forms count with responseMode email and emails with delimiter ; +// ~ should be zero +db.getCollection('forms').find({ emails: { $regex: /;/ }, responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails with delimiter , +// ~ should be zero +db.getCollection('forms').find({ emails: { $regex: /,/ }, responseMode: 'email' }).count() + +// Check total forms count with responseMode email and emails in correct format +// ~ Should be the same number as the total number of email mode forms +let isValidCountAfter = 0 +let invalidEmails = [] +db.getCollection('forms').find({ responseMode: 'email' }).forEach((form) => { + let isValid = form.emails.every((email) => { + const re = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ + return re.test(email) + }) + isValidCountAfter += (isValid ? 1 : 0) + if (!isValid) invalidEmails.push({ emails: form.emails, id: form._id, status: form.status, created: form.created, lastModified: form.lastModified }) +}) +print('isValidCountAfter', isValidCountAfter) +print('invalid count', invalidEmails.length) +print('invalid forms and emails', invalidEmails) \ No newline at end of file diff --git a/src/app/controllers/admin-console.server.controller.js b/src/app/controllers/admin-console.server.controller.js index 8dd781bac6..62b9fd02b5 100644 --- a/src/app/controllers/admin-console.server.controller.js +++ b/src/app/controllers/admin-console.server.controller.js @@ -20,7 +20,7 @@ const Form = getFormModel(mongoose) const _ = require('lodash') const logger = require('../../config/logger').createLoggerWithLabel(module) -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') // Examples search-specific constants const PAGE_SIZE = 16 // maximum number of results to return @@ -428,7 +428,7 @@ let sortByCreated = [ let searchSubmissionsForForm = (key, formId) => [ { $match: { - key: mongoose.Types.ObjectId(formId), + [key]: mongoose.Types.ObjectId(formId), }, }, ] @@ -577,12 +577,13 @@ exports.getExampleFormsUsingAggregateCollection = function (req, res) { meta: { action: 'getExampleFormsUsingAggregateCollection', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error, }) - return res.status(status).send(result) + return res.status(status).json(result) }, ) } @@ -609,13 +610,14 @@ exports.getExampleFormsUsingSubmissionsCollection = function (req, res) { meta: { action: 'getExampleFormsUsingSubmissionsCollection', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error, }) } - return res.status(status).send(result) + return res.status(status).json(result) }, ) } @@ -699,13 +701,14 @@ exports.getSingleExampleFormUsingSubmissionCollection = function (req, res) { meta: { action: 'getSingleExampleFormUsingSubmissionCollection', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error, }) } - return res.status(status).send(result) + return res.status(status).json(result) }, ) } @@ -727,13 +730,14 @@ exports.getSingleExampleFormUsingAggregateCollection = function (req, res) { meta: { action: 'getSingleExampleFormUsingAggregateCollection', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) } - return res.status(status).send(result) + return res.status(status).json(result) }, ) } @@ -808,6 +812,7 @@ exports.getLoginStats = function (req, res) { meta: { action: 'getLoginStats', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, @@ -815,11 +820,11 @@ exports.getLoginStats = function (req, res) { }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send('Error in retrieving billing records') + .json({ message: 'Error in retrieving billing records' }) } else if (!loginStats) { return res .status(StatusCodes.NOT_FOUND) - .send('No billing records found') + .json({ message: 'No billing records found' }) } else { logger.info({ message: `Billing search for ${esrvcId} by ${ @@ -827,10 +832,11 @@ exports.getLoginStats = function (req, res) { }`, meta: { action: 'getLoginStats', + trace: getTrace(req), }, }) - return res.send({ + return res.json({ loginStats, }) } diff --git a/src/app/controllers/admin-forms.server.controller.js b/src/app/controllers/admin-forms.server.controller.js index 5f6d76bbc1..f5d0a5b4cf 100644 --- a/src/app/controllers/admin-forms.server.controller.js +++ b/src/app/controllers/admin-forms.server.controller.js @@ -11,7 +11,7 @@ const { StatusCodes } = require('http-status-codes') const logger = require('../../config/logger').createLoggerWithLabel(module) const errorHandler = require('./errors.server.controller') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const { FormLogoState } = require('../../types') const { @@ -69,6 +69,7 @@ function makeModule(connection) { meta: { action: 'respondOnMongoError', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, @@ -90,7 +91,7 @@ function makeModule(connection) { statusCode = StatusCodes.INTERNAL_SERVER_ERROR } - return res.status(statusCode).send({ + return res.status(statusCode).json({ message: errorHandler.getMongoErrorMessage(err), }) } @@ -219,7 +220,7 @@ function makeModule(connection) { */ isFormActive: function (req, res, next) { if (req.form.status === 'ARCHIVED') { - return res.status(StatusCodes.NOT_FOUND).send({ + return res.status(StatusCodes.NOT_FOUND).json({ message: 'Form has been archived', }) } else { @@ -234,7 +235,7 @@ function makeModule(connection) { */ isFormEncryptMode: function (req, res, next) { if (req.form.responseMode !== 'encrypt') { - return res.status(StatusCodes.UNPROCESSABLE_ENTITY).send({ + return res.status(StatusCodes.UNPROCESSABLE_ENTITY).json({ message: 'Form is not encrypt mode', }) } @@ -247,7 +248,7 @@ function makeModule(connection) { */ create: function (req, res) { if (!req.body.form) { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Invalid Input', }) } @@ -290,12 +291,13 @@ function makeModule(connection) { meta: { action: 'makeModule.update', ip: getRequestIp(req), + trace: getTrace(req), formId: form._id, }, }) return res .status(StatusCodes.BAD_REQUEST) - .send({ message: 'Invalid update to form' }) + .json({ message: 'Invalid update to form' }) } else { const { error, formFields } = getEditedFormFields( _.cloneDeep(form.form_fields), @@ -307,11 +309,12 @@ function makeModule(connection) { meta: { action: 'makeModule.update', ip: getRequestIp(req), + trace: getTrace(req), formId: form._id, }, error, }) - return res.status(StatusCodes.BAD_REQUEST).send({ message: error }) + return res.status(StatusCodes.BAD_REQUEST).json({ message: error }) } form.form_fields = formFields delete updatedForm.editFormField @@ -374,7 +377,7 @@ function makeModule(connection) { } else if (!form) { return res .status(StatusCodes.NOT_FOUND) - .send({ message: 'Form not found for duplication' }) + .json({ message: 'Form not found for duplication' }) } else { let responseMode = req.body.responseMode || 'email' // Custom properties on the new form @@ -450,7 +453,7 @@ function makeModule(connection) { if (err) { return respondOnMongoError(req, res, err) } else if (!forms) { - return res.status(StatusCodes.NOT_FOUND).send({ + return res.status(StatusCodes.NOT_FOUND).json({ message: 'No user-created and collaborated-on forms found', }) } @@ -474,7 +477,7 @@ function makeModule(connection) { } else if (!feedback) { return res .status(StatusCodes.NOT_FOUND) - .send({ message: 'No feedback found' }) + .json({ message: 'No feedback found' }) } else { let sum = 0 let count = 0 @@ -521,12 +524,13 @@ function makeModule(connection) { meta: { action: 'makeModule.countFeedback', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errorHandler.getMongoErrorMessage(err), }) } else { @@ -550,10 +554,11 @@ function makeModule(connection) { meta: { action: 'makeModule.streamFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: 'Error retrieving from database.', }) }) @@ -564,10 +569,11 @@ function makeModule(connection) { meta: { action: 'makeModule.streamFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: 'Error converting feedback to JSON', }) }) @@ -578,10 +584,11 @@ function makeModule(connection) { meta: { action: 'makeModule.streamFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: 'Error writing feedback to HTTP stream', }) }) @@ -605,9 +612,11 @@ function makeModule(connection) { ) { return res .status(StatusCodes.BAD_REQUEST) - .send('Form feedback data not passed in') + .json({ message: 'Form feedback data not passed in' }) } else { - return res.status(StatusCodes.OK).send('Successfully received feedback') + return res + .status(StatusCodes.OK) + .json({ message: 'Successfully received feedback' }) } }, /** @@ -689,12 +698,13 @@ function makeModule(connection) { meta: { action: 'makeModule.streamFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send(err) + return res.status(StatusCodes.BAD_REQUEST).json(err) } else { - return res.status(StatusCodes.OK).send(presignedPostObject) + return res.status(StatusCodes.OK).json(presignedPostObject) } }, ) @@ -736,12 +746,13 @@ function makeModule(connection) { meta: { action: 'makeModule.streamFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send(err) + return res.status(StatusCodes.BAD_REQUEST).json(err) } else { - return res.status(StatusCodes.OK).send(presignedPostObject) + return res.status(StatusCodes.OK).json(presignedPostObject) } }, ) @@ -764,12 +775,13 @@ function makeModule(connection) { meta: { action: 'makeModule.transferOwner', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, err, }) - return res.status(StatusCodes.CONFLICT).send({ message: err.message }) + return res.status(StatusCodes.CONFLICT).json({ message: err.message }) } req.form.save(function (err, savedForm) { if (err) return respondOnMongoError(req, res, err) diff --git a/src/app/controllers/authentication.server.controller.js b/src/app/controllers/authentication.server.controller.js index 35cce1b62c..14ddb24c8e 100755 --- a/src/app/controllers/authentication.server.controller.js +++ b/src/app/controllers/authentication.server.controller.js @@ -5,7 +5,7 @@ */ const { StatusCodes } = require('http-status-codes') const PERMISSIONS = require('../utils/permission-levels').default -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) /** @@ -20,7 +20,7 @@ exports.authenticateUser = function (req, res, next) { } else { return res .status(StatusCodes.UNAUTHORIZED) - .send({ message: 'User is unauthorized.' }) + .json({ message: 'User is unauthorized.' }) } } @@ -50,6 +50,7 @@ const logUnauthorizedAccess = (req, action, requiredPermission) => { meta: { action: action, ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, @@ -79,7 +80,7 @@ exports.verifyPermission = (requiredPermission) => // Forbidden if requiredPersmission is admin but user is not if (!isFormAdmin && requiredPermission === PERMISSIONS.DELETE) { logUnauthorizedAccess(req, 'verifyPermission', requiredPermission) - return res.status(StatusCodes.FORBIDDEN).send({ + return res.status(StatusCodes.FORBIDDEN).json({ message: makeUnauthorizedMessage( req.session.user.email, req.form.title, @@ -113,7 +114,7 @@ exports.verifyPermission = (requiredPermission) => if (!hasSufficientPermission) { logUnauthorizedAccess(req, 'verifyPermission', requiredPermission) - return res.status(StatusCodes.FORBIDDEN).send({ + return res.status(StatusCodes.FORBIDDEN).json({ message: makeUnauthorizedMessage( req.session.user.email, req.form.title, diff --git a/src/app/controllers/core.server.controller.js b/src/app/controllers/core.server.controller.js index aabe21fcb0..ec95c06df6 100755 --- a/src/app/controllers/core.server.controller.js +++ b/src/app/controllers/core.server.controller.js @@ -1,21 +1,3 @@ -const mongoose = require('mongoose') -const _ = require('lodash') -const { StatusCodes } = require('http-status-codes') - -const config = require('../../config/config') -const { getRequestIp } = require('../utils/request') -const logger = require('../../config/logger').createLoggerWithLabel(module) - -const getFormStatisticsTotalModel = require('../models/form_statistics_total.server.model') - .default -const getSubmissionModel = require('../models/submission.server.model').default -const getUserModel = require('../models/user.server.model').default - -const FormStatisticsTotal = getFormStatisticsTotalModel(mongoose) -const Submission = getSubmissionModel(mongoose) - -const MIN_SUB_COUNT = 10 // minimum number of submissions before search is returned - /** * Renders root: '/' * @param {Object} req - Express request object @@ -26,137 +8,3 @@ exports.index = function (req, res) { user: JSON.stringify(req.session.user) || 'null', }) } - -/** - * Returns # forms that have > 10 responses using aggregate collection - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.formCountUsingAggregateCollection = (req, res) => { - FormStatisticsTotal.aggregate( - [ - { - $match: { - totalCount: { - $gt: MIN_SUB_COUNT, - }, - }, - }, - { - $count: 'numActiveForms', - }, - ], - function (err, [result]) { - if (err) { - logger.error({ - message: 'Mongo form statistics aggregate error', - meta: { - action: 'formCountUsingAggregateCollection', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - res.sendStatus(StatusCodes.SERVICE_UNAVAILABLE) - } else if (result) { - res.json(_.get(result, 'numActiveForms', 0)) - } else { - res.sendStatus(StatusCodes.INTERNAL_SERVER_ERROR) - } - }, - ) -} - -/** - * Returns # forms that have > 10 responses using submissions collection - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.formCountUsingSubmissionsCollection = (req, res) => { - Submission.aggregate( - [ - { - $group: { - _id: '$form', - count: { $sum: 1 }, - }, - }, - { - $match: { - count: { - $gt: MIN_SUB_COUNT, - }, - }, - }, - ], - function (err, forms) { - if (err) { - logger.error({ - message: 'Mongo submission aggregate error', - meta: { - action: 'formCountUsingSubmissionsCollection', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - res.sendStatus(StatusCodes.SERVICE_UNAVAILABLE) - } else { - res.json(forms.length) - } - }, - ) -} - -/** - * Returns total number of users - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.userCount = (req, res) => { - let User = getUserModel(mongoose) - User.estimatedDocumentCount(function (err, ct) { - if (err) { - logger.error({ - message: 'Mongo user count error', - meta: { - action: 'userCount', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - } else { - res.json(ct) - } - }) -} - -/** - * Returns total number of form submissions - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.submissionCount = (req, res) => { - let Submission = getSubmissionModel(mongoose) - Submission.estimatedDocumentCount(function (err, ct) { - if (err) { - logger.error({ - message: 'Mongo submission count error', - meta: { - action: 'submissionCount', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - } else { - let totalCount = ct + config.submissionsTopUp - res.json(totalCount) - } - }) -} diff --git a/src/app/controllers/email-submissions.server.controller.js b/src/app/controllers/email-submissions.server.controller.js index 8c3cd118f2..22fb85a9a5 100644 --- a/src/app/controllers/email-submissions.server.controller.js +++ b/src/app/controllers/email-submissions.server.controller.js @@ -9,7 +9,7 @@ const mongoose = require('mongoose') const { getEmailSubmissionModel } = require('../models/submission.server.model') const emailSubmission = getEmailSubmissionModel(mongoose) const { StatusCodes } = require('http-status-codes') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const { ConflictError } = require('../modules/submission/submission.errors') const { MB } = require('../constants/filesize') const { @@ -57,11 +57,12 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: _.get(req, 'form._id'), }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Required headers are missing', }) } @@ -108,6 +109,7 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, }, error: err, @@ -125,12 +127,13 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, }, }) return res .status(StatusCodes.REQUEST_TOO_LONG) - .send({ message: 'Your submission is too large.' }) + .json({ message: 'Your submission is too large.' }) } // Log hash of submission for incident investigation purposes @@ -155,6 +158,7 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { action: 'receiveEmailSubmissionUsingBusBoy', formId: req.form._id, ip: getRequestIp(req), + trace: getTrace(req), uin: hashedUinFin, submission: hashedSubmission, }, @@ -168,16 +172,17 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, }, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Some files were invalid. Try uploading another file.', }) } if (areAttachmentsMoreThan7MB(attachments)) { - return res.status(StatusCodes.UNPROCESSABLE_ENTITY).send({ + return res.status(StatusCodes.UNPROCESSABLE_ENTITY).json({ message: 'Please keep the size of your attachments under 7MB.', }) } @@ -192,6 +197,7 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, uin: hashedUinFin, submission: hashedSubmission, @@ -200,7 +206,7 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send({ message: 'Unable to process submission.' }) + .json({ message: 'Unable to process submission.' }) } }) @@ -210,13 +216,14 @@ exports.receiveEmailSubmissionUsingBusBoy = function (req, res, next) { meta: { action: 'receiveEmailSubmissionUsingBusBoy', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, }, error: err, }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send({ message: 'Unable to process submission.' }) + .json({ message: 'Unable to process submission.' }) }) req.pipe(busboy) @@ -246,17 +253,18 @@ exports.validateEmailSubmission = function (req, res, next) { meta: { action: 'validateEmailSubmission', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, }, error: err, }) if (err instanceof ConflictError) { - return res.status(err.status).send({ + return res.status(err.status).json({ message: 'The form has been updated. Please refresh and submit again.', }) } else { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'There is something wrong with your form submission. Please check your responses and try again. If the problem persists, please refresh the page.', }) @@ -502,12 +510,13 @@ function onSubmissionEmailFailure(err, req, res, submission) { meta: { action: 'onSubmissionEmailFailure', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Could not send submission. For assistance, please contact the person who asked you to fill in this form.', submissionId: submission._id, @@ -601,6 +610,7 @@ exports.saveMetadataToDb = function (req, res, next) { submissionId: submission.id, formId: form._id, ip: getRequestIp(req), + trace: getTrace(req), responseHash: submission.responseHash, }, }) @@ -643,6 +653,7 @@ exports.sendAdminEmail = async function (req, res, next) { submissionId: submission.id, formId: form._id, ip: getRequestIp(req), + trace: getTrace(req), submissionHash: submission.responseHash, }, }) @@ -665,6 +676,7 @@ exports.sendAdminEmail = async function (req, res, next) { submissionId: submission.id, formId: form._id, ip: getRequestIp(req), + trace: getTrace(req), submissionHash: submission.responseHash, }, error: err, diff --git a/src/app/controllers/encrypt-submissions.server.controller.js b/src/app/controllers/encrypt-submissions.server.controller.js index 74eb058f73..914c6628e9 100644 --- a/src/app/controllers/encrypt-submissions.server.controller.js +++ b/src/app/controllers/encrypt-submissions.server.controller.js @@ -15,7 +15,7 @@ const encryptSubmission = getEncryptSubmissionModel(mongoose) const { checkIsEncryptedEncoding } = require('../utils/encryption') const { ConflictError } = require('../modules/submission/submission.errors') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const { isMalformedDate, createQueryWithDateParam } = require('../utils/date') const logger = require('../../config/logger').createLoggerWithLabel(module) const { @@ -46,13 +46,14 @@ exports.validateEncryptSubmission = function (req, res, next) { meta: { action: 'validateEncryptSubmission', ip: getRequestIp(req), + trace: getTrace(req), formId: form._id, }, error, }) return res .status(StatusCodes.BAD_REQUEST) - .send({ message: 'Invalid data was found. Please submit again.' }) + .json({ message: 'Invalid data was found. Please submit again.' }) } if (req.body.responses) { @@ -65,17 +66,18 @@ exports.validateEncryptSubmission = function (req, res, next) { meta: { action: 'validateEncryptSubmission', ip: getRequestIp(req), + trace: getTrace(req), formId: form._id, }, error: err, }) if (err instanceof ConflictError) { - return res.status(err.status).send({ + return res.status(err.status).json({ message: 'The form has been updated. Please refresh and submit again.', }) } else { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'There is something wrong with your form submission. Please check your responses and try again. If the problem persists, please refresh the page.', }) @@ -113,12 +115,13 @@ function onEncryptSubmissionFailure(err, req, res, submission) { meta: { action: 'onEncryptSubmissionFailure', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Could not send submission. For assistance, please contact the person who asked you to fill in this form.', submissionId: submission._id, @@ -185,6 +188,7 @@ exports.saveResponseToDb = function (req, res, next) { message: 'Attachment upload error', meta: { action: 'saveResponseToDb', + trace: getTrace(req), }, error: err, }) @@ -246,17 +250,18 @@ exports.getMetadata = function (req, res) { meta: { action: 'getMetadata', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(HttpStatus.INTERNAL_SERVER_ERROR).send({ + return res.status(HttpStatus.INTERNAL_SERVER_ERROR).json({ message: errorHandler.getMongoErrorMessage(err), }) } if (!result) { - return res.status(HttpStatus.OK).send({ metadata: [], count: 0 }) + return res.status(HttpStatus.OK).json({ metadata: [], count: 0 }) } let entry = { number: 1, @@ -265,10 +270,10 @@ exports.getMetadata = function (req, res) { .tz('Asia/Singapore') .format('Do MMM YYYY, h:mm:ss a'), } - return res.status(HttpStatus.OK).send({ metadata: [entry], count: 1 }) + return res.status(HttpStatus.OK).json({ metadata: [entry], count: 1 }) }) } else { - return res.status(HttpStatus.OK).send({ metadata: [], count: 0 }) + return res.status(HttpStatus.OK).json({ metadata: [], count: 0 }) } } else { Submission.aggregate([ @@ -320,12 +325,13 @@ exports.getMetadata = function (req, res) { meta: { action: 'getMetadata', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errorHandler.getMongoErrorMessage(err), }) } else { @@ -344,7 +350,7 @@ exports.getMetadata = function (req, res) { number-- return entry }) - return res.status(StatusCodes.OK).send({ metadata, count }) + return res.status(StatusCodes.OK).json({ metadata, count }) } }) } @@ -379,12 +385,13 @@ exports.getEncryptedResponse = function (req, res) { meta: { action: 'getEncryptedResponse', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errorHandler.getMongoErrorMessage(err), }) } else { @@ -420,7 +427,7 @@ exports.streamEncryptedResponses = async function (req, res) { isMalformedDate(req.query.startDate) || isMalformedDate(req.query.endDate) ) { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Malformed date parameter', }) } @@ -455,10 +462,11 @@ exports.streamEncryptedResponses = async function (req, res) { meta: { action: 'streamEncryptedResponse', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(500).send({ + res.status(500).json({ message: 'Error retrieving from database.', }) }) @@ -471,10 +479,11 @@ exports.streamEncryptedResponses = async function (req, res) { meta: { action: 'streamEncryptedResponse', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(500).send({ + res.status(500).json({ message: 'Error converting submissions to JSON', }) }) @@ -485,10 +494,11 @@ exports.streamEncryptedResponses = async function (req, res) { meta: { action: 'streamEncryptedResponse', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - res.status(500).send({ + res.status(500).json({ message: 'Error writing submissions to HTTP stream', }) }) diff --git a/src/app/controllers/forms.server.controller.js b/src/app/controllers/forms.server.controller.js index 22c87ddf1c..e66925ff75 100644 --- a/src/app/controllers/forms.server.controller.js +++ b/src/app/controllers/forms.server.controller.js @@ -7,7 +7,7 @@ const mongoose = require('mongoose') const _ = require('lodash') const { StatusCodes } = require('http-status-codes') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) const getFormModel = require('../models/form.server.model').default @@ -75,7 +75,7 @@ exports.read = (requestType) => form = _.pick(form, formPublicFields) } - return res.send({ + return res.json({ form, spcpSession, myInfoError, @@ -95,20 +95,20 @@ exports.formById = async function (req, res, next) { let id = req.params && req.params.formId if (!mongoose.Types.ObjectId.isValid(id)) { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Form URL is invalid.', }) } try { const form = await Form.getFullFormById(id) if (!form) { - return res.status(StatusCodes.NOT_FOUND).send({ + return res.status(StatusCodes.NOT_FOUND).json({ message: "Oops! We can't find the form you're looking for.", }) } else { // Remove sensitive information from User object if (!form.admin) { - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: 'Server error.', }) } @@ -121,6 +121,7 @@ exports.formById = async function (req, res, next) { meta: { action: 'formById', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, diff --git a/src/app/controllers/myinfo.server.controller.js b/src/app/controllers/myinfo.server.controller.js index c290a28697..ad8592b921 100644 --- a/src/app/controllers/myinfo.server.controller.js +++ b/src/app/controllers/myinfo.server.controller.js @@ -11,7 +11,7 @@ const moment = require('moment') const { StatusCodes } = require('http-status-codes') const { sessionSecret } = require('../../config/config') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) const getMyInfoHashModel = require('../models/myinfo_hash.server.model').default const MyInfoHash = getMyInfoHashModel(mongoose) @@ -58,6 +58,7 @@ exports.addMyInfo = (myInfoService) => async (req, res, next) => { meta: { action: 'addMyInfo', ip: getRequestIp(req), + trace: getTrace(req), formId, esrvcId, }, @@ -110,6 +111,7 @@ exports.addMyInfo = (myInfoService) => async (req, res, next) => { meta: { action: 'addMyInfo', ip: getRequestIp(req), + trace: getTrace(req), formId, }, error: err, @@ -125,6 +127,7 @@ exports.addMyInfo = (myInfoService) => async (req, res, next) => { meta: { action: 'addMyInfo', ip: getRequestIp(req), + trace: getTrace(req), formId, }, error, @@ -192,10 +195,11 @@ exports.verifyMyInfoVals = function (req, res, next) { meta: { action: 'verifyMyInfoVals', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - return res.status(StatusCodes.SERVICE_UNAVAILABLE).send({ + return res.status(StatusCodes.SERVICE_UNAVAILABLE).json({ message: 'MyInfo verification unavailable, please try again later.', spcpSubmissionFailure: true, }) @@ -207,10 +211,11 @@ exports.verifyMyInfoVals = function (req, res, next) { meta: { action: 'verifyMyInfoVals', ip: getRequestIp(req), + trace: getTrace(req), formId: formObjId, }, }) - return res.status(StatusCodes.GONE).send({ + return res.status(StatusCodes.GONE).json({ message: 'MyInfo verification expired, please refresh and try again.', spcpSubmissionFailure: true, @@ -255,10 +260,11 @@ exports.verifyMyInfoVals = function (req, res, next) { meta: { action: 'verifyMyInfoVals', ip: getRequestIp(req), + trace: getTrace(req), failedFields: hashFailedAttrs, }, }) - return res.status(StatusCodes.UNAUTHORIZED).send({ + return res.status(StatusCodes.UNAUTHORIZED).json({ message: 'MyInfo verification failed.', spcpSubmissionFailure: true, }) diff --git a/src/app/controllers/public-forms.server.controller.js b/src/app/controllers/public-forms.server.controller.js index 6f1164b90c..f54cde7650 100644 --- a/src/app/controllers/public-forms.server.controller.js +++ b/src/app/controllers/public-forms.server.controller.js @@ -3,7 +3,7 @@ const mongoose = require('mongoose') const { StatusCodes } = require('http-status-codes') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) const getFormFeedbackModel = require('../models/form_feedback.server.model') .default @@ -24,7 +24,7 @@ exports.isFormPublic = function (req, res, next) { case 'ARCHIVED': return res.sendStatus(StatusCodes.GONE) default: - return res.status(StatusCodes.NOT_FOUND).send({ + return res.status(StatusCodes.NOT_FOUND).json({ message: req.form.inactiveMessage, isPageFound: true, // Flag to prevent default 404 subtext ("please check link") from showing formTitle: req.form.title, @@ -58,6 +58,7 @@ exports.redirect = async function (req, res) { message: 'Error fetching metatags', meta: { action: 'redirect', + trace: getTrace(req), }, error: err, }) @@ -81,7 +82,7 @@ exports.submitFeedback = function (req, res) { ) { return res .status(StatusCodes.BAD_REQUEST) - .send('Form feedback data not passed in') + .json({ message: 'Form feedback data not passed in' }) } FormFeedback.create( @@ -97,16 +98,17 @@ exports.submitFeedback = function (req, res) { meta: { action: 'submitFeedback', ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send('Form feedback could not be created') + .json({ message: 'Form feedback could not be created' }) } else { return res .status(StatusCodes.OK) - .send('Successfully submitted feedback') + .json({ message: 'Successfully submitted feedback' }) } }, ) diff --git a/src/app/controllers/spcp.server.controller.js b/src/app/controllers/spcp.server.controller.js index b60bfec07f..a44931d6f0 100644 --- a/src/app/controllers/spcp.server.controller.js +++ b/src/app/controllers/spcp.server.controller.js @@ -9,7 +9,7 @@ const crypto = require('crypto') const { StatusCodes } = require('http-status-codes') const axios = require('axios') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) const { mapDataToKey } = require('../../shared/util/verified-content') const getFormModel = require('../models/form.server.model').default @@ -112,7 +112,7 @@ const handleOOBAuthenticationWith = (ndiConfig, authType, extractUser) => { const payloads = String(relayState).split(',') if (payloads.length !== 2) { - return res.status(StatusCodes.BAD_REQUEST).send() + return res.sendStatus(StatusCodes.BAD_REQUEST) } const destination = payloads[0] @@ -126,7 +126,7 @@ const handleOOBAuthenticationWith = (ndiConfig, authType, extractUser) => { authType, ) ) { - res.status(StatusCodes.UNAUTHORIZED).send() + res.sendStatus(StatusCodes.UNAUTHORIZED) return } @@ -134,11 +134,11 @@ const handleOOBAuthenticationWith = (ndiConfig, authType, extractUser) => { samlArt = String(samlArt).replace(/ /g, '+') if (!destinationIsValid(destination)) - return res.status(StatusCodes.BAD_REQUEST).send() + return res.sendStatus(StatusCodes.BAD_REQUEST) getForm(destination, (err, form) => { if (err || !form || form.authType !== authType) { - res.status(StatusCodes.NOT_FOUND).send() + res.sendStatus(StatusCodes.NOT_FOUND) return } authClient.getAttributes(samlArt, destination, (err, data) => { @@ -148,6 +148,7 @@ const handleOOBAuthenticationWith = (ndiConfig, authType, extractUser) => { meta: { action: 'handleOOBAuthenticationWith', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, @@ -218,13 +219,15 @@ exports.createSpcpRedirectURL = (authClients) => { req.redirectURL = authClient.createRedirectURL(target, esrvcId) return next() } else { - return res.status(StatusCodes.BAD_REQUEST).send('Redirect URL malformed') + return res + .status(StatusCodes.BAD_REQUEST) + .json({ message: 'Redirect URL malformed' }) } } } exports.returnSpcpRedirectURL = function (req, res) { - return res.status(StatusCodes.OK).send({ redirectURL: req.redirectURL }) + return res.status(StatusCodes.OK).json({ redirectURL: req.redirectURL }) } const getSubstringBetween = (text, markerStart, markerEnd) => { @@ -260,16 +263,17 @@ exports.validateESrvcId = (req, res) => { message: 'Could not find title', meta: { action: 'validateESrvcId', + trace: getTrace(req), redirectUrl: redirectURL, data, }, }) - return res.status(StatusCodes.BAD_GATEWAY).send({ + return res.status(StatusCodes.BAD_GATEWAY).json({ message: 'Singpass returned incomprehensible content', }) } if (title.indexOf('Error') === -1) { - return res.status(StatusCodes.OK).send({ + return res.status(StatusCodes.OK).json({ isValid: true, }) } @@ -280,7 +284,7 @@ exports.validateESrvcId = (req, res) => { 'System Code: ', '', ) - return res.status(StatusCodes.OK).send({ + return res.status(StatusCodes.OK).json({ isValid: false, errorCode, }) @@ -291,12 +295,13 @@ exports.validateESrvcId = (req, res) => { message: 'Could not contact singpass to validate eservice id', meta: { action: 'validateESrvcId', + trace: getTrace(req), redirectUrl: redirectURL, statusCode, }, error: err, }) - return res.status(StatusCodes.SERVICE_UNAVAILABLE).send({ + return res.status(StatusCodes.SERVICE_UNAVAILABLE).json({ message: 'Failed to contact Singpass', }) }) @@ -353,6 +358,7 @@ exports.addSpcpSessionInfo = (authClients) => { meta: { action: 'addSpcpSessionInfo', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, @@ -415,12 +421,13 @@ exports.encryptedVerifiedFields = (signingSecretKey) => { action: 'encryptedVerifiedFields', formId: req.form._id, ip: getRequestIp(req), + trace: getTrace(req), }, error, }) return res .status(StatusCodes.BAD_REQUEST) - .send({ message: 'Invalid data was found. Please submit again.' }) + .json({ message: 'Invalid data was found. Please submit again.' }) } } } @@ -483,12 +490,13 @@ exports.isSpcpAuthenticated = (authClients) => { meta: { action: 'isSpcpAuthenticated', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - res.status(StatusCodes.UNAUTHORIZED).send({ + res.status(StatusCodes.UNAUTHORIZED).json({ message: 'User is not SPCP authenticated', spcpSubmissionFailure: true, }) diff --git a/src/app/controllers/submissions.server.controller.js b/src/app/controllers/submissions.server.controller.js index 68dc21ae31..10048e6cbb 100644 --- a/src/app/controllers/submissions.server.controller.js +++ b/src/app/controllers/submissions.server.controller.js @@ -9,7 +9,7 @@ const Submission = getSubmissionModel(mongoose) const { StatusCodes } = require('http-status-codes') -const { getRequestIp } = require('../utils/request') +const { getRequestIp, getTrace } = require('../utils/request') const { isMalformedDate, createQueryWithDateParam } = require('../utils/date') const logger = require('../../config/logger').createLoggerWithLabel(module) const MailService = require('../services/mail.service').default @@ -28,7 +28,7 @@ exports.captchaCheck = (captchaPrivateKey) => { return next() } else { if (!captchaPrivateKey) { - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: 'Captcha not set-up', }) } else if (!req.query.captchaResponse) { @@ -38,9 +38,10 @@ exports.captchaCheck = (captchaPrivateKey) => { action: 'captchaCheck', formId: req.form._id, ip: getRequestIp(req), + trace: getTrace(req), }, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Captcha was missing. Please refresh and submit again.', }) } else { @@ -60,9 +61,10 @@ exports.captchaCheck = (captchaPrivateKey) => { action: 'captchaCheck', formId: req.form._id, ip: getRequestIp(req), + trace: getTrace(req), }, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Captcha was incorrect. Please submit again.', }) } @@ -76,10 +78,11 @@ exports.captchaCheck = (captchaPrivateKey) => { action: 'captchaCheck', formId: req.form._id, ip: getRequestIp(req), + trace: getTrace(req), }, error: err, }) - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Could not verify captcha. Please submit again in a few minutes.', }) @@ -180,6 +183,7 @@ const sendEmailAutoReplies = async function (req) { meta: { action: 'sendEmailAutoReplies', ip: getRequestIp(req), + trace: getTrace(req), formId: req.form._id, submissionId: submission.id, }, @@ -204,7 +208,7 @@ exports.count = function (req, res) { isMalformedDate(req.query.startDate) || isMalformedDate(req.query.endDate) ) { - return res.status(StatusCodes.BAD_REQUEST).send({ + return res.status(StatusCodes.BAD_REQUEST).json({ message: 'Malformed date parameter', }) } @@ -223,13 +227,14 @@ exports.count = function (req, res) { meta: { action: 'count', ip: getRequestIp(req), + trace: getTrace(req), url: req.url, headers: req.headers, }, error: err, }) - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errorHandler.getMongoErrorMessage(err), }) } else { @@ -241,7 +246,7 @@ exports.count = function (req, res) { exports.sendAutoReply = function (req, res) { const { form, submission } = req // Return the reply early to the submitter - res.send({ + res.json({ message: 'Form submission successful.', submissionId: submission.id, }) diff --git a/src/app/factories/aggregate-stats.factory.js b/src/app/factories/aggregate-stats.factory.js index 4c8197290f..14b3112611 100644 --- a/src/app/factories/aggregate-stats.factory.js +++ b/src/app/factories/aggregate-stats.factory.js @@ -1,5 +1,4 @@ const featureManager = require('../../config/feature-manager').default -const core = require('../controllers/core.server.controller') const adminConsole = require('../controllers/admin-console.server.controller') const aggregStatsFactory = ({ isEnabled }) => { @@ -8,14 +7,12 @@ const aggregStatsFactory = ({ isEnabled }) => { getExampleForms: adminConsole.getExampleFormsUsingAggregateCollection, getSingleExampleForm: adminConsole.getSingleExampleFormUsingAggregateCollection, - formCount: core.formCountUsingAggregateCollection, } } else { return { getExampleForms: adminConsole.getExampleFormsUsingSubmissionsCollection, getSingleExampleForm: adminConsole.getSingleExampleFormUsingSubmissionCollection, - formCount: core.formCountUsingSubmissionsCollection, } } } diff --git a/src/app/factories/captcha.factory.js b/src/app/factories/captcha.factory.js deleted file mode 100644 index 0af3ebe274..0000000000 --- a/src/app/factories/captcha.factory.js +++ /dev/null @@ -1,23 +0,0 @@ -const featureManager = require('../../config/feature-manager').default -const submissions = require('../controllers/submissions.server.controller') -const { celebrate, Joi } = require('celebrate') - -const captchaFactory = ({ isEnabled, props }) => { - if (isEnabled && props && props.captchaPrivateKey) { - return { - captchaCheck: submissions.captchaCheck(props.captchaPrivateKey), - validateCaptcha: celebrate({ - query: Joi.object({ - captchaResponse: Joi.string().allow(null).required(), - }), - }), - } - } else { - return { - captchaCheck: (req, res, next) => next(), - validateCaptcha: (req, res, next) => next(), - } - } -} - -module.exports = captchaFactory(featureManager.get('captcha')) diff --git a/src/app/factories/captcha.factory.ts b/src/app/factories/captcha.factory.ts new file mode 100644 index 0000000000..dc666f3b4a --- /dev/null +++ b/src/app/factories/captcha.factory.ts @@ -0,0 +1,41 @@ +import { celebrate, Joi } from 'celebrate' +import { RequestHandler } from 'express' + +import FeatureManager, { + FeatureNames, + RegisteredFeature, +} from '../../config/feature-manager' +import { captchaCheck } from '../controllers/submissions.server.controller' + +// TODO(#144): Migrate middlewares and request handlers to the controller layer +interface ICaptchaFactory { + captchaCheck: RequestHandler + validateCaptcha: RequestHandler +} + +const captchaFeature = FeatureManager.get(FeatureNames.Captcha) + +const createCaptchaFactory = ({ + isEnabled, + props, +}: RegisteredFeature): ICaptchaFactory => { + // Feature is enabled and valid. + if (isEnabled && props?.captchaPrivateKey) { + return { + captchaCheck: captchaCheck(props.captchaPrivateKey) as RequestHandler, + validateCaptcha: celebrate({ + query: Joi.object({ + captchaResponse: Joi.string().allow(null).required(), + }), + }), + } + } + + // Not enabled or invalid props. + return { + captchaCheck: (_req, _res, next) => next(), + validateCaptcha: (_req, _res, next) => next(), + } +} + +export const CaptchaFactory = createCaptchaFactory(captchaFeature) diff --git a/src/app/factories/google-analytics.factory.js b/src/app/factories/google-analytics.factory.js index 1f83684250..0e0a924481 100644 --- a/src/app/factories/google-analytics.factory.js +++ b/src/app/factories/google-analytics.factory.js @@ -10,7 +10,7 @@ const googleAnalyticsFactory = ({ isEnabled }) => { } else { return { datalayer: (req, res) => { - res.type('text/javascript').status(StatusCodes.OK).send() + res.type('text/javascript').sendStatus(StatusCodes.OK) }, } } diff --git a/src/app/factories/sms.factory.js b/src/app/factories/sms.factory.js deleted file mode 100644 index fd07ba659e..0000000000 --- a/src/app/factories/sms.factory.js +++ /dev/null @@ -1,48 +0,0 @@ -const twilio = require('twilio') -const featureManager = require('../../config/feature-manager').default -const smsService = require('../services/sms.service') - -const smsFactory = ({ isEnabled, props }) => { - let twilioClient - if (isEnabled && props) { - twilioClient = twilio(props.twilioApiKey, props.twilioApiSecret, { - accountSid: props.twilioAccountSid, - }) - } - - let twilioConfig = { - msgSrvcSid: props && props.twilioMsgSrvcSid, - client: twilioClient, - } - - return { - async sendVerificationOtp(recipient, otp, formId) { - if (isEnabled) { - return smsService.sendVerificationOtp( - recipient, - otp, - formId, - twilioConfig, - ) - } else { - throw new Error( - `Verification OTP has not been configured to be sent for mobile fields`, - ) - } - }, - async sendAdminContactOtp(recipient, otp, userId) { - if (isEnabled) { - return smsService.sendAdminContactOtp( - recipient, - otp, - userId, - twilioConfig, - ) - } else { - throw new Error(`Send Admin Contact OTP has not been enabled`) - } - }, - } -} - -module.exports = smsFactory(featureManager.get('sms')) diff --git a/src/app/factories/spcp-myinfo.factory.js b/src/app/factories/spcp-myinfo.factory.js index dc2d672d79..50e732cfe7 100644 --- a/src/app/factories/spcp-myinfo.factory.js +++ b/src/app/factories/spcp-myinfo.factory.js @@ -133,22 +133,22 @@ const spcpFactory = ({ isEnabled, props }) => { encryptedVerifiedFields: (req, res, next) => next(), passThroughSpcp: (req, res, next) => next(), getLoginStats: (req, res) => - res.send({ + res.json({ loginStats: [], }), verifyMyInfoVals: (req, res, next) => next(), returnSpcpRedirectURL: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), singPassLogin: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), corpPassLogin: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), addSpcpSessionInfo: (req, res, next) => next(), isSpcpAuthenticated: (req, res, next) => next(), createSpcpRedirectURL: (req, res, next) => next(), addMyInfo: (req, res, next) => next(), validateESrvcId: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), } } } diff --git a/src/app/models/field/emailField.ts b/src/app/models/field/emailField.ts index 7d00543b91..f2793282fd 100644 --- a/src/app/models/field/emailField.ts +++ b/src/app/models/field/emailField.ts @@ -1,9 +1,9 @@ -import { isEmpty } from 'lodash' import { Schema } from 'mongoose' +import { validateEmailDomains } from '../../../shared/util/email-domain-validation' import { IEmailFieldSchema, ResponseMode } from '../../../types' -const createEmailFieldSchema = () => { +const createEmailFieldSchema = (): Schema => { const EmailFieldSchema = new Schema({ autoReplyOptions: { hasAutoReply: { @@ -49,19 +49,15 @@ const createEmailFieldSchema = () => { { type: String, trim: true, - match: [/.+\..+/, 'There are one or more invalid email domains.'], }, ], - // If there allowedEmailDomains is empty, then all email domains should be allowed. + // If allowedEmailDomains is empty, then all email domains should be allowed. default: [], validate: { - validator: (emailDomains: string[]) => { - return ( - isEmpty(emailDomains) || - new Set(emailDomains).size === emailDomains.length - ) + validator: (emailDomains: string[]): boolean => { + return validateEmailDomains(emailDomains) }, - message: 'There are one or more duplicate email domains.', + message: 'There are one or more duplicate or invalid email domains.', }, }, }) diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts index 5beb51b95a..9537330ecf 100644 --- a/src/app/models/form.server.model.ts +++ b/src/app/models/form.server.model.ts @@ -92,7 +92,7 @@ export interface IFormModel extends Model { deactivateById(formId: string): Promise } -type IEncryptedFormModel = Model +type IEncryptedFormModel = Model & IFormModel const EncryptedFormSchema = new Schema({ publicKey: { @@ -101,7 +101,31 @@ const EncryptedFormSchema = new Schema({ }, }) -type IEmailFormModel = Model +type IEmailFormModel = Model & IFormModel + +// Converts 'test@hotmail.com, test@gmail.com' to ['test@hotmail.com', 'test@gmail.com'] +function transformEmailString(v: string): string[] { + return v + .split(',') + .map((item) => item.trim().toLowerCase()) + .filter((email) => email.includes('@')) // remove "" +} + +// Function that coerces the string of comma-separated emails sent by the client +// into an array of emails +function transformEmails(v: string | string[]): string[] { + // Cases + // ['test@hotmail.com'] => ['test@hotmail.com'] ~ unchanged + // ['test@hotmail.com', 'test@gmail.com'] => ['test@hotmail.com', 'test@gmail.com'] ~ unchanged + // ['test@hotmail.com, test@gmail.com'] => ['test@hotmail.com', 'test@gmail.com'] + // ['test@hotmail.com, test@gmail.com', 'test@yahoo.com'] => ['test@hotmail.com', 'test@gmail.com', 'test@yahoo.com'] + // 'test@hotmail.com, test@gmail.com' => ['test@hotmail.com', 'test@gmail.com'] + if (Array.isArray(v)) { + return transformEmailString(v.join(',')) + } else { + return transformEmailString(v) + } +} const EmailFormSchema = new Schema({ emails: { @@ -111,18 +135,12 @@ const EmailFormSchema = new Schema({ trim: true, }, ], + set: transformEmails, validate: { validator: (v: string[]) => { - if (!Array.isArray(v) || v.length === 0) return false - // Weird artifact of legacy code, emails are mostly a single - // string of emails separated by commas. - // Split the email strings into individual emails by commas (if - // possible) and validate them. - return v.every((emailString) => - emailString - .split(',') - .every((email) => validator.isEmail(email.trim())), - ) + if (!Array.isArray(v)) return false + if (v.length === 0) return false + return v.every((email) => validator.isEmail(email)) }, message: 'Please provide valid email addresses', }, @@ -484,7 +502,7 @@ const compileFormModel = (db: Mongoose): IFormModel => { } // Hooks - FormSchema.pre('validate', function (next) { + FormSchema.pre('validate', async function (next) { // Reject save if form document is too large if (bson.calculateObjectSize(this) > 10 * MB) { const err = new Error('Form size exceeded.') @@ -493,7 +511,7 @@ const compileFormModel = (db: Mongoose): IFormModel => { } // Validate that admin exists before form is created. - User.findById(this.admin, function (error, admin) { + await User.findById(this.admin, function (error, admin) { if (error) { return next(Error(`Error validating admin for form.`)) } diff --git a/src/app/models/form_statistics_total.server.model.ts b/src/app/models/form_statistics_total.server.model.ts index d16081e965..0c14b7a0d1 100644 --- a/src/app/models/form_statistics_total.server.model.ts +++ b/src/app/models/form_statistics_total.server.model.ts @@ -1,15 +1,24 @@ -import { Model, Mongoose, Schema } from 'mongoose' +import { Mongoose, Schema } from 'mongoose' -import { IFormStatisticsTotalSchema } from '../../types' +import { + AggregateFormCountResult, + IFormStatisticsTotalModel, + IFormStatisticsTotalSchema, +} from '../../types' + +import { FORM_SCHEMA_ID } from './form.server.model' const FORM_STATS_TOTAL_SCHEMA_ID = 'FormStatisticsTotal' const FORM_STATS_COLLECTION_NAME = 'formStatisticsTotal' -type IFormStatisticsTotalModel = Model - const compileFormStatisticsTotalModel = (db: Mongoose) => { const FormStatisticsTotalSchema = new Schema( { + formId: { + type: Schema.Types.ObjectId, + ref: FORM_SCHEMA_ID, + required: true, + }, totalCount: { type: Number, required: true, @@ -37,6 +46,25 @@ const compileFormStatisticsTotalModel = (db: Mongoose) => { }, ) + // Static functions + FormStatisticsTotalSchema.statics.aggregateFormCount = function ( + this: IFormStatisticsTotalModel, + minSubCount: number, + ): Promise { + return this.aggregate([ + { + $match: { + totalCount: { + $gt: minSubCount, + }, + }, + }, + { + $count: 'numActiveForms', + }, + ]).exec() + } + const FormStatisticsTotalModel = db.model< IFormStatisticsTotalSchema, IFormStatisticsTotalModel @@ -55,7 +83,9 @@ const compileFormStatisticsTotalModel = (db: Mongoose) => { * @param db The mongoose instance to retrieve the FormStatisticsTotal model from * @returns The FormStatisticsTotal model */ -const getFormStatisticsTotalModel = (db: Mongoose) => { +const getFormStatisticsTotalModel = ( + db: Mongoose, +): IFormStatisticsTotalModel => { try { return db.model(FORM_STATS_TOTAL_SCHEMA_ID) as IFormStatisticsTotalModel } catch { diff --git a/src/app/models/submission.server.model.ts b/src/app/models/submission.server.model.ts index 1ce4759c8c..614cd70464 100644 --- a/src/app/models/submission.server.model.ts +++ b/src/app/models/submission.server.model.ts @@ -1,11 +1,13 @@ -import { Model, Mongoose, Schema } from 'mongoose' +import { Mongoose, Schema } from 'mongoose' import { AuthType, + FindFormsWithSubsAboveResult, IEmailSubmissionModel, IEmailSubmissionSchema, IEncryptedSubmissionSchema, IEncryptSubmissionModel, + ISubmissionModel, ISubmissionSchema, IWebhookResponseSchema, MyInfoAttribute, @@ -56,9 +58,29 @@ SubmissionSchema.index({ created: -1, }) -// Instance methods +// Base schema static methods +SubmissionSchema.statics.findFormsWithSubsAbove = function ( + this: ISubmissionModel, + minSubCount: number, +): Promise { + return this.aggregate([ + { + $group: { + _id: '$form', + count: { $sum: 1 }, + }, + }, + { + $match: { + count: { + $gt: minSubCount, + }, + }, + }, + ]).exec() +} -const emailSubmissionSchema = new Schema({ +const EmailSubmissionSchema = new Schema({ recipientEmails: { type: [ { @@ -83,10 +105,11 @@ const emailSubmissionSchema = new Schema({ }, }) +// EmailSubmission Instance methods /** * Returns null as email submission does not have a webhook view */ -emailSubmissionSchema.methods.getWebhookView = function (): null { +EmailSubmissionSchema.methods.getWebhookView = function (): null { return null } @@ -110,7 +133,7 @@ const webhookResponseSchema = new Schema( }, ) -const encryptSubmissionSchema = new Schema({ +const EncryptSubmissionSchema = new Schema({ encryptedContent: { type: String, trim: true, @@ -135,7 +158,7 @@ const encryptSubmissionSchema = new Schema({ * Returns an object which represents the encrypted submission * which will be posted to the webhook URL. */ -encryptSubmissionSchema.methods.getWebhookView = function ( +EncryptSubmissionSchema.methods.getWebhookView = function ( this: IEncryptedSubmissionSchema, ): WebhookView { const webhookData: WebhookData = { @@ -152,27 +175,34 @@ encryptSubmissionSchema.methods.getWebhookView = function ( } } -const compileSubmissionModel = (db: Mongoose) => { +const compileSubmissionModel = (db: Mongoose): ISubmissionModel => { const Submission = db.model('Submission', SubmissionSchema) - Submission.discriminator(SubmissionType.Email, emailSubmissionSchema) - Submission.discriminator(SubmissionType.Encrypt, encryptSubmissionSchema) - return db.model(SUBMISSION_SCHEMA_ID, SubmissionSchema) + Submission.discriminator(SubmissionType.Email, EmailSubmissionSchema) + Submission.discriminator(SubmissionType.Encrypt, EncryptSubmissionSchema) + return db.model( + SUBMISSION_SCHEMA_ID, + SubmissionSchema, + ) as ISubmissionModel } -const getSubmissionModel = (db: Mongoose) => { +const getSubmissionModel = (db: Mongoose): ISubmissionModel => { try { - return db.model(SUBMISSION_SCHEMA_ID) as Model + return db.model(SUBMISSION_SCHEMA_ID) as ISubmissionModel } catch { return compileSubmissionModel(db) } } -export const getEmailSubmissionModel = (db: Mongoose) => { +export const getEmailSubmissionModel = ( + db: Mongoose, +): IEmailSubmissionModel => { getSubmissionModel(db) return db.model(SubmissionType.Email) as IEmailSubmissionModel } -export const getEncryptSubmissionModel = (db: Mongoose) => { +export const getEncryptSubmissionModel = ( + db: Mongoose, +): IEncryptSubmissionModel => { getSubmissionModel(db) return db.model(SubmissionType.Encrypt) as IEncryptSubmissionModel } diff --git a/src/app/modules/analytics/__tests__/analytics.controller.spec.ts b/src/app/modules/analytics/__tests__/analytics.controller.spec.ts new file mode 100644 index 0000000000..b0319857d9 --- /dev/null +++ b/src/app/modules/analytics/__tests__/analytics.controller.spec.ts @@ -0,0 +1,130 @@ +import { DatabaseError } from 'dist/backend/app/modules/core/core.errors' +import { errAsync, okAsync } from 'neverthrow' +import expressHandler from 'tests/unit/backend/helpers/jest-express' + +import * as AnalyticsController from '../analytics.controller' +import { AnalyticsFactory } from '../analytics.factory' +import * as AnalyticsService from '../analytics.service' + +describe('analytics.controller', () => { + const MOCK_REQ = expressHandler.mockRequest() + afterEach(() => jest.clearAllMocks()) + + describe('handleGetUserCount', () => { + it('should return 200 with number of users on success', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + const mockUserCount = 21 + const getUserSpy = jest + .spyOn(AnalyticsService, 'getUserCount') + .mockReturnValueOnce(okAsync(mockUserCount)) + + // Act + await AnalyticsController.handleGetUserCount(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(getUserSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).not.toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith(mockUserCount) + }) + + it('should return 500 when error occurs whilst retrieving user count', async () => { + const mockRes = expressHandler.mockResponse() + const getUserSpy = jest + .spyOn(AnalyticsService, 'getUserCount') + .mockReturnValueOnce(errAsync(new DatabaseError())) + + // Act + await AnalyticsController.handleGetUserCount(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(getUserSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).toHaveBeenCalledWith(500) + expect(mockRes.json).toHaveBeenCalledWith( + 'Unable to retrieve number of users from the database', + ) + }) + }) + + describe('handleGetSubmissionCount', () => { + it('should return 200 with number of submissions on success', async () => { + // Arrange + const mockSubmissionCount = 1234 + const mockRes = expressHandler.mockResponse() + const getSubsSpy = jest + .spyOn(AnalyticsService, 'getSubmissionCount') + .mockReturnValueOnce(okAsync(mockSubmissionCount)) + + // Act + await AnalyticsController.handleGetSubmissionCount( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(getSubsSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).not.toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith(mockSubmissionCount) + }) + + it('should return 500 when error occurs whilst retrieving submission count', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + const getSubsSpy = jest + .spyOn(AnalyticsService, 'getSubmissionCount') + .mockReturnValueOnce(errAsync(new DatabaseError())) + + // Act + await AnalyticsController.handleGetSubmissionCount( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(getSubsSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).toHaveBeenCalledWith(500) + expect(mockRes.json).toHaveBeenCalledWith( + 'Unable to retrieve number of submissions from the database', + ) + }) + }) + + describe('handleGetFormCount', () => { + it('should return 200 with number of forms on success', async () => { + // Arrange + const mockFormCount = 99543 + const mockRes = expressHandler.mockResponse() + const getFormSpy = jest + .spyOn(AnalyticsFactory, 'getFormCount') + .mockReturnValueOnce(okAsync(mockFormCount)) + + // Act + await AnalyticsController.handleGetFormCount(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(getFormSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).not.toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith(mockFormCount) + }) + + it('should return 500 when error occurs whilst retrieving form count', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + const getFormSpy = jest + .spyOn(AnalyticsFactory, 'getFormCount') + .mockReturnValueOnce(errAsync(new DatabaseError())) + + // Act + await AnalyticsController.handleGetFormCount(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(getFormSpy).toHaveBeenCalledTimes(1) + expect(mockRes.status).toHaveBeenCalledWith(500) + expect(mockRes.json).toHaveBeenCalledWith( + 'Unable to retrieve number of forms from the database', + ) + }) + }) +}) diff --git a/src/app/modules/analytics/__tests__/analytics.factory.spec.ts b/src/app/modules/analytics/__tests__/analytics.factory.spec.ts new file mode 100644 index 0000000000..53a4a6de9e --- /dev/null +++ b/src/app/modules/analytics/__tests__/analytics.factory.spec.ts @@ -0,0 +1,68 @@ +import { okAsync } from 'neverthrow' +import { mocked } from 'ts-jest/utils' + +import { + FeatureNames, + IAggregateStats, + RegisteredFeature, +} from 'src/config/feature-manager' + +import { createAnalyticsFactory } from '../analytics.factory' +import * as AnalyticsService from '../analytics.service' + +jest.mock('../analytics.service') +const MockAnalyticsService = mocked(AnalyticsService) + +describe('analytics.factory', () => { + describe('aggregate-stats feature is enabled', () => { + const MOCK_ENABLED_FEATURE: RegisteredFeature = { + isEnabled: true, + props: {} as IAggregateStats, + } + const AnalyticsFactory = createAnalyticsFactory(MOCK_ENABLED_FEATURE) + + describe('getFormCount', () => { + it('should invoke AnalyticsService#getFormCountWithStatsCollection', async () => { + // Arrange + const mockFormCount = 200 + const serviceStatsSpy = MockAnalyticsService.getFormCountWithStatsCollection.mockReturnValue( + okAsync(mockFormCount), + ) + + // Act + const actualResults = await AnalyticsFactory.getFormCount() + + // Assert + expect(serviceStatsSpy).toHaveBeenCalledTimes(1) + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual(mockFormCount) + }) + }) + }) + + describe('aggregate-stats feature is disabled', () => { + const MOCK_DISABLED_FEATURE: RegisteredFeature = { + isEnabled: false, + props: {} as IAggregateStats, + } + const AnalyticsFactory = createAnalyticsFactory(MOCK_DISABLED_FEATURE) + + describe('getFormCount', () => { + it('should invoke AnalyticsService#getFormCountWithSubmissionCollection', async () => { + // Arrange + const mockFormCount = 1 + const serviceSubmissionSpy = MockAnalyticsService.getFormCountWithSubmissionCollection.mockReturnValue( + okAsync(mockFormCount), + ) + + // Act + const actualResults = await AnalyticsFactory.getFormCount() + + // Assert + expect(serviceSubmissionSpy).toHaveBeenCalledTimes(1) + expect(actualResults.isOk()).toEqual(true) + expect(actualResults._unsafeUnwrap()).toEqual(mockFormCount) + }) + }) + }) +}) diff --git a/src/app/modules/analytics/__tests__/analytics.service.spec.ts b/src/app/modules/analytics/__tests__/analytics.service.spec.ts new file mode 100644 index 0000000000..a71c333028 --- /dev/null +++ b/src/app/modules/analytics/__tests__/analytics.service.spec.ts @@ -0,0 +1,295 @@ +import { times } from 'lodash' +import mongoose, { Query } from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import getFormStatisticsTotalModel from 'src/app/models/form_statistics_total.server.model' +import getSubmissionModel from 'src/app/models/submission.server.model' +import getUserModel from 'src/app/models/user.server.model' +import { + IAgencySchema, + IFormStatisticsTotalSchema, + ISubmissionSchema, + SubmissionType, +} from 'src/types' + +import { DatabaseError } from '../../core/core.errors' +import { MIN_SUB_COUNT } from '../analytics.constants' +import { + getFormCountWithStatsCollection, + getFormCountWithSubmissionCollection, + getSubmissionCount, + getUserCount, +} from '../analytics.service' + +const FormStatsModel = getFormStatisticsTotalModel(mongoose) +const SubmissionModel = getSubmissionModel(mongoose) +const UserModel = getUserModel(mongoose) + +describe('analytics.service', () => { + beforeAll(async () => await dbHandler.connect()) + beforeEach(async () => { + await dbHandler.clearDatabase() + jest.restoreAllMocks() + }) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('getFormCountWithStatsCollection', () => { + it('should return the number of forms with more than 10 submissions when such forms exists', async () => { + // Arrange + // Number of submissions per form + const formCounts = [12, 10, 4] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + submissionPromises.push( + FormStatsModel.create({ + formId: mongoose.Types.ObjectId(), + totalCount: count, + lastSubmission: new Date(), + }), + ) + }) + await Promise.all(submissionPromises) + + // Act + const actualResult = await getFormCountWithStatsCollection() + + // Assert + const expectedResult = formCounts.filter((fc) => fc > MIN_SUB_COUNT) + .length + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(expectedResult) + }) + + it('should return 0 when no forms have above 10 submissions', async () => { + // Arrange + // Number of submissions per form + const formCounts = [1, 2] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + submissionPromises.push( + FormStatsModel.create({ + formId: mongoose.Types.ObjectId(), + totalCount: count, + lastSubmission: new Date(), + }), + ) + }) + await Promise.all(submissionPromises) + + // Act + const actualResult = await getFormCountWithStatsCollection() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(0) + }) + + it('should return DatabaseError when error occurs whilst querying database', async () => { + // Arrange + const aggregateSpy = jest + .spyOn(FormStatsModel, 'aggregateFormCount') + .mockRejectedValueOnce(new Error('some error')) + + // Act + const actualResult = await getFormCountWithStatsCollection() + + // Assert + expect(aggregateSpy).toHaveBeenCalled() + expect(actualResult.isErr()).toEqual(true) + expect(actualResult._unsafeUnwrapErr()).toEqual(new DatabaseError()) + }) + }) + + describe('getFormCountWithSubmissionCollection', () => { + it('should return the number of forms with more than 10 submissions when such forms exists', async () => { + // Arrange + const formCounts = [12, 10, 4] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + const formId = mongoose.Types.ObjectId() + times(count, () => + submissionPromises.push( + SubmissionModel.create({ + form: formId, + myInfoFields: [], + submissionType: SubmissionType.Email, + responseHash: 'hash', + responseSalt: 'salt', + }), + ), + ) + }) + await Promise.all(submissionPromises) + + // Act + const actualResult = await getFormCountWithSubmissionCollection() + + // Assert + const expectedResult = formCounts.filter((fc) => fc > MIN_SUB_COUNT) + .length + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(expectedResult) + }) + + it('should return 0 when no forms have above 10 submissions', async () => { + // Arrange + const formCounts = [1, 2] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + const formId = mongoose.Types.ObjectId() + times(count, () => + submissionPromises.push( + SubmissionModel.create({ + form: formId, + myInfoFields: [], + submissionType: SubmissionType.Email, + responseHash: 'hash', + responseSalt: 'salt', + }), + ), + ) + }) + await Promise.all(submissionPromises) + + // Act + const actualResult = await getFormCountWithSubmissionCollection() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(0) + }) + + it('should return DatabaseError when error occurs whilst querying database', async () => { + // Arrange + const aggregateSpy = jest + .spyOn(SubmissionModel, 'findFormsWithSubsAbove') + .mockRejectedValueOnce(new Error('some error')) + + // Act + const actualResult = await getFormCountWithSubmissionCollection() + + // Assert + expect(aggregateSpy).toHaveBeenCalled() + expect(actualResult.isErr()).toEqual(true) + expect(actualResult._unsafeUnwrapErr()).toEqual(new DatabaseError()) + }) + }) + + describe('getUserCount', () => { + const VALID_DOMAIN = 'example.com' + let testAgency: IAgencySchema + + beforeEach(async () => { + testAgency = await dbHandler.insertDefaultAgency({ + mailDomain: VALID_DOMAIN, + }) + }) + + it('should return 0 when there are no users in the database', async () => { + // Arrange + const initialUserCount = await UserModel.estimatedDocumentCount() + expect(initialUserCount).toEqual(0) + + // Act + const actualResult = await getUserCount() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(0) + }) + + it('should return number of users in the database', async () => { + // Arrange + const expectedNumUsers = 10 + const userPromises = times(expectedNumUsers, () => + UserModel.create({ + agency: testAgency._id, + email: `${Math.random()}@${VALID_DOMAIN}`, + }), + ) + await Promise.all(userPromises) + const initialUserCount = await UserModel.estimatedDocumentCount() + expect(initialUserCount).toEqual(expectedNumUsers) + + // Act + const actualResult = await getUserCount() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(expectedNumUsers) + }) + + it('should return DatabaseError when error occurs whilst retrieving user count', async () => { + // Arrange + const execSpy = jest.fn().mockRejectedValueOnce(new Error('boom')) + jest.spyOn(UserModel, 'estimatedDocumentCount').mockReturnValueOnce(({ + exec: execSpy, + } as unknown) as Query) + + // Act + const actualResult = await getUserCount() + + // Assert + expect(execSpy).toHaveBeenCalledTimes(1) + expect(actualResult.isErr()).toEqual(true) + expect(actualResult._unsafeUnwrapErr()).toEqual(new DatabaseError()) + }) + }) + + describe('getSubmissionCount', () => { + it('should return 0 when there are no submissions in the database', async () => { + // Arrange + const initialSubCount = await SubmissionModel.estimatedDocumentCount() + expect(initialSubCount).toEqual(0) + + // Act + const actualResult = await getSubmissionCount() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(0) + }) + + it('should return number of submissions in the database', async () => { + // Arrange + const expectedNumSubs = 10 + const submissionPromises = times(expectedNumSubs, () => + SubmissionModel.create({ + form: mongoose.Types.ObjectId(), + myInfoFields: [], + submissionType: SubmissionType.Email, + responseHash: 'hash', + responseSalt: 'salt', + }), + ) + await Promise.all(submissionPromises) + const initialUserCount = await SubmissionModel.estimatedDocumentCount() + expect(initialUserCount).toEqual(expectedNumSubs) + + // Act + const actualResult = await getSubmissionCount() + + // Assert + expect(actualResult.isOk()).toEqual(true) + expect(actualResult._unsafeUnwrap()).toEqual(expectedNumSubs) + }) + + it('should return DatabaseError when error occurs whilst retrieving submission count', async () => { + // Arrange + const execSpy = jest.fn().mockRejectedValueOnce(new Error('boom')) + jest + .spyOn(SubmissionModel, 'estimatedDocumentCount') + .mockReturnValueOnce(({ + exec: execSpy, + } as unknown) as Query) + + // Act + const actualResult = await getSubmissionCount() + + // Assert + expect(execSpy).toHaveBeenCalledTimes(1) + expect(actualResult.isErr()).toEqual(true) + expect(actualResult._unsafeUnwrapErr()).toEqual(new DatabaseError()) + }) + }) +}) diff --git a/src/app/modules/analytics/analytics.constants.ts b/src/app/modules/analytics/analytics.constants.ts new file mode 100644 index 0000000000..6d7e0de435 --- /dev/null +++ b/src/app/modules/analytics/analytics.constants.ts @@ -0,0 +1,4 @@ +/** + * Minimum number of submissions before search form is returned + */ +export const MIN_SUB_COUNT = 10 diff --git a/src/app/modules/analytics/analytics.controller.ts b/src/app/modules/analytics/analytics.controller.ts new file mode 100644 index 0000000000..5caec017f1 --- /dev/null +++ b/src/app/modules/analytics/analytics.controller.ts @@ -0,0 +1,104 @@ +import { RequestHandler } from 'express' +import { StatusCodes } from 'http-status-codes' + +import { submissionsTopUp } from '../../../config/config' +import { createLoggerWithLabel } from '../../../config/logger' +import { getRequestIp, getTrace } from '../../utils/request' + +import { AnalyticsFactory } from './analytics.factory' +import { getSubmissionCount, getUserCount } from './analytics.service' + +const logger = createLoggerWithLabel(module) + +/** + * Handler for GET /analytics/users + * @route GET /analytics/users + * @returns 200 with the number of users building forms + * @returns 500 when database error occurs whilst retrieving user count + */ +export const handleGetUserCount: RequestHandler = async (req, res) => { + const countResult = await getUserCount() + + if (countResult.isErr()) { + logger.error({ + message: 'Mongo user count error', + meta: { + action: 'handleGetUserCount', + ip: getRequestIp(req), + url: req.url, + headers: req.headers, + trace: getTrace(req), + }, + error: countResult.error, + }) + + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .json('Unable to retrieve number of users from the database') + } + + return res.json(countResult.value) +} + +/** + * Handler for GET /analytics/submissions + * @route GET /analytics/submissions + * @returns 200 with the number of submissions across forms + * @returns 500 when database error occurs whilst retrieving submissions count + */ +export const handleGetSubmissionCount: RequestHandler = async (req, res) => { + const countResult = await getSubmissionCount() + + if (countResult.isErr()) { + logger.error({ + message: 'Mongo submissions count error', + meta: { + action: 'handleGetSubmissionCount', + ip: getRequestIp(req), + url: req.url, + headers: req.headers, + trace: getTrace(req), + }, + error: countResult.error, + }) + + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .json('Unable to retrieve number of submissions from the database') + } + + // Top up submissions from config file that tracks submissions that has been + // archived (and thus deleted from the database). + const totalProperCount = countResult.value + submissionsTopUp + return res.json(totalProperCount) +} + +/** + * Handler for GET /analytics/forms + * @route GET /analytics/forms + * @returns 200 with the number of popular forms on the application + * @returns 500 when database error occurs whilst retrieving form count + */ +export const handleGetFormCount: RequestHandler = async (req, res) => { + const countResult = await AnalyticsFactory.getFormCount() + + if (countResult.isErr()) { + logger.error({ + message: 'Mongo form count error', + meta: { + action: 'handleGetFormCount', + ip: getRequestIp(req), + url: req.url, + headers: req.headers, + trace: getTrace(req), + }, + error: countResult.error, + }) + + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .json('Unable to retrieve number of forms from the database') + } + + return res.json(countResult.value) +} diff --git a/src/app/modules/analytics/analytics.factory.ts b/src/app/modules/analytics/analytics.factory.ts new file mode 100644 index 0000000000..651adc8142 --- /dev/null +++ b/src/app/modules/analytics/analytics.factory.ts @@ -0,0 +1,37 @@ +import { ResultAsync } from 'neverthrow' + +import FeatureManager, { + FeatureNames, + RegisteredFeature, +} from '../../../config/feature-manager' +import { DatabaseError } from '../core/core.errors' + +import { + getFormCountWithStatsCollection, + getFormCountWithSubmissionCollection, +} from './analytics.service' + +interface IAnalyticsFactory { + getFormCount: () => ResultAsync +} + +const aggregateFeature = FeatureManager.get(FeatureNames.AggregateStats) + +// Exported for testing. +export const createAnalyticsFactory = ({ + isEnabled, + props, +}: RegisteredFeature): IAnalyticsFactory => { + if (isEnabled && props) { + return { + getFormCount: getFormCountWithStatsCollection, + } + } + + // Not enabled, return retrieve forms with submissions collection + return { + getFormCount: getFormCountWithSubmissionCollection, + } +} + +export const AnalyticsFactory = createAnalyticsFactory(aggregateFeature) diff --git a/src/app/modules/analytics/analytics.routes.ts b/src/app/modules/analytics/analytics.routes.ts new file mode 100644 index 0000000000..cfebdc7b40 --- /dev/null +++ b/src/app/modules/analytics/analytics.routes.ts @@ -0,0 +1,33 @@ +import { Router } from 'express' + +import * as AnalyticsController from './analytics.controller' + +export const AnalyticsRouter = Router() + +/** + * Retrieves the number of popular forms on the application + * @route GET /analytics/forms + * @group analytics - form usage statistics + * @returns 200 with the number of forms with more than 10 submissions + */ +AnalyticsRouter.get('/forms', AnalyticsController.handleGetFormCount) + +/** + * Retrieves the number of users building forms on the application. + * @route GET /analytics/users + * @group analytics - form usage statistics + * @returns 200 with the number of users building forms + * @returns 500 when database error occurs whilst retrieving user count + */ +AnalyticsRouter.get('/users', AnalyticsController.handleGetUserCount) + +/** + * Retrieves the total number of submissions of forms across the application. + * @route GET /analytics/submissions + * @group analytics - form usage statistics + * @returns 200 with the total number of submissions of forms + */ +AnalyticsRouter.get( + '/submissions', + AnalyticsController.handleGetSubmissionCount, +) diff --git a/src/app/modules/analytics/analytics.service.ts b/src/app/modules/analytics/analytics.service.ts new file mode 100644 index 0000000000..231fc53a05 --- /dev/null +++ b/src/app/modules/analytics/analytics.service.ts @@ -0,0 +1,125 @@ +import mongoose from 'mongoose' +import { ResultAsync } from 'neverthrow' + +import { createLoggerWithLabel } from '../../../config/logger' +import getFormStatisticsTotalModel from '../../models/form_statistics_total.server.model' +import getSubmissionModel from '../../models/submission.server.model' +import getUserModel from '../../models/user.server.model' +import { DatabaseError } from '../core/core.errors' + +import { MIN_SUB_COUNT } from './analytics.constants' + +const FormStatisticsModel = getFormStatisticsTotalModel(mongoose) +const SubmissionModel = getSubmissionModel(mongoose) +const UserModel = getUserModel(mongoose) +const logger = createLoggerWithLabel(module) + +/** + * Retrieves the number of user documents in the database. + * @returns ok(user count) on success + * @returns err(DatabaseError) on query failure + */ +export const getUserCount = (): ResultAsync => { + return ResultAsync.fromPromise( + UserModel.estimatedDocumentCount().exec(), + (error) => { + logger.error({ + message: 'Database error when retrieving user collection count', + meta: { + action: 'getUserCount', + }, + error, + }) + + return new DatabaseError() + }, + ) +} + +/** + * Retrieves the number of submission documents in the database. + * @returns ok(submissions count) on success + * @returns err(DatabaseError) on query failure + */ +export const getSubmissionCount = (): ResultAsync => { + return ResultAsync.fromPromise( + SubmissionModel.estimatedDocumentCount().exec(), + (error) => { + logger.error({ + message: 'Database error when retrieving submission collection count', + meta: { + action: 'getSubmissionCount', + }, + error, + }) + + return new DatabaseError() + }, + ) +} + +/** + * !!! This function should only be called by {@link AnalyticsFactory} !!! + * + * ! Access to this function should be determined by whether the `aggregate-stats` feature is enabled. + * + * Retrieves the number of forms that has more than MIN_SUB_COUNT responses + * using the form statistics collection. + * @private + * @returns ok(form count) on success + * @returns err(DatabaseError) on query failure + */ +export const getFormCountWithStatsCollection = (): ResultAsync< + number, + DatabaseError +> => { + return ResultAsync.fromPromise( + FormStatisticsModel.aggregateFormCount(MIN_SUB_COUNT), + (error) => { + logger.error({ + message: + 'Database error when retrieving form count from FormStatisticsTotal collection', + meta: { + action: 'getFormCountWithStatsCollection', + }, + error, + }) + + return new DatabaseError() + }, + ).map(([result]) => { + return result?.numActiveForms ?? 0 + }) +} + +/** + * !!! This function should only be called by {@link AnalyticsFactory} !!! + * + * ! Access to this function should be determined by whether the `aggregate-stats` feature is enabled. + * + * Retrieves the number of forms that has more than MIN_SUB_COUNT responses + * using the submissions collection. + * @private + * @returns ok(form count) on success + * @returns err(DatabaseError) on query failure + */ +export const getFormCountWithSubmissionCollection = (): ResultAsync< + number, + DatabaseError +> => { + return ResultAsync.fromPromise( + SubmissionModel.findFormsWithSubsAbove(MIN_SUB_COUNT), + (error) => { + logger.error({ + message: + 'Database error when retrieving form count from submissions collection', + meta: { + action: 'getFormCountWithSubmissionCollection', + }, + error, + }) + + return new DatabaseError() + }, + ).map((forms) => forms.length) +} diff --git a/src/app/modules/auth/__tests__/auth.controller.spec.ts b/src/app/modules/auth/__tests__/auth.controller.spec.ts index cde65ca3fc..0dc23136b6 100644 --- a/src/app/modules/auth/__tests__/auth.controller.spec.ts +++ b/src/app/modules/auth/__tests__/auth.controller.spec.ts @@ -183,7 +183,7 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(200) - expect(mockRes.send).toBeCalledWith(mockUser.toObject()) + expect(mockRes.json).toBeCalledWith(mockUser.toObject()) }) it('should return with ApplicationError status and message when retrieving agency returns an ApplicationError', async () => { @@ -294,7 +294,7 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(200) - expect(mockRes.send).toBeCalledWith('Sign out successful') + expect(mockRes.json).toBeCalledWith({ message: 'Sign out successful' }) expect(mockClearCookie).toBeCalledTimes(1) expect(mockDestroy).toBeCalledTimes(1) }) @@ -335,7 +335,7 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(500) - expect(mockRes.send).toBeCalledWith('Sign out failed') + expect(mockRes.json).toBeCalledWith({ message: 'Sign out failed' }) expect(mockDestroyWithErr).toBeCalledTimes(1) expect(mockClearCookie).not.toBeCalled() }) diff --git a/src/app/modules/auth/__tests__/auth.routes.spec.ts b/src/app/modules/auth/__tests__/auth.routes.spec.ts index 5c01e21040..ef20896efa 100644 --- a/src/app/modules/auth/__tests__/auth.routes.spec.ts +++ b/src/app/modules/auth/__tests__/auth.routes.spec.ts @@ -35,7 +35,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.email is invalid', async () => { @@ -49,7 +51,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 401 when domain of body.email does not exist in Agency collection', async () => { @@ -124,7 +128,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.email is invalid', async () => { @@ -138,7 +144,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 401 when domain of body.email does not exist in Agency collection', async () => { @@ -256,7 +264,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.otp is not provided as a param', async () => { @@ -267,7 +277,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.email is invalid', async () => { @@ -281,7 +293,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.otp is less than 6 digits', async () => { @@ -293,7 +307,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 400 when body.otp is 6 characters but does not consist purely of digits', async () => { @@ -305,7 +321,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(400) - expect(response.text).toEqual('Some required parameters are missing') + expect(response.text).toEqual( + JSON.stringify({ message: 'Some required parameters are missing' }), + ) }) it('should return 401 when domain of body.email does not exist in Agency collection', async () => { @@ -487,7 +505,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(200) - expect(response.text).toEqual('Sign out successful') + expect(response.text).toEqual( + JSON.stringify({ message: 'Sign out successful' }), + ) // connect.sid should now be empty. expect(response.header['set-cookie'][0]).toEqual( expect.stringContaining('connect.sid=;'), @@ -504,7 +524,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(200) - expect(response.text).toEqual('Sign out successful') + expect(response.text).toEqual( + JSON.stringify({ message: 'Sign out successful' }), + ) }) }) }) diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index c5907ff358..1dea89bf09 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -6,7 +6,7 @@ import { isEmpty } from 'lodash' import { createLoggerWithLabel } from '../../../config/logger' import { LINKS } from '../../../shared/constants' import MailService from '../../services/mail.service' -import { getRequestIp } from '../../utils/request' +import { getRequestIp, getTrace } from '../../utils/request' import * as UserService from '../user/user.service' import * as AuthService from './auth.service' @@ -37,6 +37,7 @@ export const handleCheckUser: RequestHandler< meta: { action: 'handleCheckUser', ip: getRequestIp(req), + trace: getTrace(req), email, }, error, @@ -64,6 +65,7 @@ export const handleLoginSendOtp: RequestHandler< action: 'handleLoginSendOtp', email, ip: requestIp, + trace: getTrace(req), } return ( @@ -123,6 +125,7 @@ export const handleLoginVerifyOtp: RequestHandler< action: 'handleLoginVerifyOtp', email, ip: getRequestIp(req), + trace: getTrace(req), } const coreErrorMessage = `Failed to process OTP. Please try again later and if the problem persists, submit our Support Form (${LINKS.supportFormLink}).` @@ -168,7 +171,7 @@ export const handleLoginVerifyOtp: RequestHandler< meta: logMeta, }) - return res.status(StatusCodes.OK).send(userObj) + return res.status(StatusCodes.OK).json(userObj) }) // Step 3b: Error occured in one of the steps. .mapErr((error) => { @@ -209,11 +212,11 @@ export const handleSignout: RequestHandler = async (req, res) => { }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send('Sign out failed') + .json({ message: 'Sign out failed' }) } // No error. res.clearCookie('connect.sid') - return res.status(StatusCodes.OK).send('Sign out successful') + return res.status(StatusCodes.OK).json({ message: 'Sign out successful' }) }) } diff --git a/src/app/modules/auth/auth.routes.ts b/src/app/modules/auth/auth.routes.ts index de7babc84e..3ef281e026 100644 --- a/src/app/modules/auth/auth.routes.ts +++ b/src/app/modules/auth/auth.routes.ts @@ -1,6 +1,9 @@ import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' +import { rateLimitConfig } from '../../../config/config' +import { limitRate } from '../../utils/limit-rate' + import * as AuthController from './auth.controller' export const AuthRouter = Router() @@ -40,6 +43,7 @@ AuthRouter.post( */ AuthRouter.post( '/sendotp', + limitRate({ max: rateLimitConfig.sendAuthOtp }), celebrate({ [Segments.BODY]: Joi.object().keys({ email: Joi.string() diff --git a/src/app/modules/user/user.controller.ts b/src/app/modules/user/user.controller.ts index c35b35cf30..1e85653e6e 100644 --- a/src/app/modules/user/user.controller.ts +++ b/src/app/modules/user/user.controller.ts @@ -5,7 +5,7 @@ import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' import { IPopulatedUser } from '../../../types' -import SmsFactory from '../../factories/sms.factory' +import { SmsFactory } from '../../services/sms/sms.factory' import { ApplicationError } from '../core/core.errors' import { @@ -97,7 +97,7 @@ export const handleContactVerifyOtp: RequestHandler< // No error, update user with given contact. try { const updatedUser = await updateUserContact(contact, userId) - return res.status(StatusCodes.OK).send(updatedUser) + return res.status(StatusCodes.OK).json(updatedUser) } catch (updateErr) { // Handle update error. logger.warn({ @@ -115,7 +115,9 @@ export const handleContactVerifyOtp: RequestHandler< export const handleFetchUser: RequestHandler = async (req, res) => { const sessionUserId = getUserIdFromSession(req.session) if (!sessionUserId) { - return res.status(StatusCodes.UNAUTHORIZED).send('User is unauthorized.') + return res + .status(StatusCodes.UNAUTHORIZED) + .json({ message: 'User is unauthorized.' }) } // Retrieve user with id in session @@ -132,10 +134,10 @@ export const handleFetchUser: RequestHandler = async (req, res) => { }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send('Unable to retrieve user') + .json({ message: 'Unable to retrieve user' }) } - return res.send(retrievedUser) + return res.json(retrievedUser) } // TODO(#212): Save userId instead of entire user collection in session. diff --git a/src/app/modules/verification/verification.factory.ts b/src/app/modules/verification/verification.factory.ts index 3914e633b9..8fe8a78fed 100644 --- a/src/app/modules/verification/verification.factory.ts +++ b/src/app/modules/verification/verification.factory.ts @@ -1,8 +1,7 @@ import { RequestHandler } from 'express' import { StatusCodes } from 'http-status-codes' -import featureManager from '../../../config/feature-manager' -import { FeatureNames } from '../../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../../config/feature-manager' import * as verification from './verification.controller' @@ -31,15 +30,15 @@ const verifiedFieldsFactory = ({ const errMsg = 'Verified fields feature is not enabled' return { createTransaction: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), getTransactionMetadata: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), resetFieldInTransaction: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), getNewOtp: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), verifyOtp: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(errMsg), + res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), } } } diff --git a/src/app/modules/verification/verification.service.ts b/src/app/modules/verification/verification.service.ts index 90272a8662..bf9402c9ef 100644 --- a/src/app/modules/verification/verification.service.ts +++ b/src/app/modules/verification/verification.service.ts @@ -12,10 +12,10 @@ import { IVerificationFieldSchema, IVerificationSchema, } from '../../../types' -import smsFactory from '../../factories/sms.factory' import getFormModel from '../../models/form.server.model' import getVerificationModel from '../../models/verification.server.model' import MailService from '../../services/mail.service' +import { SmsFactory } from '../../services/sms/sms.factory' import { generateOtp } from '../../utils/otp' import { ITransaction } from './verification.types' @@ -257,17 +257,15 @@ const sendOTPForField = async ( field: IVerificationFieldSchema, recipient: string, otp: string, -): Promise => { +): Promise => { const { fieldType } = field switch (fieldType) { case 'mobile': // call sms - it should validate the recipient - await smsFactory.sendVerificationOtp(recipient, otp, formId) - break + return SmsFactory.sendVerificationOtp(recipient, otp, formId) case 'email': // call email - it should validate the recipient - await MailService.sendVerificationOtp(recipient, otp) - break + return MailService.sendVerificationOtp(recipient, otp) default: throw new Error(`sendOTPForField: ${fieldType} is unsupported`) } diff --git a/src/app/modules/webhook/webhook.controller.ts b/src/app/modules/webhook/webhook.controller.ts index 10a2aa7b4e..8fc6b9b904 100644 --- a/src/app/modules/webhook/webhook.controller.ts +++ b/src/app/modules/webhook/webhook.controller.ts @@ -27,7 +27,7 @@ export const post = ( if (webhookUrl) { // Note that we push data to webhook endpoints on a best effort basis // As such, we should not await on these post requests - pushData(webhookUrl, submissionWebhookView) + void pushData(webhookUrl, submissionWebhookView) } return next() } diff --git a/src/app/routes/core.server.routes.js b/src/app/routes/core.server.routes.js index 2690719b4d..0f7720bc84 100755 --- a/src/app/routes/core.server.routes.js +++ b/src/app/routes/core.server.routes.js @@ -4,36 +4,8 @@ * Module dependencies. */ let core = require('../../app/controllers/core.server.controller') -const aggregStatsFactory = require('../factories/aggregate-stats.factory') module.exports = function (app) { // Core routing app.route('/').get(core.index) - - /** - * Retrieves the number of popular forms on FormSG - * @route GET /analytics/forms - * @group analytics - form usage statistics - * @produces application/json - * @returns {Number} 200 - the number of forms with more than 10 submissions - */ - app.route('/analytics/forms').get(aggregStatsFactory.formCount) - - /** - * Retrieves the number of users building forms on FormSG - * @route GET /analytics/users - * @group analytics - form usage statistics - * @produces application/json - * @returns {Number} 200 - the number of users building forms - */ - app.route('/analytics/users').get(core.userCount) - - /** - * Retrieves the total number of submissions of forms across FormSG - * @route GET /analytics/submissions - * @group analytics - form usage statistics - * @produces application/json - * @returns {Number} 200 - the total number of submissions of forms - */ - app.route('/analytics/submissions').get(core.submissionCount) } diff --git a/src/app/routes/frontend.server.routes.js b/src/app/routes/frontend.server.routes.js index ba43052763..25ff95a9b0 100644 --- a/src/app/routes/frontend.server.routes.js +++ b/src/app/routes/frontend.server.routes.js @@ -22,6 +22,6 @@ module.exports = function (app) { ) app.route('/frontend/features').get((req, res) => { - res.send(featureManager.states) + res.json(featureManager.states) }) } diff --git a/src/app/routes/public-forms.server.routes.js b/src/app/routes/public-forms.server.routes.js index 7359a06beb..712fc9acc2 100644 --- a/src/app/routes/public-forms.server.routes.js +++ b/src/app/routes/public-forms.server.routes.js @@ -9,9 +9,11 @@ const submissions = require('../../app/controllers/submissions.server.controller const encryptSubmissions = require('../../app/controllers/encrypt-submissions.server.controller') const emailSubmissions = require('../../app/controllers/email-submissions.server.controller') const { celebrate, Joi } = require('celebrate') -const captchaFactory = require('../factories/captcha.factory') const spcpFactory = require('../factories/spcp-myinfo.factory') const webhookVerifiedContentFactory = require('../factories/webhook-verified-content.factory') +const { CaptchaFactory } = require('../factories/captcha.factory') +const { limitRate } = require('../utils/limit-rate') +const { rateLimitConfig } = require('../../config/config') module.exports = function (app) { /** @@ -138,10 +140,11 @@ module.exports = function (app) { * @returns {SubmissionResponse.model} 400 - submission has bad data and could not be processed */ app.route('/v2/submissions/email/:formId([a-fA-F0-9]{24})').post( - captchaFactory.validateCaptcha, + limitRate({ max: rateLimitConfig.submissions }), + CaptchaFactory.validateCaptcha, forms.formById, publicForms.isFormPublic, - captchaFactory.captchaCheck, + CaptchaFactory.captchaCheck, spcpFactory.isSpcpAuthenticated, emailSubmissions.receiveEmailSubmissionUsingBusBoy, celebrate({ @@ -199,7 +202,8 @@ module.exports = function (app) { * @returns {SubmissionResponse.model} 400 - submission has bad data and could not be processed */ app.route('/v2/submissions/encrypt/:formId([a-fA-F0-9]{24})').post( - captchaFactory.validateCaptcha, + limitRate({ max: rateLimitConfig.submissions }), + CaptchaFactory.validateCaptcha, celebrate({ body: Joi.object({ responses: Joi.array() @@ -244,7 +248,7 @@ module.exports = function (app) { }), forms.formById, publicForms.isFormPublic, - captchaFactory.captchaCheck, + CaptchaFactory.captchaCheck, encryptSubmissions.validateEncryptSubmission, spcpFactory.isSpcpAuthenticated, spcpFactory.verifyMyInfoVals, diff --git a/src/app/services/mail.service.ts b/src/app/services/mail.service.ts index e58d7948cc..93aa35f190 100644 --- a/src/app/services/mail.service.ts +++ b/src/app/services/mail.service.ts @@ -107,7 +107,10 @@ export class MailService { * @param mail Mail data to send with * @param sendOptions Extra options to better identify mail, such as form or mail id. */ - #sendNodeMail = async (mail: MailOptions, sendOptions?: SendMailOptions) => { + #sendNodeMail = async ( + mail: MailOptions, + sendOptions?: SendMailOptions, + ): Promise => { const logMeta = { action: '#sendNodeMail', mailId: sendOptions?.mailId, @@ -141,12 +144,12 @@ export class MailService { }) try { - const response = await this.#transporter.sendMail(mail) + await this.#transporter.sendMail(mail) logger.info({ message: `Mail successfully sent on attempt ${attemptNum}`, meta: logMeta, }) - return response + return true } catch (err) { // Pass errors to the callback logger.error({ @@ -182,7 +185,7 @@ export class MailService { form, submission, index, - }: SendSingleAutoreplyMailArgs) => { + }: SendSingleAutoreplyMailArgs): Promise => { const emailSubject = autoReplyMailData.subject || `Thank you for submitting ${form.title}` // Sender's name appearing after "("" symbol gets truncated. Escaping it @@ -229,7 +232,10 @@ export class MailService { * @param otp the otp to send * @throws error if mail fails, to be handled by the caller */ - sendVerificationOtp = async (recipient: string, otp: string) => { + sendVerificationOtp = async ( + recipient: string, + otp: string, + ): Promise => { // TODO(#42): Remove param guards once whole backend is TypeScript. if (!otp) { throw new Error('OTP is missing.') @@ -269,7 +275,7 @@ export class MailService { recipient: string otp: string ipAddress: string - }): ResultAsync => { + }): ResultAsync => { return generateLoginOtpHtml({ appName: this.#appName, appUrl: this.#appUrl, @@ -326,7 +332,7 @@ export class MailService { bounceType: BounceType | undefined formTitle: string formId: string - }) => { + }): Promise => { const htmlData: BounceNotificationHtmlData = { formTitle, formLink: `${this.#appUrl}/${formId}`, @@ -374,7 +380,7 @@ export class MailService { question: string answer: string | number }[] - }) => { + }): Promise => { const refNo = submission.id const formTitle = form.title const submissionTime = moment(submission.created) @@ -447,7 +453,7 @@ export class MailService { responsesData, autoReplyMailDatas, attachments = [], - }: SendAutoReplyEmailsArgs) => { + }: SendAutoReplyEmailsArgs): Promise[]> => { // Data to render both the submission details mail HTML body PDF. const renderData: AutoreplySummaryRenderData = { refNo: submission.id, diff --git a/src/app/services/sms/__tests__/sms.factory.spec.ts b/src/app/services/sms/__tests__/sms.factory.spec.ts new file mode 100644 index 0000000000..f5e4730c87 --- /dev/null +++ b/src/app/services/sms/__tests__/sms.factory.spec.ts @@ -0,0 +1,129 @@ +import Twilio from 'twilio' + +import { + FeatureNames, + ISms, + RegisteredFeature, +} from 'src/config/feature-manager' + +import { createSmsFactory } from '../sms.factory' +import * as SmsService from '../sms.service' +import { TwilioConfig } from '../sms.types' + +// This is hoisted and thus a const cannot be passed in. +jest.mock('twilio', () => + jest.fn().mockImplementation(() => ({ + mocked: 'this is mocked', + })), +) + +const MOCKED_TWILIO = ({ + mocked: 'this is mocked', +} as unknown) as Twilio.Twilio + +describe('sms.factory', () => { + beforeEach(() => jest.clearAllMocks()) + + describe('sms feature disabled', () => { + const MOCK_DISABLED_SMS_FEATURE: RegisteredFeature = { + isEnabled: false, + props: {} as ISms, + } + + const SmsFactory = createSmsFactory(MOCK_DISABLED_SMS_FEATURE) + + it('should throw error when invoking sendAdminContactOtp', () => { + // Act + const invocation = () => + SmsFactory.sendAdminContactOtp('anything', 'anything', 'anything') + + // Assert + expect(invocation).toThrowError( + 'sendAdminContactOtp: SMS feature must be enabled in Feature Manager first', + ) + }) + + it('should throw error when invoking sendVerificationOtp', () => { + // Act + const invocation = () => + SmsFactory.sendVerificationOtp('anything', 'anything', 'anything') + + // Assert + expect(invocation).toThrowError( + 'sendVerificationOtp: SMS feature must be enabled in Feature Manager first', + ) + }) + }) + + describe('sms feature enabled', () => { + const MOCK_ENABLED_SMS_FEATURE: Required> = { + isEnabled: true, + props: { + twilioAccountSid: 'ACrandomTwilioSid', + twilioApiKey: 'SKrandomTwilioAPIKEY', + twilioApiSecret: 'this is a super secret', + twilioMsgSrvcSid: 'formsg-is-great-pleasehelpme', + }, + } + + const SmsFactory = createSmsFactory(MOCK_ENABLED_SMS_FEATURE) + + it('should call SmsService counterpart when invoking sendAdminContactOtp', async () => { + // Arrange + const serviceContactSpy = jest + .spyOn(SmsService, 'sendAdminContactOtp') + .mockResolvedValue(true) + + const mockArguments: Parameters = [ + 'mockRecipient', + 'mockOtp', + 'mockFormId', + ] + + // Act + await SmsFactory.sendAdminContactOtp(...mockArguments) + + // Assert + const expectedTwilioConfig: TwilioConfig = { + msgSrvcSid: MOCK_ENABLED_SMS_FEATURE.props.twilioMsgSrvcSid, + client: MOCKED_TWILIO, + } + + expect(serviceContactSpy).toHaveBeenCalledTimes(1) + expect(serviceContactSpy).toHaveBeenCalledWith( + ...mockArguments, + expectedTwilioConfig, + ) + }) + + it('should call SmsService counterpart when invoking sendVerificationOtp', async () => { + // Arrange + const serviceVfnSpy = jest + .spyOn(SmsService, 'sendVerificationOtp') + .mockResolvedValue(true) + + const mockArguments: Parameters = [ + 'mockRecipient', + 'mockOtp', + 'mockUserId', + ] + + // Act + await SmsFactory.sendVerificationOtp(...mockArguments) + + // Assert + const expectedTwilioConfig: TwilioConfig = { + msgSrvcSid: MOCK_ENABLED_SMS_FEATURE.props.twilioMsgSrvcSid, + client: MOCKED_TWILIO, + } + + expect(serviceVfnSpy).toHaveBeenCalledTimes(1) + expect(serviceVfnSpy).toHaveBeenCalledWith( + ...mockArguments, + expectedTwilioConfig, + ) + }) + }) +}) diff --git a/src/app/services/sms/__tests__/sms.service.spec.ts b/src/app/services/sms/__tests__/sms.service.spec.ts new file mode 100644 index 0000000000..0496e118ca --- /dev/null +++ b/src/app/services/sms/__tests__/sms.service.spec.ts @@ -0,0 +1,201 @@ +import mongoose from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import getFormModel from 'src/app/models/form.server.model' +import { VfnErrors } from 'src/shared/util/verification' +import { FormOtpData, IFormSchema, IUserSchema, ResponseMode } from 'src/types' + +import getSmsCountModel from '../sms_count.server.model' +import * as SmsService from '../sms.service' +import { LogType, SmsType, TwilioConfig } from '../sms.types' + +const FormModel = getFormModel(mongoose) +const SmsCountModel = getSmsCountModel(mongoose) + +// Test numbers provided by Twilio: +// https://www.twilio.com/docs/iam/test-credentials +const TWILIO_TEST_NUMBER = '+15005550006' + +const MOCK_MSG_SRVC_SID = 'mockMsgSrvcSid' + +const MOCK_VALID_CONFIG = ({ + msgSrvcSid: MOCK_MSG_SRVC_SID, + client: { + messages: { + create: jest.fn().mockResolvedValue({ + status: 'testStatus', + sid: 'testSid', + }), + }, + }, +} as unknown) as TwilioConfig + +const MOCK_INVALID_CONFIG = ({ + msgSrvcSid: MOCK_MSG_SRVC_SID, + client: { + messages: { + create: jest.fn().mockResolvedValue({ + status: 'testStatus', + sid: undefined, + errorCode: 21211, + }), + }, + }, +} as unknown) as TwilioConfig + +const smsCountSpy = jest.spyOn(SmsCountModel, 'logSms') + +describe('sms.service', () => { + let testUser: IUserSchema + + beforeAll(async () => await dbHandler.connect()) + beforeEach(async () => { + const { user } = await dbHandler.insertFormCollectionReqs() + testUser = user + smsCountSpy.mockClear() + }) + afterEach(async () => await dbHandler.clearDatabase()) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('sendVerificationOtp', () => { + let mockOtpData: FormOtpData + let testForm: IFormSchema + + beforeEach(async () => { + testForm = await FormModel.create({ + title: 'Test Form', + emails: [testUser.email], + admin: testUser._id, + responseMode: ResponseMode.Email, + }) + + mockOtpData = { + form: testForm._id, + formAdmin: { + email: testUser.email, + userId: testUser._id, + }, + } + }) + + it('should throw error when retrieved otpData is null', async () => { + // Arrange + // Return null on Form method + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(null) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + await expect(actualPromise).rejects.toThrowError() + }) + + it('should log and send verification OTP when sending has no errors', async () => { + // Arrange + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(mockOtpData) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + // Assert + // Should resolve to true + await expect(actualPromise).resolves.toEqual(true) + // Logging should also have happened. + const expectedLogParams = { + otpData: mockOtpData, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.verification, + logType: LogType.success, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + + it('should log failure and throw error when verification OTP fails to send', async () => { + // Arrange + jest.spyOn(FormModel, 'getOtpData').mockResolvedValueOnce(mockOtpData) + + // Act + const actualPromise = SmsService.sendVerificationOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* formId= */ testForm._id, + /* defaultConfig= */ MOCK_INVALID_CONFIG, + ) + + // Assert + const expectedError = new Error(VfnErrors.InvalidMobileNumber) + expectedError.name = VfnErrors.SendOtpFailed + + await expect(actualPromise).rejects.toThrow(expectedError) + // Logging should also have happened. + const expectedLogParams = { + otpData: mockOtpData, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.verification, + logType: LogType.failure, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + }) + + describe('sendAdminContactOtp', () => { + it('should log and send contact OTP when sending has no errors', async () => { + // Act + const actualPromise = SmsService.sendAdminContactOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* userId= */ testUser._id, + /* defaultConfig= */ MOCK_VALID_CONFIG, + ) + + // Assert + // Should resolve to true + await expect(actualPromise).resolves.toEqual(true) + // Logging should also have happened. + const expectedLogParams = { + otpData: { + admin: testUser._id, + }, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.adminContact, + logType: LogType.success, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) + }) + + it('should log failure and throw error when contact OTP fails to send', async () => { + // Act + const actualPromise = SmsService.sendAdminContactOtp( + /* recipient= */ TWILIO_TEST_NUMBER, + /* otp= */ '111111', + /* userId= */ testUser._id, + /* defaultConfig= */ MOCK_INVALID_CONFIG, + ) + + // Assert + const expectedError = new Error(VfnErrors.InvalidMobileNumber) + expectedError.name = VfnErrors.SendOtpFailed + + await expect(actualPromise).rejects.toThrow(expectedError) + // Logging should also have happened. + const expectedLogParams = { + otpData: { + admin: testUser._id, + }, + msgSrvcSid: MOCK_MSG_SRVC_SID, + smsType: SmsType.adminContact, + logType: LogType.failure, + } + expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) + }) +}) diff --git a/tests/unit/backend/models/sms_count.server.model.spec.ts b/src/app/services/sms/__tests__/sms_count.server.model.spec.ts similarity index 96% rename from tests/unit/backend/models/sms_count.server.model.spec.ts rename to src/app/services/sms/__tests__/sms_count.server.model.spec.ts index a3faddf2d2..49b51e2525 100644 --- a/tests/unit/backend/models/sms_count.server.model.spec.ts +++ b/src/app/services/sms/__tests__/sms_count.server.model.spec.ts @@ -1,11 +1,11 @@ +/* eslint-disable @typescript-eslint/ban-ts-comment */ import { ObjectId } from 'bson' import { cloneDeep, merge, omit } from 'lodash' import mongoose from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' -import getSmsCountModel from 'src/app/models/sms_count.server.model' -import { IVerificationSmsCount, LogType, SmsType } from 'src/types' - -import dbHandler from '../helpers/jest-db' +import getSmsCountModel from '../sms_count.server.model' +import { IVerificationSmsCount, LogType, SmsType } from '../sms.types' const SmsCount = getSmsCountModel(mongoose) @@ -194,7 +194,7 @@ describe('SmsCount', () => { form: MOCK_FORM_ID, }).lean() - expect(actualLog!._id).toBeDefined() + expect(actualLog?._id).toBeDefined() // Retrieve object and compare to params, remove indeterministic keys const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v']) expect(actualSavedObject).toEqual(expectedLog) @@ -220,7 +220,7 @@ describe('SmsCount', () => { form: MOCK_FORM_ID, }).lean() - expect(actualLog!._id).toBeDefined() + expect(actualLog?._id).toBeDefined() // Retrieve object and compare to params, remove indeterministic keys const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v']) expect(actualSavedObject).toEqual(expectedLog) diff --git a/src/app/services/sms/sms.errors.ts b/src/app/services/sms/sms.errors.ts new file mode 100644 index 0000000000..a7f11c906b --- /dev/null +++ b/src/app/services/sms/sms.errors.ts @@ -0,0 +1,16 @@ +import { ApplicationError } from '../../modules/core/core.errors' + +export class SmsSendError extends ApplicationError { + code?: number + sendStatus: unknown + + constructor( + message = 'Error sending OTP. Please try again later and if the problem persists, contact us.', + code?: number, + sendStatus?: unknown, + ) { + super(message) + this.code = code + this.sendStatus = sendStatus + } +} diff --git a/src/app/services/sms/sms.factory.ts b/src/app/services/sms/sms.factory.ts new file mode 100644 index 0000000000..b1a70b130f --- /dev/null +++ b/src/app/services/sms/sms.factory.ts @@ -0,0 +1,65 @@ +import Twilio from 'twilio' + +import FeatureManager, { + FeatureNames, + RegisteredFeature, +} from '../../../config/feature-manager' + +import { sendAdminContactOtp, sendVerificationOtp } from './sms.service' +import { TwilioConfig } from './sms.types' + +interface ISmsFactory { + sendVerificationOtp: ( + recipient: string, + otp: string, + formId: string, + ) => ReturnType + sendAdminContactOtp: ( + recipient: string, + otp: string, + userId: string, + ) => ReturnType +} + +const smsFeature = FeatureManager.get(FeatureNames.Sms) + +// Exported for testing. +export const createSmsFactory = ( + smsFeature: RegisteredFeature, +): ISmsFactory => { + if (!smsFeature.isEnabled || !smsFeature.props) { + const errorMessage = 'SMS feature must be enabled in Feature Manager first' + return { + sendAdminContactOtp: () => { + throw new Error(`sendAdminContactOtp: ${errorMessage}`) + }, + sendVerificationOtp: () => { + throw new Error(`sendVerificationOtp: ${errorMessage}`) + }, + } + } + + const { + twilioAccountSid, + twilioApiKey, + twilioApiSecret, + twilioMsgSrvcSid, + } = smsFeature.props + + const twilioClient = Twilio(twilioApiKey, twilioApiSecret, { + accountSid: twilioAccountSid, + }) + const twilioConfig: TwilioConfig = { + msgSrvcSid: twilioMsgSrvcSid, + client: twilioClient, + } + + return { + sendVerificationOtp: (recipient, otp, formId) => + sendVerificationOtp(recipient, otp, formId, twilioConfig), + sendAdminContactOtp: (recipient, otp, userId) => + sendAdminContactOtp(recipient, otp, userId, twilioConfig), + } +} + +export const SmsFactory = createSmsFactory(smsFeature) diff --git a/src/app/services/sms.service.ts b/src/app/services/sms/sms.service.ts similarity index 81% rename from src/app/services/sms.service.ts rename to src/app/services/sms/sms.service.ts index 316a4d31c8..4d7c7f7963 100644 --- a/src/app/services/sms.service.ts +++ b/src/app/services/sms/sms.service.ts @@ -3,19 +3,22 @@ import mongoose from 'mongoose' import NodeCache from 'node-cache' import Twilio from 'twilio' -import config from '../../config/config' -import { createLoggerWithLabel } from '../../config/logger' -import { isPhoneNumber } from '../../shared/util/phone-num-validation' -import { VfnErrors } from '../../shared/util/verification' +import config from '../../../config/config' +import { createLoggerWithLabel } from '../../../config/logger' +import { isPhoneNumber } from '../../../shared/util/phone-num-validation' +import { VfnErrors } from '../../../shared/util/verification' +import { AdminContactOtpData, FormOtpData } from '../../../types' +import getFormModel from '../../models/form.server.model' + +import getSmsCountModel from './sms_count.server.model' +import { SmsSendError } from './sms.errors' import { - AdminContactOtpData, - FormOtpData, LogSmsParams, LogType, SmsType, -} from '../../types' -import getFormModel from '../models/form.server.model' -import getSmsCountModel from '../models/sms_count.server.model' + TwilioConfig, + TwilioCredentials, +} from './sms.types' const logger = createLoggerWithLabel(module) const SmsCount = getSmsCountModel(mongoose) @@ -28,13 +31,6 @@ const secretsManager = new SecretsManager({ region: config.aws.region }) // credentials, or wait 10 seconds before. const twilioClientCache = new NodeCache({ deleteOnExpire: true, stdTTL: 10 }) -type TwilioCredentials = { - accountSid: string - apiKey: string - apiSecret: string - messagingServiceSid: string -} - /** * Retrieves credentials from secrets manager * @param msgSrvcName The name of credential stored in the secret manager. @@ -71,11 +67,6 @@ const getCredentials = async ( return null } -type TwilioConfig = { - client: Twilio.Twilio - msgSrvcSid: string -} - /** * * @param msgSrvcName The name of credential stored in the secret manager @@ -135,17 +126,6 @@ const getTwilio = async ( return defaultConfig } -/** - * Retrieve the relevant data required to send an OTP from given formId - * @param formId The form to retrieve data from - * @returns Relevant OTP data containing the linked messaging service name (if available), and form details such as its id and the admin. - * - */ -const getOtpDataFromForm = async (formId: string) => { - const otpData = await Form.getOtpData(formId) - return otpData -} - /** * Sends a message to a valid phone number * @param twilioConfig The configuration used to send OTPs with @@ -179,7 +159,7 @@ const send = async ( // Sent but with error code. // Throw error to be caught in catch block. if (!sid || errorCode) { - throw new TwilioError(errorMessage, errorCode, status) + throw new SmsSendError(errorMessage, errorCode, status) } // Log success @@ -189,6 +169,7 @@ const send = async ( msgSrvcSid, logType: LogType.success, } + SmsCount.logSms(logParams).catch((err) => { logger.error({ message: 'Error logging sms count to database', @@ -200,6 +181,14 @@ const send = async ( }) }) + logger.info({ + message: 'Successfully sent sms', + meta: { + action: 'send', + otpData, + }, + }) + return true }) .catch((err) => { @@ -232,7 +221,7 @@ const send = async ( // Invalid number error code, throw a more reasonable error for error // handling. // See https://www.twilio.com/docs/api/errors/21211 - if (err.code === 21211) { + if (err?.code === 21211) { const invalidOtpError = new Error(VfnErrors.InvalidMobileNumber) invalidOtpError.name = VfnErrors.SendOtpFailed throw invalidOtpError @@ -249,12 +238,12 @@ const send = async ( * @param formId Form id for logging. * */ -const sendVerificationOtp = async ( +export const sendVerificationOtp = async ( recipient: string, otp: string, formId: string, defaultConfig: TwilioConfig, -) => { +): Promise => { logger.info({ message: `Sending verification OTP for ${formId}`, meta: { @@ -262,7 +251,7 @@ const sendVerificationOtp = async ( formId, }, }) - const otpData = await getOtpDataFromForm(formId) + const otpData = await Form.getOtpData(formId) if (!otpData) { const errMsg = `Unable to retrieve otpData from ${formId}` @@ -282,12 +271,12 @@ const sendVerificationOtp = async ( return send(twilioData, otpData, recipient, message, SmsType.verification) } -const sendAdminContactOtp = async ( +export const sendAdminContactOtp = async ( recipient: string, otp: string, userId: string, defaultConfig: TwilioConfig, -) => { +): Promise => { logger.info({ message: `Sending admin contact verification OTP for ${userId}`, meta: { @@ -304,24 +293,3 @@ const sendAdminContactOtp = async ( return send(defaultConfig, otpData, recipient, message, SmsType.adminContact) } - -class TwilioError extends Error { - code: number - status: string - - constructor(message: string, code: number, status: string) { - super(message) - this.code = code - this.status = status - this.name = this.constructor.name - - // Set the prototype explicitly. - // See https://github.com/facebook/jest/issues/8279 - Object.setPrototypeOf(this, TwilioError.prototype) - } -} - -module.exports = { - sendVerificationOtp, - sendAdminContactOtp, -} diff --git a/src/types/sms_count.ts b/src/app/services/sms/sms.types.ts similarity index 77% rename from src/types/sms_count.ts rename to src/app/services/sms/sms.types.ts index 48ca223996..a5d52222f3 100644 --- a/src/types/sms_count.ts +++ b/src/app/services/sms/sms.types.ts @@ -1,7 +1,12 @@ import { Document, Model } from 'mongoose' +import { Twilio } from 'twilio' -import { FormOtpData, IFormSchema } from './form' -import { AdminContactOtpData, IUserSchema } from './user' +import { + AdminContactOtpData, + FormOtpData, + IFormSchema, + IUserSchema, +} from 'src/types' export enum SmsType { verification = 'VERIFICATION', @@ -50,3 +55,15 @@ export type IAdminContactSmsCountSchema = ISmsCountSchema export interface ISmsCountModel extends Model { logSms: (logParams: LogSmsParams) => Promise } + +export type TwilioCredentials = { + accountSid: string + apiKey: string + apiSecret: string + messagingServiceSid: string +} + +export type TwilioConfig = { + client: Twilio + msgSrvcSid: string +} diff --git a/src/app/models/sms_count.server.model.ts b/src/app/services/sms/sms_count.server.model.ts similarity index 91% rename from src/app/models/sms_count.server.model.ts rename to src/app/services/sms/sms_count.server.model.ts index f702563f2b..c0e2299d96 100644 --- a/src/app/models/sms_count.server.model.ts +++ b/src/app/services/sms/sms_count.server.model.ts @@ -1,5 +1,8 @@ import { Mongoose, Schema } from 'mongoose' +import { FORM_SCHEMA_ID } from '../../models/form.server.model' +import { USER_SCHEMA_ID } from '../../models/user.server.model' + import { IAdminContactSmsCountSchema, ISmsCount, @@ -9,10 +12,7 @@ import { LogSmsParams, LogType, SmsType, -} from '../../types' - -import { FORM_SCHEMA_ID } from './form.server.model' -import { USER_SCHEMA_ID } from './user.server.model' +} from './sms.types' const SMS_COUNT_SCHEMA_NAME = 'SmsCount' @@ -101,7 +101,7 @@ const compileSmsCountModel = (db: Mongoose) => { * @param db The mongoose instance to retrieve the SmsCount model from * @returns The SmsCount model */ -const getSmsCountModel = (db: Mongoose) => { +const getSmsCountModel = (db: Mongoose): ISmsCountModel => { try { return db.model(SMS_COUNT_SCHEMA_NAME) as ISmsCountModel } catch { diff --git a/src/app/utils/__tests__/limit-rate.spec.ts b/src/app/utils/__tests__/limit-rate.spec.ts new file mode 100644 index 0000000000..db955cf18b --- /dev/null +++ b/src/app/utils/__tests__/limit-rate.spec.ts @@ -0,0 +1,43 @@ +import RateLimit from 'express-rate-limit' +import expressHandler from 'tests/unit/backend/helpers/jest-express' +import { mocked } from 'ts-jest/utils' + +jest.mock('express-rate-limit') +const MockRateLimit = mocked(RateLimit, true) + +// eslint-disable-next-line import/first +import { limitRate } from 'src/app/utils/limit-rate' + +const MOCK_MAX = 5 +const MOCK_WINDOW = 10 + +describe('limitRate', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + it('should create a rate-limiting middleware with defaults', () => { + limitRate() + expect(MockRateLimit).toHaveBeenCalledWith( + expect.objectContaining({ windowMs: 60000, max: 1200 }), + ) + }) + + it('should create a rate-limiting middleware with custom options', () => { + limitRate({ max: MOCK_MAX, windowMs: MOCK_WINDOW }) + expect(MockRateLimit).toHaveBeenCalledWith( + expect.objectContaining({ windowMs: MOCK_WINDOW, max: MOCK_MAX }), + ) + }) + + it('should call next() in the handler', () => { + limitRate() + const handler = MockRateLimit.mock.calls[0][0]!.handler! + const mockNext = jest.fn() + handler( + expressHandler.mockRequest(), + expressHandler.mockResponse(), + mockNext, + ) + expect(mockNext).toHaveBeenCalled() + }) +}) diff --git a/src/app/utils/limit-rate.ts b/src/app/utils/limit-rate.ts new file mode 100644 index 0000000000..31e93e388f --- /dev/null +++ b/src/app/utils/limit-rate.ts @@ -0,0 +1,40 @@ +import RateLimit, { + Options as RateLimitOptions, + RateLimit as RateLimitFn, +} from 'express-rate-limit' +import { merge } from 'lodash' + +import { createLoggerWithLabel } from '../../config/logger' + +import { getRequestIp } from './request' + +const logger = createLoggerWithLabel(module) + +/** + * Returns a middleware which logs a message if the rate of requests + * to an API endpoint exceeds a given rate. + * TODO (private #49): update this documentation. + * @param options Custom options to be passed to RateLimit + * @return Rate-limiting middleware + */ +export const limitRate = (options: RateLimitOptions = {}): RateLimitFn => { + const defaultOptions: RateLimitOptions = { + windowMs: 60 * 1000, // Apply rate per-minute + max: 1200, + handler: (req, _res, next) => { + logger.warn({ + message: 'Rate limit exceeded', + meta: { + action: 'limitRate', + url: req.url, + ip: getRequestIp(req), + method: req.method, + rateLimitInfo: req.rateLimit, + }, + }) + // TODO (private #49): terminate the request with HTTP 429 + return next() + }, + } + return RateLimit(merge(defaultOptions, options)) +} diff --git a/src/app/utils/mail.ts b/src/app/utils/mail.ts index 3495d8cbcc..f2d3a2f1c9 100644 --- a/src/app/utils/mail.ts +++ b/src/app/utils/mail.ts @@ -128,9 +128,7 @@ export const isToFieldValid = (addresses: string | string[]): boolean => { const mails = flattenDeep( flattenDeep([addresses]).map((addrString) => String(addrString) - // Split by both commas and semicolons, as some legacy emails are - // delimited by semicolons. - .split(/,|;/) + .split(',') .map((addr) => addr.trim()), ), ) diff --git a/src/app/utils/request.ts b/src/app/utils/request.ts index f8e0c1ca0c..377b4c2850 100644 --- a/src/app/utils/request.ts +++ b/src/app/utils/request.ts @@ -1,5 +1,14 @@ import { Request } from 'express' -export const getRequestIp = (req: Request) => { +interface IRequestWithId extends Request { + // Need to extend Request interface as id is not present in native express + id?: string +} + +export const getRequestIp = (req: IRequestWithId) => { return req.get('cf-connecting-ip') ?? req.ip } + +export const getTrace = (req: IRequestWithId) => { + return req.get('cf-ray') ?? req.id // trace using cloudflare cf-ray header, with x-request-id header as backup +} diff --git a/src/config/config.ts b/src/config/config.ts index 8d91c6034b..592a4a62cb 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -226,6 +226,7 @@ const config: Config = { isLoginBanner: basicVars.banner.isLoginBanner, siteBannerContent: basicVars.banner.siteBannerContent, adminBannerContent: basicVars.banner.adminBannerContent, + rateLimitConfig: basicVars.rateLimit, configureAws, } diff --git a/src/config/feature-manager/index.ts b/src/config/feature-manager/index.ts index cf39394a8a..eaf41a85d4 100644 --- a/src/config/feature-manager/index.ts +++ b/src/config/feature-manager/index.ts @@ -8,6 +8,8 @@ import spcpMyInfo from './spcp-myinfo.config' import verifiedFields from './verified-fields.config' import webhookVerifiedContent from './webhook-verified-content.config' +export * from './types' + const featureManager = new FeatureManager() // Register features and associated middleware/fallbacks diff --git a/src/config/feature-manager/types.ts b/src/config/feature-manager/types.ts index d9e6a98c75..6311f76413 100644 --- a/src/config/feature-manager/types.ts +++ b/src/config/feature-manager/types.ts @@ -80,6 +80,11 @@ export interface IFeatureManager { [FeatureNames.WebhookVerifiedContent]: IWebhookVerifiedContent } +export interface RegisteredFeature { + isEnabled: boolean + props?: IFeatureManager[T] +} + export interface RegisterableFeature { name: K schema: Schema diff --git a/src/config/feature-manager/util/FeatureManager.class.ts b/src/config/feature-manager/util/FeatureManager.class.ts index 09e5aaa84b..77e5f7f146 100644 --- a/src/config/feature-manager/util/FeatureManager.class.ts +++ b/src/config/feature-manager/util/FeatureManager.class.ts @@ -3,16 +3,16 @@ import validator from 'convict-format-with-validator' import _ from 'lodash' import { createLoggerWithLabel } from '../../logger' -import { FeatureNames, IFeatureManager, RegisterableFeature } from '../types' +import { + FeatureNames, + IFeatureManager, + RegisterableFeature, + RegisteredFeature, +} from '../types' const logger = createLoggerWithLabel(module) convict.addFormat(validator.url) -interface RegisteredFeature { - isEnabled: boolean - props: IFeatureManager[T] -} - export default class FeatureManager { public states: Partial> // Map some feature names to some env vars @@ -96,9 +96,9 @@ export default class FeatureManager { * Return props registered for requested feature * @param name the feature to return properties for */ - props(name: K) { + props(name: K): IFeatureManager[K] { if (this.states[name] !== undefined) { - return this.properties[name] + return this.properties[name] as IFeatureManager[K] } // Not enabled or not in state. throw new Error(`A feature called ${name} does not exist`) @@ -109,11 +109,10 @@ export default class FeatureManager { * and whether requested feature is enabled * @param name the name of the feature to return */ - get(name: FeatureNames): RegisteredFeature { + get(name: K): RegisteredFeature { return { isEnabled: this.isEnabled(name), - // TODO (#317): remove usage of non-null assertion - props: this.props(name)!, + props: this.props(name), } } } diff --git a/src/config/formsg-sdk.ts b/src/config/formsg-sdk.ts index 84aa64ea8e..5dda108394 100644 --- a/src/config/formsg-sdk.ts +++ b/src/config/formsg-sdk.ts @@ -3,9 +3,8 @@ import { get } from 'lodash' import * as vfnConstants from '../shared/util/verification' -import { FeatureNames } from './feature-manager/types' import { formsgSdkMode } from './config' -import featureManager from './feature-manager' +import featureManager, { FeatureNames } from './feature-manager' const formsgSdk = formsgSdkPackage({ webhookSecretKey: get( diff --git a/src/config/schema.ts b/src/config/schema.ts index d725ff29d8..f7953860aa 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -266,6 +266,21 @@ export const optionalVarsSchema: Schema = { env: 'NODE_ENV', }, }, + rateLimit: { + submissions: { + doc: 'Per-minute, per-IP request limit for submissions endpoints', + format: 'int', + default: 200, + env: 'SUBMISSIONS_RATE_LIMIT', + }, + sendAuthOtp: { + doc: + 'Per-minute, per-IP request limit for OTPs to log in to the admin console', + format: 'int', + default: 60, + env: 'SEND_AUTH_OTP_RATE_LIMIT', + }, + }, } export const prodOnlyVarsSchema: Schema = { diff --git a/src/loaders/express/error-handler.ts b/src/loaders/express/error-handler.ts index 27f532203b..d04b26b99a 100644 --- a/src/loaders/express/error-handler.ts +++ b/src/loaders/express/error-handler.ts @@ -42,7 +42,7 @@ const errorHandlerMiddlewares = (): ( }) return res .status(StatusCodes.BAD_REQUEST) - .send('Some required parameters are missing') + .json({ message: 'Some required parameters are missing' }) } logger.error({ @@ -54,7 +54,7 @@ const errorHandlerMiddlewares = (): ( }) return res .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send({ message: genericErrorMessage }) + .json({ message: genericErrorMessage }) } } @@ -63,7 +63,7 @@ const errorHandlerMiddlewares = (): ( _req, res, ) { - res.status(StatusCodes.NOT_FOUND).send() + res.sendStatus(StatusCodes.NOT_FOUND) } return [genericErrorHandlerMiddleware, catchNonExistentRoutesMiddleware] diff --git a/src/loaders/express/helmet.ts b/src/loaders/express/helmet.ts index 008c0b909a..05e6da84a6 100644 --- a/src/loaders/express/helmet.ts +++ b/src/loaders/express/helmet.ts @@ -4,8 +4,7 @@ import { ContentSecurityPolicyOptions } from 'helmet/dist/middlewares/content-se import { get } from 'lodash' import config from '../../config/config' -import featureManager from '../../config/feature-manager' -import { FeatureNames } from '../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../config/feature-manager' const helmetMiddlewares = () => { // Only add the "Strict-Transport-Security" header if request is https. diff --git a/src/loaders/express/index.ts b/src/loaders/express/index.ts index 92cf6be6f6..e7aab1b535 100644 --- a/src/loaders/express/index.ts +++ b/src/loaders/express/index.ts @@ -1,12 +1,14 @@ import compression from 'compression' import express, { Express } from 'express' import device from 'express-device' +import addRequestId from 'express-request-id' import http from 'http' import { Connection } from 'mongoose' import nocache from 'nocache' import path from 'path' import url from 'url' +import { AnalyticsRouter } from '../../app/modules/analytics/analytics.routes' import { AuthRouter } from '../../app/modules/auth/auth.routes' import { BounceRouter } from '../../app/modules/bounce/bounce.routes' import UserRouter from '../../app/modules/user/user.routes' @@ -105,6 +107,9 @@ const loadExpressApp = async (connection: Connection) => { app.use(nocache()) // Add headers to prevent browser caching front-end code + // Generate UUID for request and add it to X-Request-Id header + app.use(addRequestId()) + // Setting the app static folder app.use('/public', express.static(path.resolve('./dist/frontend'))) @@ -130,6 +135,7 @@ const loadExpressApp = async (connection: Connection) => { app.use('/user', UserRouter) app.use('/emailnotifications', BounceRouter) app.use('/transaction', VfnRouter) + app.use('/analytics', AnalyticsRouter) app.use(sentryMiddlewares()) diff --git a/src/loaders/express/locals.ts b/src/loaders/express/locals.ts index 5439d35184..a3e5fe77f2 100644 --- a/src/loaders/express/locals.ts +++ b/src/loaders/express/locals.ts @@ -2,8 +2,7 @@ import ejs from 'ejs' import { get } from 'lodash' import config from '../../config/config' -import featureManager from '../../config/feature-manager' -import { FeatureNames } from '../../config/feature-manager/types' +import featureManager, { FeatureNames } from '../../config/feature-manager' // Construct js with environment variables needed by frontend const frontendVars = { diff --git a/src/loaders/express/logging.ts b/src/loaders/express/logging.ts index bfed0e8006..c1538ddd30 100644 --- a/src/loaders/express/logging.ts +++ b/src/loaders/express/logging.ts @@ -2,6 +2,7 @@ import expressWinston from 'express-winston' import get from 'lodash/get' import winston from 'winston' +import { getTrace } from '../../app/utils/request' import config from '../../config/config' import { customFormat } from '../../config/logger' @@ -12,6 +13,7 @@ type LogMeta = { userId: string contentLength?: string transactionId?: string + trace?: string } const loggingMiddleware = () => { @@ -44,6 +46,7 @@ const loggingMiddleware = () => { // Define our own token for client ip // req.headers['cf-connecting-ip'] : Cloudflare // req.ip : Contains the remote IP address of the request. + // trace: use cloudflare cf-ray header, with x-request-id header as backup // If trust proxy setting is true, the value of this property is // derived from the left-most entry in the X-Forwarded-For header. // This header can be set by the client or by the proxy. @@ -52,6 +55,7 @@ const loggingMiddleware = () => { // req.connection.remoteAddress. clientIp: req.get('cf-connecting-ip') || req.ip, userId: get(req, 'session.user._id', ''), + trace: getTrace(req), } const contentLength = res.get('content-length') diff --git a/src/loaders/mongoose.ts b/src/loaders/mongoose.ts index 2b14368727..6c135111af 100644 --- a/src/loaders/mongoose.ts +++ b/src/loaders/mongoose.ts @@ -97,7 +97,7 @@ export default async (): Promise => { emailDomain: 'data.gov.sg', logo: '/public/modules/core/img/govtech.jpg', }) - agency.save() + await agency.save() } return mongoose.connection diff --git a/src/public/modules/core/componentViews/avatar-dropdown.html b/src/public/modules/core/componentViews/avatar-dropdown.html index 874653b06a..37e3c975fd 100644 --- a/src/public/modules/core/componentViews/avatar-dropdown.html +++ b/src/public/modules/core/componentViews/avatar-dropdown.html @@ -11,7 +11,9 @@ ng-mouseleave="vm.isDropdownHover=false" > -
!
+
+ ! +
    diff --git a/src/public/modules/core/components/avatar-dropdown.client.component.js b/src/public/modules/core/components/avatar-dropdown.client.component.js index 2e0f9b9244..2f21f54df9 100644 --- a/src/public/modules/core/components/avatar-dropdown.client.component.js +++ b/src/public/modules/core/components/avatar-dropdown.client.component.js @@ -51,6 +51,10 @@ function avatarDropdownController( if (trueUser.contact) return const features = await Features.getFeatureStates() + + // Only show exclamation mark in avatar if sms feature is enabled. + vm.showExclamation = features.sms && !vm.user.contact + // Do not proceed if sms feature is not available. if (!features.sms) return diff --git a/src/public/modules/core/css/avatar-dropdown.css b/src/public/modules/core/css/avatar-dropdown.css index fbffccc786..e4fb20b7d7 100644 --- a/src/public/modules/core/css/avatar-dropdown.css +++ b/src/public/modules/core/css/avatar-dropdown.css @@ -115,15 +115,6 @@ } .navbar__dropdown__user__avatar { - display: -webkit-inline-box; - display: -ms-inline-flexbox; - display: inline-flex; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - -webkit-box-pack: center; - -ms-flex-pack: center; - justify-content: center; color: #fff; text-transform: uppercase; height: 40px; @@ -134,6 +125,9 @@ border-radius: 50%; outline: none; margin-right: 16px; + display: inline-table; + text-align: center; + line-height: 36px; } .navbar__dropdown__user__details { @@ -143,6 +137,7 @@ -webkit-box-align: center; -ms-flex-align: center; align-items: center; + word-break: break-all; } .navbar__dropdown__user__details--email { diff --git a/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js b/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js index 8ac932fed2..0e2fd74c54 100644 --- a/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js @@ -57,21 +57,14 @@ function CollaboratorModalController( $scope.lockCollaboratorScroll = false $scope.isDisplayTransferOwnerModal = false - $scope.isDisplayTransferSuccessMessage = false - $scope.isDisplayAlertMessage = false $scope.transferOwnerEmail = undefined - $scope.successMessage = undefined - $scope.alertMessage = undefined /** * Transfers ownership of the form to the selected user, reset UI messages */ $scope.transferOwner = () => { - $scope.resetMessages() - if ($scope.transferOwnerEmail === $scope.myform.admin.email) { - $scope.isDisplayAlertMessage = true - $scope.alertMessage = 'You are already the owner of this form' + Toastr.error('You are already the owner of this form') $scope.isDisplayTransferOwnerModal = false return } @@ -83,11 +76,11 @@ function CollaboratorModalController( .$promise.then((res) => { $scope.myform = res.form externalScope.refreshFormDataFromCollab($scope.myform) - $scope.isDisplayTransferSuccessMessage = true + Toastr.success('Form ownership transferred. You are now an Editor.') }) .catch((err) => { - $scope.alertMessage = err.data.message - $scope.isDisplayAlertMessage = true + Toastr.error(err.data.message) + return }) .finally(() => { $scope.isDisplayTransferOwnerModal = false @@ -108,8 +101,7 @@ function CollaboratorModalController( externalScope.refreshFormDataFromCollab($scope.myform) }) .catch((err) => { - $scope.alertMessage = err.data.message - $scope.isDisplayAlertMessage = true + Toastr.error(err.data.message) return err }) } @@ -121,8 +113,6 @@ function CollaboratorModalController( * @param {Role} newRole - The selected role */ $scope.updateRole = function (index, newRole) { - $scope.resetMessages() - if ( $scope.myform.permissionList && index > -1 && @@ -194,8 +184,6 @@ function CollaboratorModalController( * Admins, the user themselves, and users already in the permissionList cannot be added to the form. */ $scope.saveCollabEmail = () => { - $scope.resetMessages() - let email = $scope.collab.form.email.toLowerCase() let permissions = $scope.roleToPermissions($scope.collab.form.role) @@ -207,9 +195,7 @@ function CollaboratorModalController( ) || email === $scope.myform.admin.email ) { - $scope.isDisplayAlertMessage = true - $scope.alertMessage = - 'This user is an existing collaborator. Edit role below.' + Toastr.error('This user is an existing collaborator. Edit role below.') return } @@ -233,20 +219,19 @@ function CollaboratorModalController( if (err) { // Make the alert message correspond to the error code if (err.status === HttpStatus.BAD_REQUEST) { - $scope.alertMessage = 'Outdated admin page, please refresh' + Toastr.error('Outdated admin page, please refresh.') } else if (err.status === HttpStatus.UNPROCESSABLE_ENTITY) { - $scope.alertMessage = `${email} is not part of a whitelisted agency` + Toastr.error(`${email} is not part of a whitelisted agency.`) } else { - $scope.alertMessage = 'Error adding collaborator' + Toastr.error('Error adding collaborator.') } resetCollabForm() - $scope.resetMessages() return } // If no error, clear email input $scope.btnStatus = 3 // pressed; saved $scope.closeEditCollaboratorDropdowns() - $scope.resetMessages() + $timeout(() => { resetCollabForm() }, 1000) @@ -279,14 +264,6 @@ function CollaboratorModalController( $scope.lockCollaboratorScroll = false } - /** - * Clear all error and success messages - */ - $scope.resetMessages = () => { - $scope.isDisplayTransferSuccessMessage = false - $scope.isDisplayAlertMessage = false - } - /** * Display transfer ownership verification view */ diff --git a/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js b/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js index 4db7718a63..753cec2386 100644 --- a/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js +++ b/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js @@ -1,5 +1,9 @@ 'use strict' +const { + validateEmailDomains, +} = require('../../../../../shared/util/email-domain-validation') + angular .module('forms') .directive('validateEmailDomainFromText', validateEmailDomainFromText) @@ -20,11 +24,7 @@ function validateEmailDomainFromText() { .split('\n') .map((s) => s.trim()) .filter((s) => s) - if (emailDomains.length === 0) return true - const validDedupedDomainsSet = new Set( - emailDomains.filter((s) => s.match(/@.+\..+/)), - ) - return validDedupedDomainsSet.size === emailDomains.length + return validateEmailDomains(emailDomains) } }, } diff --git a/src/public/modules/forms/admin/views/collaborator.client.modal.html b/src/public/modules/forms/admin/views/collaborator.client.modal.html index 93a02959d4..ca9e8d514d 100644 --- a/src/public/modules/forms/admin/views/collaborator.client.modal.html +++ b/src/public/modules/forms/admin/views/collaborator.client.modal.html @@ -36,23 +36,6 @@
-
- - {{ alertMessage }} -
-
- - - Form ownership transferred. You are now an Editor. - -
-
diff --git a/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js b/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js index f4caabca69..095857d8c2 100644 --- a/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js +++ b/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js @@ -12,6 +12,10 @@ function validateEmailDomain() { ? new Set(scope.vm.field.allowedEmailDomains) : new Set() ngModel.$validators.emailDomainValidator = (emailAddress) => { + // Early return, do not even check domains if string is empty. + if (!emailAddress) { + return true + } if (allowedEmailDomains.size) { const emailDomain = emailAddress.split('@').pop() return allowedEmailDomains.has('@' + emailDomain) diff --git a/src/server.ts b/src/server.ts index 2339986064..9b3de0eabd 100755 --- a/src/server.ts +++ b/src/server.ts @@ -21,4 +21,4 @@ const initServer = async () => { }) } -initServer() +void initServer() diff --git a/src/shared/util/email-domain-validation.ts b/src/shared/util/email-domain-validation.ts new file mode 100644 index 0000000000..061201fb92 --- /dev/null +++ b/src/shared/util/email-domain-validation.ts @@ -0,0 +1,14 @@ +import { isEmpty } from 'lodash' +import validator from 'validator' + +export const validateEmailDomains = (emailDomains: string[]): boolean => { + // We need to prepend "bob" to the email domain so that it can form an email address and can be + // validated by validator.isEmail. + return ( + isEmpty(emailDomains) || + (new Set(emailDomains).size === emailDomains.length && + emailDomains.every((emailDomain) => + validator.isEmail('bob' + emailDomain), + )) + ) +} diff --git a/src/types/config.ts b/src/types/config.ts index 184c126672..9e0282ba70 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -46,6 +46,11 @@ export type MailConfig = { transporter: Mail } +export type RateLimitConfig = { + submissions: number + sendAuthOtp: number +} + export type Config = { app: AppConfig db: DbConfig @@ -68,6 +73,7 @@ export type Config = { isLoginBanner: string siteBannerContent: string adminBannerContent: string + rateLimitConfig: RateLimitConfig // Functions configureAws: () => Promise @@ -122,6 +128,10 @@ export interface IOptionalVarsSchema { maxConnections: number socketTimeout: number } + rateLimit: { + submissions: number + sendAuthOtp: number + } } export interface IBucketUrlSchema { diff --git a/src/types/form_statistics_total.ts b/src/types/form_statistics_total.ts index 4ca46a9ba9..d0d66dde8a 100644 --- a/src/types/form_statistics_total.ts +++ b/src/types/form_statistics_total.ts @@ -1,6 +1,9 @@ -import { Document } from 'mongoose' +import { Document, Model } from 'mongoose' + +import { IFormSchema } from './form' export interface IFormStatisticsTotal { + formId: IFormSchema['_id'] totalCount: number lastSubmission: Date _id: Document['_id'] @@ -9,3 +12,12 @@ export interface IFormStatisticsTotal { export interface IFormStatisticsTotalSchema extends IFormStatisticsTotal, Document {} + +export type AggregateFormCountResult = + | [{ numActiveForms: number } | undefined] + | never[] + +export interface IFormStatisticsTotalModel + extends Model { + aggregateFormCount(minSubCount: number): Promise +} diff --git a/src/types/index.ts b/src/types/index.ts index b1e4ab0b98..ae47076e39 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -10,7 +10,6 @@ export * from './login' export * from './mail' export * from './myinfo_hash' export * from './response' -export * from './sms_count' export * from './sns' export * from './submission' export * from './token' diff --git a/src/types/submission.ts b/src/types/submission.ts index 70cf5bd3a7..6ac39ce25d 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -11,12 +11,12 @@ export enum SubmissionType { export interface ISubmission { form: IFormSchema['_id'] - authType: AuthType - myInfoFields: MyInfoAttribute[] + authType?: AuthType + myInfoFields?: MyInfoAttribute[] submissionType: SubmissionType created?: Date lastModified?: Date - _id: Document['_id'] + _id?: Document['_id'] recipientEmails?: string[] responseHash?: string responseSalt?: string @@ -42,9 +42,21 @@ export interface WebhookView { } export interface ISubmissionSchema extends ISubmission, Document { + _id: Document['_id'] getWebhookView(): WebhookView | null } +export type FindFormsWithSubsAboveResult = { + _id: IFormSchema['_id'] + count: number +} + +export interface ISubmissionModel extends Model { + findFormsWithSubsAbove( + minSubCount: number, + ): Promise +} + export interface IEmailSubmission extends ISubmission { recipientEmails: string[] responseHash: string @@ -85,7 +97,9 @@ export interface IWebhookResponse { } } -export type IEmailSubmissionModel = Model -export type IEncryptSubmissionModel = Model +export type IEmailSubmissionModel = Model & + ISubmissionModel +export type IEncryptSubmissionModel = Model & + ISubmissionModel export interface IWebhookResponseSchema extends IWebhookResponse, Document {} diff --git a/tests/unit/backend/controllers/core.server.controller.spec.js b/tests/unit/backend/controllers/core.server.controller.spec.js index 2d39975bee..d3a2b7f1f5 100644 --- a/tests/unit/backend/controllers/core.server.controller.spec.js +++ b/tests/unit/backend/controllers/core.server.controller.spec.js @@ -1,23 +1,9 @@ const mongoose = require('mongoose') const dbHandler = require('../helpers/db-handler') -const User = dbHandler.makeModel('user.server.model', 'User') -const Agency = dbHandler.makeModel('agency.server.model', 'Agency') -const Submission = dbHandler.makeModel('submission.server.model', 'Submission') -const FormStatisticsTotal = dbHandler.makeModel( - 'form_statistics_total.server.model', - 'FormStatisticsTotal', -) describe('Core Controller', () => { // Declare global variables - - const testAgency = new Agency({ - shortName: 'govtest', - fullName: 'Government Testing Agency', - emailDomain: 'test.gov.sg', - logo: '/invalid-path/test.jpg', - }) const Controller = spec( 'dist/backend/app/controllers/core.server.controller', { @@ -58,122 +44,4 @@ describe('Core Controller', () => { }) }) }) - - describe('formCount', () => { - it('should return the number of forms with more than 10 submissions from form stats collection', (done) => { - // Number of submissions per form - let formCounts = [12, 10, 4, 20] - let submissionPromises = [] - formCounts.forEach((count) => { - let formId = mongoose.Types.ObjectId() - submissionPromises.push( - new FormStatisticsTotal({ - form: formId, - totalCount: count, - lastSubmission: new Date(), - }).save(), - ) - }) - Promise.all(submissionPromises) - .then(() => { - res.json.and.callFake(() => { - expect(res.json).toHaveBeenCalledWith( - _.filter(formCounts, (fc) => fc > 10).length, - ) - done() - }) - Controller.formCountUsingAggregateCollection(req, res) - }) - .catch(done) - }) - - it('should return the number of forms with more than 10 submissions from submissions collection', (done) => { - // Number of submissions per form - let formCounts = [ - { - formId: mongoose.Types.ObjectId(), - count: 12, - }, - { - formId: mongoose.Types.ObjectId(), - count: 10, - }, - { - formId: mongoose.Types.ObjectId(), - count: 4, - }, - ] - let submissionPromises = [] - formCounts.forEach(({ formId, count }) => { - while (count > 0) { - submissionPromises.push( - new Submission({ - form: formId, - myInfoFields: [], - submissionType: 'emailSubmission', - responseHash: 'hash', - responseSalt: 'salt', - }).save(), - ) - count-- - } - }) - Promise.all(submissionPromises) - .then(() => { - res.json.and.callFake(() => { - expect(res.json).toHaveBeenCalledWith( - _.filter(formCounts, (fc) => fc.count > 10).length, - ) - done() - }) - Controller.formCountUsingSubmissionsCollection(req, res) - }) - .catch(done) - }) - }) - - describe('userCount', () => { - it('should return the number of users', (done) => { - // Number of users to populate - let numUsers = 30 - testAgency - .save() - .then(() => { - let userPromises = _.times(numUsers, (time) => - new User({ - email: `user${time}@test.gov.sg`, - agency: mongoose.Types.ObjectId(), - }).save(), - ) - return Promise.all(userPromises) - }) - .then(() => { - res.json.and.callFake(() => { - expect(res.json).toHaveBeenCalledWith(numUsers) - done() - }) - Controller.userCount(req, res) - }) - .catch(done) - }) - }) - - describe('submissionCount', () => { - it('should return the number of submissions', (done) => { - // Number of submissions to populate - let numSubmissions = 300 - let submissionPromises = _.times(numSubmissions, () => - new Submission({ form: mongoose.Types.ObjectId() }).save(), - ) - Promise.all(submissionPromises) - .then(() => { - res.json.and.callFake(() => { - expect(res.json).toHaveBeenCalledWith(numSubmissions) - done() - }) - Controller.submissionCount(req, res) - }) - .catch(done) - }) - }) }) diff --git a/tests/unit/backend/controllers/forms.server.controller.spec.js b/tests/unit/backend/controllers/forms.server.controller.spec.js index 94afd5eb85..9749eedfaf 100644 --- a/tests/unit/backend/controllers/forms.server.controller.spec.js +++ b/tests/unit/backend/controllers/forms.server.controller.spec.js @@ -57,7 +57,7 @@ describe('Form Controller', () => { 'betaFlags', ]) Controller.read(Controller.REQUEST_TYPE.ADMIN)(req, res) - expect(res.send).toHaveBeenCalledWith({ + expect(res.json).toHaveBeenCalledWith({ form: expectedForm, spcpSession: res.locals.spcpSession, myInfoError: res.locals.myInfoError, @@ -95,7 +95,7 @@ describe('Form Controller', () => { ]) Controller.read(Controller.REQUEST_TYPE.PUBLIC)(req, res) - expect(res.send).toHaveBeenCalledWith({ + expect(res.json).toHaveBeenCalledWith({ form: expectedForm, spcpSession: res.locals.spcpSession, myInfoError: res.locals.myInfoError, diff --git a/tests/unit/backend/controllers/myinfo.server.controller.spec.js b/tests/unit/backend/controllers/myinfo.server.controller.spec.js index 1ced28c6f3..2759682568 100644 --- a/tests/unit/backend/controllers/myinfo.server.controller.spec.js +++ b/tests/unit/backend/controllers/myinfo.server.controller.spec.js @@ -686,6 +686,7 @@ describe('MyInfo Controller', () => { ), ).toBeTruthy() expect(res.send).not.toHaveBeenCalled() + expect(res.json).not.toHaveBeenCalled() done() }) Controller.verifyMyInfoVals(req, res, next) diff --git a/tests/unit/backend/controllers/public-forms.server.controller.spec.js b/tests/unit/backend/controllers/public-forms.server.controller.spec.js index da933cb960..c29df9e2d4 100644 --- a/tests/unit/backend/controllers/public-forms.server.controller.spec.js +++ b/tests/unit/backend/controllers/public-forms.server.controller.spec.js @@ -170,7 +170,7 @@ describe('Public-Forms Controller', () => { expect(args).toBe(StatusCodes.OK) return res }) - res.send.and.callFake(() => { + res.json.and.callFake(() => { FormFeedback.findOne( { formId: req.params.formId, diff --git a/tests/unit/backend/controllers/spcp.server.controller.spec.js b/tests/unit/backend/controllers/spcp.server.controller.spec.js index 888c0fd132..3e3af1e46d 100644 --- a/tests/unit/backend/controllers/spcp.server.controller.spec.js +++ b/tests/unit/backend/controllers/spcp.server.controller.spec.js @@ -67,6 +67,7 @@ describe('SPCP Controller', () => { 'redirect', 'cookie', 'locals', + 'sendStatus', ]) res.status.and.returnValue(res) res.send.and.returnValue(res) @@ -155,9 +156,11 @@ describe('SPCP Controller', () => { } const expectResponse = (code, content) => { - expect(res.status).toHaveBeenCalledWith(code) if (content) { - expect(res.send).toHaveBeenCalledWith(content) + expect(res.status).toHaveBeenCalledWith(code) + expect(res.json).toHaveBeenCalledWith(content) + } else { + expect(res.sendStatus).toHaveBeenCalledWith(code) } } @@ -177,22 +180,34 @@ describe('SPCP Controller', () => { it('should return 400 if target not provided', () => { req.query.target = '' Controller.createSpcpRedirectURL(authClients)(req, res, next) - expectResponse(StatusCodes.BAD_REQUEST) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith({ + message: 'Redirect URL malformed', + }) }) it('should return 400 if authType not provided', () => { req.query.authType = '' Controller.createSpcpRedirectURL(authClients)(req, res, next) - expectResponse(StatusCodes.BAD_REQUEST) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith({ + message: 'Redirect URL malformed', + }) }) it('should return 400 if esrvcId not provided', () => { req.query.esrvcId = '' Controller.createSpcpRedirectURL(authClients)(req, res, next) - expectResponse(StatusCodes.BAD_REQUEST) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith({ + message: 'Redirect URL malformed', + }) }) it('should return 400 if authType is NIL', () => { Controller.createSpcpRedirectURL(authClients)(req, res, next) - expectResponse(StatusCodes.BAD_REQUEST) + expect(res.status).toHaveBeenCalledWith(400) + expect(res.json).toHaveBeenCalledWith({ + message: 'Redirect URL malformed', + }) }) it('should return 200 and redirectUrl if authType is SP', () => { req.query.authType = 'SP' @@ -363,7 +378,7 @@ describe('SPCP Controller', () => { let expectedRelayState const expectBadRequestOnLogin = (statusCode, done) => { - res.status.and.callFake((status) => { + res.sendStatus.and.callFake((status) => { expect(status).toEqual(statusCode) done() return res diff --git a/tests/unit/backend/models/form.server.model.spec.ts b/tests/unit/backend/models/form.server.model.spec.ts index f850d5ed01..fa3beacb7b 100644 --- a/tests/unit/backend/models/form.server.model.spec.ts +++ b/tests/unit/backend/models/form.server.model.spec.ts @@ -522,6 +522,43 @@ describe('Form Model', () => { expect(Object.keys(saved)).not.toContain('extra') }) + it('should coerce comma-separated email string into an array of emails', async () => { + // Arrange + Act + const mockEmailsString = 'test1@b.com, test2@b.com' + const mockEmailsArray = ['test1@b.com', 'test2@b.com'] + const formParams = merge({}, MOCK_EMAIL_FORM_PARAMS, { + emails: mockEmailsString, + }) + const validForm = new EmailForm(formParams) + const saved = await validForm.save() + + // Assert + // Object Id should be defined when successfully saved to MongoDB. + expect(saved._id).toBeDefined() + expect(saved.created).toBeInstanceOf(Date) + expect(saved.lastModified).toBeInstanceOf(Date) + // Retrieve object and compare to params, remove indeterministic keys + // Remove permissionList too due to objects inside having + // indeterministic IDs, check separately. + const actualSavedObject = omit(saved.toObject(), [ + '_id', + 'created', + 'lastModified', + '__v', + 'emails', + 'permissionList', + ]) + const expectedObject = merge( + {}, + omit(EMAIL_FORM_DEFAULTS, 'permissionList'), + omit(MOCK_EMAIL_FORM_PARAMS, 'emails'), + ) + expect(actualSavedObject).toEqual(expectedObject) + + const actualEmails = saved.toObject().emails + expect(actualEmails).toEqual(mockEmailsArray) + }) + it('should create and save successfully with valid permissionList emails', async () => { // Arrange // permissionList has email with valid domain diff --git a/tests/unit/backend/models/form_statistics_total.server.model.spec.ts b/tests/unit/backend/models/form_statistics_total.server.model.spec.ts new file mode 100644 index 0000000000..b3270c14aa --- /dev/null +++ b/tests/unit/backend/models/form_statistics_total.server.model.spec.ts @@ -0,0 +1,74 @@ +import mongoose from 'mongoose' + +import getFormStatisticsTotalModel from 'src/app/models/form_statistics_total.server.model' +import { AggregateFormCountResult, IFormStatisticsTotalSchema } from 'src/types' + +import dbHandler from '../helpers/jest-db' + +const FormStatsModel = getFormStatisticsTotalModel(mongoose) + +describe('FormStatisticsTotal Model', () => { + beforeAll(async () => await dbHandler.connect()) + afterEach(async () => await dbHandler.clearDatabase()) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('Statics', () => { + describe('aggregateFormCount', () => { + it('should return number of active forms that is above the given minimum submission threshold', async () => { + // Number of submissions per form + const formCounts = [12, 10, 4, 20] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + submissionPromises.push( + FormStatsModel.create({ + formId: mongoose.Types.ObjectId(), + totalCount: count, + lastSubmission: new Date(), + }), + ) + }) + await Promise.all(submissionPromises) + const minSubCount = 10 + + // Act + const actualResult = await FormStatsModel.aggregateFormCount( + minSubCount, + ) + + // Assert + const expectedResult: AggregateFormCountResult = [ + { + numActiveForms: formCounts.filter((fc) => fc > minSubCount).length, + }, + ] + expect(actualResult).toEqual(expectedResult) + }) + + it('should return empty array if no documents meet the given submission threshold', async () => { + // Number of submissions per form + const formCounts = [12, 10, 4] + const submissionPromises: Promise[] = [] + formCounts.forEach((count) => { + submissionPromises.push( + FormStatsModel.create({ + formId: mongoose.Types.ObjectId(), + totalCount: count, + lastSubmission: new Date(), + }), + ) + }) + await Promise.all(submissionPromises) + const minSubCount = 12 + + // Act + const actualResult = await FormStatsModel.aggregateFormCount( + minSubCount, + ) + + // Assert + const expectedResult: AggregateFormCountResult = [] + expect(actualResult).toEqual(expectedResult) + }) + }) + }) +}) diff --git a/tests/unit/backend/models/submission.server.model.spec.ts b/tests/unit/backend/models/submission.server.model.spec.ts index 9950990e58..0de2776b9a 100644 --- a/tests/unit/backend/models/submission.server.model.spec.ts +++ b/tests/unit/backend/models/submission.server.model.spec.ts @@ -1,73 +1,155 @@ import { ObjectID } from 'bson' +import { times } from 'lodash' import mongoose from 'mongoose' import getSubmissionModel from 'src/app/models/submission.server.model' -import { AuthType, SubmissionType } from '../../../../src/types' +import { + AuthType, + ISubmissionSchema, + SubmissionType, +} from '../../../../src/types' import dbHandler from '../helpers/jest-db' const Submission = getSubmissionModel(mongoose) // TODO: Add more tests for the rest of the submission schema. -describe('Submission Schema', () => { +describe('Submission Model', () => { beforeAll(async () => await dbHandler.connect()) afterEach(async () => await dbHandler.clearDatabase()) afterAll(async () => await dbHandler.closeDatabase()) const MOCK_ENCRYPTED_CONTENT = 'abcdefg encryptedContent' - describe('methods.getWebhookView', () => { - it('should return non-null view with encryptedSubmission type (without verified content)', async () => { - // Arrange - const formId = new ObjectID() - - const submission = await Submission.create({ - submissionType: SubmissionType.Encrypt, - form: formId, - encryptedContent: MOCK_ENCRYPTED_CONTENT, - version: 1, - authType: AuthType.NIL, - myInfoFields: [], - webhookResponses: [], + describe('Statics', () => { + describe('findFormsWithSubsAbove', () => { + it('should return ids and counts of forms with more than given minimum submissions', async () => { + // Arrange + const formCounts = [4, 2, 4] + const formIdsAndCounts = times(formCounts.length, (it) => ({ + _id: mongoose.Types.ObjectId(), + count: formCounts[it], + })) + const submissionPromises: Promise[] = [] + formIdsAndCounts.forEach(({ count, _id: formId }) => { + times(count, () => + submissionPromises.push( + Submission.create({ + form: formId, + myInfoFields: [], + submissionType: SubmissionType.Email, + responseHash: 'hash', + responseSalt: 'salt', + }), + ), + ) + }) + await Promise.all(submissionPromises) + const minSubCount = 3 + + // Act + const actualResult = await Submission.findFormsWithSubsAbove( + minSubCount, + ) + + // Assert + const expectedResult = formIdsAndCounts.filter( + ({ count }) => count > minSubCount, + ) + expect(actualResult).toEqual(expect.arrayContaining(expectedResult)) }) - // Act - const actualWebhookView = submission.getWebhookView() + it('should return an empty array if no forms have submission counts higher than given count', async () => { + // Arrange + const formCounts = [1, 1, 2] + const formIdsAndCounts = times(formCounts.length, (it) => ({ + _id: mongoose.Types.ObjectId(), + count: formCounts[it], + })) + const submissionPromises: Promise[] = [] + formIdsAndCounts.forEach(({ count, _id: formId }) => { + times(count, () => + submissionPromises.push( + Submission.create({ + form: formId, + myInfoFields: [], + submissionType: SubmissionType.Email, + responseHash: 'hash', + responseSalt: 'salt', + }), + ), + ) + }) + await Promise.all(submissionPromises) + // Tests for greater than, should not return even if equal to some + // submission count. + const minSubCount = 2 - // Assert - expect(actualWebhookView).toEqual({ - data: { - formId: expect.any(String), - submissionId: expect.any(String), - created: expect.any(Date), - encryptedContent: MOCK_ENCRYPTED_CONTENT, - verifiedContent: undefined, - version: 1, - }, + // Act + const actualResult = await Submission.findFormsWithSubsAbove( + minSubCount, + ) + + // Assert + expect(actualResult).toEqual([]) }) }) + }) - it('should return null view with non-encryptSubmission type', async () => { - // Arrange - const formId = new ObjectID() - const submission = await Submission.create({ - submissionType: SubmissionType.Email, - form: formId, - encryptedContent: MOCK_ENCRYPTED_CONTENT, - version: 1, - authType: AuthType.NIL, - myInfoFields: [], - recipientEmails: [], - responseHash: 'hash', - responseSalt: 'salt', - hasBounced: false, + describe('Methods', () => { + describe('getWebhookView', () => { + it('should return non-null view with encryptedSubmission type (without verified content)', async () => { + // Arrange + const formId = new ObjectID() + + const submission = await Submission.create({ + submissionType: SubmissionType.Encrypt, + form: formId, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 1, + authType: AuthType.NIL, + myInfoFields: [], + webhookResponses: [], + }) + + // Act + const actualWebhookView = submission.getWebhookView() + + // Assert + expect(actualWebhookView).toEqual({ + data: { + formId: expect.any(String), + submissionId: expect.any(String), + created: expect.any(Date), + encryptedContent: MOCK_ENCRYPTED_CONTENT, + verifiedContent: undefined, + version: 1, + }, + }) }) - // Act - const actualWebhookView = submission.getWebhookView() + it('should return null view with non-encryptSubmission type', async () => { + // Arrange + const formId = new ObjectID() + const submission = await Submission.create({ + submissionType: SubmissionType.Email, + form: formId, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 1, + authType: AuthType.NIL, + myInfoFields: [], + recipientEmails: [], + responseHash: 'hash', + responseSalt: 'salt', + hasBounced: false, + }) + + // Act + const actualWebhookView = submission.getWebhookView() - // Assert - expect(actualWebhookView).toBeNull() + // Assert + expect(actualWebhookView).toBeNull() + }) }) }) }) diff --git a/tests/unit/backend/modules/user/user.controller.spec.ts b/tests/unit/backend/modules/user/user.controller.spec.ts index d057cfdc02..cb1600bd94 100644 --- a/tests/unit/backend/modules/user/user.controller.spec.ts +++ b/tests/unit/backend/modules/user/user.controller.spec.ts @@ -1,16 +1,16 @@ import { StatusCodes } from 'http-status-codes' import { mocked } from 'ts-jest/utils' -import SmsFactory from 'src/app/factories/sms.factory' import * as UserController from 'src/app/modules/user/user.controller' import { InvalidOtpError } from 'src/app/modules/user/user.errors' import * as UserService from 'src/app/modules/user/user.service' +import { SmsFactory } from 'src/app/services/sms/sms.factory' import { IPopulatedUser, IUser, IUserSchema } from 'src/types' import expressHandler from '../../helpers/jest-express' jest.mock('src/app/modules/user/user.service') -jest.mock('src/app/factories/sms.factory') +jest.mock('src/app/services/sms/sms.factory') const MockUserService = mocked(UserService) const MockSmsFactory = mocked(SmsFactory) @@ -200,7 +200,7 @@ describe('user.controller', () => { MOCK_REQ.body.userId, ) expect(mockRes.status).toBeCalledWith(StatusCodes.OK) - expect(mockRes.send).toBeCalledWith(MOCK_UPDATED_USER) + expect(mockRes.json).toBeCalledWith(MOCK_UPDATED_USER) }) it('should return 401 when user id is not in session', async () => { @@ -348,7 +348,7 @@ describe('user.controller', () => { await UserController.handleFetchUser(MOCK_REQ, mockRes, jest.fn()) // Assert - expect(mockRes.send).toBeCalledWith(mockPopulatedUser) + expect(mockRes.json).toBeCalledWith(mockPopulatedUser) }) it('should return 401 when user id is not in session', async () => { @@ -368,7 +368,7 @@ describe('user.controller', () => { // Assert // Should trigger unauthorized response. expect(mockRes.status).toBeCalledWith(StatusCodes.UNAUTHORIZED) - expect(mockRes.send).toBeCalledWith('User is unauthorized.') + expect(mockRes.json).toBeCalledWith({ message: 'User is unauthorized.' }) }) it('should return 500 when retrieved user is null', async () => { @@ -382,7 +382,9 @@ describe('user.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(StatusCodes.INTERNAL_SERVER_ERROR) - expect(mockRes.send).toBeCalledWith('Unable to retrieve user') + expect(mockRes.json).toBeCalledWith({ + message: 'Unable to retrieve user', + }) }) }) }) diff --git a/tests/unit/backend/modules/verification/verification.service.spec.ts b/tests/unit/backend/modules/verification/verification.service.spec.ts index babde3bff3..5df8847826 100644 --- a/tests/unit/backend/modules/verification/verification.service.spec.ts +++ b/tests/unit/backend/modules/verification/verification.service.spec.ts @@ -3,7 +3,6 @@ import { ObjectId } from 'bson' import mongoose from 'mongoose' import { mocked } from 'ts-jest/utils' -import smsFactory from 'src/app/factories/sms.factory' import getFormModel from 'src/app/models/form.server.model' import getVerificationModel from 'src/app/models/verification.server.model' import { @@ -14,6 +13,7 @@ import { verifyOtp, } from 'src/app/modules/verification/verification.service' import MailService from 'src/app/services/mail.service' +import { SmsFactory } from 'src/app/services/sms/sms.factory' import { generateOtp } from 'src/app/utils/otp' import formsgSdk from 'src/config/formsg-sdk' import { BasicField, IUserSchema, IVerificationSchema } from 'src/types' @@ -29,8 +29,8 @@ jest.mock('src/app/utils/otp') const MockGenerateOtp = mocked(generateOtp, true) jest.mock('src/config/formsg-sdk') const MockFormsgSdk = mocked(formsgSdk, true) -jest.mock('src/app/factories/sms.factory') -const MockSmsFactory = mocked(smsFactory, true) +jest.mock('src/app/services/sms/sms.factory') +const MockSmsFactory = mocked(SmsFactory, true) jest.mock('src/app/services/mail.service') const MockMailService = mocked(MailService, true) jest.mock('bcrypt') diff --git a/tests/unit/backend/services/mail.service.spec.ts b/tests/unit/backend/services/mail.service.spec.ts index 2f5811d02e..6a03ffc962 100644 --- a/tests/unit/backend/services/mail.service.spec.ts +++ b/tests/unit/backend/services/mail.service.spec.ts @@ -91,8 +91,7 @@ describe('mail.service', () => { it('should send verification otp successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -103,7 +102,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -126,14 +125,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -144,7 +142,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSendVerification).resolves.toEqual(mockedResponse) + await expect(pendingSendVerification).resolves.toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -232,8 +230,7 @@ describe('mail.service', () => { it('should send login otp successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -246,7 +243,7 @@ describe('mail.service', () => { // Assert expect(actualResult.isOk()).toBe(true) - expect(actualResult._unsafeUnwrap()).toEqual(mockedResponse) + expect(actualResult._unsafeUnwrap()).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -270,14 +267,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const expectedArgument = await generateExpectedArg() @@ -290,7 +286,7 @@ describe('mail.service', () => { // Assert expect(actualResult.isOk()).toBe(true) - expect(actualResult._unsafeUnwrap()).toEqual(mockedResponse) + expect(actualResult._unsafeUnwrap()).toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -430,8 +426,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a single string', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const expectedArgument = generateExpectedArgWithToField( MOCK_VALID_SUBMISSION_PARAMS.form.emails, @@ -443,7 +438,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -452,8 +447,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a single comma separated string', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsCommaSeparated = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -469,7 +463,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -477,8 +471,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with multiple emails', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formMultipleEmailsArray = [MOCK_VALID_EMAIL, MOCK_VALID_EMAIL_2] const modifiedParams = cloneDeep(MOCK_VALID_SUBMISSION_PARAMS) @@ -492,7 +485,7 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) @@ -500,8 +493,7 @@ describe('mail.service', () => { it('should send submission mail to admin successfully if form.emails is an array with a mixture of emails and comma separated emails strings', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -516,16 +508,15 @@ describe('mail.service', () => { const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) + await expect(pendingSend).resolves.toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) }) - it('should send submission mail to admin successfully if form.emails is an array with a mixture of emails, semi-colon, and comma separated emails strings', async () => { + it('should reject with error when if form.emails is an array with a mixture of emails, semi-colon, and comma separated emails strings', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -534,16 +525,11 @@ describe('mail.service', () => { const modifiedParams = cloneDeep(MOCK_VALID_SUBMISSION_PARAMS) modifiedParams.form.emails = formEmailsMixture - const expectedArgument = generateExpectedArgWithToField(formEmailsMixture) - // Act const pendingSend = mailService.sendSubmissionToAdmin(modifiedParams) // Assert - await expect(pendingSend).resolves.toEqual(mockedResponse) - // Check arguments passed to sendNodeMail - expect(sendMailSpy).toHaveBeenCalledTimes(1) - expect(sendMailSpy).toHaveBeenCalledWith(expectedArgument) + await expect(pendingSend).rejects.toThrowError('Invalid email error') }) it('should reject with error when form.emails array contains an invalid email string', async () => { @@ -576,10 +562,9 @@ describe('mail.service', () => { responseCode: 454, message: 'oh no something went wrong', } - const mockedResponse = 'mockedSuccessResponse' sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const formEmailsMixture = [ `${MOCK_VALID_EMAIL}, ${MOCK_VALID_EMAIL_2}`, @@ -596,7 +581,7 @@ describe('mail.service', () => { ) // Assert - await expect(pendingSendSubmission).resolves.toEqual(mockedResponse) + await expect(pendingSendSubmission).resolves.toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved @@ -731,12 +716,9 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with defaults', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(MOCK_AUTOREPLY_PARAMS) @@ -751,7 +733,7 @@ describe('mail.service', () => { it('should send array of multiple autoreply mails successfully with defaults', async () => { // Arrange const firstMockedResponse = 'mockedSuccessResponse1' - const secondMockedResponse = 'mockedSuccessResponse1' + const secondMockedResponse = 'mockedSuccessResponse2' sendMailSpy .mockResolvedValueOnce(firstMockedResponse) .mockResolvedValueOnce(secondMockedResponse) @@ -765,8 +747,8 @@ describe('mail.service', () => { secondExpectedArg.to = MOCK_VALID_EMAIL_3 const expectedResponse = await Promise.allSettled([ - Promise.resolve(firstMockedResponse), - Promise.resolve(secondMockedResponse), + Promise.resolve(true), + Promise.resolve(true), ]) // Act @@ -783,8 +765,7 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with custom autoreply subject', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customSubject = 'customSubject' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) @@ -792,9 +773,7 @@ describe('mail.service', () => { const expectedArg = { ...defaultExpectedArg, subject: customSubject } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -808,8 +787,7 @@ describe('mail.service', () => { it('should send single autoreply mail successfully with custom autoreply sender', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customSender = 'customSender@example.com' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) @@ -819,9 +797,7 @@ describe('mail.service', () => { ...defaultExpectedArg, from: `${customSender} <${MOCK_SENDER_EMAIL}>`, } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -835,8 +811,7 @@ describe('mail.service', () => { it('should send single autoreply mail with attachment if autoReply.includeFormSummary is true', async () => { // Arrange - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) customDataParams.autoReplyMailDatas[0].includeFormSummary = true @@ -867,9 +842,7 @@ describe('mail.service', () => { }, ], } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -903,19 +876,16 @@ describe('mail.service', () => { responseCode: 454, message: 'oh no something went wrong', } - const mockedResponse = 'mockedSuccessResponse' sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') const customSubject = 'customSubject' const customDataParams = cloneDeep(MOCK_AUTOREPLY_PARAMS) customDataParams.autoReplyMailDatas[0].subject = customSubject const expectedArg = { ...defaultExpectedArg, subject: customSubject } - const expectedResponse = await Promise.allSettled([ - Promise.resolve(mockedResponse), - ]) + const expectedResponse = await Promise.allSettled([Promise.resolve(true)]) // Act const pendingSend = mailService.sendAutoReplyEmails(customDataParams) @@ -1024,8 +994,7 @@ describe('mail.service', () => { it('should send permanent bounce notification successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1037,7 +1006,7 @@ describe('mail.service', () => { }) const expectedArgs = await generateExpectedArg(BounceType.Permanent) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgs) @@ -1046,8 +1015,7 @@ describe('mail.service', () => { it('should send transient bounce notification successfully', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' - sendMailSpy.mockResolvedValueOnce(mockedResponse) + sendMailSpy.mockResolvedValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1059,7 +1027,7 @@ describe('mail.service', () => { }) const expectedArgs = await generateExpectedArg(BounceType.Transient) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail expect(sendMailSpy).toHaveBeenCalledTimes(1) expect(sendMailSpy).toHaveBeenCalledWith(expectedArgs) @@ -1085,14 +1053,13 @@ describe('mail.service', () => { it('should autoretry when 4xx error is thrown by sendNodeMail and pass if second try passes', async () => { // Arrange // sendMail should return mocked success response - const mockedResponse = 'mockedSuccessResponse' const mock4xxReject = { responseCode: 454, message: 'oh no something went wrong', } sendMailSpy .mockRejectedValueOnce(mock4xxReject) - .mockReturnValueOnce(mockedResponse) + .mockReturnValueOnce('mockedSuccessResponse') // Act const sent = await mailService.sendBounceNotification({ @@ -1105,7 +1072,7 @@ describe('mail.service', () => { const expectedArgs = await generateExpectedArg(MOCK_BOUNCE_TYPE) // Assert - expect(sent).toEqual(mockedResponse) + expect(sent).toEqual(true) // Check arguments passed to sendNodeMail // Should have been called two times since it rejected the first one and // resolved diff --git a/tests/unit/backend/services/sms.service.spec.js b/tests/unit/backend/services/sms.service.spec.js deleted file mode 100644 index b62845fc38..0000000000 --- a/tests/unit/backend/services/sms.service.spec.js +++ /dev/null @@ -1,130 +0,0 @@ -const dbHandler = require('../helpers/db-handler') -const { VfnErrors } = spec('dist/backend/shared/util/verification') -const { SmsType, LogType } = spec('dist/backend/types') - -const Form = dbHandler.makeModel('form.server.model', 'Form') -const SmsCount = dbHandler.makeModel('sms_count.server.model', 'SmsCount') - -const SmsService = spec('dist/backend/app/services/sms.service') - -// Test numbers provided by Twilio: -// https://www.twilio.com/docs/iam/test-credentials -const TWILIO_TEST_NUMBER = '+15005550006' - -const MOCK_MSG_SRVC_SID = 'mockMsgSrvcSid' - -describe('sms.service', () => { - const MOCK_VALID_CONFIG = { - msgSrvcSid: MOCK_MSG_SRVC_SID, - client: { - messages: { - create: jasmine.createSpy().and.resolveTo({ - status: 'testStatus', - sid: 'testSid', - }), - }, - }, - } - - const MOCK_INVALID_CONFIG = { - msgSrvcSid: MOCK_MSG_SRVC_SID, - client: { - messages: { - create: jasmine.createSpy().and.resolveTo({ - status: 'testStatus', - sid: undefined, - errorCode: 21211, - }), - }, - }, - } - - beforeAll(async () => await dbHandler.connect()) - afterEach(async () => await dbHandler.clearDatabase()) - afterAll(async () => await dbHandler.closeDatabase()) - - describe('sendVerificationOtp', () => { - let testForm - let mockOtpData - beforeEach(async () => { - const { form, user } = await dbHandler.preloadCollections() - - mockOtpData = { - form: form._id, - formAdmin: { - email: user.email, - userId: user._id, - }, - } - - testForm = form - }) - it('should throw error if otpData is null', async () => { - // Arrange - // Return null on Form method - spyOn(Form, 'getOtpData').and.returnValue(null) - - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_VALID_CONFIG, - ), - ).toBeRejectedWithError() - }) - - it('should log and send verification OTP if twilio has no errors', async () => { - // Arrange - spyOn(Form, 'getOtpData').and.returnValue(mockOtpData) - const smsCountSpy = spyOn(SmsCount, 'logSms').and.callThrough() - // Act + Assert - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_VALID_CONFIG, - ), - // Should resolve to true - ).toBeResolvedTo(true) - - // Logging should also have happened. - const expectedLogParams = { - otpData: mockOtpData, - msgSrvcSid: MOCK_MSG_SRVC_SID, - smsType: SmsType.verification, - logType: LogType.success, - } - expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) - }) - - it('should log failure and throw error if twilio failed to send', async () => { - // Arrange - spyOn(Form, 'getOtpData').and.returnValue(mockOtpData) - const smsCountSpy = spyOn(SmsCount, 'logSms').and.callThrough() - - // Act + Assert - const expectedError = new Error(VfnErrors.InvalidMobileNumber) - expectedError.name = VfnErrors.SendOtpFailed - - await expectAsync( - SmsService.sendVerificationOtp( - /* recipient= */ TWILIO_TEST_NUMBER, - /* otp= */ '111111', - /* formId= */ testForm._id, - /* defaultConfig= */ MOCK_INVALID_CONFIG, - ), - ).toBeRejectedWith(expectedError) - - // Logging should also have happened. - const expectedLogParams = { - otpData: mockOtpData, - msgSrvcSid: MOCK_MSG_SRVC_SID, - smsType: SmsType.verification, - logType: LogType.failure, - } - expect(smsCountSpy).toHaveBeenCalledWith(expectedLogParams) - }) - }) -})