Simple permissions management using roles #218
Conversation
| @@ -46,7 +46,7 @@ | |||
| {% block navigation %} | |||
| <ul id="header-menu"> | |||
| {% block navigation_items %} | |||
| {% for item in easyadmin_config('entities') %} | |||
| {% for item in easyadmin_config('entities') if is_granted(item.role|default('IS_AUTHENTICATED_ANONYMOUSLY')) %} | |||
There was a problem hiding this comment.
I think that relying on IS_AUTHENTICATED_ANONYMOUSLY is not a good practice, because you can still have a security config allowing you to be unauthenticated, even before being authenticated with an anonymous token.
There was a problem hiding this comment.
Then how would you do it without using is_granted()? With an expression?
There was a problem hiding this comment.
With this:
item.role and is_granted(item.role)Because is_granted will not be triggered if there is nothing in item_role
There was a problem hiding this comment.
Shouldn't it be if item.role is defined or is_granted(item.role)?
The way you say it still crash when the entity doesn't have the role defined.
I figured out the right way to apply it to this foreach filter:
if item.role is not defined or is_granted(item.role)
That way it wouldn't crash it the role is not defined and it will still work when it is.
Thanks!
There was a problem hiding this comment.
And what about if you set an empty role ? (still a bug, but anyway it might be an issue)
There was a problem hiding this comment.
We can add the correspondent check for an empty string, but I think it's not necessary because the developer will see the 403 and then go check the config.yml file.
|
I think that @javiereguiluz will not merge this PR but anyway it's a good use if the security component to manage roles in the back-end. If I might introduce some "cons", I think that directly relying on the component is not "standalone enough", but anyway, who's using Symfony without this component ? ^^ |
|
As @Pierstoval cleverly guessed, I'm afraid that I cannot merge this PR and I have to close it. I know that this can be considered rude, so please let me explain myself. This bundle defines a roadmap which is followed with discipline. This laser sharp focus is what could make us succeed one day. Trust me that I'd like to have something like @aruku's proposal, but I know that first we have to implement other basic features. We cannot add great features to something which is not great. Right now this bundle is barely "good enough" to create basic backends. And that's why there is a long road ahead before considering adding security-related features. Thanks for understanding it. |
|
Of course @javiereguiluz, I completely understand. Thanks to you for considering it. |
Obviously, but it's adding great features that makes something great 😉 |
With this tiny change we can control whether the user needs a specific role granted to access an entity management screen. Related with #216.
EDIT: I just discovered that GitHub doesn't match the user I made the commits with with the one I use here; I guess that's because I have two different ssh keys in my system, one with my company's email address and another with a personal email. If this a problem, I'll redo the commits with the right user or try to get GitHub to know that user is me.