diff --git a/config/custom-environment-variables.yml b/config/custom-environment-variables.yml index 9b760f285eba36f4646da86a70f6182ed4d908a6..6019d033a4e420aaae2e22b23abc7a5ae1887937 100644 --- a/config/custom-environment-variables.yml +++ b/config/custom-environment-variables.yml @@ -30,9 +30,6 @@ public: __name: 'ALLOWED_ORIGIN' __format: 'json' - persistence: - dir: 'PERSISTENCE_DIR' - services: bitbucketServer: authorizedOrigins: 'BITBUCKET_SERVER_ORIGINS' diff --git a/config/default.yml b/config/default.yml index e782ca08572cb816eaac144e025c591572edba40..170c291292a445139551c82d943cebb3a593504b 100644 --- a/config/default.yml +++ b/config/default.yml @@ -16,9 +16,6 @@ public: cors: allowedOrigin: [] - persistence: - dir: './private' - services: github: baseUri: 'https://api.github.com/' diff --git a/core/server/server.js b/core/server/server.js index 60988a40b2a49f779cc1d04b24bd4194dd2bf81d..5619f3b761e3fc76348a358508ba0b7dd020dc4d 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -117,9 +117,6 @@ const publicConfigSchema = Joi.object({ cors: { allowedOrigin: Joi.array().items(optionalUrl).required(), }, - persistence: { - dir: Joi.string().required(), - }, services: Joi.object({ bitbucketServer: defaultService, drone: defaultService, @@ -223,7 +220,6 @@ class Server { } this.githubConstellation = new GithubConstellation({ - persistence: publicConfig.persistence, service: publicConfig.services.github, private: privateConfig, }) diff --git a/core/token-pooling/fs-token-persistence.js b/core/token-pooling/fs-token-persistence.js deleted file mode 100644 index 95c8eb69d9b1b8e3e4412c7936fd05c5cc346cb0..0000000000000000000000000000000000000000 --- a/core/token-pooling/fs-token-persistence.js +++ /dev/null @@ -1,53 +0,0 @@ -'use strict' - -const fsos = require('fsos') -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) - this._tokens = new Set(tokens) - return tokens - } - - async save() { - const tokens = Array.from(this._tokens) - await fsos.set(this.path, JSON.stringify(tokens)) - } - - async onTokenAdded(token) { - /* istanbul ignore if */ - if (!this._tokens) { - throw Error('initialize() has not been called') - } - this._tokens.add(token) - await this.save() - } - - async onTokenRemoved(token) { - /* istanbul ignore if */ - if (!this._tokens) { - throw Error('initialize() has not been called') - } - this._tokens.delete(token) - await this.save() - } -} - -module.exports = FsTokenPersistence diff --git a/core/token-pooling/fs-token-persistence.spec.js b/core/token-pooling/fs-token-persistence.spec.js deleted file mode 100644 index 4624538c9ed2346111517351a6fb7153ec4e6a06..0000000000000000000000000000000000000000 --- a/core/token-pooling/fs-token-persistence.spec.js +++ /dev/null @@ -1,72 +0,0 @@ -'use strict' - -const fs = require('fs') -const tmp = require('tmp') -const readFile = require('fs-readfile-promise') -const { expect } = require('chai') -const FsTokenPersistence = require('./fs-token-persistence') - -describe('File system token persistence', function () { - let path, persistence - beforeEach(function () { - path = tmp.tmpNameSync() - persistence = new FsTokenPersistence({ path }) - }) - - context('when the file does not exist', function () { - it('does nothing', async function () { - const tokens = await persistence.initialize() - expect(tokens).to.deep.equal([]) - }) - - it('saving creates an empty file', async function () { - await persistence.initialize() - - await persistence.save() - - const json = JSON.parse(await readFile(path)) - expect(json).to.deep.deep.equal([]) - }) - }) - - context('when the file exists', function () { - const initialTokens = ['a', 'b', 'c'].map(char => char.repeat(40)) - - beforeEach(async function () { - fs.writeFileSync(path, JSON.stringify(initialTokens)) - }) - - it('loads the contents', async function () { - const tokens = await persistence.initialize() - expect(tokens).to.deep.equal(initialTokens) - }) - - context('when tokens are added', function () { - it('saves the change', async function () { - const newToken = 'e'.repeat(40) - const expected = Array.from(initialTokens) - expected.push(newToken) - - await persistence.initialize() - await persistence.noteTokenAdded(newToken) - - const savedTokens = JSON.parse(await readFile(path)) - 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() - - await persistence.noteTokenRemoved(toRemove) - - const savedTokens = JSON.parse(await readFile(path)) - expect(savedTokens).to.deep.equal(expected) - }) - }) - }) -}) diff --git a/core/token-pooling/redis-token-persistence.js b/core/token-pooling/redis-token-persistence.js index 992dd7e8dddde009d23fe9848490bca75e545219..05a5ee3fbc724ea5688c291f95d00ef030d2dcbb 100644 --- a/core/token-pooling/redis-token-persistence.js +++ b/core/token-pooling/redis-token-persistence.js @@ -3,13 +3,13 @@ const { URL } = require('url') const Redis = require('ioredis') const log = require('../server/log') -const TokenPersistence = require('./token-persistence') -module.exports = class RedisTokenPersistence extends TokenPersistence { +module.exports = class RedisTokenPersistence { constructor({ url, key }) { - super() this.url = url this.key = key + this.noteTokenAdded = this.noteTokenAdded.bind(this) + this.noteTokenRemoved = this.noteTokenRemoved.bind(this) } async initialize() { @@ -40,4 +40,20 @@ module.exports = class RedisTokenPersistence extends TokenPersistence { async onTokenRemoved(token) { await this.redis.srem(this.key, token) } + + async noteTokenAdded(token) { + try { + await this.onTokenAdded(token) + } catch (e) { + log.error(e) + } + } + + async noteTokenRemoved(token) { + try { + await this.onTokenRemoved(token) + } catch (e) { + log.error(e) + } + } } diff --git a/core/token-pooling/token-persistence.js b/core/token-pooling/token-persistence.js deleted file mode 100644 index 685ccf97eb46b250183145493c278444a7eda258..0000000000000000000000000000000000000000 --- a/core/token-pooling/token-persistence.js +++ /dev/null @@ -1,44 +0,0 @@ -'use strict' - -const log = require('../server/log') - -// This is currently bound to the legacy github auth code. That will be -// replaced with a dependency-injected token provider. -class TokenPersistence { - constructor() { - this.noteTokenAdded = this.noteTokenAdded.bind(this) - this.noteTokenRemoved = this.noteTokenRemoved.bind(this) - } - - async initialize() { - throw Error('initialize() is not implemented') - } - - async stop() {} - - async onTokenAdded(token) { - throw Error('onTokenAdded() is not implemented') - } - - async noteTokenAdded(token) { - try { - await this.onTokenAdded(token) - } catch (e) { - log.error(e) - } - } - - async onTokenRemoved(token) { - throw Error('onTokenRemoved() is not implemented') - } - - async noteTokenRemoved(token) { - try { - await this.onTokenRemoved(token) - } catch (e) { - log.error(e) - } - } -} - -module.exports = TokenPersistence diff --git a/package-lock.json b/package-lock.json index 60876ca34def2b366d84f8a65e4fee883a450e0f..8de6c2fd509c40159f160ae6d1e3704e76dc8d49 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8867,11 +8867,6 @@ "integrity": "sha1-iYUI2iIm84DfkEcoRWhJwVAaSw0=", "dev": true }, - "asap": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/asap/-/asap-2.0.6.tgz", - "integrity": "sha1-5QNHYR1+aQlDIIu9r+vLwvuGbUY=" - }, "asn1": { "version": "0.2.4", "resolved": "https://registry.npmjs.org/asn1/-/asn1-0.2.4.tgz", @@ -16796,25 +16791,6 @@ "nan": "^2.12.1" } }, - "fsos": { - "version": "1.1.6", - "resolved": "https://registry.npmjs.org/fsos/-/fsos-1.1.6.tgz", - "integrity": "sha512-44MKwAuDfB14pojgokzqEhavMO0s1vv4H+WhsmHYB8fmoJI6YUephlD30Vak6paE6bbY3xd3b3Wa7vAgSglk8A==", - "requires": { - "mkdirp": "~0.5.1", - "promise": "~7.0.4" - }, - "dependencies": { - "mkdirp": { - "version": "0.5.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz", - "integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==", - "requires": { - "minimist": "^1.2.5" - } - } - } - }, "function-bind": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.1.tgz", @@ -29523,14 +29499,6 @@ "tdigest": "^0.1.1" } }, - "promise": { - "version": "7.0.4", - "resolved": "https://registry.npmjs.org/promise/-/promise-7.0.4.tgz", - "integrity": "sha1-Nj6EpMNsg1a4kP7WLJHOhdAu1Tk=", - "requires": { - "asap": "~2.0.3" - } - }, "promise-inflight": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/promise-inflight/-/promise-inflight-1.0.1.tgz", diff --git a/package.json b/package.json index 9d408c3f3610fbd39880f419bfe1c1702c6b8ff1..17c07c99cb3d076f11f58964afd4212449e853ca 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "url": "https://github.com/badges/shields" }, "dependencies": { - "joi": "17.2.1", "@sentry/node": "^5.24.2", "@shields_io/camp": "^18.0.0", "badge-maker": "file:badge-maker", @@ -38,11 +37,11 @@ "emojic": "^1.1.16", "escape-string-regexp": "^4.0.0", "fast-xml-parser": "^3.17.4", - "fsos": "^1.1.6", "glob": "^7.1.6", "graphql": "^14.7.0", "graphql-tag": "^2.11.0", "ioredis": "4.17.3", + "joi": "17.2.1", "joi-extension-semver": "5.0.0", "js-yaml": "^3.14.0", "jsonpath": "~1.0.2", diff --git a/scripts/redis-connectivity-test.js b/scripts/redis-connectivity-test.js index 2aaf4baffec2910c523d8133c285f2f35a2296c6..de161e029a8913e26456d008e523c5598090690b 100644 --- a/scripts/redis-connectivity-test.js +++ b/scripts/redis-connectivity-test.js @@ -5,7 +5,6 @@ console.log(config) const GithubConstellation = require('../services/github/github-constellation') const { persistence } = new GithubConstellation({ - persistence: config.public.persistence, service: config.public.services.github, private: config.private, }) diff --git a/services/github/github-constellation.js b/services/github/github-constellation.js index fe6ccc4a9dd684f9f55e4ca0daff424786f423dd..7b8d32242e085ed94af5ddf17b86c6efd5421743 100644 --- a/services/github/github-constellation.js +++ b/services/github/github-constellation.js @@ -1,9 +1,7 @@ 'use strict' -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 log = require('../../core/server/log') const GithubApiProvider = require('./github-api-provider') const { setRoutes: setAdminRoutes } = require('./auth/admin') @@ -30,20 +28,12 @@ class GithubConstellation { this.shieldsSecret = config.private.shields_secret const { redis_url: redisUrl, gh_token: globalToken } = config.private - const { dir: persistenceDir } = config.persistence if (redisUrl) { - log('RedisTokenPersistence configured with redisUrl') + log('Token persistence configured with redisUrl') this.persistence = new RedisTokenPersistence({ url: redisUrl, key: 'githubUserTokens', }) - } else { - const userTokensPath = path.resolve( - persistenceDir, - 'github-user-tokens.json' - ) - log(`FsTokenPersistence configured with ${userTokensPath}`) - this.persistence = new FsTokenPersistence({ path: userTokensPath }) } this.apiProvider = new GithubApiProvider({ @@ -71,6 +61,10 @@ class GithubConstellation { this.scheduleDebugLogging() + if (!this.persistence) { + return + } + let tokens = [] try { tokens = await this.persistence.initialize() @@ -95,6 +89,9 @@ class GithubConstellation { } onTokenAdded(tokenString) { + if (!this.persistence) { + throw Error('Token persistence is not configured') + } this.apiProvider.addToken(tokenString) process.nextTick(async () => { try { @@ -106,13 +103,15 @@ class GithubConstellation { } onTokenInvalidated(tokenString) { - process.nextTick(async () => { - try { - await this.persistence.noteTokenRemoved(tokenString) - } catch (e) { - log.error(e) - } - }) + if (this.persistence) { + process.nextTick(async () => { + try { + await this.persistence.noteTokenRemoved(tokenString) + } catch (e) { + log.error(e) + } + }) + } } async stop() { @@ -121,10 +120,12 @@ class GithubConstellation { this.debugInterval = undefined } - try { - await this.persistence.stop() - } catch (e) { - log.error(e) + if (this.persistence) { + try { + await this.persistence.stop() + } catch (e) { + log.error(e) + } } } }