vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
MIT License
5.42k stars 953 forks source link

Bug in Stripe paymentIntent.create() triggered with express checkout + normal checkout #2364

Open yasserlens opened 10 months ago

yasserlens commented 10 months ago

Describe the bug The stripe Service's createPaymentIntent() attaches metadata to stripe's paymentIntent.create() call. The metadata is not guaranteed to be the same for the same order at different stages of the checkout journey. This causes the following error: Keys for idempotent requests can only be used with the same parameters they were first used with. Try using a key other than 'SVHLKDQ3DHMA6SPT_9000' if you meant to execute a different request.

The reason is as follows:

To Reproduce Steps to reproduce the behavior:

Keys for idempotent requests can only be used with the same parameters they were first used with. Try using a key other than 'SVHLKDQ3DHMA6SPT_9000' if you meant to execute a different request.

Expected behavior The idempotency key error shouldn't be triggered when calling createPaymentIntent multiple times for the same order.

Environment (please complete the following information):

Additional context See pull request to fix this below

yasserlens commented 10 months ago

cc @michaelbromley since you added the idempotency key

yasserlens commented 10 months ago

OK Here's the issue: When checking Stripe logs, I can see that the payment intent requests being generated contain different information for the same idempotency key. This is because the metadata generated isn't the same:

Here are possible solutions: 1) Remove customer info from the submitted metadata (namely, customerEmail and shippingAddress). I think this is required for express shipping methods as explained above. 2) Or, Remove the idempotency key (I wouldn't go this path since it may cause issues I'm not aware of)

yasserlens commented 10 months ago

Created a pull request that should fix this https://github.com/vendure-ecommerce/vendure/pull/2366

michaelbromley commented 10 months ago

I'm copying over some relevant parts of the conversation from the attached PR:


What I'm not understanding is that by default, the options.metadata function is undefined. This means that unless you yourself are defining a metadata function which returns the customerEmail and shippingAddress keys, how is this being included in the metadata that gets sent to Stripe?


Great Catch - Michael. I actually do define this metadata in Stripe's config when setting up (in vendure-config.ts). The reason why they're their is to make sure customer details are attached to the payment, so we could find the customer payment in Stripe if needed given shipping info.

We can remove this and this should solve the issue without going with this change. However, keeping such metadata which can change from one call to another for the same order could result in this happening again to other Stripe plugin users. Do we want to keep this metadata in the paymentIntent.create() call?


So I think we should still allow people to send whatever metadata they like, but maybe we need to handle the idempotency key differently. What if we base the idempotency key off not only the order code and total, but also a hash of the metadata?

yasserlens commented 10 months ago

Yeah this sounds like a reasonable solution, since metadata can represent the state of the order at the time the payment intent is created.

any preferred library you used in Vendure to generate hashes? I can submit a PR for this. Let me know.

michaelbromley commented 10 months ago

You can use the built-in Node crypto module. Here's what ChatGPT suggests, which I think is along the right lines:

const crypto = require('crypto');

function hashObject(object) {
  const hash = crypto.createHash('sha256'); // You can choose a different hashing algorithm if desired
  hash.update(JSON.stringify(object));
  return hash.digest('hex');
}

// Example usage
const myObject = { name: 'John', age: 30, city: 'New York' };
const objectHash = hashObject(myObject);
console.log('Hash:', objectHash);