On pondělí 23. ledna 2017 17:07:30 CET Lukas Zapletal wrote: > > If you're concerned about the permissions table, this does not help. > > Permission are created there by plugin. If the permission is removed > > later, it should be removed from all roles anyway, user could already > > assign it to both core and plugin role. > > Down below. > > > I suppose you wrote a migration that unassigned this permission from all > > filters and removed the permission from permissions table. Why does this > > not work for core? > > I agree that technically no difference, but ability to have a list of > default roles and permissions (what I call "pull mechanism" but > basically its a list or hash of these) allows us to audit this. It's > something we already have today (hashes for core is in seeds, hashes > for plugins can be added via the existing API hash mentioned). I > simply think that we lost track of this when we will be creating new > permissions for existing roles via this call (usually in engine.rb). > > >> I really think we need to say "this is by design, add required roles > >> which are provided by plugins" here. Not all RFEs are valid. > > > > IMHO this is usability issue. And therefore valid RFE. > > Sure, I made it to extreme, but I am trying to put the requirement > from the RFE on weights together with above. > > > I read your counter-proposal (I don't think it's "counter" :-) If I > > understand it correctly, we could break i down into following steps. > > 1) add DSL to extend core roles > > 2) make core roles locked for editing > > Locking is indeed nice, but it does not solve my concerns above - lost > track. But I like it, let me build on top of that: what if the API was > adding the permission twice - once into the core (locked) role and > once into "PluginName Manager/Reader" (locked) role as a copy. So > users could decide what to include for their Users or UserGroups. This > could keep some flexibility and also enable better audit. The API > would be responsible for copying them so plugin authors would not > care.
Yup, that was the plan from the very beginning, just without locking. Locking can be added and everyone is happy. > > > 3) add DSL to remove permissions from roles > > I don't insist on this one, removing permissions can be done via > migrations as you noted. This only makes sense if it's actually better > to have such an API method(s). It might end up in situation when > migration would have been much more easier and cleaner - then this is > a no go of course. > > > I find the locking better since it's similar concept to templates where we > > have the same problem. But I like the proposal in general for future > > plugin > > uninstallation. If audit can't give us the answer we should start tracking > > it, ideally the same way for all resources created by plugin. > > Sure, locking is a good start, my biggest concern was to have a role > (e.g. Manager) modified by core, plugins, users. Three individual > subjects, locking should help here. > > If locking is the thing we agreed here, I vote for implementing it > with the new API so we are clean from the day one. Technically we can > make the upgrade easier if we decide to create new locked roles with > names like "Default Manager" and "Default Reader" (or Viewer not sure) > keeping the original "Manager" and "Reader" unchanged. If we also > implement a comparison tool, that is huge improvement in user > experience IMHO. Ok, we discussed the migration path. We should be (with small changes*) able to detect whether users modified existing Manager role. In case they did, we would rename them to "Customized Manager" and create new Manager role with default permissions but locked. Locked means no changes to filters and attributes can be made by user, plugin can extend via plugin DSL. If existing Manager role was not changed we just lock it. The same for the Viewer. No locking/unlocking actions as we have for templates. If this is acceptable migration path, then I think we can start working on it. Ivan had also a good point with task oriented roles. Once we have locking we can add more locked roles like "Provisioning capabilities". And potentially revisit automatic adding permissions to locked Manager and Viewer roles so they won't get out of sync. Sounds as a plan? *small changes means we extract hashes of permissions in seeds to some constant so it's available without running seeds and extending existing plugin DSL method "role" so we know whether plugin extends existing Manager/Viewer role or user did it manually. -- Marek -- 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.
