Skip to content

Commit

Permalink
Correct console messages, comments, and some bugs (OpenUserJS#1800)
Browse files Browse the repository at this point in the history
* Authentication confirmation doesn't happen at all on object construction. This routine is instead used to store the values for later usage. A call to a URL will show if there is a higher rate limit. In the newer dep `authorization` is calculated as well.
* Clean up and add existence checks in repoManager. Don't necessarily have to terminate the app if API Keys for GH aren't there... should just use lower rate limit... although other features might depend on those values. Appears to be part of the webhook but not confirmed.

Applies to OpenUserJS#1705 OpenUserJS#37  and post OpenUserJS#1729

Auto-merge
  • Loading branch information
Martii authored Apr 17, 2021
1 parent 66c9082 commit a303691
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 26 deletions.
32 changes: 21 additions & 11 deletions libs/githubClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ Strategy.findOne({ name: 'github' }, async function (aErr, aStrat) {
console.error(aErr);

if (aStrat && process.env.DISABLE_SCRIPT_IMPORT !== 'true') {
// This authentication authorization is currently required to authorize this app
// to have the GitHub authentication callback work when the strategy `id` and `key` is found
// and additional usage of the `id` and `key` elsewhere in the Code

// TODO: Incomplete migration here
auth = createOAuthAppAuth({
clientType: 'oauth-app',
clientId: aStrat.id,
Expand All @@ -48,6 +46,7 @@ Strategy.findOne({ name: 'github' }, async function (aErr, aStrat) {

// TODO: Do something with `appAuthentication`


// DEPRECATED: This method will break on May 5th, 2021. See #1705
// and importing a repo will be severely hindered with possible timeouts/failures
github.authenticate({
Expand All @@ -56,15 +55,26 @@ Strategy.findOne({ name: 'github' }, async function (aErr, aStrat) {
secret: aStrat.key
});

// TODO: error handler for UnhandledPromiseRejectionWarning if it crops up after deprecation

if (github.auth) {
console.log(colors.green('GitHub client (a.k.a this app) is authenticated'));
} else {
console.log(colors.yellow('GitHub client (a.k.a this app) is partially authenticated'));
}
// TODO: error handler for UnhandledPromiseRejectionWarning if it crops up after deprecation.
// Forced invalid credentials and no error thrown but doesn't mean that they won't appear.

if (github.auth) {
console.log(colors.green([
'GitHub client (a.k.a this app) DOES contain authentication credentials.',
'Higher rate limit may be available'
].join('\n')));
}
else {
console.log(colors.red([
'GitHub client (a.k.a this app) DOES NOT contain authentication credentials.',
'Critical error with dependency.'
].join('\n')));
}
} else {
console.warn(colors.red('GitHub client NOT authenticated. Will have a lower Rate Limit.'));
console.warn(colors.yellow([
'GitHub client (a.k.a this app) DOES NOT contain authentication credentials.',
'Lower rate limit will be available.'
].join('\n')));
}

});
Expand Down
32 changes: 17 additions & 15 deletions libs/repoManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,14 @@ Strategy.findOne({ name: 'github' }, function (aErr, aStrat) {
}

if (!aStrat) {
console.error( colors.red( [
'Default GitHub Strategy document not found in DB',
'Terminating app'
console.warn( colors.red([
'Default GitHub Strategy document not found in DB.',
'Lower rate limit will be available.'
].join('\n')));

process.exit(1);
return;
} else {
clientId = aStrat.id;
clientKey = aStrat.key;
}

clientId = aStrat.id;
clientKey = aStrat.key;
});

// Requests a GitHub url and returns the chunks as buffers
Expand Down Expand Up @@ -100,15 +97,20 @@ function fetchRaw(aHost, aPath, aCallback, aOptions) {
// Use for call the GitHub JSON api
// Returns the JSON parsed object
function fetchJSON(aPath, aCallback) {
var encodedAuth = null;
var opts = null;

// The old authentication method, which GitHub deprecated
//aPath += '?client_id=' + clientId + '&client_secret=' + clientKey;
// We must now use OAuth Basic (user+key) or Bearer (token)
var encodedAuth = Buffer.from(`${clientId}:${clientKey}`).toString('base64');
var opts = {
headers: {
authorization: `basic ${encodedAuth}`
}
};
if (clientId && clientKey) {
encodedAuth = Buffer.from(`${clientId}:${clientKey}`).toString('base64');
opts = {
headers: {
authorization: `basic ${encodedAuth}`
}
};
}
fetchRaw('api.github.com', aPath, function (aBufs) {
aCallback(JSON.parse(Buffer.concat(aBufs).toString()));
}, opts);
Expand Down

0 comments on commit a303691

Please sign in to comment.