Re: Row Level Security UPDATE Confusion

On Fri, Apr 14, 2017 at 7:41 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:

>
>
>
> On Thu, Apr 13, 2017 at 5:31 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
>> Rod, all,
>>
>> * Joe Conway (mail(at)joeconway(dot)com) wrote:
>> > On 04/13/2017 01:31 PM, Stephen Frost wrote:
>> > > * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> > >> On Thu, Apr 6, 2017 at 4:05 PM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com>
>> wrote:
>> > >> > I'm a little confused on why a SELECT policy fires against the NEW
>> record
>> > >> > for an UPDATE when using multiple FOR policies. The ALL policy
>> doesn't seem
>> > >> > to have that restriction.
>> > >>
>> > >> My guess is that you have found a bug.
>> > >
>> > > Indeed. Joe's been looking into it and I'm hoping to find some time
>> to
>> > > dig into it shortly.
>> >
>> > >> CREATE POLICY split_select ON t FOR SELECT TO split
>> > >> USING (value > 0);
>> > >> CREATE POLICY split_update ON t FOR UPDATE TO split
>> > >> USING (true) WITH CHECK (true);
>> >
>> > Yes -- from what I can see in gdb:
>>
>> Actually, looking at this again, the complaint appears to be that you
>> can't "give away" records. That was a topic of much discussion and I'm
>> reasonably sure that was what we ended up deciding made the most sense.
>> You have to be able to see records to be able to update them (you can't
>> update records you can't see), and you have to be able to see the result
>> of your update. I don't doubt that we could improve the documentation
>> around this (and apparently the code comments, according to Joe..).
>>
>>
> Then there is a bug in the simpler statement which happily lets you give
> away records.
>
> CREATE POLICY simple_all ON t TO simple USING (value > 0) WITH CHECK
> (true);
>
> SET session authorization simple;
> SELECT * FROM t;
> UPDATE t SET value = value * -1 WHERE value = 1;
> -- No error and value is -1 at the end.
>

My actual use-case involves a range. Most users can see and manipulate the
record when CURRENT_TIMESTAMP is within active_period. Some users
(staff/account admins) can see recently dead records too. And a 3rd group
(senior staff) have no time restriction, though there are a few customers
they cannot see due to their information being a touch more sensitive.
I've simplified the below rules to just deal with active_period and the
majority of user view (@> CURRENT_TIMESTAMP).

NOTE: the active_period range is '[)' by default, so records with upper() =
CURRENT_TIMESTAMP are not visible with @> CURRENT_TIMESTAMP restriction.

CREATE A TABLE t (id integer, active_period tstzrange NOT NULL DEFAULT
tstzrange(current_timestamp, NULL));

The below policy is allowed but requires that 1ms slop to accommodate the wi

Updated record invisible to USING but requires a trigger to enforce
specific upper
and starting values. I have a trigger enforcing specific upper/lower values
for the range
for specific ROLEs. So I had the thought that I might move ROLE specific
trigger logic into
the RLS mechanism.

CREATE POLICY hide_old ON t TO s;
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( active_period && tstzrange(current_timestamp - interval
'0.001 seconds', current_timestamp, '[]'));

-- This is effectively a delete for the above policy. It becomes
immediately invisible due to USING restriction.
UPDATE t SET active_period = tstzrange(lower(active_period),
CURRENT_TIMESTAMP);
SELECT count(*) FROM t; -- 0 records

I tried to tighten the above rules, so INSERT must have upper of NULL and
UPDATE must
set upper to exactly CURRENT_TIMESTAMP. Clearly I can achieve this using
triggers for
enforcement but I tried to abuse RLS instead because it is a role specific
restriction.

I was surprised when hide_old_select->USING was preventing the UPDATE when
the simple single policy version let it through.

CREATE POLICY hide_old_select ON t FOR SELECT TO s
USING ( active_period @> CURRENT_TIMESTAMP);
CREATE POLICY hide_old_insert ON t FOR INSERT to s
WITH CHECK ( lower(active_period) = CURRENT_TIMESTAMP AND
upper(active_period) IS NULL);

CREATE POLICY hide_old_update ON t FOR UPDATE TO s
USING ( active_period @> CURRENT_TIMESTAMP)
WITH CHECK ( upper(active_period) = CURRENT_TIMESTAMP);

-- Disallowed due to hide_old_select policy.
UPDATE t SET active_period = tstzrange(lower(active_period),
CURRENT_TIMESTAMP);

I'm happy to help with testing and documentation but first I need to
understand what the intended functionality was. Right now it seems
inconsistent between the simple single policy version and the multi policy
version; the docs imply the single policy version is correct (it only seems
to mention SELECT checks on RETURNING clauses).

--
Rod Taylor