Create a middleware to verify signatures and adminOnly

[dgrcode]

This is calling for a middleware. I envision using something like this:

app.post("/challenges", userOnly, verifySignature("challengeSubmit"), async (req, res) => {
  // ...
}

That means the middleware would need to be curried:

const verifySignature = messageId => (req, res, next) => {
  // ...
  const verifyOptions = { ...req.body }
  verifyOptions.messageId = messageId
}

I also showed in the snippet a way to avoid creating a different verifyOptions for each case, which is handy to abstract the code in a middleware. We know the required options must come from the client in the request's body, so we just pass all that to the verifySignature call ¯\_(ツ)_/¯

Given the current code works, we might want to open an enhancement issue for this.

Originally posted by @dgrcode in #48 (comment)

[dgrcode]

We might be even able to extract that middleware into a small npm package that can be used to effortlessly work with signatures on other projects. Something with an api similar to

// server
import { verifySignatureMiddleware, signatureMessages } from 'easy-web3-sign/server'
// sets up everything to generate messages to sign
// Potentially receives a configurator to show a readable message plus a bunch of metadata
// that can be use to generate a specific hash to make the message to sign unique
//  e.g.: "Sign to submit challenge two [hash: 23a9ef83c0233b]"
// or maybe that's not needed and the config goes into the middleware ¯\_(ツ)_/¯
app.use(signatureMessages)

app.post('/needs-signature', verifySignatureMiddleware, (req, res) => {
  // ...
})

// client
import { sign } from 'easy-web3-sign/client'

// handles getting messages, showing errors, etc
<button onClick={() => sign(onSuccess, onError)} />

Definitely not something to implement as part of this project, but could be useful for moonshots in general (not sure where I should drop a suggestion like this).

[dgrcode]

This will be needed after #29 is implemented. We'll remove the usage of JWTs, and therefore will need to verify signatures to confirm someone is admin.

[dgrcode]

Just read some comments from the server:

!! SKIPPING ADMIN CHECKS. THIS SHOULD BE FIXED IN #51

Now that this is live we should probably tackle this one

[carletex]

@dgrcode I don't see it critical, but could plan on taking this one over the next weeks.

We are using the adminOnly in two places:

1. app.patch("/challenges", adminOnly,

Here it doesn't matter because the signature is doing the validation work.

2. app.get("/challenges", adminOnly

Every one can gets this data right now, which It's not very sensitive, but I guess we don't want to expose it.

I see 3 options:

1. Signature read for the review challenge. You sign => you get all the challenges ready to review.
2. On the adminOnly middleware we check if the userAddress (on the request header) is an admin (not very hard to hack XD)
3. Go back to JWT.

I'll go with 1 or 3, depending on future functionalities. What do you think?

[dgrcode]

I think the problem is actually on the app.patch. We do request a signature, but we never check that the signature is signed by an admin. Any valid signature will work. We just check the address who signed is the address in the header. Once we trust the address (thanks to the signature), we should also check the address has the admin role. (We're just doing authentication checks, not authorisation checks)

For reading (app.get) I don't mind it that much. 1 and 3 are more secure, but as you say the information they'd get is not very sensitive. Also, for exploiting option 2 you need to know an admin address, and worst case you get read access because you won't be able to fake their signature. So I wouldn't even mind just checking the address in the headers.

[carletex]

You're right. I think something like this will fix it for now: #105

Even if they guess an admin address, they won't be able to make the signature work.