Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrimStrings middleware breaks signature verification - Part 3 #173

Closed
goeki85 opened this issue Nov 10, 2022 · 5 comments
Closed

TrimStrings middleware breaks signature verification - Part 3 #173

goeki85 opened this issue Nov 10, 2022 · 5 comments

Comments

@goeki85
Copy link

goeki85 commented Nov 10, 2022

  • Cashier Paddle Version: 1.6.1
  • Laravel Version: 9.27.0
  • PHP Version: 8.0.2

Description:

This issue is already described in #120 and #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
Copy link
Member

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
Copy link
Author

goeki85 commented Nov 10, 2022

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 #152 by adding customer_name to $except in TrimStrings middleware.

Thanks!

@driesvints
Copy link
Member

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

@crnkovic
Copy link

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

@patrickomeara
Copy link
Contributor

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']);
})

patrickomeara added a commit to patrickomeara/cashier-paddle that referenced this issue Aug 7, 2024
Paddle verifies the signature based on the input as they have it on their end. If Cashier receives strings that have been trimmed the signature verification fails. The fields need to stay as they are.

laravel#120
laravel#152
laravel#173
taylorotwell pushed a commit that referenced this issue Aug 8, 2024
Paddle verifies the signature based on the input as they have it on their end. If Cashier receives strings that have been trimmed the signature verification fails. The fields need to stay as they are.

#120
#152
#173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants