Improving exception message in case of expression language syntax error

Author: moufmouf

src/FieldsBuilder.php

@@ -234,6 +234,7 @@ private function getFieldsByAnnotations(?object $controller, string $annotationN
             }
 
             $fieldDescriptor = new QueryFieldDescriptor();
+            $fieldDescriptor->setRefMethod($refMethod);
 
             $docBlockObj     = $this->cachedDocBlockFactory->getDocBlock($refMethod);
             $docBlockComment = $docBlockObj->getSummary() . "\n" . $docBlockObj->getDescription()->render();
@@ -390,6 +391,7 @@ private function getQueryFieldsFromSourceFields(array $sourceFields, ReflectionC
             }
 
             $fieldDescriptor = new QueryFieldDescriptor();
+            $fieldDescriptor->setRefMethod($refMethod);
             $fieldDescriptor->setName($sourceField->getName());
 
             $methodName = $refMethod->getName();

src/Middlewares/BadExpressionInSecurityException.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Middlewares;
+
+use Exception;
+use TheCodingMachine\GraphQLite\QueryFieldDescriptor;
+use Throwable;
+
+/**
+ * Exception wrapping exceptions occurring when the Security annotation is evaluated
+ */
+class BadExpressionInSecurityException extends Exception
+{
+    public static function wrapException(Throwable $e, QueryFieldDescriptor $fieldDescriptor): self
+    {
+        $refMethod = $fieldDescriptor->getRefMethod();
+        $message = 'An error occurred while evaluating expression in @Security annotation of method "' . $refMethod->getDeclaringClass()->getName() . '::' . $refMethod->getName() . '": ' . $e->getMessage();
+
+        return new self($message, $e->getCode(), $e);
+    }
+}

src/Middlewares/SecurityFieldMiddleware.php

@@ -17,6 +17,7 @@
 use TheCodingMachine\GraphQLite\QueryFieldDescriptor;
 use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
 use TheCodingMachine\GraphQLite\Security\AuthorizationServiceInterface;
+use Throwable;
 use Webmozart\Assert\Assert;
 use function array_combine;
 use function array_keys;
@@ -88,11 +89,17 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler
 
         $parameters = $queryFieldDescriptor->getParameters();
 
