5
\$\begingroup\$

Below is the WebHook endpoint in its entirety. I have five seconds to acknowledge if the payload is correctly formed. After that, the payload is inserted into a queue for processing.

Importantly, it works, but it feels naive outside a Class and without any exception handling or error checking beyond the signature match. It does however, achieve exactly what I need.

Should for example, $rawPayload = file_get_contents("php://input"); and $authKey = $_SERVER['HTTP_SIGNATURE']; be inside a try catch block?

Alternatively, should it be threaded in case the endpoint is flooded?

<?php

namespace NJIS\Helpers;

use NJIS\Helpers\XeroWebHookHandler;

$key = $_ENV['XERO_WEBHOOK_KEY'];
$rawPayload = file_get_contents("php://input");
$signature = base64_encode(hash_hmac('sha256', $rawPayload, $key, true));
$authKey = $_SERVER['HTTP_SIGNATURE'];

if($signature === $authKey)
{
    http_response_code(200);
    XeroWebHookHandler::insertPayloadIntoDatabase($rawPayload);
}   
else
{
    http_response_code(401);
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Just before I write an answer is this just a file on the webserver ? E.G on apache your document root is /var/www/someFolder where you have a file someApi.php and you make calls to that server ? Or is a part of a larger frame work with some sort of router ? \$\endgroup\$ – Dan Apr 16 '18 at 20:33
  • \$\begingroup\$ This is a file on a web server. I was originally using a router, however the endpoint is extraordinarily strict and denied responses containing session cookies and other junk that was bundled into the http response. Further -the endpoint didn't seem to like the way Swing handled response codes. Because of this, .htaccess redirects to this file before any of the routing happens. \$\endgroup\$ – pcgben Apr 17 '18 at 0:32
2
\$\begingroup\$

Only several years late, but since someone bumped the question let's actually answer it. This code does look somewhat reasonable, but it should be programmed more defensively to avoid situations where people can insert anything in your queue.

Handle all possible outputs

hash_hmac can return a string or the boolean false. You pass this directly to base64_encode, which will happily turn false into an empty string.

var_dump(base64_encode(false)); // string(0) ""

Currently, hash_hmac returns false if the algorithm is not supported. If for some reason sha256 is dropped as an algorithm, you can put anything in the queue with an empty signature. Adding an !empty($signature) check to the if-statement will allow the code to fail safely in case something odd is going on.

file_get_contents can return false on failure, and hash_hmac will happily treat this as an empty string for key generation. You could add a sanity check that fails early if you have nothing useful to add to the database.

Exceptions, try and catch

None of your functions seem to throw, except maybe for XeroWebHookHandler::insertPayloadIntoDatabase. This also means that adding a try-catch block does not do anything useful for you. If you have a way of telling if this function fails, you can move http_response_code below this function and potentially return a different response code if the function fails.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.