Best way to add a json body-parser that comes before the root json body-parser in webserver.js

Avan Sardar

Hey everyone,

I hope this is the right place post my question. I was wondering where and or how to add a body-parser verify function?

inside the configureBodyParser function in webserver.js there is

const jsonOpts = nconf.get('bodyParser:json') || {};
app.use(bodyParser.json(jsonOpts));

I am trying to add a verify key with a function as a value which I assume you would add to the config.js file? I am not quiet sure how to add a function for the value.

This all started with me trying to use app.use(bodyParser.json()) in a plugin but then I realized that if there is already one app.use(bodyParser.json()) somewhere else (webserver.js) that it will ignore any other ones that are declared afterwards?

Any help would be greatly appreciated.

Oh yeah, I should mention that my ultimate end goal is to convert the incoming body of one of the endpoints to raw format.

julian

Hi Avan,

The bodyParser library is mostly used as a safeguard against malformed data that could potentially crash the server.

I have to wonder -- do you have to hook into the body parser?

Depending on what you're sending in, and where you're sending it, you could always just mount a route handler to where you're sending this data that needs to be parsed, and then handle it there.

Avan Sardar

Hey @julian

Thanks for the quick reply. Not sure if I completely understand your suggestion. But I feel the fundamental problem remains: webserver.js is hogging parsing incoming data coming in as json? If I remove the

app.use(bodyParser.json(jsonOpts));

in webserver.js and add the following parsing code in my endpoint I can get it to work.

var rawBodySaver = function (req, res, buf, encoding) {
	  if (buf && buf.length) {
	    req.rawBody = buf.toString(encoding || 'utf8');
	  }
	}
	// //
	app.use(bodyParser.json({ verify: rawBodySaver }));

If I mount a route handler which to be honest I haven't looked into it yet and I don't know what it entails I feel like the fundamental problem of the webserver.js processing incoming data still remains, right?

I might be missing something even more fundamental.

Avan Sardar

@julian in case you are still interested. I realized I didn't provide my post with enough context or state what high level problem I am trying to solve. I am writing a private plugin that involves accepting stripe payments. In order to handle stripe event hooks correctly I need the incoming data to be in raw format hence the usage of body-parser. However I just couldn't figure out how to get around the json bodyparser in webserver.js. So after a long discussion on Stackoverflow somebody found a temporary hack. However we also came to the conclusion that it would be nice to have a hook that gets fired up even earlier than

static:app.preload

That way I can attach my middleware before the json parser in webserver. Am I missing another hook that I might be able to use? Or can you think of another way to resolve my issue that does not involve using the hack in the Stackoverflow answer?

Thank you.

https://stackoverflow.com/questions/60067199/how-to-get-around-previously-declared-json-body-parser-in-nodebb

julian

Hmm, I believe we already support a stripe plugin. Although I am not sure if it is open source and available for free use.

That said, there should be no need to actually parse the raw json body, as you should be able to do this in the new route that you mount.

From inside static:app.load, take the router (which is passed in), and attach a new mount. You can see how this is done in the quickstart plugin: https://github.com/NodeBB/nodebb-plugin-quickstart/

Avan Sardar

@julian said in Best Way to Add a bodyParser verify function:

static:app.load

Hey @julian

I am sorry if I am not clear but the problem is the order of execution of the body-parser middleware. The webserver.js body-parser comes before any body-parser I can attach. Does that make sense? I even I attach a new mount to router using the static:app.load hook I cannot change the order of execution. Nodebb will always process incoming json through the webserver.js first and last.

The only solution I personally see if to introduce a new hook that comes before the
static:app.preload hook. Currently setupExpressApp() which is where the json body-parser is being called comes before initializeNodeBB() which is where static:app.preload is fired up.

Maybe I am missing something else though.

PitaJ

Why do you need to introduce a different body parser? What does a verify function do? What do you need that isn't covered by the default body parser?

julian

@Avan-Sardar said in Best Way to Add a bodyParser verify function:

Nodebb will always the process incoming json through the webserver.js first and last.

Correct, but here's where @PitaJ and I recommend that you don't need to replace the built-in body parser.

All you need to do to consume the stripe webhook data is a route handler mounted to the endpoint you wish stripe to send data to.

Avan Sardar

Thanks for responding @julian @PitaJ

Stripe requires the incoming data to be parsed to raw data.

Here is a snippet of their recommended method of handling incoming hook event data:

// ...
// Match the raw body to content type application/json
app.post('/webhook', bodyParser.raw({type: 'application/json'}), (request, response) => {
  const sig = request.headers['stripe-signature'];

  let event;

  try {
    event = stripe.webhooks.constructEvent(request.body, sig, endpointSecret);
  }
  catch (err) {
    response.status(400).send(`Webhook Error: ${err.message}`);
  }
// ...

It looks like the stripe event construction requires the incoming data to passed as raw data.

Here is the link to the documentation page: https://stripe.com/docs/webhooks/signatures

Like I said before I might be missing something. I am aware there was/is a stripe plugin already so I assume there is a way to handle this situation without the introduction of a new hook.

julian

We already parse the request body into JSON, so that should actually be fine. In your code you're using req.headers and req.body, both of which are available already, so there's no need to use your own body parser...

Avan Sardar

@julian

I definitely feel like I am missing something. In the code snippet I provided it uses the bodyParser.raw({type: 'application/json'} middleware. According to the official body-parser documentation:

"bodyParser.raw([options])
Returns middleware that parses all bodies as a Buffer and only looks at requests where the Content-Type header matches the type option. This parser supports automatic inflation of gzip and deflate encodings.

A new body object containing the parsed data is populated on the request object after the middleware (i.e. req.body). This will be a Buffer object of the body."

I don't need the data to be json but in raw(buffer) format. The incoming data is in json format but I need to parse it.

PitaJ

@Avan-Sardar have you tried it without anything special, just excluding the bodyparser.raw middleware? It could be that that stripe API function will accept a json object too.

But if not, you're right, we should allow a way to hook in before the bodyparser is installed.

Avan Sardar

@PitaJ said in Best way to add a json body-parser that comes before the root json body-parser in webserver.js:

middleware

@PitaJ

Yes I have tried that along with a few other things. No luck. Stripe wants it in raw format. The stackoverflow answer I shared has a solution where it changes the order of execution of the middleware. It was little too advanced for my taste. I am still hoping to attach my middleware a little more early on.

julian

What happens when you don't include the call to bodyparser?

Avan Sardar

@julian

The problem remains. Incoming data will will stay in json format. There is an alternative way of handling the stripe hook event that doesn't involve parsing it raw format but my understanding is that it is less secure way of handling the event. I will continue to look for a solution but it seems more and more like the only good solution is to introduce a new hook or move the current hook up.

julian

@Avan-Sardar we're investigating... You're wanting this to do the signature verification, right?

Avan Sardar

@julian

You got it.