Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite and test Github auth logic, separating standard and search quota #1205

Merged
merged 94 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
94 commits
Select commit Hold shift + click to select a range
202b508
Rewrite and test Github auth logic
paulmelnikow Oct 25, 2017
a105e98
Clean lint
paulmelnikow Oct 25, 2017
054314a
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 1, 2017
e045dbf
Clean diff
paulmelnikow Nov 1, 2017
8617f46
Rm unused
paulmelnikow Nov 1, 2017
815ec67
Adjust limits
paulmelnikow Nov 1, 2017
224266d
Readability improvements
paulmelnikow Nov 1, 2017
d138ce0
Simplify reserveFraction
paulmelnikow Nov 1, 2017
188d389
Validate tokens
paulmelnikow Nov 1, 2017
d99d095
Add an integration test
paulmelnikow Nov 1, 2017
71ccb19
Renames + docs
paulmelnikow Nov 1, 2017
ee5dba1
TokenPersistence: Tests and fixes
paulmelnikow Nov 2, 2017
06047cd
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 2, 2017
7cc3ea6
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 2, 2017
8f587bd
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 10, 2017
139b8ad
Apply fix from #1244
paulmelnikow Nov 10, 2017
c49f6d9
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 30, 2017
afe0da5
Update package-lock
paulmelnikow Nov 30, 2017
d67e875
Apply fix from https://github.com/badges/shields/commit/59c06628740bc…
paulmelnikow Jul 26, 2018
b03df7d
constEq -> crypto.timingSafeEqual
paulmelnikow Jul 26, 2018
e744f3c
Incorporate work from https://github.com/badges/shields/commit/127b46…
paulmelnikow Jul 26, 2018
31f7e8c
Incorporate work from https://github.com/badges/shields/commit/1d313b…
paulmelnikow Jul 26, 2018
a654fb1
Simplify initialization
paulmelnikow Jul 26, 2018
798b46e
Apply changes from https://github.com/badges/shields/commit/9882a44e5…
paulmelnikow Jul 26, 2018
5addd68
Clean lint
paulmelnikow Jul 26, 2018
1f86e85
Fix some tests
paulmelnikow Jul 26, 2018
b949ecc
Merge remote-tracking branch 'origin/master' into github-token-pool-2
paulmelnikow Jul 26, 2018
58fbda3
Update package-lock
paulmelnikow Jul 26, 2018
df6f303
Fix syntax
paulmelnikow Jul 26, 2018
5bcf33c
Clean lint
paulmelnikow Jul 26, 2018
c68bf49
Rm old github-auth
paulmelnikow Jul 26, 2018
b024e47
Fix most tests
paulmelnikow Jul 26, 2018
f67d335
Clean lint, fix up tests
paulmelnikow Jul 26, 2018
71e19ca
Merge branch 'master' into github-token-pool-2
paulmelnikow Jul 26, 2018
bdd72af
TokenPersistence.stop -> async
paulmelnikow Jul 26, 2018
5e46181
More fixes
paulmelnikow Jul 26, 2018
b01e92b
Reset some formatting changes
paulmelnikow Jul 26, 2018
a65c4b0
Reset some formatting changes
paulmelnikow Jul 26, 2018
52c4e42
Fix
paulmelnikow Jul 26, 2018
4134d27
Consolidate TokenPool classes
paulmelnikow Jul 26, 2018
2f8efda
Update TokenProvider for interface parity
paulmelnikow Jul 26, 2018
f3119f8
Go full dependency injection
paulmelnikow Jul 26, 2018
9b152f1
Restore integration test script, lost in merge
paulmelnikow Jul 27, 2018
c849a0e
github-auth -> services/github/auth
paulmelnikow Jul 27, 2018
3d625b7
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 2, 2018
1966a03
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 7, 2018
030296b
update package-lock
paulmelnikow Aug 7, 2018
89dfce2
Updates from merge
paulmelnikow Aug 7, 2018
c44ef30
Merge commit 'ab051b3' into github-token-pool-2
paulmelnikow Aug 9, 2018
d8b3802
Merge commit '7a664ca' into github-token-pool-2
paulmelnikow Aug 9, 2018
9654883
Run prettier
paulmelnikow Aug 9, 2018
2a4f2bc
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 12, 2018
9b3d5c6
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 17, 2018
b92de26
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 19, 2018
d854927
Roughly merge changes in github-constellation
paulmelnikow Aug 19, 2018
367f202
Add missing init code
paulmelnikow Aug 19, 2018
0e2b471
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 29, 2018
0ee7eed
Merge branch 'master' into github-token-pool-2
paulmelnikow Aug 29, 2018
8d74c63
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 15, 2018
e035788
Update from master
paulmelnikow Nov 15, 2018
0342509
Clean diff
paulmelnikow Nov 15, 2018
baf33ac
Clean diff
paulmelnikow Nov 15, 2018
2ffc965
Clean diff, try that again
paulmelnikow Nov 15, 2018
d050f48
Rm dup test
paulmelnikow Nov 15, 2018
e334cc8
Clean diff
paulmelnikow Nov 27, 2018
35b8e44
WIP
paulmelnikow Nov 27, 2018
8933591
Merge branch 'master' into github-token-pool-2
paulmelnikow Nov 27, 2018
ad4e5ee
Fixes
paulmelnikow Nov 27, 2018
585332c
Rm obsolete
paulmelnikow Nov 27, 2018
eb372a9
Add test coverage for change to TokenPool
paulmelnikow Nov 27, 2018
4bcc876
Temp fix to integration tests
paulmelnikow Nov 27, 2018
19f398c
Fix other integration tests
paulmelnikow Nov 27, 2018
375d609
Set credentials for integration tests
paulmelnikow Nov 27, 2018
7ea08cc
Merge branch 'master' into github-token-pool-2
paulmelnikow Dec 2, 2018
a84dc89
Merge branch 'master' into github-token-pool-2
paulmelnikow Jan 4, 2019
2248429
Make emitter private
paulmelnikow Jan 4, 2019
96e5e82
Rm unnecessary conditional
paulmelnikow Jan 4, 2019
e37d076
Clean up
paulmelnikow Jan 4, 2019
b4d8dda
Fix token persistence tests
paulmelnikow Jan 4, 2019
d6258e4
Work / cleanup
paulmelnikow Jan 4, 2019
5f68f3a
Updates
paulmelnikow Jan 4, 2019
d187d10
Rm emitter, no longer needed
paulmelnikow Jan 4, 2019
76c2e80
Fixes
paulmelnikow Jan 4, 2019
4d8dd16
Clean diff
paulmelnikow Jan 4, 2019
6fda89e
Fix integ tests
paulmelnikow Jan 4, 2019
c6576fa
Add debug output for a flaky test
paulmelnikow Jan 5, 2019
cebe1c8
Merge branch 'master' into github-token-pool-2
paulmelnikow Jan 8, 2019
023953d
Merge branch 'master' into github-token-pool-2
paulmelnikow Jan 9, 2019
78773e0
Remove obsolete secret.json setup
paulmelnikow Jan 9, 2019
7320221
Mock creds using sinon
paulmelnikow Jan 9, 2019
ac4619c
Reset circle config
paulmelnikow Jan 9, 2019
fd63acc
Add test for add-token endpoint
paulmelnikow Jan 9, 2019
f3b9191
Merge branch 'master' into github-token-pool-2
paulmelnikow Jan 10, 2019
4dd8cd4
Merge branch 'master' into github-token-pool-2
paulmelnikow Jan 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Go full dependency injection
  • Loading branch information
