umbraco / Umbraco.Commerce.Issues

18 stars 2 forks source link

Stripe Checkout V13.1.0 - Dictionary values aren't working for public / secret keys, or webhook keys #535

Open karlitros opened 3 days ago

karlitros commented 3 days ago

Describe the bug We’ve got a requirement in building a store for a customer, whereby they have multiple Stripe accounts, and they want to allow users in different countries to pay in to their different stripe accounts. There are US-specific, UK-specific and Australia-specific Stripe accounts, and we have one Umbraco Commerce Store.

We have Stripe Checkout set up as a payment provider, and would like to use the dictionary values button to specify different web hook, secret and publishable keys for each Stripe account, but this doesn’t seem to work. If I just specify a normal publishable, secret and web hook key, the payment goes through fine, but if I attempt to use dictionary values, I get an error in the logs, like the attached.

Steps To Reproduce Steps to reproduce the behaviour:

  1. Create more than 1 website, and assign a different culture to each.
  2. Create a single commerce store, and associate the same store with both websites.
  3. Create a store payment method for Stripe Checkout.
  4. Click the little dictionary button next to the public / secret / web hook keys, and create region specific keys for each.
  5. Attempt to pay.

Expected behaviour You should be able to pay whichever stripe account that's associated with the given culture website, and the web hook should also be successfully called at the end of the transaction, to convert the cart item to an order.

Screenshots

image image image image

Additional context We’re using Umbraco version 13.3.2 and with Stripe Checkout v13.1.0

Umbraco Commerce version: We're using Umbraco.Commerce v13.1.1

mattbrailsford commented 3 days ago

Hey @karlitros

I had a quick investigatory look to see if I could spot the issue and I think I see what the problem is.

So Umbraco Commerce uses the orders language to "translate" the settings, however, for payment providers that use a global webhook like the Stripe provider does, the webhook handler doesn't initially know which order the request is for. To resolve an order from a global webhook, payment providers implement a GetOrderReferenceAsync where they process the webhook body and extract order meta data that then updates the payment provider context and actually translates the values in time for the main ProcessCallbackAync method call.

The problem is, in the Stripe payment providers webhook handling, it also attempts to make a connection with the Stripe API, which, because during the GetOrderReferenceAsync method call we don't have an order, the settings passed to this method are not translated.

So we are in a sort of chicken and egg situation.

Looking at the code for the Stripe provider, the main reason we need to API credentials is for this block

https://github.com/umbraco/Umbraco.Commerce.PaymentProviders.Stripe/blob/main/src/Umbraco.Commerce.PaymentProviders.Stripe/StripePaymentProviderBase.cs#L203-L225

For each event type we re-fetch the entity, even though it is embedded in the webhook. The reason we do this is because the webhook model versions don't always match the version of the SDK library being used which can cause errors and so instead we just ignore the embedded version and fetch it using the SDK library (You can see this documented in the code on line 196)

So it looks like our options are:

1) We ditch looking up the models via the SDK and do something else to extract the entity models that doesn't require a connection and thus ensure the GetOrderReferenceAsync method never needs any settings. 2) Find some other way of inferring the right "translation" of the settings to use, though I'm not sure what that would be or how we can do that

mattbrailsford commented 1 day ago

On second thoughts, given the Stripe accounts are country specific not really language specific, a better option might be to set up the Stripe payment method three times (‘Stripe - US’, ‘Stripe - GB’ and ‘Stripe - AU’) and configure the “allowed in countries” option to only allow their use in their prospective country.

This would allow you to set the api credentials without needing to use dictionaries and so they should work fine. It would just mean you’d have to setup 3 webhook handlers at Stripes end but I assume you’re having to do that anyway and pointed them all to the same endpoint.

karlitros commented 1 day ago

Thanks for looking into this Matt. Good idea on separate payment methods, I’ll have a crack at that.

Is it worth the suggestion of removing the dictionary buttons from those fields in future releases to prevent the confusion?

thanks again!

mattbrailsford commented 12 hours ago

I thought that very same thing last night too so I think that’s a good suggestion. I think in reality it should only be fields that relate to the front end such as the URLs you return to that need to be dictionary inputs.

I’ll take a look at swapping those out 👍🏻