Fixed Unhandled error when user upgrade no longer exists

[mattrogowski]

Affected version

2.2.5

When a payment is received for a recurring user upgrade, but the user upgrade no longer exists, this is not gracefully handled and throws a An unexpected error occurred. Please try again later. error with a 500 status, and the following error in the server error log:

TypeError: Argument 2 passed to XF\Service\User\Upgrade::__construct() must be an instance of XF\Entity\UserUpgrade, null given, called in ~/src/XF/Container.php on line 270 src/XF/Service/User/Upgrade.php:32

Full stack trace:

#0 src/XF/Container.php(270): XF\Service\User\Upgrade->__construct(Object(XF\Pub\App), NULL, Object(XFMG\XF\Entity\User))
#1 src/XF/App.php(1701): XF\Container->createObject('XF\\Service\\User...', Array)
#2 src/XF/Container.php(228): XF\App->XF\{closure}('XF\\Service\\User...', Array, Object(XF\Container))
#3 src/XF/App.php(3002): XF\Container->create('service', 'XF:User\\Upgrade', Array)
#4 src/XF/Purchasable/UserUpgrade.php(140): XF\App->service('XF:User\\Upgrade', NULL, Object(XFMG\XF\Entity\User))
#5 src/XF/Payment/AbstractProvider.php(172): XF\Purchasable\UserUpgrade->completePurchase(Object(XF\Payment\CallbackState))
#6 payment_callback.php(63): XF\Payment\AbstractProvider->completeTransaction(Object(XF\Payment\CallbackState))
#7 /Users/mattrogowski/.composer/vendor/laravel/valet/server.php(219): require('/Users/mattrogo...')
#8 {main}

This has seemingly caused PayPal's IPN to be disabled on a client site due to repeated error responses to the IPN.

To reproduce (locally):

• Create a user upgrade
• Purchase the upgrade
• Delete the upgrade entirely
• Manually simulate the payment callback - I am using Postman for this
  • You will need to get the request_key from xf_purchase_request and put this in the relevant field for the payment provider fake payload
  • May also need to make hacky edits to the payment provider classes to force it to accept the fake request (bypass integrity checks)
• You should then get a response of An unexpected error occurred. Please try again later. and the server error will be logged

I have reproduced this with PayPal and Stripe, XF 2.2.5.

From what I can see, XF\Payment\PayPal::validateTransaction() has a check for if the upgrade exists on line 245, but only if $state->legacy is true. However, XF\Purchasable\UserUpgrade::completePurchase() tries to find a user upgrade on line 135 and assumes it finds one - if $state->legacy is false, it uses $userUpgradeId from line 128, and then doesn't handle not finding an upgrade.

 

[Jake B.]

Adding to this, if there are a decent number of users in that user upgrade this will also cause PayPal to disable IPN automatically

 

[djbaxter]

I have experienced similar issues when an upgrade is deleted or edited.

 

[mattrogowski]

We’ve temporarily worked around this by creating a new user upgrade with the same settings as the deleted one, set it to not change any user groups, deactivated it so it doesn’t show up for people to buy, and manually changed the ID in the database to be the ID of the original one. That way the IPN requests will find the upgrade with the required ID, which won’t actually do anything, but it also won’t error.

 

[Chris D]

From what I can see, XF\Payment\PayPal::validateTransaction() has a check for if the upgrade exists on line 245, but only if $state->legacy is true. However, XF\Purchasable\UserUpgrade::completePurchase() tries to find a user upgrade on line 135 and assumes it finds one - if $state->legacy is false, it uses $userUpgradeId from line 128, and then doesn't handle not finding an upgrade.

The explicit difference for the legacy case here is because prior to XF 2.0 all payments would have been through PayPal and only user upgrades were supported. Aside from that, payment handlers should be entirely agnostic of the purchasable element.

But I agree that we're not correctly handling this. It's potentially something we need to handle much earlier, and outside of the payment handling and purchasable handling code because theoretically I believe this could affect any type of purchasable item (assuming that third party implementations may be taking the same approach as us and not accounting for that missing purchasable case).

We have a solution in mind, though custom purchasable handlers will have to implement the logic to guard against it in their own code. Will be handled with user upgrades automatically though.

 

[Chris D]

Semi-related but it's worth noting that in XF 1.x we used to have a notice when deleting a user upgrade that recommended disabling rather than deleting.

If it is a user upgrade that has ever been purchased - particularly one that has recurring payments - we don't recommend deleting.

We've reimplemented that notice in XF 2.2.6 (we think it was missing due to an oversight during development of XF2).

 

[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.6).

Change log:

Validate a purchasable item exists during the callback stage of a payment.

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