pagarme-js icon indicating copy to clipboard operation
pagarme-js copied to clipboard

Wrong postback examples on docs

Open devdrops opened this issue 8 years ago • 8 comments

At https://docs.pagar.me/reference#validando-um-postback, we found that the JS sample lacks a basic detail for the operation.

const pagarme = require('pagarme')

// Calculate signature:
pagarme.postback.calculateSignature('X-Hub-Signature', 'postbackBody')
// returns a hash

// Verify signature:
pagarme.postback.verifySignature('X-Hub-Signature', 'postbackBody', 'expectedHash')
// returns true or false

This example is kinda wrong, due to the fact that it leads the developer to believe that he doesn't need Pagar.me's API Key. The right approach would be to make explicit and clear that the developer need the client to perform such actions, which contains the API Key.

devdrops avatar Aug 11 '17 12:08 devdrops

Awesome! Could you open a PR fixing this please? :D

MarcoWorms avatar Aug 11 '17 14:08 MarcoWorms

Just faced this problem. In which point should the API key fit in the example ?

lucasm0ta avatar Jun 29 '18 14:06 lucasm0ta

Weird, I have just tried to use the approach without a client and it worked perfectly.

const pagarme = require('pagarme')

const signature = pagarme.postback.calculateSignature('X-Hub-Signature', 'postbackBody')

const verify = pagarme.postback.verifySignature('X-Hub-Signature', 'postbackBody', signature)

console.log('signature', signature) // signature e5f37003f940c0e578e7e07471164cbc987253af
console.log('verify', verify) // verify true

And it doesn't seem wrong to me, because those functions have nothing to do with a client, they are pure functions that don't depend on any state.

IMPORTANT EDIT

:rotating_light: DO NOT USE THIS SOLUTION :rotating_light:

Use @bmamone solution down below instead :smile:

evaporei avatar Jun 29 '18 16:06 evaporei

I just tested and the approach used by @otaviopace always returns true, no matter the payload neither the signature.

I'm also trying to figure out which point should I use the client or the api key itself.

EDIT:

const pagarme = require('pagarme');

pagarme.client.connect({ api_key: 'key' })
  .then(client => {
    const verify = client.security.verify('postbackBody', 'X-Hub-Signature'));
  });

The example above is the correct way to do it.

victorbrjorge avatar Sep 26 '18 17:09 victorbrjorge

Hmmm, I just tried passing a random set of characters to the verify function and it returned false as expected:

const pagarme = require('pagarme')

const RANDOM_SIGNATURE = 'aslkfdjalksjdflkajsf'

const verify = pagarme.postback.verifySignature('X-Hub-Signature', 'postbackBody', RANDOM_SIGNATURE)

console.log('verify', verify) // false

IMPORTANT EDIT

:rotating_light: DO NOT USE THIS SOLUTION :rotating_light:

Use @bmamone solution down below instead :smile:

evaporei avatar Sep 26 '18 21:09 evaporei

If you check the actual code for verifySignature:

function calculateSignature (key, postbackBody) {
  return createHmac('sha1', key)
    .update(postbackBody)
    .digest('hex')
}

function verifySignature (key, postbackBody, headerSignature) {
  const signature = calculateSignature(key, postbackBody)
  return equals(signature, headerSignature)
}

The first parameter is expected to be the api key, the second one the postbackbody and the third one the X-Hub-Signature header.

The function does nothing but create a hash of the body using your api key and check if it is equal to the same hashing done by pagarme's server.

So there is no way for this to be done without using your api key on the process.

victorbrjorge avatar Sep 26 '18 21:09 victorbrjorge

Just lost some time trying to get this right... not only docs are completely wrong, leading to possible security breaches, but also the function does not work at all.

First, the docs part:

For this to work, you of course need your API KEY to have something to compare the signature to. Also, you should NOT use the string 'X-Hub-Signature', its actually a reference to the header you need to compare to:

pagarme.postback.calculateSignature(process.env.PAGARME_API_KEY, rawBody)

pagarme.postback.verifySignature(process.env.PAGARME_API_KEY, rawBody, req.headers['x-hub-signature'])

So I did that and after trying a lot, it still would not say a valid signature was valid.

If you log both X-Hub-Signature header and the calculateSignature result, you will see that verifySignature will NEVER return true, as X-Hub-Signature header is prefixed with sha1=.

So this function does not work AT ALL. To make it worse, the docs are misleading unaware devs:

@otaviopace solution in this issue is basically generating a signature using a constant string X-Hub-Signature and whatever payload, then verifying it against the very same signature, by signing it again with X-Hub-Signature string, and not the actual header sent from Pagarme. So its not validating anything and creates a security breach

I did EXACTLY that, prior to finding this issue, but luckily realized that it wasn't validating it against any useful information.

Then I find this issue, which was reported in 2017, and it's clearly misleading devs into security breaches.

Docs and code needs fixing !!

For those who want to use this function, just use calculateSignature, prefix it with sha1= and compare it to X-Hub-Signature. Or you can use Node.js crypto lib:

      
const signature = createHmac('sha1', process.env.PAGARME_API_KEY).update(rawBody, 'utf-8').digest('hex')

       
if (req.headers['x-hub-signature'] !== `sha1=${signature}`) {
     // INVALID SIGNATURE
}

bmamone avatar Mar 25 '21 22:03 bmamone

@bmamone thanks for the detailed explanation :clap: :heart: I've edited my comment so that it does not mislead any more developers into doing this wrong.

Unfortunately I can't help you more since I don't work for @pagarme anymore :confused:

evaporei avatar Mar 25 '21 22:03 evaporei