Use eth_signTypedData for authenticating #2
Conversation
|
Reading through the comments (and the disclaimer up top) on the Medium article, it seems like this might not be the best idea to implement right away as the API might change? |
johnhforrest
left a comment
johnhforrest
left a comment
There was a problem hiding this comment.
Will approve after the package-lock stuff is sorted out
src/routes/auth-token/create.js
Outdated
| import logger from '../../services/logger' | ||
|
|
||
| const hashedPersonalMessage = ethUtil.hashPersonalMessage(ethUtil.toBuffer(config.personalMessageToSign)) | ||
| const { typedDataToSign } = config |
There was a problem hiding this comment.
Remove this line and just use config.typedDataToSign below, i.e.:
const recoveredAddress = ethSigUtil.recoverTypedSignature({
sig: request.parameters.signedData,
data: config.typedDataToSign,
})There was a problem hiding this comment.
I'm doing something similar on the front end and it complained that I wasn't destructuring. Happy to make this change though.
src/routes/auth-token/create.js
Outdated
| // NOTE: signedData should not have the 0x prefix as Joi will validate it as | ||
| // a raw Buffer | ||
| signedData: Joi.binary().encoding('hex').length(65).required(), | ||
| signedData: Joi.string().length(132).required(), |
There was a problem hiding this comment.
Will length always be 132? Is there any documentation/source code you can point me to that verifies this will always be 132? Could always just remove the length too.
There was a problem hiding this comment.
Will let recoverTypedSignature deal with length if it needs to.
src/config.js
Outdated
| { | ||
| type: 'string', | ||
| name: 'Sign In', | ||
| value: 'Sign in to Codex Title Viewer', |
There was a problem hiding this comment.
Can we keep this message similar to what it was before?
Please sign this message to authenticate with the Codex Title Viewer.
Or is that too verbose 🤔
src/routes/auth-token/create.js
Outdated
| const userAddressBuffer = ethUtil.toBuffer(request.parameters.userAddress) | ||
| const senderAddressBuffer = ethUtil.publicToAddress(publicKey) | ||
| const recoveredAddress = ethSigUtil.recoverTypedSignature({ | ||
| data: typedDataToSign, |
There was a problem hiding this comment.
Can you flip these two lines so they're in descending order by length, i.e.:
const recoveredAddress = ethSigUtil.recoverTypedSignature({
sig: request.parameters.signedData,
data: config.typedDataToSign,
})There was a problem hiding this comment.
Happy to. Is longest parameter name first part of our style guide? Haven't come across that before :)
There was a problem hiding this comment.
Not really I guess, it's just my personal preference. It's also kind of arbitrary how I decide that, it's just whatever "looks" best. I typically try to cascade things like this though.
🚨Do Not Merge 🚨
We are waiting until
eth_signTypedDatais widely supportedCorresponding web app PR https://github.com/codex-protocol/web.codex-title-viewer/pull/9