Skip to content

Commit db4d604

Browse files
oprypkhantcoprypkhantc
authored
Fix authorization middleware on long running servers (#571)
1 parent 1e64ea7 commit db4d604

9 files changed

+274
-78
lines changed

‎src/InputField.php

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use GraphQL\Type\Definition\ResolveInfo;
1414
use GraphQL\Type\Definition\Type;
1515
use TheCodingMachine\GraphQLite\Exceptions\GraphQLAggregateException;
16-
use TheCodingMachine\GraphQLite\Middlewares\MissingAuthorizationException;
1716
use TheCodingMachine\GraphQLite\Middlewares\ResolverInterface;
1817
use TheCodingMachine\GraphQLite\Middlewares\SourceResolverInterface;
1918
use TheCodingMachine\GraphQLite\Parameters\MissingArgumentException;
@@ -123,25 +122,6 @@ public function forConstructorHydration(): bool
123122
return $this->forConstructorHydration;
124123
}
125124

126-
/**
127-
* @param bool $isNotLogged False if the user is logged (and the error is a 403), true if the error is unlogged (the error is a 401)
128-
*
129-
* @return InputField
130-
*/
131-
public static function unauthorizedError(InputFieldDescriptor $fieldDescriptor, bool $isNotLogged): self
132-
{
133-
$callable = static function () use ($isNotLogged): void {
134-
if ($isNotLogged) {
135-
throw MissingAuthorizationException::unauthorized();
136-
}
137-
throw MissingAuthorizationException::forbidden();
138-
};
139-
140-
$fieldDescriptor->setResolver($callable);
141-
142-
return self::fromDescriptor($fieldDescriptor);
143-
}
144-
145125
private static function fromDescriptor(InputFieldDescriptor $fieldDescriptor): self
146126
{
147127
return new self(

‎src/Middlewares/AuthorizationFieldMiddleware.php

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use TheCodingMachine\GraphQLite\Annotations\HideIfUnauthorized;
1313
use TheCodingMachine\GraphQLite\Annotations\Logged;
1414
use TheCodingMachine\GraphQLite\Annotations\Right;
15-
use TheCodingMachine\GraphQLite\QueryField;
1615
use TheCodingMachine\GraphQLite\QueryFieldDescriptor;
1716
use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
1817
use TheCodingMachine\GraphQLite\Security\AuthorizationServiceInterface;
@@ -39,8 +38,19 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler
3938
$rightAnnotation = $annotations->getAnnotationByType(Right::class);
4039
assert($rightAnnotation === null || $rightAnnotation instanceof Right);
4140

41+
// Avoid wrapping resolver callback when no annotations are specified.
42+
if (! $loggedAnnotation && ! $rightAnnotation) {
43+
return $fieldHandler->handle($queryFieldDescriptor);
44+
}
45+
4246
$failWith = $annotations->getAnnotationByType(FailWith::class);
4347
assert($failWith === null || $failWith instanceof FailWith);
48+
$hideIfUnauthorized = $annotations->getAnnotationByType(HideIfUnauthorized::class);
49+
assert($hideIfUnauthorized instanceof HideIfUnauthorized || $hideIfUnauthorized === null);
50+
51+
if ($failWith !== null && $hideIfUnauthorized !== null) {
52+
throw IncompatibleAnnotationsException::cannotUseFailWithAndHide();
53+
}
4454

4555
// If the failWith value is null and the return type is non-nullable, we must set it to nullable.
4656
$type = $queryFieldDescriptor->getType();
@@ -50,28 +60,32 @@ public function process(QueryFieldDescriptor $queryFieldDescriptor, FieldHandler
5060
$queryFieldDescriptor->setType($type);
5161
}
5262

53-
if ($this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
54-
return $fieldHandler->handle($queryFieldDescriptor);
63+
// When using the same Schema instance for multiple subsequent requests, this middleware will only
64+
// get called once, meaning #[HideIfUnauthorized] only works when Schema is used for a single request
65+
// and then discarded. This check is to keep the latter case working.
66+
if ($hideIfUnauthorized !== null && ! $this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
67+
return null;
5568
}
5669

57-
$hideIfUnauthorized = $annotations->getAnnotationByType(HideIfUnauthorized::class);
58-
assert($hideIfUnauthorized instanceof HideIfUnauthorized || $hideIfUnauthorized === null);
70+
$resolver = $queryFieldDescriptor->getResolver();
5971

60-
if ($failWith !== null && $hideIfUnauthorized !== null) {
61-
throw IncompatibleAnnotationsException::cannotUseFailWithAndHide();
62-
}
72+
$queryFieldDescriptor->setResolver(function (...$args) use ($rightAnnotation, $loggedAnnotation, $failWith, $resolver) {
73+
if ($this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
74+
return $resolver(...$args);
75+
}
6376

64-
if ($failWith !== null) {
65-
$failWithValue = $failWith->getValue();
77+
if ($failWith !== null) {
78+
return $failWith->getValue();
79+
}
6680

67-
return QueryField::alwaysReturn($queryFieldDescriptor, $failWithValue);
68-
}
81+
if ($loggedAnnotation !== null && ! $this->authenticationService->isLogged()) {
82+
throw MissingAuthorizationException::unauthorized();
83+
}
6984

70-
if ($hideIfUnauthorized !== null) {
71-
return null;
72-
}
85+
throw MissingAuthorizationException::forbidden();
86+
});
7387

74-
return QueryField::unauthorizedError($queryFieldDescriptor, $loggedAnnotation !== null && ! $this->authenticationService->isLogged());
88+
return $fieldHandler->handle($queryFieldDescriptor);
7589
}
7690

7791
/**

‎src/Middlewares/AuthorizationInputFieldMiddleware.php

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,33 @@ public function process(InputFieldDescriptor $inputFieldDescriptor, InputFieldHa
3636
$rightAnnotation = $annotations->getAnnotationByType(Right::class);
3737
assert($rightAnnotation === null || $rightAnnotation instanceof Right);
3838

39-
if ($this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
39+
// Avoid wrapping resolver callback when no annotations are specified.
40+
if (! $loggedAnnotation && ! $rightAnnotation) {
4041
return $inputFieldHandler->handle($inputFieldDescriptor);
4142
}
4243

4344
$hideIfUnauthorized = $annotations->getAnnotationByType(HideIfUnauthorized::class);
4445
assert($hideIfUnauthorized instanceof HideIfUnauthorized || $hideIfUnauthorized === null);
4546

46-
if ($hideIfUnauthorized !== null) {
47+
if ($hideIfUnauthorized !== null && ! $this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
4748
return null;
4849
}
49-
return InputField::unauthorizedError($inputFieldDescriptor, $loggedAnnotation !== null && ! $this->authenticationService->isLogged());
50+
51+
$resolver = $inputFieldDescriptor->getResolver();
52+
53+
$inputFieldDescriptor->setResolver(function (...$args) use ($rightAnnotation, $loggedAnnotation, $resolver) {
54+
if ($this->isAuthorized($loggedAnnotation, $rightAnnotation)) {
55+
return $resolver(...$args);
56+
}
57+
58+
if ($loggedAnnotation !== null && ! $this->authenticationService->isLogged()) {
59+
throw MissingAuthorizationException::unauthorized();
60+
}
61+
62+
throw MissingAuthorizationException::forbidden();
63+
});
64+
65+
return $inputFieldHandler->handle($inputFieldDescriptor);
5066
}
5167

5268
/**

‎src/QueryField.php

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use GraphQL\Type\Definition\Type;
1616
use TheCodingMachine\GraphQLite\Context\ContextInterface;
1717
use TheCodingMachine\GraphQLite\Exceptions\GraphQLAggregateException;
18-
use TheCodingMachine\GraphQLite\Middlewares\MissingAuthorizationException;
1918
use TheCodingMachine\GraphQLite\Middlewares\ResolverInterface;
2019
use TheCodingMachine\GraphQLite\Middlewares\SourceResolverInterface;
2120
use TheCodingMachine\GraphQLite\Parameters\MissingArgumentException;
@@ -143,6 +142,7 @@ public function __construct(string $name, OutputType $type, array $arguments, Re
143142
}
144143

145144
$config += $additionalConfig;
145+
146146
parent::__construct($config);
147147
}
148148

@@ -186,25 +186,6 @@ public static function alwaysReturn(QueryFieldDescriptor $fieldDescriptor, mixed
186186
return self::fromDescriptor($fieldDescriptor);
187187
}
188188

189-
/**
190-
* @param bool $isNotLogged False if the user is logged (and the error is a 403), true if the error is unlogged (the error is a 401)
191-
*
192-
* @return QueryField
193-
*/
194-
public static function unauthorizedError(QueryFieldDescriptor $fieldDescriptor, bool $isNotLogged): self
195-
{
196-
$callable = static function () use ($isNotLogged): void {
197-
if ($isNotLogged) {
198-
throw MissingAuthorizationException::unauthorized();
199-
}
200-
throw MissingAuthorizationException::forbidden();
201-
};
202-
203-
$fieldDescriptor->setResolver($callable);
204-
205-
return self::fromDescriptor($fieldDescriptor);
206-
}
207-
208189
private static function fromDescriptor(QueryFieldDescriptor $fieldDescriptor): self
209190
{
210191
$type = $fieldDescriptor->getType();

‎tests/Middlewares/AuthorizationFieldMiddlewareTest.php

Lines changed: 93 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,120 @@
33
namespace TheCodingMachine\GraphQLite\Middlewares;
44

55
use GraphQL\Type\Definition\FieldDefinition;
6-
use PHPUnit\Framework\TestCase;
7-
use ReflectionMethod;
86
use TheCodingMachine\GraphQLite\AbstractQueryProviderTest;
97
use TheCodingMachine\GraphQLite\Annotations\Exceptions\IncompatibleAnnotationsException;
108
use TheCodingMachine\GraphQLite\Annotations\FailWith;
119
use TheCodingMachine\GraphQLite\Annotations\HideIfUnauthorized;
1210
use TheCodingMachine\GraphQLite\Annotations\Logged;
11+
use TheCodingMachine\GraphQLite\Annotations\MiddlewareAnnotationInterface;
12+
use TheCodingMachine\GraphQLite\Annotations\MiddlewareAnnotations;
13+
use TheCodingMachine\GraphQLite\Annotations\Right;
1314
use TheCodingMachine\GraphQLite\QueryFieldDescriptor;
15+
use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
16+
use TheCodingMachine\GraphQLite\Security\AuthorizationServiceInterface;
1417
use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
1518
use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService;
1619

1720
class AuthorizationFieldMiddlewareTest extends AbstractQueryProviderTest
1821
{
22+
public function testReturnsResolversValueWhenAuthorized(): void
23+
{
24+
$authenticationService = $this->createMock(AuthenticationServiceInterface::class);
25+
$authenticationService->method('isLogged')
26+
->willReturn(true);
27+
$authorizationService = $this->createMock(AuthorizationServiceInterface::class);
28+
$authorizationService->method('isAllowed')
29+
->willReturn(true);
30+
$middleware = new AuthorizationFieldMiddleware($authenticationService, $authorizationService);
31+
32+
$descriptor = $this->stubDescriptor([new Logged(), new Right('test')]);
33+
$descriptor->setResolver(fn () => 123);
34+
35+
$field = $middleware->process($descriptor, $this->stubFieldHandler());
36+
37+
self::assertNotNull($field);
38+
self::assertSame(123, ($field->resolveFn)());
39+
}
40+
1941

20-
public function testException(): void
42+
public function testFailsForHideIfUnauthorizedAndFailWith(): void
2143
{
2244
$middleware = new AuthorizationFieldMiddleware(new VoidAuthenticationService(), new VoidAuthorizationService());
2345

24-
$descriptor = new QueryFieldDescriptor();
25-
$descriptor->setMiddlewareAnnotations($this->getAnnotationReader()->getMiddlewareAnnotations(new ReflectionMethod(__CLASS__, 'stub')));
26-
2746
$this->expectException(IncompatibleAnnotationsException::class);
28-
$middleware->process($descriptor, new class implements FieldHandlerInterface {
29-
public function handle(QueryFieldDescriptor $fieldDescriptor): ?FieldDefinition
30-
{
31-
return FieldDefinition::create(['name'=>'foo']);
32-
}
33-
});
47+
$middleware->process($this->stubDescriptor([new Logged(), new HideIfUnauthorized(), new FailWith(value: 123)]), $this->stubFieldHandler());
48+
}
49+
50+
public function testHidesFieldForHideIfUnauthorized(): void
51+
{
52+
$middleware = new AuthorizationFieldMiddleware(new VoidAuthenticationService(), new VoidAuthorizationService());
53+
54+
$field = $middleware->process($this->stubDescriptor([new Logged(), new HideIfUnauthorized()]), $this->stubFieldHandler());
55+
56+
self::assertNull($field);
57+
}
58+
59+
public function testReturnsFailsWithValueWhenNotAuthorized(): void
60+
{
61+
$middleware = new AuthorizationFieldMiddleware(new VoidAuthenticationService(), new VoidAuthorizationService());
62+
63+
$field = $middleware->process($this->stubDescriptor([new Logged(), new FailWith(value: 123)]), $this->stubFieldHandler());
64+
65+
self::assertNotNull($field);
66+
self::assertSame(123, ($field->resolveFn)());
67+
}
68+
69+
public function testThrowsUnauthorizedExceptionWhenNotAuthorized(): void
70+
{
71+
$middleware = new AuthorizationFieldMiddleware(new VoidAuthenticationService(), new VoidAuthorizationService());
72+
73+
$field = $middleware->process($this->stubDescriptor([new Logged()]), $this->stubFieldHandler());
74+
75+
self::assertNotNull($field);
76+
77+
$this->expectExceptionObject(MissingAuthorizationException::unauthorized());
78+
79+
($field->resolveFn)();
80+
}
81+
82+
public function testThrowsForbiddenExceptionWhenNotAuthorized(): void
83+
{
84+
$authenticationService = $this->createMock(AuthenticationServiceInterface::class);
85+
$authenticationService->method('isLogged')
86+
->willReturn(true);
87+
$middleware = new AuthorizationFieldMiddleware($authenticationService, new VoidAuthorizationService());
88+
89+
$field = $middleware->process($this->stubDescriptor([new Logged(), new Right('test')]), $this->stubFieldHandler());
90+
91+
self::assertNotNull($field);
92+
93+
$this->expectExceptionObject(MissingAuthorizationException::forbidden());
94+
95+
($field->resolveFn)();
3496
}
3597

3698
/**
37-
* @Logged()
38-
* @HideIfUnauthorized()
39-
* @FailWith(null)
99+
* @param MiddlewareAnnotationInterface[] $annotations
40100
*/
41-
public function stub()
101+
private function stubDescriptor(array $annotations): QueryFieldDescriptor
42102
{
103+
$descriptor = new QueryFieldDescriptor();
104+
$descriptor->setMiddlewareAnnotations(new MiddlewareAnnotations($annotations));
105+
$descriptor->setResolver(fn () => self::fail('Should not be called.'));
43106

107+
return $descriptor;
108+
}
109+
110+
private function stubFieldHandler(): FieldHandlerInterface
111+
{
112+
return new class implements FieldHandlerInterface {
113+
public function handle(QueryFieldDescriptor $fieldDescriptor): FieldDefinition|null
114+
{
115+
return new FieldDefinition([
116+
'name' => 'foo',
117+
'resolve' => $fieldDescriptor->getResolver(),
118+
]);
119+
}
120+
};
44121
}
45122
}

0 commit comments

Comments
 (0)