Skip to content

Conversation

@joost-florijn-kega
Copy link
Contributor

@joost-florijn-kega joost-florijn-kega commented Mar 19, 2018

The headers 'Cache-Control' and 'Pragma' need to be replaced otherwise they will be ignored because ZendHttp will always return the first entry.

https://github.com/zendframework/zend-http/blob/master/src/Headers.php#L257

This fixes issue #14049

Preconditions
It happens on a fresh Magento 2.2.2 installation via composer.
PHP 7.1
MySQL 5.6
Steps to reproduce
Installed Magento 2.2.2 via composer
Go to frontend and register a new customer. Logoff and login with this customer.
Open a new browser or use another device to repeat the step (2).
Under device 1, open a new tab and go to http://localhost/customer/section/load/?sections=&update_section_id=true
Using device 2, go to the same URL, http://localhost/customer/section/load/?sections=&update_section_id=true
Logoff from device 1 and 2. Execute the URL under both devices.
It is important to create more than 1 user browsing the store in order to get the problem.

During some tests, we confirmed this issue under live stores, that affects the store header that contains the customer name.

Actual result
It is possible to get another customer session information from sections controller, without timestamp.
Sometimes, this url is trigged from knockout without timestamp, and shows another customer name in the store header.

Expected result
Always return the user data for the current logged user.
In case of no session, return no data from cache.

Suggestion
Enforce session id check in order to avoid send cached data to another session.
It works with timestamp url hint, but for sensitive data, it is easy to decrement the timestamp and start to get customers who logged and visited the store.

The headers 'Cache-Control' and 'Pragma' need to be replaced otherwise they will be ignored because ZendHttp will always return the first entry.

https://github.com/zendframework/zend-http/blob/master/src/Headers.php#L257
@miguelbalparda
Copy link
Contributor

@magearhiopterecs it seems this is touching something you worked on, care to review?

@magearhiopterecs
Copy link

@miguelbalparda
Thank you!
It looks like you're right.

@miguelbalparda
Copy link
Contributor

@joost-florijn-kega can you please forwardport this to 2.3? Thanks!

@magento-engcom-team magento-engcom-team added this to the April 2018 milestone Apr 3, 2018
@sidolov sidolov changed the base branch from 2.2 to 2.2-develop April 4, 2018 19:29
@sidolov
Copy link
Contributor

sidolov commented Apr 4, 2018

Hi @joost-florijn-kega , we can't accept PRs to 2.2 branch, I changed your base branch to 2.2-develop

@magento-engcom-team
Copy link
Contributor

@joost-florijn-kega thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-team magento-team merged commit f3bd0c2 into magento:2.2-develop Apr 5, 2018
@sidolov
Copy link
Contributor

sidolov commented Apr 6, 2018

Hi @joost-florijn-kega , consider porting this solution to other Magento versions.
Recently our team introduced a simple way to port fixes across versions, details available here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants