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

Auth::login and Auth::loginUsingId causes log out #997

Closed
fylzero opened this issue Mar 15, 2022 · 19 comments
Closed

Auth::login and Auth::loginUsingId causes log out #997

fylzero opened this issue Mar 15, 2022 · 19 comments

Comments

@fylzero
Copy link

fylzero commented Mar 15, 2022

  • Jetstream Version: 2.6.7
  • Jetstream Stack: Inertia
  • Uses Teams: no
  • Laravel Version: 9.4.1
  • PHP Version: 8.1.3
  • Database Driver & Version: MySQL v8.0.28 for macos12.2 on arm64 (Homebrew)

Description:

Auth::login and Auth::loginUsingId causes user to be logged out

Steps To Reproduce:

  • Install fresh copy of Laravel 9 w/ Jetstream + Inertia
  • Create a route to switch users with Auth::login($user) or Auth::loginUsingId($id)
  • Create 2 user accounts
  • Log in as user 1
  • Try using the route to login as user 2
  • Return to the dashboard
  • You are logged out instead of being logged in as the other user

https://github.com/fylzero/bug-report

@driesvints
Copy link
Member

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as separate commits on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

@driesvints
Copy link
Member

@fylzero is this one resolved as well now?

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints Yes, this has resolved as well. Thanks again!

@fylzero fylzero closed this as completed Mar 15, 2022
@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints Sorry to say, this seems to have resolved in a fresh Laravel app but not in my existing app after updating. Apologies in advance if I am missing something obvious.

Steps I've taken:

  • composer dump-autoload
  • php artisan optimize:clear
  • Removed vendor directory
  • composer install
  • composer update
  • I'm definitely on v9.5.1 and the reversion change you all just made is there
  • My linter still says getDefaultDriver is not available (in vendor/laravel/framework/src/Illuminate/Session/Middleware/AuthenticateSession.php)
  • I am still getting logged out when trying to impersonate another user

@fylzero fylzero reopened this Mar 15, 2022
@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints I was mistaken. This issue still exists when creating a new project. I'll put together a bug-report repo.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints Here is the repo, I've linked in the OP as well.

https://github.com/fylzero/bug-report

Instructions:

  • Pull repo, setup database, migrate/fresh/seed
  • There are 2 pre-seeded users w/ password set to password
  • Login as User 1 dries@exmaple.com
  • Go to the /switch/2 route - this should log you in as User 2 phil@example.com
  • Return to the root url of the app - /
  • You will be logged out instead of logged in as User 2

@relaypilot
Copy link

relaypilot commented Mar 15, 2022

@fylzero Can you try reverting those changes made in 9.4.1 and see if it resolves the issue?
laravel/framework@50b46db

Line to comment out:
404labfr/laravel-impersonate#154 (comment)

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@fylzero Can you try reverting those changes made in 9.4.1 and see if it resolves the issue? laravel/framework@50b46db

Line to comment out: 404labfr/laravel-impersonate#154 (comment)

My app/Http/Kernel.php file does not contain \Illuminate\Contracts\Session\Middleware\AuthenticatesSessions::class, however, if. I comment out \Laravel\Jetstream\Http\Middleware\AuthenticateSession::class I do not get logged out.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints I have verified that when trying to use this functionality the evaluations for $request->session()->get('password_hash_'.$this->auth->getDefaultDriver()) and $request->user()->getAuthPassword() are not equal to each other and this is causing the system to log the user out.

See: vendor/laravel/framework/src/Illuminate/Session/Middleware/AuthenticateSession.php:55

@driesvints
Copy link
Member

@fylzero those changes are in the foundation kernel, not the app one. Have a closer look.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints You are correct. Yes, commenting this out in the Foundation Kernel also resolves the logout issue.

I don't understand why but the evaluation on line 55 of vendor/laravel/framework/src/Illuminate/Session/Middleware/AuthenticateSession.php is coming back as true and logging an impersonated user out. Any thoughts on this? I don't really understand this code and my linter is saying these functions don't exist, so I can't seem to make it deeper down the rabbit hole.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints It appears in Jetstream/Sanctum this is causing this to compare the password_hash_sanctum to the password_hash_web tokens. $request->user()->getAuthPassword() is pulling the web token, not Sanctum.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@driesvints I read the changelog in Laravel and yes reverting the change here: laravel/framework@50b46db

...also resolved the issue. I believe that is the problem.

@taylorotwell
Copy link
Member

I can't recreate this.

@taylorotwell
Copy link
Member

taylorotwell commented Mar 15, 2022

@fylzero what you are reporting in your example repo is NOT a bug. That is in fact PROVING that session authentication is working correctly. You used login but didn't update the flashed password in the session that authenticate session uses to compare the current user's password to the one in the session - thus, you are logged out.

I can't recreate it when using the impersonation package in question because they fully flush the session by logging out and then logging in again.

CleanShot 2022-03-15 at 16 21 22@2x

This works on Jetstream using the lab404/laravel-impersonate package.

When using authenticated sessions, a naive Auth::login() call is not sufficient. More work has to be done in the session (which the impersonate package does) to make everything work right.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

@taylorotwell Fair enough. This was working before that update so, wasn't sure. I'll look at what the package is doing and make the necessary changes. Obviously this only "broke" user impersonation so, not super mission-critical.

Thanks for providing this info!

@taylorotwell
Copy link
Member

taylorotwell commented Mar 15, 2022

Yeah - it's a bit complicated. It helps if you have a good grasp of how authenticated sessions work. I'll try to summarize:

When using authenticated sessions...

  • We store the current user's hashed password in the session...
  • On each request, we compare the user's hashed password from the database against the hashed password in the session
  • If they do not match, log the user out

So, with this in place, when a user updates their password in a user profile section of an application, all other sessions for that user will automatically be logged out because their copy of the password in their sessions no longer matches the database. Pretty slick actually!

So, back to your example, you log a new user into the application, but you never update the session password hash accordingly. Therefore, they no longer match and you are logged out.

@fylzero
Copy link
Author

fylzero commented Mar 15, 2022

For anyone else running into what I did, this seems to work for me but definitely feels backwards / like I'm cheating. getAuthPassword() seems to be grabbing the password_hash_web from session and I'm just forcing the sanctum token to match that to hack through things. I could be wrong, but it would be nice to do this the other way around imo. I also couldn't figure out how to dynamically grab the session key variable how vendor/laravel/framework/src/Illuminate/Session/Middleware/AuthenticateSession.php does. I may keep working on this later... but this does solve my problem for now.

// Log in as impersonated user
$user = Auth::loginUsingId($userId);

// Store impersonated user's password hash in session
session(['password_hash_sanctum' => $user->getAuthPassword()]);

@larskoole
Copy link

The simplest solution is to put auth()->shouldUse('web'); above any auth()->login() or auth()->logout() in your controller. No need for setting the new password hash.

I don't know if this brings any security risks though.

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

5 participants