[5.x] Logout user from other devices when changing password

[duncanmcclean]

This pull request makes it so when a user changes their own password, it'll log them out from their other devices.

Fixes #10524.

[jasonvarga]

Check out this flow:

• Browser 1 and Browser 2 are both logged into the same account
• Browser 1 changes the password
• Browser 2 tries to navigate to another page in the CP
• Browser 2 gets an error about a login route

By default that middleware redirects to a route named login. I think you should call AuthenticateSession::redirectUsing(fn () => cp_route('login')). But somehow only when it's a CP route. If someone is using that middleware on the frontend, you don't want to hijack the redirect.

[duncanmcclean]

Good catch! I've added that into the CpServiceProvider 👍

[925dk]

Does this fix work if it's an admin changing the user's pw? I think it has to as well.

[jasonvarga]

This is still going to redirect to the CP on a non-CP page.

I meant if you use the middleware on a front-end route you don't want to get taken to the CP. In those cases, we want to just not call this method, and let them deal with it themselves. e.g. Keep the login route or have them adjust it.

Maybe we can have another earlier middleware that gets run for CP routes that calls this method.

[jasonvarga]

Does this fix work if it's an admin changing the user's pw? I think it has to as well.

Right now, no. But I agree that it should.

I just thought about this chunk of code:

if ($currentPassword = $request->current_password) {
    Auth::logoutOtherDevices($currentPassword);
}

That shouldn't be the way to do it. The password has been changed by that point, so the new password ($request->password) is the current password, not $request->current_password, which is now the old password.

We should be able to do Auth::logoutOtherDevices($request->password); but that throws an error. Somewhere in the Laravel code, it checks that the password you passed to logoutOtherDevices matches the one already on the user. It doesn't match because the one it checks against is one cached to a property in SessionGuard.php.

I'm not sure where the issue lies. Still looking into it. 🤔

[jasonvarga]

Does this fix work if it's an admin changing the user's pw? I think it has to as well.

Right now, no. But I agree that it should.

Actually, scratch that. The Auth::logoutOtherDevices can only log out other devices of the currently authenticated user. It would be nice if it could log out others but I don't think that'll be possible. At least not in a simple way.

[925dk]

@jasonvarga Hmm. Maybe you don't need logoutOtherDevices at all? You just want to set a new password, and so the pw hash on file will change. Then the AuthenticateSession middleware will take care of the rest? (by doing logout if hash in session dosen't match hash on file).

[jasonvarga]

You're right!

[jasonvarga]

Hmm. Maybe you don't need logoutOtherDevices at all? You just want to set a new password, and so the pw hash on file will change. Then the AuthenticateSession middleware will take care of the rest? (by doing logout if hash in session dosen't match hash on file).

This was true, but it also logged out the current user. Adding Auth::login() to log the user back in addressed this.