Auth keeps all users logged in after password change

[donnysim]

Let's say someone saw my password and logged in, strange activity starts happening, I changed my password, strange activity continues to happen because the culprit is still logged in. Then he changes the password, I use password reset form, strange activity continues to happen - he's still logged in.

This is considered a security issue, no? especially if someone get to know admin panels password

[donnysim]

The only way to make that user logout is to delete the database entry. BUT if you create an entry with the same ID, it can contain other email/password, does not matter - the culprit will be logged in again.

[hulkur]

Delete all sessions with this user id.

[donnysim]

And how do you do that? I have no control over the sessions.

[RomainLanz]

Hey @donnysim !

It's up to you to decide if you want to logout the user when the password is changed. You only need to add auth()->logout() after patching your user.

[donnysim]

The problem is not loggin out the current user, but the UNKNOWN user that is logged in with the current account.

[donnysim]

Even wordpress has fixed this issue from v3 :|

[RomainLanz]

Oh you're right!

It should exists something like auth()->logoutAll() to logout all user using the current account.

[themsaid]

How would laravel know the current account to log it out? I think this is something to be handled in every app based on the use case.

[donnysim]

Shouldn't it at least store a hash of email and password in session, and on session login check if they were changed? If they were, your session is invalid?

[themsaid]

This behaviour might not be desired in all applications afaik.

[donnysim]

But this poses a security problem afaik :\ if at least an options is added to enable this behavior, I won't mind.

[donnysim]

Let's put it this way, you have a paid service, you give your friend your password for one day and say check it out, then change your password, thinking that he doesn't have access anymore, but in reality he has a lifetime access until you delete your account or sessions is cleared. We wouldn't want that now, would we.

[srmklive]

@donnysim as @themsaid pointed out this is something that needs to be handled on case-by-case basis. The case you are describing is something you need to handle on your own.

[donnysim]

@srmklive then please state this in the documentation with big red letters that this is the case.

[taylorotwell]

You can do this using the database session driver since you can query for all sessions by user_id and log out that user across all sessions.

[donnysim]

@taylorotwell how does the guard handle session restore? does it call any event I can catch to handle this case?

[donnysim]

Nwm, found one, Events\Authenticated

[donnysim]

If it's not held as security issue then you can close it.

[themsaid]

Actually I'll be looking into the matter today, hopefully come up with a PR if things look clean enough.

[taylorotwell]

I think it’s worth fixing. @themsaid is looking into it today.

[themsaid]

Moving the discussion to the PR https://github.com/laravel/framework/pull/16323

[srmklive]

@donnysim As i said before, the use case you are mentioning can be handled on your own. For this you may need to create a custom auth guard which implements all the functionality you are looking for. Glad that such feature is being considered to be implemented in 5.4

[themsaid]

A new Middleware was added to 5.3 \Illuminate\Session\Middleware\AuthenticateSession that should handle this functionality, it works by setting a password_hash session key on user authentication and checks for its existence in all future requests.

[thewebartisan7]

I have uncommented the middleware in web group, \Illuminate\Session\Middleware\AuthenticateSession::classand logoutOthersDevices() works fine after enabling this.

However since I am using multi guard, if I try to login with second guard, then first guard get log-out by this middeware.

Checking the middleare handle() method I see that I was loggout here:

        if ($request->session()->get('password_hash') !== $request->user()->getAuthPassword()) {
            $this->logout($request);
        }

How to make this new middleware in multi guard scenario? I am not sure how to handle this.

Thanks

[djurovicigoor]

@thewebartisan7 Did you find a proper way to handle a multi guard scenario?

[thewebartisan7]

No sorry. And as I understand there is not a way. I simple stop use multi auth time ago as it introduce too much complexity, and now handle via roles and permissions everything.

Cao

[tomhapbia]

No sorry. And as I understand there is not a way. I simple stop use multi auth time ago as it introduce too much complexity, and now handle via roles and permissions everything.

@thewebartisan7 @djurovicigoor

You can override default AuthenticateSession middleware or create a new AuthenticateSession middleware for multi guards with a custom password_hash field to store in session (similar with default AuthenticateSession middleware). For example $guard . '_password_hash'.

In my case, I created a new AuthenticateSession middleware similar with default and I handle this by:

public function handle($request, Closure $next)
    {
        $authGuards = [ADMIN, COMPANY, COMPANY_USER, USER];
        $guards = [];
        foreach ($authGuards as $guard) {
            if (auth()->guard($guard)->check()) {
                $guards[] = $guard;
            }
        }

        if (! $guards || ! $request->hasSession()) {
            return $next($request);
        }

        foreach ($guards as $guard) {
            if (! $request->session()->has($guard . '_password_hash')) {
                $this->storeIdHashInSession($request, $guard);
            }

            if ($request->session()->get($guard . '_password_hash') !== $request->user($guard)->getAuthPassword()) {
                $this->logout($request, $guard);
            }

            $this->storeIdHashInSession($request, $guard);
        }
        return $next($request);
    }

    protected function storeIdHashInSession($request, $guard): void
    {
        if (! $request->user($guard)) {
            return;
        }

        $request->session()->put([
            $guard . '_password_hash' => $request->user($guard)->getAuthPassword(),
        ]);
    }
    protected function logout($request)
    {
        $this->auth->logoutCurrentDevice();

        $request->session()->flush();
    }

You can define a parameter $guard (for handle specify guard name) in handle() function without loop throught array $guards