search

vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
Other
5.69k stars 1.01k forks source link

Standardised Order Getting in Payment Plugins / RequestContext / Stripe Plug-in Bug #1517

Closed Sean12697 closed 2 years ago

Sean12697 commented 2 years ago

Is your feature request related to a problem? Please describe.

I am currently attempting to implement the Stripe Payment Plug-in and I am facing issues with it receiving the Order from the context, I'm slightly uncertain if this is to do with my specific Storefront implementation, although I have successfully integrated the Braintree plug-in in the past. This has lead me to look back at how the order is being fetched from the Braintree plug-in and to compare it to the Stripe plug-in.

Braintree:

https://github.com/vendure-ecommerce/vendure/blob/6050279860c280252138904e77fdf580c002fa3b/packages/payments-plugin/src/braintree/braintree.resolver.ts#L29

Stripe:

https://github.com/vendure-ecommerce/vendure/blob/6050279860c280252138904e77fdf580c002fa3b/packages/payments-plugin/src/stripe/stripe.resolver.ts#L14

Clearly the Stripe implementation is more secure, since there are less vectors for a user to supplier a mismatching Order ID, especially if the UuidIdStrategy has not been implemented for the Vendure entityIdStrategy.

Describe the solution you'd like A standardised and secure method of obtaining an Order within the Payment Plug-ins, being Stripe's if it does work and my Storefront is configured incorrectly and isn't sending relevant details to product the RequestContext (specifically ctx.session) on the Vendure Backend; or Braintree's if the Stripe solution (getOrderFromContext()) does not work.

Describe alternatives you've considered Documentation describing why there are two different approaches taken and the reason why orderID is required from the API in the Braintree implementation and cannot be received from the context.

Sean12697 commented 2 years ago

I have now forked and modified the Stripe Plug-in with success by making it look more like the Braintree intervention, using orderService.findOne() with the relevant imports and extending the shopApiExtensions schema with (orderId: ID!), although this might not be the best long term solution, if it is, I can make a PR to the Stripe Plug-in so changes are up-streamed and are not just based on a fork.

michaelbromley commented 2 years ago

Hi! Thanks for the report! You are right - the Braintree resolver should not need the orderId since it can be derived from the context as in the Stripe and Mollie plugins. There is no good reason to pass the ID and as you point out, this is a potential security issue.

If you are not able to receive the order from the context, this suggests that maybe something is off in the session handling. Does the cart work otherwise? Usually when there is a session issue, adding multiple items to the order results in a new order being created each time.