umbraco / Umbraco.Commerce.Issues

18 stars 2 forks source link

Invalid/Broken HTML when trying redirect to stripe #532

Open marvingbh opened 5 days ago

marvingbh commented 5 days ago

Describe the bug Invalid/Broken HTML after redirect to /umbraco/commerce/storefront/api/v1/checkout/{orderId}/pay/{token}

Steps To Reproduce Steps to reproduce the behavior:

  1. Go to https://github.com/umbraco/Umbraco.Headless.Demo
  2. install front and backend
  3. setup UMBRACOCOMMERCECHECKOUT_MODE to Redirect
  4. Try to purchase something using stripe checkout

Expected behavior Redirect to Stripe and collect the payment

Screenshots

Additional context Using Umbraco 13.1, trying to do a headless checkout workflow with Hosted option (just redirect to payment), following the documentation, everything goes fine until the call /umbraco/commerce/storefront/api/v1/checkout/{orderId}/token, and redirect to the payUrl, the payURL is something like:

https://domain.com/umbraco/commerce/storefront/api/v1/checkout/2a0bfa0e-26e0-4e1a-ae38-0190746bcec4/pay/aa5973cd-cdaf-4d80-9770-9561909aa2de

Redirecting to the URL gives a broken HTML and it doesn't work. I already tried the same thing using the https://github.com/umbraco/Umbraco.Headless.Demo, it also has the same problem using the redirect option on UMBRACOCOMMERCECHECKOUT_MODE.

the HTML returned:

<!DOCTYPE html>
<html>

<head>
    <meta charset="utf-8">
    <title>Umbraco Commerce - Redirecting...</title>
    <script
        src="https://js.stripe.com/v3/"><script>
                var stripe = Stripe('pk_test_xxxxxxxxxxxx');
                window.handleStripeCheckout = function (e) {
                    e.preventDefault();
                stripe.redirectToCheckout({
                    sessionId: 'cs_test_xxxxxxxxxx'
        }).then(function (result) {
                    // If `redirectToCheckout` fails due to a browser or network
                    // error, display the localized error message to your customer
                    // using `result.error.message`.
                });
                return false;
    }
                <script></head><body><form action="https://domain.com/umbraco/commerce/payment/continue/stripe-checkout/2a0bfa0e-26e0-4e1a-ae38-0190746bcec4/ORDER-01645-61742-GPQPJ/b905d5b42dc26929a36594a6126879fb7a817297" method="post" id="form6235abd338fe4bbca11d98466d5ecb3a"></form><script>window.onload = function(event) { var frm = document.getElementById("form6235abd338fe4bbca11d98466d5ecb3a"); if (frm.hasAttribute("onsubmit")) {frm.dispatchEvent(new Event("submit", { cancelable: true })); } else {frm.submit(); } }</script>
    </body>

</html>

using the devtools console, with that broken page, and create the stripe object and do stripe.redirectToCheckout(), it redirect correct to stripe and I can make the payment.

can anyone help me?

Umbraco Commerce version: 13.1.4

mattbrailsford commented 5 days ago

Good spot. Looks like we aren’t closing some script tags correctly. I’ll take a look at fixing those tomorrow.

mattbrailsford commented 5 days ago

I've pushed a fix out for this in 13.1.5 released today 👍

marvingbh commented 5 days ago

@mattbrailsford one question about the behavior, now the HTML is correct, but is not redirecting to stripe, it is redirecting to checkout/confirmation after getting 302 in umbraco/commerce/payment/continue/stripe-checkout/xxxx

is that correct? I was expecting to be redirected to Stripe

here is the new HTML

<!DOCTYPE html><html><head><meta charset="utf-8"><title>Umbraco Commerce - Redirecting...</title><script src="https://js.stripe.com/v3/"></script><script>
                        var stripe = Stripe('pk_test_xxxxx');
                        window.handleStripeCheckout = function (e) {
                            e.preventDefault();
                            stripe.redirectToCheckout({
                                sessionId: 'cs_test_b1r9lOQRrYFS9GZpNX26bfpnsdmfBFRMEiNfMwSlAQQEsYDZt85DEVKFmR'
                            }).then(function (result) {
                              // If `redirectToCheckout` fails due to a browser or network
                              // error, display the localized error message to your customer
                              // using `result.error.message`.
                            });
                            return false;
                        }
                    </script></head><body><form action="https://domain.com/umbraco/commerce/payment/continue/stripe-checkout/efb2e4d3-b561-4a7d-95bf-0190789d942f/ORDER-01646-45714-WFSCY/536ea54b2caef1b4c7523a1ccbdfac1dfa5b1d34" method="post" id="forma9d2b043a3774c079706a241d6287bc7"></form><script>window.onload = function(event) { var frm = document.getElementById("forma9d2b043a3774c079706a241d6287bc7"); if (frm.hasAttribute("onsubmit")) { frm.dispatchEvent(new Event("submit", { cancelable: true })); } else { frm.submit(); } }</script></body></html>
