laravel / cashier-paddle

Cashier Paddle provides an expressive, fluent interface to Paddle's subscription billing services.
https://laravel.com/docs/cashier-paddle
MIT License
239 stars 57 forks source link

TrimStrings middleware breaks signature verification - Part 3 #173

Closed goeki85 closed 1 year ago

goeki85 commented 1 year ago

Description:

This issue is already described in https://github.com/laravel/cashier-paddle/issues/120 and https://github.com/laravel/cashier-paddle/issues/152

Cashier responds with a 403 error to the webhook subscription_payment_succeeded from Paddle if the field customer_name inside the webhook payload has any whitespaces before or after the given string ( John Doe) or (John Doe ).

I got lots of conversations about this with the paddle support. Here are the important parts:

Screen Shot 2022-11-10 at 09 37 23 Screen Shot 2022-11-10 at 09 37 31 Screen Shot 2022-11-10 at 09 37 41 Screen Shot 2022-11-10 at 09 38 17

I also think that this would be the solution. So in the first step, Cashier should ignore the payload and only check if the signature is correct. Then, if the signature is correct the whitespaces can be trimmed and manipulated in step 2.

So during the validation (Step 1), Cashier should not sanitize the incoming data. It should accept it as it comes from Paddle. After the validation took place, the spaces can be trimmed.

driesvints commented 1 year ago

It should not be possible to trigger this as these strings are trimmed before the paylink is generated: https://github.com/laravel/cashier-paddle/blob/1.x/src/Concerns/PerformsCharges.php#L77

I disagree that this is a Cashier issue. Clearly, as Paddle indicates themselves this is a Paddle issue. It's up to Paddle to make sure that whitespace does not matter here like other payment providers do (Stripe for example).

goeki85 commented 1 year ago

Hey Dries,

totally understand that and i know it's not normal to get a field with additional whitespaces. But not everyone is using Paylinks with Cashier. I myself for example use the Paddle.js checkout because this is required to use the affiliate feature of Paddle.

I just sent the passthrough parameter manually and build it like Cashier does build it. So this issue is not related to generated paylinks from Cashier in any way.

But long story short, i just "fixed" it like @BKirev described in https://github.com/laravel/cashier-paddle/issues/152 by adding customer_name to $except in TrimStrings middleware.

Thanks!

driesvints commented 1 year ago

@goeki85 ah that's actually a good solution that I hadn't thought of. Thanks for sharing 👍

crnkovic commented 5 months ago

Whoa thanks, it was an annoying issue for me, but adding customer_name to TrimStrings did fix it!

patrickomeara commented 1 month ago

Thanks @goeki85 and to those in part 1 and part 2. This one had me stumped.

For those on Laravel 11:

// bootstrap/app.php
->withMiddleware(function (Middleware $middleware) {
    $middleware->trimStrings(except: ['customer_name']);
})