Re: A little RLS oversight?

On 27 July 2015 at 18:13, Joe Conway <mail(at)joeconway(dot)com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/27/2015 10:03 AM, Joe Conway wrote:
>> On 07/26/2015 07:59 AM, Joe Conway wrote:
>>> On 07/26/2015 07:19 AM, Dean Rasheed wrote:
>>>> Attached is an updated patch (still needs some docs for the
>>>> functions).
>>>
>>> Thanks for that. I'll add the docs.
>>
>> Documentation added. Also added comment to check_enable_rls about
>> passing InvalidOid versus GetUserId().
>>
>> I believe this is ready to go -- any other comments?
>
> Strike that - now I really think it is ready to go :-)
>
> In this patch I additionally changed instances of:
> check_enable_rls(indrelid, GetUserId(), true)
> to
> check_enable_rls(indrelid, InvalidOid, true)
> per Dean's earlier remark and my new comment.
>

Looks good to me, except I'm not sure about those latest changes
because I don't understand the reasoning behind the logic in
check_enable_rls() when row_security is set to OFF.

I would expect that if the current user has permission to bypass RLS,
and they have set row_security to OFF, then it should be off for all
tables that they have access to, regardless of how they access those
tables (directly or through a view). If it really is intentional that
RLS remains active when querying through a view not owned by the
table's owner, then the other calls to check_enable_rls() should
probably be left as they were, since the table might have been updated
through such a view and that code can't really tell at that point.

Regards,
Dean