Optionally persist [Github] tokens in Redis (#1939)

This is a fairly simple addition of a Redis-backed TokenPersistence. When GithubConstellation is initialized, it will create a FsTokenPersistence or a RedisTokenPersistence based on configuration. Have added tests of the Redis backend as an integration test, and ensured the server starts up correctly when a `REDIS_URL` is configured.

Ref: #1848

.circleci/config.yml

@@ -26,6 +26,7 @@ jobs:
   main:
     docker:
       - image: shieldsio/shields-ci-node-8:0.0.3
+      - image: redis
     working_directory: ~/repo
     steps:
       - checkout
@@ -62,6 +63,7 @@ jobs:
   main@node-latest:
     docker:
       - image: shieldsio/shields-ci-node-latest:0.0.3
+      - image: redis
     working_directory: ~/repo
     steps:
       - checkout

CONTRIBUTING.md

@@ -130,6 +130,10 @@ e.g. **[Travis] Fix timeout issues**
 
 When changing other code, please add unit tests.
 
+To run the integration tests, you must have redis installed and in your PATH.
+Use `brew install redis`, `yum install redis`, etc. The test runner will
+start the server automatically.
+
 [service-tests]: https://github.com/badges/shields/blob/master/doc/service-tests.md
 
 ### Code organization

Makefile

@@ -69,17 +69,6 @@ deploy-heroku:
 	git push -f origin gh-pages:gh-pages) || git checkout master
 	git checkout master
 
-setup:
-	curl http://download.redis.io/releases/redis-2.8.8.tar.gz >redis.tar.gz \
-	&& tar xf redis.tar.gz \
-	&& rm redis.tar.gz \
-	&& mv redis-2.8.8 redis \
-	&& cd redis \
-	&& make
-
-redis:
-	./redis/src/redis-server
-
 test:
 	npm test
 

doc/self-hosting.md

