[5.x] Fix secure flag for session cookies
#43882
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I've labeled the PR as release blocker because it's security relevant |
|
I have tested this item ✅ successfully on a459e59 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43882. |
1 similar comment
|
I have tested this item ✅ successfully on a459e59 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43882. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43882. |
4 tasks
|
My bad! Must have had something in mind at some point - but no clue what 3 years later :( |
dgrammatiko
pushed a commit
to dgrammatiko/joomla-cms
that referenced
this pull request
Aug 11, 2024
* Fix setting secure cookie flag for https-encorced sites * apply for all sessions --------- Co-authored-by: Quy <quy@nomonkeybiz.com>
dgrammatiko
pushed a commit
to dgrammatiko/joomla-cms
that referenced
this pull request
Aug 11, 2024
* Fix setting secure cookie flag for https-encorced sites * apply for all sessions --------- Co-authored-by: Quy <quy@nomonkeybiz.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of Changes
The rework of the session management introduced in 5.x broke the code that sets the
secureflag for the session cookie on sites with enforced HTTPS. The $options variable, containing the config isn't passed to the storage anymore, resulting in the absence of the flag regardless of the configuration state:84776fb#diff-bc8698a8418bcc017e622d13e0d460e94f502f09ae0f9c50cb6cd9f7ede73cb0L90
Testing Instructions
Enable "force SSL" on a 5.x site, inspect the session cookie using your developer tools. Verify that the flag is absent. Apply the patch and delete the cookie in your browser. Refresh.
Actual result BEFORE applying this Pull Request
No
secureflagExpected result AFTER applying this Pull Request
secureflag is set as expectedLink to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed