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.

Sign up for GitHub

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

Jump to bottom

Fix for #7044: use secure cookie on HTTPS servers #7061

Closed
wants to merge 2 commits into from
Closed

Fix for #7044: use secure cookie on HTTPS servers #7061

wants to merge 2 commits into from

Conversation

smz
Copy link
Contributor

@smz smz commented May 29, 2015

Description

This should fix #7044 by using a secure cookie when the server is an HTTPS server.

Test instructions

  • verify that there is no regression on normal servers by using a multilingual site, switchnig to a non-default language, exiting the browser, re-visiting the test site with the naked domain and observing that you are redirected to the non-default language you had previously selected.
  • observe how the language cookie is delivered on HTTPS servers

@smz smz mentioned this pull request May 29, 2015
Closed
@smz
Copy link
Contributor Author

smz commented May 29, 2015

Of the two testing instructions I personally could test the first one only as I have no HTTPS server at hand.
@andrepereiradasilva, can you please perform the second test?

@smz
Minor fix...
ec4d4c4 
As we do that for the other parameters, let's use a temp local variable
for the last parameter too
Made cookie expire time variable parameter name consistent with its
definition
@Bakual
Copy link
Contributor

Bakual commented May 29, 2015

Code review looks fine, thanks! 👍

@andrepereiradasilva
Copy link
Contributor

In HTTPS the cookie is delivered in secure mode!
image

thanks!

@Fedik
Copy link
Member

Fedik commented May 29, 2015

I think, would be good idea to use "httponly" cookies always ... not only for ssl
this will protect cookies from access from a js scripts and make joomla more secure
Protecting Your Cookies: HttpOnly

ignore me, it for different issue

@andrepereiradasilva
Copy link
Contributor

yes, httponly is a good security practice too, so javascript can't read the cookies.
Is more used in session cookies to prevent XSS attacks but is always a good practice.

@smz
Copy link
Contributor Author

smz commented May 29, 2015

Personally I don't think this is necessary at all: in the worst case a minor data leak could happen ("someone" could know about your language preferences, and that's it). On the other hand I can envision a scenario where a legit JS could be willing to access the language cookie for good reasons, so for me it is... 👎

@andrepereiradasilva
Copy link
Contributor

yes, agree

This was referenced May 30, 2015
Closed
Closed
@Bakual Bakual added this to the Joomla! 3.4.2 milestone Jun 3, 2015
@Bakual Bakual added the RTC This Pull Request is Ready To Commit label Jun 3, 2015
@Bakual
Copy link
Contributor

Bakual commented Jun 3, 2015

Merged into staging. Thanks!

@Bakual Bakual closed this in 469a4ad Jun 3, 2015
@smz smz deleted the LanguageFilterHttpsCookie branch June 5, 2015 02:04
johanjanssens pushed a commit to joomlatools/joomla-fork that referenced this pull request Jun 19, 2015
@smz @johanjanssens
[fix] Use secure language cookie on HTTPS servers. Made cookie expire…
a3c6095 
… time variable parameter name consistent with its definition. Fixes joomla#7061.
@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Joomla! 3.4.2
Development

Successfully merging this pull request may close these issues.

Usage of cookies in plugin language filter
5 participants
@smz @Bakual @andrepereiradasilva @Fedik @zero-24