Allow class FQCN strings as authorization resources (draft, refs #135)#332
Draft
dereuromark wants to merge 2 commits into
Draft
Allow class FQCN strings as authorization resources (draft, refs #135)#332dereuromark wants to merge 2 commits into
dereuromark wants to merge 2 commits into
Conversation
…d MapResolver Enables the long-requested pattern from issue #135: $user->can('add', Article::class); which is the natural shape for menu/button visibility checks and any authorization gate that runs before an entity instance is on hand. OrmResolver: a string matching one of the standard entity/table namespace markers (\Model\Entity\ or \Model\Table\) is decomposed into namespace + name and routed through the existing findPolicy() conventions. MapResolver: a string that resolves to an existing class is treated as the map key. Non-class strings still raise InvalidArgumentException; valid class strings without a registered policy raise MissingPolicyException, in line with the object case.
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.
Draft — proposing an API shape, not asking for merge yet. Closes #135 if accepted.
Why
The recurring use case from #135 is:
i.e. checking permission to create a resource at a moment when no instance exists yet (menu rendering, button visibility, list-view "New" links, before
newEmptyEntity()).Today this requires creating a throwaway entity just to satisfy the resolver, or mapping
ServerRequest::classto aRequestPolicyand usingRequestAuthorizationMiddleware. Both feel like workarounds.The 2020 thread on #135 (markstory: "the only limitation … has been a lack of imagination and use cases"; LordSimal in 2023: "this will be possible in the next major version") concluded the feature is wanted but had no champion. Picking it back up.
What
OrmResolvergetPolicy()learns to accept a class FQCN string. If the string contains one of the conventional namespace markers (\Model\Entity\or\Model\Table\), the namespace and name segments are extracted and routed through the existingfindPolicy()lookup — so the resolution is identical to what would happen for an instance of that class:A string that is not a class (
class_exists() === false) is left to fall through to the existingMissingPolicyExceptionpath. A class that does not match either marker also throwsMissingPolicyException— same behavior as a non-resolvable instance.MapResolvergetPolicy()learns to accept a class FQCN string. If the string resolves to an existing class, it is used directly as the map key:A registered FQCN with no policy mapping raises
MissingPolicyException(parity with the object case). A non-class string still raisesInvalidArgumentException, just with an updated message that names the offending string instead of just its type.The pre-existing
testGetPolicyPrimitiveassertion message was updated accordingly — that is the only BC-visible change in the test suite.Discussion points for maintainers
OrmResolveracceptclass_exists($resource) === false? Current draft requires the class to exist. An alternative is to accept any string that contains the marker and letfindPolicy()either return a policy or throw. Stricter is friendlier IMO but I can flip it.MapResolver— should we widen the type hint ongetPolicy()? The interface signature ismixed, so technically nothing changes; the docblock and message strings now describe the broader contract.MapResolver::map()enforcesclass_exists($resourceClass)so this is moot, but if anyone uses it with classes only autoloaded under certain conditions it could be a footgun.ResolverCollectionalready chains resolvers, so a user with a partial map + ORM resolver gets the natural fallback for free. No changes there.Verification
composer test— 146 tests, 296 assertions (was 140 / 289). 6 new tests covering both resolvers, the missing-policy paths, and the unrelated-class case.composer stan— clean.composer cs-check— clean.