Wit all due respect, no.. this works perfectly as intended.
I have been using it and testing it a lot over the last 24 hours, changing it from 1 minute to 10 minutes and it is perfect. It may not be the most elegant solution, but it works perfectly.
You may be missing the part of the IF statement.. this assures that is the session has registered any activity during the allowed time, it will skip the destroy.
I like this a lot..
Thanks
I had to go back to my original solution, putting Brian's solution in place means that when the session should be expired you get a nasty double load page, since headers had already been sent to page (not sure which since it is really fast) but there was this white screen with black letters showing for an instant, I assume that in a slow connection they will show for a longer time.
The redirect, we do not need to instruct, since the framework itself is self-correcting and will send the page to the default page (index in my case).
The comment Brian put "Otherwise if it “times out” when a user is not logged in, and you try clicking on a secure page, it just does the redirect." I have not experienced this and do not understand clearly what he means. What I have seen is that the 'time out' is measured against the value in the Session, and then it does the destroy.
You could at this point add a normal header redirect (the redirect class is still not available since I put back at the top of the init) and have even more control on where to send the person, it cold be a page designed for logged out people, that is what I have just done.
Maybe something else is different? I'm not sure where the double redirect or redirect after headers sent would come from.
if (isset($_SESSION['LAST_ACTIVITY']) && ((time()-$_SESSION['LAST_ACTIVITY']) > $site_settings->session_timeout) && $user->isLoggedIn()) {
session_unset(); // unset $_SESSION variable for the run-time
session_destroy(); // destroy session data in storage
Redirect::to($us_url_root);
}
$_SESSION['LAST_ACTIVITY'] = time(); // update last activity time stamp
Destroying the session of a non logged in user doesn't make sense, and that is the only additional check being done ($user->isLoggedIn(). The Redirect to $us_url_root simply reloads the page, and in effect generates a new "logged out session".
I am not the brightest person here so, could you please explain how or why it would not make sense to destroy it? If that was the case, because since the user is not logged in, this count would never happen.
Maybe what I am trying to say is that to me, that validation ($user->isLoggedIn() ) is redundant and would never be used since that user is not logged already, but if the user happens to open the browser again before the garbage collection removes his session the other part would catch it, or am I missing something?
As for the headers, it is very easy to test if you implement your solution and set the timeout to like 10 you can test several times. I do not have debugging capabilities so I can not follow the execution, maybe you can and find where the double redirect happens.
Thanks for the quick reply. I have incentive to get this right the first time since I'm working on US5 code, so myself "just being right" doesn't help anything. I had a flakey internet connection yesterday when I was working on this, and that may have impacted some of the behaviors I was seeing.
I have removed the $user->isLoggedIn() check and the redirect, and "in general" things are still working as expected. That being said, when I am not logged in, and I wait the session_timeout duration of 10 seconds (usually 15 or 20 seconds to be safe) I saw yesterday, and occasionally today that I would get a token mismatch error when I would try to go immediately to login. The second behavior I saw was that while logged out, if the session timed out, my next mouse click would not take me to the right page, though it has not been consistent that I have seen that.
On another side, I figured that the session doesn't need to be destroyed when no one is logged in, so it would not accomplish anything to destroy the session when a user isn't logged in. But I don't have anything firm supporting that.
Lastly, when I was looking at it yesterday, I recalled seeing a Stackexchange discussion indicating that those session unset and destroy functions would destroy the server-side data, but that it still persisted for the given page view until a new page was requested, hence I think why I added the redirect to reload. But it is entirely possible that action is entirely unnecessary.
In conclusion, I'm of the opinion that I think what you have is fine, but just keep an eye out for strange behaviors (extra clicks needed, token errors, etc) so that if it does become a problem, I have a feeling it is somewhat related to some of these session activities.
I love it when people can talk without getting upset  .. don't you think it is fantastic?
Appreciate your patience with me and I will keep an eye open for those problems you mention.
I only mentioned this because I honestly think that the 2 things I have said would be of great benefit for the framework, which again, is great and I am deeply grateful for the great job done.
I am an avid user of session data, in all the programs I have done, I use it extensively, so much as to pass data from one page to others that may need it down the road. Like remembering a selection you did once that may be required later, like for instance the zip code of your favorite pharmacy (again, I LIVE in the healthcare industry) so if you get to have to select a Doctor, I do the search based on that zip that you already selected in an entirely different section of the code.
Again, thanks, you guys rock!
Can you summarize the "2 things I have said"? I just want to capture here the outcome so it is clear since this thread is referenced in the bug/feature report.
Sure..
1.- This issue about controlling the 'forced log out' time.
2.- The selection of redirection page upon login, be able to configure it instead of sending people to their profile.
3.- Noted that the page validation sequence in the sample page is commented thus rendering the security useless. ( <?php //if (!securePage($_SERVER['PHP_SELF'])){die();} ?> ) so it can be fixed for future release.
I have also talked about the database abstraction layer and the possibility of using something like adodb which is vastly used and very well documented albeit it seems to go into de-use.
I think this covers the whole of my interaction with you guys
Okay, so this thread was about 1) and is in US5 already
2) I have covered elsewhere and is in US5 already
3) I think you pointed this out to @mudmin already. The sample pages will sort of be gone in US5, but instead an example website demonstrating how to implement various things, customize menus, styling, etc. so that should be a non-issue in US5.
|
