spookylukey / django-paypal

A pluggable Django application for integrating PayPal Payments Standard or Payments Pro
MIT License
729 stars 208 forks source link

PayPalPaymentsForm renders value in DOM #224

Closed KalobTaulien closed 4 years ago

KalobTaulien commented 4 years ago

Hey there,

There's a problem with the button when using {{ form.render }} — the purchase value is editable through the DOM. The amount you want to pay can be changed to anything you want. Most often it's set to 0.01 to buy anything for one penny.

image

I know this is technically a PayPal problem, but they have other solutions around this that I think should be highlighted more in the docs.

newearthmartin commented 4 years ago

What you are pointing at is true. However, nothing prevents you from adding a check for the amount when you receive notice of the payment via IPN to see if the user tried to change this number.

Also, this is addressed in the new js paypal button, in which you call directly paypal apis to present the button. So the button comes directly from paypal itself. This is not implemented in django-paypal yet. I created an issue to start discussing implementation. https://github.com/spookylukey/django-paypal/issues/221

El dom., 23 feb. 2020 14:26, Kalob Taulien notifications@github.com escribió:

Hey there,

There's a problem with the button when using {{ form.render }} — the purchase value is editable through the DOM. The amount you want to pay can be changed to anything you want. Most often it's set to 0.01 to buy anything for one penny.

[image: image] https://user-images.githubusercontent.com/4743971/75116544-bb56df00-5626-11ea-8943-eb6be95b9d43.png

I know this is technically a PayPal problem, but they have other solutions around this that I think should be highlighted more in the docs.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/spookylukey/django-paypal/issues/224?email_source=notifications&email_token=AALP7QRJG5XAXXAPGC3E43DREKWVVA5CNFSM4KZ4KFX2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IPSKHTQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALP7QXNC6SPPER3ET4QDNLREKWVVANCNFSM4KZ4KFXQ .

spookylukey commented 4 years ago

There are clear warnings here that you need to check the data received:

https://django-paypal.readthedocs.io/en/stable/standard/ipn.html

For example, under point 5, and in the example code.

Are you looking for something more than that? Perhaps it could be clearer in some way? I'm open to suggestions.

newearthmartin commented 4 years ago

Ah! I didn't know it was a GET form. If so, we don't need to put the form in the template. We could just have a button that goes to a Django view and this view redirects to paypal with all the data, without the user being able to tamper the form.

El lun., 24 feb. 2020 11:05, Luke Plant notifications@github.com escribió:

There are clear warnings here that you need to check the data received:

https://django-paypal.readthedocs.io/en/stable/standard/ipn.html

For example, under point 5, and in the example code.

Are you looking for something more than that? Perhaps it could be clearer in some way? I'm open to suggestions.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spookylukey/django-paypal/issues/224?email_source=notifications&email_token=AALP7QVUZ6Q77XT7EILYPU3REPH4BA5CNFSM4KZ4KFX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMX4XWA#issuecomment-590334936, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALP7QXFBU6DBHO4JK3XWSLREPH4BANCNFSM4KZ4KFXQ .

spookylukey commented 4 years ago

If you do a redirect, the user will still be able to tamper with data (they can change the URL you direct them to). The correct way to do it is to ensure the data is what you expect at the end, without assuming you have prevented tampering.

I'm closing this ticket now, thanks.