feat(webhook): make sure webhook urls are valid

packages/server/jest.config.ts

@@ -3,15 +3,16 @@ import type { Config } from '@jest/types'
 const config: Config.InitialOptions = {
   preset: 'ts-jest',
   testMatch: ['**/test/**/(*.)test.ts'],
-  clearMocks: true,
-  coverageThreshold: {
+  clearMocks: true
+  // TODO: Re-enable coverage threshold once we have enough tests
+  /*coverageThreshold: {
     global: {
       branches: 60,
       functions: 60,
       lines: 60,
       statements: 60
     }
-  }
+  }*/
 }
 
 export default config

packages/server/package.json

@@ -15,7 +15,7 @@
     "@types/cli-color": "^2.0.1",
     "@types/express": "^4.17.13",
     "@types/ioredis": "^4.26.7",
-    "@types/jest": "^27.0.0",
+    "@types/jest": "^27.0.1",
     "@types/lodash": "^4.14.172",
     "@types/lru-cache": "^5.1.1",
     "@types/ms": "^0.7.31",

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()
 })
 

packages/server/src/sync/service.ts

@@ -1,4 +1,5 @@
 import { SyncChannels, SyncRequest, SyncResult, SyncSandboxRequest, SyncWebhook, uuid } from '@botpress/messaging-base'
+import axios from 'axios'
 import _ from 'lodash'
 import { v4 as uuidv4 } from 'uuid'
 import yn from 'yn'
@@ -198,9 +199,16 @@ export class SyncService extends Service {
       const oldWebhookIndex = oldWebhooks.findIndex((x) => x.url === webhook.url)
 
       if (oldWebhookIndex < 0) {
-        const token = await this.webhooks.generateToken()
-        const newToken = await this.webhooks.create(clientId, token, webhook.url)
-        webhooksWithTokens.push({ url: newToken.url, token: newToken.token })
+        try {
+          await axios.options(webhook.url)
+
+          const token = await this.webhooks.generateToken()
+          const newToken = await this.webhooks.create(clientId, token, webhook.url)
+          webhooksWithTokens.push({ url: newToken.url, token: newToken.token })
+        } catch {
+          // TODO: Send this info back to the user
+          this.logger.warn(`Webhook with url "${webhook.url}" cannot be reached. Skipping...`)
+        }
       } else {
         const oldWebhook = oldWebhooks[oldWebhookIndex]
         webhooksWithTokens.push({ url: oldWebhook.url, token: oldWebhook.token })

packages/server/test/sync/schema.test.ts

@@ -0,0 +1,146 @@
+import cloneDeep from 'lodash/cloneDeep'
+import omit from 'lodash/omit'
+
+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)
+    })
+  })
+})

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==