Meteor-Community-Packages / picker

Server Side Router for Meteor
https://atmospherejs.com/communitypackages/picker
MIT License
16 stars 3 forks source link

Picker should handle middleware errors #3

Open rubenhelsloot opened 2 years ago

rubenhelsloot commented 2 years ago

Thank you for reviving this package!

I wanted to ask to open issue #12 here too, because it's highly relevant.

Situation

I maintain an application that accepts incoming webhooks from Shopify. The data on that webhook is sent in JSON format, so I've registered a bodyparser.json() middleware in Picker. Normally, I then check that the hash of the JSON body is the same as the value of one of the request headers. This is also described in their docs.

However, when the body size is too large, req.body is empty with no warning whatsoever. Calculating the hmac fails and the webhook is aborted. But the true error happened way earlier in the request, namely when the middleware parsing failed. Ideally, I'd want to have been notified on parse error, not later.

A subset of my code:

Picker.middleware(bodyParser.json({
  verify: (req, res, buf) => {
    req.rawBody = buf;
  },
}));

/**
 * Cryptomagically check the webhook shared secret to make sure the
 * webhook originates from Shopify
 */
const checkShopifyWebhookSecret = (req, country) => {
  const webhookSecret = Meteor.settings.shopifyWebhookSecret[country]
  const hmac = req.headers['x-shopify-hmac-sha256']

  let success = true;
  try {
    // Create a hash using the body and our key
    const hash = crypto
      .createHmac('sha256', webhookSecret)
      .update(req.rawBody, 'utf8', 'hex')
      .digest('base64')

    // check if webhook is valid
    success = hash === hmac;
  } catch (e) {
    logger.info("Unable to validate webhook", { body: req.body, error: e });
    success = false;
  }

  if (!success) throw new Error('invalid-webhook-request');
}
StorytellerCZ commented 1 year ago

@rubenhelsloot thank you for raising this issues. I have added it to my todos and hopefully will get a chance to look at it in the coming months.

perbergland commented 11 months ago

Just stumbled on this problem myself but Picker seems to be going away in favour of (express core functionality?) in Meteor 3 so maybe not worth fixing.