From 169c06200b1fa975b72b72005d7599e72fb8b3db Mon Sep 17 00:00:00 2001 From: Laurent Leclerc Poulin Date: Mon, 16 Aug 2021 17:49:11 -0400 Subject: [PATCH] feat(webhook): make sure webhook urls are valid (#133) * feat(webhook): make sure webhook urls are valid and reachable uri * added unit tests for sync schema validation * temporarily disable coverage threshold * fix references * removed webhook validation using option request * fix tests --- jest.config.ts | 27 +++++ package.json | 5 +- packages/server/jest.config.ts | 17 --- packages/server/package.json | 6 +- packages/server/src/sync/schema.ts | 2 +- packages/server/test/sync/schema.test.ts | 145 +++++++++++++++++++++++ yarn.lock | 64 ++-------- 7 files changed, 186 insertions(+), 80 deletions(-) create mode 100644 jest.config.ts delete mode 100644 packages/server/jest.config.ts create mode 100644 packages/server/test/sync/schema.test.ts diff --git a/jest.config.ts b/jest.config.ts new file mode 100644 index 000000000..3f21d3f90 --- /dev/null +++ b/jest.config.ts @@ -0,0 +1,27 @@ +import type { Config } from '@jest/types' +import { defaults as tsjPreset } from 'ts-jest/presets' + +const config: Config.InitialOptions = { + preset: 'ts-jest', + // TODO: Re-enable coverage threshold once we have enough tests + /*coverageThreshold: { + global: { + branches: 60, + functions: 60, + lines: 60, + statements: 60 + } + }*/ + + projects: [ + { + testMatch: ['/packages/server/test/**/(*.)test.ts'], + displayName: { name: 'Server', color: 'blue' }, + testEnvironment: 'node', + transform: tsjPreset.transform, + clearMocks: true + } + ] +} + +export default config diff --git a/package.json b/package.json index 5e47866b3..030e3874d 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "start": "yarn workspace @botpress/messaging-server start", "dev": "yarn workspace @botpress/messaging-server dev", "board": "yarn workspace @botpress/messaging-board dev", - "test": "yarn workspace @botpress/messaging-server test" + "test": "yarn workspace @botpress/messaging-server build && jest" }, "bin": "./packages/server/dist/index.js", "pkg": { @@ -32,6 +32,7 @@ "outputPath": "./bin" }, "devDependencies": { + "@types/jest": "^27.0.1", "@typescript-eslint/eslint-plugin": "^4.29.1", "@typescript-eslint/parser": "^4.29.1", "conventional-changelog": "^3.1.24", @@ -41,11 +42,13 @@ "eslint-plugin-import": "^2.24.0", "eslint-plugin-jsdoc": "^35.5.1", "fs-extra": "^10.0.0", + "jest": "^27.0.6", "parcel": "^2.0.0-beta.3.1", "pkg": "^4.3.7", "prettier": "^2.3.2", "rimraf": "^3.0.2", "semver": "^7.3.5", + "ts-jest": "^27.0.4", "ts-node-dev": "^1.1.8", "typescript": "^4.3.5", "yargs": "^17.0.1" diff --git a/packages/server/jest.config.ts b/packages/server/jest.config.ts deleted file mode 100644 index ffb1cf1ef..000000000 --- a/packages/server/jest.config.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { Config } from '@jest/types' - -const config: Config.InitialOptions = { - preset: 'ts-jest', - testMatch: ['**/test/**/(*.)test.ts'], - clearMocks: true, - coverageThreshold: { - global: { - branches: 60, - functions: 60, - lines: 60, - statements: 60 - } - } -} - -export default config diff --git a/packages/server/package.json b/packages/server/package.json index 0f0ac0f73..fa85ed980 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -7,7 +7,6 @@ "build": "yarn --silent && tsc --build", "watch": "yarn --silent && tsc --build --watch", "start": "node dist/index.js", - "test": "jest", "dev": "yarn --silent && ts-node-dev --inspect --inspect-publish-uid=http --transpile-only src/index.ts" }, "devDependencies": { @@ -15,15 +14,12 @@ "@types/cli-color": "^2.0.1", "@types/express": "^4.17.13", "@types/ioredis": "^4.26.7", - "@types/jest": "^27.0.0", "@types/lodash": "^4.14.172", "@types/lru-cache": "^5.1.1", "@types/ms": "^0.7.31", "@types/node": "^12.18.3", "@types/redlock": "^4.0.2", - "@types/uuid": "^8.3.1", - "jest": "^27.0.6", - "ts-jest": "^27.0.4" + "@types/uuid": "^8.3.1" }, "dependencies": { "@botpress/messaging-base": "0.0.1", diff --git a/packages/server/src/sync/schema.ts b/packages/server/src/sync/schema.ts index ea72cae2b..9d164fc97 100644 --- a/packages/server/src/sync/schema.ts +++ b/packages/server/src/sync/schema.ts @@ -2,7 +2,7 @@ import Joi from 'joi' import { Channel } from '../channels/base/channel' const SyncWebhookSchema = Joi.object({ - url: Joi.string().required(), + url: Joi.string().uri().required(), token: Joi.string() }) diff --git a/packages/server/test/sync/schema.test.ts b/packages/server/test/sync/schema.test.ts new file mode 100644 index 000000000..03652df9f --- /dev/null +++ b/packages/server/test/sync/schema.test.ts @@ -0,0 +1,145 @@ +import cloneDeep from 'lodash/cloneDeep' + +import { ChannelService } from '../../src/channels/service' +import { ConfigService } from '../../src/config/service' +import { DatabaseService } from '../../src/database/service' +import { makeSyncRequestSchema } from '../../src/sync/schema' + +jest.mock('../../src/database/service') +jest.mock('../../src/config/service') + +const channels = { + messenger: { + accessToken: 'accessToken', + appSecret: 'appSecret', + verifyToken: 'verifyToken' + }, + slack: { + signingSecret: 'signingSecret', + useRTM: false, + botToken: 'botToken' + }, + smooch: { + keyId: 'keyId', + secret: 'secret', + forwardRawPayloads: ['text'] + }, + teams: { + appId: 'appId', + appPassword: 'appPassword', + proactiveMessages: { + proactiveMessages: 'proactiveMessages' + } + }, + telegram: { + botToken: 'botToken' + }, + twilio: { + accountSID: 'accountSID', + authToken: 'authToken' + }, + vonage: { + useTestingApi: true, + apiKey: 'apiKey', + apiSecret: 'apiSecret', + signatureSecret: 'signatureSecret', + applicationId: 'applicationId', + privateKey: 'privateKey' + } +} +const webhooks = [ + { + url: 'https://url.com' + }, + { + url: 'http://localhost:3100' + } +] +const invalidWebhook = [ + { + url: '83.23.12.1:3100' + } +] + +describe('Sync', () => { + let channelService: ChannelService + + const validObj = { + channels, + webhooks, + name: 'a test config', + id: 'a5869bcf-fee3-45f4-b083-1d177ea0d9cc', + token: 'Eg4NWrUhW8TEMrZ39Gc4Y+vs2tl0VYt0/PgL4yhCZx1r/5QP0RQ2hbg/bWsKSfdn4R4/pQcK9S9tMk5XDdzkAbsb' + } + const validObjWithUnknownProperties = { + ...cloneDeep(validObj), + unknownProperty: 'unknown' + } + const validObjWithInvalidWebhook = { + ...cloneDeep(validObj), + webhooks: invalidWebhook + } + + beforeEach(() => { + const configService = new ConfigService() + const databaseService = new DatabaseService(configService) + channelService = new ChannelService(databaseService) + }) + + describe('makeSyncRequestSchema', () => { + test('Should not throw any error with all required fields', async () => { + const schema = makeSyncRequestSchema(channelService.list()) + + const obj = {} + const { error, value } = schema.validate(obj) + if (error) { + throw error + } + + expect(value).toEqual(obj) + }) + + test('Should not throw any error with an object that contains only valid properties', async () => { + const schema = makeSyncRequestSchema(channelService.list()) + + const { error, value } = schema.validate(validObj) + if (error) { + throw error + } + + expect(value).toEqual(validObj) + }) + + test('Should strip unknown properties from object', async () => { + const schema = makeSyncRequestSchema(channelService.list()) + + const { error, value } = schema.validate(validObjWithUnknownProperties) + if (error) { + throw error + } + + expect(value).toEqual(validObj) + }) + + test('Should only accept objects', async () => { + const schema = makeSyncRequestSchema(channelService.list()) + + const val = '' + const { error, value } = schema.validate(val) + + expect(error).not.toBeUndefined() + expect(error!.message).toEqual('"value" must be of type object') + expect(value).toEqual(val) + }) + + test('Should only accept valid URI as webhook url', async () => { + const schema = makeSyncRequestSchema(channelService.list()) + + const { error, value } = schema.validate(validObjWithInvalidWebhook) + + expect(error).not.toBeUndefined() + expect(error!.message).toEqual('"webhooks[0].url" must be a valid uri') + expect(value).toEqual(validObjWithInvalidWebhook) + }) + }) +}) diff --git a/yarn.lock b/yarn.lock index a3f8e37ed..8e8f8d8bb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -751,17 +751,6 @@ source-map "^0.6.1" write-file-atomic "^3.0.0" -"@jest/types@^26.6.2": - version "26.6.2" - resolved "https://registry.yarnpkg.com/@jest/types/-/types-26.6.2.tgz#bef5a532030e1d88a2f5a6d933f84e97226ed48e" - integrity sha512-fC6QCp7Sc5sX6g8Tvbmj4XUTbyrik0akgRy03yjXbQaBWWNWGE7SGtJk98m0N8nzegD/7SggrUlivxo5ax4KWQ== - dependencies: - "@types/istanbul-lib-coverage" "^2.0.0" - "@types/istanbul-reports" "^3.0.0" - "@types/node" "*" - "@types/yargs" "^15.0.0" - chalk "^4.0.0" - "@jest/types@^27.0.6": version "27.0.6" resolved "https://registry.yarnpkg.com/@jest/types/-/types-27.0.6.tgz#9a992bc517e0c49f035938b8549719c2de40706b" @@ -1597,13 +1586,13 @@ dependencies: "@types/istanbul-lib-report" "*" -"@types/jest@^27.0.0": - version "27.0.0" - resolved "https://registry.yarnpkg.com/@types/jest/-/jest-27.0.0.tgz#f1c28f741371739c7cd0e8edb5ed8e67acfa6c35" - integrity sha512-IlpQZVpxufe+3qPaAqEoSPHVSxnJh1cf0BqqWHJeKiAUbwnHdmNzjP3ZCWSZSTbmAGXQPNk9QmM3Bif0pR54rg== +"@types/jest@^27.0.1": + version "27.0.1" + resolved "https://registry.yarnpkg.com/@types/jest/-/jest-27.0.1.tgz#fafcc997da0135865311bb1215ba16dba6bdf4ca" + integrity sha512-HTLpVXHrY69556ozYkcq47TtQJXpcWAWfkoqz+ZGz2JnmZhzlRjprCIyFnetSy8gpDWwTTGBcRVv1J1I1vBrHw== dependencies: - jest-diff "^26.0.0" - pretty-format "^26.0.0" + jest-diff "^27.0.0" + pretty-format "^27.0.0" "@types/json-schema@^7.0.7": version "7.0.9" @@ -1806,13 +1795,6 @@ resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-20.2.0.tgz#dd3e6699ba3237f0348cd085e4698780204842f9" integrity sha512-37RSHht+gzzgYeobbG+KWryeAW8J33Nhr69cjTqSYymXVZEN9NbRYWoYlRtDhHKPVT1FyNKwaTPC1NynKZpzRA== -"@types/yargs@^15.0.0": - version "15.0.14" - resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-15.0.14.tgz#26d821ddb89e70492160b66d10a0eb6df8f6fb06" - integrity sha512-yEJzHoxf6SyQGhBhIYGXQDSCkJjB6HohDShto7m8vaKg9Yp0Yn8+71J9eakh2bnPg6BfsH9PRMhiRTZnd4eXGQ== - dependencies: - "@types/yargs-parser" "*" - "@types/yargs@^15.0.4": version "15.0.13" resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-15.0.13.tgz#34f7fec8b389d7f3c1fd08026a5763e072d3c6dc" @@ -3848,11 +3830,6 @@ detect-newline@^3.0.0: resolved "https://registry.yarnpkg.com/detect-newline/-/detect-newline-3.1.0.tgz#576f5dfc63ae1a192ff192d8ad3af6308991b651" integrity sha512-TLz+x/vEXm/Y7P7wn1EJFNLxYpUD4TgMosxY6fAVJUnJMbupHBOncxyWUG9OpTaH9EBD7uFI5LfEgmMOc54DsA== -diff-sequences@^26.6.2: - version "26.6.2" - resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-26.6.2.tgz#48ba99157de1923412eed41db6b6d4aa9ca7c0b1" - integrity sha512-Mv/TDa3nZ9sbc5soK+OoA74BsS3mL37yixCvUAQkiuA4Wz6YtwP/K47n2rv2ovzHZvoiQeA5FTQOschKkEwB0Q== - diff-sequences@^27.0.6: version "27.0.6" resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-27.0.6.tgz#3305cb2e55a033924054695cc66019fd7f8e5723" @@ -6152,17 +6129,7 @@ jest-config@^27.0.6: micromatch "^4.0.4" pretty-format "^27.0.6" -jest-diff@^26.0.0: - version "26.6.2" - resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-26.6.2.tgz#1aa7468b52c3a68d7d5c5fdcdfcd5e49bd164394" - integrity sha512-6m+9Z3Gv9wN0WFVasqjCL/06+EFCMTqDEUl/b87HYK2rAPTyfz4ZIuSlPhY51PIQRWx5TaxeF1qmXKe9gfN3sA== - dependencies: - chalk "^4.0.0" - diff-sequences "^26.6.2" - jest-get-type "^26.3.0" - pretty-format "^26.6.2" - -jest-diff@^27.0.6: +jest-diff@^27.0.0, jest-diff@^27.0.6: version "27.0.6" resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-27.0.6.tgz#4a7a19ee6f04ad70e0e3388f35829394a44c7b5e" integrity sha512-Z1mqgkTCSYaFgwTlP/NUiRzdqgxmmhzHY1Tq17zL94morOHfHu3K4bgSgl+CR4GLhpV8VxkuOYuIWnQ9LnFqmg== @@ -6215,11 +6182,6 @@ jest-environment-node@^27.0.6: jest-mock "^27.0.6" jest-util "^27.0.6" -jest-get-type@^26.3.0: - version "26.3.0" - resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-26.3.0.tgz#e97dc3c3f53c2b406ca7afaed4493b1d099199e0" - integrity sha512-TpfaviN1R2pQWkIihlfEanwOXK0zcxrKEE4MlU6Tn7keoXdN6/3gK/xl0yEh8DOunn5pOVGKf8hB4R9gVh04ig== - jest-get-type@^27.0.6: version "27.0.6" resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-27.0.6.tgz#0eb5c7f755854279ce9b68a9f1a4122f69047cfe" @@ -8753,17 +8715,7 @@ prettier@^2.3.2: resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.3.2.tgz#ef280a05ec253712e486233db5c6f23441e7342d" integrity sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ== -pretty-format@^26.0.0, pretty-format@^26.6.2: - version "26.6.2" - resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-26.6.2.tgz#e35c2705f14cb7fe2fe94fa078345b444120fc93" - integrity sha512-7AeGuCYNGmycyQbCqd/3PWH4eOoX/OiCa0uphp57NVTeAGdJGaAliecxwBDHYQCIvrW7aDBZCYeNTP/WX69mkg== - dependencies: - "@jest/types" "^26.6.2" - ansi-regex "^5.0.0" - ansi-styles "^4.0.0" - react-is "^17.0.1" - -pretty-format@^27.0.6: +pretty-format@^27.0.0, pretty-format@^27.0.6: version "27.0.6" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-27.0.6.tgz#ab770c47b2c6f893a21aefc57b75da63ef49a11f" integrity sha512-8tGD7gBIENgzqA+UBzObyWqQ5B778VIFZA/S66cclyd5YkFLYs2Js7gxDKf0MXtTc9zcS7t1xhdfcElJ3YIvkQ==