Auth::user() throws an AuthException if not logged in.

Added method `Auth::isLoggedIn()`. The `user()` method always returns a user and never `null`, as it typical to use `Auth::user()->doSomething()` while assuming a user is logged in due to access control on the page. Having the method return `null` is an issue when using static code analysis.
jasny · Dec 27, 2019 · bf34ca7 · bf34ca7
1 parent af16b4e
commit bf34ca7
Show file tree

Hide file tree

Showing 11 changed files with 195 additions and 103 deletions.
diff --git a/README.md b/README.md
@@ -15,6 +15,7 @@ Authentication, authorization and access control for PHP.
 * Authorization [context](#context) (eg. "is the user an _admin_ of this _team_").  
 * PSR-14 [events](#events) for login and logout.
 * PSR-15 [middleware](#access-control-middleware) for access control.
+* Session invalidation after password change.
 * [Confirmation tokens](#confirmation) for signup confirmation and forgot-password.
 * Customizable to meet the requirements of your application.
 
@@ -244,9 +245,11 @@ Triggers a [logout event](#events).
 
 #### user
 
-    Auth::user(): UserInterface|null
+    Auth::user(): UserInterface
 
-Get the current user. Returns `null` if no user is logged in.
+Get the current user.
+
+Use `isLoggedIn()` to see if there is a logged in user. This function throws an `AuthException` if no user is logged in.
 
 ### Events
 
@@ -431,11 +434,17 @@ $auth->user()->getAuthRole(); // Returns 'admin' which supersedes 'moderator'.
 
 ### Methods
 
+#### isLoggedIn
+
+    Auth::isLoggedIn(): bool
+
+Check if a user if logged in.
+
 #### is
 
     Auth::is(string $role): bool
 
-Check if a user has a specific role or superseding role
+Check if a user has a specific role or superseding role.
 
 #### getAvailableRoles
 
@@ -451,13 +460,13 @@ Returns a copy of the `Authz` service with the current user and context.
 
 #### forUser
 
-    Auth::authz(User $user): Authz
+    Auth::authz(User|null $user): Authz
 
 Returns a copy of the `Authz` service with the given user, in the current context.
 
 #### inContextOf
 
-    Auth::inContextOf(Context $context): Authz
+    Auth::inContextOf(Context|null $context): Authz
 
 Returns a copy of the `Authz` service with the current user, in the given context.
 

diff --git a/src/Auth.php b/src/Auth.php
@@ -121,6 +121,17 @@ final public function getAvailableRoles(): array
         return $this->authz->getAvailableRoles();
     }
 
+
+    /**
+     * Check if the current user is logged in.
+     */
+    final public function isLoggedIn(): bool
+    {
+        $this->assertInitialized();
+
+        return $this->authz->isLoggedIn();
+    }
+
     /**
      * Check if the current user is logged in and has specified role.
      *
@@ -142,9 +153,9 @@ final public function is(string $role): bool
     /**
      * Get current authenticated user.
      *
-     * @return User|null
+     * @throws AuthException if no user is logged in.
      */
-    final public function user(): ?User
+    final public function user(): User
     {
         $this->assertInitialized();
 
@@ -171,7 +182,7 @@ public function loginAs(User $user): void
     {
         $this->assertInitialized();
 
-        if ($this->authz->user() !== null) {
+        if ($this->authz->isLoggedIn()) {
             throw new \LogicException("Already logged in");
         }
 
@@ -187,7 +198,7 @@ public function login(string $username, string $password): void
     {
         $this->assertInitialized();
 
-        if ($this->authz->user() !== null) {
+        if ($this->authz->isLoggedIn()) {
             throw new \LogicException("Already logged in");
         }
 
@@ -226,12 +237,12 @@ public function logout(): void
     {
         $this->assertInitialized();
 
-        $user = $this->authz->user();
-
-        if ($user === null) {
-            return; // already logged out
+        if (!$this->authz()->isLoggedIn()) {
+            return;
         }
 
+        $user = $this->authz->user();
+
         $this->authz = $this->authz->forUser(null)->inContextOf(null);
         $this->updateSession();
 
@@ -268,14 +279,14 @@ public function recalc(): self
      */
     protected function updateSession(): void
     {
-        $user = $this->authz->user();
-        $context = $this->authz->context();
-
-        if ($user === null) {
+        if (!$this->authz->isLoggedIn()) {
             $this->session->clear();
             return;
         }
 
+        $user = $this->authz->user();
+        $context = $this->authz->context();
+
         $uid = $user->getAuthId();
         $cid = $context !== null ? $context->getAuthId() : null;
         $checksum = $user->getAuthChecksum();

diff --git a/src/AuthException.php b/src/AuthException.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Jasny\Auth;
+
+/**
+ * Authentication exception.
+ */
+class AuthException extends \RuntimeException
+{
+}
diff --git a/src/AuthMiddleware.php b/src/AuthMiddleware.php
@@ -94,7 +94,7 @@ protected function isAllowed(ServerRequest $request): bool
         }
 
         if (is_bool($requiredRole)) {
-            return ($this->auth->user() !== null) === $requiredRole;
+            return $this->auth->isLoggedIn() === $requiredRole;
         }
 
         return Pipeline::with(is_array($requiredRole) ? $requiredRole : [$requiredRole])
@@ -106,9 +106,7 @@ protected function isAllowed(ServerRequest $request): bool
      */
     protected function forbidden(ServerRequest $request, ?Response $response = null): Response
     {
-        $unauthorized = $this->auth->user() === null;
-
-        $forbiddenResponse = $this->createResponse($unauthorized ? 401 : 403, $response)
+        $forbiddenResponse = $this->createResponse($this->auth->isLoggedIn() ? 403 : 401, $response)
             ->withProtocolVersion($request->getProtocolVersion());
         $forbiddenResponse->getBody()->write('Access denied');
 
@@ -126,11 +124,16 @@ protected function createResponse(int $status, ?Response $originalResponse = nul
     {
         if ($this->responseFactory !== null) {
             return $this->responseFactory->createResponse($status);
-        } elseif ($originalResponse !== null) {
-            return $originalResponse->withStatus($status)->withBody(clone $originalResponse->getBody());
-            ;
-        } else {
-            throw new \LogicException('Response factory not set');
         }
+
+        if ($originalResponse !== null) {
+            // There is no standard way to get an empty body without a factory. One of these methods may work.
+            $body = clone $originalResponse->getBody();
+            $body->rewind();
+
+            return $originalResponse->withStatus($status)->withBody($body);
+        }
+
+        throw new \LogicException('Response factory not set');
     }
 }
diff --git a/src/Authz/StateTrait.php b/src/Authz/StateTrait.php
@@ -4,6 +4,7 @@
 
 namespace Jasny\Auth\Authz;
 
+use Jasny\Auth\AuthException;
 use Jasny\Auth\AuthzInterface as Authz;
 use Jasny\Auth\ContextInterface as Context;
 use Jasny\Auth\UserInterface as User;
@@ -53,10 +54,14 @@ public function inContextOf(?Context $context): Authz
     /**
      * Get current authenticated user.
      *
-     * @return User|null
+     * @throws AuthException if no user is logged in.
      */
-    final public function user(): ?User
+    final public function user(): User
     {
+        if ($this->user === null) {
+            throw new AuthException("The user is not logged in");
+        }
+
         return $this->user;
     }
 
@@ -67,4 +72,12 @@ final public function context(): ?Context
     {
         return $this->context;
     }
+
+    /**
+     * Check if the current user is logged in.
+     */
+    public function isLoggedIn(): bool
+    {
+        return $this->user !== null;
+    }
 }
diff --git a/src/AuthzInterface.php b/src/AuthzInterface.php
@@ -46,17 +46,20 @@ public function recalc(): self;
 
     /**
      * Get current authenticated user.
-     *
-     * @return User|null
      */
-    public function user(): ?User;
+    public function user(): User;
 
     /**
      * Get the current context.
      */
     public function context(): ?Context;
 
 
+    /**
+     * Check if the current user is logged in.
+     */
+    public function isLoggedIn(): bool;
+
     /**
      * Check if the current user is logged in and has specified role.
      */

diff --git a/src/LoginException.php b/src/LoginException.php
@@ -7,7 +7,7 @@
 /**
  * Exception on failed login attempt.
  */
-class LoginException extends \RuntimeException
+class LoginException extends AuthException
 {
     public const CANCELLED = 0;
     public const INVALID_CREDENTIALS = 1;

diff --git a/tests/AuthMiddlewareDoublePassTest.php b/tests/AuthMiddlewareDoublePassTest.php
@@ -37,7 +37,7 @@ public function setUp(): void
 
     public function testNoRequirements()
     {
-        $this->authz->expects($this->never())->method('user');
+        $this->authz->expects($this->never())->method($this->anything());
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(null);
@@ -61,13 +61,13 @@ public function testNoRequirements()
 
     public function testRequireUser()
     {
-        $user = $this->createMock(User::class);
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn($user);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(true);
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(true);
 
         $initialResp = $this->createMock(Response::class);
+        $initialResp->expects($this->never())->method($this->anything());
 
         $response = $this->createMock(Response::class);
         $response->expects($this->never())->method($this->anything());
@@ -86,7 +86,8 @@ public function testRequireUser()
 
     public function testRequireNoUser()
     {
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn(null);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(false);
+        $this->authz->expects($this->never())->method('user');
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(false);
@@ -110,7 +111,8 @@ public function testRequireNoUser()
 
     public function testLoginRequired()
     {
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn(null);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(false);
+        $this->authz->expects($this->never())->method('user');
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(true);
@@ -162,9 +164,7 @@ public function testAccessGranted()
 
     public function testAccessDenied()
     {
-        $user = $this->createMock(User::class);
-
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn($user);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(true);
         $this->authz->expects($this->once())->method('is')->with('foo')->willReturn(false);
 
         $request = $this->createMock(ServerRequest::class);

diff --git a/tests/AuthMiddlewareTest.php b/tests/AuthMiddlewareTest.php
@@ -40,7 +40,7 @@ public function setUp(): void
 
     public function testNoRequirements()
     {
-        $this->authz->expects($this->never())->method('user');
+        $this->authz->expects($this->never())->method($this->anything());
 
         $this->responseFactory->expects($this->never())->method('createResponse');
 
@@ -62,8 +62,7 @@ public function testNoRequirements()
 
     public function testRequireUser()
     {
-        $user = $this->createMock(User::class);
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn($user);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(true);
 
         $this->responseFactory->expects($this->never())->method('createResponse');
 
@@ -85,7 +84,7 @@ public function testRequireUser()
 
     public function testRequireNoUser()
     {
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn(null);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(false);
 
         $this->responseFactory->expects($this->never())->method('createResponse');
 
@@ -107,7 +106,7 @@ public function testRequireNoUser()
 
     public function testLoginRequired()
     {
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn(null);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(false);
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(true);
@@ -158,7 +157,7 @@ public function testAccessDenied()
     {
         $user = $this->createMock(User::class);
 
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn($user);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(true);
         $this->authz->expects($this->once())->method('is')->with('foo')->willReturn(false);
 
         $request = $this->createMock(ServerRequest::class);
@@ -192,7 +191,7 @@ public function testMissingResponseFactory()
             fn(ServerRequest $request) => $request->getAttribute('auth'),
         );
 
-        $this->authz->expects($this->atLeastOnce())->method('user')->willReturn(null);
+        $this->authz->expects($this->atLeastOnce())->method('isLoggedIn')->willReturn(false);
 
         $request = $this->createMock(ServerRequest::class);
         $request->expects($this->once())->method('getAttribute')->with('auth')->willReturn(true);