Skip to content

feat: accept PHP callables as #[Security] rules - #822

Open
oojacoboo wants to merge 4 commits into
thecodingmachine:masterfrom
oojacoboo:design/callable-attributes
Open

feat: accept PHP callables as #[Security] rules#822
oojacoboo wants to merge 4 commits into
thecodingmachine:masterfrom
oojacoboo:design/callable-attributes

Conversation

@oojacoboo

@oojacoboo oojacoboo commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Adds a rule: parameter to #[Security], so a gate can be stated as a typed PHP object instead of
an ExpressionLanguage string. The expression form is untouched and fully supported.

What it adds

// The recommended form. An invokable object carries its own typed constructor arguments,
// which is why there is no separate args: parameter to go wrong.
#[Security(rule: new PageSizeWithin(100))]

// Array callable. The only form that can name a container-resolved method.
#[Security(rule: [MyRules::class, 'canEdit'])]

A rule receives a SecurityRuleContext (or the SecurityRuleContextInterface it implements) and
returns bool. Callables are normalized to closures once at schema build, so an unresolvable
callable fails at startup with a class and method name in the message rather than on a request.

rule: must be passed by name. The first positional argument is the expression, so a bare string
stays unambiguous; $rule is typed array|object|null and does not accept one.

PHP versions

Everything above works on 8.2 through 8.5. First-class callable syntax
(MyRules::canEdit(...)) and inline static closures are additionally accepted, but only compile on
8.5, and neither is serializable. They are documented as a convenience, not as the point of the
feature.

To be explicit, because the framing invites it: this is not "closures in attributes". That is
impossible below 8.5 and would be the wrong thing to ask for.

Backward compatibility

getExpression() deliberately keeps its string return type and throws when the annotation carries
a rule; hasExpression() is the guard. Widening it to string|null would have broken any subclass
overriding it with the narrower type, so the throwing variant was chosen instead. Every existing
#[Security("...")], positional or named, including $data array construction, behaves identically.

failWith, message and statusCode work the same for rule-backed attributes and remain per
attribute. Stacked attributes keep AND semantics, and a rule attribute may be stacked with an
expression one.

SchemaFactory is in the diff, on purpose

SchemaFactory builds one CallableResolver and injects it into both security middlewares and into
ParameterizedCallableResolver, which previously resolved callables itself. This removes a
second callable-resolution implementation rather than adding one, and it is why #[Prefetch] and
#[Security] now accept the same shapes. AnnotationReader, FieldsBuilder and
QueryFieldDescriptor are untouched.

One independent fix riding along

e65afa0 validates #[Security] expressions with parse() rather than lint(). It is unrelated to
rules and fixes the expression path on its own. Happy to split it into its own PR if you would
rather review it separately.

New public API

Three types are new and would be permanent commitments, so they are worth arguing rather than
skimming:

  • SecurityRuleContextInterface is what consumers type-hint. A rule depending on the interface
    rather than the concrete class stays unit testable against a hand-written double, with no schema to
    build. It exposes methods rather than public properties because an interface cannot declare
    properties before PHP 8.4 and this package supports 8.2.
  • SecurityRuleContext is the concrete context handed to rules. Its arguments are keyed by
    parameter name, built once, rather than inheriting the positional array_combine() zip and its
    compensating array_slice() for injectSource.
  • SecurityRuleMessageInterface lets a rule state its own refusal message. This is opt in; a rule
    that does not implement it keeps Access denied., and an explicit message: always wins.

Tests

tests/ gains 1,659 lines: the annotation in every construction shape, CallableResolver, both
security middlewares (SecurityFieldMiddleware had no test at all before this), the context
including injectSource argument keying, and an end-to-end pass through a real schema execution. CI
is green on 8.2, 8.3, 8.4 and 8.5, including the --prefer-lowest legs, plus Psalm and php-security.

Not claimed

No performance claim. Nothing was benchmarked, so nothing is asserted about rules being faster than a
cached ParsedExpression walk.

Downstream integrations were not audited. graphqlite-bundle and the Laravel package may compile
annotations into a container dump, which would matter for a Closure-valued rule specifically. The
serializable shapes (invokable objects, array callables) are unaffected either way.

#[Security] gains a `rule` parameter taking a callable that receives a
SecurityRuleContext and returns bool. Accepted forms: array callable (a
non-static method resolves through the container), invokable object for
parameterised rules, and on PHP 8.5 first-class callables and static
closures. Unlike an expression, a rule is refactor-safe, debuggable and
unit testable on its own.

Expressions remain fully supported, and are hardened alongside: they are
now parsed at schema build rather than failing on first resolution, and
must evaluate to bool like rules do.

Adds CallableResolver, which normalises every form to a Closure once at
schema build; #[Prefetch] now shares it and accepts invokable objects and
Closures too.

Claude-Session: https://claude.ai/code/session_0156c2BrkiiJtVZkF5dkPgC6
ExpressionLanguage::lint() only exists from Symfony 6.1, so the
--prefer-lowest CI cells — which resolve symfony/expression-language down
to 4.4 — died with "Call to undefined method". parse() has been available
since 2.4 and rejects the same three things: bad syntax, unknown
variables, unknown functions. Verified identical messages on 4.4.9 and
8.0.8, so the exception text is unchanged.

Claude-Session: https://claude.ai/code/session_0156c2BrkiiJtVZkF5dkPgC6
Rules could only be written against the final SecurityRuleContext, so a
rule was usable inside a GraphQLite field resolution and nowhere else.

The interface declares the context contract, letting a rule be unit
tested against a fake and reused from an HTTP middleware, a console
command or a message handler. The readonly properties stay on the
concrete class and gain getUser/getSource/getArguments accessors on the
contract, since an interface cannot declare properties before the
8.4 property hooks this library predates.

No middleware change is needed: rules are invoked without a declared
parameter type, so an interface-typed rule accepts the concrete
instance by subtyping. A middleware test pins that.
A rule is reusable, but its refusal message was not: every call site had
to repeat the same sentence, and a rule could not explain why it refused.

SecurityRuleMessageInterface lets a rule supply that message. Precedence
is explicit message, then the rule's, then "Access denied.", so the
feature is opt in and no existing attribute changes behavior. Security
now stores the message as given and resolves the default on read, which
is what lets the middleware tell an absent message from an explicit one.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.17544% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.76%. Comparing base (53f9d49) to head (4b48628).
⚠️ Report is 165 commits behind head on master.

Files with missing lines Patch % Lines
src/Middlewares/SecurityInputFieldMiddleware.php 87.27% 7 Missing ⚠️
src/Middlewares/SecurityFieldMiddleware.php 96.55% 2 Missing ⚠️
src/CallableResolver.php 97.05% 1 Missing ⚠️
.../Middlewares/NonBooleanSecurityResultException.php 93.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #822      +/-   ##
============================================
- Coverage     95.72%   91.76%   -3.97%     
- Complexity     1773     2093     +320     
============================================
  Files           154      201      +47     
  Lines          4586     5610    +1024     
============================================
+ Hits           4390     5148     +758     
- Misses          196      462     +266     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants