From 7b48bfccf9ed12c71461651bbf52a3214b58d82e Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Fri, 23 Dec 2016 09:39:59 -0600 Subject: [PATCH] Refactor auth component. Reorganize methods, refactor code, general cleaning. --- src/Illuminate/Auth/Events/Attempting.php | 11 +- src/Illuminate/Auth/Recaller.php | 78 ++++ src/Illuminate/Auth/RequestGuard.php | 4 +- src/Illuminate/Auth/SessionGuard.php | 410 ++++++++---------- .../Contracts/Auth/StatefulGuard.php | 3 +- tests/Auth/AuthGuardTest.php | 8 +- 6 files changed, 276 insertions(+), 238 deletions(-) create mode 100644 src/Illuminate/Auth/Recaller.php diff --git a/src/Illuminate/Auth/Events/Attempting.php b/src/Illuminate/Auth/Events/Attempting.php index 805db22305bc..3d0473000fda 100644 --- a/src/Illuminate/Auth/Events/Attempting.php +++ b/src/Illuminate/Auth/Events/Attempting.php @@ -18,23 +18,14 @@ class Attempting */ public $remember; - /** - * Indicates if the user should be authenticated if successful. - * - * @var bool - */ - public $login; - /** * Create a new event instance. * * @param array $credentials * @param bool $remember - * @param bool $login */ - public function __construct($credentials, $remember, $login) + public function __construct($credentials, $remember) { - $this->login = $login; $this->remember = $remember; $this->credentials = $credentials; } diff --git a/src/Illuminate/Auth/Recaller.php b/src/Illuminate/Auth/Recaller.php new file mode 100644 index 000000000000..6d1253d15824 --- /dev/null +++ b/src/Illuminate/Auth/Recaller.php @@ -0,0 +1,78 @@ +recaller = $recaller; + } + + /** + * Get the user ID from the recaller. + * + * @return string + */ + public function id() + { + return explode('|', $this->recaller, 2)[0]; + } + + /** + * Get the "remember token" token from the recaller. + * + * @return string + */ + public function token() + { + return explode('|', $this->recaller, 2)[1]; + } + + /** + * Determine if the recaller is valid. + * + * @return bool + */ + public function valid() + { + return $this->properString() && $this->hasBothSegments(); + } + + /** + * Determine if the recaller is an invalid string. + * + * @return bool + */ + protected function properString() + { + return is_string($this->recaller) && Str::contains($this->recaller, '|'); + } + + /** + * Determine if the recaller has both segments. + * + * @return bool + */ + protected function hasBothSegments() + { + $segments = explode('|', $this->recaller); + + return count($segments) == 2 && trim($segments[0]) !== '' && trim($segments[1]) !== ''; + } +} diff --git a/src/Illuminate/Auth/RequestGuard.php b/src/Illuminate/Auth/RequestGuard.php index e6b9a7e7f73e..30b3a1d77404 100644 --- a/src/Illuminate/Auth/RequestGuard.php +++ b/src/Illuminate/Auth/RequestGuard.php @@ -50,7 +50,9 @@ public function user() return $this->user; } - return $this->user = call_user_func($this->callback, $this->request); + return $this->user = call_user_func( + $this->callback, $this->request + ); } /** diff --git a/src/Illuminate/Auth/SessionGuard.php b/src/Illuminate/Auth/SessionGuard.php index a29a6c456316..c274a7dd9ef6 100644 --- a/src/Illuminate/Auth/SessionGuard.php +++ b/src/Illuminate/Auth/SessionGuard.php @@ -21,7 +21,7 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth /** * The name of the Guard. Typically "session". * - * Corresponds to driver name in authentication configuration. + * Corresponds to guard name in authentication configuration. * * @var string */ @@ -81,7 +81,7 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth * * @var bool */ - protected $tokenRetrievalAttempted = false; + protected $recallAttempted = false; /** * Create a new authentication guard. @@ -137,10 +137,10 @@ public function user() // If the user is null, but we decrypt a "recaller" cookie we can attempt to // pull the user data on that cookie which serves as a remember cookie on // the application. Once we have a user we can return it to the caller. - $recaller = $this->getRecaller(); + $recaller = $this->recaller(); if (is_null($user) && ! is_null($recaller)) { - $user = $this->getUserByRecaller($recaller); + $user = $this->userFromRecaller($recaller); if ($user) { $this->updateSession($user->getAuthIdentifier()); @@ -153,79 +153,55 @@ public function user() } /** - * Get the ID for the currently authenticated user. - * - * @return int|null - */ - public function id() - { - if ($this->loggedOut) { - return; - } - - if ($this->user()) { - return $this->user()->getAuthIdentifier(); - } - - return $this->session->get($this->getName()); - } - - /** - * Pull a user from the repository by its recaller ID. + * Pull a user from the repository by its "remember me" cookie token. * * @param string $recaller * @return mixed */ - protected function getUserByRecaller($recaller) + protected function userFromRecaller($recaller) { - if ($this->validRecaller($recaller) && ! $this->tokenRetrievalAttempted) { - $this->tokenRetrievalAttempted = true; + if (! $recaller->valid() || $this->recallAttempted) { + return; + } - list($id, $token) = explode('|', $recaller, 2); + // If the user is null, but we decrypt a "recaller" cookie we can attempt to + // pull the user data on that cookie which serves as a remember cookie on + // the application. Once we have a user we can return it to the caller. + $this->recallAttempted = true; - $this->viaRemember = ! is_null($user = $this->provider->retrieveByToken($id, $token)); + $this->viaRemember = ! is_null($user = $this->provider->retrieveByToken( + $recaller->id(), $recaller->token() + )); - return $user; - } + return $user; } /** * Get the decrypted recaller cookie for the request. * - * @return string|null - */ - protected function getRecaller() - { - return $this->request->cookies->get($this->getRecallerName()); - } - - /** - * Get the user ID from the recaller cookie. - * - * @return string|null + * @return \Illuminate\Auth\Recaller|null */ - protected function getRecallerId() + protected function recaller() { - if ($this->validRecaller($recaller = $this->getRecaller())) { - return head(explode('|', $recaller)); + if ($recaller = $this->request->cookies->get($this->getRecallerName())) { + return new Recaller($recaller); } } /** - * Determine if the recaller cookie is in a valid format. + * Get the ID for the currently authenticated user. * - * @param mixed $recaller - * @return bool + * @return int|null */ - protected function validRecaller($recaller) + public function id() { - if (! is_string($recaller) || ! Str::contains($recaller, '|')) { - return false; + if ($this->loggedOut) { + return; } - $segments = explode('|', $recaller); - - return count($segments) == 2 && trim($segments[0]) !== '' && trim($segments[1]) !== ''; + return $this->user() + ? $this->user()->getAuthIdentifier() + : $this->session->get($this->getName()); } /** @@ -236,6 +212,8 @@ protected function validRecaller($recaller) */ public function once(array $credentials = []) { + $this->fireAttemptEvent($credentials); + if ($this->validate($credentials)) { $this->setUser($this->lastAttempted); @@ -245,6 +223,23 @@ public function once(array $credentials = []) return false; } + /** + * Log the given user ID into the application without sessions or cookies. + * + * @param mixed $id + * @return \Illuminate\Contracts\Auth\Authenticatable|false + */ + public function onceUsingId($id) + { + if (! is_null($user = $this->provider->retrieveById($id))) { + $this->setUser($user); + + return $user; + } + + return false; + } + /** * Validate a user's credentials. * @@ -253,7 +248,9 @@ public function once(array $credentials = []) */ public function validate(array $credentials = []) { - return $this->attempt($credentials, false, false); + $user = $this->provider->retrieveByCredentials($credentials); + + return $this->hasValidCredentials($user, $credentials); } /** @@ -276,7 +273,7 @@ public function basic($field = 'email', $extraConditions = []) return; } - return $this->getBasicResponse(); + return $this->failedBasicResponse(); } /** @@ -288,10 +285,10 @@ public function basic($field = 'email', $extraConditions = []) */ public function onceBasic($field = 'email', $extraConditions = []) { - $credentials = $this->getBasicCredentials($this->getRequest(), $field); + $credentials = $this->basicCredentials($this->getRequest(), $field); if (! $this->once(array_merge($credentials, $extraConditions))) { - return $this->getBasicResponse(); + return $this->failedBasicResponse(); } } @@ -309,9 +306,9 @@ protected function attemptBasic(Request $request, $field, $extraConditions = []) return false; } - $credentials = $this->getBasicCredentials($request, $field); - - return $this->attempt(array_merge($credentials, $extraConditions)); + return $this->attempt(array_merge( + $this->basicCredentials($request, $field), $extraConditions + )); } /** @@ -321,7 +318,7 @@ protected function attemptBasic(Request $request, $field, $extraConditions = []) * @param string $field * @return array */ - protected function getBasicCredentials(Request $request, $field) + protected function basicCredentials(Request $request, $field) { return [$field => $request->getUser(), 'password' => $request->getPassword()]; } @@ -331,11 +328,9 @@ protected function getBasicCredentials(Request $request, $field) * * @return \Symfony\Component\HttpFoundation\Response */ - protected function getBasicResponse() + protected function failedBasicResponse() { - $headers = ['WWW-Authenticate' => 'Basic']; - - return new Response('Invalid credentials.', 401, $headers); + return new Response('Invalid credentials.', 401, ['WWW-Authenticate' => 'Basic']); } /** @@ -346,9 +341,9 @@ protected function getBasicResponse() * @param bool $login * @return bool */ - public function attempt(array $credentials = [], $remember = false, $login = true) + public function attempt(array $credentials = [], $remember = false) { - $this->fireAttemptEvent($credentials, $remember, $login); + $this->fireAttemptEvent($credentials, $remember); $this->lastAttempted = $user = $this->provider->retrieveByCredentials($credentials); @@ -356,9 +351,7 @@ public function attempt(array $credentials = [], $remember = false, $login = tru // to validate the user against the given credentials, and if they are in // fact valid we'll log the users into the application and return true. if ($this->hasValidCredentials($user, $credentials)) { - if ($login) { - $this->login($user, $remember); - } + $this->login($user, $remember); return true; } @@ -366,9 +359,7 @@ public function attempt(array $credentials = [], $remember = false, $login = tru // If the authentication attempt fails we will fire an event so that the user // may be notified of any suspicious attempts to access their account from // an unrecognized user. A developer may listen to this event as needed. - if ($login) { - $this->fireFailedEvent($user, $credentials); - } + $this->fireFailedEvent($user, $credentials); return false; } @@ -386,47 +377,21 @@ protected function hasValidCredentials($user, $credentials) } /** - * Fire the attempt event with the arguments. + * Log the given user ID into the application. * - * @param array $credentials - * @param bool $remember - * @param bool $login - * @return void + * @param mixed $id + * @param bool $remember + * @return \Illuminate\Contracts\Auth\Authenticatable|false */ - protected function fireAttemptEvent(array $credentials, $remember, $login) + public function loginUsingId($id, $remember = false) { - if (isset($this->events)) { - $this->events->fire(new Events\Attempting( - $credentials, $remember, $login - )); - } - } + if (! is_null($user = $this->provider->retrieveById($id))) { + $this->login($user, $remember); - /** - * Fire the failed authentication attempt event with the given arguments. - * - * @param \Illuminate\Contracts\Auth\Authenticatable|null $user - * @param array $credentials - * @return void - */ - protected function fireFailedEvent($user, array $credentials) - { - if (isset($this->events)) { - $this->events->fire(new Events\Failed($user, $credentials)); + return $user; } - } - /** - * Register an authentication attempt event listener. - * - * @param mixed $callback - * @return void - */ - public function attempting($callback) - { - if (isset($this->events)) { - $this->events->listen(Events\Attempting::class, $callback); - } + return false; } /** @@ -444,7 +409,7 @@ public function login(AuthenticatableContract $user, $remember = false) // queue a permanent cookie that contains the encrypted copy of the user // identifier. We will then decrypt this later to retrieve the users. if ($remember) { - $this->createRememberTokenIfDoesntExist($user); + $this->ensureRememberTokenIsSet($user); $this->queueRecallerCookie($user); } @@ -457,33 +422,6 @@ public function login(AuthenticatableContract $user, $remember = false) $this->setUser($user); } - /** - * Fire the login event if the dispatcher is set. - * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param bool $remember - * @return void - */ - protected function fireLoginEvent($user, $remember = false) - { - if (isset($this->events)) { - $this->events->fire(new Events\Login($user, $remember)); - } - } - - /** - * Fire the authenticated event if the dispatcher is set. - * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @return void - */ - protected function fireAuthenticatedEvent($user) - { - if (isset($this->events)) { - $this->events->fire(new Events\Authenticated($user)); - } - } - /** * Update the session with the given ID. * @@ -498,42 +436,16 @@ protected function updateSession($id) } /** - * Log the given user ID into the application. - * - * @param mixed $id - * @param bool $remember - * @return \Illuminate\Contracts\Auth\Authenticatable|false - */ - public function loginUsingId($id, $remember = false) - { - $user = $this->provider->retrieveById($id); - - if (! is_null($user)) { - $this->login($user, $remember); - - return $user; - } - - return false; - } - - /** - * Log the given user ID into the application without sessions or cookies. + * Create a new "remember me" token for the user if one doesn't already exist. * - * @param mixed $id - * @return \Illuminate\Contracts\Auth\Authenticatable|false + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @return void */ - public function onceUsingId($id) + protected function ensureRememberTokenIsSet(AuthenticatableContract $user) { - $user = $this->provider->retrieveById($id); - - if (! is_null($user)) { - $this->setUser($user); - - return $user; + if (empty($user->getRememberToken())) { + $this->cycleRememberToken($user); } - - return false; } /** @@ -544,9 +456,9 @@ public function onceUsingId($id) */ protected function queueRecallerCookie(AuthenticatableContract $user) { - $value = $user->getAuthIdentifier().'|'.$user->getRememberToken(); - - $this->getCookieJar()->queue($this->createRecaller($value)); + $this->getCookieJar()->queue($this->createRecaller( + $user->getAuthIdentifier().'|'.$user->getRememberToken() + )); } /** @@ -575,7 +487,7 @@ public function logout() $this->clearUserDataFromStorage(); if (! is_null($this->user)) { - $this->refreshRememberToken($user); + $this->cycleRememberToken($user); } if (isset($this->events)) { @@ -599,10 +511,9 @@ protected function clearUserDataFromStorage() { $this->session->remove($this->getName()); - if (! is_null($this->getRecaller())) { - $recaller = $this->getRecallerName(); - - $this->getCookieJar()->queue($this->getCookieJar()->forget($recaller)); + if (! is_null($this->recaller())) { + $this->getCookieJar()->queue($this->getCookieJar() + ->forget($this->getRecallerName())); } } @@ -612,7 +523,7 @@ protected function clearUserDataFromStorage() * @param \Illuminate\Contracts\Auth\Authenticatable $user * @return void */ - protected function refreshRememberToken(AuthenticatableContract $user) + protected function cycleRememberToken(AuthenticatableContract $user) { $user->setRememberToken($token = Str::random(60)); @@ -620,18 +531,115 @@ protected function refreshRememberToken(AuthenticatableContract $user) } /** - * Create a new "remember me" token for the user if one doesn't already exist. + * Register an authentication attempt event listener. + * + * @param mixed $callback + * @return void + */ + public function attempting($callback) + { + if (isset($this->events)) { + $this->events->listen(Events\Attempting::class, $callback); + } + } + + /** + * Fire the attempt event with the arguments. + * + * @param array $credentials + * @param bool $remember + * @return void + */ + protected function fireAttemptEvent(array $credentials, $remember) + { + if (isset($this->events)) { + $this->events->fire(new Events\Attempting( + $credentials, $remember + )); + } + } + + /** + * Fire the login event if the dispatcher is set. * * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param bool $remember * @return void */ - protected function createRememberTokenIfDoesntExist(AuthenticatableContract $user) + protected function fireLoginEvent($user, $remember = false) { - if (empty($user->getRememberToken())) { - $this->refreshRememberToken($user); + if (isset($this->events)) { + $this->events->fire(new Events\Login($user, $remember)); } } + /** + * Fire the authenticated event if the dispatcher is set. + * + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @return void + */ + protected function fireAuthenticatedEvent($user) + { + if (isset($this->events)) { + $this->events->fire(new Events\Authenticated($user)); + } + } + + /** + * Fire the failed authentication attempt event with the given arguments. + * + * @param \Illuminate\Contracts\Auth\Authenticatable|null $user + * @param array $credentials + * @return void + */ + protected function fireFailedEvent($user, array $credentials) + { + if (isset($this->events)) { + $this->events->fire(new Events\Failed($user, $credentials)); + } + } + + /** + * Get the last user we attempted to authenticate. + * + * @return \Illuminate\Contracts\Auth\Authenticatable + */ + public function getLastAttempted() + { + return $this->lastAttempted; + } + + /** + * Get a unique identifier for the auth session value. + * + * @return string + */ + public function getName() + { + return 'login_'.$this->name.'_'.sha1(static::class); + } + + /** + * Get the name of the cookie used to store the "recaller". + * + * @return string + */ + public function getRecallerName() + { + return 'remember_'.$this->name.'_'.sha1(static::class); + } + + /** + * Determine if the user was authenticated via "remember me" cookie. + * + * @return bool + */ + public function viaRemember() + { + return $this->viaRemember; + } + /** * Get the cookie creator instance used by the guard. * @@ -760,44 +768,4 @@ public function setRequest(Request $request) return $this; } - - /** - * Get the last user we attempted to authenticate. - * - * @return \Illuminate\Contracts\Auth\Authenticatable - */ - public function getLastAttempted() - { - return $this->lastAttempted; - } - - /** - * Get a unique identifier for the auth session value. - * - * @return string - */ - public function getName() - { - return 'login_'.$this->name.'_'.sha1(static::class); - } - - /** - * Get the name of the cookie used to store the "recaller". - * - * @return string - */ - public function getRecallerName() - { - return 'remember_'.$this->name.'_'.sha1(static::class); - } - - /** - * Determine if the user was authenticated via "remember me" cookie. - * - * @return bool - */ - public function viaRemember() - { - return $this->viaRemember; - } } diff --git a/src/Illuminate/Contracts/Auth/StatefulGuard.php b/src/Illuminate/Contracts/Auth/StatefulGuard.php index f644e4bb9f1b..cfd623dac0c5 100644 --- a/src/Illuminate/Contracts/Auth/StatefulGuard.php +++ b/src/Illuminate/Contracts/Auth/StatefulGuard.php @@ -9,10 +9,9 @@ interface StatefulGuard extends Guard * * @param array $credentials * @param bool $remember - * @param bool $login * @return bool */ - public function attempt(array $credentials = [], $remember = false, $login = true); + public function attempt(array $credentials = [], $remember = false); /** * Log a user into the application without sessions or cookies. diff --git a/tests/Auth/AuthGuardTest.php b/tests/Auth/AuthGuardTest.php index 51785def4ace..7eb70cd54d21 100755 --- a/tests/Auth/AuthGuardTest.php +++ b/tests/Auth/AuthGuardTest.php @@ -207,13 +207,13 @@ public function testUserIsSetToRetrievedUser() public function testLogoutRemovesSessionTokenAndRememberMeCookie() { list($session, $provider, $request, $cookie) = $this->getMocks(); - $mock = $this->getMockBuilder('Illuminate\Auth\SessionGuard')->setMethods(['getName', 'getRecallerName', 'getRecaller'])->setConstructorArgs(['default', $provider, $session, $request])->getMock(); + $mock = $this->getMockBuilder('Illuminate\Auth\SessionGuard')->setMethods(['getName', 'getRecallerName', 'recaller'])->setConstructorArgs(['default', $provider, $session, $request])->getMock(); $mock->setCookieJar($cookies = m::mock('Illuminate\Cookie\CookieJar')); $user = m::mock('Illuminate\Contracts\Auth\Authenticatable'); $user->shouldReceive('setRememberToken')->once(); $mock->expects($this->once())->method('getName')->will($this->returnValue('foo')); $mock->expects($this->once())->method('getRecallerName')->will($this->returnValue('bar')); - $mock->expects($this->once())->method('getRecaller')->will($this->returnValue('non-null-cookie')); + $mock->expects($this->once())->method('recaller')->will($this->returnValue('non-null-cookie')); $provider->shouldReceive('updateRememberToken')->once(); $cookie = m::mock('Symfony\Component\HttpFoundation\Cookie'); @@ -228,12 +228,12 @@ public function testLogoutRemovesSessionTokenAndRememberMeCookie() public function testLogoutDoesNotEnqueueRememberMeCookieForDeletionIfCookieDoesntExist() { list($session, $provider, $request, $cookie) = $this->getMocks(); - $mock = $this->getMockBuilder('Illuminate\Auth\SessionGuard')->setMethods(['getName', 'getRecaller'])->setConstructorArgs(['default', $provider, $session, $request])->getMock(); + $mock = $this->getMockBuilder('Illuminate\Auth\SessionGuard')->setMethods(['getName', 'recaller'])->setConstructorArgs(['default', $provider, $session, $request])->getMock(); $mock->setCookieJar($cookies = m::mock('Illuminate\Cookie\CookieJar')); $user = m::mock('Illuminate\Contracts\Auth\Authenticatable'); $user->shouldReceive('setRememberToken')->once(); $mock->expects($this->once())->method('getName')->will($this->returnValue('foo')); - $mock->expects($this->once())->method('getRecaller')->will($this->returnValue(null)); + $mock->expects($this->once())->method('recaller')->will($this->returnValue(null)); $provider->shouldReceive('updateRememberToken')->once(); $mock->getSession()->shouldReceive('remove')->once()->with('foo');