emartech / escher-js

Library for HTTP request signing (JavaScript implementation)
MIT License
12 stars 11 forks source link

Remove forced type check of request body #9

Closed adampatarino closed 6 years ago

adampatarino commented 6 years ago

Escher is authentication framework, so it shouldn't make assumptions on the data type of the request body. This is especially clear when using alongside other request body parsing middleware in frameworks like express or koa.

Example:

const bodyParser = require('body-parser'),
          Escher = require('escher-auth');

app.use(bodyParser.json());

// ... setup escher options

escher.authenticate(request, keyDB);

This throws an error

Error: The request body shouldn't be empty if the request method is POST

However, the body is present, in the form of an object:

{
    "body": {
        "customer_id": "00000000",
        "resource_id": null,
        "environment": "suite00.emarsys.net",
        "language": "en"
    }
}

This pull request changes the type check of req.body from a strict String or Buffer to a simple exists check:

//Before
!(typeof reqBody === 'string' || reqBody instanceof Buffer)

// After
!request.body
szeist commented 6 years ago

Hi Adam,

You are right about that this is an authentication library and the mentioned error message is misleading. However the escher signature can be calculated only on strings or buffers, so I'm still thinking that we need to enforce the verifiable request body type. Maybe we can throw a "The request body should be only string or buffer".

The signature validation will only pass if the tested request body is exactly the same as the signed request body. So if we try to convert the parsed body back to string (e.g.: JSON.stringify) and the result is not exactly the same with the original request body (e.g.: the original request contained some additional spaces which didn't break the JSON) the validation will fail.

If you are using Koa, I suggest the koa-escher-auth middleware.

All the best, Istvan

adampatarino commented 6 years ago

@szeist Ok I didn't realize escher was using the body content as part of the authentication method. I've built a workaround in my express midleware to JSON.parse after escher has validated the connection.

Thanks for the added detail!