Description: ------------ Would it be possible to add the ability to destroy and recreate a session upon logon. Perhaps by specifying the option as part of the conf array: 'login' => array( 'method' => 'post', 'username' => 'handle', 'destroy' => true, // <<--- 'password' => 'passwd', 'force' => false, 'function' => '', 'remember' => 'rememberMe' ), Is this kind of thing handled in login already? I haven't seen it! -- Simon H Expected result: ---------------- The session should not be carried between the "not logged in" and "logged in" states. If a session is hijacked during a not logged in state, once the user logs in, the hijacker would have elevated privelages within the app.

if I remember correctly a login should always trigger a logout, so this would then cause a destroy if you set the logout parameter accordingly.

It doesn't appear to be triggering a logout during login as when I login I have the same session ID as I had before I logged in, and I do have destroy=true set for logout.

Have a look at what is going on in LiveUser::processLogout(). There the $logout param is set to true if login credentials have been passed to LiveUser indirectly (through the login parameters) or directly (via the factory or singleton).

I've had a look at this and I can see that it destroys the session, but processLogout() is only called in init() and logout() is only called in processLogout() when you provide a post or get or whatever [logout] == 1 say. The problem is that I'm trying to destroy the session during the LOGIN process which means that the logout request var is not set. Now, I did try adding the logout var to the login form as a hidden field, but hey, it just logs the user straight out, or maybe just doesnt log them in. So I still have the problem of the session ID carrying through from a not logged in state to a logged in state, i.e. through the login process. Am I just being stupid?

err .. processLogout() is always called. as you can see in processLogout() it checks if a handle/password have been set. if they are then logout() is called even if logout was orginally wasnt passed as true to LiveUser.

I did some debugging, and it would appear that in processLogout that $this->logout() wasn't getting called during a login (thus not resetting the session). Once I hacked processLogout for it to work it did what logout() should do...logged me out and redirected me to the redirect page. Therefore login was broken. So, so far I'd have to say that the current method or even how it is itended to work, isn't working, at least for me. What I've also discovered on my travels is that even though the session is destroyed in logout() the ID stays the same. What is required here is the use of: session_regenerate_id(); ...right after session_start() in logout(). Or even something like: session_start(); session_unset(); session_regenerate_id(); I think it makes sense to add this in also at the point of successful login, noting that the session vars are carried to the new ID and rendering highjacking based on sessionids useless! session_regenerate_id() has a dependancy of PHP 4 >= 4.3.2 and also new features for cookies as of 4.3.3... see: http://www.php.net/manual/en/function.session-regenerate-id.php...There are manual ways of acheiving the same thing obviously. Am I onto something, or still talking bobos?

Here's a patch that adds the flexibility to the conf array by adding 'destroy' and 'regenid' to 'login' and 'regenid' to 'logout'. Works a treat for me :-). --- LiveUser.php Mon Apr 05 03:07:55 2004 +++ LiveUser.php.new Mon Apr 05 03:10:49 2004 @@ -277,14 +277,17 @@ * 'password' => 'Form input containing password', * 'remember' => '(optional) Form checkbox containing <Remember Me> info', * 'function' => '(optional) Function to be called when accessing a page without logging in first', - * 'force' => 'Should the user be forced to login' + * 'force' => 'Should the user be forced to login', + * 'destroy' => 'Whether to destroy the session on login', + * 'regenid' => 'Whether to regenerate the session id' * ), * 'logout' => array( * 'method' => 'get or post', * 'trigger' => 'GET or POST var that triggers the logout process', * 'redirect' => 'Page path to be redirected to after logout', * 'function' => '(optional) Function to be called when accessing a page without logging in first', - * 'destroy' => 'Whether to destroy the session on logout' + * 'destroy' => 'Whether to destroy the session on logout', + * 'regenid' => 'Whether to regenerate the session id' * ), * // The cookie options are optional. If they are specified, the Remember Me * // feature is activated. @@ -860,7 +863,14 @@ return $res; } } - + if ($this->_options['login']['destroy'] == true) { + session_destroy(); + session_start(); + session_unset(); + if ($this->_options['login']['regenid'] == true) { + session_regenerate_id(); + } + } $this->freeze(); } else { if (!$auth->isActive) { @@ -870,6 +880,7 @@ } $counter++; } + return true; } @@ -1127,11 +1138,14 @@ $this->_options['session_save_handler']['gc'] ); } - // Set the name of the current session session_name($this->_options['session']['name']); // If there's no session yet, start it now - @session_start(); + session_start(); + session_unset(); + if ($this->_options['logout']['regenid'] == true) { + session_regenerate_id(); + } } // Finally, set the LoginManager's internal auth object back to

Uhm I dont quite understand why logout() wasnt called in processLogout() if you passed a handle/password? Especially since your patch doesnt seem to address any issues that would fix anything in that regard. Generally the regen sounds like a good feature to add. BTW: the @ in front of session_start is needed for older versions of PHP where otherwise we would get a warning.

Yeah, agreed, but regardless, if logout is called during the login via processLogout() in init() then logout does what it says on the tin...logs out and redirects based on the redirect set in conf. So my method of handling it in the login process seems to work in that respect.

Last bit got lost.... Do you want me to resubmit or email you the patch with the @ in front of session_start again (DOHH!!)? Do you think the patch will be added?

Well I would rather want to figure out why logout() is not being called, then the problem will be solved. Again adding the session regeneration as an option is of with me aside from that. So please take a look at processLogout(). As you can see there are several places where $logout is set to true. The relevant section for you is on line 1046 and following (LiveUser.php version 1.84 in CVS). This if block should set $logout to true. Ah! Could it be that you are directly passing the handle/password to the LiveUser factory, but you login method is set to true?

