diff --git a/.eslintrc.yml b/.eslintrc.yml index 153b7513df..fb634c5880 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -30,6 +30,7 @@ overrides: 'category', 'isDeprecated', 'route', + 'auth', 'examples', '_cacheLength', 'defaultBadgeData', diff --git a/core/base-service/auth-helper.js b/core/base-service/auth-helper.js new file mode 100644 index 0000000000..784311d6d6 --- /dev/null +++ b/core/base-service/auth-helper.js @@ -0,0 +1,43 @@ +'use strict' + +class AuthHelper { + constructor({ userKey, passKey, isRequired = false }, privateConfig) { + if (!userKey && !passKey) { + throw Error('Expected userKey or passKey to be set') + } + + this._userKey = userKey + this._passKey = passKey + this.user = userKey ? privateConfig[userKey] : undefined + this.pass = passKey ? privateConfig[passKey] : undefined + this.isRequired = isRequired + } + + get isConfigured() { + return ( + (this._userKey ? Boolean(this.user) : true) && + (this._passKey ? Boolean(this.pass) : true) + ) + } + + get isValid() { + if (this.isRequired) { + return this.isConfigured + } else { + const configIsEmpty = !this.user && !this.pass + return this.isConfigured || configIsEmpty + } + } + + get basicAuth() { + const { user, pass } = this + return this.isConfigured ? { user, pass } : undefined + } + + get bearerAuthHeader() { + const { pass } = this + return this.isConfigured ? { Authorization: `Bearer ${pass}` } : undefined + } +} + +module.exports = { AuthHelper } diff --git a/core/base-service/auth-helper.spec.js b/core/base-service/auth-helper.spec.js new file mode 100644 index 0000000000..3bf4887cd3 --- /dev/null +++ b/core/base-service/auth-helper.spec.js @@ -0,0 +1,96 @@ +'use strict' + +const { expect } = require('chai') +const { test, given, forCases } = require('sazerac') +const { AuthHelper } = require('./auth-helper') + +describe('AuthHelper', function() { + it('throws without userKey or passKey', function() { + expect(() => new AuthHelper({}, {})).to.throw( + Error, + 'Expected userKey or passKey to be set' + ) + }) + + describe('isValid', function() { + function validate(config, privateConfig) { + return new AuthHelper(config, privateConfig).isValid + } + test(validate, () => { + forCases([ + // Fully configured user + pass. + given( + { userKey: 'myci_user', passKey: 'myci_pass', isRequired: true }, + { myci_user: 'admin', myci_pass: 'abc123' } + ), + given( + { userKey: 'myci_user', passKey: 'myci_pass' }, + { myci_user: 'admin', myci_pass: 'abc123' } + ), + // Fully configured user or pass. + given( + { userKey: 'myci_user', isRequired: true }, + { myci_user: 'admin' } + ), + given( + { passKey: 'myci_pass', isRequired: true }, + { myci_pass: 'abc123' } + ), + given({ userKey: 'myci_user' }, { myci_user: 'admin' }), + given({ passKey: 'myci_pass' }, { myci_pass: 'abc123' }), + // Empty config. + given({ userKey: 'myci_user', passKey: 'myci_pass' }, {}), + given({ userKey: 'myci_user' }, {}), + given({ passKey: 'myci_pass' }, {}), + ]).expect(true) + + forCases([ + // Partly configured. + given( + { userKey: 'myci_user', passKey: 'myci_pass', isRequired: true }, + { myci_user: 'admin' } + ), + given( + { userKey: 'myci_user', passKey: 'myci_pass' }, + { myci_user: 'admin' } + ), + // Missing required config. + given( + { userKey: 'myci_user', passKey: 'myci_pass', isRequired: true }, + {} + ), + given({ userKey: 'myci_user', isRequired: true }, {}), + given({ passKey: 'myci_pass', isRequired: true }, {}), + ]).expect(false) + }) + }) + + describe('basicAuth', function() { + function validate(config, privateConfig) { + return new AuthHelper(config, privateConfig).basicAuth + } + test(validate, () => { + forCases([ + given( + { userKey: 'myci_user', passKey: 'myci_pass', isRequired: true }, + { myci_user: 'admin', myci_pass: 'abc123' } + ), + given( + { userKey: 'myci_user', passKey: 'myci_pass' }, + { myci_user: 'admin', myci_pass: 'abc123' } + ), + ]).expect({ user: 'admin', pass: 'abc123' }) + given({ userKey: 'myci_user' }, { myci_user: 'admin' }).expect({ + user: 'admin', + pass: undefined, + }) + given({ passKey: 'myci_pass' }, { myci_pass: 'abc123' }).expect({ + user: undefined, + pass: 'abc123', + }) + given({ userKey: 'myci_user', passKey: 'myci_pass' }, {}).expect( + undefined + ) + }) + }) +}) diff --git a/core/base-service/base.js b/core/base-service/base.js index 736ea13fc7..4db3ecae4a 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -4,6 +4,7 @@ const decamelize = require('decamelize') // See available emoji at http://emoji.muan.co/ const emojic = require('emojic') const Joi = require('@hapi/joi') +const { AuthHelper } = require('./auth-helper') const { assertValidCategory } = require('./categories') const checkErrorResponse = require('./check-error-response') const coalesceBadge = require('./coalesce-badge') @@ -11,6 +12,7 @@ const { NotFound, InvalidResponse, Inaccessible, + ImproperlyConfigured, InvalidParameter, Deprecated, } = require('./errors') @@ -132,6 +134,33 @@ module.exports = class BaseService { throw new Error(`Route not defined for ${this.name}`) } + /** + * Configuration for the authentication helper that prepares credentials + * for upstream requests. + * + * @abstract + * @return {object} auth + * @return {string} auth.userKey + * (Optional) The key from `privateConfig` to use as the username. + * @return {string} auth.passKey + * (Optional) The key from `privateConfig` to use as the password. + * If auth is configured, either `userKey` or `passKey` is required. + * @return {string} auth.isRequired + * (Optional) If `true`, the service will return `NotFound` unless the + * configured credentials are present. + * + * See also the config schema in `./server.js` and `doc/server-secrets.md`. + * + * To use the configured auth in the handler or fetch method, pass the + * credentials to the request. For example: + * `{ options: { auth: this.authHelper.basicAuth } }` + * `{ options: { headers: this.authHelper.bearerAuthHeader } }` + * `{ options: { qs: { token: this.authHelper.pass } } }` + */ + static get auth() { + return undefined + } + /** * Example URLs for this service. These should use the format * specified in `route`, and can be used to demonstrate how to use badges for @@ -238,8 +267,9 @@ module.exports = class BaseService { return result } - constructor({ sendAndCacheRequest }, { handleInternalErrors }) { + constructor({ sendAndCacheRequest, authHelper }, { handleInternalErrors }) { this._requestFetcher = sendAndCacheRequest + this.authHelper = authHelper this._handleInternalErrors = handleInternalErrors } @@ -303,6 +333,7 @@ module.exports = class BaseService { color: 'red', } } else if ( + error instanceof ImproperlyConfigured || error instanceof InvalidResponse || error instanceof Inaccessible || error instanceof Deprecated @@ -353,12 +384,26 @@ module.exports = class BaseService { trace.logTrace('inbound', emojic.ticket, 'Named params', namedParams) trace.logTrace('inbound', emojic.crayon, 'Query params', queryParams) - const serviceInstance = new this(context, config) + // Like the service instance, the auth helper could be reused for each request. + // However, moving its instantiation to `register()` makes `invoke()` harder + // to test. + const authHelper = this.auth + ? new AuthHelper(this.auth, config.private) + : undefined + + const serviceInstance = new this({ ...context, authHelper }, config) let serviceError + if (authHelper && !authHelper.isValid) { + const prettyMessage = authHelper.isRequired + ? 'credentials have not been configured' + : 'credentials are misconfigured' + serviceError = new ImproperlyConfigured({ prettyMessage }) + } + const { queryParamSchema } = this.route let transformedQueryParams - if (queryParamSchema) { + if (!serviceError && queryParamSchema) { try { transformedQueryParams = validate( { diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js index 0a9297fca5..7c0d68e1e2 100644 --- a/core/base-service/base.spec.js +++ b/core/base-service/base.spec.js @@ -64,7 +64,7 @@ class DummyService extends BaseService { } describe('BaseService', function() { - const defaultConfig = { handleInternalErrors: false } + const defaultConfig = { handleInternalErrors: false, private: {} } it('Invokes the handler as expected', async function() { expect( @@ -482,4 +482,43 @@ describe('BaseService', function() { } }) }) + + describe('auth', function() { + class AuthService extends DummyService { + static get auth() { + return { + passKey: 'myci_pass', + isRequired: true, + } + } + + async handle() { + return { + message: `The CI password is ${this.authHelper.pass}`, + } + } + } + + it('when auth is configured properly, invoke() sets authHelper', async function() { + expect( + await AuthService.invoke( + {}, + { defaultConfig, private: { myci_pass: 'abc123' } }, + { namedParamA: 'bar.bar.bar' } + ) + ).to.deep.equal({ message: 'The CI password is abc123' }) + }) + + it('when auth is not configured properly, invoke() returns inacessible', async function() { + expect( + await AuthService.invoke({}, defaultConfig, { + namedParamA: 'bar.bar.bar', + }) + ).to.deep.equal({ + color: 'lightgray', + isError: true, + message: 'credentials have not been configured', + }) + }) + }) }) diff --git a/core/base-service/errors.js b/core/base-service/errors.js index 7c5d2aa5c1..14286b1c43 100644 --- a/core/base-service/errors.js +++ b/core/base-service/errors.js @@ -72,6 +72,23 @@ class Inaccessible extends ShieldsRuntimeError { } } +class ImproperlyConfigured extends ShieldsRuntimeError { + get name() { + return 'ImproperlyConfigured' + } + get defaultPrettyMessage() { + return 'improperly configured' + } + + constructor(props = {}) { + const message = props.underlyingError + ? `ImproperlyConfigured: ${props.underlyingError.message}` + : 'ImproperlyConfigured' + super(props, message) + this.response = props.response + } +} + class InvalidParameter extends ShieldsRuntimeError { get name() { return 'InvalidParameter' @@ -106,6 +123,7 @@ class Deprecated extends ShieldsRuntimeError { module.exports = { ShieldsRuntimeError, NotFound, + ImproperlyConfigured, InvalidResponse, Inaccessible, InvalidParameter, diff --git a/core/server/server.js b/core/server/server.js index 4dd1d20583..1800d59dbd 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -248,6 +248,7 @@ module.exports = class Server { profiling: config.public.profiling, fetchLimitBytes: bytes(config.public.fetchLimit), rasterUrl: config.public.rasterUrl, + private: config.private, } ) ) diff --git a/dangerfile.js b/dangerfile.js index 886b18e092..ce5461dc5c 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -112,10 +112,13 @@ if (allFiles.length > 100) { // eslint-disable-next-line promise/prefer-await-to-then danger.git.diffForFile(file).then(({ diff }) => { - if (diff.includes('serverSecrets') && !secretsDocs.modified) { + if ( + (diff.includes('authHelper') || diff.includes('serverSecrets')) && + !secretsDocs.modified + ) { warn( [ - `:books: Remember to ensure any changes to \`serverSecrets\` `, + `:books: Remember to ensure any changes to \`config.private\` `, `in \`${file}\` are reflected in the [server secrets documentation]`, '(https://github.com/badges/shields/blob/master/doc/server-secrets.md)', ].join('') diff --git a/server.js b/server.js index efc81f32ba..0ebdc3f902 100644 --- a/server.js +++ b/server.js @@ -1,16 +1,17 @@ 'use strict' /* eslint-disable import/order */ +const fs = require('fs') +const path = require('path') + require('dotenv').config() // Set up Sentry reporting as early in the process as possible. +const config = require('config').util.toObject() const Raven = require('raven') -const serverSecrets = require('./lib/server-secrets') - -Raven.config(process.env.SENTRY_DSN || serverSecrets.sentry_dsn).install() +Raven.config(process.env.SENTRY_DSN || config.private.sentry_dsn).install() Raven.disableConsoleAlerts() -const config = require('config').util.toObject() if (+process.argv[2]) { config.public.bind.port = +process.argv[2] } @@ -21,6 +22,14 @@ if (process.argv[3]) { console.log('Configuration:') console.dir(config.public, { depth: null }) +const legacySecretsPath = path.join(__dirname, 'private', 'secret.json') +if (fs.existsSync(legacySecretsPath)) { + console.error( + `Legacy secrets file found at ${legacySecretsPath}. It should be deleted and secrets replaced with environment variables or config/local.yml` + ) + process.exit(1) +} + const Server = require('./core/server/server') const server = (module.exports = new Server(config)) diff --git a/services/azure-devops/azure-devops-base.js b/services/azure-devops/azure-devops-base.js index 803156c6c6..f6d8b13fb3 100644 --- a/services/azure-devops/azure-devops-base.js +++ b/services/azure-devops/azure-devops-base.js @@ -15,6 +15,10 @@ const latestBuildSchema = Joi.object({ }).required() module.exports = class AzureDevOpsBase extends BaseJsonService { + static get auth() { + return { passKey: 'azure_devops_token' } + } + async fetch({ url, options, schema, errorMessages }) { return this._requestJson({ schema, @@ -29,7 +33,7 @@ module.exports = class AzureDevOpsBase extends BaseJsonService { project, definitionId, branch, - headers, + auth, errorMessages ) { // Microsoft documentation: https://docs.microsoft.com/en-us/rest/api/azure/devops/build/builds/list?view=azure-devops-rest-5.0 @@ -41,7 +45,7 @@ module.exports = class AzureDevOpsBase extends BaseJsonService { statusFilter: 'completed', 'api-version': '5.0-preview.4', }, - headers, + auth, } if (branch) { diff --git a/services/azure-devops/azure-devops-coverage.service.js b/services/azure-devops/azure-devops-coverage.service.js index 9a72c5b1ff..c084b5c6fe 100644 --- a/services/azure-devops/azure-devops-coverage.service.js +++ b/services/azure-devops/azure-devops-coverage.service.js @@ -5,7 +5,7 @@ const { coveragePercentage: coveragePercentageColor, } = require('../color-formatters') const AzureDevOpsBase = require('./azure-devops-base') -const { keywords, getHeaders } = require('./azure-devops-helpers') +const { keywords } = require('./azure-devops-helpers') const documentation = `
@@ -100,7 +100,7 @@ module.exports = class AzureDevOpsCoverage extends AzureDevOpsBase { } async handle({ organization, project, definitionId, branch }) { - const headers = getHeaders() + const auth = this.authHelper.basicAuth const errorMessages = { 404: 'build pipeline or coverage not found', } @@ -109,7 +109,7 @@ module.exports = class AzureDevOpsCoverage extends AzureDevOpsBase { project, definitionId, branch, - headers, + auth, errorMessages ) // Microsoft documentation: https://docs.microsoft.com/en-us/rest/api/azure/devops/test/code%20coverage/get%20build%20code%20coverage?view=azure-devops-rest-5.0 @@ -119,7 +119,7 @@ module.exports = class AzureDevOpsCoverage extends AzureDevOpsBase { buildId, 'api-version': '5.0-preview.1', }, - headers, + auth, } const json = await this.fetch({ url, diff --git a/services/azure-devops/azure-devops-helpers.js b/services/azure-devops/azure-devops-helpers.js index 2bfbc66d38..a05dabcdfb 100644 --- a/services/azure-devops/azure-devops-helpers.js +++ b/services/azure-devops/azure-devops-helpers.js @@ -1,7 +1,6 @@ 'use strict' const Joi = require('@hapi/joi') -const serverSecrets = require('../../lib/server-secrets') const { isBuildStatus } = require('../build-status') const keywords = ['vso', 'vsts', 'azure-devops'] @@ -23,15 +22,4 @@ async function fetch(serviceInstance, { url, qs = {}, errorMessages }) { return { status } } -function getHeaders() { - const headers = {} - if (serverSecrets.azure_devops_token) { - const pat = serverSecrets.azure_devops_token - const auth = Buffer.from(`:${pat}`).toString('base64') - headers.Authorization = `basic ${auth}` - } - - return headers -} - -module.exports = { keywords, fetch, getHeaders } +module.exports = { keywords, fetch } diff --git a/services/azure-devops/azure-devops-tests.service.js b/services/azure-devops/azure-devops-tests.service.js index e1b9623f68..e0ee802cb2 100644 --- a/services/azure-devops/azure-devops-tests.service.js +++ b/services/azure-devops/azure-devops-tests.service.js @@ -6,7 +6,6 @@ const { renderTestResultBadge, } = require('../test-results') const AzureDevOpsBase = require('./azure-devops-base') -const { getHeaders } = require('./azure-devops-helpers') const commonAttrs = { keywords: ['vso', 'vsts', 'azure-devops'], @@ -192,7 +191,7 @@ module.exports = class AzureDevOpsTests extends AzureDevOpsBase { skipped_label: skippedLabel, } ) { - const headers = getHeaders() + const auth = this.authHelper.basicAuth const errorMessages = { 404: 'build pipeline or test result summary not found', } @@ -201,7 +200,7 @@ module.exports = class AzureDevOpsTests extends AzureDevOpsBase { project, definitionId, branch, - headers, + auth, errorMessages ) @@ -211,7 +210,7 @@ module.exports = class AzureDevOpsTests extends AzureDevOpsBase { url: `https://dev.azure.com/${organization}/${project}/_apis/test/ResultSummaryByBuild`, options: { qs: { buildId }, - headers, + auth, }, schema: buildTestResultSummarySchema, errorMessages, diff --git a/services/bintray/bintray.service.js b/services/bintray/bintray.service.js index 09e3940558..a32cc34de4 100644 --- a/services/bintray/bintray.service.js +++ b/services/bintray/bintray.service.js @@ -2,7 +2,6 @@ const Joi = require('@hapi/joi') const { renderVersionBadge } = require('../version') -const serverSecrets = require('../../lib/server-secrets') const { BaseJsonService } = require('..') const schema = Joi.object() @@ -23,6 +22,10 @@ module.exports = class Bintray extends BaseJsonService { } } + static get auth() { + return { userKey: 'bintray_user', passKey: 'bintray_apikey' } + } + static get examples() { return [ { @@ -42,19 +45,11 @@ module.exports = class Bintray extends BaseJsonService { } async fetch({ subject, repo, packageName }) { - const options = {} - if (serverSecrets.bintray_user) { - options.auth = { - user: serverSecrets.bintray_user, - pass: serverSecrets.bintray_apikey, - } - } - // https://bintray.com/docs/api/#_get_version return this._requestJson({ schema, url: `https://bintray.com/api/v1/packages/${subject}/${repo}/${packageName}/versions/_latest`, - options, + options: { auth: this.authHelper.basicAuth }, }) } diff --git a/services/drone/drone-build.service.js b/services/drone/drone-build.service.js index 3375fed785..2d2b005fd4 100644 --- a/services/drone/drone-build.service.js +++ b/services/drone/drone-build.service.js @@ -1,7 +1,6 @@ 'use strict' const Joi = require('@hapi/joi') -const serverSecrets = require('../../lib/server-secrets') const { isBuildStatus, renderBuildStatusBadge } = require('../build-status') const { optionalUrl } = require('../validators') const { BaseJsonService } = require('..') @@ -29,6 +28,10 @@ module.exports = class DroneBuild extends BaseJsonService { } } + static get auth() { + return { passKey: 'drone_token' } + } + static get examples() { return [ { @@ -85,11 +88,7 @@ module.exports = class DroneBuild extends BaseJsonService { qs: { ref: branch ? `refs/heads/${branch}` : undefined, }, - } - if (serverSecrets.drone_token) { - options.headers = { - Authorization: `Bearer ${serverSecrets.drone_token}`, - } + headers: this.authHelper.bearerAuthHeader, } if (!server) { server = 'https://cloud.drone.io' diff --git a/services/drone/drone-build.spec.js b/services/drone/drone-build.spec.js new file mode 100644 index 0000000000..41537d050a --- /dev/null +++ b/services/drone/drone-build.spec.js @@ -0,0 +1,36 @@ +'use strict' + +const { expect } = require('chai') +const nock = require('nock') +const { cleanUpNockAfterEach, defaultContext } = require('../test-helpers') +const DroneBuild = require('./drone-build.service') + +describe('DroneBuild', function() { + cleanUpNockAfterEach() + + it('Sends auth headers to cloud instance', async function() { + const token = 'abc123' + + const scope = nock('https://cloud.drone.io', { + reqheaders: { Authorization: `Bearer abc123` }, + }) + .get(/.*/) + .reply(200, { status: 'passing' }) + + expect( + await DroneBuild.invoke( + defaultContext, + { + private: { drone_token: token }, + }, + { user: 'atlassian', repo: 'python-bitbucket' } + ) + ).to.deep.equal({ + label: undefined, + message: 'passing', + color: 'brightgreen', + }) + + scope.done() + }) +}) diff --git a/services/drone/drone-build.tester.js b/services/drone/drone-build.tester.js index 9a9f3e07da..18046723c7 100644 --- a/services/drone/drone-build.tester.js +++ b/services/drone/drone-build.tester.js @@ -3,7 +3,6 @@ const Joi = require('@hapi/joi') const { isBuildStatus } = require('../build-status') const t = (module.exports = require('../tester').createServiceTester()) -const { mockDroneCreds, token, restore } = require('./drone-test-helpers') t.create('cloud-hosted build status on default branch') .get('/drone/drone.json') @@ -27,35 +26,27 @@ t.create('cloud-hosted build status on unknown repo') }) t.create('self-hosted build status on default branch') - .before(mockDroneCreds) .get('/badges/shields.json?server=https://drone.shields.io') .intercept(nock => - nock('https://drone.shields.io/api/repos', { - reqheaders: { authorization: `Bearer ${token}` }, - }) + nock('https://drone.shields.io/api/repos') .get('/badges/shields/builds/latest') .reply(200, { status: 'success' }) ) - .finally(restore) .expectBadge({ label: 'build', message: 'passing', }) t.create('self-hosted build status on named branch') - .before(mockDroneCreds) .get( '/badges/shields/feat/awesome-thing.json?server=https://drone.shields.io' ) .intercept(nock => - nock('https://drone.shields.io/api/repos', { - reqheaders: { authorization: `Bearer ${token}` }, - }) + nock('https://drone.shields.io/api/repos') .get('/badges/shields/builds/latest') .query({ ref: 'refs/heads/feat/awesome-thing' }) .reply(200, { status: 'success' }) ) - .finally(restore) .expectBadge({ label: 'build', message: 'passing', diff --git a/services/drone/drone-test-helpers.js b/services/drone/drone-test-helpers.js deleted file mode 100644 index 8cc3c5b2c5..0000000000 --- a/services/drone/drone-test-helpers.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict' - -const sinon = require('sinon') -const serverSecrets = require('../../lib/server-secrets') - -const token = 'my-token' - -function mockDroneCreds() { - serverSecrets['drone_token'] = undefined - sinon.stub(serverSecrets, 'drone_token').value(token) -} - -function restore() { - sinon.restore() -} - -module.exports = { - token, - mockDroneCreds, - restore, -} diff --git a/services/jenkins/jenkins-base.js b/services/jenkins/jenkins-base.js index a74507438f..58c9e682c9 100644 --- a/services/jenkins/jenkins-base.js +++ b/services/jenkins/jenkins-base.js @@ -1,9 +1,15 @@ 'use strict' -const serverSecrets = require('../../lib/server-secrets') const { BaseJsonService } = require('..') module.exports = class JenkinsBase extends BaseJsonService { + static get auth() { + return { + userKey: 'jenkins_user', + passKey: 'jenkins_pass', + } + } + async fetch({ url, schema, @@ -11,18 +17,13 @@ module.exports = class JenkinsBase extends BaseJsonService { errorMessages = { 404: 'instance or job not found' }, disableStrictSSL, }) { - const options = { qs, strictSSL: disableStrictSSL === undefined } - - if (serverSecrets.jenkins_user) { - options.auth = { - user: serverSecrets.jenkins_user, - pass: serverSecrets.jenkins_pass, - } - } - return this._requestJson({ url, - options, + options: { + qs, + strictSSL: disableStrictSSL === undefined, + auth: this.authHelper.basicAuth, + }, schema, errorMessages, }) diff --git a/services/jenkins/jenkins-build.spec.js b/services/jenkins/jenkins-build.spec.js index a94054d4fe..32cb856518 100644 --- a/services/jenkins/jenkins-build.spec.js +++ b/services/jenkins/jenkins-build.spec.js @@ -1,7 +1,10 @@ 'use strict' +const { expect } = require('chai') +const nock = require('nock') const { test, forCases, given } = require('sazerac') const { renderBuildStatusBadge } = require('../build-status') +const { cleanUpNockAfterEach, defaultContext } = require('../test-helpers') const JenkinsBuild = require('./jenkins-build.service') describe('JenkinsBuild', function() { @@ -54,4 +57,35 @@ describe('JenkinsBuild', function() { renderBuildStatusBadge({ status: 'not built' }) ) }) + + describe('auth', function() { + cleanUpNockAfterEach() + + const user = 'admin' + const pass = 'password' + const config = { private: { jenkins_user: user, jenkins_pass: pass } } + + it('sends the auth information as configured', async function() { + const scope = nock('https://jenkins.ubuntu.com') + .get('/server/job/curtin-vmtest-daily-x/api/json?tree=color') + // This ensures that the expected credentials are actually being sent with the HTTP request. + // Without this the request wouldn't match and the test would fail. + .basicAuth({ user, pass }) + .reply(200, { color: 'blue' }) + + expect( + await JenkinsBuild.invoke(defaultContext, config, { + protocol: 'https', + host: 'jenkins.ubuntu.com', + job: 'server/job/curtin-vmtest-daily-x', + }) + ).to.deep.equal({ + label: undefined, + message: 'passing', + color: 'brightgreen', + }) + + scope.done() + }) + }) }) diff --git a/services/jenkins/jenkins-build.tester.js b/services/jenkins/jenkins-build.tester.js index 424f10e264..fb3dbff938 100644 --- a/services/jenkins/jenkins-build.tester.js +++ b/services/jenkins/jenkins-build.tester.js @@ -1,8 +1,6 @@ 'use strict' const Joi = require('@hapi/joi') -const sinon = require('sinon') -const serverSecrets = require('../../lib/server-secrets') const { isBuildStatus } = require('../build-status') const t = (module.exports = require('../tester').createServiceTester()) @@ -22,30 +20,3 @@ t.create('build found (view)') t.create('build found (job)') .get('/https/ci.eclipse.org/jgit/job/jgit.json') .expectBadge({ label: 'build', message: isJenkinsBuildStatus }) - -const user = 'admin' -const pass = 'password' - -function mockCreds() { - serverSecrets['jenkins_user'] = undefined - serverSecrets['jenkins_pass'] = undefined - sinon.stub(serverSecrets, 'jenkins_user').value(user) - sinon.stub(serverSecrets, 'jenkins_pass').value(pass) -} - -t.create('with mock credentials') - .before(mockCreds) - .get('/https/jenkins.ubuntu.com/server/job/curtin-vmtest-daily-x.json') - .intercept(nock => - nock('https://jenkins.ubuntu.com/server/job/curtin-vmtest-daily-x') - .get(`/api/json?tree=color`) - // This ensures that the expected credentials from serverSecrets are actually being sent with the HTTP request. - // Without this the request wouldn't match and the test would fail. - .basicAuth({ - user, - pass, - }) - .reply(200, { color: 'blue' }) - ) - .finally(sinon.restore) - .expectBadge({ label: 'build', message: 'passing' }) diff --git a/services/jira/jira-base.js b/services/jira/jira-base.js deleted file mode 100644 index 4f9e5f0d2c..0000000000 --- a/services/jira/jira-base.js +++ /dev/null @@ -1,28 +0,0 @@ -'use strict' - -const serverSecrets = require('../../lib/server-secrets') -const { BaseJsonService } = require('..') - -module.exports = class JiraBase extends BaseJsonService { - static get category() { - return 'issue-tracking' - } - - async fetch({ url, qs, schema, errorMessages }) { - const options = { qs } - - if (serverSecrets.jira_user) { - options.auth = { - user: serverSecrets.jira_user, - pass: serverSecrets.jira_pass, - } - } - - return this._requestJson({ - schema, - url, - options, - errorMessages, - }) - } -} diff --git a/services/jira/jira-common.js b/services/jira/jira-common.js new file mode 100644 index 0000000000..9f51ce3fd6 --- /dev/null +++ b/services/jira/jira-common.js @@ -0,0 +1,8 @@ +'use strict' + +const authConfig = { + userKey: 'jira_user', + passKey: 'jira_pass', +} + +module.exports = { authConfig } diff --git a/services/jira/jira-issue.service.js b/services/jira/jira-issue.service.js index fe95067f41..f903f4e6f5 100644 --- a/services/jira/jira-issue.service.js +++ b/services/jira/jira-issue.service.js @@ -1,7 +1,8 @@ 'use strict' const Joi = require('@hapi/joi') -const JiraBase = require('./jira-base') +const { authConfig } = require('./jira-common') +const { BaseJsonService } = require('..') const schema = Joi.object({ fields: Joi.object({ @@ -14,7 +15,11 @@ const schema = Joi.object({ }).required(), }).required() -module.exports = class JiraIssue extends JiraBase { +module.exports = class JiraIssue extends BaseJsonService { + static get category() { + return 'issue-tracking' + } + static get route() { return { base: 'jira/issue', @@ -22,6 +27,10 @@ module.exports = class JiraIssue extends JiraBase { } } + static get auth() { + return authConfig + } + static get examples() { return [ { @@ -68,16 +77,15 @@ module.exports = class JiraIssue extends JiraBase { async handle({ protocol, hostAndPath, issueKey }) { // Atlassian Documentation: https://developer.atlassian.com/cloud/jira/platform/rest/v2/#api-api-2-issue-issueIdOrKey-get - const url = `${protocol}://${hostAndPath}/rest/api/2/issue/${encodeURIComponent( - issueKey - )}` - const json = await this.fetch({ - url, + const json = await this._requestJson({ schema, - errorMessages: { - 404: 'issue not found', - }, + url: `${protocol}://${hostAndPath}/rest/api/2/issue/${encodeURIComponent( + issueKey + )}`, + options: { auth: this.authHelper.basicAuth }, + errorMessages: { 404: 'issue not found' }, }) + const issueStatus = json.fields.status const statusName = issueStatus.name let statusColor diff --git a/services/jira/jira-issue.spec.js b/services/jira/jira-issue.spec.js new file mode 100644 index 0000000000..bccde62ef7 --- /dev/null +++ b/services/jira/jira-issue.spec.js @@ -0,0 +1,34 @@ +'use strict' + +const { expect } = require('chai') +const nock = require('nock') +const { cleanUpNockAfterEach, defaultContext } = require('../test-helpers') +const JiraIssue = require('./jira-issue.service') +const { user, pass, config } = require('./jira-test-helpers') + +describe('JiraIssue', function() { + cleanUpNockAfterEach() + + it('sends the auth information as configured', async function() { + const scope = nock('https://myprivatejira.test') + .get(`/rest/api/2/issue/${encodeURIComponent('secure-234')}`) + // This ensures that the expected credentials are actually being sent with the HTTP request. + // Without this the request wouldn't match and the test would fail. + .basicAuth({ user, pass }) + .reply(200, { fields: { status: { name: 'in progress' } } }) + + expect( + await JiraIssue.invoke(defaultContext, config, { + protocol: 'https', + hostAndPath: 'myprivatejira.test', + issueKey: 'secure-234', + }) + ).to.deep.equal({ + label: 'secure-234', + message: 'in progress', + color: 'lightgrey', + }) + + scope.done() + }) +}) diff --git a/services/jira/jira-issue.tester.js b/services/jira/jira-issue.tester.js index 1729b9d2d3..edb6aab527 100644 --- a/services/jira/jira-issue.tester.js +++ b/services/jira/jira-issue.tester.js @@ -1,13 +1,12 @@ 'use strict' const t = (module.exports = require('../tester').createServiceTester()) -const { mockJiraCreds, restore, user, pass } = require('./jira-test-helpers') -t.create('live: unknown issue') +t.create('unknown issue') .get('/https/issues.apache.org/jira/notArealIssue-000.json') .expectBadge({ label: 'jira', message: 'issue not found' }) -t.create('live: known issue') +t.create('known issue') .get('/https/issues.apache.org/jira/kafka-2896.json') .expectBadge({ label: 'kafka-2896', message: 'Resolved' }) @@ -161,26 +160,3 @@ t.create('blue-gray status color') message: 'cloudy', color: 'blue', }) - -t.create('with mock credentials') - .before(mockJiraCreds) - .get('/https/myprivatejira.com/secure-234.json') - .intercept(nock => - nock('https://myprivatejira.com/rest/api/2/issue') - .get(`/${encodeURIComponent('secure-234')}`) - // This ensures that the expected credentials from serverSecrets are actually being sent with the HTTP request. - // Without this the request wouldn't match and the test would fail. - .basicAuth({ - user, - pass, - }) - .reply(200, { - fields: { - status: { - name: 'in progress', - }, - }, - }) - ) - .finally(restore) - .expectBadge({ label: 'secure-234', message: 'in progress' }) diff --git a/services/jira/jira-sprint.service.js b/services/jira/jira-sprint.service.js index 7722477b44..f86b0d10a2 100644 --- a/services/jira/jira-sprint.service.js +++ b/services/jira/jira-sprint.service.js @@ -1,7 +1,8 @@ 'use strict' const Joi = require('@hapi/joi') -const JiraBase = require('./jira-base') +const { authConfig } = require('./jira-common') +const { BaseJsonService } = require('..') const schema = Joi.object({ total: Joi.number(), @@ -26,7 +27,11 @@ const documentation = `
` -module.exports = class JiraSprint extends JiraBase { +module.exports = class JiraSprint extends BaseJsonService { + static get category() { + return 'issue-tracking' + } + static get route() { return { base: 'jira/sprint', @@ -34,6 +39,10 @@ module.exports = class JiraSprint extends JiraBase { } } + static get auth() { + return authConfig + } + static get examples() { return [ { @@ -79,21 +88,23 @@ module.exports = class JiraSprint extends JiraBase { // Atlassian Documentation: https://developer.atlassian.com/cloud/jira/platform/rest/v2/#api-group-Search // There are other sprint-specific APIs but those require authentication. The search API // allows us to get the needed data without being forced to authenticate. - const url = `${protocol}://${hostAndPath}/rest/api/2/search` - const qs = { - jql: `sprint=${sprintId} AND type IN (Bug,Improvement,Story,"Technical task")`, - fields: 'resolution', - maxResults: 500, - } - const json = await this.fetch({ - url, + const json = await this._requestJson({ + url: `${protocol}://${hostAndPath}/rest/api/2/search`, schema, - qs, + options: { + qs: { + jql: `sprint=${sprintId} AND type IN (Bug,Improvement,Story,"Technical task")`, + fields: 'resolution', + maxResults: 500, + }, + auth: this.authHelper.basicAuth, + }, errorMessages: { 400: 'sprint not found', 404: 'sprint not found', }, }) + const numTotalIssues = json.total const numCompletedIssues = json.issues.filter(issue => { if (issue.fields.resolution != null) { diff --git a/services/jira/jira-sprint.spec.js b/services/jira/jira-sprint.spec.js new file mode 100644 index 0000000000..a0d9265316 --- /dev/null +++ b/services/jira/jira-sprint.spec.js @@ -0,0 +1,47 @@ +'use strict' + +const { expect } = require('chai') +const nock = require('nock') +const { cleanUpNockAfterEach, defaultContext } = require('../test-helpers') +const JiraSprint = require('./jira-sprint.service') +const { + user, + pass, + config, + sprintId, + sprintQueryString, +} = require('./jira-test-helpers') + +describe('JiraSprint', function() { + cleanUpNockAfterEach() + + it('sends the auth information as configured', async function() { + const scope = nock('https://myprivatejira.test') + .get('/jira/rest/api/2/search') + .query(sprintQueryString) + // This ensures that the expected credentials are actually being sent with the HTTP request. + // Without this the request wouldn't match and the test would fail. + .basicAuth({ user, pass }) + .reply(200, { + total: 2, + issues: [ + { fields: { resolution: { name: 'done' } } }, + { fields: { resolution: { name: 'Unresolved' } } }, + ], + }) + + expect( + await JiraSprint.invoke(defaultContext, config, { + protocol: 'https', + hostAndPath: 'myprivatejira.test/jira', + sprintId, + }) + ).to.deep.equal({ + label: 'completion', + message: '50%', + color: 'orange', + }) + + scope.done() + }) +}) diff --git a/services/jira/jira-sprint.tester.js b/services/jira/jira-sprint.tester.js index 7813afefe6..faa8e72c5f 100644 --- a/services/jira/jira-sprint.tester.js +++ b/services/jira/jira-sprint.tester.js @@ -2,20 +2,13 @@ const t = (module.exports = require('../tester').createServiceTester()) const { isIntegerPercentage } = require('../test-validators') -const { mockJiraCreds, restore, user, pass } = require('./jira-test-helpers') +const { sprintId, sprintQueryString } = require('./jira-test-helpers') -const sprintId = 8 -const queryString = { - jql: `sprint=${sprintId} AND type IN (Bug,Improvement,Story,"Technical task")`, - fields: 'resolution', - maxResults: 500, -} - -t.create('live: unknown sprint') +t.create('unknown sprint') .get('/https/jira.spring.io/abc.json') .expectBadge({ label: 'jira', message: 'sprint not found' }) -t.create('live: known sprint') +t.create('known sprint') .get('/https/jira.spring.io/94.json') .expectBadge({ label: 'completion', @@ -27,7 +20,7 @@ t.create('100% completion') .intercept(nock => nock('http://issues.apache.org/jira/rest/api/2') .get('/search') - .query(queryString) + .query(sprintQueryString) .reply(200, { total: 2, issues: [ @@ -59,7 +52,7 @@ t.create('0% completion') .intercept(nock => nock('http://issues.apache.org/jira/rest/api/2') .get('/search') - .query(queryString) + .query(sprintQueryString) .reply(200, { total: 1, issues: [ @@ -84,7 +77,7 @@ t.create('no issues in sprint') .intercept(nock => nock('http://issues.apache.org/jira/rest/api/2') .get('/search') - .query(queryString) + .query(sprintQueryString) .reply(200, { total: 0, issues: [], @@ -101,7 +94,7 @@ t.create('issue with null resolution value') .intercept(nock => nock('https://jira.spring.io:8080/rest/api/2') .get('/search') - .query(queryString) + .query(sprintQueryString) .reply(200, { total: 2, issues: [ @@ -125,39 +118,3 @@ t.create('issue with null resolution value') message: '50%', color: 'orange', }) - -t.create('with mock credentials') - .before(mockJiraCreds) - .get(`/https/myprivatejira/jira/${sprintId}.json`) - .intercept(nock => - nock('https://myprivatejira/jira/rest/api/2') - .get('/search') - .query(queryString) - // This ensures that the expected credentials from serverSecrets are actually being sent with the HTTP request. - // Without this the request wouldn't match and the test would fail. - .basicAuth({ - user, - pass, - }) - .reply(200, { - total: 2, - issues: [ - { - fields: { - resolution: { - name: 'done', - }, - }, - }, - { - fields: { - resolution: { - name: 'Unresolved', - }, - }, - }, - ], - }) - ) - .finally(restore) - .expectBadge({ label: 'completion', message: '50%' }) diff --git a/services/jira/jira-test-helpers.js b/services/jira/jira-test-helpers.js index 291f84dfae..ad31e8d228 100644 --- a/services/jira/jira-test-helpers.js +++ b/services/jira/jira-test-helpers.js @@ -1,25 +1,20 @@ 'use strict' -const sinon = require('sinon') -const serverSecrets = require('../../lib/server-secrets') +const sprintId = 8 +const sprintQueryString = { + jql: `sprint=${sprintId} AND type IN (Bug,Improvement,Story,"Technical task")`, + fields: 'resolution', + maxResults: 500, +} const user = 'admin' const pass = 'password' - -function mockJiraCreds() { - serverSecrets['jira_user'] = undefined - serverSecrets['jira_pass'] = undefined - sinon.stub(serverSecrets, 'jira_user').value(user) - sinon.stub(serverSecrets, 'jira_pass').value(pass) -} - -function restore() { - sinon.restore() -} +const config = { private: { jira_user: user, jira_pass: pass } } module.exports = { + sprintId, + sprintQueryString, user, pass, - mockJiraCreds, - restore, + config, } diff --git a/services/nexus/nexus.service.js b/services/nexus/nexus.service.js index 8f1c506b6e..9f540f5a44 100644 --- a/services/nexus/nexus.service.js +++ b/services/nexus/nexus.service.js @@ -3,7 +3,6 @@ const Joi = require('@hapi/joi') const { version: versionColor } = require('../color-formatters') const { addv } = require('../text-formatters') -const serverSecrets = require('../../lib/server-secrets') const { optionalDottedVersionNClausesWithOptionalSuffix, } = require('../validators') @@ -49,6 +48,10 @@ module.exports = class Nexus extends BaseJsonService { } } + static get auth() { + return { userKey: 'nexus_user', passKey: 'nexus_pass' } + } + static get examples() { return [ { @@ -167,19 +170,10 @@ module.exports = class Nexus extends BaseJsonService { this.addQueryParamsToQueryString({ qs, queryOpt }) } - const options = { qs } - - if (serverSecrets.nexus_user) { - options.auth = { - user: serverSecrets.nexus_user, - pass: serverSecrets.nexus_pass, - } - } - const json = await this._requestJson({ schema, url, - options, + options: { qs, auth: this.authHelper.basicAuth }, errorMessages: { 404: 'artifact not found', }, diff --git a/services/nexus/nexus.spec.js b/services/nexus/nexus.spec.js index 92e5c91382..1cca34fed0 100644 --- a/services/nexus/nexus.spec.js +++ b/services/nexus/nexus.spec.js @@ -1,6 +1,8 @@ 'use strict' const { expect } = require('chai') +const nock = require('nock') +const { cleanUpNockAfterEach, defaultContext } = require('../test-helpers') const Nexus = require('./nexus.service') const { InvalidResponse, NotFound } = require('..') @@ -68,4 +70,37 @@ describe('Nexus', function() { } }) }) + + describe('auth', function() { + cleanUpNockAfterEach() + + const user = 'admin' + const pass = 'password' + const config = { private: { nexus_user: user, nexus_pass: pass } } + + it('sends the auth information as configured', async function() { + const scope = nock('https://repository.jboss.org/nexus') + .get('/service/local/lucene/search') + .query({ g: 'jboss', a: 'jboss-client' }) + // This ensures that the expected credentials are actually being sent with the HTTP request. + // Without this the request wouldn't match and the test would fail. + .basicAuth({ user, pass }) + .reply(200, { data: [{ latestRelease: '2.3.4' }] }) + + expect( + await Nexus.invoke(defaultContext, config, { + repo: 'r', + scheme: 'https', + hostAndPath: 'repository.jboss.org/nexus', + groupId: 'jboss', + artifactId: 'jboss-client', + }) + ).to.deep.equal({ + message: 'v2.3.4', + color: 'blue', + }) + + scope.done() + }) + }) }) diff --git a/services/nexus/nexus.tester.js b/services/nexus/nexus.tester.js index 3b4ea29a93..5d9df348ad 100644 --- a/services/nexus/nexus.tester.js +++ b/services/nexus/nexus.tester.js @@ -1,23 +1,11 @@ 'use strict' -const sinon = require('sinon') const { isVPlusDottedVersionNClausesWithOptionalSuffix: isVersion, } = require('../test-validators') const t = (module.exports = require('../tester').createServiceTester()) -const serverSecrets = require('../../lib/server-secrets') -const user = 'admin' -const pass = 'password' - -function mockNexusCreds() { - serverSecrets['nexus_user'] = undefined - serverSecrets['nexus_pass'] = undefined - sinon.stub(serverSecrets, 'nexus_user').value(user) - sinon.stub(serverSecrets, 'nexus_pass').value(pass) -} - -t.create('live: search release version valid artifact') +t.create('search release version valid artifact') .timeout(15000) .get('/r/https/oss.sonatype.org/com.google.guava/guava.json') .expectBadge({ @@ -25,7 +13,7 @@ t.create('live: search release version valid artifact') message: isVersion, }) -t.create('live: search release version of an nonexistent artifact') +t.create('search release version of an nonexistent artifact') .timeout(10000) .get( '/r/https/oss.sonatype.org/com.google.guava/nonexistent-artifact-id.json' @@ -35,7 +23,7 @@ t.create('live: search release version of an nonexistent artifact') message: 'artifact or version not found', }) -t.create('live: search snapshot version valid snapshot artifact') +t.create('search snapshot version valid snapshot artifact') .timeout(10000) .get('/s/https/oss.sonatype.org/com.google.guava/guava.json') .expectBadge({ @@ -43,7 +31,7 @@ t.create('live: search snapshot version valid snapshot artifact') message: isVersion, }) -t.create('live: search snapshot version of an nonexistent artifact') +t.create('search snapshot version of an nonexistent artifact') .timeout(10000) .get( '/s/https/oss.sonatype.org/com.google.guava/nonexistent-artifact-id.json' @@ -54,14 +42,14 @@ t.create('live: search snapshot version of an nonexistent artifact') color: 'red', }) -t.create('live: repository version') +t.create('repository version') .get('/developer/https/repository.jboss.org/nexus/ai.h2o/h2o-automl.json') .expectBadge({ label: 'nexus', message: isVersion, }) -t.create('live: repository version with query') +t.create('repository version with query') .get( '/fs-public-snapshots/https/repository.jboss.org/nexus/com.progress.fuse/fusehq:c=agent-apple-osx:p=tar.gz.json' ) @@ -70,7 +58,7 @@ t.create('live: repository version with query') message: isVersion, }) -t.create('live: repository version of an nonexistent artifact') +t.create('repository version of an nonexistent artifact') .get( '/developer/https/repository.jboss.org/nexus/jboss/nonexistent-artifact-id.json' ) @@ -208,25 +196,3 @@ t.create('user query params') message: 'v3.2.1', color: 'blue', }) - -t.create('auth') - .before(mockNexusCreds) - .get('/r/https/repository.jboss.org/nexus/jboss/jboss-client.json') - .intercept(nock => - nock('https://repository.jboss.org/nexus') - .get('/service/local/lucene/search') - .query({ g: 'jboss', a: 'jboss-client' }) - // This ensures that the expected credentials from serverSecrets are actually being sent with the HTTP request. - // Without this the request wouldn't match and the test would fail. - .basicAuth({ - user, - pass, - }) - .reply(200, { data: [{ latestRelease: '2.3.4' }] }) - ) - .finally(sinon.restore) - .expectBadge({ - label: 'nexus', - message: 'v2.3.4', - color: 'blue', - }) diff --git a/services/npm/npm-base.js b/services/npm/npm-base.js index 829eb1af4c..d70a307fdb 100644 --- a/services/npm/npm-base.js +++ b/services/npm/npm-base.js @@ -1,7 +1,6 @@ 'use strict' const Joi = require('@hapi/joi') -const serverSecrets = require('../../lib/server-secrets') const { optionalUrl } = require('../validators') const { isDependencyMap } = require('../package-json-helpers') const { BaseJsonService, InvalidResponse, NotFound } = require('..') @@ -38,6 +37,10 @@ const queryParamSchema = Joi.object({ // Abstract class for NPM badges which display data about the latest version // of a package. module.exports = class NpmBase extends BaseJsonService { + static get auth() { + return { passKey: 'npm_token' } + } + static buildRoute(base, { withTag } = {}) { if (withTag) { return { @@ -74,15 +77,16 @@ module.exports = class NpmBase extends BaseJsonService { } async _requestJson(data) { - // Use a custom Accept header because of this bug: - //