From 8ba6ad1ec6455bf3f5f5f55bff00f08fe5b245a9 Mon Sep 17 00:00:00 2001 From: chris48s Date: Mon, 31 Jul 2023 10:48:22 +0100 Subject: [PATCH 1/2] allow calling [github] without auth --- .../github/github-api-provider.integration.js | 34 ++++++++++++++++--- services/github/github-api-provider.js | 28 ++++++++++----- services/github/github-auth-service.js | 9 ++++- services/github/github-constellation.js | 15 ++++++-- services/github/github-search.service.js | 8 +++-- 5 files changed, 76 insertions(+), 18 deletions(-) diff --git a/services/github/github-api-provider.integration.js b/services/github/github-api-provider.integration.js index 4e5832c90ef06..ce02069d2af72 100644 --- a/services/github/github-api-provider.integration.js +++ b/services/github/github-api-provider.integration.js @@ -17,11 +17,32 @@ describe('Github API provider', function () { let githubApiProvider - context('without token pool', function () { + context('with no auth', function () { before(function () { githubApiProvider = new GithubApiProvider({ baseUrl, - withPooling: false, + authType: GithubApiProvider.AUTH_TYPES.NO_AUTH, + }) + }) + + it('should be able to run 10 requests', async function () { + this.timeout('20s') + for (let i = 0; i < 10; ++i) { + const { res } = await githubApiProvider.fetch( + fetch, + '/repos/rust-lang/rust', + {}, + ) + expect(res.statusCode).to.equal(200) + } + }) + }) + + context('with global token', function () { + before(function () { + githubApiProvider = new GithubApiProvider({ + baseUrl, + authType: GithubApiProvider.AUTH_TYPES.GLOBAL_TOKEN, globalToken: token, reserveFraction, }) @@ -30,7 +51,12 @@ describe('Github API provider', function () { it('should be able to run 10 requests', async function () { this.timeout('20s') for (let i = 0; i < 10; ++i) { - await githubApiProvider.fetch(fetch, '/repos/rust-lang/rust', {}) + const { res } = await githubApiProvider.fetch( + fetch, + '/repos/rust-lang/rust', + {}, + ) + expect(res.statusCode).to.equal(200) } }) }) @@ -40,7 +66,7 @@ describe('Github API provider', function () { before(function () { githubApiProvider = new GithubApiProvider({ baseUrl, - withPooling: true, + authType: GithubApiProvider.AUTH_TYPES.TOKEN_POOL, reserveFraction, }) githubApiProvider.addToken(token) diff --git a/services/github/github-api-provider.js b/services/github/github-api-provider.js index a37513761b1a9..520daa6533e56 100644 --- a/services/github/github-api-provider.js +++ b/services/github/github-api-provider.js @@ -33,11 +33,17 @@ const bodySchema = Joi.object({ // Provides an interface to the Github API. Manages the base URL. class GithubApiProvider { + static AUTH_TYPES = { + NO_AUTH: 'No Auth', + GLOBAL_TOKEN: 'Global Token', + TOKEN_POOL: 'Token Pool', + } + // reserveFraction: The amount of much of a token's quota we avoid using, to // reserve it for the user. constructor({ baseUrl, - withPooling = true, + authType, onTokenInvalidated = tokenString => {}, globalToken, reserveFraction = 0.25, @@ -45,13 +51,13 @@ class GithubApiProvider { }) { Object.assign(this, { baseUrl, - withPooling, + authType, onTokenInvalidated, globalToken, reserveFraction, }) - if (this.withPooling) { + if (this.authType === this.constructor.AUTH_TYPES.TOKEN_POOL) { this.standardTokens = new TokenPool({ batchSize: 25 }) this.searchTokens = new TokenPool({ batchSize: 5 }) this.graphqlTokens = new TokenPool({ batchSize: 25 }) @@ -60,7 +66,7 @@ class GithubApiProvider { } addToken(tokenString) { - if (this.withPooling) { + if (this.authType === this.constructor.AUTH_TYPES.TOKEN_POOL) { this.standardTokens.add(tokenString) this.searchTokens.add(tokenString) this.graphqlTokens.add(tokenString) @@ -157,7 +163,7 @@ class GithubApiProvider { let token let tokenString - if (this.withPooling) { + if (this.authType === this.constructor.AUTH_TYPES.TOKEN_POOL) { try { token = this.tokenForUrl(url) } catch (e) { @@ -167,7 +173,7 @@ class GithubApiProvider { }) } tokenString = token.id - } else { + } else if (this.authType === this.constructor.AUTH_TYPES.GLOBAL_TOKEN) { tokenString = this.globalToken } @@ -176,14 +182,20 @@ class GithubApiProvider { ...{ headers: { 'User-Agent': userAgent, - Authorization: `token ${tokenString}`, 'X-GitHub-Api-Version': this.restApiVersion, ...options.headers, }, }, } + if ( + this.authType === this.constructor.AUTH_TYPES.TOKEN_POOL || + this.authType === this.constructor.AUTH_TYPES.GLOBAL_TOKEN + ) { + mergedOptions.headers.Authorization = `token ${tokenString}` + } + const response = await requestFetcher(`${baseUrl}${url}`, mergedOptions) - if (this.withPooling) { + if (this.authType === this.constructor.AUTH_TYPES.TOKEN_POOL) { if (response.res.statusCode === 401) { this.invalidateToken(token) } else if (response.res.statusCode < 500) { diff --git a/services/github/github-auth-service.js b/services/github/github-auth-service.js index b26bec191cb67..67f249f02c9e5 100644 --- a/services/github/github-auth-service.js +++ b/services/github/github-auth-service.js @@ -63,7 +63,14 @@ class GithubAuthV4Service extends BaseGraphqlService { `, ) - return super._requestGraphql({ ...attrs, ...{ url, query } }) + return super._requestGraphql({ + ...attrs, + ...{ + url, + query, + httpErrorMessages: { 401: 'auth required for graphql api' }, + }, + }) } } diff --git a/services/github/github-constellation.js b/services/github/github-constellation.js index 75754b6c2bcd0..cafdb52d0d18c 100644 --- a/services/github/github-constellation.js +++ b/services/github/github-constellation.js @@ -23,19 +23,28 @@ class GithubConstellation { this._debugEnabled = config.service.debug.enabled this._debugIntervalSeconds = config.service.debug.intervalSeconds + let authType = GithubApiProvider.AUTH_TYPES.NO_AUTH + const { postgres_url: pgUrl, gh_token: globalToken } = config.private if (pgUrl) { - log.log('Token persistence configured with dbUrl') + log.log('Github Token persistence configured with pgUrl') this.persistence = new SqlTokenPersistence({ url: pgUrl, table: 'github_user_tokens', }) + authType = GithubApiProvider.AUTH_TYPES.TOKEN_POOL + } + + if (globalToken) { + authType = GithubApiProvider.AUTH_TYPES.GLOBAL_TOKEN } + log.log(`Github using auth type: ${authType}`) + this.apiProvider = new GithubApiProvider({ baseUrl: config.service.baseUri, globalToken, - withPooling: !globalToken, + authType, onTokenInvalidated: tokenString => this.onTokenInvalidated(tokenString), restApiVersion: config.service.restApiVersion, }) @@ -52,7 +61,7 @@ class GithubConstellation { } async initialize(server) { - if (!this.apiProvider.withPooling) { + if (this.apiProvider.authType !== GithubApiProvider.AUTH_TYPES.TOKEN_POOL) { return } diff --git a/services/github/github-search.service.js b/services/github/github-search.service.js index 69ace99dd0309..cd50e6abdb468 100644 --- a/services/github/github-search.service.js +++ b/services/github/github-search.service.js @@ -2,7 +2,7 @@ import Joi from 'joi' import { metric } from '../text-formatters.js' import { nonNegativeInteger } from '../validators.js' import { GithubAuthV3Service } from './github-auth-service.js' -import { httpErrorsFor, documentation } from './github-helpers.js' +import { documentation } from './github-helpers.js' const schema = Joi.object({ total_count: nonNegativeInteger }).required() @@ -49,7 +49,11 @@ export default class GithubSearch extends GithubAuthV3Service { }, }, schema, - httpErrors: httpErrorsFor('repo not found'), + httpErrors: { + 401: 'auth required for search api', + 404: 'repo not found', + 422: 'repo not found', + }, }) return this.constructor.render({ query, totalCount }) } From 80857c535f076ad17666ecf0675bbadfc6cbe873 Mon Sep 17 00:00:00 2001 From: chris48s Date: Mon, 31 Jul 2023 11:14:14 +0100 Subject: [PATCH 2/2] set a default for authType, fix broken tests --- services/github/github-api-provider.js | 2 +- services/github/github-api-provider.spec.js | 6 +++++- services/github/github-auth-service.spec.js | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/services/github/github-api-provider.js b/services/github/github-api-provider.js index 520daa6533e56..4adf5d5ab29d7 100644 --- a/services/github/github-api-provider.js +++ b/services/github/github-api-provider.js @@ -43,7 +43,7 @@ class GithubApiProvider { // reserve it for the user. constructor({ baseUrl, - authType, + authType = this.constructor.AUTH_TYPES.NO_AUTH, onTokenInvalidated = tokenString => {}, globalToken, reserveFraction = 0.25, diff --git a/services/github/github-api-provider.spec.js b/services/github/github-api-provider.spec.js index ae9b9fdaff06b..31f4dcb5187fa 100644 --- a/services/github/github-api-provider.spec.js +++ b/services/github/github-api-provider.spec.js @@ -8,7 +8,11 @@ describe('Github API provider', function () { let mockStandardToken, mockSearchToken, mockGraphqlToken, provider beforeEach(function () { - provider = new GithubApiProvider({ baseUrl, reserveFraction }) + provider = new GithubApiProvider({ + baseUrl, + authType: GithubApiProvider.AUTH_TYPES.TOKEN_POOL, + reserveFraction, + }) mockStandardToken = { update: sinon.spy(), invalidate: sinon.spy() } sinon.stub(provider.standardTokens, 'next').returns(mockStandardToken) diff --git a/services/github/github-auth-service.spec.js b/services/github/github-auth-service.spec.js index 378d5034f6cf6..d75df3f9db6d4 100644 --- a/services/github/github-auth-service.spec.js +++ b/services/github/github-auth-service.spec.js @@ -41,6 +41,7 @@ describe('GithubAuthV3Service', function () { ) const githubApiProvider = new GithubApiProvider({ baseUrl: 'https://github-api.example.com', + authType: GithubApiProvider.AUTH_TYPES.TOKEN_POOL, restApiVersion: '2022-11-28', }) const mockToken = { update: sinon.mock(), invalidate: sinon.mock() }