Re: [PATCH] DefaultACLs - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: [PATCH] DefaultACLs |
Date | |
Msg-id | 25451.1249433596@sss.pgh.pa.us Whole thread Raw |
In response to | Re: [PATCH] DefaultACLs (Stephen Frost <sfrost@snowman.net>) |
Responses |
Re: [PATCH] DefaultACLs
|
List | pgsql-hackers |
I looked at this patch a bit. I haven't actually read any of the code yet, just reviewed the on-list discussions and the docs, but I think I can make a few comments at the definitional level. First: there's already been some discussion about the VIEW problem. I understand that the patch adds a "GRANT ON VIEW" syntax that works like "GRANT ON SEQUENCE" in that if you say VIEW it insists the object really is a view, but without taking away the existing laxness that allows you to still say GRANT ON TABLE for a view. This seems reasonable in isolation, but it creates a big problem for both this patch and the GRANT ON ALL patch, in that it's unclear how to treat views if they could be considered either tables or views. Unfortunately I think we have no choice but to live with the "existing laxness", because (a) to do otherwise will break pretty nearly every pg_dump script, and (b) the SQL standard says we have to accept "GRANT ON TABLE view". In fact "GRANT ON VIEW" is not in the SQL standard and there is no good reason to expect applications will adopt it at all. So my feeling is that adding GRANT ON VIEW is a bad idea. The main argument for doing it seemed to be that the author wanted to be able to grant different default privileges for tables and views, but I'm unconvinced that there's a strong use-case for that. You could very easily have one set of default privileges for all of tables, views, and sequences, simply ignoring any bits that weren't relevant for particular objects. It seems to me that that would handle common cases just fine. For complicated cases not so much, but it's not clear to me that filtering by the object subtype is all that useful for complicated cases anyway. People will want more functionality than that. Which leads into my next item. Second: both this patch and GRANT ON ALL are built on the assumption that the only way to filter/classify objects is by schema membership. Now I don't object to that as an initial implementation restriction, but I don't like hard-wiring it into the syntax. It is very clear to me that we'll want other filter rules in the future --- an immediate example is being able to say that new children of an inheritance parent table should inherit its GRANTs. So to my mind, designing the syntax around ALTER SCHEMA is right out. Maybe we could do something likeALTER DEFAULT PRIVILEGES ON TABLES IN SCHEMA foo GRANT ... where the "IN SCHEMA foo" part would be subject to generalization later. This also matches up a bit better with the proposed syntax for GRANT ON ALL (which also uses "IN SCHEMA foo"). Third: speaking of syntax, I don't like the way that this is gratituously different from GRANT/REVOKE. I don't like using ADD/DROP instead of GRANT/REVOKE, nor the unnecessary AND business. I think we should minimize confusion by using something that is spelled as nearly like GRANT/REVOKE as possible. Fourth: the system's existing treatment of default permissions is owner-dependent, that is the implied set of permissions is typically GRANT ALL TO <owner> (from himself, with grant option). I do not understand how schema-level default ACLs will play nicely with that, except in the special case where the schema owner also owns every contained object. If you copy the schema-level ACL directly to a contained object with a different owner it will definitely be the wrong thing, but if you try to translate the ownership it will confuse people too. And confusion in a security-related behavior is a Bad Thing. Furthermore, having the schema owner able to control the grants made for objects not owned by him is a huge security hole. What I suggest as a way to resolve this last point is that a default ACL should apply only to objects owned by the user who creates/modifies the default ACL. In this view, the question of which schema the objects are in is just an additional filter condition, not the primary determinant of which objects a default ACL applies to. Every user has his own set of default ACLs. (This is another reason not to design the syntax around ALTER SCHEMA.) regards, tom lane
pgsql-hackers by date: