search

isotope / core

Core repository of Isotope eCommerce, an eCommerce extension for Contao Open Source CMS
https://isotopeecommerce.org
135 stars 107 forks source link

Ungültiges Anfrage-Token #2154

Closed Bear30 closed 4 years ago

Bear30 commented 4 years ago

Hallo,

ich habe bei 2 Shops unter Contao 4.9, Isotope 2.6.12 das Problem, dass beim Reinlegen eine Produkts in den Warenkorb die Contaofehlerseite aufgeht. Da steht dann

"Ungültiges Anfrage Token". "Das Request-Token konnte nicht validiert werden."

Klickt man auf den Link landet man wieder im Produkt. Manchmal geht es dann beim 2. Versuch sauber weiter, manchmal aber auch nicht.

Im Log-File steht nichts. Der Fehler tritt bei jedem Browser auf.

Könnt ihr das bitte mal bei euch nachstellen, denn das sind Produktivshops.

Grüsse Bernard

Bear30 commented 4 years ago

Oh ich seh gerade der andere Shop hat Isotope 2.6.13. Der Fehler bleibt aber derselbe.

Bear30 commented 4 years ago

Ich habe nochmals etwas herumgespielt

Bear30 commented 4 years ago

Das Problem scheint an Contao 4.9.3 zu liegen.

Bei einem Varianten-Shop hängt es auch bei der ersten Auswahl einer Produktvariante mit der "Lade Produktdaten". Auch hier scheint das Cookie-Setzen schief zu gehen.

Ich bin bei dem betroffenen Shop jetzt zur 4.8.8 zurück. Hier läuft alles ohne Probleme. Auch der Token-Fehler ist weg. Nummer 2 ist jetzt wieder auf Contao 4.4. Der läuft jetzt auch wieder.

fritzmg commented 4 years ago

Ich kann das Problem in Contao 4.9.3 nicht bestätigen.

Bear30 commented 4 years ago

@Fritzmg: Hast Deinen Browsercache und dei komplette Browserhistorie auch wirklich gelöscht? Chrome geht bei mir nämlich scheinbar davor auch ... erst nach dem Löschen kommt der Fehler!

Die 4.9.4 hat das "Ungültiges Anfrage Token" auch. Die Variantenendlosschleife kann ich aktuell nicht prüfen, da ich die Shops auf 4.8 bzw. 4.4 zurückgesetzt habe. Das sind alles Produktivshops. Die müssen laufen!

4.8.x und 4.4.x gehen übrigens jetzt ohne Probleme.

fritzmg commented 4 years ago

Browser Cache, ja, History, nein. Hat auch nichts mit beidem zu tun. Wenn dann mit den Cookies.

Bear30 commented 4 years ago

OK, das war ungeschickt ausgedrückt von mir. Nach dem Löschen der Cookies sieht man das Problem ...

Bear30 commented 4 years ago

Was macht eigentlich dieses csrf_https-contao_csrf_token?

Das hat beim 1. Aufrufen der Produktliste nachdem Löschen der Cookies vor dem Klick in den Warenkorb und dann Auftreten des Token-Fehlers den (zufälligen) Wert csrf_https-contao_csrf_token:PsZI8sFNIVKRrDuv96HCpZ4nyc7EnJfQdxGUmRfptgQ

Geht man dann auf den zurück-Btn des Browser bekommt es den neuen Wert csrf_https-contao_csrf_token:F5MK5G0rq9B2Rz4vVWUflFLkqhweX9fcvkfg_UFdX90 und dann geht es ohne Fehler durch den Bestellvorgang. Das csrf-Cookie ändert ab dato dann seinen Wert nicht mehr

Ein Klick auf den Link im Token-Fehler führt nur zum Erneuten Auftreten des Fehlers. Das hilft also nicht weiter. Offensichtlich scheint dieses 1. csrf-Token das Problem zu sein

fritzmg commented 4 years ago

Was macht eigentlich dieses csrf_https-contao_csrf_token?

https://en.wikipedia.org/wiki/Cross-site_request_forgery

Da Isotope die Session benutzt sollte das CSRF Token Cookie eigentlich bei jedem Request gesetzt werden. In deiner Instanz ist das aber aus irgendeinem Grund nicht der Fall.

Bear30 commented 4 years ago

Grad gesehen: Die 4.8.x ändert das Token auch beim 1. Aufruf

csrf_https-contao_csrf_token:k8oOp7f1SNYwWQrOcxiOKlXlzvFYzm6zg9UPVlyzx6s csrf_https-contao_csrf_token:fw2eVwgUsUQUveuiClp1Tw3_aoTY-EyFTNcH1Dufm-U

Es läuft dort aber trotzdem ohne Fehler

Bear30 commented 4 years ago

Hallo,

ich habe gerade testweise einen der betroffenen Shops auf die 4.9.5 geupdatet da ich gesehen hatte, dass in den Contao-Tickets was am Cookieverhalten angepasst wurde. Die Fehlermeldung im Chrome bei geleertem Browsercache bleibt dieselbe: Ungültiges Anfrage-Token

Bitte kümmert euch da mal drum!

fritzmg commented 4 years ago

Bitte kümmert euch da mal drum!

Contao so wie Contao Isotope ist Open Source Software. Du kannst dich gerne entweder selbst daran beteiligen das Problem zu finden, oder jemand anderen damit beauftragen.

Bear30 commented 4 years ago

Coole Antwort! Ich werde wohl mir zukünftig verkneifen hier auf ein Problem hinzuweisen ...

netzarbeiter commented 4 years ago

Open Source funktioniert nur, wenn sich alle beteiligen

aschempp commented 4 years ago

Da das Problem (scheinbar) nur bei dir besteht (da sich sonst niemand dazu meldet, auch nicht unsere Isotope Circle Mitglieder), ist es schwer einen Fehler zu finden. Idealerweise müsstest du einen Contao/PHP Entwickler den Fehler auf deinem System analysieren lassen, denn nur so lässt sich herausfinden an was es liegen könnte. Ev. ist es auch das Zusammenspiel mit anderen Erweiterungen, z.B. einem Cookie-Consent.

HenryVolkmer commented 4 years ago

Hi,

bear's Bugreport is valid.

Symfonys CSRF-Protection is managed via SessionTokenStorage which store its Token in a PHP-Session. For each POST-Request made without a PHPSESSID-Cookie (even if a CSRF-Token or REQUEST_TOKEN was sent in Request), these Steps are fired:

  1. send Cookie "csrf_contao_csrf_token=deleted; ..."
  2. send Cookie "PHPSESSID=...."

Isoptop utilisize Haste\Http\Response, which use SymfonyResponse(), and here is the Problem:

The Cookie in 1. (deletion) is not configured properly:

The misconfiguration causes a Browser-Warning in Firefox V79:

 Cookie “csrf_contao_csrf_token” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. To know more about the “sameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite

A correct configuration would be

What is the Impact?

The xhr-object treats a Warning as error, therfore onload will not be fired (no ajaxSuccess() and Isotope.hideBox() without onload). The Browser will still show the "Loading Product"-Message and nothing else is going to happen.

Reproduce:

  1. Logout from Backend, clear Browsers Cookies- and Cache-Storage (Alternative: create a new Firefox-profile)

  2. head to reader-page (GET, non XHR) inspect Cookie: csrf_contato_csrf_token (SameSite LAX,Secure:false)

  3. make an xhr-request (for simplicity jquery is enabled)

xhr = new XMLHttpRequest();
form = jQuery('form');
xhr.open(form.attr('method'),form.attr('action'));
xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded');
xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
xhr.send(form.serialize());

Result

Counter-Check

implement an onerror listener here:

// assets/js/isotope.js Line 205
xhr.onerror = function() {
    console.log('fired due a error or a warning');
    ajaxSuccess(xhr.response);
};

Fix

fritzmg commented 4 years ago

The Cookie in 1. (deletion) is not configured properly:

  • Samesite: none
  • Secure: false

That should not be a problem any more in Contao 4.4.51 (assuming you have regenerated the app.php and app_dev.php) and 4.9.5.

HenryVolkmer commented 4 years ago

The Cookie in 1. (deletion) is not configured properly:

  • Samesite: none
  • Secure: false

That should not be a problem any more in Contao 4.4.51 (assuming you have regenerated the app.php and app_dev.php) and 4.9.5.

Haste instantiate Symfony's HttpFoundation directly without DI (and bypass Contao-Things), so: its a problem in all Versions of Contao and SF.

fritzmg commented 4 years ago

Yes, but the CSRF Token Cookie is not set there. It is set when you request the product detail page. And that is the problem @bear30 has (or originally had?) - in his Contao instance the CSRF Token Cookie is not set for some reason.

HenryVolkmer commented 4 years ago

You are right, no Haste-Problem.

The CSRF-Token is set if a request satisfys this. Otherwise, the Cookies are removed (clearCookie()) which causes the misconfigured Cookie-Deletion.

An inital GET-Request to the Reader-Page without any Cookies nor a PHPESESSID does not satisfy requiresCsrf() and leads to the (CSRF)-Cookie-Deletion.

Since http-foundation 4.4, it is possible to pass a samesite param for Cookie-Deletion, for earlier Versions, i suggest to ensure the requiresCsrf()-Requirements by providing the PHPSESSID.

fritzmg commented 4 years ago

When using Isotope, this condition should be satisfied, as Isotope does use the session in various places.

Although... possibly, if don't have a cart module for example in your page layout, may be a session is in fact never started.

fritzmg commented 4 years ago

But I still don't get why the AJAX Request would fail, if the CSRF Token Cookie is not present. a CSRF Token is not required for AJAX Requests.

HenryVolkmer commented 4 years ago

if no User is logged and no CSRF-Token nor a PHPSESSID was sent, a CSRF-Cookie with value deleted (deletion-cookie) will be sent.

Thats it.

fritzmg commented 4 years ago

Yes, but the question is, why that is causing a problem.

HenryVolkmer commented 4 years ago

Because Symfony < 4.4 doesnt Supports Samesite-Propertys for Deletion-Cookies (Back in this Time, this wasnt a Issue: no Browser-Warnings). Its not a Backend-Issue more than a Modern-Browser-Issue with deprecated Backends.

fritzmg commented 4 years ago

Then the issue should be fixed in Contao 4.4.51 and 4.9.5, if it is really only because of the browser warning. But why does the browser warning occur in the first place, if no cookie is set?

HenryVolkmer commented 4 years ago

Because, if no csrf is required, all Cookies are cleared by Contao which leads in to Response with an new Cookie:

Set-Cookie: csrf_contao_csrf_token=deleted; expires=Tue, 20-Aug-2019 18:11:53 GMT; Max-Age=0; path=/; httponly

which causes the Browser-Warning, which stops isotope.js.

fritzmg commented 4 years ago

which leads in to Response with an new Cookie:

Which Cookie would that be?

Set-Cookie: csrf_contao_csrf_token=deleted; expires=Tue, 20-Aug-2019 18:11:53 GMT; Max-Age=0; path=/; httponly

That's the Cookie that got deleted.

HenryVolkmer commented 4 years ago

Browsers interpret the Set-Cookie header as command to set a Cookie, regardless of the content (deleted in our case). The fact, that this Cookie has no SameSite=Lax-Attribute or is not secured causes a Browser-Warning.

Btw. there is no "delete cookie"-Response-Header. For this reason, we make existing Cookies invalid by setting nonsense Data (deleted).

I propose to ensure a Session in Module.php:

\Session::getInstance()->set('isotopeecommerce',true);

I commit a PR tomorrow, good night!

fritzmg commented 4 years ago

@HenryVolkmer which Contao Version are you using?

Ensuring a session would not be the correct way to fix this.

HenryVolkmer commented 4 years ago

Im running contao/core-bundle 4.8.8. Why Do you think, an ensured Session would not fix the issue? (it would, because i tested it). How would you implement a Fix?

fritzmg commented 4 years ago

Im running contao/core-bundle 4.8.8.

As already said, you need to update to Contao 4.9.5.

Bear30 commented 4 years ago

fritzmg is right. The 4.9.5 works for me.

HenryVolkmer commented 4 years ago

Of course its NOT working with 4.9.5 or even Contao dev-master. Because, as already said, contao sends a (for current browsers) malformed Cookie which causes a Browser-Warning.

@bear30: you got an Session-Start and a PHPSESSID-Cookie by:

or using a different (older?) Browser, or suppress Warnings in Browser.

fritzmg commented 4 years ago

Of course its NOT working with 4.9.5 or even Contao dev-master. Because, as already said, contao sends a (for current browsers) malformed Cookie which causes a Browser-Warning.

That should not happen as this was specifically fixed in Contao 4.9.5 (and 4.4.51).

HenryVolkmer commented 4 years ago

In order to fix it in Contao (and sf http-foundation >= 4.4), the removeCookies() sould be modified:

...
use Symfony\Component\HttpFoundation\Cookie;
...
if ($this->isCsrfCookie($key, $value)) {
   /**
    * @see https://github.com/symfony/http-foundation/blob/v4.4.11/ResponseHeaderBag.php#L257
    */
    $sameSite = $isSecure ? Cookie::SAMESITE_NONE : Cookie::SAMESITE_LAX;
    $response->headers->clearCookie($key, $basePath, null, $isSecure,true,$sameSite);
}
...

Can we please have an Session-Ensurement in Isotope? There are no BC's.

fritzmg commented 4 years ago

In order to fix it in Contao (and sf http-foundation >= 4.4), the removeCookies() sould be modified:

That should not be necessary, since the secure flag is set.

Can we please have an Session-Ensurement in Isotope? There are no BC's.

Why would you want that? You should definitely not start a session, if you don't need a session.

Bear30 commented 4 years ago

In 4.9.5 now it works fine. Also if i clean cache.

I had in 3 shops 2 problems. First was this version 4.9.4 with this cookie-error, second error was a line with a invalid css command in my css file. I didn't understand why this css error maked in the 4.9.5 this "Ungültiges Anfrage Token". 4.8.x, 4.4.x, 3.5.x had worked over years with this css error. Now it fixed.

Thank you fritzmg!

HenryVolkmer commented 4 years ago
  1. we need to Satisfy requiresCsrf()
  2. i don't care how we do it, but it must be done (either by opening a Session or sending a Nonsense-Non-CSRF-Cookie)
  3. The Session-Approach is reusable (Isotope will need a Session on certain points)
  4. the secure-flag is a bool which means, it can (and will be in most dev-envs) FALSE
  5. the secure-flag will always be false if the Contao-installation is requested by a Reverse-Proxy
  6. i want to speak to your Manager ;)
HenryVolkmer commented 4 years ago

@bear30 in current state, its technically not possible.

fritzmg commented 4 years ago
  1. we need to Satisfy requiresCsrf()

Why do you want to satisfy that?

  1. the secure-flag is a bool which means, it can (and will be in most dev-envs) FALSE

If you are not connected via https, then you also do not get the warning. When connected via https, cookies need to either have secure: true or sameSite: lax.

  1. the secure-flag will always be false if the Contao-installation is requested by a Reverse-Proxy

Only if you did not configure your environment properly. But if that is the case, you will have more problems.

HenryVolkmer commented 4 years ago

This is wrong. All responses with Cookie (SameSite=none) will throw a Warning in modern Browsers. An Exception are https-secured Connections: if a connection is secure, you can set the Secure-Flag. If you sent an Cookie (SameSite=none) over an unsecured Connection (immensely common for Reverseproxys), you will get a Warning.

Please refer to MDN

samesite

HenryVolkmer commented 4 years ago

@aschempp, @Toflar, @blairwinans, @leofeyer whats your Opinion about this issue?

Bear30 commented 4 years ago

In 4.9.5 for me all works perfect with isotope and the 4.9.5. By the way my shops are live, new orders are placed last days and nobody reports any order error to me. In 4.9.4 i had many error reports via email! That's all i can say ...

HenryVolkmer commented 4 years ago

@bear30 when its okay for you, could you please provide a Link to a Product-Reader-Page?

Bear30 commented 4 years ago

https://isybau.info/kaufen/isybau-xml-typ-sh-algr.html

HenryVolkmer commented 4 years ago

@bear30 ty!

The connection is secure (https), therefore cookies get the secure-Attribute which is in Combination with SameSite=false totally fine (No Browserwarnings).

fritzmg commented 4 years ago

All responses with Cookie (SameSite=none) will throw a Warning in modern Browsers.

Does Contao 4.4.51+ or 4.9.5+ send a Cookie with SameSite=none without Secure? If yes then that's probably bug. In any case, this discussion is unrelated to Isotope and you should open a ticket here: https://github.com/contao/contao/issues

HenryVolkmer commented 4 years ago

@fritzmg YES. Contao send Cookies with SameSite=none without Secure! This is, what i reporting in several ways in this thread, were my statements so incomprehensible? I mean, i even provided a Bug-Fix for contao in this thread.

HenryVolkmer commented 4 years ago

related https://github.com/isotope/core/issues/2146