PHP-Auth icon indicating copy to clipboard operation
PHP-Auth copied to clipboard

Automatic logout

Open dotsensei opened this issue 3 years ago • 7 comments

Has anyone implemented an automated logout with this library? I.e. logout if there is no user activity in 15-30 minutes - logout him, and redirecting to the login page. I would appreciate some guidance. Thank you.

dotsensei avatar Mar 11 '21 13:03 dotsensei

Sounds useful, so i've made a Gist. Feel free to modify it in anyway you need.

https://gist.github.com/eypsilon/aa0ef898608ff4c441d7d7af94bc6b7b

Usage:

// the first parameter sets the max lifetime of an session in seconds
(new AutoLogout)->watch(3, '/redirect_to');

// with a callback as closure
(new AutoLogout)->watch(3, null, function($s, $r, $st) {
    // custom events
});

eypsilon avatar Mar 11 '21 17:03 eypsilon

Thanks again, @eypsilon! This looks good. There’s just one thing that I think could be improved:

I know this is currently independent of any Auth instance, which may be helpful in some cases. But since you depend on \Delight\Auth\UserManager and the session field and thus this library here anyway, you can probably just rely on an Auth instance as well. You could then replace the call to \session_destroy() with a call to Auth#logOut, which should be better (and compatible with future changes). You could even call Auth#destroySession, but that shouldn’t be necessary.

@dotsensei If you use the solution shared above, make sure that the PHP configuration value session.cookie_lifetime is greater than or equal to your first parameter for AutoLogout#watch (i.e. $seconds). If it is not, the solution above still ensures a maximum time of inactivity, but your users’ sessions can expire earlier than expected, even before reaching the defined limit. That may not be what you want. The same is true for session.gc_maxlifetime, unless session.gc_probability is less than or equal to zero.

If that automatic logout after a given time of inactivity is something more people want, we can (and should) add this directly to this library here. So let’s leave this issue open.

ocram avatar Mar 11 '21 22:03 ocram

you can probably just rely on an Auth instance as well. You could then replace the call to \session_destroy() with a call to Auth#logOut

Since there are many available options for the logout process (logOut, destroySession, logOutEveryWhere, logOutEveryWhereElse, etc.), it's pretty difficult to decide which of them should be available. A pretty simple way to use internal methods is actually the callback function.

(new AutoLogout)->watch(2, null, function() use($auth) {
    // at this point, the session is expired
    $auth->logOut();
    $auth->destroySession();
});

But FULL ACK, this should be part of the library, it just makes sense. And it would be easier to implement, as anything i do.

I've written an alternate Class, that can be used instead. It only checks and logs the User out, when the session expires. Everything else can be done, when the method is called.

https://gist.github.com/eypsilon/8f3a5934e54367b0f4fdbcab72210c04

if ((new AutoLogout)->watch($auth, 3)) {
    // session is expired and destroyed
}

we can (and should) add this directly to this library here.

[X]

eypsilon avatar Mar 12 '21 07:03 eypsilon

Thanks! Fully agree with everything you said, and that newer class looks even better.

ocram avatar Mar 12 '21 15:03 ocram

I was curious so I tested it in the library and it just fits perfectly in it.

// in: vendor/delight-im/auth/src/Auth.php

/** @var String session field for users last action time */
const SESSION_FIELD_LAST_ACTION = 'auth_last_action';

/**
 * Logs the user automatically out after X seconds. Runs only, when a User is logged in.
 * 
 * @param Int $seconds max lifetime in seconds
 * @param Array $run a list with internal methods to run if session is expired, default: ['logOut', 'destroySession']
 * @return Bool returns true, if the session is expired
 */
public function autoLogout(Int $seconds, Array $run=['logOut', 'destroySession']): Bool
{
	if ($this->check())
		if ($l = ($_SESSION[self::SESSION_FIELD_LAST_ACTION] ?? false) AND \time() > $l+$seconds) {
			foreach($run as $task) 
				if (\method_exists($this, $task))
					$this->$task();
			return true;
		} else $_SESSION[self::SESSION_FIELD_LAST_ACTION] = \time();
	return false;
}

And to use the method, we can call it right after instantiation. And this method should definitely be part of the lib (not necessarily what i've posted, but the functionality in generell), because it's an often used mechanism on Pages with Auth-Services.

$auth = new \Delight\Auth\Auth($db);
$auth->autoLogout(3);

eypsilon avatar Mar 12 '21 16:03 eypsilon

It will be something very similar. Either a method call, as you suggested, or a new parameter somewhere. But the thing is, of course, that we might want to allow for detection of automatic logouts, so that you can easily implement your redirect, etc. So the method call with a boolean return value makes a lot of sense.

We won’t need the second parameter $run = [ 'logOut', 'destroySession' ], however. I know there are a few methods for logouts and it can be confusing, but here the choice is actually clear: logOutEverywhereElse makes no sense when your session on the local device has expired, logOutEverywhere (similarly) is excessive (and may be counterproductive), destroySession is simply not necessary (and you could do it when performing the redirect), so that leaves us with only logOut.

ocram avatar Mar 12 '21 19:03 ocram

We won’t need the second parameter

The loop is actually a relic of the previous versions of the class, where the focus was more on the callback functionality. You are right, internally it's not necessary anymore.

But to keep it practicable, it should be a method, so we can do stuff like:

if ($auth->autoLogout(3)) {
    // notify_user | some_custom_stuff | ...
}

eypsilon avatar Mar 13 '21 15:03 eypsilon