Add AuthorizationComponent::skipAuthorizationActions() - #340
Draft
dereuromark wants to merge 2 commits into
Draft
Conversation
The `skipAuthorization` config key already marks controller actions as public, but unlike `authorizeModel()` and `mapAction()` it has no fluent setter, so it can only be set through `loadComponent()` options or `setConfig()`. Add a variadic setter that merges into the existing config, matching the `authorizeModel(string ...$actions)` signature. `authorizeAction()` runs on `Controller.startup`, which dispatches after `Controller.initialize`, so registering actions from `beforeFilter()` takes effect.
Listing an action in `skipAuthorization` only affected the automatic check in `authorizeAction()`. A manual `can()` or `authorize()` for the current action, for example a single gate in `beforeFilter()`, still ran the policy and failed on actions the application had already declared public. Treat the current action as authorized in `can()`, `canResult()` and `authorize()` when it is on the list. Only an implicit check is affected: an explicit action such as `can($article, 'delete')` always runs its policy. The raw action name is matched through the shared `isSkippedAction()` helper, the same key `authorizeAction()` uses, so both paths agree when `actionMap` is set. Document the three ways to skip authorization and how they differ.
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, mainly to have something concrete to discuss on #339.
The behavior asked for there mostly exists already: the
skipAuthorizationconfig key is handled inauthorizeAction()and is documented indocs/en/component.md. Two things were missing.A fluent setter, so a controller can declare its own public actions instead of centralizing them in
AppController:Signature and merge semantics mirror
authorizeModel(string ...$actions).Manual checks honoring the list. Previously only the automatic check in
authorizeAction()looked atskipAuthorization, so a single gate inbeforeFilter()still ran the policy for actions the application had already declared public:can(),canResult()andauthorize()now treat the current action as authorized when it is on the list.Two constraints on that, both covered by tests:
can($article, 'delete')is unchanged bydeletebeing on the list. Otherwise a list of public controller actions would silently turn into a list of always-allowed policy verbs, which is not what the config means and would be easy to trip over in a template.isSkippedAction()helper, soauthorizeAction()and the manual checks agree whenactionMapis set. Matching the mapped name in one path and the raw name in the other would makemapAction('edit', 'modify')skip in one place and not the other.canResult()returns aResult, nottrue, since it is typed: ResultInterfaceandauthorize()callsgetStatus()on what it returns.Docs now describe the three ways to skip and when each fits, per LordSimal's request on the issue.
Open question before this leaves draft: the name.
skipAuthorizationActions()matches the config key but is long. Alternatives raised on the issue areallowUnauthorized(),authorizeAction()andauthorizeController(). Worth noting #172 and #176 went the other way, collapsingauthorizeModel()andmapAction()into a singleactionconfig, so a third fluent setter may be the wrong shape regardless of the name.Refs #339