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.

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

Automatic logout #250

Open
dotsensei opened this issue Mar 11, 2021 · 7 comments
Open

Automatic logout #250

dotsensei opened this issue Mar 11, 2021 · 7 comments

Comments

@dotsensei
Copy link

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.

@eypsilon
Copy link

eypsilon commented Mar 11, 2021

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
});

@ocram
Copy link
Contributor

ocram commented Mar 11, 2021

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.

@eypsilon
Copy link

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]

@ocram
Copy link
Contributor

ocram commented Mar 12, 2021

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

@eypsilon
Copy link

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);

@ocram
Copy link
Contributor

ocram commented Mar 12, 2021

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.

@eypsilon
Copy link

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 | ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants