Add GitHub signup oracle #41

Merged
bumi merged 13 commits from feature/github-signup into master 2019-08-28 13:56:54 +00:00
4 changed files with 955 additions and 245 deletions

View File

@ -24,6 +24,9 @@ As usual in Hubot, you can add all config as environment variables.
| `KREDITS_WALLET_PATH` | Path to an Etherum wallet JSON file (default: `./wallet.json`) |
| `KREDITS_WALLET_PASSWORD` | Wallet password |
| `KREDITS_PROVIDER_URL` | Ethereum JSON-RPC URL (default: `http://localhost:7545`) |
| `KREDITS_WEB_URL` | URL of the Kredits Web app (default: `https://kredits.kosmos.org`) |
| `KREDITS_DAO_ADDRESS` | DAO Kernel address |
| `SESSION_SECRET` | Secret used to sign the Session ID |
## Integrations

View File

@ -1,5 +1,8 @@
const util = require('util');
const fetch = require('node-fetch');
const session = require('express-session');
const grant = require('grant-express');
const cors = require('cors');
const amountFromLabels = require('./utils/amount-from-labels');
const kindFromLabels = require('./utils/kind-from-labels');
@ -21,6 +24,8 @@ module.exports = async function(robot, kredits) {
robot.logger.debug('[hubot-kredits] Ignoring GitHub actions from ', util.inspect(repoBlackList));
}
const kreditsWebUrl = process.env.KREDITS_WEB_URL || 'https://kredits.kosmos.org';
const Contributor = kredits.Contributor;
const Contribution = kredits.Contribution;
@ -172,4 +177,95 @@ module.exports = async function(robot, kredits) {
}
});
//
// GitHub signup
//
if (process.env.GITHUB_KEY && process.env.GITHUB_SECRET) {
const grantConfig = {
defaults: {
protocol: (process.env.GRANT_PROTOCOL || "http"),
host: (process.env.GRANT_HOST || 'localhost:8888'),
transport: 'session',
response: 'tokens',
path: '/kredits/signup'
},
github: {
key: process.env.GITHUB_KEY,
secret: process.env.GITHUB_SECRET,
callback: '/kredits/signup/github'
}
};
robot.router.use(session({ secret: process.env.SESSION_SECRET || 'grant' }));
robot.router.use('/kredits/signup', grant(grantConfig));
robot.router.get('/kredits/signup/github', async (req, res) => {
const access_token = req.session.grant.response.access_token;
res.redirect(`${kreditsWebUrl}/signup/github#access_token=${access_token}`);
});
robot.router.options('/kredits/signup/github', cors());
robot.router.post('/kredits/signup/github', cors(), async (req, res) => {
const accessToken = req.body.accessToken;
if (!accessToken) {
res.status(400).json({});
return;
}
try {
const githubResponse = await fetch('https://api.github.com/user', {
headers: {
'Accept': 'application/vnd.github.v3+json',
'Authorization': `token ${accessToken}`
}
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.
});
} catch (error) {
robot.logger.error('[hubot-kredits] Fetching user data from GitHub failed:', error);
res.status(500).json({ error });
};
if (githubResponse.status >= 300) {
res.status(githubResponse.status).json({});
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 || user.login;
contributorAttr.kind = "person";
contributorAttr.url = user.blog;
contributorAttr.github_username = user.login;
contributorAttr.github_uid = user.id;
kredits.Contributor.add(contributorAttr, { gasLimit: 350000 })
raucao commented 2019-07-26 10:30:08 +00:00 (Migrated from github.com)
Review

Would be useful to know from where and how the contributor was added exactly. We use the [hubot-kredits] prefix in the other files. And could add something like "Contributor added from GitHub signup".

However, creating the contributor needs to be separated from the GitHub hook, because in order to create a more complete profile, we should be able to add more accounts before creating the contributor on chain. This is important, because the bot shouldn't have permission to update hashes afterwards, only to create new contributor records.

Splitting that out can be done in a future PR, of course, but maybe it makes sense to at least split out the functionality from the route already.

Would be useful to know from where and how the contributor was added exactly. We use the `[hubot-kredits]` prefix in the other files. And could add something like "Contributor added from GitHub signup". However, creating the contributor needs to be separated from the GitHub hook, because in order to create a more complete profile, we should be able to add more accounts *before* creating the contributor on chain. This is important, because the bot shouldn't have permission to update hashes afterwards, only to create new contributor records. Splitting that out can be done in a future PR, of course, but maybe it makes sense to at least split out the functionality from the route already.
galfert commented 2019-07-26 19:03:22 +00:00 (Migrated from github.com)
Review

I changed the log message. But apart from adding the CORS headers I didn't work on the signup route itself. So I don't know exactly how or what to split out. Maybe @bumi can jump in here?

I changed the log message. But apart from adding the CORS headers I didn't work on the signup route itself. So I don't know exactly how or what to split out. Maybe @bumi can jump in here?
.then(transaction => {
robot.logger.info('[hubot-kredits] Contributor added from GitHub signup', transaction.hash);
res.status(201);
res.json({
transactionHash: transaction.hash,
github_username: user.login
});
}, error => {
robot.logger.error(`[hubot-kredits] Adding contributor failed: ${error}`);
res.status(422);
res.json({ error })
});
} else {
res.json({
github_username: user.login
});
}
});
} else {
robot.logger.warning('[hubot-kredits] No GITHUB_KEY and GITHUB_SECRET configured for OAuth signup');
}
};

1093
package-lock.json generated

File diff suppressed because it is too large Load Diff

View File

@ -10,11 +10,15 @@
"create-wallet": "scripts/create-wallet.js"
},
"dependencies": {
"cors": "^2.8.5",
"eth-provider": "^0.2.2",
"ethers": "^4.0.27",
"group-array": "^0.3.3",
"express": "^4.17.1",
"express-session": "^1.16.2",
"grant-express": "^4.6.1",
"group-array": "^1.0.0",
raucao commented 2019-07-26 10:31:25 +00:00 (Migrated from github.com)
Review

Why do we have to import express, when hubot comes with express by default?

Why do we have to import express, when hubot comes with express by default?
galfert commented 2019-07-26 17:25:36 +00:00 (Migrated from github.com)
Review

Whoops, that's a leftover of something I tried to fix a problem.

Whoops, that's a leftover of something I tried to fix a problem.
galfert commented 2019-07-26 18:02:52 +00:00 (Migrated from github.com)
Review

Correction: when removing that line, I actually get an error when starting HAL8000

Error: Cannot find module 'express'

Correction: when removing that line, I actually get an error when starting HAL8000 > Error: Cannot find module 'express'
"kosmos-schemas": "^1.1.2",
"kredits-contracts": "^5.3.0",
"kredits-contracts": "^5.4.0",
"node-cron": "^2.0.3",
"node-fetch": "^2.3.0",
"prompt": "^1.0.0"