TritonDataCenter / node-http-signature

Reference implementation of Joyent's HTTP Signature Scheme
https://tritondatacenter.com
MIT License
405 stars 118 forks source link

shoud use `request.originalUrl` instead of `url` to fill up `request-target`? #87

Open lesion opened 5 years ago

lesion commented 5 years ago

https://github.com/joyent/node-http-signature/blob/523e7c5a3a081e046813f62ab182e294a08eaf0d/lib/parser.js#L266

express.js will overwrite req.url using app.use:

app.use('/first', anotherRouter)
anotherRouter.post('/second', verifySignature)

inside verifySignature the req param has the property url without the /first part of path so it fails. I'm actually overwriting it to make it works:

req.url = '/first' + req.url
const parsed = httpSignature.parseRequest(req)

so the question: shoud we use originalUrl param instead?

lesion commented 5 years ago

for reference:

https://expressjs.com/en/api.html#req.originalUrl

This property is much like req.url; however, it retains the original request URL, allowing you to rewrite req.url freely for internal routing purposes. For example, the “mounting” feature of app.use() will rewrite req.url to strip the mount point.

If you're ok with this I would prepare a PR

wmurphyrd commented 4 years ago

I'm finding signatures created with http-signature that include (request-target) will fail later validation by http-signature because the signer and validator do not construct the same signature string. The proposed fix would solve the issue for me

Here's a workaround:

      const tempUrl = req.url
      req.url = req.originalUrl
      sigHead = httpSignature.parse(req)
      req.url = tempUrl