Skip to content

Commit 469ac79

Browse files
committed
refactor: make PolicyFor's resource optional and rename to Policy
1 parent 93fb705 commit 469ac79

File tree

9 files changed

+98
-42
lines changed

9 files changed

+98
-42
lines changed

docs/2-features/04-authentication.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,17 +236,23 @@ This paradigm is known as [policy-based access control](https://en.wikipedia.org
236236

237237
### Defining policies
238238

239-
To create a policy, you may define a method in any class and annotate it with the {b`#[Tempest\Auth\AccessControl\PolicyFor]`} attribute. Typically, this is done in a dedicated policy class.
239+
To create a policy, you may define a method in any class and annotate it with the {b`#[Tempest\Auth\AccessControl\Policy]`} attribute. Typically, this is done in a dedicated policy class.
240240

241-
The attribute expects the class name of the resource as its first parameter, and an optional action name as the second parameter. If the action name is not provided, the kebab-cased method name is used instead.
241+
The attribute expects the class name of the resource as its first parameter, and the action name as the second parameter. If the resource is not specified, it will be inferred by the method's first parameter. Similarly, if the action name is not provided, the kebab-cased method name is used instead.
242242

243243
```php app/PostPolicy.php
244-
use Tempest\Auth\AccessControl\PolicyFor;
244+
use Tempest\Auth\AccessControl\Policy;
245245
use Tempest\Auth\AccessControl\AccessDecision;
246246

247247
final class PostPolicy
248248
{
249-
#[PolicyFor(Post::class)]
249+
#[Policy(Post::class)]
250+
public function create(): bool
251+
{
252+
return true;
253+
}
254+
255+
#[Policy]
250256
public function view(Post $post): bool
251257
{
252258
if (! $post->published) {
@@ -256,7 +262,7 @@ final class PostPolicy
256262
return true;
257263
}
258264

259-
#[PolicyFor(Post::class, action: ['edit', 'update'])]
265+
#[Policy(action: ['edit', 'update'])]
260266
public function edit(Post $post, ?User $user): bool
261267
{
262268
if ($user === null) {

packages/auth/src/AccessControl/PolicyFor.php renamed to packages/auth/src/AccessControl/Policy.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,17 @@
77

88
/**
99
* When applied on a method, this attribute indicates that the method is a policy method for the specified resource.
10-
* The method must accept an action as its first parameter, the resource instance as its second, and the subject as its last.
10+
* The method must accept the resource instance as its first parameter and the subject as its second one.
1111
*/
1212
#[Attribute(Attribute::TARGET_METHOD)]
13-
final class PolicyFor
13+
final class Policy
1414
{
1515
/**
16-
* @param class-string $resource A resource class that this policy applies to.
16+
* @param class-string $resource A resource class that this policy applies to. If not specified, it will be inferred by the method's first argument.
1717
* @param null|UnitEnum|string|iterable<string|UnitEnum|null> $action An optional action that this policy applies to. If null, the policy applies to all actions for the resource.
1818
*/
1919
public function __construct(
20-
public string $resource,
20+
public ?string $resource = null,
2121
public null|UnitEnum|string|iterable $action = null,
2222
) {}
2323
}

packages/auth/src/AccessControl/PolicyDiscovery.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function __construct(
2020
public function discover(DiscoveryLocation $location, ClassReflector $class): void
2121
{
2222
foreach ($class->getPublicMethods() as $method) {
23-
$policy = $method->getAttribute(PolicyFor::class);
23+
$policy = $method->getAttribute(Policy::class);
2424

2525
if (! $policy) {
2626
continue;

packages/auth/src/AuthConfig.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
namespace Tempest\Auth;
66

77
use BackedEnum;
8-
use Tempest\Auth\AccessControl\PolicyFor;
8+
use Tempest\Auth\AccessControl\Policy;
99
use Tempest\Auth\Authentication\CanAuthenticate;
10+
use Tempest\Auth\Exceptions\PolicyIsInvalid;
11+
use Tempest\Auth\Exceptions\PolicyMethodIsInvalid;
1012
use Tempest\Reflection\MethodReflector;
1113
use Tempest\Support\Arr;
1214
use Tempest\Support\Str;
@@ -23,8 +25,18 @@ public function __construct(
2325
public array $policies = [],
2426
) {}
2527

26-
public function registerPolicy(MethodReflector $handler, PolicyFor $policy): self
28+
public function registerPolicy(MethodReflector $handler, Policy $policy): self
2729
{
30+
if (! $policy->resource) {
31+
$policy->resource = $handler->getParameter(key: 0)?->getType()?->getName();
32+
}
33+
34+
if (! $policy->resource) {
35+
throw PolicyIsInvalid::resourceCouldNotBeInferred(
36+
policyName: sprintf('%s::%s', $handler->getDeclaringClass()->getName(), $handler->getName()),
37+
);
38+
}
39+
2840
$this->policies[$policy->resource] ??= [];
2941

3042
if ($policy->action === null) {
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Tempest\Auth\Exceptions;
4+
5+
use Exception;
6+
7+
final class PolicyIsInvalid extends Exception implements AuthenticationException
8+
{
9+
public static function resourceCouldNotBeInferred(string $policyName): self
10+
{
11+
return new self(sprintf(
12+
"The resource for policy `%s` could not be inferred because it is missing from the method's parameters. You must specify it in the attribute instead.",
13+
$policyName,
14+
));
15+
}
16+
}

tests/Integration/Auth/AccessControl/AccessControlTest.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
use PHPUnit\Framework\Attributes\Test;
88
use Tempest\Auth\AccessControl\AccessControl;
99
use Tempest\Auth\AccessControl\AccessDecision;
10-
use Tempest\Auth\AccessControl\PolicyFor;
10+
use Tempest\Auth\AccessControl\Policy;
1111
use Tempest\Auth\Authentication\Authenticator;
1212
use Tempest\Auth\Authentication\AuthenticatorInitializer;
1313
use Tempest\Auth\Authentication\CanAuthenticate;
@@ -125,13 +125,13 @@ public function __construct(
125125

126126
final class ArticlePolicy
127127
{
128-
#[PolicyFor(Article::class, action: 'view')]
128+
#[Policy(Article::class, action: 'view')]
129129
public function view(): bool
130130
{
131131
return true;
132132
}
133133

134-
#[PolicyFor(Article::class, action: 'edit')]
134+
#[Policy(Article::class, action: 'edit')]
135135
public function edit(Article $article, ?TestUser $subject): bool
136136
{
137137
if ($subject === null) {
@@ -141,7 +141,7 @@ public function edit(Article $article, ?TestUser $subject): bool
141141
return $subject->role === 'admin' || $subject->role === 'author' && $article->authorId === $subject->id->value;
142142
}
143143

144-
#[PolicyFor(Article::class, action: 'delete')]
144+
#[Policy(Article::class, action: 'delete')]
145145
public function delete(Article $article, ?TestUser $subject): bool|AccessDecision
146146
{
147147
if ($subject === null) {
@@ -159,7 +159,7 @@ public function delete(Article $article, ?TestUser $subject): bool|AccessDecisio
159159
return AccessDecision::denied('Insufficient permissions for this action');
160160
}
161161

162-
#[PolicyFor(Article::class, action: 'manage')]
162+
#[Policy(Article::class, action: 'manage')]
163163
public function manage(?Article $_, ?TestUser $subject): bool
164164
{
165165
return $subject?->role === 'admin';
@@ -168,13 +168,13 @@ public function manage(?Article $_, ?TestUser $subject): bool
168168

169169
final class CommentPolicy
170170
{
171-
#[PolicyFor(ArticleComment::class, action: 'view')]
171+
#[Policy(ArticleComment::class, action: 'view')]
172172
public function view(): bool
173173
{
174174
return true;
175175
}
176176

177-
#[PolicyFor(ArticleComment::class, action: 'edit')]
177+
#[Policy(ArticleComment::class, action: 'edit')]
178178
public function edit(ArticleComment $comment, ?TestUser $subject): bool
179179
{
180180
if ($subject === null) {
@@ -184,7 +184,7 @@ public function edit(ArticleComment $comment, ?TestUser $subject): bool
184184
return $comment->authorId === $subject->id->value || $subject->role === 'admin';
185185
}
186186

187-
#[PolicyFor(ArticleComment::class, action: 'delete')]
187+
#[Policy(ArticleComment::class, action: 'delete')]
188188
public function delete(ArticleComment $_, ?TestUser $subject): bool
189189
{
190190
return $subject?->role === 'admin';

tests/Integration/Auth/AccessControl/HasPolicyTests.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace Tests\Tempest\Integration\Auth\AccessControl;
44

5-
use Tempest\Auth\AccessControl\PolicyFor;
5+
use Tempest\Auth\AccessControl\Policy;
66
use Tempest\Auth\AuthConfig;
77
use Tempest\Reflection\ClassReflector;
88

@@ -16,7 +16,7 @@ public function registerPoliciesFrom(string|object $class): self
1616
$config = $this->container->get(AuthConfig::class);
1717

1818
foreach (new ClassReflector($class)->getPublicMethods() as $method) {
19-
if ($policy = $method->getAttribute(PolicyFor::class)) {
19+
if ($policy = $method->getAttribute(Policy::class)) {
2020
$config->registerPolicy($method, $policy);
2121
}
2222
}

tests/Integration/Auth/AccessControl/PolicyBasedAccessControlTest.php

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@
77
use PHPUnit\Framework\Attributes\Test;
88
use Tempest\Auth\AccessControl\AccessControl;
99
use Tempest\Auth\AccessControl\AccessDecision;
10+
use Tempest\Auth\AccessControl\Policy;
1011
use Tempest\Auth\AccessControl\PolicyBasedAccessControl;
11-
use Tempest\Auth\AccessControl\PolicyFor;
1212
use Tempest\Auth\AuthConfig;
1313
use Tempest\Auth\Authentication\Authenticator;
1414
use Tempest\Auth\Authentication\AuthenticatorInitializer;
1515
use Tempest\Auth\Authentication\CanAuthenticate;
1616
use Tempest\Auth\Exceptions\AccessWasDenied;
1717
use Tempest\Auth\Exceptions\NoPolicyWereFoundForResource;
18+
use Tempest\Auth\Exceptions\PolicyIsInvalid;
1819
use Tempest\Auth\Exceptions\PolicyMethodIsInvalid;
1920
use Tempest\Database\PrimaryKey;
2021
use Tests\Tempest\Integration\Auth\Fixtures\InMemoryAuthenticatorInitializer;
@@ -271,6 +272,17 @@ public function throws_exception_when_policy_resource_parameter_type_is_invalid(
271272
$accessControl->isGranted('view', $post, $user);
272273
}
273274

275+
#[Test]
276+
public function throws_exception_when_policy_resource_is_not_specified_anywhere(): void
277+
{
278+
$this->expectException(PolicyIsInvalid::class);
279+
$this->expectExceptionMessageMatches(
280+
"/The resource for policy `.*::missingResourceType` could not be inferred because it is missing from the method's parameters\. You must specify it in the attribute instead\./",
281+
);
282+
283+
$this->registerPoliciesFrom(MissingResourceTypePolicy::class);
284+
}
285+
274286
#[Test]
275287
public function throws_exception_when_policy_subject_parameter_type_is_invalid(): void
276288
{
@@ -358,19 +370,19 @@ public function __construct(
358370

359371
final class PostPolicy
360372
{
361-
#[PolicyFor(Post::class, action: 'view')]
373+
#[Policy(Post::class, action: 'view')]
362374
public function view(): bool
363375
{
364376
return true;
365377
}
366378

367-
#[PolicyFor(Post::class, action: PostAction::VIEW)]
379+
#[Policy(Post::class, action: PostAction::VIEW)]
368380
public function viewEnum(): bool
369381
{
370382
return true;
371383
}
372384

373-
#[PolicyFor(Post::class, action: 'edit')]
385+
#[Policy(action: 'edit')]
374386
public function edit(?Post $resource, ?User $subject): bool
375387
{
376388
if (! ($subject instanceof User)) {
@@ -380,7 +392,7 @@ public function edit(?Post $resource, ?User $subject): bool
380392
return $resource?->authorId === $subject->id->value;
381393
}
382394

383-
#[PolicyFor(Post::class, action: PostAction::EDIT)]
395+
#[Policy(action: PostAction::EDIT)]
384396
public function editEnum(?Post $resource, ?User $subject): bool
385397
{
386398
if (! ($subject instanceof User)) {
@@ -390,7 +402,7 @@ public function editEnum(?Post $resource, ?User $subject): bool
390402
return $resource?->authorId === $subject->id->value;
391403
}
392404

393-
#[PolicyFor(Post::class, action: 'delete')]
405+
#[Policy(action: 'delete')]
394406
public function delete(?Post $resource, ?User $subject): bool|AccessDecision
395407
{
396408
if (! ($subject instanceof User)) {
@@ -402,7 +414,7 @@ public function delete(?Post $resource, ?User $subject): bool|AccessDecision
402414
: AccessDecision::denied('Only the author can delete their post');
403415
}
404416

405-
#[PolicyFor(Post::class, action: PostAction::DELETE)]
417+
#[Policy(action: PostAction::DELETE)]
406418
public function deleteEnum(?Post $resource, ?User $subject): bool|AccessDecision
407419
{
408420
if (! ($subject instanceof User)) {
@@ -414,7 +426,7 @@ public function deleteEnum(?Post $resource, ?User $subject): bool|AccessDecision
414426
: AccessDecision::denied('Only the author can delete their post');
415427
}
416428

417-
#[PolicyFor(Post::class, action: 'create')]
429+
#[Policy(Post::class, action: 'create')]
418430
public function create(): bool
419431
{
420432
return true;
@@ -423,7 +435,7 @@ public function create(): bool
423435

424436
final class UserPolicy
425437
{
426-
#[PolicyFor(User::class, action: 'manage')]
438+
#[Policy(action: 'manage')]
427439
public function manage(?User $resource, ?User $subject): bool
428440
{
429441
if (! ($subject instanceof User)) {
@@ -446,7 +458,7 @@ public function __construct(
446458

447459
final class MultiActionPolicy
448460
{
449-
#[PolicyFor(Document::class, action: ['read', 'download'])]
461+
#[Policy(action: ['read', 'download'])]
450462
public function readAndDownload(?Document $resource, ?User $subject): bool
451463
{
452464
if (! ($subject instanceof User)) {
@@ -459,7 +471,7 @@ public function readAndDownload(?Document $resource, ?User $subject): bool
459471

460472
final class MultiAuthenticatablePolicy
461473
{
462-
#[PolicyFor(Document::class)]
474+
#[Policy]
463475
public function view(?Document $_resource, null|User|ServiceAccount $subject): bool
464476
{
465477
if (! ($subject instanceof CanAuthenticate)) {
@@ -470,10 +482,20 @@ public function view(?Document $_resource, null|User|ServiceAccount $subject): b
470482
}
471483
}
472484

485+
final class MissingResourceTypePolicy
486+
{
487+
// no resource is specified
488+
#[Policy]
489+
public function missingResourceType(): bool
490+
{
491+
return true;
492+
}
493+
}
494+
473495
final class InvalidResourceTypePolicy
474496
{
475497
// expects a User as resource but will receive a Post
476-
#[PolicyFor(Post::class, action: 'view')]
498+
#[Policy(Post::class, action: 'view')]
477499
public function invalidResourceType(User $_resource, ?User $_subject): bool
478500
{
479501
return true;
@@ -483,7 +505,7 @@ public function invalidResourceType(User $_resource, ?User $_subject): bool
483505
final class InvalidSubjectTypePolicy
484506
{
485507
// expects a Document as subject but will receive a User
486-
#[PolicyFor(Post::class, action: 'edit')]
508+
#[Policy(Post::class, action: 'edit')]
487509
public function invalidSubjectType(?Post $_resource, Document $_subject): bool
488510
{
489511
return true;
@@ -492,7 +514,7 @@ public function invalidSubjectType(?Post $_resource, Document $_subject): bool
492514

493515
final class PolicyWithoutActionNames
494516
{
495-
#[PolicyFor(Post::class)]
517+
#[Policy(Post::class)]
496518
public function canMarkAsPublished(?Post $post, ?User $user): bool
497519
{
498520
if ($user === null) {
@@ -502,7 +524,7 @@ public function canMarkAsPublished(?Post $post, ?User $user): bool
502524
return $post?->authorId === $user->id->value;
503525
}
504526

505-
#[PolicyFor(Post::class)]
527+
#[Policy(Post::class)]
506528
public function approveForPublication(?Post $post, ?User $user): bool
507529
{
508530
if ($user === null) {

0 commit comments

Comments
 (0)