-
-
Notifications
You must be signed in to change notification settings - Fork 450
Fix admin login for non-secure cookies #4960
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes admin login issues when using non-secure cookies by preventing the SameSite attribute from being set to "None" for non-HTTPS connections, which is incompatible with browser security requirements.
- Adds logic to conditionally remove SameSite=None for non-secure cookies in session handling
- Introduces constants for SameSite cookie values to improve code maintainability
- Updates cookie handling to use consistent constant references instead of hardcoded strings
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
app/code/core/Mage/Adminhtml/Model/System/Config/Source/Cookie/Samesite.php | Adds constants for SameSite values (NONE, STRICT, LAX) and updates option array to use them |
app/code/core/Mage/Core/Model/Cookie.php | Updates cookie methods to use SameSite constants and improves type hints and documentation |
app/code/core/Mage/Core/Model/Session/Abstract/Varien.php | Adds conditional logic to remove SameSite=None for non-secure cookies and fixes namespace references |
tests/unit/Mage/Core/Model/CookieTest.php | Adds comprehensive unit tests for the Cookie model functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Only one notable change: delete param and fallback to browser default. if (!$cookie->isSecure() && $cookieParams['samesite'] === CookieSamesite::NONE) {
// PHP doesn't allow to set SameSite=None for non-secure cookies
unset($cookieParams['samesite']);
} Merged. |
Description (*)
Setting cookie SameSite to "None" breaks admin login for unsecured pages.
Related PR