More auth improvements (#269)

* Added the auth scheme name to the authentication result * Added policy name to AuthorizationResult * Fixed missing arg * Fixed Psalm errors * Renamed a class and method * Fixed misleading comment * WIP * Fixed scheme name usage * Fixed a bunch of Psalm errors * Fixed tests * Ran linter * Fixed more Psalm issues * Updated CHANGELOG * Updated Authenticator to accept multiple scheme names, still need to refactor middleware to use that and to actually set user in accessor * Fixed tests
aphiria · Jun 26, 2023 · f0440aa · f0440aa
1 parent ffbd3cc
commit f0440aa
Show file tree

Hide file tree

Showing 27 changed files with 586 additions and 392 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,10 @@
 ### Changed
 
 - Updated to PHPUnit 10.1 ([#248](https://github.com/aphiria/aphiria/pull/248), [#250](https://github.com/aphiria/aphiria/pull/250))
+- Updated `IAuthenticator::authenticate()`, `IAuthenticator::challenge()`, `IAuthenticator::forbid()`, `IAuthenticator::logIn()`, and `IAuthenticator::logOut()` to take in no, one, or many authentication scheme names ([#269](https://github.com/aphiria/aphiria/pull/269))
+- Added the authentication scheme name(s) to `AuthenticationResult` ([#269](https://github.com/aphiria/aphiria/pull/269))
+- Removed `IUserAccessor` property from `Authenticator` ([#269](https://github.com/aphiria/aphiria/pull/269))
+- Renamed `SchemeNotFoundException` to `AuthenticationSchemeNotFoundException` ([#269](https://github.com/aphiria/aphiria/pull/269))
 
 ### Added
 
@@ -13,6 +17,7 @@
 - Added ability to easily deserialize request and response bodies in integration tests ([#253](https://github.com/aphiria/aphiria/pull/253))
 - Added `PrincipalBuilder` and `IdentityBuilder` ([#257](https://github.com/aphiria/aphiria/pull/257))
 - Added `IPrincipal::mergeIdentities()` ([#262](https://github.com/aphiria/aphiria/pull/262))
+- Added `AggregateAuthenticationException` when authenticating against multiple schemes and all of them failing ([#269](https://github.com/aphiria/aphiria/pull/269))
 
 ## [v1.0.0-alpha8](https://github.com/aphiria/aphiria/compare/v1.0.0-alpha7...v1.0.0-alpha8) (2022-12-10)
 

diff --git a/src/Authentication/src/AggregateAuthenticationException.php b/src/Authentication/src/AggregateAuthenticationException.php
@@ -0,0 +1,35 @@
+<?php
+
+/**
+ * Aphiria
+ *
+ * @link      https://www.aphiria.com
+ * @copyright Copyright (C) 2023 David Young
+ * @license   https://github.com/aphiria/aphiria/blob/1.x/LICENSE.md
+ */
+
+declare(strict_types=1);
+
+namespace Aphiria\Authentication;
+
+use Exception;
+
+/**
+ * Defines an exception that aggregates multiple authentication exceptions
+ */
+final class AggregateAuthenticationException extends Exception
+{
+    /** @var list<Exception> The list of exceptions that created this exception */
+    public readonly array $innerExceptions;
+
+    /**
+     * @param string $message The message to set
+     * @param Exception|list<Exception> $exceptions The exception or list of exceptions to aggregate
+     */
+    public function __construct(string $message, Exception|array $exceptions)
+    {
+        parent::__construct($message);
+
+        $this->innerExceptions = $exceptions instanceof Exception ? [$exceptions] : $exceptions;
+    }
+}
diff --git a/src/Authentication/src/AuthenticationResult.php b/src/Authentication/src/AuthenticationResult.php
@@ -23,14 +23,19 @@
  */
 readonly class AuthenticationResult
 {
+    /** @var list<string> The list of scheme names evaluated in the result */
+    public array $schemeNames;
+
     /**
      * @param bool $passed Whether or not authentication passed
+     * @param list<string>|string $schemeNames The name of the authentication scheme used
      * @param IPrincipal|null $user The authenticated user if one was found, otherwise null
      * @param Exception|null $failure The failure that occurred, or null if none did
      * @throws InvalidArgumentException Thrown if the result is in an invalid state
      */
-    public function __construct(
+    protected function __construct(
         public bool $passed,
+        array|string $schemeNames,
         public ?IPrincipal $user = null,
         public ?Exception $failure = null
     ) {
@@ -41,27 +46,31 @@ public function __construct(
         if ($this->passed && $this->user === null) {
             throw new InvalidArgumentException('Passing authentication results must specify a user');
         }
+
+        $this->schemeNames = \is_string($schemeNames) ? [$schemeNames] : $schemeNames;
     }
 
     /**
      * Creates a failed authentication result
      *
      * @param Exception|string $failure The exception that occurred or a failure message
+     *  @param list<string>|string $schemeNames The name or names of the authentication scheme that were evaluated
      * @return static A failed authentication result
      */
-    public static function fail(Exception|string $failure): static
+    public static function fail(Exception|string $failure, array|string $schemeNames): static
     {
-        return new static(false, failure: \is_string($failure) ? new Exception($failure) : $failure);
+        return new static(false, $schemeNames, failure: \is_string($failure) ? new Exception($failure) : $failure);
     }
 
     /**
      * Creates a passing authentication result
      *
      * @param IPrincipal $user The authenticated user
+     * @param list<string>|string $schemeNames The name or names of the authentication scheme that were evaluated
      * @return static A passing authentication result
      */
-    public static function pass(IPrincipal $user): static
+    public static function pass(IPrincipal $user, array|string $schemeNames): static
     {
-        return new static(true, $user);
+        return new static(true, $schemeNames, $user);
     }
 }
diff --git a/...ntication/src/SchemeNotFoundException.php → ...AuthenticationSchemeNotFoundException.php b/...ntication/src/SchemeNotFoundException.php → ...AuthenticationSchemeNotFoundException.php
@@ -17,6 +17,6 @@
 /**
  * Defines the exception that's thrown when an authentication scheme is not found
  */
-final class SchemeNotFoundException extends Exception
+final class AuthenticationSchemeNotFoundException extends Exception
 {
 }
diff --git a/src/Authentication/src/Authenticator.php b/src/Authentication/src/Authenticator.php
@@ -16,6 +16,8 @@
 use Aphiria\Net\Http\IRequest;
 use Aphiria\Net\Http\IResponse;
 use Aphiria\Security\IPrincipal;
+use Exception;
+use InvalidArgumentException;
 use OutOfBoundsException;
 
 /**
@@ -26,92 +28,134 @@ class Authenticator implements IAuthenticator
     /**
      * @param AuthenticationSchemeRegistry $schemes The registry of authentication schemes
      * @param IAuthenticationSchemeHandlerResolver $handlerResolver The resolver for authentication handlers
-     * @param IUserAccessor $userAccessor What we'll use to access the current user
      */
     public function __construct(
         private readonly AuthenticationSchemeRegistry $schemes,
-        private readonly IAuthenticationSchemeHandlerResolver $handlerResolver,
-        private readonly IUserAccessor $userAccessor = new RequestPropertyUserAccessor()
+        private readonly IAuthenticationSchemeHandlerResolver $handlerResolver
     ) {
     }
 
     /**
      * @inheritdoc
      */
-    public function authenticate(IRequest $request, string $schemeName = null): AuthenticationResult
+    public function authenticate(IRequest $request, array|string $schemeNames = null): AuthenticationResult
     {
-        $scheme = $this->getScheme($schemeName);
-        $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
-        $authResult = $handler->authenticate($request, $scheme);
-
-        if ($authResult->passed && $authResult->user !== null) {
-            if (($user = $this->userAccessor->getUser($request)) instanceof IPrincipal) {
-                // Merge this user with any previously-set user so that all the identities and claims are set for all schemes authenticated against
-                // We store this merged identity in a new authentication result, and return that one instead
-                $user->mergeIdentities($authResult->user);
-                $authResult = new AuthenticationResult($authResult->passed, $user, $authResult->failure);
+        // This will contain resolved (ie non-null) scheme names
+        $resolvedSchemeNames = [];
+        // This will contain all failed authentication results' failures
+        $authResultFailures = [];
+        $user = $authResult = null;
+
+        foreach (self::normalizeSchemeNames($schemeNames) as $schemeName) {
+            $scheme = $this->getScheme($schemeName);
+            $resolvedSchemeNames[] = $scheme->name;
+            $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+            $authResult = $handler->authenticate($request, $scheme);
+
+            if ($authResult->passed) {
+                if ($user instanceof IPrincipal && $authResult->user instanceof IPrincipal) {
+                    // We've successfully authenticated with another scheme, so merge identities
+                    $user->mergeIdentities($authResult->user);
+                } else {
+                    // This was the first successful authentication result
+                    $user = $authResult->user;
+                }
+            } elseif ($authResult->failure instanceof Exception) {
+                $authResultFailures[] = $authResult->failure;
             }
+        }
+
+        // Auth result will never be null, but the check below makes Psalm happy
+        if ($authResult instanceof AuthenticationResult && \count($resolvedSchemeNames) === 1) {
+            // Just pass back the auth result directly
+            return $authResult;
+        }
 
-            $this->userAccessor->setUser($authResult->user, $request);
+        if ($user === null) {
+            // Authentication did not pass, so aggregate the failures
+            return AuthenticationResult::fail(new AggregateAuthenticationException('All authentication schemes failed to authenticate', $authResultFailures), $resolvedSchemeNames);
         }
 
-        return $authResult;
+        return AuthenticationResult::pass($user, $resolvedSchemeNames);
     }
 
     /**
      * @inheritdoc
      */
-    public function challenge(IRequest $request, IResponse $response, string $schemeName = null): void
+    public function challenge(IRequest $request, IResponse $response, array|string $schemeNames = null): void
     {
-        $scheme = $this->getScheme($schemeName);
-        $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
-        $handler->challenge($request, $response, $scheme);
+        foreach (self::normalizeSchemeNames($schemeNames) as $schemeName) {
+            $scheme = $this->getScheme($schemeName);
+            $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+            $handler->challenge($request, $response, $scheme);
+        }
     }
 
     /**
      * @inheritdoc
      */
-    public function forbid(IRequest $request, IResponse $response, string $schemeName = null): void
+    public function forbid(IRequest $request, IResponse $response, array|string $schemeNames = null): void
     {
-        $scheme = $this->getScheme($schemeName);
-        $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
-        $handler->forbid($request, $response, $scheme);
+        foreach (self::normalizeSchemeNames($schemeNames) as $schemeName) {
+            $scheme = $this->getScheme($schemeName);
+            $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+            $handler->forbid($request, $response, $scheme);
+        }
     }
 
     /**
      * @inheritdoc
      */
-    public function logIn(IPrincipal $user, IRequest $request, IResponse $response, string $schemeName = null): void
+    public function logIn(IPrincipal $user, IRequest $request, IResponse $response, array|string $schemeNames = null): void
     {
         if (!($user->getPrimaryIdentity()?->isAuthenticated() ?? false)) {
             throw new NotAuthenticatedException('User identity must be set and authenticated to log in');
         }
 
-        $scheme = $this->getScheme($schemeName);
-        $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+        foreach (self::normalizeSchemeNames($schemeNames) as $schemeName) {
+            $scheme = $this->getScheme($schemeName);
+            $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
 
-        if (!$handler instanceof ILoginAuthenticationSchemeHandler) {
-            throw new UnsupportedAuthenticationHandlerException($handler::class . ' does not implement ' . ILoginAuthenticationSchemeHandler::class);
-        }
+            if (!$handler instanceof ILoginAuthenticationSchemeHandler) {
+                throw new UnsupportedAuthenticationHandlerException($handler::class . ' does not implement ' . ILoginAuthenticationSchemeHandler::class);
+            }
 
-        $handler->logIn($user, $request, $response, $scheme);
-        $this->userAccessor->setUser($user, $request);
+            $handler->logIn($user, $request, $response, $scheme);
+        }
     }
 
     /**
      * @inheritdoc
      */
-    public function logOut(IRequest $request, IResponse $response, string $schemeName = null): void
+    public function logOut(IRequest $request, IResponse $response, array|string $schemeNames = null): void
+    {
+        foreach (self::normalizeSchemeNames($schemeNames) as $schemeName) {
+            $scheme = $this->getScheme($schemeName);
+            $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+
+            if (!$handler instanceof ILoginAuthenticationSchemeHandler) {
+                throw new UnsupportedAuthenticationHandlerException($handler::class . ' does not implement ' . ILoginAuthenticationSchemeHandler::class);
+            }
+
+            $handler->logOut($request, $response, $scheme);
+        }
+    }
+
+    /**
+     * Normalizes scheme names into an array of scheme names
+     *
+     * @param list<string|null>|string|null $schemeNames The scheme name or names to normalize
+     * @return list<string|null> The normalized scheme names
+     */
+    private static function normalizeSchemeNames(array|string|null $schemeNames): array
     {
-        $scheme = $this->getScheme($schemeName);
-        $handler = $this->handlerResolver->resolve($scheme->handlerClassName);
+        $normalizedSchemeNames = \is_array($schemeNames) ? $schemeNames : [$schemeNames];
 
-        if (!$handler instanceof ILoginAuthenticationSchemeHandler) {
-            throw new UnsupportedAuthenticationHandlerException($handler::class . ' does not implement ' . ILoginAuthenticationSchemeHandler::class);
+        if (\count($normalizedSchemeNames) === 0) {
+            throw new InvalidArgumentException('You must specify at least one scheme name or pass in null if using the default scheme');
         }
 
-        $handler->logOut($request, $response, $scheme);
-        $this->userAccessor->setUser(null, $request);
+        return $normalizedSchemeNames;
     }
 
     /**
@@ -120,15 +164,15 @@ public function logOut(IRequest $request, IResponse $response, string $schemeNam
      * @template T of AuthenticationSchemeOptions
      * @param string|null $schemeName The name of the authentication scheme to get, or null if getting the default one
      * @return AuthenticationScheme<T> The authentication scheme
-     * @throws SchemeNotFoundException Thrown if no scheme could be found
+     * @throws AuthenticationSchemeNotFoundException Thrown if no scheme could be found
      */
     private function getScheme(?string $schemeName): AuthenticationScheme
     {
         if ($schemeName === null) {
             $scheme = $this->schemes->getDefaultScheme();
 
             if ($scheme === null) {
-                throw new SchemeNotFoundException('No default authentication scheme found');
+                throw new AuthenticationSchemeNotFoundException('No default authentication scheme found');
             }
 
             return $scheme;
@@ -137,7 +181,7 @@ private function getScheme(?string $schemeName): AuthenticationScheme
         try {
             return $this->schemes->getScheme($schemeName);
         } catch (OutOfBoundsException $ex) {
-            throw new SchemeNotFoundException("No authentication scheme with name \"$schemeName\" found", 0, $ex);
+            throw new AuthenticationSchemeNotFoundException("No authentication scheme with name \"$schemeName\" found", 0, $ex);
         }
     }
 }
diff --git a/src/Authentication/src/AuthenticatorBuilder.php b/src/Authentication/src/AuthenticatorBuilder.php
@@ -21,8 +21,6 @@ class AuthenticatorBuilder
 {
     /** @var IAuthenticationSchemeHandlerResolver|null The handler resolver to use, or null if none is set */
     private ?IAuthenticationSchemeHandlerResolver $handlerResolver = null;
-    /** @var IUserAccessor|null The user accessor to use, or null if none is set */
-    private ?IUserAccessor $userAccessor = null;
 
     /**
      * @param AuthenticationSchemeRegistry $schemes The authentication schemes to use
@@ -44,13 +42,7 @@ public function build(): IAuthenticator
             throw new RuntimeException('No handler resolver was specified');
         }
 
-        if ($this->userAccessor === null) {
-            $authenticator = new Authenticator($this->schemes, $this->handlerResolver);
-        } else {
-            $authenticator = new Authenticator($this->schemes, $this->handlerResolver, $this->userAccessor);
-        }
-
-        return $authenticator;
+        return new Authenticator($this->schemes, $this->handlerResolver);
     }
 
     /**
@@ -80,17 +72,4 @@ public function withScheme(AuthenticationScheme $scheme, bool $isDefault = false
 
         return $this;
     }
-
-    /**
-     * Sets the user accessor the authenticator will use
-     *
-     * @param IUserAccessor $userAccessor The user accessor to use
-     * @return static For chaining
-     */
-    public function withUserAccessor(IUserAccessor $userAccessor): static
-    {
-        $this->userAccessor = $userAccessor;
-
-        return $this;
-    }
 }