-        $queryFieldDescriptor->setCallable(function (...$args) use ($securityAnnotations, $callable, $failWith, $parameters) {
+        $queryFieldDescriptor->setCallable(function (...$args) use ($securityAnnotations, $callable, $failWith, $parameters, $queryFieldDescriptor) {
             $variables = $this->getVariables($args, $parameters, $callable);
 
             foreach ($securityAnnotations as $annotation) {
-                if (! $this->language->evaluate($annotation->getExpression(), $variables)) {
+                try {
+                    $authorized = $this->language->evaluate($annotation->getExpression(), $variables);
+                } catch (Throwable $e) {
+                    throw BadExpressionInSecurityException::wrapException($e, $queryFieldDescriptor);
+                }
+
+                if (! $authorized) {
                     if ($annotation->isFailWithSet()) {
                         return $annotation->getFailWith();
                     }

src/QueryFieldDescriptor.php

@@ -6,6 +6,7 @@
 
 use GraphQL\Type\Definition\OutputType;
 use GraphQL\Type\Definition\Type;
+use ReflectionMethod;
 use TheCodingMachine\GraphQLite\Annotations\MiddlewareAnnotations;
 use TheCodingMachine\GraphQLite\Parameters\ParameterInterface;
 
@@ -40,6 +41,8 @@ class QueryFieldDescriptor
     private $comment;
     /** @var MiddlewareAnnotations */
     private $middlewareAnnotations;
+    /** @var ReflectionMethod */
+    private $refMethod;
 
     public function getName(): string
     {
@@ -160,4 +163,14 @@ public function setMiddlewareAnnotations(MiddlewareAnnotations $middlewareAnnota
     {
         $this->middlewareAnnotations = $middlewareAnnotations;
     }
+
+    public function getRefMethod(): ReflectionMethod
+    {
+        return $this->refMethod;
+    }
+
+    public function setRefMethod(ReflectionMethod $refMethod): void
+    {
+        $this->refMethod = $refMethod;
+    }
 }

tests/AbstractQueryProviderTest.php

@@ -15,7 +15,9 @@
 use Mouf\Picotainer\Picotainer;
 use PHPUnit\Framework\TestCase;
 use Psr\Container\ContainerInterface;
+use Symfony\Component\Cache\Adapter\Psr16Adapter;
 use Symfony\Component\Cache\Simple\ArrayCache;
+use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
 use Symfony\Component\Lock\Factory as LockFactory;
 use Symfony\Component\Lock\Store\FlockStore;
 use Symfony\Component\Lock\Store\SemaphoreStore;
@@ -37,7 +39,10 @@
 use TheCodingMachine\GraphQLite\Containers\EmptyContainer;
 use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer;
 use TheCodingMachine\GraphQLite\Middlewares\AuthorizationFieldMiddleware;
+use TheCodingMachine\GraphQLite\Middlewares\FieldMiddlewarePipe;
+use TheCodingMachine\GraphQLite\Middlewares\SecurityFieldMiddleware;
 use TheCodingMachine\GraphQLite\Reflection\CachedDocBlockFactory;
+use TheCodingMachine\GraphQLite\Security\SecurityExpressionLanguageProvider;
 use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
 use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService;
 use TheCodingMachine\GraphQLite\Types\ArgumentResolver;
@@ -260,6 +265,14 @@ protected function getAnnotationReader(): AnnotationReader
 
     protected function buildFieldsBuilder(): FieldsBuilder
     {
+        $fieldMiddlewarePipe = new FieldMiddlewarePipe();
+        $fieldMiddlewarePipe->pipe(new AuthorizationFieldMiddleware(
+            new VoidAuthenticationService(),
+            new VoidAuthorizationService()
+        ));
+        $expressionLanguage = new ExpressionLanguage(new Psr16Adapter(new ArrayCache()), [new SecurityExpressionLanguageProvider()]);
+        $fieldMiddlewarePipe->pipe(new SecurityFieldMiddleware($expressionLanguage, new VoidAuthenticationService(), new VoidAuthorizationService()));
+
         return new FieldsBuilder(
             $this->getAnnotationReader(),
             $this->getTypeMapper(),
@@ -274,10 +287,7 @@ protected function buildFieldsBuilder(): FieldsBuilder
             new CompositeParameterMapper([
                 new ResolveInfoParameterMapper()
             ]),
-            new AuthorizationFieldMiddleware(
-                new VoidAuthenticationService(),
-                new VoidAuthorizationService()
-            )
+            $fieldMiddlewarePipe
         );
     }
 

tests/FieldsBuilderTest.php

@@ -23,6 +23,7 @@
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerNoReturnType;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithArrayParam;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithArrayReturnType;
+use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithBadSecurity;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithFailWith;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInputType;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidInputType;
@@ -57,6 +58,7 @@
 use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper;
 use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper;
 use TheCodingMachine\GraphQLite\Middlewares\AuthorizationFieldMiddleware;
+use TheCodingMachine\GraphQLite\Middlewares\BadExpressionInSecurityException;
 use TheCodingMachine\GraphQLite\Parameters\MissingArgumentException;
 use TheCodingMachine\GraphQLite\Reflection\CachedDocBlockFactory;
 use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
@@ -665,4 +667,23 @@ public function testPrefetchMethod(): void
         $this->assertSame('string', $testField->args[0]->name);
         $this->assertSame('int', $testField->args[1]->name);
     }
+
+    public function testSecurityBadQuery(): void
+    {
+        $controller = new TestControllerWithBadSecurity();
+
+        $queryProvider = $this->buildFieldsBuilder();
+
+        $queries = $queryProvider->getQueries($controller);
+
+        $this->assertCount(1, $queries);
+        $query = $queries[0];
+        $this->assertSame('testBadSecurity', $query->name);
+
+        $resolve = $query->resolveFn;
+
+        $this->expectException(BadExpressionInSecurityException::class);
+        $this->expectExceptionMessage('An error occurred while evaluating expression in @Security annotation of method "TheCodingMachine\GraphQLite\Fixtures\TestControllerWithBadSecurity::testBadSecurity": Unexpected token "name" of value "is" around position 6 for expression `this is not valid expression language`.');
+        $result = $resolve(new stdClass(), [], null, $this->createMock(ResolveInfo::class));
+    }
 }

tests/Fixtures/TestControllerWithBadSecurity.php

@@ -0,0 +1,24 @@
+<?php
+
+
+namespace TheCodingMachine\GraphQLite\Fixtures;
+
+use ArrayObject;
+use TheCodingMachine\GraphQLite\Annotations\FailWith;
+use TheCodingMachine\GraphQLite\Annotations\Logged;
+use TheCodingMachine\GraphQLite\Annotations\Mutation;
+use TheCodingMachine\GraphQLite\Annotations\Query;
+use TheCodingMachine\GraphQLite\Annotations\Right;
+use TheCodingMachine\GraphQLite\Annotations\Security;
+
+class TestControllerWithBadSecurity
+{
+    /**
+     * @Query
+     * @Security("this is not valid expression language")
+     */
+    public function testBadSecurity(): TestObject
+    {
+        return new TestObject('foo');
+    }
+}