Authenticator: Improve auth methods handling, esp. for api endpoints

.gitlab-ci.yml

@@ -217,7 +217,7 @@ test:
     - ./bin/migrate
   script:
     - >-
-      php -d pcov.enabled=1 -d pcov.directory=. vendor/bin/phpunit -vvv --colors=never
+      php -d memory_limit=1024M -d pcov.enabled=1 -d pcov.directory=. vendor/bin/phpunit -vvv --colors=never
       --coverage-text --coverage-html "${HOMEDIR}/coverage/"
       --log-junit "${HOMEDIR}/unittests.xml"
   after_script:

includes/controller/shifts_controller.php

@@ -386,15 +386,10 @@ function shift_next_controller()
  */
 function shifts_json_export_controller()
 {
-    $request = request();
+    $user = auth()->userFromApi();
-    $user = auth()->apiUser('key');
 
-    if (
+    if (!$user) {
-        !$request->has('key')
+        throw new HttpForbidden('{"error":"Missing or invalid ?key="}', ['content-type' => 'application/json']);
-        || !$request->input('key')
-        || !$user
-    ) {
-        throw new HttpForbidden('{"error":"Missing or invalid key"}', ['content-type' => 'application/json']);
     }
 
     if (!auth()->can('shifts_json_export')) {

includes/pages/user_atom.php

@@ -11,14 +11,10 @@ use Illuminate\Support\Collection as SupportCollection;
 function user_atom()
 {
     $request = request();
-    $user = auth()->apiUser('key');
+    $user = auth()->userFromApi();
 
-    if (
+    if (!$user) {
-        !$request->has('key')
+        throw new HttpForbidden('Missing or invalid ?key=', ['content-type' => 'text/text']);
-        || !$request->input('key')
-        || empty($user)
-    ) {
-        throw new HttpForbidden('Missing or invalid key', ['content-type' => 'text/text']);
     }
 
     if (!auth()->can('atom')) {

includes/pages/user_ical.php

@@ -9,15 +9,10 @@ use Illuminate\Support\Collection;
  */
 function user_ical()
 {
-    $request = request();
+    $user = auth()->userFromApi();
-    $user = auth()->apiUser('key');
 
-    if (
+    if (!$user) {
-        !$request->has('key')
+        throw new HttpForbidden('Missing or invalid ?key=', ['content-type' => 'text/text']);
-        || !$request->input('key')
-        || !$user
-    ) {
-        throw new HttpForbidden('Missing or invalid key', ['content-type' => 'text/text']);
     }
 
     if (!auth()->can('ical')) {

src/Helpers/Authenticator.php

@@ -6,6 +6,7 @@ use Carbon\Carbon;
 use Engelsystem\Models\Group;
 use Engelsystem\Models\User\User;
 use Engelsystem\Models\User\User as UserRepository;
+use Illuminate\Support\Str;
 use Psr\Http\Message\ServerRequestInterface;
 use Symfony\Component\HttpFoundation\Session\Session;
 
@@ -30,7 +31,7 @@ class Authenticator
     }
 
     /**
-     * Load the user from session
+     * Load the user from session or api auth
      */
     public function user(): ?User
     {
@@ -38,47 +39,50 @@ class Authenticator
             return $this->user;
         }
 
+        $this->user = $this->userFromSession();
+        if (!$this->user && request()->getAttribute('route-api', false)) {
+            $this->user = $this->userFromApi();
+        }
+
+        return $this->user;
+    }
+
+    /**
+     * Load the user from session
+     */
+    public function userFromSession(): ?User
+    {
+        if ($this->user) {
+            return $this->user;
+        }
+
         $userId = $this->session->get('user_id');
         if (!$userId) {
             return null;
         }
 
-        $user = $this
+        $this->user = $this
             ->userRepository
             ->find($userId);
-        if (!$user) {
-            return null;
-        }
-
-        $this->user = $user;
 
         return $this->user;
     }
 
     /**
-     * Get the user by his api key
+     * Get the user by its api key
      */
-    public function apiUser(string $parameter = 'api_key'): ?User
+    public function userFromApi(): ?User
     {
         if ($this->user) {
             return $this->user;
         }
 
-        $params = $this->request->getQueryParams();
+        $this->user = $this->userByHeaders();
-        if (!isset($params[$parameter])) {
+        if ($this->user) {
-            return null;
+            return $this->user;
         }
 
-        /** @var User|null $user */
+        $this->user = $this->userByQueryParam();
-        $user = $this
-            ->userRepository
-            ->whereApiKey($params[$parameter])
-            ->first();
-        if (!$user) {
-            return $this->user();
-        }
-
-        $this->user = $user;
 
         return $this->user;
     }
@@ -150,6 +154,50 @@ class Authenticator
         return true;
     }
 
+    /**
+     * Get the user by authorization bearer or x-api-key headers
+     */
+    protected function userByHeaders(): ?User
+    {
+        $header = $this->request->getHeader('authorization');
+        if (!empty($header) && Str::startsWith(Str::lower($header[0]), 'bearer ')) {
+            return $this->userByApiKey(Str::substr($header[0], 7));
+        }
+
+        $header = $this->request->getHeader('x-api-key');
+        if (!empty($header)) {
+            return $this->userByApiKey($header[0]);
+        }
+
+        return null;
+    }
+
+    /**
+     * Get the user by query parameters
+     */
+    protected function userByQueryParam(): ?User
+    {
+        $params = $this->request->getQueryParams();
+        if (!empty($params['key'])) {
+            $this->user = $this->userByApiKey($params['key']);
+        }
+
+        return $this->user;
+    }
+
+    /**
+     * Get the user by its api key
+     */
+    protected function userByApiKey(string $key): ?User
+    {
+        $this->user = $this
+            ->userRepository
+            ->whereApiKey($key)
+            ->first();
+
+        return $this->user;
+    }
+
     public function setPassword(User $user, string $password): void
     {
         $user->password = password_hash($password, $this->passwordAlgorithm);

src/Middleware/RequestHandler.php

@@ -26,6 +26,8 @@ class RequestHandler implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $requestHandler = $request->getAttribute('route-request-handler');
+        $this->container->instance(ServerRequestInterface::class, $request);
+        $this->container->instance('request', $request);
 
         /** @var CallableHandler|MiddlewareInterface|RequestHandlerInterface $requestHandler */
         $requestHandler = $this->resolveRequestHandler($requestHandler);

src/Middleware/SessionHandler.php

@@ -18,13 +18,15 @@ class SessionHandler implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $requestPath = $request->getAttribute('route-request-path');
+        $isApi = in_array($requestPath, $this->paths);
+        $request = $request->withAttribute('route-api', $isApi);
 
         $return = $handler->handle($request);
 
         $cookies = $request->getCookieParams();
         if (
-            $this->session instanceof NativeSessionStorage
+            $isApi
-            && in_array($requestPath, $this->paths)
+            && $this->session instanceof NativeSessionStorage
             && !isset($cookies[$this->session->getName()])
         ) {
             $this->destroyNative();

src/helpers.php

@@ -14,6 +14,8 @@ use Symfony\Component\HttpFoundation\Session\SessionInterface;
 
 /**
  * Get the global app instance
+ * @return mixed|Application
+ * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation
  */
 function app(string $id = null): mixed
 {
@@ -44,6 +46,8 @@ function back(int $status = 302, array $headers = []): Response
 
 /**
  * Get or set config values
+ * @return mixed|Config
+ * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation
  */
 function config(string|array $key = null, mixed $default = null): mixed
 {
@@ -87,6 +91,10 @@ function redirect(string $path, int $status = 302, array $headers = []): Respons
     return $redirect->to($path, $status, $headers);
 }
 
+/**
+ * @return mixed|Request
+ * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation
+ */
 function request(string $key = null, mixed $default = null): mixed
 {
     /** @var Request $request */
@@ -114,6 +122,10 @@ function response(mixed $content = '', int $status = 200, array $headers = []):
     return $response;
 }
 
+/**
+ * @return mixed|SessionInterface
+ * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation
+ */
 function session(string $key = null, mixed $default = null): mixed
 {
     /** @var SessionInterface $session */
@@ -175,9 +187,6 @@ function url(string $path = null, array $parameters = []): UrlGeneratorInterface
     return $urlGenerator->to($path, $parameters);
 }
 
-/**
- * @param mixed[] $data
- */
 function view(string $template = null, array $data = []): Renderer|string
 {
     /** @var Renderer $renderer */

tests/Unit/Helpers/AuthenticatorTest.php

@@ -3,6 +3,7 @@
 namespace Engelsystem\Test\Unit\Helpers;
 
 use Engelsystem\Helpers\Authenticator;
+use Engelsystem\Http\Request;
 use Engelsystem\Models\Group;
 use Engelsystem\Models\Privilege;
 use Engelsystem\Models\User\User;
@@ -12,97 +13,164 @@ use Engelsystem\Test\Unit\ServiceProviderTest;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Http\Message\ServerRequestInterface;
 use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
 
 class AuthenticatorTest extends ServiceProviderTest
 {
     use HasDatabase;
 
     /**
-     * @covers \Engelsystem\Helpers\Authenticator::__construct
      * @covers \Engelsystem\Helpers\Authenticator::user
+     * @covers \Engelsystem\Helpers\Authenticator::__construct
      */
-    public function testUser(): void
+    public function testUserNotAuthorized(): void
     {
-        /** @var ServerRequestInterface|MockObject $request */
+        $request = new Request();
-        $request = $this->getMockForAbstractClass(ServerRequestInterface::class);
+        $session = new Session(new MockArraySessionStorage());
-        /** @var Session|MockObject $session */
-        $session = $this->createMock(Session::class);
         /** @var UserModelImplementation|MockObject $userRepository */
         $userRepository = new UserModelImplementation();
-        /** @var User|MockObject $user */
+        $this->app->instance('request', $request);
-        $user = $this->createMock(User::class);
-
-        $session->expects($this->exactly(3))
-            ->method('get')
-            ->with('user_id')
-            ->willReturnOnConsecutiveCalls(
-                null,
-                42,
-                1337
-            );
 
         $auth = new Authenticator($request, $session, $userRepository);
+        $user = $auth->user();
 
-        // Not in session
+        $this->assertNull($user);
-        $this->assertNull($auth->user());
-
-        // Unknown user
-        UserModelImplementation::$id = 42;
-        $this->assertNull($auth->user());
-
-        // User found
-        UserModelImplementation::$id = 1337;
-        UserModelImplementation::$user = $user;
-        $this->assertEquals($user, $auth->user());
-
-        // User cached
-        UserModelImplementation::$id = null;
-        UserModelImplementation::$user = null;
-        $this->assertEquals($user, $auth->user());
     }
 
     /**
-     * @covers \Engelsystem\Helpers\Authenticator::apiUser
+     * @covers \Engelsystem\Helpers\Authenticator::user
+     * @covers \Engelsystem\Helpers\Authenticator::userFromSession
      */
-    public function testApiUser(): void
+    public function testUserViaFromSession(): void
     {
-        /** @var ServerRequestInterface|MockObject $request */
+        $this->initDatabase();
-        $request = $this->getMockForAbstractClass(ServerRequestInterface::class);
-        /** @var Session|MockObject $session */
-        $session = $this->createMock(Session::class);
-        /** @var UserModelImplementation|MockObject $userRepository */
-        $userRepository = new UserModelImplementation();
-        /** @var User|MockObject $user */
-        $user = $this->createMock(User::class);
 
-        $request->expects($this->exactly(3))
+        $request = new Request();
-            ->method('getQueryParams')
+        $session = new Session(new MockArraySessionStorage());
-            ->with()
-            ->willReturnOnConsecutiveCalls(
-                [],
-                ['api_key' => 'iMaNot3xiSt1nGAp1Key!'],
-                ['foo_key' => 'SomeSecretApiKey']
-            );
 
-        /** @var Authenticator|MockObject $auth */
+        $session->set('user_id', 42);
-        $auth = new Authenticator($request, $session, $userRepository);
+        User::factory()->create(['id' => 42]);
 
-        // No key
+        $auth = new Authenticator($request, $session, new User());
-        $this->assertNull($auth->apiUser());
+        $user = $auth->user();
 
-        // Unknown user
+        $this->assertInstanceOf(User::class, $user);
-        UserModelImplementation::$apiKey = 'iMaNot3xiSt1nGAp1Key!';
+        $this->assertEquals(42, $user->id);
-        $this->assertNull($auth->apiUser());
 
-        // User found
+        // Cached in user()
-        UserModelImplementation::$apiKey = 'SomeSecretApiKey';
+        $user2 = $auth->user();
-        UserModelImplementation::$user = $user;
+        $this->assertEquals($user, $user2);
-        $this->assertEquals($user, $auth->apiUser('foo_key'));
 
-        // User cached
+        // Cached in userFromSession()
-        UserModelImplementation::$apiKey = null;
+        $user3 = $auth->userFromSession();
-        UserModelImplementation::$user = null;
+        $this->assertEquals($user, $user3);
-        $this->assertEquals($user, $auth->apiUser());
+    }
+
+    /**
+     * @covers \Engelsystem\Helpers\Authenticator::user
+     * @covers \Engelsystem\Helpers\Authenticator::userFromApi
+     * @covers \Engelsystem\Helpers\Authenticator::userByHeaders
+     */
+    public function testUserViaFromApi(): void
+    {
+        $this->initDatabase();
+
+        $request = new Request();
+        $session = new Session(new MockArraySessionStorage());
+
+        $request = $request->withHeader('Authorization', 'Bearer F00Bar');
+        $request = $request->withAttribute('route-api', true);
+        $this->app->instance('request', $request);
+        User::factory()->create(['api_key' => 'F00Bar']);
+
+        $auth = new Authenticator($request, $session, new User());
+        $user = $auth->user();
+
+        $this->assertInstanceOf(User::class, $user);
+        $this->assertEquals('F00Bar', $user->api_key);
+
+        // Cached in userFromApi()
+        $user2 = $auth->userFromApi();
+        $this->assertEquals($user, $user2);
+    }
+
+    /**
+     * @covers \Engelsystem\Helpers\Authenticator::userFromSession
+     */
+    public function testUserFromSessionNotFound(): void
+    {
+        $this->initDatabase();
+
+        $request = new Request();
+        $session = new Session(new MockArraySessionStorage());
+
+        $auth = new Authenticator($request, $session, new User());
+
+        $user = $auth->userFromSession();
+        $this->assertNull($user);
+
+        $session->set('user_id', 42);
+        $user2 = $auth->userFromSession();
+        $this->assertNull($user2);
+    }
+
+    /**
+     * @covers \Engelsystem\Helpers\Authenticator::userFromApi
+     * @covers \Engelsystem\Helpers\Authenticator::userByQueryParam
+     * @covers \Engelsystem\Helpers\Authenticator::userByApiKey
+     */
+    public function testUserFromApiByQueryParam(): void
+    {
+        $this->initDatabase();
+
+        $request = new Request();
+        $session = new Session(new MockArraySessionStorage());
+
+        $request = $request->withQueryParams(['key' => 'F00Bar']);
+
+        $auth = new Authenticator($request, $session, new User());
+
+        // User not found
+        $user = $auth->userFromApi();
+        $this->assertNull($user);
+
+        // User exists
+        User::factory()->create(['api_key' => 'F00Bar']);
+        $user2 = $auth->userFromApi();
+        $this->assertInstanceOf(User::class, $user2);
+        $this->assertEquals('F00Bar', $user2->api_key);
+    }
+
+    /**
+     * @covers \Engelsystem\Helpers\Authenticator::userByHeaders
+     */
+    public function testUserByHeaders(): void
+    {
+        $this->initDatabase();
+
+        $request = new Request();
+        $request = $request->withAttribute('route-api', true);
+        $session = new Session(new MockArraySessionStorage());
+        $this->app->instance('request', $request);
+
+        $auth = new Authenticator($request, $session, new User());
+
+        // Header not set
+        $user = $auth->userFromApi();
+        $this->assertNull($user);
+
+        // User not found
+        $request = $request->withHeader('x-api-key', 'SomeWrongKey');
+        $auth = new Authenticator($request, $session, new User());
+        $user = $auth->userFromApi();
+        $this->assertNull($user);
+
+        $request = $request->withHeader('x-api-key', 'F00Bar');
+        $auth = new Authenticator($request, $session, new User());
+        User::factory()->create(['api_key' => 'F00Bar']);
+        $user = $auth->user();
+        $this->assertInstanceOf(User::class, $user);
+        $this->assertEquals('F00Bar', $user->api_key);
     }
 
     /**

tests/Unit/Middleware/RequestHandlerTest.php

@@ -130,6 +130,9 @@ class RequestHandlerTest extends TestCase
             ->method('make')
             ->with($className)
             ->willReturn($middlewareInterface);
+        $container->expects($this->exactly(2))
+            ->method('instance')
+            ->withConsecutive([ServerRequestInterface::class, $request], ['request', $request]);
 
         $return = $middleware->process($request, $handler);
         $this->assertEquals($return, $response);

tests/Unit/Middleware/SessionHandlerTest.php

@@ -39,9 +39,14 @@ class SessionHandlerTest extends TestCase
         $request->expects($this->exactly(2))
             ->method('getAttribute')
             ->with('route-request-path')
-            ->willReturn('/foo');
+            ->willReturnOnConsecutiveCalls('/foo', '/lorem');
 
-        $sessionStorage->expects($this->exactly(2))
+        $request->expects($this->exactly(2))
+            ->method('withAttribute')
+            ->withConsecutive(['route-api', true], ['route-api', false])
+            ->willReturn($request);
+
+        $sessionStorage->expects($this->once())
             ->method('getName')
             ->willReturn('SESSION');