I am not passing login details through factory, I am using $_POST. This is where it should be getting called: if (isset($httpvar[$this->_options['login']['username']]) && $httpvar[$this->_options['login']['username']] && isset($httpvar[$this->_options['login']['password']]) && $httpvar[$this->_options['login']['password']] ) { $logout = true; } but it doesn't appear to be...I'll test some more. BUT, you're missing a key point...I still have the problem, that if logout() is called then it just logs me out and redirects me to the logout redirect page before getting a chance to tryLogin. So regardless of why this isnt working, running logout() during the login phase isn't a solution anyway. So even if I get processLogout working how will using logout during login work?

Uhm could you please figure out why this is not working? I really need to persist on this as you are either expieriencing a bug or its simply a user error. Regarding the problem with the redirect. I am not using redirects as my framework handles this in a more intelligent way. To me it seems like it might be a good idea tu supress the redirect if logout() is being called due to handle/password being passed and not due to logout being passed as true.

Sorry for the delay, been realy busy. I've tried everything to find out what the problem is here and it all falls down to strange behaviour. The following lines for some reason don't always kick in when they should: if (isset($httpvar[$this->_options['login']['username']]) && $httpvar[$this->_options['login']['username']] && isset($httpvar[$this->_options['login']['password']]) && $httpvar[$this->_options['login']['password']] ) { $logout = true; } So logout doesnt always get set. Now, when I but prints and die()'s into the code it shows that $logout isnt always getting set, or the above if statement isnt always validating right. I just can't figure it out. Maybe its my environment, or something, but what I do know, is that my patch below makes it work explicitly. Sorry I can't be more help, but I'm sure at least some of the suggestions here if not the whole patch is worth a look. Has anyone else seen this behaviour?

Ok I have added the regenid feature. I decided to use the existing destroy option as regenid is done inside the destroy if in my implementation. so instead of 'destroy' => true // default yu would do 'destroy' => 'regenid' As for why the logout is not performed might it be related to the fact that sometimes you are using post and sometimes get? anyways maybe using the new 'method' => 'request' setting will solve that problem?

This extension to destroy on logout is welcomed, but I'm just as (if not more) concerned about regenid on logon. Would it be possible to implement this?

well .. a logon should initiate a logout .. if it doesnt then we have a bug .. unfortunately I cannot reproduce the bug. however I dont think we adre doing ourselves a favor if we add code to LiveUser that covers up this bug so I guess you would have to figure out why no logout is initiated on login ..

Hi Lukas Right, I've had another look at this is my understanding of how it works: During login... In init, processLogout() is called ($logout=false). The first check, in processLogout(), checks if the user wants to logout by looking for the logout method and the logout trigger in $_REQUEST and friends. Since I haven't set the trigger $logout = false; The second check, in processLogout(), checks if the user is logged in by checking the session for the "auth_name", so again, if I'm not logged in these are not set - $logout=false; So, since $logout=false, logout() won't get called during login, thus my session won't get regenid'd. I've set both login and logout to "request" just in case. Can anyone confirm that logout() is getting called during login? The easy way to do it is set ['destroy'] = 'regenid'; and look for the sessionid changing during login. I still have an issue though, even if this did work...in logout() we can redirect using header(), now if I have this set, I will get redirected on every login. If we're gonna stick with calling logout during login then there must be a work around or suggested method to redirect ONLY on a proper logout. Perhaps just calling the redirect if the logout trigger is set, something like: if (!empty($this->_options['logout']['redirect']) && (isset($httpvar[$this->_options['logout']['trigger']]) && $httpvar[$this->_options['logout']['trigger']]) ) { header('Location: ' . $this->_options['logout']['redirect']); exit(); } I'm open to suggestions or will work offline with anyone to get this fixed. Thanks for your patience! -- Simon H

This bug has been fixed in CVS. In case this was a documentation problem, the fix will show up at the end of next Sunday (CET) on pear.php.net. In case this was a pear.php.net website problem, the change will show up on the website in short time. Thank you for the report, and for helping us make PEAR better.

I've updated to the latest cvs and tested this. I've had a look at the code and the logout seems to be working ok, but I still don't see $this->logout() getting called during a login. During login... In init() processLogout gets called which doesnt call logout() because we are not already logged in, nor triggering a logout. Also further in init we check again to see if the user is logged in already before trying to logout. So, everywhere logout() is called its requiring that we're already logged in or trying to logout, but what we want, is to logout (without setting $direct) on login, even if we're not already logged in - regenerating the session id. Thus avioding the session being hijacked in an unprivelaged state and the gaining escalated rights as the user logs in and keeps the same sessionid. I still think it would be nice to have this as an option in the conf array ['login'] destroy options as in ['logout']. Thanks for the changes so far tho :-). Simon H

This bug has been fixed in CVS. In case this was a documentation problem, the fix will show up at the end of next Sunday (CET) on pear.php.net. In case this was a pear.php.net website problem, the change will show up on the website in short time. Thank you for the report, and for helping us make PEAR better.

Lukas Why did you remove the regenid from logout when putting it into login? I would have thought that its just as useful to regenid on logout, or least causes no harm leaving the feature in. -- Simon H

I removed it because I dont really see a point in it being there. It just makes things more complicated and can result in multiple regen's. Essentially on logout your session id is no longer a security risk and on the next logon you will get the new id again. So I dont really see a potential security risk here anymore

any more comments on this one ?

Sorry, I just havent had a chance to look at the changes commited yet. Will check it out later this evening or tomorrow.

Yeah, this is cool. Thanks for the persistance!

This bug has been fixed in CVS. In case this was a documentation problem, the fix will show up at the end of next Sunday (CET) on pear.php.net. In case this was a pear.php.net website problem, the change will show up on the website in short time. Thank you for the report, and for helping us make PEAR better.