Customers pays for the wrong order

[kimhf]

Customers can end up paying for the wrong order when using the Vipps "Buy now" button on single products.

Browser caching is not the issue, but caching solutions like Kinstas Edge Cache (Cloudflare) will end up delivering a cached page for /vipps-buy-product.

Adding a query parameter to the url of special paged like this should bypass the issue.

[iverok]

This should not be the case, and we test this using varnish among others.

The page that starts the purchase, or presents the user with a terms-and-condition button uses no-cache-headers, which should ensure that page isn't cached.

The purchase is then started using Ajax POST, which uses a nonce, so that even if that page is cached, it should fail on check_ajax_referer.

The order to be completed is stored in the Woo session, which means that a cookie has been generated in this case, which should also normally stop caching.

If you have a reproducible case of this, more than one thing is wrong. It is true that adding a cachebuster query parameter could then help, but it's not uncommon for caches to ignore those as well (eg. to strip out campaign parameters and so forth), so I'd like to know more about how this is obtained.

[kimhf]

I believe this line could be the issue:

window.location.replace(result["url"]);

This will hit the cache.

If needed I can try and set up a test site where the issue is reproduced.

[iverok]

Yes, please do. The call in question will indeed load a page using GET, but that page should have headers like this:

Cache-Control: no-cache, must-revalidate, max-age=0
Expires: Wed, 11 Jan 1984 05:00:00 GMT

You should also have various woo cookies active (because there has been a session created) and the generated page will have something like this:

<input type="hidden" id="sec" name="sec" value="<whatever>">

-- with a nonce, which if cached, will not work when the user tries to start the purchase.

So for this to happen, the edge cache will have to ignore both cache headers, expire and the session cookie, but the error should then not be completing a wrong purchase - you should pretty much just get an error. Which is annoying enough of course.

[iverok]

I'm aware that there caches that are set up to ignore headers etc out there for speed reasons of course (wrong as this is, sometimes needs must) ; but I'd like to have an actual example of this happening so I know what to work with. For instance I've seen stripping query-parameters, which would be the suggested fix, as a way to cache more data. In that case, these pages would have to be listed as exceptions in the cache itself anyhow.

We have seen Cloudflare filter away callbacks to the wc-api, so thats something to be aware of.

[kimhf]

Here is a test site.

https://vippstest.kinsta.cloud/

I have now done a test with buying product A, product B and product C via the "buy now with vipps" button, in separate orders and individual private browser windows. The result was 3 orders with Product A. This is not necessarily clear to the customer before after the order has been submitted and paid. I can probably provide admin access if that is helpful.

I agree with you that this looks like it should work. Kinsta even clam they respect no-cache value in Cache-Control here https://kinsta.com/docs/edge-caching/#troubleshooting-edge-caching Still in reality it does not work.

The query variable solution can work to bypass some faulty cache setups, but not all. Of course I agree it should not be necessary in an ideal world. Still in this case it has serious consequences for the clients it affects, so maybe that redundancy is still a good idea.

I'm also following up this issue with Kinsta support to see if there are issues on their end. They can add url exceptions for specific cases. It's also possible that they can add the vipps url as a global rule, at least that is something they will consider. Still this does not seem like a fully good solution and we a guarantied to end up with similar issues in the future.

[iverok]

Thank you for reproducing - I'll check out what is actually being sent, and especially what happens with the nonce as soon as I can.

If this cache is written so as to ignore cache header rules, it probably special-cases known woocommerce pages. If so, you should be able to add more exceptions for the special Vipps pages, which are of course exactly the same as e.g. the woo checkout page in this regard.

If we add a cachebuster query arg, this may in fact still be stripped from the cache key (which happens so as to remove campaign trackers and that sort of thing; basically you may see cache setups that strips away everything except maybe ?s and ?add_to_cart because it "knows" that whatever these are, they aren't relevant for the backend.). So even that isn't a sure thing if the cache setup is this aggressive enough. Basically then the only way is to configure the cache to make exceptions for everything /vipps- -- at least the express checkout page.

There is also the problem of WAFs like mod_sec sometimes blocking URLs that look like they are passing sessions in GET arguments.

Another issue with using an extra query-arg is that each of these would be cached as well, which could be at least an annoyance depending on the TTL of the cached pages.

We can of course add a filter or an "advanced option" to do this though. But first I'd like to see exactly what is produced/cached.

[iverok]

So for this page, the cache-control has been replaced with "public, s-maxage=2592000". The Expires header is still in 19834, but this is ignored.

Is there any proxy between you and the cache, or any setting on your server that would replace the cache-control header like this?

[iverok]

Could you try to add this e.g. in your childthemes functions.php:

add_filter('nocache_headers', function ($headers) {
    $headers['Cache-Control'] =  'no-cache, must-revalidate, no-store, notransform, max-age=0, s-maxage=0';
    return $headers;
}, 99);

[kimhf]

I'm waiting for a reply regarding the changed headers. When the cache-control headers have changed, they are being served from the cache, and no longer from php. I expect the result of their investigation is that the cache is configured wrong.

The rest of the vipps urls already have a query parameter as far as I can tell, and non of them are being cached. From my experience the only urls with query parameters that might be cached in a typical WordPress cache setup is /?s=searchterm

Here is a simple test :
https://vippstest.kinsta.cloud/this-should-never-cache
vs
https://vippstest.kinsta.cloud/this-should-never-cache?bust

[kimhf]

I have added the nocache_headers filter and cleared the cache.

Test with those Cache-Control headers:
https://vippstest.kinsta.cloud/this-should-never-never-cache

[iverok]

Thank you. The payment page still gets

public, s-maxage=2592000

Just to be 100% sure: You don't have any plugins or configuration stuff that would overwrite these after WP but before the cache?

[iverok]

I tested adding a query-parameter and it disables caching completely, so in this case this would resolve the issue. We might add an option to do so depending on why this was cached originally, just to have a simple way to fix this for people affected.

As for the WP nonce, as long as you are the same user, it is unfortunately not nonce-y enough to avoid the "stored" purchase - it is valid for the same user for 24 hours or so.

As for query parameters, they may be stripped, cached or taken as indicators of uncacheable pages; depending on the application. "s" and "add_to_cart" obviously special.

[kimhf]

I can confirm that there is no other plugins, except for the hosting plugin and Twenty Twenty-One theme. The headers for the test urls above are not run through any filters.

The nonce does not seem to prevent other customers from buying the same order multiple times. You can argue that it is the cache that is configured wrong, but the plugin could maybe fail more gracefully.

Some other ideas could be:

• Load the form values via a api endpoint or ajax.
• Make sure the product is actually added to the cart, some cache solutions might be looking for the cart related cookies.
• Show the customer a summery of what product is being bought so they can discover the issue if it happens (many customers never noticed this before after having bought the wrong products).

Still I think the query param idea is by far the easiest to implement and would prevent this from happening in most cases, with few, if any, downsides. If they are in fact stripped and still cached for some reason, the worst that could happen is the status quo, and many many more plugins would be broken on that setup.

[iverok]

Well, of course the cache is wrongly configured: it ignores the nocache- and expires headers. Additionally, there is a session active at this point; that's ignored too. So that's not in question at all. (The product being purchased is loaded into the page from a WC()->session, so the page being cached was generated with cookies present.)

If you have this problem for a given cache, the normal procedure would be to add exceptions for pages that are wrongly cached using the caches' own configuration or tools. Most caches allow for this, since any number of applications may require it for some reason or other.

So the question here is if we should add a fairly sketchy workaround for bugs in other systems. As for downsides, that will always remain to be seen - for instance it is a thing that WAFs have rules that block URLs that look like they are trying to steal sessions. The decision must then come down to whatever requires the least support in the future.

I have to push a small change today anyway; so I'm adding a query-arg now; just for this bug. If it causes issues for other people, we'll probably remove it and replace it with a filter (so you can add your own.)

The new version with the workaround should be available shortly; it would be great if you could test out if it solves your issue.

[kimhf]

The changes to the plugin seem to do the trick. I'm not able to reproduce that issue now.

About possible pitfalls, I guess there is a slight chance that "nc" as the variable name could conflict with some plugin, but I guess the same could be said for the url itself.

The feedback I got from Kinsta is basically "will not fix", but we can request custom exclusions.

Did still get some useful information related to checkout plugins on their platform. By default most query strings are uncached and the woocommerce_items_in_cart cookie is uncached. Sub pages to checkout ( something like /checkout/vipps-buy-product ) should also typically be uncached.

[iverok]

Excellent.

Yeah, I think 'nc' should be safe. It seems adding a query string defeats caching completely, which works perfectly in this case; but I did a random id based on microtime and customer session anyway, just to be sure for other caches.

Custom exclusions would be the normal way of handling stuff like this if the choice is made to ignore cache-defeating headers, but the user should really have control over this, as there are many plugins that do a lot of weird uncacheable stuff. Also there are several other "express checkout" or "alternative checkout" plugins which also would need special treatment, as well as custom profile pages and stuff like that. But I guess none of those are my problem.

Adding endpoints to the Woo checkout pages is an interesting strategy and one that would avoid a lot of problems like this, and others too. It would entail making sure to be compatible with stuff like Klarna Checkout which may or may not be a challenge. But I'll keep it in mind as a potential strategy.

Just to be clear; I'm not saying that configuring a cache like this is wrong per se: If you know that your application is basically WooCommerce, you know that the only pages that are definitely dynamic would be the cart, checkout and 'my account'-pages. There is the minicart of course, but that can be special-cased by plugins (or handled by ESI), and you would want to disable "recently watched products" and not use dynamic pricing (or use appropriate plugins for them, or again, ESI).

You could then strip most GET arguments (leaving s and add_to_cart, or just add_to_cart if using something like algolia) to avoid being defeated by campaign trackers and the result would be a very resilient site where practically every page view would hit the cache.

This would then continue to work even if the store admin installs plugins that add cookies and other cache-breaking stuff. Those plugin would then maybe not work, but you'd keep your speed and uptime. However it should then be possible for a plugin author to simply state "please don't cache these pages if you want this to work".