From 131fd4314e5e174e13d91afde4c66841d74f551b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Soko=C5=82owski?= <jakub@status.im>
Date: Thu, 7 Feb 2019 00:05:52 +0100
Subject: [PATCH] move checking of repos whitelist to comments.js
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jakub Sokołowski <jakub@status.im>
---
 src/comments.js |  7 ++++++-
 src/schema.js   | 24 ++++++++++--------------
 src/server.js   |  6 ++----
 test/sample.js  |  2 --
 test/schema.js  | 22 ++--------------------
 5 files changed, 20 insertions(+), 41 deletions(-)

diff --git a/src/comments.js b/src/comments.js
index 5e6fa2a..490dc4b 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -46,9 +46,10 @@ const extractArchiveBuilds = (builds) => {
 }
 
 class Comments {
-  constructor({client, owner, builds}) {
+  constructor({client, owner, repos, builds}) {
     this.gh = client
     this.db = builds
+    this.repos = repos /* whitelist of repos to which we post */
     this.owner = owner /* name of user who makes the comments */
     /* add helper for formatting dates */
     Handlebars.registerHelper('date', dateHelper)
@@ -101,6 +102,10 @@ class Comments {
   }
 
   async update ({repo, pr}) {
+    /* check if repo is in a whitelist */
+    if (!this.repos.includes(repo)) {
+      throw Error(`Repo not whitelisted: ${repo}`)
+    }
     /* check if comment was already posted */
     let comment_id = await this.db.getCommentID({repo, pr})
     if (comment_id) {
diff --git a/src/schema.js b/src/schema.js
index 7522855..1264ec2 100644
--- a/src/schema.js
+++ b/src/schema.js
@@ -1,17 +1,13 @@
 const Joi = require('joi')
  
-/* whitelisted repos are controlled by env variables in server.js */
-const genSchema = (REPOS_WHITELIST) => (
-  Joi.object().keys({
-    id: Joi.alternatives().try(Joi.number().positive(), Joi.string()).required(),
-    commit: Joi.string().regex(/^[a-zA-Z0-9]{6,40}$/).required(),
-    repo: Joi.string().max(30).required().valid(REPOS_WHITELIST),
-    success: Joi.boolean().required(),
-    platform: Joi.string().max(20).required(),
-    duration: Joi.string().max(20).required(),
-    url: Joi.string().uri().required(),
-    pkg_url: Joi.string().uri().allow(null),
-  })
-)
+const schema = Joi.object().keys({
+  id: Joi.alternatives().try(Joi.number().positive(), Joi.string()).required(),
+  commit: Joi.string().regex(/^[a-zA-Z0-9]{6,40}$/).required(),
+  success: Joi.boolean().required(),
+  platform: Joi.string().max(20).required(),
+  duration: Joi.string().max(20).required(),
+  url: Joi.string().uri().required(),
+  pkg_url: Joi.string().uri().allow(null),
+})
 
-module.exports = genSchema
+module.exports = schema
diff --git a/src/server.js b/src/server.js
index 2b73506..6a2b53e 100644
--- a/src/server.js
+++ b/src/server.js
@@ -5,7 +5,7 @@ const Octokit = require('@octokit/rest')
 const App = require('./app')
 const Builds = require('./builds')
 const Comments = require('./comments')
-const Schema = require('./schema')
+const schema = require('./schema')
 
 /* DEFAULTS */
 const LOG_LEVEL        = process.env.LOG_LEVEL        || 'INFO'
@@ -25,12 +25,10 @@ const builds = new Builds(DB_PATH, DB_SAVE_INTERVAL)
 /* necessary to post and update comments */
 const gh = new Octokit({auth: `token ${GH_TOKEN}`})
 
-/* set valid repo names */
-const schema = Schema(GH_REPO_NAMES)
-
 const ghc = new Comments({
   client: gh,
   owner: GH_REPO_OWNER,
+  repos: GH_REPO_NAMES,
   builds: builds,
 })
 const app = App({ghc, schema})
diff --git a/test/sample.js b/test/sample.js
index e7b0b24..9439ceb 100644
--- a/test/sample.js
+++ b/test/sample.js
@@ -2,7 +2,6 @@
 const BUILD = {
   id: 'ID-1',
   commit: 'abcd1234',
-  repo: 'REPO-1',
   success: true,
   platform: 'PLATFORM-1',
   duration: 'DURATION-1',
@@ -13,7 +12,6 @@ const BUILD = {
 const getBuild = (idx) => ({
   id: `ID-${idx}`,
   commit: `COMMIT-${Math.floor(idx/4)}`,
-  repo: `REPO-${Math.floor(idx/8)}`,
   success: (idx%3) ? true : false,
   platform: `PLATFORM-${idx}`,
   duration: `DURATION-${idx} 12 sec`,
diff --git a/test/schema.js b/test/schema.js
index a967c22..50d6925 100644
--- a/test/schema.js
+++ b/test/schema.js
@@ -3,15 +3,14 @@ const sinon = require('sinon')
 const Joi = require('joi')
 
 const sample = require('./sample')
-const Schema = require('../src/schema')
+const schema = require('../src/schema')
 
-let build, schema
+let build
 
 describe('Schema', () => {
   beforeEach(() => {
     /* refresh for every test */
     build = Object.assign({}, sample.BUILD)
-    schema = Schema(['REPO-1'])
   })
   
   describe('id', () => {
@@ -49,23 +48,6 @@ describe('Schema', () => {
     })
   })
 
-  describe('repo', () => {
-    it('has to be a repo', async () => {
-      let rval = await Joi.validate(build, schema)
-      expect(rval).to.eql(build)
-    })
-
-    it('can\'t be a null', () => {
-      build.repo = null
-      expect(Joi.validate(build, schema)).rejectedWith('"repo" must be a string')
-    })
-
-    it('has to be on whitelist', () => {
-      build.repo = 'REPO-WRONG'
-      expect(Joi.validate(build, schema)).rejectedWith('"repo" must be one of [REPO-1]')
-    })
-  })
-
   describe('pkg_url', () => {
     it('has to be a URL', async () => {
       let rval = await Joi.validate(build, schema)