Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cookie collisions from PR # 14116 and improve overall security #14403

Merged
merged 4 commits into from Jul 3, 2019

Conversation

@mvorisek
Copy link
Contributor

commented Jun 28, 2019

Fix cookie collisions introduced in #14116 and several security improvements.

Questions Answers
Branch? develop
Description? See commit descriptions.
Type? bug fix / improvement
Category? CO
BC breaks? no
Deprecations? no
How to test? No new functionality, no need for extra tests. You should be able to login & continue to navigate in FO and BO

This change is Reviewable

@mvorisek mvorisek requested a review from PrestaShop/prestashop-core-developers as a code owner Jun 28, 2019

@prestonBot

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2019

Hello @mvorisek!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@mvorisek

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

How to get details about this test failure https://travis-ci.org/PrestaShop/PrestaShop/jobs/551683586 ?

@matks

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

How to get details about this test failure https://travis-ci.org/PrestaShop/PrestaShop/jobs/551683586 ?

Unfortunately that's a false positive, we've had a lot recently (see #14384). I restart the build

@mvorisek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

Tests passed. Can someone review and merge?

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Hi,
Can you please complete the description of your pull request with this template: https://github.com/PrestaShop/PrestaShop/blob/develop/.github/PULL_REQUEST_TEMPLATE.md

I'm in favor of making this change but the only problem I see is if someone already have a _COOKIE_IV_ defined in his configuration he can't connect anymore. Same with _COOKIE_KEY_.
WDYT @Quetzacoalt91 @matks ?

Kind Regards

@mvorisek

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

@PierreRambaud I have added the PR template. Yes, all existing cookie will not be honored/discarded. But only once (only once when site owner updates the PS core).

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

You're right, don't know why my session won't get back when I tried 🤔

@matks

matks approved these changes Jul 2, 2019

@matks matks added this to the 1.7.7.0 milestone Jul 2, 2019

@sarahdib sarahdib added QA ✔️ and removed waiting for QA labels Jul 3, 2019

@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Thank you @mvorisek

@matks matks merged commit 2ff3488 into PrestaShop:develop Jul 3, 2019

2 checks passed

PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mvorisek mvorisek deleted the mvorisek:fix_cookie_collisions branch Jul 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.