Email notifications with gitlab

config.js

@@ -84,6 +84,12 @@ const schema = {
     default: null,
     env: 'GITHUB_TOKEN'
   },
+  githubWebhookSecret: {
+    doc: 'Token to verify that webhook requests are from GitHub',
+    format: String,
+    default: null,
+    env: 'GITHUB_WEBHOOK_SECRET'
+  },
   gitlabAccessTokenUri: {
     doc: 'URI for the GitLab authentication provider.',
     format: String,
@@ -102,6 +108,12 @@ const schema = {
     default: null,
     env: 'GITLAB_TOKEN'
   },
+  gitlabWebhookSecret: {
+    doc: 'Token to verify that webhook requests are from GitLab',
+    format: String,
+    default: null,
+    env: 'GITLAB_WEBHOOK_SECRET'
+  },
   port: {
     doc: 'The port to bind the application to.',
     format: 'port',

controllers/handlePR.js

@@ -1,61 +1,128 @@
 'use strict'
 
 const config = require('../config')
-const GitHub = require('../lib/GitHub')
+const gitFactory = require('../lib/GitServiceFactory')
 const Staticman = require('../lib/Staticman')
 
 module.exports = async (repo, data) => {
   const ua = config.get('analytics.uaTrackingId')
     ? require('universal-analytics')(config.get('analytics.uaTrackingId'))
     : null
 
-  if (!data.number) {
-    return
+  /*
+   * Unfortunately, all we have available to us at this point is the request body (as opposed to
+   * the full request). Meaning, we don't have the :service portion of the request URL available.
+   * As such, switch between GitHub and GitLab using the repo URL. For example:
+   *  "url": "https://api.github.com/repos/foo/staticman-test"
+   *  "url": "git@gitlab.com:foo/staticman-test.git"
+   */
+  const calcIsGitHub = function (data) {
+    return data.repository.url.includes('github.com')
   }
+  const calcIsGitLab = function (data) {
+    return data.repository.url.includes('gitlab.com')
+  }
+
+  /*
+   * Because we don't have the full request available to us here, we can't set the (Staticman)
+   * version option. Fortunately, it isn't critical.
+   */
+  const unknownVersion = ''
 
-  const github = await new GitHub({
-    username: data.repository.owner.login,
-    repository: data.repository.name,
-    version: '1'
+  let gitService = null
+  let mergeReqNbr = null
+  if (calcIsGitHub(data)) {
+    gitService = await gitFactory.create('github', {
+      branch: data.pull_request.base.ref,
+      repository: data.repository.name,
+      username: data.repository.owner.login,
+      version: unknownVersion
+    })
+    mergeReqNbr = data.number
+  } else if (calcIsGitLab(data)) {
+    const repoUrl = data.repository.url
+    const repoUsername = repoUrl.substring(repoUrl.indexOf(':') + 1, repoUrl.indexOf('/'))
+    gitService = await gitFactory.create('gitlab', {
+      branch: data.object_attributes.target_branch,
+      repository: data.repository.name,
+      username: repoUsername,
+      version: unknownVersion
+    })
+    mergeReqNbr = data.object_attributes.iid
+  } else {
+    return Promise.reject(new Error('Unable to determine service.'))
+  }
+
+  if (!mergeReqNbr) {
+    return Promise.reject(new Error('No pull/merge request number found.'))
+  }
+
+  let review = await gitService.getReview(mergeReqNbr).catch((error) => {
+    return Promise.reject(new Error(error))
   })
 
-  try {
-    let review = await github.getReview(data.number)
-    if (review.sourceBranch.indexOf('staticman_')) {
-      return null
-    }
+  if (review.sourceBranch.indexOf('staticman_') < 0) {
+    /*
+     * Don't throw an error here, as we might receive "real" (non-bot) pull requests for files
+     * other than Staticman-processed comments.
+     */
+    return null
+  }
 
-    if (review.state !== 'merged' && review.state !== 'closed') {
-      return null
-    }
+  if (review.state !== 'merged' && review.state !== 'closed') {
+    /*
+     * Don't throw an error here, as we'll regularly receive webhook calls whenever a pull/merge
+     * request is opened, not just merged/closed.
+     */
+    return null
+  }
+
+  if (review.state === 'merged') {
+    /*
+     * The "staticman_notification" comment section of the comment pull/merge request only
+     * exists if notifications were enabled at the time the pull/merge request was created.
+     */
+    const bodyMatch = review.body.match(/(?:.*?)<!--staticman_notification:(.+?)-->(?:.*?)/i)
 
-    if (review.state === 'merged') {
-      const bodyMatch = review.body.match(/(?:.*?)<!--staticman_notification:(.+?)-->(?:.*?)/i)
+    if (bodyMatch && (bodyMatch.length === 2)) {
+      try {
+        const parsedBody = JSON.parse(bodyMatch[1])
+        const staticman = await new Staticman(parsedBody.parameters)
 
-      if (bodyMatch && (bodyMatch.length === 2)) {
-        try {
-          const parsedBody = JSON.parse(bodyMatch[1])
-          const staticman = await new Staticman(parsedBody.parameters)
+        staticman.setConfigPath(parsedBody.configPath)
 
-          staticman.setConfigPath(parsedBody.configPath)
-          staticman.processMerge(parsedBody.fields, parsedBody.options)
-        } catch (err) {
-          return Promise.reject(err)
+        await staticman.processMerge(parsedBody.fields, parsedBody.options)
+        if (ua) {
+          ua.event('Hooks', 'Create/notify mailing list').send()
+        }
+      } catch (err) {
+        if (ua) {
+          ua.event('Hooks', 'Create/notify mailing list error').send()
         }
+
+        return Promise.reject(err)
       }
     }
 
-    if (ua) {
-      ua.event('Hooks', 'Delete branch').send()
-    }
-    return github.deleteBranch(review.sourceBranch)
-  } catch (e) {
-    console.log(e.stack || e)
+    /*
+     * Only necessary for GitHub, as GitLab automatically deletes the backing branch for the
+     * pull/merge request. For GitHub, this will throw the following error if the branch has
+     * already been deleted:
+     *  HttpError: Reference does not exist"
+     */
+    if (calcIsGitHub(data)) {
+      try {
+        await gitService.deleteBranch(review.sourceBranch)
+        if (ua) {
+          ua.event('Hooks', 'Delete branch').send()
+        }
+      } catch (err) {
+        if (ua) {
+          ua.event('Hooks', 'Delete branch error').send()
+        }
 
-    if (ua) {
-      ua.event('Hooks', 'Delete branch error').send()
+        return Promise.reject(err)
+      }
     }
-
-    return Promise.reject(e)
   }
 }

controllers/webhook.js

@@ -0,0 +1,64 @@
+'use strict'
+
+const path = require('path')
+const config = require(path.join(__dirname, '/../config'))
+const handlePR = require('./handlePR')
+
+module.exports = async (req, res, next) => {
+  switch (req.params.service) {
+    case 'gitlab':
+      let errorMsg = null
+      let event = req.headers['x-gitlab-event']
+
+      if (!event) {
+        errorMsg = 'No event found in the request'
+      } else {
+        if (event === 'Merge Request Hook') {
+          const webhookSecretExpected = config.get('gitlabWebhookSecret')
+          const webhookSecretSent = req.headers['x-gitlab-token']
+
+          let reqAuthenticated = true
+          if (webhookSecretExpected) {
+            reqAuthenticated = false
+            if (!webhookSecretSent) {
+              errorMsg = 'No secret found in the webhook request'
+            } else if (webhookSecretExpected === webhookSecretSent) {
+              /*
+               * Whereas GitHub uses the webhook secret to sign the request body, GitLab does not.
+               * As such, just check that the received secret equals the expected value.
+               */
+              reqAuthenticated = true
+            } else {
+              errorMsg = 'Unable to verify authenticity of request'
+            }
+          }
+
+          if (reqAuthenticated) {
+            await handlePR(req.params.repository, req.body).catch((error) => {
+              console.error(error.stack || error)
+              errorMsg = error.message
+            })
+          }
+        }
+      }
+
+      if (errorMsg !== null) {
+        res.status(400).send({
+          error: errorMsg
+        })
+      } else {
+        res.status(200).send({
+          success: true
+        })
+      }
+
+      break
+    default:
+      res.status(400).send({
+        /*
+         * We are expecting GitHub webhooks to be handled by the express-github-webhook module.
+         */
+        error: 'Unexpected service specified.'
+      })
+  }
+}

lib/ErrorHandler.js

@@ -17,81 +17,83 @@ class ApiError {
   }
 }
 
-const ErrorHandler = function () {
-  this.ERROR_MESSAGES = {
-    'missing-input-secret': 'reCAPTCHA: The secret parameter is missing',
-    'invalid-input-secret': 'reCAPTCHA: The secret parameter is invalid or malformed',
-    'missing-input-response': 'reCAPTCHA: The response parameter is missing',
-    'invalid-input-response': 'reCAPTCHA: The response parameter is invalid or malformed',
-    'RECAPTCHA_MISSING_CREDENTIALS': 'Missing reCAPTCHA API credentials',
-    'RECAPTCHA_FAILED_DECRYPT': 'Could not decrypt reCAPTCHA secret',
-    'RECAPTCHA_CONFIG_MISMATCH': 'reCAPTCHA options do not match Staticman config',
-    'PARSING_ERROR': 'Error whilst parsing config file',
-    'GITHUB_AUTH_TOKEN_MISSING': 'The site requires a valid GitHub authentication token to be supplied in the `options[github-token]` field',
-    'MISSING_CONFIG_BLOCK': 'Error whilst parsing Staticman config file'
+class ErrorHandler {
+  constructor () {
+    this.ERROR_MESSAGES = {
+      'missing-input-secret': 'reCAPTCHA: The secret parameter is missing',
+      'invalid-input-secret': 'reCAPTCHA: The secret parameter is invalid or malformed',
+      'missing-input-response': 'reCAPTCHA: The response parameter is missing',
+      'invalid-input-response': 'reCAPTCHA: The response parameter is invalid or malformed',
+      'RECAPTCHA_MISSING_CREDENTIALS': 'Missing reCAPTCHA API credentials',
+      'RECAPTCHA_FAILED_DECRYPT': 'Could not decrypt reCAPTCHA secret',
+      'RECAPTCHA_CONFIG_MISMATCH': 'reCAPTCHA options do not match Staticman config',
+      'PARSING_ERROR': 'Error whilst parsing config file',
+      'GITHUB_AUTH_TOKEN_MISSING': 'The site requires a valid GitHub authentication token to be supplied in the `options[github-token]` field',
+      'MISSING_CONFIG_BLOCK': 'Error whilst parsing Staticman config file'
+    }
+
+    this.ERROR_CODE_ALIASES = {
+      'missing-input-secret': 'RECAPTCHA_MISSING_INPUT_SECRET',
+      'invalid-input-secret': 'RECAPTCHA_INVALID_INPUT_SECRET',
+      'missing-input-response': 'RECAPTCHA_MISSING_INPUT_RESPONSE',
+      'invalid-input-response': 'RECAPTCHA_INVALID_INPUT_RESPONSE'
+    }
   }
 
-  this.ERROR_CODE_ALIASES = {
-    'missing-input-secret': 'RECAPTCHA_MISSING_INPUT_SECRET',
-    'invalid-input-secret': 'RECAPTCHA_INVALID_INPUT_SECRET',
-    'missing-input-response': 'RECAPTCHA_MISSING_INPUT_RESPONSE',
-    'invalid-input-response': 'RECAPTCHA_INVALID_INPUT_RESPONSE'
+  getErrorCode (error) {
+    return this.ERROR_CODE_ALIASES[error] || error
   }
-}
 
-ErrorHandler.prototype.getErrorCode = function (error) {
-  return this.ERROR_CODE_ALIASES[error] || error
-}
+  getMessage (error) {
+    return this.ERROR_MESSAGES[error]
+  }
 
-ErrorHandler.prototype.getMessage = function (error) {
-  return this.ERROR_MESSAGES[error]
-}
+  log (err, instance) {
+    let parameters = {}
+    let prefix = ''
 
-ErrorHandler.prototype.log = function (err, instance) {
-  let parameters = {}
-  let prefix = ''
+    if (instance) {
+      parameters = instance.getParameters()
 
-  if (instance) {
-    parameters = instance.getParameters()
+      prefix += `${parameters.username}/${parameters.repository}`
+    }
 
-    prefix += `${parameters.username}/${parameters.repository}`
+    console.log(`${prefix}`, err)
   }
 
-  console.log(`${prefix}`, err)
-}
+  _save (errorCode, data = {}) {
+    const {err} = data
 
-ErrorHandler.prototype._save = function (errorCode, data = {}) {
-  const {err} = data
+    if (err) {
+      err._smErrorCode = err._smErrorCode || errorCode
 
-  if (err) {
-    err._smErrorCode = err._smErrorCode || errorCode
+      // Re-wrap API request errors as these could expose
+      // request/response details that the user should not
+      // be allowed to see e.g. access tokens.
+      // `request-promise` is the primary offender here,
+      // but we similarly do not want others to leak too.
+      if (
+        err instanceof StatusCodeError ||
+        err instanceof RequestError
+      ) {
+        const statusCode = err.statusCode || err.code
 
-    // Re-wrap API request errors as these could expose
-    // request/response details that the user should not
-    // be allowed to see e.g. access tokens.
-    // `request-promise` is the primary offender here,
-    // but we similarly do not want others to leak too.
-    if (
-      err instanceof StatusCodeError ||
-      err instanceof RequestError
-    ) {
-      const statusCode = err.statusCode || err.code
+        return new ApiError(err.message, statusCode, err._smErrorCode)
+      }
 
-      return new ApiError(err.message, statusCode, err._smErrorCode)
+      return err
     }
 
-    return err
-  }
+    let payload = {
+      _smErrorCode: errorCode
+    }
 
-  let payload = {
-    _smErrorCode: errorCode
-  }
+    if (data.data) {
+      payload.data = data.data
+    }
 
-  if (data.data) {
-    payload.data = data.data
+    return payload
   }
-
-  return payload
 }
 
 const errorHandler = new ErrorHandler()