Deprecated passing more than one attribute to @IsGranted #648
Conversation
wouterj
force-pushed
the
issue-645/is-granted-multiple-attributes
branch
from
bf946fe
to
05f8154
Compare
Co-Authored-By: Jérôme <jerome.vasseur@jhome.fr>
|
Thanks @jvasseur! I'm not using this bundle, so wasn't aware of the difference between those annotations. |
| --- | ||
|
|
||
| * Deprecated passing more than one attribute to `@IsGranted()`. Use an expression instead | ||
| (e.g. `@Security("has_role('ROLE_USER') or has_role('ROLE_ADMIN')")`). |
There was a problem hiding this comment.
| (e.g. `@Security("has_role('ROLE_USER') or has_role('ROLE_ADMIN')")`). | |
| (e.g. `@Security("is_granted('ROLE_USER') or is_granted('ROLE_ADMIN')")`). |
I think is_granted() is a more accurate replacement
| @@ -73,7 +73,7 @@ public function onKernelControllerArguments(KernelEvent $event) | |||
| } | |||
| } | |||
|
|
|||
| if (!$this->authChecker->isGranted($configuration->getAttributes(), $subject)) { | |||
| if (!$this->authChecker->isGranted($configuration->getAttribute(), $subject)) { | |||
There was a problem hiding this comment.
If you're passing an array of attributes and are using Symfony 5, you will see the deprecation from this bundle, but the feature won't work correctly because Symfony 5 doesn't support an array of attributes, right?
I think we need to either:
A) "Mimic" the "or" behavior here - i.e. if it's an array, we call isGranted() twice in an "or" kind of way
B) Throw an actual exception if you're using Symfony 5 with this bundle and are passing an array of attributes. That's a BC Break, but an obvious one (better than the silent one of it just not working) AND the user should have seen deprecation warnings on Symfony 4.4 anyways. I think this would prevent WTF's like this one: https://github.com/sensiolabs/SensioFrameworkExtraBundle/issues/645#issuecomment-569914831
…nted] (HypeMC) This PR was merged into the 6.2 branch. Discussion ---------- [Security] Remove using multiple attributes with #[IsGranted] | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46978 (comment) | License | MIT | Doc PR | - Passing multiple attributes to `isGranted()` has been removed in #33584, so the following doesn't work any more: ```php #[IsGranted(attributes: ['ROLE_ADMIN'])] public function index(Post $post) { } #[IsGranted(attributes: ['ROLE_USER', 'ROLE_ADMIN'])] public function index(Post $post) { } ``` As mentioned in sensiolabs/SensioFrameworkExtraBundle#648 , expressions should be used instead, see #46978 . This PR removes the possibility of using multiple attributes with `#[IsGranted]`. Also, it's currently possible to use `#[IsGranted()]` with no attributes (`null`). Since this doesn't seem to work either, nor can I find a reason why it even should, this PR removes that option as well. If I'm wrong about this one, please let me know. Commits ------- 663dc3e [Security] Remove using multiple attributes with #[IsGranted]
|
Closing as this repository is not maintained anymore. As of Symfony 6.2, all features have been moved to Symfony core now. |
Fixes #645
I'm not so sure about the version, but looking at the tags, I expected the next version will be 5.6 (as 5.5 is already released?).