Add sign in with publicKey #2034
Conversation
| @@ -27,6 +27,7 @@ | |||
| "babel-plugin-add-module-exports": "1.0.2", | |||
| "babel-plugin-lodash": "3.3.4", | |||
| "bcrypt": "3.0.6", | |||
| "blockstack": "^19.1.0", | |||
There was a problem hiding this comment.
You said integrating this blockstack library might not be mandatory. What would be the alternative? If it's simple, then let's do that.
There was a problem hiding this comment.
I had a look but it is a bit more complicated.
server/controllers/users.js
Outdated
| if (process.env.NODE_ENV !== 'production' && user.email.match(/.*test.*@opencollective.com$/)) { | ||
| if ( | ||
| (process.env.NODE_ENV !== 'production' && user.email.match(/.*test.*@opencollective.com$/)) || | ||
| user.publicKey |
There was a problem hiding this comment.
If there is no e2e tests, then let's not modify that.
server/controllers/users.js
Outdated
| @@ -34,19 +59,25 @@ export const exists = async (req, res) => { | |||
| export const signin = (req, res, next) => { | |||
There was a problem hiding this comment.
At this stage, can you have a specific signin endpoint for blockstack? And use a dedicated exists method?
To minimize footprint in server/controllers/users.js, supporting functions may be loaded from lib/blockstack.js
| @@ -204,12 +206,25 @@ const mutations = { | |||
| throw new Error('User already exists for given email'); | |||
| } | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
I don't think it's useful to do any modification in these GraphQL queries. The signin endpoint should be enough.
There was a problem hiding this comment.
This is for creating a new profile with blockstack. However, we can remove these, and limit the creation of new profiles with blockstack to personal profiles that are handled through signin, not createUser
There was a problem hiding this comment.
Ok, sorry, I was a bit wrong. createUser was indeed used for signups.
But, please handle that in a separate endpoint (ie: blockstackSignin / blockstackSignup), or a separated GraphQL mutation (ie: createUserWithBlockstack).
There was a problem hiding this comment.
I have re-added it and moved everything into separate methods.
|
I have added the updateUserPublicKey mutation to better handle existing users |
|
Can I do more to get this one merged? |
|
@friedger nothing on the API side, this is more on the frontend side that we had open questions. We need to have another look and test again all the different cases. |
|
This was pretty good work and I'm sorry we let this PR go stale. Unfortunately, after discussing it with the team last week, there was no consensus to move it forward and I'll have to close it. This could be revisited once supporting multiple identity providers become a priority. |
This PR
publicKeyto UserInputTyperedirectto sign in requests if the user input comes with apublicKeyvalue. Theredirectvalue is the encrypted redirect link.This is a proof of concept towards
opencollective/opencollective#1749
Corresponding UI looks like this:
opencollective/opencollective-frontend#1833
Probably it is not necessary to include blockstack.js as only function
encryptContentis needed.After sign in the user name contains the blockstack id of the user:
