On úterý 17. ledna 2017 14:45:53 CET Ondrej Prazak wrote: > On Tue, Jan 17, 2017 at 12:08 PM, Tomas Strachota <[email protected]> > > wrote: > > On Mon, Jan 16, 2017 at 6:55 PM, oprazak <[email protected]> wrote: > > > Hi, > > > I recently started identifying problematic areas in Permissions and > > > > Roles, > > > > > especially with regard to plugins. Foreman provides 'Viewer' and > > > > 'Manager' > > > > > roles out of the box and users expect these roles to work for plugins as > > > well. But plugins generally do not add their permissions to core's > > > roles, > > > some of them just have their own plugin-specific roles, some not. So if > > > a > > > plugin is installed, user has to go and find what role/permission is > > > > missing > > > > > or ask someone who can grant permissions. > > > > > > After a brief discussion in team C, it was decided that plugins should > > > > add > > > > > their permissions to 'Manager' role and their 'view_' permissions to > > > 'Viewer' role. They should also keep their plugin-specific roles (or > > > > create > > > > > them) for higher granularity and backward compatibility. > > > I started initial work which should allow plugin maintainers to do this, > > > > it > > > > > can be found at [1]. I decided to create 2 methods that would be called > > > > from > > > > > Engine, but we could take a different approach and add permissions from > > > plugins automatically by modifying the 'permission' method that is > > > called > > > from 'security_block'. > > > > > > The way I see things: > > > > > > * adding permissions explicitly, using DSL > > > > > > - extra work for plugin maintainers > > > - permissions can be ommited/forgotten > > > > > > # can be covered with tests? something like the already existing > > > > > > AccessPermissionsTest > > > > > > - extra code in Engine > > > - better control, can handle special cases > > > > > > * adding permissions automatically without plugin knowing about it by > > > modifying 'permission' method > > > > > > - no need for plugin maintainers to do anything > > > - cannot handle special cases > > > - if user removes permission from these default roles, there is no way > > > > to > > > > > detect it and permissions get added again on server restart > > > > I'm afraid this could be quite frustrating if you try to modify your > > roles. I'm not entirely sure what part of permissions is stored in the > > db and what is in the code, but would it be possible to add > > permissions during seed and don't touch it on server start? > > All permissions are in db and plugins get loaded before migration runs, > so it should be possible I think. Seems much better (and less frustrating > for users) than resetting to initial state on each restart, > the downside is you will need a migration each time you introduce new > permissions.
Or use the flag the same way as you describe below so it would happen only once, basically when you install the plugin? > > > > # users do not modify default roles, they can clone it and modify > > > the > > > > > > cloned one > > > > > > # if the default roles are not meant to be modified, they should be > > > > > > 'frozen' and user should not be able to modify them, implementing this > > > > would > > > > > add code and complexity > > > > Even though it's more work for plugin maintainers I prefer option A > > because it gives you more control over what gets added. > > > > BTW how would both approaches behave on migration of existing > > installations? Would it add the permissions too? > > Yes, both would add the permissions, forgot to mention that they are > added on each restart for the case A as well :-( > Maybe we could add some sort of 'dirty' flag on the role, as in: when > modified by user, do not touch this role. But then it would not add > permissions when plugins were installed after role was modified. Tricky. The flag would have to be on the permission - role association I guess and it would have to persist even after it's removal. For other resources in seeds we abuse audits for this, if audit exists, we don't touch the resource. > > > > Anything that I have missed? Where do you stand as a plugin maintainers? > > > > Can > > > > > you think of other ways to go? > > > > > > Ondra > > > > > > [1] https://github.com/theforeman/foreman/pull/4168 > > > > > > -- > > > You received this message because you are subscribed to the Google > > > Groups > > > "foreman-dev" group. > > > To unsubscribe from this group and stop receiving emails from it, send > > > an > > > email to [email protected]. > > > For more options, visit https://groups.google.com/d/optout. > > > > -- > > You received this message because you are subscribed to the Google Groups > > "foreman-dev" group. > > To unsubscribe from this group and stop receiving emails from it, send an > > email to [email protected]. > > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
