Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Apr 13, 2016 at 6:53 PM, Stephen Frost <sfr...@snowman.net> wrote: > > Requiring that SET ROLE be allowed will mean that many more paths must > > be checked and adjusted, such as in all of the CreateObject statements > > and potentionally many other paths that I'm not thinking of here, not > > the least of which are all of the *existing* checks near > > get_rolespec_oid/tuple which will require additional checks for the > > CURRENT_USER case to see if the current user is a default role. > > I don't get it. I think that these new roles should work just like > any other roles, except for existing at initdb time. I don't see why > they would require checking or adjusting any code-paths at all. It > would presumably have made the patch you committed smaller and > simpler. The only thing you'd need to do is (approximately) not dump > them.
Perhaps it would be helpful to return to the initial goal of these default roles. We've identified certain privileges which are currently superuser-only and we would like to be able to be GRANT those to non-superuser roles. Where our GRANT system is able to provide the granularity desired, we have simply removed the superuser checks, set up appropriate default values and, through the "pg_dump dump catalog ACLs" patch, allowed administrators to make the changes to those objects via the 'GRANT privilege ON object TO role' system. For those cases where our GRANT system is unable to provide the granularity desired and it isn't sensible to extend the GRANT system to cover the exact granularity we wish to provide, we needed to come up with an alternative approach. That approach is the use of special default roles, where we can allow exactly the level of granularity desired and give administrators a way to grant those privileges to users. What this means in a nutshell is that we've collectivly decided that we'd rather support: /* uses the 'GRANT role TO role' system */ GRANT pg_signal_backend TO test_user; than: /* * uses the 'GRANT privilege ON object TO role' system * seems like cluster-level is actually the right answer here, but * we don't support such an object type currently, so using database * for the example */ GRANT SIGNAL BACKEND ON DATABASE my_database TO test_user; The goal being that the result of the two commands is identical (where the second command is only hypothetical at this point, but hopefully the meaning is conveyed). However, GRANT'ing a role to a user traditionally also allows the user to 'SET ROLE' to that user, to create objects as that user, and other privileges. This results in two issues, as I see it: 1) The administrator who is looking to grant the specific 'signal backend' privilege to a user is unlikely to intend or desire for that user to also be able to SET ROLE to the role, or for that user to be able to create objects as the default role. 2) If the default role ends up owning objects then we have much less flexibility in making changes to that role because we must ensure that those objects are preserved across upgrades, etc. Further, there seems to be no particular use-case which is satisfied by allowing SET ROLE'ing to the default roles or for those roles to own objects; indeed, it seems more likely to simply spark confusion and ultimately may prevent administrators from making use of this system for granting privileges as they are unable to GRANT just the specific privilege they wish to. It now occurs to me that perhaps we can improve on this situation in the future by formally supporting a role attribute that is "do not allow SET ROLE to this user" and then changing the default roles to have that attribute set (and disallowing users from changing it). That's just additional flexibility over what the current patch does, however, but perhaps it helps illustrate that there are cases where only the privileges of the role are intended to be GRANT'd and not the ability to SET ROLE to the role or to create objects as that role. I hope that helps. I'll be in NY next week also, so perhaps that would be a good opportunity to have an interactive discussion on this topic. Thanks! Stephen
signature.asc
Description: Digital signature