Improve how InjectUser handles unauthenticated user (#575)

Author: oprypkhantc

src/Mappers/Parameters/InjectUserParameterHandler.php

@@ -30,6 +30,10 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock,
             return $next->mapParameter($parameter, $docBlock, $paramTagType, $parameterAnnotations);
         }
 
-        return new InjectUserParameter($this->authenticationService);
+        // Now we need to know if authentication is optional. If type isn't nullable we'll assume the user
+        // is required for that parameter. If type is missing, it's also assumed optional.
+        $optional = $parameter->getType()?->allowsNull() ?? true;
+
+        return new InjectUserParameter($this->authenticationService, $optional);
     }
 }

src/Parameters/InjectUserParameter.php

@@ -5,20 +5,31 @@
 namespace TheCodingMachine\GraphQLite\Parameters;
 
 use GraphQL\Type\Definition\ResolveInfo;
+use TheCodingMachine\GraphQLite\Middlewares\MissingAuthorizationException;
 use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
 
 /**
  * A parameter filled from the current user.
  */
 class InjectUserParameter implements ParameterInterface
 {
-    public function __construct(private readonly AuthenticationServiceInterface $authenticationService)
+    public function __construct(
+        private readonly AuthenticationServiceInterface $authenticationService,
+        private readonly bool $optional,
+    )
     {
     }
 
     /** @param array<string, mixed> $args */
     public function resolve(object|null $source, array $args, mixed $context, ResolveInfo $info): object|null
     {
-        return $this->authenticationService->getUser();
+        $user = $this->authenticationService->getUser();
+
+        // If user is required but wasn't provided, we'll throw unauthorized error the same way #[Logged] does.
+        if (! $user && ! $this->optional) {
+            throw MissingAuthorizationException::unauthorized();
+        }
+
+        return $user;
     }
 }

tests/Integration/EndToEndTest.php

@@ -1737,6 +1737,29 @@ public function getUser(): object|null
         $this->assertSame(42, $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['data']['injectedUser']);
     }
 
+    public function testEndToEndInjectUserUnauthenticated(): void
+    {
+        $container = $this->createContainer([
+            AuthenticationServiceInterface::class => static fn () => new VoidAuthenticationService(),
+        ]);
+
+        $schema = $container->get(Schema::class);
+        assert($schema instanceof Schema);
+
+        $queryString = '
+            query {
+                injectedUser
+            }
+        ';
+
+        $result = GraphQL::executeQuery(
+            $schema,
+            $queryString,
+        );
+
+        $this->assertSame('You need to be logged to access this field', $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors'][0]['message']);
+    }
+
     public function testInputOutputNameConflict(): void
     {
         $arrayAdapter = new ArrayAdapter();

tests/Mappers/Parameters/InjectUserParameterHandlerTest.php

@@ -0,0 +1,61 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Mappers\Parameters;
+
+use Generator;
+use phpDocumentor\Reflection\DocBlock;
+use ReflectionMethod;
+use stdClass;
+use TheCodingMachine\GraphQLite\AbstractQueryProviderTest;
+use TheCodingMachine\GraphQLite\Annotations\InjectUser;
+use TheCodingMachine\GraphQLite\Parameters\InjectUserParameter;
+use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
+
+class InjectUserParameterHandlerTest extends AbstractQueryProviderTest
+{
+    /**
+     * @dataProvider mapParameterProvider
+     */
+    public function testMapParameter(bool $optional, string $method): void
+    {
+        $authenticationService = $this->createMock(AuthenticationServiceInterface::class);
+
+        $refMethod = new ReflectionMethod(__CLASS__, $method);
+        $parameter = $refMethod->getParameters()[0];
+
+        $mapped = (new InjectUserParameterHandler($authenticationService))->mapParameter(
+            $parameter,
+            new DocBlock(),
+            null,
+            $this->getAnnotationReader()->getParameterAnnotationsPerParameter([$parameter])['user'],
+            $this->createMock(ParameterHandlerInterface::class),
+        );
+
+        self::assertEquals(
+            new InjectUserParameter($authenticationService, $optional),
+            $mapped
+        );
+    }
+
+    public function mapParameterProvider(): Generator
+    {
+        yield 'required user' => [false, 'requiredUser'];
+        yield 'optional user' => [true, 'optionalUser'];
+        yield 'missing type' => [true, 'missingType'];
+    }
+
+    private function requiredUser(
+        #[InjectUser] stdClass $user,
+    ) {
+    }
+
+    private function optionalUser(
+        #[InjectUser] stdClass|null $user,
+    ) {
+    }
+
+    private function missingType(
+        #[InjectUser] $user,
+    ) {
+    }
+}

tests/Parameters/InjectUserParameterTest.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Parameters;
+
+use Generator;
+use GraphQL\Type\Definition\ResolveInfo;
+use PHPUnit\Framework\TestCase;
+use stdClass;
+use TheCodingMachine\GraphQLite\Middlewares\MissingAuthorizationException;
+use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
+
+class InjectUserParameterTest extends TestCase
+{
+    /**
+     * @dataProvider resolveReturnsUserProvider
+     */
+    public function testResolveReturnsUser(stdClass|null $user, bool $optional): void
+    {
+        $authenticationService = $this->createMock(AuthenticationServiceInterface::class);
+        $authenticationService->method('getUser')
+            ->willReturn($user);
+
+        $resolved = (new InjectUserParameter($authenticationService, $optional))->resolve(
+            null,
+            [],
+            null,
+            $this->createStub(ResolveInfo::class)
+        );
+
+        self::assertSame($user, $resolved);
+    }
+
+    public function resolveReturnsUserProvider(): Generator
+    {
+        yield 'non optional and has user' => [new stdClass(), false];
+        yield 'optional and has user' => [new stdClass(), true];
+        yield 'optional and doesnt have user' => [null, true];
+    }
+
+    public function testThrowsMissingAuthorization(): void
+    {
+        $authenticationService = $this->createMock(AuthenticationServiceInterface::class);
+        $authenticationService->method('getUser')
+            ->willReturn(null);
+
+        $this->expectExceptionObject(MissingAuthorizationException::unauthorized());
+
+        (new InjectUserParameter($authenticationService, false))->resolve(
+            null,
+            [],
+            null,
+            $this->createStub(ResolveInfo::class)
+        );
+    }
+}

website/docs/annotations-reference.md

@@ -162,6 +162,8 @@ to access it (according to the `@Logged` and `@Right` annotations).
 Use the `@InjectUser` annotation to inject an instance of the current user logged in into a parameter of your
 query / mutation / field.
 
+See [the authentication and authorization page](authentication-authorization.mdx) for more details.
+
 **Applies on**: methods annotated with `@Query`, `@Mutation` or `@Field`.
 
 Attribute      | Compulsory | Type   | Definition

website/docs/authentication-authorization.mdx

@@ -223,7 +223,8 @@ The `@InjectUser` annotation can be used next to:
 * `@Field` annotations
 
 The object injected as the current user depends on your framework. It is in fact the object returned by the
-["authentication service" configured in GraphQLite](implementing-security.md).
+["authentication service" configured in GraphQLite](implementing-security.md). If user is not authenticated and
+parameter's type is not nullable, an authorization exception is thrown, similar to `@Logged` annotation.
 
 ## Hiding fields / queries / mutations