From c5186cc501ba4a3a436f08087bfc79a234a09d6c Mon Sep 17 00:00:00 2001 From: Martii Date: Sat, 17 Apr 2021 00:54:10 -0600 Subject: [PATCH] Correct console messages, comments, and some bugs * 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 #1705 #37 and post #1729 --- libs/githubClient.js | 32 +++++++++++++++++++++----------- libs/repoManager.js | 32 +++++++++++++++++--------------- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/libs/githubClient.js b/libs/githubClient.js index 4ed5ad638..4527648b2 100644 --- a/libs/githubClient.js +++ b/libs/githubClient.js @@ -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, @@ -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({ @@ -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'))); } }); diff --git a/libs/repoManager.js b/libs/repoManager.js index d5d3c7923..65ddbcc9c 100644 --- a/libs/repoManager.js +++ b/libs/repoManager.js @@ -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 @@ -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);