Add GitHub signup oracle #41

Merged
bumi merged 13 commits from feature/github-signup into master 2019-08-28 13:56:54 +00:00
Showing only changes of commit 137e9eb4ed - Show all commits

View File

@@ -1,5 +1,6 @@
const util = require('util');
const fetch = require('node-fetch');
const grant = require('grant-express');
const amountFromLabels = require('./utils/amount-from-labels');
const kindFromLabels = require('./utils/kind-from-labels');
@@ -172,4 +173,72 @@ module.exports = async function(robot, kredits) {
}
});
raucao commented 2019-07-26 09:50:32 +00:00 (Migrated from github.com)
Review

I think we need to define the signup paths here, if we don't want to apply these things to all hubot routes.

I think we need to define the signup paths here, if we don't want to apply these things to all hubot routes.
raucao commented 2019-07-26 09:51:36 +00:00 (Migrated from github.com)
Review

This function could easily go in its own file.

This function could easily go in its own file.
raucao commented 2019-07-26 09:53:25 +00:00 (Migrated from github.com)
Review

I think default options should be set at the top of the file instead of directly in a router function.

I think default options should be set at the top of the file instead of directly in a router function.
raucao commented 2019-07-26 09:54:21 +00:00 (Migrated from github.com)
Review

Why JSON? Shouldn't sendStatus(200) be enough?

Why JSON? Shouldn't `sendStatus(200)` be enough?
galfert commented 2019-07-26 17:16:06 +00:00 (Migrated from github.com)
Review

I used status(200) without the JSON, but that didn't work. I'll give sendStatus(200) a try.

I used `status(200)` without the JSON, but that didn't work. I'll give `sendStatus(200)` a try.
raucao commented 2019-07-26 17:32:02 +00:00 (Migrated from github.com)
Review

Yeah, status only sets the status, but doesn't send a response by itself.

Yeah, `status` only sets the status, but doesn't send a response by itself.
galfert commented 2019-07-26 18:56:23 +00:00 (Migrated from github.com)
Review

Found a better solution using a middleware. With that it was easy to just add the CORS headers to the routes that need it.

Found a better solution using a middleware. With that it was easy to just add the CORS headers to the routes that need it.
galfert commented 2019-07-26 18:57:40 +00:00 (Migrated from github.com)
Review

Done

Done
if (process.env.GITHUB_KEY && process.env.GITHUB_SECRET) {
const grantConfig = {
defaults: {
protocol: (process.env.GRANT_PROTOCOL || "http"),
host: (process.env.GRANT_HOST || 'localhost:3000')
},
github: {
key: process.env.GITHUB_KEY,
secret: process.env.GITHUB_SECRET,
scope: ['user', 'public_repo'],
callback: 'https://kredits.kosmos.org/signup/github'
}
};
robot.router.use('/kredits/signup/github/', grant(grantConfig));
robot.router.post('/kredits/signup/github', async (req, res) => {
const accessToken = req.body.accessToken;
if (!accessToken) {
res.status(400).json({});
return;
}
const githubResponse = await fetch('https://api.github.com/user', {
headers: {
'Accept': 'application/vnd.github.v3+json',
'Authorization': `token ${accessToken}`
}
});
if (githubResponse.status >= 300) {
res.sendStatus(githubResponse.status);
return;
}
const user = await githubResponse.json();
const contributor = await kredits.Contributor.findByAccount({
site: 'github.com',
username: user.login
});
if (!contributor) {
let contributorAttr = {};
contributorAttr.account = req.body.account;
contributorAttr.name = user.name;
contributorAttr.kind = "person";
contributorAttr.url = user.blog;
bumi commented 2019-07-26 09:09:14 +00:00 (Migrated from github.com)
Review

why don't you directly reditect to kredits-web? - using the grant callback option?

why don't you directly reditect to kredits-web? - using the grant callback option?
galfert commented 2019-07-26 09:15:03 +00:00 (Migrated from github.com)
Review

That first redirect should go to the backend server, because that one only contains a (temporary) code that you can then use together with the client secret to exchange it for the actual access token. But you shouldn't put the client secret in a client-side web app.

That first redirect should go to the backend server, because that one only contains a (temporary) code that you can then use together with the client secret to exchange it for the actual access token. But you shouldn't put the client secret in a client-side web app.
galfert commented 2019-07-26 09:17:22 +00:00 (Migrated from github.com)
Review

Also, the endpoint to retrieve the access token using the code and client secret (POST https://github.com/login/oauth/access_token) doesn't have CORS headers, so requests from Web apps wouldn't work.

Also, the endpoint to retrieve the access token using the code and client secret (POST https://github.com/login/oauth/access_token) doesn't have CORS headers, so requests from Web apps wouldn't work.
contributorAttr.github_username = user.login;
contributorAttr.github_uid = user.id;
kredits.Contributor.add(contributorAttr)
.then(transaction => {
robot.logger.info('Contributor added', transaction.hash);
res.json({
transactionHash: transaction.hash,
github_username: user.login
});
});
} else {
res.json({
github_username: user.login
});
}
});
} else {
robot.logger.warn('No GITHUB_KEY and GITHUB_SECRET configured for oauth signup');
}
};