Skip to content

Commit c5186cc

Browse files
committed
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 OpenUserJS#1705 OpenUserJS#37 and post OpenUserJS#1729
1 parent 66c9082 commit c5186cc

File tree

2 files changed

+38
-26
lines changed

2 files changed

+38
-26
lines changed

libs/githubClient.js

+21-11
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ Strategy.findOne({ name: 'github' }, async function (aErr, aStrat) {
3232
console.error(aErr);
3333

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

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

4947
// TODO: Do something with `appAuthentication`
5048

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

59-
// TODO: error handler for UnhandledPromiseRejectionWarning if it crops up after deprecation
60-
61-
if (github.auth) {
62-
console.log(colors.green('GitHub client (a.k.a this app) is authenticated'));
63-
} else {
64-
console.log(colors.yellow('GitHub client (a.k.a this app) is partially authenticated'));
65-
}
58+
// TODO: error handler for UnhandledPromiseRejectionWarning if it crops up after deprecation.
59+
// Forced invalid credentials and no error thrown but doesn't mean that they won't appear.
60+
61+
if (github.auth) {
62+
console.log(colors.green([
63+
'GitHub client (a.k.a this app) DOES contain authentication credentials.',
64+
'Higher rate limit may be available'
65+
].join('\n')));
66+
}
67+
else {
68+
console.log(colors.red([
69+
'GitHub client (a.k.a this app) DOES NOT contain authentication credentials.',
70+
'Critical error with dependency.'
71+
].join('\n')));
72+
}
6673
} else {
67-
console.warn(colors.red('GitHub client NOT authenticated. Will have a lower Rate Limit.'));
74+
console.warn(colors.yellow([
75+
'GitHub client (a.k.a this app) DOES NOT contain authentication credentials.',
76+
'Lower rate limit will be available.'
77+
].join('\n')));
6878
}
6979

7080
});

libs/repoManager.js

+17-15
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,14 @@ Strategy.findOne({ name: 'github' }, function (aErr, aStrat) {
4040
}
4141

4242
if (!aStrat) {
43-
console.error( colors.red( [
44-
'Default GitHub Strategy document not found in DB',
45-
'Terminating app'
43+
console.warn( colors.red([
44+
'Default GitHub Strategy document not found in DB.',
45+
'Lower rate limit will be available.'
4646
].join('\n')));
47-
48-
process.exit(1);
49-
return;
47+
} else {
48+
clientId = aStrat.id;
49+
clientKey = aStrat.key;
5050
}
51-
52-
clientId = aStrat.id;
53-
clientKey = aStrat.key;
5451
});
5552

5653
// Requests a GitHub url and returns the chunks as buffers
@@ -100,15 +97,20 @@ function fetchRaw(aHost, aPath, aCallback, aOptions) {
10097
// Use for call the GitHub JSON api
10198
// Returns the JSON parsed object
10299
function fetchJSON(aPath, aCallback) {
100+
var encodedAuth = null;
101+
var opts = null;
102+
103103
// The old authentication method, which GitHub deprecated
104104
//aPath += '?client_id=' + clientId + '&client_secret=' + clientKey;
105105
// We must now use OAuth Basic (user+key) or Bearer (token)
106-
var encodedAuth = Buffer.from(`${clientId}:${clientKey}`).toString('base64');
107-
var opts = {
108-
headers: {
109-
authorization: `basic ${encodedAuth}`
110-
}
111-
};
106+
if (clientId && clientKey) {
107+
encodedAuth = Buffer.from(`${clientId}:${clientKey}`).toString('base64');
108+
opts = {
109+
headers: {
110+
authorization: `basic ${encodedAuth}`
111+
}
112+
};
113+
}
112114
fetchRaw('api.github.com', aPath, function (aBufs) {
113115
aCallback(JSON.parse(Buffer.concat(aBufs).toString()));
114116
}, opts);

0 commit comments

Comments
 (0)