feat(auth): improve OAuth user flow (#1587)

Co-authored-by: Enzo Innocenzi <[email protected]>

Author: brendt

docs/2-features/17-oauth.md

@@ -52,35 +52,22 @@ final readonly class DiscordOAuthController
     #[Get('/auth/discord')]
     public function redirect(): Redirect
     {
-        // Saves a unique token in the user's session
-        $this->session->set('discord:oauth', $this->oauth->getState());
-
-        // Redirects to the OAuth provider's authorization page
-        return new Redirect($this->oauth->getAuthorizationUrl());
+        return $this->oauth->createRedirect(scopes: ['identify']);
     }
 
     #[Get('/auth/discord/callback')]
     public function callback(Request $request): Redirect
     {
-        // Validates the saved session token to prevent CSRF attacks
-        if ($this->session->get('discord:oauth') !== $request->get('state')) {
-            return new Redirect('/?error=invalid_state');
-        }
-
-        // Fetches the user information from the OAuth provider
-        $discordUser = $this->oauth->fetchUser($request->get('code'));
-
-        // Creates or updates the user in the database
-        $user = query(User::class)->updateOrCreate([
-            'discord_id' => $discordUser->id,
-        ], [
-            'discord_id' => $discordUser->id,
-            'username' => $discordUser->nickname,
-            'email' => $discordUser->email,
-        ]);
-
-        // Finally, authenticates the user in the application
-        $this->authenticator->authenticate($user);
+        $user = $this->oauth->authenticate(
+            request: $request,
+            map: fn (OAuthUser $user): User => query(User::class)->updateOrCreate([
+                'discord_id' => $user->id,
+            ], [
+                'discord_id' => $user->id,
+                'username' => $user->nickname,
+                'email' => $user->email,
+            ])
+        );
 
         return new Redirect('/');
     }
@@ -245,7 +232,7 @@ Below is an example of a complete testing flow for an OAuth authentication:
 final class OAuthControllerTest extends IntegrationTestCase
 {
     #[Test]
-    public function ensure_oauth(): void
+    public function oauth(): void
     {
         // We create a fake OAuth client that will return
         // the specified user when the OAuth flow is completed
@@ -268,7 +255,7 @@ final class OAuthControllerTest extends IntegrationTestCase
         // We then simulate the callback from the provider
         // with a fake code and the expected state
         $this->http
-            ->get("/oauth/discord/callback?code=some-fake-code&state={$oauth->getState()}")
+            ->get("/oauth/discord/callback", query: ['code' => 'some-fake-code', 'state' => $oauth->getState()])
             ->assertRedirect('/');
 
         // We assert that an access token was retrieved

packages/auth/src/Exceptions/OAuthStateWasInvalid.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Auth\Exceptions;
+
+use Exception;
+
+final class OAuthStateWasInvalid extends Exception implements AuthenticationException
+{
+}

packages/auth/src/OAuth/GenericOAuthClient.php

@@ -4,28 +4,54 @@
 
 namespace Tempest\Auth\OAuth;
 
+use BackedEnum;
+use Closure;
 use League\OAuth2\Client\Provider\AbstractProvider;
 use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
 use League\OAuth2\Client\Token\AccessToken;
+use Tempest\Auth\Authentication\Authenticatable;
+use Tempest\Auth\Authentication\Authenticator;
+use Tempest\Auth\Exceptions\OAuthStateWasInvalid;
 use Tempest\Auth\Exceptions\OAuthTokenCouldNotBeRetrieved;
 use Tempest\Auth\Exceptions\OAuthUserCouldNotBeRetrieved;
+use Tempest\Http\Request;
+use Tempest\Http\Responses\Redirect;
+use Tempest\Http\Session\Session;
 use Tempest\Mapper\ObjectFactory;
 use Tempest\Router\UriGenerator;
+use UnitEnum;
 
-final readonly class GenericOAuthClient implements OAuthClient
+final class GenericOAuthClient implements OAuthClient
 {
     private AbstractProvider $provider;
 
     public function __construct(
-        private(set) OAuthConfig $config,
-        private UriGenerator $uri,
-        private ObjectFactory $factory,
+        private(set) readonly OAuthConfig $config,
+        private readonly UriGenerator $uri,
+        private readonly ObjectFactory $factory,
+        private readonly Session $session,
+        private readonly Authenticator $authenticator,
         ?AbstractProvider $provider = null,
     ) {
         $this->provider = $provider ?? $this->config->createProvider();
     }
 
-    public function getAuthorizationUrl(array $scopes = [], array $options = []): string
+    public string $sessionKey {
+        get {
+            $tag = $this->config->tag;
+
+            $key = match (true) {
+                is_string($tag) => $tag,
+                $tag instanceof BackedEnum => $tag->value,
+                $tag instanceof UnitEnum => $tag->name,
+                default => 'default',
+            };
+
+            return "oauth:{$key}";
+        }
+    }
+
+    public function buildAuthorizationUrl(array $scopes = [], array $options = []): string
     {
         return $this->provider->getAuthorizationUrl([
             'scope' => $scopes ?? $this->config->scopes,
@@ -34,12 +60,19 @@ public function getAuthorizationUrl(array $scopes = [], array $options = []): st
         ]);
     }
 
+    public function createRedirect(array $scopes = [], array $options = []): Redirect
+    {
+        $this->session->set($this->sessionKey, $this->provider->getState());
+
+        return new Redirect($this->buildAuthorizationUrl());
+    }
+
     public function getState(): ?string
     {
         return $this->provider->getState();
     }
 
-    public function getAccessToken(string $code): AccessToken
+    public function requestAccessToken(string $code): AccessToken
     {
         try {
             return $this->provider->getAccessToken('authorization_code', [
@@ -51,7 +84,7 @@ public function getAccessToken(string $code): AccessToken
         }
     }
 
-    public function getUser(AccessToken $token): OAuthUser
+    public function fetchUser(AccessToken $token): OAuthUser
     {
         try {
             return $this->config->mapUser(
@@ -63,10 +96,22 @@ public function getUser(AccessToken $token): OAuthUser
         }
     }
 
-    public function fetchUser(string $code): OAuthUser
+    public function authenticate(Request $request, Closure $map): Authenticatable
     {
-        return $this->getUser(
-            token: $this->getAccessToken($code),
+        if ($this->session->get($this->sessionKey) !== $request->get('state')) {
+            throw new OAuthStateWasInvalid();
+        }
+
+        $user = $this->fetchUser(
+            token: $this->requestAccessToken(
+                code: $request->get('code'),
+            ),
         );
+
+        $authenticable = $map($user);
+
+        $this->authenticator->authenticate($authenticable);
+
+        return $authenticable;
     }
 }

packages/auth/src/OAuth/OAuthClient.php

@@ -4,14 +4,26 @@
 
 namespace Tempest\Auth\OAuth;
 
+use Closure;
 use League\OAuth2\Client\Token\AccessToken;
+use Tempest\Auth\Authentication\Authenticatable;
+use Tempest\Http\Request;
+use Tempest\Http\Responses\Redirect;
 
+/**
+ * @template T of Authenticatable
+ */
 interface OAuthClient
 {
     /**
      * Gets the authorization URL for the OAuth provider.
      */
-    public function getAuthorizationUrl(array $scopes = [], array $options = []): string;
+    public function buildAuthorizationUrl(array $scopes = [], array $options = []): string;
+
+    /**
+     * Creates a redirect response for the OAuth flow.
+     */
+    public function createRedirect(array $scopes = [], array $options = []): Redirect;
 
     /**
      * Gets the state parameter for CSRF protection.
@@ -21,15 +33,19 @@ public function getState(): ?string;
     /**
      * Exchanges an authorization code for an access token.
      */
-    public function getAccessToken(string $code): AccessToken;
+    public function requestAccessToken(string $code): AccessToken;
 
     /**
      * Gets user information from an OAuth provider using an access token.
      */
-    public function getUser(AccessToken $token): OAuthUser;
+    public function fetchUser(AccessToken $token): OAuthUser;
 
     /**
-     * Completes OAuth flow with code and get user information.
+     * Authenticates a user based on the given OAuth callback request.
+     *
+     * @template T of Authenticatable
+     *
+     * @param Closure(OAuthUser): T $map A callback that should return an authenticatable model from the given OAuthUser. Typically, the callback is also responsible for saving the user to the database.
      */
-    public function fetchUser(string $code): OAuthUser;
+    public function authenticate(Request $request, Closure $map): Authenticatable;
 }

packages/auth/src/OAuth/Testing/OAuthTester.php

@@ -4,6 +4,7 @@
 
 namespace Tempest\Auth\OAuth\Testing;
 
+use Tempest\Auth\Authentication\Authenticator;
 use Tempest\Auth\OAuth\OAuthClient;
 use Tempest\Auth\OAuth\OAuthConfig;
 use Tempest\Auth\OAuth\OAuthUser;
@@ -24,10 +25,11 @@ public function fake(OAuthUser $user, null|string|UnitEnum $tag = null): Testing
     {
         $config = $this->container->get(OAuthConfig::class, $tag);
         $uri = $this->container->get(UriGenerator::class);
+        $authenticator = $this->container->get(Authenticator::class);
 
         $this->container->singleton(
             className: OAuthClient::class,
-            definition: $client = new TestingOAuthClient($user, $config, $uri),
+            definition: $client = new TestingOAuthClient($user, $config, $authenticator, $uri),
             tag: $tag,
         );
 

packages/auth/src/OAuth/Testing/TestingOAuthClient.php

@@ -4,11 +4,16 @@
 
 namespace Tempest\Auth\OAuth\Testing;
 
+use Closure;
 use League\OAuth2\Client\Token\AccessToken;
 use PHPUnit\Framework\Assert;
+use Tempest\Auth\Authentication\Authenticatable;
+use Tempest\Auth\Authentication\Authenticator;
 use Tempest\Auth\OAuth\OAuthClient;
 use Tempest\Auth\OAuth\OAuthConfig;
 use Tempest\Auth\OAuth\OAuthUser;
+use Tempest\Http\Request;
+use Tempest\Http\Responses\Redirect;
 use Tempest\Router\UriGenerator;
 use Tempest\Support\Arr;
 use Tempest\Support\Random;
@@ -38,19 +43,23 @@ final class TestingOAuthClient implements OAuthClient
         get => Arr\last($this->authorizationUrls)['url'] ?? null;
     }
 
+    /** @var array{url: string, scopes: array, options: array, state: string}[] */
     private array $authorizationUrls = [];
 
+    /** @var array{access_token: string, token_type: 'Bearer', expires_in: int, code: string}[] */
     private array $accessTokens = [];
 
-    private array $callbacks = [];
+    /** @var array{token: AccessToken, code: string, user: OAuthUser}[] */
+    private array $users = [];
 
     public function __construct(
         private(set) OAuthUser $user,
-        private(set) OAuthConfig $config,
-        private UriGenerator $uri,
+        private(set) readonly OAuthConfig $config,
+        private readonly Authenticator $authenticator,
+        private readonly UriGenerator $uri,
     ) {}
 
-    public function getAuthorizationUrl(array $scopes = [], array $options = []): string
+    public function buildAuthorizationUrl(array $scopes = [], array $options = []): string
     {
         $this->state = Random\secure_string(16);
 
@@ -80,12 +89,13 @@ public function getState(): ?string
         return $this->state;
     }
 
-    public function getAccessToken(string $code): AccessToken
+    public function requestAccessToken(string $code): AccessToken
     {
         $token = new AccessToken([
-            'access_token' => 'fat-' . $code,
+            'access_token' => 'tok-' . $code,
             'token_type' => 'Bearer',
             'expires_in' => 3600,
+            'code' => $code,
         ]);
 
         $this->accessTokens[] = [
@@ -96,23 +106,31 @@ public function getAccessToken(string $code): AccessToken
         return $token;
     }
 
-    public function getUser(AccessToken $token): OAuthUser
+    public function fetchUser(AccessToken $token): OAuthUser
     {
+        $this->users[] = [
+            'token' => $token,
+            'code' => Arr\get_by_key($token->getValues(), 'code'),
+            'user' => $this->user,
+        ];
+
         return $this->user;
     }
 
-    public function fetchUser(string $code): OAuthUser
+    public function createRedirect(array $scopes = [], array $options = []): Redirect
     {
-        $token = $this->getAccessToken($code);
-        $user = $this->getUser($token);
+        return new Redirect($this->buildAuthorizationUrl($scopes, $options));
+    }
 
-        $this->callbacks[] = [
-            'code' => $code,
-            'token' => $token,
-            'user' => $user,
-        ];
+    public function authenticate(Request $request, Closure $map): Authenticatable
+    {
+        $user = $this->fetchUser($this->requestAccessToken($request->get('code')));
+
+        $authenticatable = $map($user);
+
+        $this->authenticator->authenticate($authenticatable);
 
-        return $user;
+        return $authenticatable;
     }
 
     /**
@@ -189,8 +207,8 @@ public function assertAuthorizationUrlGenerated(?array $scopes = null, ?array $o
     public function assertUserFetched(string $code): void
     {
         Assert::assertNotEmpty(
-            actual: array_filter($this->callbacks, fn (array $callback) => $callback['code'] === $code),
-            message: sprintf('Callback with code "%s" was not handled.', $code),
+            actual: array_filter($this->users, fn (array $user) => $user['code'] === $code),
+            message: sprintf('User with code "%s" was not handled.', $code),
         );
     }
 
@@ -238,6 +256,6 @@ public function getAccessTokenCount(): int
      */
     public function getCallbackCount(): int
     {
-        return count($this->callbacks);
+        return count($this->users);
     }
 }

packages/http/src/Responses/Redirect.php

@@ -12,8 +12,9 @@ final class Redirect implements Response
 {
     use IsResponse;
 
-    public function __construct(string $to)
-    {
+    public function __construct(
+        private(set) string $to,
+    ) {
         $this->status = Status::FOUND;
         $this->addHeader('Location', $to);
     }