67P / hubot-kredits

Kosmos Kredits chat/service integrations
https://wiki.kosmos.org/Kredits
MIT License
2 stars 1 forks source link

Add GitHub signup oracle #41

Closed bumi closed 4 years ago

bumi commented 5 years ago

This adds a GitHub oauth signup oracle that allows users to create a new contributor profile through a oauth verification.

hubot implements an API and the oracle to create the contributor. The frontend is handled by kredits-web.

Flow: kredits web -> reditects to on hubot: /kredits/signup/github/connect/github -> user gets redirected to github -> redirected to kredits web with access token -> API call to hubot POST /kredits/signup/github to create the contributor.

galfert commented 4 years ago

I finished the implementation. This should all work together with 67P/kredits-web#142 now.

When creating the Github OAuth app, the Authorization callback URL should be the base URL of HAL8000 (e.g. http://localhost:8888 for development).

When starting HAL8000, set the following new ENV vars:

I haven't tested the actual creation of the contributor yet (i.e. this part), because I don't have a full local Kredits contracts setup yet. For testing, I just added the contributorAttr object to the response and logged it in Kredits Web.

galfert commented 4 years ago

Did anybody try this out yet?

gregkare commented 4 years ago

I have just successfully tried this, until the screen that asks for an ETH address, I stopped there because my local kredits-web couldn't be configured to use my local chain (https://github.com/67P/kredits-web#3-kredits-web)

I had to generate a wallet and copy it to my test hubot's root and set addresses: { Kernel: '0x... } in index.js for hubot to start

raucao commented 4 years ago

Those docs are wrong or outdated. npm run start:local always uses the local contracts, but you need to always set KREDITS_KERNEL_ADDRESS to the DAO address that you can show in kredits-contracts by running npm run dao:address.

raucao commented 4 years ago

I had to generate a wallet and copy it to my test hubot's root and set addresses: { Kernel: '0x... } in index.js for hubot to start

Yes, hubot-kredits needs a wallet that is allowed to create contributors in your local contracts. That part is missing entirely from the instructions here, unless I missed it.

Also, I think we still have to hardcode the local DAO address in its code, unless I missed that as well.

raucao commented 4 years ago

I haven't tested the actual creation of the contributor yet (i.e. this part), because I don't have a full local Kredits contracts setup yet.

It would be extremely useful if the authors of this PR had actually tested it from end to end themselves.

raucao commented 4 years ago

I merged support for setting the local DAO address via the KREDITS_DAO_ADDRESS env var into this branch, making it much easier to test.

galfert commented 4 years ago

I had to add the gasLimit option to the Contributor.add method, similar to what kredits-web is doing. Otherwise I would get this error when trying to create a new contributor:

{ Error: VM Exception while processing transaction: revert
    at getResult (/Users/galfert/code/active/kosmos/hubot-kredits/node_modules/ethers/providers/json-rpc-provider.js:40:21)
    at exports.XMLHttpRequest.request.onreadystatechange (/Users/galfert/code/active/kosmos/hubot-kredits/node_modules/ethers/utils/web.js:111:30)
    at exports.XMLHttpRequest.dispatchEvent (/Users/galfert/code/active/kosmos/hubot-kredits/node_modules/xmlhttprequest/lib/XMLHttpRequest.js:591:25)
    at setState (/Users/galfert/code/active/kosmos/hubot-kredits/node_modules/xmlhttprequest/lib/XMLHttpRequest.js:610:14)
    at IncomingMessage.<anonymous> (/Users/galfert/code/active/kosmos/hubot-kredits/node_modules/xmlhttprequest/lib/XMLHttpRequest.js:447:13)
    at IncomingMessage.emit (events.js:187:15)
    at endReadableNT (_stream_readable.js:1094:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

With the gasLimit option it works now.

raucao commented 4 years ago

Unfortunately, I get the following error from the callback loading:

hal7000> (node:7741) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'access_token' of undefined
raucao commented 4 years ago

@galfert @bumi Sure this shouldn't be a kredits-3 by now?

(Btw, I don't want kredits for my commits. Was just drive-by fixing stuff in my review and mostly doing the Web part of this.)

raucao commented 4 years ago

Oh, and I think this is good to merge, right?

galfert commented 4 years ago

Yes, you're right. I changed the label.

I think we are good to go.

bumi commented 4 years ago

need to add more documentation, then we merge.

raucao commented 4 years ago

need to add more documentation, then we merge.

I added an issue for it. We can wait with tagging the release until the new feature is documented properly.