feat: accept PHP callables as #[Security] rules - #822
Open
oojacoboo wants to merge 4 commits into
Open
Conversation
#[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 Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
rule:parameter to#[Security], so a gate can be stated as a typed PHP object instead ofan ExpressionLanguage string. The expression form is untouched and fully supported.
What it adds
A rule receives a
SecurityRuleContext(or theSecurityRuleContextInterfaceit implements) andreturns
bool. Callables are normalized to closures once at schema build, so an unresolvablecallable 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 stringstays unambiguous;
$ruleis typedarray|object|nulland 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 on8.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 itsstringreturn type and throws when the annotation carriesa rule;
hasExpression()is the guard. Widening it tostring|nullwould have broken any subclassoverriding it with the narrower type, so the throwing variant was chosen instead. Every existing
#[Security("...")], positional or named, including$dataarray construction, behaves identically.failWith,messageandstatusCodework the same for rule-backed attributes and remain perattribute. Stacked attributes keep AND semantics, and a rule attribute may be stacked with an
expression one.
SchemaFactory is in the diff, on purpose
SchemaFactorybuilds oneCallableResolverand injects it into both security middlewares and intoParameterizedCallableResolver, which previously resolved callables itself. This removes asecond callable-resolution implementation rather than adding one, and it is why
#[Prefetch]and#[Security]now accept the same shapes.AnnotationReader,FieldsBuilderandQueryFieldDescriptorare untouched.One independent fix riding along
e65afa0 validates
#[Security]expressions withparse()rather thanlint(). It is unrelated torules 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:
SecurityRuleContextInterfaceis what consumers type-hint. A rule depending on the interfacerather 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.
SecurityRuleContextis the concrete context handed to rules. Itsargumentsare keyed byparameter name, built once, rather than inheriting the positional
array_combine()zip and itscompensating
array_slice()forinjectSource.SecurityRuleMessageInterfacelets a rule state its own refusal message. This is opt in; a rulethat does not implement it keeps
Access denied., and an explicitmessage:always wins.Tests
tests/gains 1,659 lines: the annotation in every construction shape,CallableResolver, bothsecurity middlewares (
SecurityFieldMiddlewarehad no test at all before this), the contextincluding
injectSourceargument keying, and an end-to-end pass through a real schema execution. CIis green on 8.2, 8.3, 8.4 and 8.5, including the
--prefer-lowestlegs, plus Psalm and php-security.Not claimed
No performance claim. Nothing was benchmarked, so nothing is asserted about rules being faster than a
cached
ParsedExpressionwalk.Downstream integrations were not audited.
graphqlite-bundleand the Laravel package may compileannotations into a container dump, which would matter for a
Closure-valued rule specifically. Theserializable shapes (invokable objects, array callables) are unaffected either way.