On 27 July 2015 at 22:36, Joe Conway <mail@joeconway.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/27/2015 01:25 PM, Dean Rasheed wrote:
>> 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.
>
> After looking again at those three call sites, I'm now on the fence.
> All three are in functions which are trying to avoid leaking info in
> error messages. If we leave the original coding, then the messages
> will remain the same for a given user regardless of the row_security
> setting, whereas if we change them as in my latest patch, the messages
> will be different depending on row_security for users with BYPASSRLS.
>
> I think if we do the former (original coding) there should probably be
> a comment added to indicate we are doing that intentionally.
>
> Thoughts?
>
I could go either way on that, but I don't think it's too serious to
worry about leaking information to users with BYPASSRLS.
Regards,
Dean