@bx80 opened this Pull Request on January 4th 2022 Contributor

Description:

Fixes #18538

When using the page overlay feature, the notifyParentIframe call intermittently gets sent without a session cookie. This is quite random and only occurs approximately once in every 20-30 calls. It appears to be a race condition related iframe access to cookies.

Because there is no session cookie then a new session is started and the response to the notifyParentIframe sets a new session cookie which invalidates the session, this is apparent when the loadSidebar call is made shortly after and the token_auth is rejected and the user is forcibly logged out.

Without a solution to the race condition issue this PR is a workaround to prevent the session breaking. It's not possible to check the session cookie via javascript so a server side check is implemented which will remove the set-cookie header when responding to a notifyParentIframe without any cookies.

To recreate:
1) Create some visits to a test site clicking between one or two pages.
2) Open the page overlay view
3) Set the browser console to view network requested filtered by notifyParentIframe
4) Click back and forth between the pages that were tracked
5) Eventually the sidebar will show a 'session expired' error.
6) Check the last notifyParentIframe call in the console, it will be missing a request session cookie and will receive a set cookie for a new session

With this fix the notifyParentIframe call will still be made without a session cookie, but no set cookie will be returned in the response and the session will not break.

See also L3-197

Review

@sgiehl commented on January 18th 2022 Member

I tried to reproduce the issue for a while, but wasn't able to with my normal setup. So I started to have some deeper thoughts about this issue. I excluded a possible browser bug, so it needed to be something within Matomo that let's the browser struggle with the session cookie.

So I took a closer look at the requests that are sent by the Overlay. All requests that are sent to index.php are actually setting the session cookie. But there is one big difference those requests are having.
We have a special handling for the samesite cookie attribute implemented for all requests that are going to the Overlay module:
https://github.com/matomo-org/matomo/blob/9873cb71e66be0f80839c76e923c3b866dd23b46/core/Session.php#L166-L183

The overlay page is sending multiple requests to the API. Such requests are actually setting the value to lax, while the requests going to the overlay module are setting the value to none.

That wasn't a problem in my setup as the tracked url and matomo were both running on the same domain. So I set up another vhost to have different urls for matomo and the tracked site. Using this set up it was easy to reproduce the problem.

So the problem actually is that depending on the request that last finished, before the notifyParentIframe request is sent, the samesite attribute is set to lax or none. With none everything works fine, but if the attribute is lax the iframe can't access the cookie, so it is not sent with the request, causing a new session to be started.

Your workaround would actually solve that, but it's not really a proper solution. I'll close this PR and create a new one with a better fix soon.

This Pull Request was closed on January 18th 2022
Powered by GitHub Issue Mirror