Closed bborman22 closed 1 year ago
FangedParakeet
commented
1 year ago Just a heads up, but I've renamed this issue from a POC to an MVP, because the scope of our efforts is much greater than a puny prototype and we are consequently expending as much energy to possible to create an end-product that may be limited in its initial ability, but that is still robust enough to be released and reused.
timur27
commented
1 year ago @FangedParakeet There's one thing regarding the 3DS authentication for credit cards we'd need to find the best approach to handle.
For the legacy card (non-UPE), the flow looks the following:
A pop-up authentication window allows users to authenticate or fail the transaction. On the 7th second, there is a very quick URL update which then disappears, but it essentially looks following:
As you can see, payment intent and client secret are attached to the URL. The payment intent and secret are parsed and used by this method. Surprisingly, the intent is then confirmed by the front-end implementation here.
The UPE with deferred intent creation does not inherit this piece of implementation. I have concerns about just copying this approach to the deferred intent creation, as we want to avoid exposing payment intent on the client side as part of the project.
Instead, for the deferred intent creation, we currently provide the return_url for the card gateway the same way we do for all the non-card payment methods (acf9e3ba06f985d602adf92911f2558d3b3ba70a and 1cafab750ad191d9fa4ab8c353297e4c4f233467), which allows us to avoid processing intents/client secrets on the client side, but the flow looks a bit different since Stripe acts differently when the return_url is part of the request.
On the 8th second, you might notice that the URL has some client secret. I'm not sure if we can do anything to avoid this. A big advantage of this approach is we don't handle intents/secrets in the client scripts, and it also works almost out-of-the-box. I'd like us to refine the advantages and drawbacks of both solutions and choose the path forward. I'm leaning towards the second option but unsure if we shouldn't confirm with a broader audience.
FangedParakeet
commented
1 year ago There's one thing regarding the 3DS authentication for credit cards we'd need to find the best approach to handle.
This is a great callout. This is part of the 3DS flow, where if the status on the intent is requires_action after we attempt to confirm it, we attach the intent ID and client secret to the current page URL, which is consequently captured on the front-end, and eventually we parse these details and use them to confirm the intent on the client.
AFAIA, this is now the only remaining flow where the intent is still confirmed on the client. Client-side confirmation here is required, because we need to rely on Stripe JS to present the authentication modal before we can complete the transaction (I believe that's what this call to confirmCardPayment is achieving): this is similar to the original UPE flow where we require Stripe to take action and lose a little control in the process.
I'm not sure that there is an obvious workaround for this scenario, without creating a rather large refactor of this payment flow. It's worth noting that when public key encryption is enabled, this attack is restricted since the client secret is concealed. I would suggest that this could eventually be enforced, though our own settings state that "it's not necessarily a good idea to have this enabled at all times," although, I am unaware why exactly that might not be such a good thing to do.
Personally, I don't believe this resolution should be a part of this MVP issue, since it already affects the legacy card element. It's a reasonably slim edge case, since a MITM attacker would only be able to take advantage of this on transactions with cards that already require additional authentication. Moreover there is a present defence with our encryption implementation. It's probably worth discovering why this is not a permanent solution and, if truly not, what other recourse we may have at our disposal, but I think this has a broader scope than our shortcode checkout...but let me know if you feel otherwise!
timur27
commented
1 year ago @FangedParakeet Indeed, the 3DS authentication and ways to handle it are the broader scope. However, I'm curious if we see the potential in the second solution (which is now implemented in https://github.com/Automattic/woocommerce-payments/pull/5869 and can be verified by testing the PR). The second recording here demonstrates how it works. This solution allows us to confirm the intent as we do for the UPE methods, automatically eliminating the need to process the client's payment intent/secret data. I like its simplicity, but as mentioned before, I'm still unsure how hard we want to stick to the pop-up authentication window instead of the separate page for the 3DS card authentication.
I'd appreciate hearing your opinion on this. I'm happy to take care of this if we need to consult anyone else. I think that implementing it as the second recording demonstrates here can be included to the current scope of the MVC, whereas building handlers for the payment intent confirmation on the client could be considered as a candidate for a separate ticket.
FangedParakeet
commented
1 year ago However, I'm curious if we see the potential in the second solution (which is now implemented in https://github.com/Automattic/woocommerce-payments/pull/5869 and can be verified by testing the PR). The second recording https://github.com/Automattic/woocommerce-payments/issues/5770#issuecomment-1513440779 demonstrates how it works.
That's quite interesting: I didn't realise the UPE offered us another solution there. Honestly, I don't really know enough about the development of our existing 3DS implementation or any pitfalls of alternate approaches. I think it would be beneficial to seek some feedback to this approach in the #wcpay slack channel or perhaps take it to P2, if you feel your solution requires a more in-depth explanation.
On the 8th second, you might notice that the URL has some client secret. I'm not sure if we can do anything to avoid this.
I'm not certain that we need to do anything here, since at this point the payment is complete and the intent is essentially used and disposable...but again, I'd defer to the wisdom of others before stating this with much confidence. 😅
timur27
commented
1 year ago I confirmed that the pop-up modal is a preferred option and the redirect should be avoided (more details in p1681887162644549-slack-CGGCLBN58). I will push 3DS authentication handling to the PR by tomorrow.
Description
Please refer to pc2DNy-2e2-p2 for more details.
Please refer to this file for the current description of this new payment flow.
If you scroll down to the section titled "Server-side confirmation", you will see that this almost exactly matches our current payment flow for our legacy card element, where the payment intent is created and confirmed simultaneously and subsequent to creating the payment method. This will allow us to resolve almost all of our current security and integration concerns with our existing UPE implementation.
This issue exists to create the initial PoC implementation of this new payment flow, once the tool has been released to beta and we are given access to it by Stripe. This issue is simply to implement the basic shortcode checkout payment flow and there will be other issues created to support other checkout and payment flows. It's also assumed that this tool will be in a fluid state while in limited beta release, so this PoC will not need to be considered in a final state once completed.
Acceptance criteria
Additional context
Stripe UPE – Options to solve WooPay compatibility and WC Pay loss of control over the payments UI/UX New integration path for Payment Element