mattbrailsford commented 5 days ago

Doh! Nope, looks like as well as the mallformed markup we also forgot to add the payment provider form attributes to the form. The form should have an onsubmit handler on it which should intercept the submit and trigger the stripe JS code.

Let me get a nightly build created for you to test.

mattbrailsford commented 5 days ago

Ok, there should be a 13.1.6-preview.2 on our nightly feed now that should now render the form attributes too. Are you able to give it a test and let me know if this fixes things for you?

You can find details about how to use the nightly feeds here https://docs.umbraco.com/umbraco-cms/fundamentals/setup/install/installing-nightly-builds

PS sorry for not testing this more thoroughly. I was already planning on getting 13.1.5 out today so thought fixing the malformed HTML was an easy fix to throw in but I should have really tested it to make sure there wasn't something else missing 🤦‍♂️

marvingbh commented 5 days ago

just tested it, and now it redirects correctly to Stripe, but after filling the card and making the payment is returning to checkout/error even with everything correct in Stripe side, that could be a different problem that I will investigate further. thank you very much!

mattbrailsford commented 5 days ago

Excellent (that the forms works) and yea, the redirect sounds like something more at Stripes end potentially, but do let me know if we appear to have anything else missing.

I'll look at pushing out a new patch release next week given I only just released 13.1.5 today, but you should be safe to run on the nightly till then.

marvingbh commented 5 days ago

just a heads up, this version is working fine in Windows, but in Linux it compiles but gets the above error when running, 13.1.5 works fine in both

Unhandled exception. System.Reflection.ReflectionTypeLoadException: Could not load all types from "Cms.Core, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" due to LoaderExceptions, skipping:
. System.IO.FileNotFoundException: Could not load file or assembly 'Umbraco.Core, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

. System.IO.FileNotFoundException: Could not load file or assembly 'Umbraco.Web.Common, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

Could not load file or assembly 'Umbraco.Core, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

Could not load file or assembly 'Umbraco.Web.Common, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

   at Umbraco.Cms.Core.Composing.TypeFinder.GetTypesWithFormattedException(Assembly a)
   at Umbraco.Cms.Core.Composing.TypeFinder.GetClassesWithBaseType(Type baseType, IEnumerable`1 assemblies, Boolean onlyConcreteClasses, Func`2 additionalFilter)
   at Umbraco.Cms.Core.Composing.TypeFinder.FindClassesOfType(Type assignTypeFrom, IEnumerable`1 assemblies, Boolean onlyConcreteClasses)
   at Umbraco.Extensions.TypeFinderExtensions.FindClassesOfType[T](ITypeFinder typeFinder, IEnumerable`1 assemblies, Boolean onlyConcreteClasses)
   at Umbraco.Cms.Core.Composing.TypeLoader.<>c__DisplayClass21_0`1.<GetTypes>b__0()
   at Umbraco.Cms.Core.Composing.TypeLoader.GetTypesInternalLocked(Type baseType, Type attributeType, Func`1 finder, String action, Boolean cache)
   at Umbraco.Cms.Core.Composing.TypeLoader.GetTypesInternal(Type baseType, Type attributeType, Func`1 finder, String action, Boolean cache)
   at Umbraco.Cms.Core.Composing.TypeLoader.GetTypes[T](Boolean cache, IEnumerable`1 specificAssemblies)
   at Umbraco.Cms.Core.DependencyInjection.UmbracoBuilderExtensions.AddAllCoreCollectionBuilders(IUmbracoBuilder builder)
   at Umbraco.Cms.Core.DependencyInjection.UmbracoBuilder.AddCoreServices()
   at Umbraco.Cms.Core.DependencyInjection.UmbracoBuilder..ctor(IServiceCollection services, IConfiguration config, TypeLoader typeLoader, ILoggerFactory loggerFactory, IProfiler profiler, AppCaches appCaches, IHostingEnvironment hostingEnvironment)
   at Umbraco.Extensions.UmbracoBuilderExtensions.AddUmbraco(IServiceCollection services, IWebHostEnvironment webHostEnvironment, IConfiguration config)
   at Umbraco.Extensions.WebApplicationBuilderExtensions.CreateUmbracoBuilder(WebApplicationBuilder builder)
   at Program.<Main>$(String[] args) in /home/marving/if/portal/src/cms-api/Program.cs:line 28
   at Program.<Main>(String[] args)
System.IO.FileNotFoundException: Could not load file or assembly 'Umbraco.Core, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

File name: 'Umbraco.Core, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'
System.IO.FileNotFoundException: Could not load file or assembly 'Umbraco.Web.Common, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

File name: 'Umbraco.Web.Common, Version=14.0.0.0, Culture=neutral, PublicKeyToken=null'
mattbrailsford commented 5 days ago

Hmmm, interesting. I wonder if it’s just having a problem with it being a prerelease. In theory it’s exactly the same apart from the small code change for the form attributes 🤔

Thanks for the heads up though 👍🏻