[WIP] Update: Key-Pair auth data check #542
Conversation
src/server/middleware/auth.ts
Outdated
| if (data && req.method === "POST" && data !== hashRequestBody(req)) { | ||
| error = "The request body has been tampered with."; | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
We can only do this check when the request is of POST type, otherwise the req.body will come as empty
There was a problem hiding this comment.
I wonder if we even need this check. If the developer hashes a request body and calls a GET endpoint, I think it's desirable (stricter) that the auth fails.
There was a problem hiding this comment.
Added the POST check as GET spec doesn't support body as per the HTTP/1.1. Some web servers (Apache, Nginx & Traefik) drop the body param for GET request when forwarding the request. The spec doesn't forbid it, but the HTTP servers do drop it.
src/server/middleware/auth.ts
Outdated
| @@ -278,6 +282,11 @@ const handleKeypairAuth = async ( | |||
| throw error; | |||
| } | |||
|
|
|||
| if (data && req.method === "POST" && data !== hashRequestBody(req)) { | |||
| error = "The request body has been tampered with."; | |||
There was a problem hiding this comment.
This error sounds scary when the dev may have just made a mistake. Let's make sure errors directly state the issue and optionally provide a way to fix it:
The request body does not match the hash in the access token. See: https://portal.thirdweb.com/engine/features/keypair-authentication
There was a problem hiding this comment.
Makes sense.
can we tinyurl this?
PR-Codex overview
The focus of this PR is to enhance authentication middleware in the server by adding keypair authentication with body hashing validation.
Detailed summary
createHashimport for hashingonRequesttopreValidationhandleKeypairAuthto include body hashing validationhashRequestBodyfunction to hash request body