Re: unclear about row-level security USING vs. CHECK - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: unclear about row-level security USING vs. CHECK
Date
Msg-id 20150928191533.GU3685@tamriel.snowman.net
Whole thread Raw
In response to Re: unclear about row-level security USING vs. CHECK  (Peter Eisentraut <peter_e@gmx.net>)
Responses Re: unclear about row-level security USING vs. CHECK  (Robert Haas <robertmhaas@gmail.com>)
Re: unclear about row-level security USING vs. CHECK  ("Charles Clavadetscher" <clavadetscher@swisspug.org>)
Re: unclear about row-level security USING vs. CHECK  (Dean Rasheed <dean.a.rasheed@gmail.com>)
List pgsql-hackers
* Peter Eisentraut (peter_e@gmx.net) wrote:
> I see.  But it is a bit odd to hide this very fundamental behavior
> somewhere in a paragraph that starts out with something about roles.

I'm happy to change that.  You're right, it should be a paragraph by
itself.

> There is also a mistake, I believe: DELETE policies also take both a
> CHECK and a USING clause.

DELETE never adds records and therefore does not take a CHECK clause,
only a USING clause:

=*# create policy p1 on t1 for delete using (c1 > 5) with check (c1 > 10);
ERROR:  WITH CHECK cannot be applied to SELECT or DELETE

There has been some discussion about changing that, but that would be a
future change and not for 9.5.

> I still find something about this weird, but I'm not sure what.  It's
> not clear to me at what level this USING->CHECK mapping is applied.  I
> can write FOR ALL USING and it will be mapped to CHECK for all actions,
> including INSERT, but when I write FOR INSERT USING it complains.  Why
> doesn't it do the mapping that case, too?

INSERT is only adding records and therefore only the CHECK policy
applies:

=*# create policy p1 on t1 for insert using (c1 > 5) with check (c1 > 10);
ERROR:  only WITH CHECK expression allowed for INSERT

The USING clause is for existing records while the CHECK option is for
new records, which is why DELETE only has a USING clause and INSERT only
has a WITH CHECK clause.  ALL allows you to specify clauses for all
commands, which is why it accepts both.  The only other case which
allows both is UPDATE, where records are both retrived and added.

> >> (Btw., what's the meaning of a policy for DELETE?)
> >
> > The DELETE policy controls what records a user is able to delete.
>
> That needs to be documented somewhere.

This is included in the CREATE POLICY documentation:

DELETE
   Using DELETE for a policy means that it will apply to DELETEcommands. Only rows which pass this policy will be seen
bya DELETEcommand. Rows may be visible through a SELECT which are not seen bya DELETE, as they do not pass the USING
expressionfor the DELETE,and rows which are not visible through the SELECT policy may bedeleted if they pass the DELETE
USINGpolicy. The DELETE policy onlyaccepts the USING expression as it only ever applies in cases whererecords are being
extractedfrom the relation for deletion. 

I'm certainly all for improving the documentation, of course.  What
about the above isn't clear regarding what DELETE policies do?  Or is
the issue that it wasn't covered in ddl-rowsecurity?  Perhaps we should
simply move much of the CREATE POLICY documentation into ddl-rowsecurity
instead, since that's where people seem to be looking for this
information?

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Sat, Sep 26, 2015 at 9:46 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > On 9/23/15 3:41 PM, Stephen Frost wrote:
> > I see.  But it is a bit odd to hide this very fundamental behavior
> > somewhere in a paragraph that starts out with something about roles.
> >
> > There is also a mistake, I believe: DELETE policies also take both a
> > CHECK and a USING clause.
> >
> > I still find something about this weird, but I'm not sure what.  It's
> > not clear to me at what level this USING->CHECK mapping is applied.  I
> > can write FOR ALL USING and it will be mapped to CHECK for all actions,
> > including INSERT, but when I write FOR INSERT USING it complains.  Why
> > doesn't it do the mapping that case, too?
>
> We are really pushing our luck only hammering this stuff out now.  But
> I think I agree with Peter's concerns, FWIW.

I listed out the various alternatives but didn't end up getting any
responses to it.  I'm still of the opinion that the documentation is the
main thing which needs improving here, but we can also change CREATE
POLICY, et al, to require an explicit WITH CHECK clause for the commands
where that makes sense if that's the consensus.

Thanks!

Stephen

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: RLS open items are vague and unactionable
Next
From: Stephen Frost
Date:
Subject: Re: Rename withCheckOptions to insertedCheckClauses