paulmelnikow committed Jul 26, 2018
commit f3119f8421e561438325e4d493d454461ceec20f
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,25 @@

const { expect } = require('chai');
const { PoolingTokenProvider } = require('./token-provider');
const GithubProvider = require('./github-provider');
const GithubApiProvider = require('./github-api-provider');
const serverSecrets = require('../server-secrets');

describe('Github provider with token pool', function() {
const githubUri = process.env.GITHUB_URL || 'https://api.github.com';
const reserveFraction = 0.333;

let tokenProvider, githubProvider;
let tokenProvider, githubApiProvider;
before(function() {
tokenProvider = new PoolingTokenProvider();
tokenProvider.addToken(serverSecrets.gh_token);

githubProvider = new GithubProvider(githubUri, tokenProvider);
githubProvider.reserveFraction = reserveFraction;
githubApiProvider = new GithubApiProvider(githubUri, tokenProvider);
githubApiProvider.reserveFraction = reserveFraction;
});

const headers = [];
async function performOneRequest() {
const { res } = await githubProvider.requestAsPromise(
const { res } = await githubApiProvider.requestAsPromise(
require('request'),
'/repos/rust-lang/rust',
{}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';

class GithubProvider {
constructor(baseUri, tokenProvider) {
this.baseUri = baseUri;
class GithubApiProvider {
constructor(baseUrl, tokenProvider) {
this.baseUrl = baseUrl;
this.tokenProvider = tokenProvider;

// How much of a token's quota do we reserve for the user?
Expand Down Expand Up @@ -31,6 +31,8 @@ class GithubProvider {
// limit. Inject `request` so we can pass in `cachingRequest` from
// `request-handler.js`.
request(request, url, query, callback) {
const { baseUrl } = this;

let token;
try {
token = this.tokenForUri(url);
Expand All @@ -41,7 +43,7 @@ class GithubProvider {

const options = {
url,
baseUrl: this.baseUri,
baseUrl,
qs: query,
headers: {
'User-Agent': 'Shields.io',
Expand Down Expand Up @@ -75,4 +77,4 @@ class GithubProvider {
}
}

module.exports = GithubProvider;
module.exports = GithubApiProvider;
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const { expect } = require('chai');
const sinon = require('sinon');
const GithubProvider = require('./github-provider');
const GithubApiProvider = require('./github-api-provider');

describe('Github API provider', function() {
const baseUri = 'https://github-api.example.com';
Expand All @@ -16,7 +16,7 @@ describe('Github API provider', function() {
nextToken: sinon.stub().returns(mockToken),
nextSearchToken: sinon.stub().returns(mockSearchToken),
};
provider = new GithubProvider(baseUri, mockTokenProvider);
provider = new GithubApiProvider(baseUri, mockTokenProvider);
provider.reserveFraction = reserveFraction;
});

Expand Down
95 changes: 95 additions & 0 deletions lib/github-auth/github-constellation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
'use strict';

const path = require('path');
const serverSecrets = require('../server-secrets');
const log = require('../log');
const {
StaticTokenProvider,
PoolingTokenProvider,
} = require('./token-provider');
const TokenPersistence = require('./token-persistence');
const GithubApiProvider = require('./github-api-provider');
const { setRoutes: setAcceptorRoutes } = require('./acceptor');
const { setRoutes: setAdminRoutes } = require('./admin');

// Convenience class with all the stuff related to the Github API and its
// authorization tokens, to simplify server initialization.
class GithubConstellation {
constructor(config) {
this._debugEnabled = config.service.debug.enabled;
this._debugIntervalSeconds = config.service.debug.intervalSeconds;
this._userTokensPath = path.resolve(
config.persistence.dir,
'github-user-tokens.json'
);

const globalTokenString = (serverSecrets || {}).gh_token;

if (globalTokenString) {
// When a global gh_token is configured, use that in place of our token
// pool. This produces more predictable behavior, and more predictable
// failures when that token is exhausted.
this.usingPooling = false;
this.coreTokenProvider = this.searchTokenProvider = new StaticTokenProvider(
globalTokenString
);
} else {
this.usingPooling = true;
this.coreTokenProvider = new PoolingTokenProvider();
this.searchTokenProvider = new PoolingTokenProvider();
}

const githubUrl = process.env.GITHUB_URL || 'https://api.github.com';
this.apiProvider = new GithubApiProvider(githubUrl, this.coreTokenProvider);
}

scheduleDebugLogging() {
if (this._debugEnabled) {
this.debugInterval = setInterval(() => {
if (this.coreTokenProvider) {
log('coreTokenProvider', this.coreTokenProvider.getTokenDebugInfo());
}
if (this.searchTokenProvider) {
log(
'searchTokenProvider',
this.searchTokenProvider.getTokenDebugInfo()
);
}
}, 1000 * this._debugIntervalSeconds);
}
}

async initialize(server) {
this.scheduleDebugLogging();

if (this.usingPooling) {
this.persistence = new TokenPersistence(
this.coreTokenProvider,
this._userTokensPath
);
await this.persistence.initialize();
this.coreTokenProvider
.toNative()
.forEach(t => this.searchTokenProvider.addToken(t));
}
// TODO Catch errors and send them to Sentry.

setAdminRoutes(this.coreTokenProvider, server);

if (serverSecrets && serverSecrets.gh_client_id) {
setAcceptorRoutes(this.coreTokenProvider, server);
}
}

stop() {
if (this.persistence) {
this.persistence.stop();
}
if (this.debugInterval) {
clearInterval(this.debugInterval);
this.debugInterval = undefined;
}
}
}

module.exports = GithubConstellation;
83 changes: 0 additions & 83 deletions lib/github-auth/index.js

This file was deleted.

10 changes: 5 additions & 5 deletions lib/github-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const {
} = require('./color-formatters');

// GitHub commits since integration.
function mapGithubCommitsSince({ camp, cache }, githubProvider) {
function mapGithubCommitsSince({ camp, cache }, githubApiProvider) {
camp.route(/^\/github\/commits-since\/([^/]+)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function (data, match, sendBadge, request) {
const user = match[1]; // eg, SubtitleEdit
Expand All @@ -29,7 +29,7 @@ function mapGithubCommitsSince({ camp, cache }, githubProvider) {
if (badgeData.template === 'social') {
badgeData.logo = getLogo('github', data);
}
githubProvider.request(request, apiUrl, {}, (err, res, buffer) => {
githubApiProvider.request(request, apiUrl, {}, (err, res, buffer) => {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
Expand All @@ -52,7 +52,7 @@ function mapGithubCommitsSince({ camp, cache }, githubProvider) {

if (version === 'latest') {
const url = `/repos/${user}/${repo}/releases/latest`;
githubProvider.request(request, url, {}, (err, res, buffer) => {
githubApiProvider.request(request, url, {}, (err, res, buffer) => {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
Expand All @@ -73,7 +73,7 @@ function mapGithubCommitsSince({ camp, cache }, githubProvider) {
}

//Github Release & Pre-Release Date Integration release-date-pre (?:\/(all))?
function mapGithubReleaseDate({ camp, cache }, githubProvider) {
function mapGithubReleaseDate({ camp, cache }, githubApiProvider) {
camp.route(/^\/github\/(release-date|release-date-pre)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function (data, match, sendBadge, request) {
const releaseType = match[1]; // eg, release-date-pre / release-date
Expand All @@ -88,7 +88,7 @@ function mapGithubReleaseDate({ camp, cache }, githubProvider) {
if (badgeData.template === 'social') {
badgeData.logo = getLogo('github', data);
}
githubProvider.request(request, apiUrl, {}, (err, res, buffer) => {
githubApiProvider.request(request, apiUrl, {}, (err, res, buffer) => {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
Expand Down
17 changes: 8 additions & 9 deletions lib/suggest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@

const nodeUrl = require('url');
const request = require('request');
const { defaultProvider: githubProvider } = require('./github-auth');

// data: {url}, JSON-serializable object.
// end: function(json), with json of the form:
// - badges: list of objects of the form:
// - link: target as a string URL.
// - badge: shields image URL.
// - name: string
function suggest (allowedOrigin, data, end, ask) {
function suggest (allowedOrigin, githubApiProvider, data, end, ask) {
// The typical dev and production setups are cross-origin. However, in
// Heroku deploys and some self-hosted deploys these requests may come from
// the same host.
Expand All @@ -29,12 +28,12 @@ function suggest (allowedOrigin, data, end, ask) {
try {
url = nodeUrl.parse(data.url);
} catch(e) { end({err: ''+e}); return; }
findSuggestions(url, end);
findSuggestions(githubApiProvider, url, end);
}

// url: string
// cb: function({badges})
function findSuggestions (url, cb) {
function findSuggestions (githubApiProvider, url, cb) {
let promises = [];
if (url.hostname === 'github.com') {
const userRepo = url.pathname.slice(1).split('/');
Expand Down Expand Up @@ -96,10 +95,10 @@ function githubStars (user, repo) {
});
}

function githubLicense (user, repo) {
function githubLicense (githubApiProvider, user, repo) {
return new Promise((resolve) => {
const apiUrl = `/repos/${user}/${repo}/license`;
githubProvider.request(request, apiUrl, {}, function(err, res, buffer) {
githubApiProvider.request(request, apiUrl, {}, function(err, res, buffer) {
if (err !== null) {
resolve(null);
return;
Expand Down Expand Up @@ -127,10 +126,10 @@ function githubLicense (user, repo) {
});
}

function setRoutes (allowedOrigin, camp) {
camp.ajax.on(
function setRoutes (allowedOrigin, githubApiProvider, server) {
server.ajax.on(
'suggest/v1',
(data, end, ask) => suggest(allowedOrigin, data, end, ask));
(data, end, ask) => suggest(allowedOrigin, githubApiProvider, data, end, ask));
}

module.exports = {
Expand Down
Loading