@@ -116,6 +116,10 @@ npm run build  # Not sure why, but this needs to be run before deploying.
 now
 ```
 
+## Persistence
+
+To enable Redis-backed GitHub token persistence, point `REDIS_URL` to your
+Redis installation.
 
 Server secrets
 --------------

lib/fs-token-persistence.js

@@ -0,0 +1,45 @@
+'use strict'
+
+const fsos = require('fsos')
+const githubAuth = require('./github-auth')
+const TokenPersistence = require('./token-persistence')
+
+class FsTokenPersistence extends TokenPersistence {
+  constructor({ path }) {
+    super()
+    this.path = path
+  }
+
+  async initialize() {
+    let contents
+    try {
+      contents = await fsos.get(this.path)
+    } catch (e) {
+      if (e.code === 'ENOENT') {
+        contents = '[]'
+      } else {
+        throw e
+      }
+    }
+
+    const tokens = JSON.parse(contents)
+    tokens.forEach(tokenString => {
+      githubAuth.addGithubToken(tokenString)
+    })
+  }
+
+  async save() {
+    const tokens = githubAuth.getAllTokenIds()
+    await fsos.set(this.path, JSON.stringify(tokens))
+  }
+
+  async onTokenAdded(token) {
+    await this.save()
+  }
+
+  async onTokenRemoved(token) {
+    await this.save()
+  }
+}
+
+module.exports = FsTokenPersistence

lib/token-persistence.spec.js → lib/fs-token-persistence.spec.js

@@ -4,17 +4,17 @@ const fs = require('fs')
 const tmp = require('tmp')
 const readFile = require('fs-readfile-promise')
 const { expect } = require('chai')
-const TokenPersistence = require('./token-persistence')
+const FsTokenPersistence = require('./fs-token-persistence')
 const githubAuth = require('./github-auth')
 
-describe('Token persistence', function() {
+describe('File system token persistence', function() {
   beforeEach(githubAuth.removeAllTokens)
   afterEach(githubAuth.removeAllTokens)
 
   let path, persistence
   beforeEach(function() {
     path = tmp.tmpNameSync()
-    persistence = new TokenPersistence({ path })
+    persistence = new FsTokenPersistence({ path })
   })
 
   context('when the file does not exist', function() {
@@ -46,7 +46,7 @@ describe('Token persistence', function() {
     })
 
     context('when tokens are added', function() {
-      it('writes them to the file', async function() {
+      it('saves the change', async function() {
         const newToken = 'e'.repeat(40)
         const expected = initialTokens.slice()
         expected.push(newToken)
@@ -55,13 +55,13 @@ describe('Token persistence', function() {
         githubAuth.addGithubToken(newToken)
         await persistence.noteTokenAdded(newToken)
 
-        const json = JSON.parse(await readFile(path))
-        expect(json).to.deep.equal(expected)
+        const savedTokens = JSON.parse(await readFile(path))
+        expect(savedTokens).to.deep.equal(expected)
       })
     })
 
     context('when tokens are removed', function() {
-      it('writes them to the file', async function() {
+      it('saves the change', async function() {
         const expected = Array.from(initialTokens)
         const toRemove = expected.pop()
 
@@ -70,8 +70,8 @@ describe('Token persistence', function() {
         githubAuth.rmGithubToken(toRemove)
         await persistence.noteTokenRemoved(toRemove)
 
-        const json = JSON.parse(await readFile(path))
-        expect(json).to.deep.equal(expected)
+        const savedTokens = JSON.parse(await readFile(path))
+        expect(savedTokens).to.deep.equal(expected)
       })
     })
   })

lib/redis-token-persistence.integration.js

@@ -0,0 +1,112 @@
+'use strict'
+
+const { promisify } = require('util')
+const RedisServer = require('redis-server')
+const redis = require('redis')
+const { expect } = require('chai')
+const RedisTokenPersistence = require('./redis-token-persistence')
+const githubAuth = require('./github-auth')
+
+describe('Redis token persistence', function() {
+  beforeEach(githubAuth.removeAllTokens)
+  afterEach(githubAuth.removeAllTokens)
+
+  let server
+  // In CI, expect redis already to be running.
+  if (!process.env.CI) {
+    beforeEach(async function() {
+      server = new RedisServer({ config: { host: 'localhost' } })
+      await server.open()
+    })
+  }
+
+  const key = 'tokenPersistenceIntegrationTest'
+
+  let client
+  beforeEach(async function() {
+    client = redis.createClient()
+    const del = promisify(client.del).bind(client)
+    await del(key)
+  })
+  afterEach(async function() {
+    if (client) {
+      const quit = promisify(client.quit).bind(client)
+      await quit()
+      client = undefined
+    }
+  })
+
+  if (!process.env.CI) {
+    afterEach(async function() {
+      await server.close()
+      server = undefined
+    })
+  }
+
+  let persistence
+  beforeEach(function() {
+    persistence = new RedisTokenPersistence({ key })
+  })
+  afterEach(async function() {
+    if (persistence) {
+      await persistence.stop()
+      persistence = undefined
+    }
+  })
+
+  context('when the key does not exist', function() {
+    it('does nothing', async function() {
+      await persistence.initialize()
+      expect(githubAuth.getAllTokenIds()).to.deep.equal([])
+    })
+  })
+
+  context('when the key exists', function() {
+    const initialTokens = ['a', 'b', 'c'].map(char => char.repeat(40))
+
+    beforeEach(async function() {
+      const rpush = promisify(client.rpush).bind(client)
+      await rpush(key, initialTokens)
+    })
+
+    let lrange
+    beforeEach(function() {
+      lrange = promisify(client.lrange).bind(client)
+    })
+
+    it('loads the contents', async function() {
+      await persistence.initialize()
+      expect(githubAuth.getAllTokenIds()).to.deep.equal(initialTokens)
+    })
+
+    context('when tokens are added', function() {
+      it('saves the change', async function() {
+        const newToken = 'e'.repeat(40)
+        const expected = initialTokens.slice()
+        expected.push(newToken)
+
+        await persistence.initialize()
+        githubAuth.addGithubToken(newToken)
+        await persistence.noteTokenAdded(newToken)
+
+        const savedTokens = await lrange(key, 0, -1)
+        expect(savedTokens).to.deep.equal(expected)
+      })
+    })
+
+    context('when tokens are removed', function() {
+      it('saves the change', async function() {
+        const expected = Array.from(initialTokens)
+        const toRemove = expected.pop()
+
+        await persistence.initialize()
+
+        githubAuth.rmGithubToken(toRemove)
+        await persistence.noteTokenRemoved(toRemove)
+
+        const savedTokens = await lrange(key, 0, -1)
+        expect(savedTokens).to.deep.equal(expected)
+      })
+    })
+  })
+})

lib/redis-token-persistence.js

@@ -0,0 +1,52 @@
+'use strict'
+
+const redis = require('redis')
+const { promisify } = require('util')
+const log = require('./log')
+const githubAuth = require('./github-auth')
+const TokenPersistence = require('./token-persistence')
+
+class RedisTokenPersistence extends TokenPersistence {
+  constructor({ url, key }) {
+    super()
+    this.url = url
+    this.key = key
+  }
+
+  async initialize() {
+    this.client = redis.createClient(this.url)
+    this.client.on('error', e => {
+      log.error(e)
+    })
+
+    const lrange = promisify(this.client.lrange).bind(this.client)
+
+    let tokens
+    try {
+      tokens = await lrange(this.key, 0, -1)
+    } catch (e) {
+      log.error(e)
+    }
+
+    tokens.forEach(tokenString => {
+      githubAuth.addGithubToken(tokenString)
+    })
+  }
+
+  async stop() {
+    const quit = promisify(this.client.quit).bind(this.client)
+    await quit()
+  }
+
+  async onTokenAdded(token) {
+    const rpush = promisify(this.client.rpush).bind(this.client)
+    await rpush(this.key, token)
+  }
+
+  async onTokenRemoved(token) {
+    const lrem = promisify(this.client.lrem).bind(this.client)
+    await lrem(this.key, 0, token)
+  }
+}
+
+module.exports = RedisTokenPersistence

lib/server-config.js

@@ -51,6 +51,7 @@ const config = {
   },
   persistence: {
     dir: process.env.PERSISTENCE_DIR || './private',
+    redisUrl: process.env.REDIS_URL || process.env.REDISTOGO_URL,
   },
   services: {
     github: {

lib/token-persistence.js

@@ -1,53 +1,40 @@
 'use strict'
 
-const fsos = require('fsos')
-const githubAuth = require('./github-auth')
 const log = require('./log')
 
 // This is currently bound to the legacy github auth code. That will be
 // replaced with a dependency-injected token provider.
 class TokenPersistence {
-  constructor({ path }) {
-    this.path = path
-
+  constructor() {
     this.noteTokenAdded = this.noteTokenAdded.bind(this)
     this.noteTokenRemoved = this.noteTokenRemoved.bind(this)
   }
 
   async initialize() {
-    let contents
-    try {
-      contents = await fsos.get(this.path)
-    } catch (e) {
-      if (e.code === 'ENOENT') {
-        contents = '[]'
-      } else {
-        throw e
-      }
-    }
-
-    const tokens = JSON.parse(contents)
-    tokens.forEach(tokenString => {
-      githubAuth.addGithubToken(tokenString)
-    })
+    throw Error('initialize() is not implemented')
   }
 
-  async save() {
-    const tokens = githubAuth.getAllTokenIds()
-    await fsos.set(this.path, JSON.stringify(tokens))
+  async stop() {}
+
+  async onTokenAdded(token) {
+    throw Error('onTokenAdded() is not implemented')
   }
 
   async noteTokenAdded(token) {
     try {
-      await this.save()
+      await this.onTokenAdded(token)
     } catch (e) {
       log.error(e)
     }
   }
 
+  async onTokenRemoved(token) {
+    throw Error('onTokenRemoved() is not implemented')
+  }
+
   async noteTokenRemoved(token) {
     try {
-      await this.save()
+      await this.onTokenRemoved(token)
     } catch (e) {
       log.error(e)
     }