Обсуждение: Possible typo in create_policy.sgml
Hi, Following is perhaps a typo: - qualifications of queries which are run against the table the policy is on, + qualifications of queries which are run against the table if the policy is on, Attached fixes it if so. Thanks, Amit
Вложения
On Tue, Jan 6, 2015 at 12:26 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Following is perhaps a typo: > > - qualifications of queries which are run against the table the policy > is on, > + qualifications of queries which are run against the table if the > policy is on, > > Attached fixes it if so. I don't think that's a typo, although it's not particularly well-worded IMHO. I might rewrite the whole paragraph like this: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. Generally, the system will enforce filter conditions imposed using security policies prior to qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert, Amit,
* Robert Haas (robertmhaas@gmail.com) wrote:
> I don't think that's a typo, although it's not particularly
> well-worded IMHO. I might rewrite the whole paragraph like this:
>
> A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows
> in a table to those rows which match the relevant policy expression.
> Existing table rows are checked against the expression specified via
> USING, while new rows that would be created via INSERT or UPDATE are
> checked against the expression specified via WITH CHECK. Generally,
> the system will enforce filter conditions imposed using security
> policies prior to qualifications that appear in the query itself, in
> order to the prevent the inadvertent exposure of the protected data to
> user-defined functions which might not be trustworthy. However,
> functions and operators marked by the system (or the system
> administrator) as LEAKPROOF may be evaluated before policy
> expressions, as they are assumed to be trustworthy.
Looks reasonable to me. Amit, does this read better for you? If so, I
can handle making the change to the docs.
Thanks!
Stephen
On Tue, Jan 6, 2015 at 11:25 AM, Stephen Frost <sfrost@snowman.net> wrote: > Looks reasonable to me. Amit, does this read better for you? If so, I > can handle making the change to the docs. The docs also prominently say: "The security-barrier qualifications will always be evaluated prior to any user-defined functions or user-provided WHERE clauses, while the with-check expression will be evaluated against the rows which are going to be added to the table. By adding policies to a table, a user can limit the rows which a given user can select, insert, update, or delete. This capability is also known as Row Level Security or RLS." I would prefer it if it was clearer based on the syntax description which qual is which. The security barrier qual "expression" should have an identifier/name in the syntax description that is more suggestive of "security barrier qual", emphasizing its distinctness from "check_expression". For example, I think "barrier_expression" would be clearer. -- Peter Geoghegan
On Tue, Jan 6, 2015 at 2:48 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Tue, Jan 6, 2015 at 11:25 AM, Stephen Frost <sfrost@snowman.net> wrote: >> Looks reasonable to me. Amit, does this read better for you? If so, I >> can handle making the change to the docs. > > The docs also prominently say: > > "The security-barrier qualifications will always be evaluated prior to > any user-defined functions or user-provided WHERE clauses, while the > with-check expression will be evaluated against the rows which are > going to be added to the table. By adding policies to a table, a user > can limit the rows which a given user can select, insert, update, or > delete. This capability is also known as Row Level Security or RLS." > > I would prefer it if it was clearer based on the syntax description > which qual is which. The security barrier qual "expression" should > have an identifier/name in the syntax description that is more > suggestive of "security barrier qual", emphasizing its distinctness > from "check_expression". For example, I think "barrier_expression" > would be clearer. I thought my rewrite clarified this distinction pretty well. Maybe I'm wrong? We're talking about the same paragraph. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas <robertmhaas@gmail.com> wrote: > I thought my rewrite clarified this distinction pretty well. Maybe > I'm wrong? We're talking about the same paragraph. Sorry, I didn't express myself clearly. I think that you did get it right, but I would like to see that distinction also reflected in the actual sgml PARAMETER class tag. "expression" is way too generic here. -- Peter Geoghegan
I also don't see this behavior documented (this is from process_policies()): /* * If we end up with only USING quals, then use those as * WITH CHECK quals also. */ if (with_check_quals == NIL) with_check_quals = copyObject(quals); Now, I do see a reference to it under "Per-Command policies - ALL". It says: "If an INSERT or UPDATE command attempts to add rows to the table which do not pass the ALL WITH CHECK (or USING, if no WITH CHECK expression is defined) expression, the command will error." But is that really the right place for it? Does it not equally well apply to FOR UPDATE policies, that can on their own have both barriers quals and WITH CHECK options? Basically, that seems to me like a *generic* property of policies (it's a generic thing that the WITH CHECK options/expressions "shadow" the USING security barrier quals as check options), and so should be documented as such. -- Peter Geoghegan
On 07-01-2015 AM 04:25, Stephen Frost wrote: > Robert, Amit, > > * Robert Haas (robertmhaas@gmail.com) wrote: >> I don't think that's a typo, although it's not particularly >> well-worded IMHO. I might rewrite the whole paragraph like this: >> >> A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows >> in a table to those rows which match the relevant policy expression. >> Existing table rows are checked against the expression specified via >> USING, while new rows that would be created via INSERT or UPDATE are >> checked against the expression specified via WITH CHECK. Generally, >> the system will enforce filter conditions imposed using security >> policies prior to qualifications that appear in the query itself, in >> order to the prevent the inadvertent exposure of the protected data to >> user-defined functions which might not be trustworthy. However, >> functions and operators marked by the system (or the system >> administrator) as LEAKPROOF may be evaluated before policy >> expressions, as they are assumed to be trustworthy. > > Looks reasonable to me. Amit, does this read better for you? If so, I > can handle making the change to the docs. > Yes, it looks reasonable to me to. Thanks, Amit
On Tue, Jan 6, 2015 at 4:07 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> I thought my rewrite clarified this distinction pretty well. Maybe >> I'm wrong? We're talking about the same paragraph. > > Sorry, I didn't express myself clearly. I think that you did get it > right, but I would like to see that distinction also reflected in the > actual sgml PARAMETER class tag. "expression" is way too generic here. I'm happy to see us change that if it makes sense, but I'm not sure what that actually does. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Tue, Jan 6, 2015 at 4:07 PM, Peter Geoghegan <pg@heroku.com> wrote:
> > On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> >> I thought my rewrite clarified this distinction pretty well. Maybe
> >> I'm wrong? We're talking about the same paragraph.
> >
> > Sorry, I didn't express myself clearly. I think that you did get it
> > right, but I would like to see that distinction also reflected in the
> > actual sgml PARAMETER class tag. "expression" is way too generic here.
>
> I'm happy to see us change that if it makes sense, but I'm not sure
> what that actually does.
If I'm following correctly, Peter's specifically talking about:
[ USING ( <replaceable class="parameter">expression</replaceable> ) ] [ WITH CHECK ( <replaceable
class="parameter">check_expression</replaceable>) ]
Where the USING parameter is 'expression' but the WITH CHECK parameter
is 'check_expression'. He makes a good point, I believe, as
"expression" is overly generic. I don't like the idea of using
"barrier_expression" though as that ends up introducing a new term- how
about "using_expression"?
This would need to be reflected below in the documentation which talks
about "expression" as the parameter to "USING", but I can certainly
handle cleaning that up.
Thanks!
Stephen
On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost <sfrost@snowman.net> wrote: > If I'm following correctly, Peter's specifically talking about: > > [ USING ( <replaceable class="parameter">expression</replaceable> ) ] > [ WITH CHECK ( <replaceable class="parameter">check_expression</replaceable> ) ] > > Where the USING parameter is 'expression' but the WITH CHECK parameter > is 'check_expression'. He makes a good point, I believe, as > "expression" is overly generic. I don't like the idea of using > "barrier_expression" though as that ends up introducing a new term- how > about "using_expression"? Oh. Well, I guess we could change that. I don't think it's a problem, myself. I thought he was talking about something in the SGML markup. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Peter Geoghegan (pg@heroku.com) wrote:
> I also don't see this behavior documented (this is from process_policies()):
>
> /*
> * If we end up with only USING quals, then use those as
> * WITH CHECK quals also.
> */
> if (with_check_quals == NIL)
> with_check_quals = copyObject(quals);
>
> Now, I do see a reference to it under "Per-Command policies - ALL". It says:
>
> "If an INSERT or UPDATE command attempts to add rows to the table
> which do not pass the ALL WITH CHECK (or USING, if no WITH CHECK
> expression is defined) expression, the command will error."
>
> But is that really the right place for it? Does it not equally well
> apply to FOR UPDATE policies, that can on their own have both barriers
> quals and WITH CHECK options? Basically, that seems to me like a
> *generic* property of policies (it's a generic thing that the WITH
> CHECK options/expressions "shadow" the USING security barrier quals as
> check options), and so should be documented as such.
Ah, yes, good point, I can add more documentation around that.
Thanks!
Stephen
On Wed, Jan 7, 2015 at 12:06 PM, Stephen Frost <sfrost@snowman.net> wrote: > Where the USING parameter is 'expression' but the WITH CHECK parameter > is 'check_expression'. He makes a good point, I believe, as > "expression" is overly generic. I don't like the idea of using > "barrier_expression" though as that ends up introducing a new term- how > about "using_expression"? Yes, I think "using_expression" works best. -- Peter Geoghegan
On 6 January 2015 at 19:25, Stephen Frost <sfrost@snowman.net> wrote: > Robert, Amit, > > * Robert Haas (robertmhaas@gmail.com) wrote: >> I don't think that's a typo, although it's not particularly >> well-worded IMHO. I might rewrite the whole paragraph like this: >> >> A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows >> in a table to those rows which match the relevant policy expression. >> Existing table rows are checked against the expression specified via >> USING, while new rows that would be created via INSERT or UPDATE are >> checked against the expression specified via WITH CHECK. Generally, >> the system will enforce filter conditions imposed using security >> policies prior to qualifications that appear in the query itself, in >> order to the prevent the inadvertent exposure of the protected data to >> user-defined functions which might not be trustworthy. However, >> functions and operators marked by the system (or the system >> administrator) as LEAKPROOF may be evaluated before policy >> expressions, as they are assumed to be trustworthy. > > Looks reasonable to me. Amit, does this read better for you? If so, I > can handle making the change to the docs. > I have a wider concern about the wording on this page - both the rewritten paragraph and elsewhere talk about policies in terms of limiting access to or filtering out rows. However, since policy expressions are OR'ed together and there is a default-deny policy when RLS is enabled, I think it should be talking about policies in terms of permitting access to tables that have row security enabled. Regards, Dean
On 8 January 2015 at 08:30, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > I have a wider concern about the wording on this page - both the > rewritten paragraph and elsewhere talk about policies in terms of > limiting access to or filtering out rows. > > However, since policy expressions are OR'ed together and there is a > default-deny policy when RLS is enabled, I think it should be talking > about policies in terms of permitting access to tables that have row > security enabled. > [There's also a typo further down -- "filter out the records which are visible", should be "not visible"] What do you think of the attached rewording? Regards, Dean
Вложения
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> [There's also a typo further down -- "filter out the records which are
> visible", should be "not visible"]
>
> What do you think of the attached rewording?
Rewording it this way is a great idea. Hopefully that will help address
the confusion which we've seen. The only comment I have offhand is:
should we should add a sentence to this paragraph about the default-deny
policy? I feel like that would help explain why the policies are
allowing access to rows.
Thanks!
Stephen
On 8 January 2015 at 18:57, Stephen Frost <sfrost@snowman.net> wrote: >> What do you think of the attached rewording? > > Rewording it this way is a great idea. Hopefully that will help address > the confusion which we've seen. The only comment I have offhand is: > should we should add a sentence to this paragraph about the default-deny > policy? > Yes, good idea, although I think perhaps that sentence should be added to the preceding paragraph, after noting that RLS has to be enabled on the table for the policies to be applied: The <command>CREATE POLICY</command> command defines a new policy for a table. Note that row level security must alsobe enabled on the table using <command>ALTER TABLE</command> in order for created policies to be applied. Once rowlevel security has been enabled, a default-deny policy is used and no rows in the table are visible unless permitted by a specific policy. A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rowsthat would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressionsprior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposureof the protected data to user-defined functions which might not be trustworthy. However, functions and operatorsmarked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, asthey are assumed to be trustworthy. Also, perhaps the "ALTER TABLE" in the first paragraph should be turned into a link. Regards, Dean
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 8 January 2015 at 18:57, Stephen Frost <sfrost@snowman.net> wrote:
> >> What do you think of the attached rewording?
> >
> > Rewording it this way is a great idea. Hopefully that will help address
> > the confusion which we've seen. The only comment I have offhand is:
> > should we should add a sentence to this paragraph about the default-deny
> > policy?
>
> Yes, good idea, although I think perhaps that sentence should be added
> to the preceding paragraph, after noting that RLS has to be enabled on
> the table for the policies to be applied:
I'm a bit on the fence about these ending up as different paragraphs
then, but ignoring that for the moment, I'd suggest we further clarify
with:
The <command>CREATE POLICY</command> command defines a new policy for a table. Note that row level security must
alsobe enabled on the table using <command>ALTER TABLE</command> in order for created policies to be applied. Once
rowlevel security has been enabled, a default-deny policy is used and no rows in the table are visible, except to the
tableowner or superuser, unless permitted by a specific policy.
A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security
enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new
rowsthat would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For
policy expressions specified via USING which grant access to existing rows, the system will generally test the policy
expressionsprior to any qualifications that appear in the query itself, in order to the prevent the inadvertent
exposureof the protected data to user-defined functions which might not be trustworthy. However, functions and
operatorsmarked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions,
asthey are assumed to be trustworthy.
> Also, perhaps the "ALTER TABLE" in the first paragraph should be
> turned into a link.
Ah, yes, agreed.
Thanks!
Stephen
On 9 January 2015 at 20:46, Stephen Frost <sfrost@snowman.net> wrote: > I'd suggest we further clarify > with: > > The <command>CREATE POLICY</command> command defines a new policy for a > table. Note that row level security must also be enabled on the table using > <command>ALTER TABLE</command> in order for created policies to be applied. > Once row level security has been enabled, a default-deny policy is used and > no rows in the table are visible, except to the table owner or > superuser, unless permitted by a specific policy. > > A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows > in a table that has row level security enabled. Access to existing table > rows is granted if they match a policy expression specified via USING, > while new rows that would be created via INSERT or UPDATE are checked > against policy expressions specified via WITH CHECK. For policy > expressions specified via USING which grant access to existing rows, the > system will generally test the policy expressions prior to any > qualifications that appear in the query itself, in order to the prevent the > inadvertent exposure of the protected data to user-defined functions which > might not be trustworthy. However, functions and operators marked by the > system (or the system administrator) as LEAKPROOF may be evaluated before > policy expressions, as they are assumed to be trustworthy. > Looks good to me. Regards, Dean
On Fri, Jan 9, 2015 at 3:46 PM, Stephen Frost <sfrost@snowman.net> wrote: > A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows > in a table that has row level security enabled. Access to existing table > rows is granted if they match a policy expression specified via USING, > while new rows that would be created via INSERT or UPDATE are checked > against policy expressions specified via WITH CHECK. For policy > expressions specified via USING which grant access to existing rows, the > system will generally test the policy expressions prior to any > qualifications that appear in the query itself, in order to the prevent the > inadvertent exposure of the protected data to user-defined functions which > might not be trustworthy. However, functions and operators marked by the > system (or the system administrator) as LEAKPROOF may be evaluated before > policy expressions, as they are assumed to be trustworthy. I think that sticking "while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK" into the middle of this is horribly confusing, as it's a completely separate mechanism from the rest of what's being discussed here. I think there needs to be some initial language that clarifies that USING expressions apply to old rows and WITH CHECK expressions to new rows, and then you can go into more detail. But mentioning WITH CHECK parenthetically in the middle of the rest of this I think will not lead to clarity. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Peter,
* Peter Geoghegan (pg@heroku.com) wrote:
> I also don't see this behavior documented (this is from process_policies()):
[...]
> But is that really the right place for it? Does it not equally well
> apply to FOR UPDATE policies, that can on their own have both barriers
> quals and WITH CHECK options? Basically, that seems to me like a
> *generic* property of policies (it's a generic thing that the WITH
> CHECK options/expressions "shadow" the USING security barrier quals as
> check options), and so should be documented as such.
Thanks, you're right, the documentation there can be improved. I've
pushed up a change to clarify that the USING quals will be used for the
WITH CHECK case if no WITH CHECK quals are specified. Hopefully that's
clear now, but please let me know if you feel further improvements to
this would help.
I do think we will be making additional changes in this area before too
long, but good to get it cleaned up now anyway, so we don't forget to
do so later.
Thanks!
Stephen
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Fri, Jan 9, 2015 at 3:46 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows
> > in a table that has row level security enabled. Access to existing table
> > rows is granted if they match a policy expression specified via USING,
> > while new rows that would be created via INSERT or UPDATE are checked
> > against policy expressions specified via WITH CHECK. For policy
> > expressions specified via USING which grant access to existing rows, the
> > system will generally test the policy expressions prior to any
> > qualifications that appear in the query itself, in order to the prevent the
> > inadvertent exposure of the protected data to user-defined functions which
> > might not be trustworthy. However, functions and operators marked by the
> > system (or the system administrator) as LEAKPROOF may be evaluated before
> > policy expressions, as they are assumed to be trustworthy.
>
> I think that sticking "while new rows that would be created via INSERT
> or UPDATE are checked against policy expressions specified via WITH
> CHECK" into the middle of this is horribly confusing, as it's a
> completely separate mechanism from the rest of what's being discussed
> here. I think there needs to be some initial language that clarifies
> that USING expressions apply to old rows and WITH CHECK expressions to
> new rows, and then you can go into more detail. But mentioning WITH
> CHECK parenthetically in the middle of the rest of this I think will
> not lead to clarity.
I agree, especially after going back and re-reading this while fixing
the issue mentioned earlier by Peter (which was an orthogonal complaint
about the shadowing of WITH CHECK by USING, if WITH CHECK isn't
specified). We really need a paragraph on "USING" policies and another
on "WITH CHECK" policies. How about a reword along these lines:
When row level security is enabled on a table, all access to that table by users other than the owner or a superuser
mustbe through a policy. This requirement applies to both selecting out existing rows from the table and to adding
rowsto the table (through either INSERT or UPDATE).
Granting access to existing rows in a table is done by specifying a USING expression which will be added to queries
whichreference the table. Every row in the table which a USING expression returns true will be visible.
Granting access to add rows to a table is done by specifying a WITH CHECK expressison. A WITH CHECK expression must
returntrue for every row being added to the table or an error will be returned and the command will be aborted. For
policyexpressions specified via USING which grant access to existing rows, the system will generally test the policy
expressionsprior to any qualifications that appear in the query itself, in order to the prevent the inadvertent
exposureof the protected data to user-defined functions which might not be trustworthy. However, functions and
operatorsmarked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as
theyare assumed to be trustworthy.
Thoughts?
Thanks!
Stephen
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > If I'm following correctly, Peter's specifically talking about:
> >
> > [ USING ( <replaceable class="parameter">expression</replaceable> ) ]
> > [ WITH CHECK ( <replaceable class="parameter">check_expression</replaceable> ) ]
> >
> > Where the USING parameter is 'expression' but the WITH CHECK parameter
> > is 'check_expression'. He makes a good point, I believe, as
> > "expression" is overly generic. I don't like the idea of using
> > "barrier_expression" though as that ends up introducing a new term- how
> > about "using_expression"?
>
> Oh. Well, I guess we could change that. I don't think it's a
> problem, myself. I thought he was talking about something in the SGML
> markup.
I agree that it's not a big deal, but I agree with Peter that it's
worthwhile to clarify, especially since this will be seen in psql's \h
w/o the rest of the context of the CREATE POLICY documentation.
I've gone ahead and made this minor change.
Thanks!
Stephen
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> [There's also a typo further down -- "filter out the records which are
> visible", should be "not visible"]
I agree, that's not really worded quite right. I've reworded this along
the lines of what you suggested (though not exactly- if you get a chance
to review it, that'd be great) and pushed it, so we don't forget to do
so later.
> What do you think of the attached rewording?
Regarding the larger/earlier paragraph, Would be great if you could
comment on my latest suggestion.
Thanks!
Stephen
On 29 January 2015 at 04:00, Stephen Frost <sfrost@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost <sfrost@snowman.net> wrote: >> > If I'm following correctly, Peter's specifically talking about: >> > >> > [ USING ( <replaceable class="parameter">expression</replaceable> ) ] >> > [ WITH CHECK ( <replaceable class="parameter">check_expression</replaceable> ) ] >> > >> > Where the USING parameter is 'expression' but the WITH CHECK parameter >> > is 'check_expression'. He makes a good point, I believe, as >> > "expression" is overly generic. I don't like the idea of using >> > "barrier_expression" though as that ends up introducing a new term- how >> > about "using_expression"? >> > I've gone ahead and made this minor change. > Don't forget the ALTER POLICY page. This and some of the other things being discussed on this thread ought to be copied there too. Regards, Dean
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 29 January 2015 at 04:00, Stephen Frost <sfrost@snowman.net> wrote:
> > * Robert Haas (robertmhaas@gmail.com) wrote:
> >> On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost <sfrost@snowman.net> wrote:
> >> > If I'm following correctly, Peter's specifically talking about:
> >> >
> >> > [ USING ( <replaceable class="parameter">expression</replaceable> ) ]
> >> > [ WITH CHECK ( <replaceable class="parameter">check_expression</replaceable> ) ]
> >> >
> >> > Where the USING parameter is 'expression' but the WITH CHECK parameter
> >> > is 'check_expression'. He makes a good point, I believe, as
> >> > "expression" is overly generic. I don't like the idea of using
> >> > "barrier_expression" though as that ends up introducing a new term- how
> >> > about "using_expression"?
> >>
> > I've gone ahead and made this minor change.
>
> Don't forget the ALTER POLICY page. This and some of the other things
> being discussed on this thread ought to be copied there too.
Ah, yeah, good point, will address soon.
Thanks!
Stephen
On Wed, Jan 28, 2015 at 10:45 PM, Stephen Frost <sfrost@snowman.net> wrote: > I agree, especially after going back and re-reading this while fixing > the issue mentioned earlier by Peter (which was an orthogonal complaint > about the shadowing of WITH CHECK by USING, if WITH CHECK isn't > specified). We really need a paragraph on "USING" policies and another > on "WITH CHECK" policies. How about a reword along these lines: > > When row level security is enabled on a table, all access to that > table by users other than the owner or a superuser must be through a > policy. This requirement applies to both selecting out existing rows > from the table and to adding rows to the table (through either INSERT > or UPDATE). > > Granting access to existing rows in a table is done by specifying a > USING expression which will be added to queries which reference the > table. Every row in the table which a USING expression returns true > will be visible. > > Granting access to add rows to a table is done by specifying a WITH > CHECK expressison. A WITH CHECK expression must return true for > every row being added to the table or an error will be returned and > the command will be aborted. I hate to be a critic, but this existing sentence in the documentation seems to explain nearly all of the above: "A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK." The only thing I can see we might need to add is a sentence that says something like "If the USING clause returns false for a particular row, that row will not be visible to the user; if a WITH CHECK expression does not return true, an error occurs." -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Jan 28, 2015 at 10:45 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > I agree, especially after going back and re-reading this while fixing
> > the issue mentioned earlier by Peter (which was an orthogonal complaint
> > about the shadowing of WITH CHECK by USING, if WITH CHECK isn't
> > specified). We really need a paragraph on "USING" policies and another
> > on "WITH CHECK" policies. How about a reword along these lines:
> >
> > When row level security is enabled on a table, all access to that
> > table by users other than the owner or a superuser must be through a
> > policy. This requirement applies to both selecting out existing rows
> > from the table and to adding rows to the table (through either INSERT
> > or UPDATE).
> >
> > Granting access to existing rows in a table is done by specifying a
> > USING expression which will be added to queries which reference the
> > table. Every row in the table which a USING expression returns true
> > will be visible.
> >
> > Granting access to add rows to a table is done by specifying a WITH
> > CHECK expressison. A WITH CHECK expression must return true for
> > every row being added to the table or an error will be returned and
> > the command will be aborted.
>
> I hate to be a critic, but this existing sentence in the documentation
> seems to explain nearly all of the above: "A policy limits the ability
> to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows
> which match the relevant policy expression. Existing table rows are
> checked against the expression specified via USING, while new rows
> that would be created via INSERT or UPDATE are checked against the
> expression specified via WITH CHECK." The only thing I can see we
> might need to add is a sentence that says something like "If the USING
> clause returns false for a particular row, that row will not be
> visible to the user; if a WITH CHECK expression does not return true,
> an error occurs."
Well, I agree (I suppose perhaps that I have to, since I'm pretty sure I
wrote that.. :D), but what Dean was suggesting was a reword that
approached it from the other direction- that is, talk about policies as
granting access, instead of limiting it. I thought that was an
excellent approach which will hopefully reduce confusion. To that end,
how about this reword:
-----------------
A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
which match the relevant policy expression. Existing table rows are
checked against the expression specified via USING, while new rows
that would be created via INSERT or UPDATE are checked against the
expression specified via WITH CHECK. When a USING expression returns
false for a given row, that row is not visible to the user. When a WITH
CHECK expression returns false for a row which is to be added, an error
occurs.
-----------------
This would need to be after we talk about row level security being
enabled for a table and the default-deny policy or it might not make
sense, but otherwise I'm thinking it works.
Thoughts?
Thanks!
Stephen
On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost <sfrost@snowman.net> wrote: > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows > which match the relevant policy expression. Existing table rows are > checked against the expression specified via USING, while new rows > that would be created via INSERT or UPDATE are checked against the > expression specified via WITH CHECK. When a USING expression returns > false for a given row, that row is not visible to the user. When a WITH > CHECK expression returns false for a row which is to be added, an error > occurs. Yeah, that's not bad. I think it's an improvement, in fact. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> > which match the relevant policy expression. Existing table rows are
> > checked against the expression specified via USING, while new rows
> > that would be created via INSERT or UPDATE are checked against the
> > expression specified via WITH CHECK. When a USING expression returns
> > false for a given row, that row is not visible to the user. When a WITH
> > CHECK expression returns false for a row which is to be added, an error
> > occurs.
>
> Yeah, that's not bad. I think it's an improvement, in fact.
Excellent, thanks for the feedback. I'll see about making those changes
tomorrow.
Thanks again!
Stephen
On 30 January 2015 at 03:40, Stephen Frost <sfrost@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost <sfrost@snowman.net> wrote: >> > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows >> > which match the relevant policy expression. Existing table rows are >> > checked against the expression specified via USING, while new rows >> > that would be created via INSERT or UPDATE are checked against the >> > expression specified via WITH CHECK. When a USING expression returns >> > false for a given row, that row is not visible to the user. When a WITH >> > CHECK expression returns false for a row which is to be added, an error >> > occurs. >> >> Yeah, that's not bad. I think it's an improvement, in fact. > Yes I like that too. My main concern was that we should be describing policies in terms of permitting access to the table, not limiting access, because of the default-deny policy, and this new text clears that up. One additional quibble -- it's misleading to say "expression returns false" here (and later in the check_expression parameter description) because if the expression returns null, that's also a failure. So it ought to be "false or null", but perhaps it could just be described in terms of rows matching the expression, with a separate note to say that a row only matches a policy expression if that expression returns true, not false or null. Regards, Dean
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 30 January 2015 at 03:40, Stephen Frost <sfrost@snowman.net> wrote:
> > * Robert Haas (robertmhaas@gmail.com) wrote:
> >> On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost <sfrost@snowman.net> wrote:
> >> > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> >> > which match the relevant policy expression. Existing table rows are
> >> > checked against the expression specified via USING, while new rows
> >> > that would be created via INSERT or UPDATE are checked against the
> >> > expression specified via WITH CHECK. When a USING expression returns
> >> > false for a given row, that row is not visible to the user. When a WITH
> >> > CHECK expression returns false for a row which is to be added, an error
> >> > occurs.
> >>
> >> Yeah, that's not bad. I think it's an improvement, in fact.
>
> Yes I like that too. My main concern was that we should be describing
> policies in terms of permitting access to the table, not limiting
> access, because of the default-deny policy, and this new text clears
> that up.
Great, thanks, pushed.
> One additional quibble -- it's misleading to say "expression returns
> false" here (and later in the check_expression parameter description)
> because if the expression returns null, that's also a failure. So it
> ought to be "false or null", but perhaps it could just be described in
> terms of rows matching the expression, with a separate note to say
> that a row only matches a policy expression if that expression returns
> true, not false or null.
Good point, I've made a few minor changes to address that also, please
let me know if you see any issus.
Thanks!
Stephen
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > Don't forget the ALTER POLICY page. This and some of the other things > being discussed on this thread ought to be copied there too. Thanks, I've fixed this also. Stephen