From: Stephen Frost <sfrost@snowman.net>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Thom Brown <thom@linux.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Haribabu Kommi <kommi.haribabu@gmail.com>
Subject: Re: Add support for restrictive RLS policies
Date: Mon, 5 Dec 2016 13:15:42 -0500
Message-ID: <20161205181542.GD23417@tamriel.snowman.net> (raw)
In-Reply-To: <CAEZATCVp6P0qmOEtiDWmMOrfcOvnhR_razna5Sa6YQaD27pdZg@mail.gmail.com> <CAEZATCWMD2gaJCzLtxu0v79JCvJL4cUjbNMENjLTrBP07zz_fw@mail.gmail.com> <CAEZATCURYzb8M_DMsmKQ9LQ+rZUmM=3irATtLgkRxopw=j=yjg@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> This note reads a little awkwardly to me. I think I would write it as:
>
> Note that <command>ALTER POLICY</command> only allows the set of roles
> to which the policy applies and the <literal>USING</literal> and
> <literal>WITH CHECK</literal> expressions to be modified. To change
> other properties of a policy, such as the command to which it applies
> or whether it is permissive or restrictive, the policy must be dropped
> and recreated.
Done.
[...]
> really makes sense in this context). So perhaps an additional note
> along the lines of:
>
> Note that there needs to be at least one permissive policy to grant
> access to records before restrictive policies can be usefully used to
> reduce that access. If only restrictive policies exist, then no records
> will be accessible. When a mix of permissive and restrictive policies
> are present, a record is only accessible if at least one of the
> permissive policies passes, in addition to all the restrictive
> policies.
Done.
> Also, I don't think it's necessary to keep quoting "restrictive" and
> "permissive" here.
Quotes removed.
> In get_policies_for_relation(), after the loop that does this:
[...]
> I think it should sort the restrictive policies by name, for the same
> reason that hook-restrictive policies are sorted -- so that the WITH
> CHECK expressions are checked in a well-defined order (which was
> chosen to be consistent with the order of checking of other similar
> things, like CHECK constraints and triggers). I also think that this
> should be a separate sort step from the sort for hook policies,
> inserted just after the loop above, so that the order is all internal
> policies sorted by name, followed by all hook policies sorted by name,
> to be consistent with the existing principle that hook policies are
> applied after internal policies.
Done, adjusted the comments a bit also and added to the regression tests
to test that the sorting is happening as expected.
> I looked through this in a little more detail and I found one other
> issue: the documentation for the system catalog table pg_policy and
> the view pg_policies needs to be updated to include the new columns
> that have been added.
I had noticed this also while going through it again, but thanks again
for the thorough review!
> Other than that, it all looks good to me, subject to the previous comments.
Updated patch attached.
I'll push this in a bit.
Thanks to all who helped!
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, dean.a.rasheed@gmail.com, alvherre@2ndquadrant.com, jeevan.chalke@enterprisedb.com, robertmhaas@gmail.com, tgl@sss.pgh.pa.us, thom@linux.com, kommi.haribabu@gmail.com
Subject: Re: Add support for restrictive RLS policies
In-Reply-To: <20161205181542.GD23417@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox