Inject shieldsSecret into GitHub token admin endpoint (#5631)

Ref #3393

core/server/secret-is-valid.js

@@ -1,7 +1,5 @@
 'use strict'
 
-const serverSecrets = require('../../lib/server-secrets')
-
 function constEq(a, b) {
   if (a.length !== b.length) {
     return false
@@ -13,9 +11,10 @@ function constEq(a, b) {
   return zero === 0
 }
 
-module.exports = function secretIsValid(secret = '') {
-  return (
-    serverSecrets.shields_secret &&
-    constEq(secret, serverSecrets.shields_secret)
-  )
+function makeSecretIsValid(shieldsSecret) {
+  return function secretIsValid(secret = '') {
+    return shieldsSecret && constEq(secret, shieldsSecret)
+  }
 }
+
+module.exports = { makeSecretIsValid }

services/github/auth/admin.js

@@ -1,8 +1,10 @@
 'use strict'
 
-const secretIsValid = require('../../../core/server/secret-is-valid')
+const { makeSecretIsValid } = require('../../../core/server/secret-is-valid')
+
+function setRoutes({ shieldsSecret }, { apiProvider, server }) {
+  const secretIsValid = makeSecretIsValid(shieldsSecret)
 
-function setRoutes(apiProvider, server) {
   // Allow the admin to obtain the tokens for operational and debugging
   // purposes. This could be used to:
   //
@@ -26,6 +28,4 @@ function setRoutes(apiProvider, server) {
   })
 }
 
-module.exports = {
-  setRoutes,
-}
+module.exports = { setRoutes }

services/github/auth/admin.spec.js

@@ -1,32 +1,14 @@
 'use strict'
 
 const { expect } = require('chai')
-const sinon = require('sinon')
 const Camp = require('@shields_io/camp')
 const portfinder = require('portfinder')
-const serverSecrets = require('../../../lib/server-secrets')
 const got = require('../../../core/got-test-client')
 const GithubApiProvider = require('../github-api-provider')
 const { setRoutes } = require('./admin')
 
 describe('GitHub admin route', function () {
-  const validCredentials = {
-    username: '',
-    password: '7'.repeat(40),
-  }
-
-  let sandbox
-  beforeEach(function () {
-    sandbox = sinon.createSandbox()
-    // Make this work when there is no `shields_secret` defined.
-    serverSecrets.shields_secret = undefined
-    sandbox
-      .stub(serverSecrets, 'shields_secret')
-      .value(validCredentials.password)
-  })
-  afterEach(function () {
-    sandbox.restore()
-  })
+  const shieldsSecret = '7'.repeat(40)
 
   let port, baseUrl
   before(async function () {
@@ -48,15 +30,14 @@ describe('GitHub admin route', function () {
 
   before(function () {
     const apiProvider = new GithubApiProvider({ withPooling: true })
-    setRoutes(apiProvider, camp)
+    setRoutes({ shieldsSecret }, { apiProvider, server: camp })
   })
 
   context('the password is correct', function () {
     it('returns a valid JSON response', async function () {
-      const { username, password } = validCredentials
       const { statusCode, body } = await got(`${baseUrl}/$github-auth/tokens`, {
-        username,
-        password,
+        username: '',
+        password: shieldsSecret,
         responseType: 'json',
       })
       expect(statusCode).to.equal(200)
@@ -65,14 +46,18 @@ describe('GitHub admin route', function () {
   })
 
   // Disabled because this code isn't modified often and the test is very
-  // slow. I wasn't able to make this work with fake timers:
+  // slow. To run it, run `SLOW=true npm run test:core`
+  //
+  // I wasn't able to make this work with fake timers:
   // https://github.com/sinonjs/sinon/issues/1739
-  // context('the password is missing', function() {
-  //   it('returns the expected message', async function() {
-  //     this.timeout(11000)
-  //     const res = await fetch(`${baseUrl}/$github-auth/tokens`)
-  //     expect(res.ok).to.be.true
-  //     expect(await res.text()).to.equal('"Invalid secret."')
-  //   })
-  // })
+  if (process.env.SLOW) {
+    context('the password is missing', function () {
+      it('returns the expected message', async function () {
+        this.timeout(11000)
+        const { statusCode, body } = await got(`${baseUrl}/$github-auth/tokens`)
+        expect(statusCode).to.equal(200)
+        expect(body).to.equal('"Invalid secret."')
+      })
+    })
+  }
 })

services/github/github-constellation.js

@@ -4,7 +4,6 @@ const path = require('path')
 const { AuthHelper } = require('../../core/base-service/auth-helper')
 const RedisTokenPersistence = require('../../core/token-pooling/redis-token-persistence')
 const FsTokenPersistence = require('../../core/token-pooling/fs-token-persistence')
-const serverSecrets = require('../../lib/server-secrets')
 const log = require('../../core/server/log')
 const GithubApiProvider = require('./github-api-provider')
 const { setRoutes: setAdminRoutes } = require('./auth/admin')
@@ -28,8 +27,9 @@ class GithubConstellation {
   constructor(config) {
     this._debugEnabled = config.service.debug.enabled
     this._debugIntervalSeconds = config.service.debug.intervalSeconds
+    this.shieldsSecret = config.private.shields_secret
 
-    const { redis_url: redisUrl } = config.private
+    const { redis_url: redisUrl, gh_token: globalToken } = config.private
     const { dir: persistenceDir } = config.persistence
     if (redisUrl) {
       log('RedisTokenPersistence configured with redisUrl')
@@ -46,10 +46,8 @@ class GithubConstellation {
       this.persistence = new FsTokenPersistence({ path: userTokensPath })
     }
 
-    const globalToken = serverSecrets.gh_token
-    const baseUrl = process.env.GITHUB_URL || 'https://api.github.com'
     this.apiProvider = new GithubApiProvider({
-      baseUrl,
+      baseUrl: process.env.GITHUB_URL || 'https://api.github.com',
       globalToken,
       withPooling: !globalToken,
       onTokenInvalidated: tokenString => this.onTokenInvalidated(tokenString),
@@ -84,7 +82,8 @@ class GithubConstellation {
       this.apiProvider.addToken(tokenString)
     })
 
-    setAdminRoutes(this.apiProvider, server)
+    const { shieldsSecret, apiProvider } = this
+    setAdminRoutes({ shieldsSecret }, { apiProvider, server })
 
     if (this.oauthHelper.isConfigured) {
       setAcceptorRoutes({