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.

Sign up for GitHub

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

Jump to bottom

Fix/unable to logout #511

Merged
merged 11 commits into from
Apr 10, 2024
+50 −8
Merged

Fix/unable to logout #511

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d3bbdb4
fix user not being able to logout
GigaGiorgadze Apr 6, 2024
cb1a78d
fix use auth guard to grab password aswell
GigaGiorgadze Apr 6, 2024
afc6507
test: write test for hash getting removed from session
GigaGiorgadze Apr 8, 2024
3c2f2ce
Merge branch 'fix/unable-to-logout' of github.com:GigaGiorgadze/sanct…
GigaGiorgadze Apr 8, 2024
e313c54
change global auth helper to injected auth factory
GigaGiorgadze Apr 9, 2024
c8b9495
Update AuthenticateSession.php
taylorotwell Apr 9, 2024
20d5023
instead of checking first guard user the guard that user is authentic…
GigaGiorgadze Apr 9, 2024
3b52ad2
fix style ci complaint
GigaGiorgadze Apr 9, 2024
30069db
formatting
taylorotwell Apr 10, 2024
6c20b6d
formatting
taylorotwell Apr 10, 2024
15114b0
formatting
taylorotwell Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/Http/Middleware/AuthenticateSession.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ public function handle(Request $request, Closure $next): Response
}

return tap($next($request), function () use ($request, $guards) {
if (! is_null($request->user())) {
$this->storePasswordHashInSession($request, $guards->keys()->first());
$guard = $guards->keys()->first();

if ($this->auth->guard($guard)->hasUser()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanctum can be configured to attempt to use an array of guards to authenticate users. How do you know the first guard in the list is the one that actually was used and is the one that has the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point 👀. i did not give it much thought since it was like that before. i rewrote it to getting first guard that user is authenticated with.

$this->storePasswordHashInSession($request, $guard);
}
});
}
Expand All @@ -79,12 +81,12 @@ public function handle(Request $request, Closure $next): Response
*/
protected function storePasswordHashInSession($request, string $guard)
{
if (! $request->user()) {
if (! $this->auth->guard($guard)->hasUser()) {
return;
}

$request->session()->put([
"password_hash_{$guard}" => $request->user()->getAuthPassword(),
"password_hash_{$guard}" => $this->auth->guard($guard)->user()->getAuthPassword(),
]);
}
}
32 changes: 31 additions & 1 deletion tests/Controller/FrontendRequestsAreStatefulTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

class FrontendRequestsAreStatefulTest extends TestCase
{
use RefreshDatabase, WithWorkbench;
use RefreshDatabase;
use WithWorkbench;

protected function defineEnvironment($app)
{
Expand Down Expand Up @@ -57,6 +58,13 @@ protected function defineRoutes($router)

return $request->user()->email;
})->middleware($webMiddleware);

$router->get('/sanctum/api/logout', function () {
auth()->guard('web')->logout();
session()->flush();

return 'logged out';
})->middleware($apiMiddleware);
}

public function test_middleware_keeps_session_logged_in_when_sanctum_request_changes_password()
Expand Down Expand Up @@ -143,6 +151,28 @@ public function test_middleware_can_deauthorize_valid_user_using_acting_as_after
])->assertStatus(401);
}

public function test_middleware_removes_password_hash_after_session_is_cleared_during_request()
{
$user = UserFactory::new()->create();

$this->actingAs($user)
->getJson('/web/user', [
'origin' => config('app.url'),
])
->assertOk()
->assertSee($user->email);

$this->getJson('/sanctum/web/user', [
'origin' => config('app.url'),
])
->assertOk()
->assertSee($user->email)->assertSessionHas('password_hash_web', $user->getAuthPassword());

$this->getJson('/sanctum/api/logout', [
'origin' => config('app.url'),
])->assertOk()->assertSee('logged out')->assertSessionMissing('password_hash_web');
}

public static function sanctumGuardsDataProvider()
{
yield [null];
Expand Down