Adam, all, * Brightwell, Adam (adam.brightw...@crunchydatasolutions.com) wrote: > Attached is a patch for RLS that was create against master at > 01363beae52700c7425cb2d2452177133dad3e93 and is ready for review.
Many thanks for posting this. As others may realize already, I've reviewed and modified this patch already, working with Adam to get it ready. I'm continuing to review and test it, but in general I'm quite happy with how it's shaping up- additional review, testing, and comments are always appreciated though. > Alter a RLS policy named <name> on <table>. Specifying a command is > optional, if provided then the policy's command is changed otherwise it is > left as-is. Specifying a role is optional, if provided then the policy's > role is changed otherwise it is left as-is. The <condition> must always be > provided and is therefore always replaced. I'm pretty sure the <condition> is also optional in this patch (that was a late change that I made), but the documentation needs to be updated. > * Plancache Invalidation: If a relation has a row-security policy and > row-security is enabled then the invalidation will occur when either the > row_security GUC is changed OR when a the current user changes. This > invalidation ONLY takes place for cached plans where the target relation > has a row security policy. I know there was a lot of discussion about this previously, but I'm fine with the initial version simply invalidating plans which involve RLS-enabled relations and role changes. This patch doesn't create any regressions for individuals who are not using RLS. We can certainly look into improving this in the future to have per-role plan caches but it's a fair bit of additional non-trivial code that can be added independently. > * Security Qual Expression: All row-security policies are OR'ed together. This was also a point of much discussion, but I continue to feel this is the right approach for the initial version. We can add flexability here later, if necessary, but OR'ing these together is in-line with how role membership works today (you have right for all roles you are a member of, persuant to inherit/noinherit status, of course). > * row_security GUC - enable/disable row level security. Note that, as discussed, pg_dump will set row_security off, unless specifically asked to enable it. row_security will also be set to off when the user logging in is a superuser or does a 'set role' to a superuser. Currently, if a user logging in is *not* a superuser, or a 'set role' is done to a non-superuser, row_security gets re-set to enabled. This is one aspect of the patch that I think we should change (which is a matter of removing just a few lines of code and then updating the regression tests to do 'set row_security = on;' before running), because if you log in as a superuser and then 'set role' to a non-superuser, it occurs to me now (it didn't really when I wrote this originally) as a bit surprising that row_security gets set to 'on' when doing a 'set role'. One thing that I really like about this approach is that a superuser can explicitly set 'row_security' to on and be able to see what happens. Clearly, in an environment of untrusted users, that could be dangerous, but it can also be an extremely useful way of testing things, particularly in development environments where everyone is a superuser. This deserves a bit more documentation also. > * BYPASSRLS and NOBYPASSRLS role attribute - allows user to bypass RLS if > row_security GUC is set to OFF. If a user sets row_security to OFF and > does not have this attribute, then an error is raised when attempting to > query a relation with a RLS policy. (note that the superuser is always considered to have the bypassrls attribute) > * psql \d <table> support: psql describe support for listing policy > information per table. This works pretty well for me, but we may want to add some indication that RLS is on a table in the \dp listing. Thanks! Stephen
Description: Digital signature