Skip to content

Commit

Permalink
Fixes $request->user() or Auth::user() run query each time if tok…
Browse files Browse the repository at this point in the history
…en doesn't exists

fixes #49201

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
  • Loading branch information
crynobone committed Dec 1, 2023
1 parent e574ffa commit 409647d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 2 deletions.
11 changes: 10 additions & 1 deletion src/Illuminate/Auth/RequestGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ class RequestGuard implements Guard
*/
protected $request;

/**
* Indicates if a token user retrieval has been attempted.
*
* @var bool
*/
protected $recallAttempted = false;

/**
* Create a new authentication guard.
*
Expand All @@ -50,10 +57,12 @@ public function user()
// If we've already retrieved the user for the current request we can just
// return it back immediately. We do not want to fetch the user data on
// every call to this method because that would be tremendously slow.
if (! is_null($this->user)) {
if (! is_null($this->user) || $this->recallAttempted) {
return $this->user;
}

$this->recallAttempted = true;

return $this->user = call_user_func(
$this->callback, $this->request, $this->getProvider()
);
Expand Down
11 changes: 10 additions & 1 deletion src/Illuminate/Auth/TokenGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ class TokenGuard implements Guard
*/
protected $hash = false;

/**
* Indicates if a token user retrieval has been attempted.
*
* @var bool
*/
protected $recallAttempted = false;

/**
* Create a new authentication guard.
*
Expand Down Expand Up @@ -72,10 +79,12 @@ public function user()
// If we've already retrieved the user for the current request we can just
// return it back immediately. We do not want to fetch the user data on
// every call to this method because that would be tremendously slow.
if (! is_null($this->user)) {
if (! is_null($this->user) || $this->recallAttempted) {
return $this->user;
}

$this->recallAttempted = true;

$user = null;

$token = $this->getTokenForRequest();
Expand Down
33 changes: 33 additions & 0 deletions tests/Auth/AuthRequestGuardTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace Illuminate\Tests\Auth;

use Illuminate\Auth\RequestGuard;
use Illuminate\Contracts\Auth\UserProvider;
use Illuminate\Http\Request;
use Mockery as m;
use PHPUnit\Framework\TestCase;

class AuthRequestGuardTest extends TestCase
{
protected function tearDown(): void
{
m::close();
}

public function testUserDoesntCallRetrieveByCredentialMoreThanOnceWhenGivenAuthentication()
{
$provider = m::mock(UserProvider::class);
$provider->shouldReceive('retrieveByCredentials')->once()->with(['api_token' => 'foo'])->andReturnNull();
$request = Request::create('/', 'GET', ['api_token' => 'foo']);

$guard = new RequestGuard(function ($request, $provider) {
return $provider->retrieveByCredentials($request->query());
}, $request, $provider);

$this->assertNull($guard->user());

// Ensure mocked provider.retrieveByCredential expectation only called once.
$this->assertNull($guard->user());
}
}
14 changes: 14 additions & 0 deletions tests/Auth/AuthTokenGuardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ public function testUserCanBeRetrievedByAuthHeaders()
$this->assertSame(1, $user->id);
}

public function testUserDoesntCallRetrieveByCredentialMoreThanOnceWhenGivenAuthentication()
{
$provider = m::mock(UserProvider::class);
$provider->shouldReceive('retrieveByCredentials')->once()->with(['api_token' => 'foo'])->andReturnNull();
$request = Request::create('/', 'GET', ['api_token' => 'foo']);

$guard = new TokenGuard($provider, $request);

$this->assertNull($guard->user());

// Ensure mocked provider.retrieveByCredential expectation only called once.
$this->assertNull($guard->user());
}

public function testUserCanBeRetrievedByBearerToken()
{
$provider = m::mock(UserProvider::class);
Expand Down

0 comments on commit 409647d

Please sign in to comment.