Fixed Create payment log for "unknown source"

[mattrogowski]

Affected version

2.2.8

As I'm sure this is working exactly as intended, I've created this as a suggestion and not a bug report...

When setting up a payment callback/webhook URL, you're obviously required to specify the payment provider (i.e. /payment_callback.php?_xfProvider=xxxxx). If no query string is provided, it's treated as a legacy PayPal payment - makes perfect sense as that's the old XF1 URL.

However - if you hit that URL for a payment that is not a legacy PayPal payment, it runs this:

if (!$response || $response->getBody()->getContents() != 'VERIFIED' || $response->getStatusCode() != 200)
{
    $host = \XF\Util\Ip::getHost($state->ip);
    if (preg_match('#(^|\.)paypal.com$#i', $host))
    {
        $state->logType = 'error';
        $state->logMessage = 'Request not validated';
    }
    else
    {
        $state->logType = false;
        $state->logMessage = 'Request not validated (from unknown source)';
    }
    return false;
}

This will output "Request not validated (from unknown source)" if it's not a PayPal request, but it won't log it to the payment log - I'm wondering what the rationale for that is.

Yesterday we had an admittedly niche edge-case issue where the payment provider themselves started sending postbacks to the wrong URL - it sent them to just /payment_callback.php (everything was configured correctly, they'd messed up on their end). This meant the requests got treated as legacy PayPal payments, ran this code, but the problem was they just failed silently, as it doesn't log this error anywhere. We were getting errors from the payment provider that postbacks were failing, but there was no record of anything coming in to XF at all. It was only when I edited /payment_callback.php to log the post data to the server error log that I eventually noticed the request URL was wrong, and we reported the issue to the payment provider. However, it would have been an enormous help if this error message (which was the most critical one in this case) was logged to the payment log like all the rest, as I would have found the issue straight away - it's one of those "shouldn't be here but we are" scenarios but would have cut 3 hours of debugging down to 3 minutes (never occurred to me the URL had changed as it had worked for months).

Obviously this would be a very rare occurrence and wouldn't happen if everything was configured and working correctly, but it would be a quality of life improvement if every failure reason was logged to the payment log, instead of doing all of them except this one. It would also help people debug if they've set the callback up incorrectly and omitted the query string by accident.

If the issue with logging this error would be that it would log direct requests to the URL (i.e. you going to the URL manually), I'd imagine it could simply check if it's a POST request and log it, and not log it for GET requests, because on the balance of probability, if a POST request is running this code and isn't from PayPal, something else is wrong and it'd be super help to know that that's where the code execution has got to (which, after all, is the benefit of having the payment provider log). Quick way of doing it maybe:

$state->logType = !empty($state->_POST) ? 'error' : false;

That way if a POST request has been sent we can see it's got here and work out why, and if you just go to the URL directly, it logs nothing as it does now.

 

[Chris D]

As I'm sure this is working exactly as intended, I've created this as a suggestion and not a bug report...

I think the suggestion and suggested change is reasonable so we'll track it as a bug for now.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.9).

Change log:

Log payment callbacks that come from an unknown source

There may be a delay before changes are rolled out to the XenForo Community.