User tests: Successful: Unsuccessful:
Code Review awaiting PLT decisions.
Part of joomla-framework/session#55 that fixes issues where REMOTE_ADDR changes, which then forces a user to be logged out.
Possible solution for joomla-framework/session#54 and thus #35244 that fixes #35244 (comment) CloudFlare (and other reverse proxy's that set REMOTE_ADDR as their IP and not the clients IP (which is perfectly valid for a proxy to do that, but causes session logout issues as REMOTE_ADDR frequently changes.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Title |
|
Labels |
Added:
?
|
I agree with this PR and I have to add that it's necessary regardless of whether a site is behind a proxy / load balancer / CDN.
My 4G connections hops between different IPs every few minutes or whenever my phone switches between cell towers. This happens far more frequently than you think. My bedroom is covered by one cel tower, my office (at the other end of the house) by another. Line of sight to both from each respective window. Because of weather conditions I may even be stationary but my phone switches between cell towers. This means I can never stay connected logged in to my Joomla 4 blog when I am on 4G, e.g. when the VDSL connection goes down.
Speaking of my VDSL, my ISP gives a DHCP lease of 3600 seconds and changes my IP every hour, presumably as a means to dissuade running a server from the VDSL connection. Needless to say, this causes my session to reset on each DHCP lease anniversary, i.e. once every hour.
Tying sessions to IP addresses is not something that should be done by default. It's entered around the assumption that your IP will not change throughout a session's lifetime which is unrealistic out here in the real world. It works on an Intranet most of the time (assuming the DHCP leases are long enough) or when you are given the option to pay for a static IP but doesn't quite work with mobile connections and most cabled ISPs. Considering that sessions are a fundamental requirement for providing services to our sites' visitors having these validators is an anti–feature.
Removing these validators is, therefore, a good idea. If people get iffy about it we could always have Yet Another Global Configuration Option (disabled by default) to enable them. And, please, let's not pretend that this project particularly cares about Global Configuration options proliferation when a feature that should have been a plugin (block FLoC, which is a technology that's not even deployed yet) warrants its own Global Configuration option...
a feature that should have been a plugin (block FLoC, which is a technology that's not even deployed yet)
and probably never will be
My comment for evidence that this should be merged can be found in the framework upstream pr joomla-framework/session#55 (comment)
@arnepluhar may you please do a test a mark it sucessful? https://issues.joomla.org/tracker/joomla-cms/35509
I have tested this item
I tested using a Joomla 4.0.3 site behind CloudFlare.
IMHO the commend of @arnepluhar in #35509 should also count as a successful test. He did it as a GitHub code review. The rules about what constitutes a valid test should expand to GitHub code reviews; these didn't even exist when the rules were decided upon back in the day and they make FAR MORE SENSE than comments filed on a PR!
@HLeithner sorry.
I actually did the same like @nikosdion…
I have tested this item successfully on e8d6784
I tested using two Joomla 4.0.3 sites behind CloudFlare.
@HLeithner sorry.
I actually did the same like @nikosdion…
I have tested this item successfully on e8d6784
I tested using two Joomla 4.0.3 sites behind CloudFlare.
I have tested this item
I have tested this item successfully on e8d6784
I tested using two Joomla 4.0.3 sites on different domains behind CloudFlare.
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-09-24 07:02:06 |
Closed_By | ⇒ | HLeithner | |
Labels |
Added:
?
|
Wow not it's a commit message....
IMHO the commend of @arnepluhar in #35509 should also count as a successful test. He did it as a GitHub code review. The rules about what constitutes a valid test should expand to GitHub code reviews; these didn't even exist when the rules were decided upon back in the day and they make FAR MORE SENSE than comments filed on a PR!
Github is configured to allow maintainers merge only if all tests are successful.
Thanks all.
Github is configured to allow maintainers merge only if all tests are successful.
And yet maintainers merge all the time without any testing!
@HLeithner GitHub tells me that he approved the changes which is the same message I see when we do GitHub code review on our own repositories ??♂️ I don't have any permissions on this repo so all I can tell you is what I see on the public GitHub interface.
Since you know it's build long before github has such features. The github repository is configured that all tests have to be green that a "normal" cms-maintainer can merge pull request. RL and Org admins can override this.
@nikosdion and @HLeithner … My behavior should not be the reason for any trouble here. I was not aware of issues.joomla.org before I was mentioned here. Before I "approved" the change in Github, as I thought this was the way to submit test results, but my comment on that was not exhaustive anyway.
Maybe somebody can clarify the difference between test and approval, if there is any and link the test procedure from time to time directly to the issue-discussion. Doing so, people who are facing the referenced issues are maybe more likely able to submit test results comprehensively.
Approval = result of code reviews in most cases, and a test in the issue tracker is normally the result of a real test following provided testing instructions.
We require 2 human real tests in any case, regardless if there are approvals or not.
@arnepluhar everything is good, no trouble
For administrators it looks like this on merge:
Maintainers can not merge this PR as it doesn't have all required green check marks^^
issues.joomla.org is an application used to provide the "HumanTestResults" and has some alternative features, it's more or less an alternative frontend for github with some extras.
we probably need to improve the JTracker to count reviewers as a human test
I often review code but I dont always test everything so I dont submit a tested successfuly
Hello, I waited to update to 4.0.4 to activate cloudflare and avoid problems with the sessions, however the problem still exists, just a little different, the session is not destroyed and the user maintains its normal section, BUT the problem is that If the user closes his session and immediately tries to log in again, he throws the detestable invalid token, even waiting for a few minutes! The only option is for the user to open another url of the same domain, let's say he opens another article and from the form he enters again, but from the main path of the domain he cannot enter through the invalid token.
I have enabled the back of the balancer and I even checked and version 4.0.4 already has the lines removed
use Joomla\Session\Validator\AddressValidator;
use Joomla\Session\Validator\ForwardedValidator;
$session->addValidator(new AddressValidator($input, $session));
$session->addValidator(new ForwardedValidator($input, $session));
The sessions are managed by the database, although despite the documentation it has never been clear to me if it would be better to manage it by files
Hello, I waited to update to 4.0.4 to activate cloudflare and avoid problems with the sessions, however the problem still exists, just a little different, the session is not destroyed and the user maintains its normal section, BUT the problem is that If the user closes his session and immediately tries to log in again, he throws the detestable invalid token, even waiting for a few minutes! The only option is for the user to open another url of the same domain, let's say he opens another article and from the form he enters again, but from the main path of the domain he cannot enter through the invalid token. I have enabled the back of the balancer and I even checked and version 4.0.4 already has the lines removed
use Joomla\Session\Validator\AddressValidator; use Joomla\Session\Validator\ForwardedValidator;
$session->addValidator(new AddressValidator($input, $session)); $session->addValidator(new ForwardedValidator($input, $session));
The sessions are managed by the database, although despite the documentation it has never been clear to me if it would be better to manage it by files
@Stuartemk could you please make a new issue, because it seems not to be the ip validation and if I understand you correct. Joomla doesn't destroy the session on logout?
That sounds more like normal GET request caching - the whole point of CloudFlare. The page where your login form is, is probably cached aggressively by CloudFlare.
If you have a dynamic site (I.e, stuff changes (like a security token) or you have forms and logins (protected by a security token) then you cannot cache the GET requests with CloudFlare and you need to correctly set up CloudFlare for your use.
Hello, I waited to update to 4.0.4 to activate cloudflare and avoid problems with the sessions, however the problem still exists, just a little different, the session is not destroyed and the user maintains its normal section, BUT the problem is that If the user closes his session and immediately tries to log in again, he throws the detestable invalid token, even waiting for a few minutes! The only option is for the user to open another url of the same domain, let's say he opens another article and from the form he enters again, but from the main path of the domain he cannot enter through the invalid token. I have enabled the back of the balancer and I even checked and version 4.0.4 already has the lines removed
use Joomla\Session\Validator\AddressValidator; use Joomla\Session\Validator\ForwardedValidator;$session->addValidator(new AddressValidator($input, $session)); $session->addValidator(new ForwardedValidator($input, $session));
The sessions are managed by the database, although despite the documentation it has never been clear to me if it would be better to manage it by files
@Stuartemk could you please make a new issue, because it seems not to be the ip validation and if I understand you correct. Joomla doesn't destroy the session on logout?
Hello, it seems to me that I already found the problem but not the solution, thank you very much for the answer, it provided a clue where to look.
That sounds more like normal GET request caching - the whole point of CloudFlare. The page where your login form is, is probably cached aggressively by CloudFlare.
If you have a dynamic site (I.e, stuff changes (like a security token) or you have forms and logins (protected by a security token) then you cannot cache the GET requests with CloudFlare and you need to correctly set up CloudFlare for your use.
Thanks for answering, but not in this case it is not a Cloudflare configuration problem, it is a Joomla problem, more specifically about the cache, I will open an issue giving the details.
specifically about the cache
Well yeah, if you have page cache plugin enabled then again, this would be the expected behaviour.
específicamente sobre el caché
Bueno, sí, si tiene habilitado el complemento de caché de página, este sería el comportamiento esperado.
Should it be an expected behavior that using any enabled cache option does not destroy the token on logout? I do not believe it.
It is expected behaviour that the "System - Page Cache" plugin CACHES THE WHOLE PAGE, including any token in the HTML and for this reason you should not enable that plugin, but enable the caching in Joomla Global Configuration that DOES take tokens into account.
It is expected behaviour that the "System - Page Cache" plugin CACHES THE WHOLE PAGE, including any token in the HTML and for this reason you should not enable that plugin, but enable the caching in Joomla Global Configuration that DOES take tokens into account.
What you are saying is that it is an expected behavior if the cache plugin is enabled because it captures the tokens, and that enabling the cache in the general configuration does not capture the tokens, so there would be no problem when enabling the cache in the general configuration. with the cache plugin disabled, but the error occurs in all possible configurations where at least one of the two caches is enabled global configuration or plugin or both.
Even though REDIS is managing the sessions #35971 (comment)
My stalker is back here too