-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Credentials] credentials and groups #154
Comments
@pjam you need to utilize the groups functionality. /**
* Get groups.
*
* @param Object $Object
* @return array
*/
protected function getGroups(Object $Object)
{
return $this->get('security.authorization_checker')->isGranted("ROLE_SUPER_ADMIN") ? array(0 -> 'SUPER_ADMIN') : array();
} Then you need to attach the credentials to the field in the generator.yml like: fields:
fieldname:
groups: [SUPER_ADMIN] |
Thanks for the help @tobias-93 it is working now. 👍 But this is some kind of workaround right? It's supposed to work the way I tried, it is like that in the current docs. Or will this really be the way to go? |
I'm curious to the reasoning behind this? In 1.x, the authorization of a field was very straightforward with the 'credentials' option. Why would we force the usage of groups? In a normal security model, actions would be permission/specific-credential based, not group-based? This seems like a time-consuming process (not to mention unnecessary code) for permissions. I need to write a getGroups method for every single CRUD controller in my system? Additionally, what if one field in a form requires one permission, but another field requires a different permission? |
Hi, This is somethig we planned to improve using a centralized service to manage groups (take a look to #82 ). I understand your point "it was easier before", but it was also less flexible (once refactoring will be done). Any idea is welcome: how could we let the functionnality of groups available but also making it easier to use with simple ROLEs constraints? Maybe the group functionnality is not the right way to proceed? In my personnal projects I'm using Maybe we should let @loostro give us its feedbacks. @mikeyudin yours are of course welcome too... Without groups, how would we manage credentials based on something not limited to "ROLES"? (actually... Voters might make the trick?) |
If anything, I think the previous method was more flexible. I could specify different credentials for different fields. Now I am forced to say "All fields on this page are allowed if user has X group." Permissions are often more complex than that. If something more complex than I think advanced security is out of the scope of this bundle. We should only have to worry about passing a value to How does groups solve something like this: "User can see field if they have A and B, but not C?". I can think of several ways of doing it with the credentials method. Let's not also forget the major BC break that this causes. I understand this is a major release, but even still, reducing the amount of BC breaks should always be at the forefront of everyone's mind. |
@mikeyudin keep in mind that the example I have was very basic and simple. You can define many groups in the getGroups method and use these for certain fields (you can still keep specifying credentials for different fields). All you say is already possible. |
I guess I just fail to see the point of this. The entire principle behind the yaml-based admin generator is to reduce code and keep as much as possible in a single configuration file. Now we are pulling a major piece of that (security) out of the config and into code, increasing the complexity ten-fold on large applications. A precedent is being set for this to happen in other areas as well. If I wanted to code out the specs of all of my admin pages, I would've gone with SonataAdmin. The answer still has not been stated: What does this new method offer that the previous method did not? Perhaps this is mentioned in another issue/PR? It should be a good reason to cause such a major BC break. I am curious to know what @loostro has to say about this. |
@mikeyudin from what I understand from the example given by @tobias-93 you can achieve those complex permissions the same way. Just define the returned array as you want. I guess the size of that array doesn't need to be equal to the number of different roles you have in your app. Either way, using the groups way or the symfony1 way, I think we can take advantage of the Role Hierarchy system to build complex permissions and also to "simulate" groups, or not? And would it be hard to implement the "credentials" option in the generator like the old way? I would try but I just recently started to use sf2 instead of sf1 and I still lose myself in the code. lol |
Just add my 2¢... |
@mikeyudin I see your point... let's find a way to make all of this the most easy to use. As I said, I'm using a lot JMSecurityExtraBundle for these crendentials. And it do the trick... I didn't use yet the new version on a real project to let you know what are benefits from the Let's try to find use case where JMS is not suffisant enough. |
I guess it would also be acceptable to use both systems: I suggest that the old credential method is reinstated, but that the group configuration can also stay. The credentials/group check would then be depending on your own preference: The more flexible one, or the easier one. |
I guess the easiest implementation would be to force either However I think that groups functionality is more readable and flexible and it should stay. |
See #221 |
Hi.
Could you give some pointers on how to use credentials in generator.yml?
I want a list field visible only with some credentials and tried
without luck, error thrown "Could not set option "credentials" on "fieldname" column in "Entity" list builder.". Is it possible to use credentials for fields? How?
Regards
The text was updated successfully, but these errors were encountered: