Re: [PATCH] Add reloption for views to enable RLS

On Tue, 2022-02-15 at 13:02 +0100, Christoph Heiss wrote:

> > >
> I converted the option to run_as_owner=true|false in the attached v7.
> It now definitely seems like the right way to move forward and getting
> more feedback.

I think we are straying from the target.

"run_as_owner" seems wrong to me, because it is all about permission
checking and *not* about running. As we have established, the query
is always executed by the caller.

So my preferred bikeshed colors would be "permissions_owner" or
"permissions_caller".

About the documentation:

--- a/doc/src/sgml/ref/alter_view.sgml
+++ b/doc/src/sgml/ref/alter_view.sgml
@@ -156,11 +156,21 @@ ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RESET
<listitem>
<para>
Changes the security-barrier property of the view. The value must
- be Boolean value, such as <literal>true</literal>
+ be a Boolean value, such as <literal>true</literal>
or <literal>false</literal>.
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>run_as_owner</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Changes the user as which the subquery is run. Default is
+ <literal>true</literal>. The value must be a Boolean value, such as
+ <literal>true</literal> or <literal>false</literal>.
+ </para>
+ </listitem>
+ </varlistentry>

Correct would be

If set to <literal>true</literal> (which is the default value), permissions
on the underlying relations are checked as view owner, otherwise as the user
executing the query.

(I used "relation" to express that it doesn't hold for functions.)

--- a/doc/src/sgml/ref/create_view.sgml
+++ b/doc/src/sgml/ref/create_view.sgml
@@ -265,13 +278,39 @@ CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
</para>

<para>
- Access to tables referenced in the view is determined by permissions of
- the view owner. In some cases, this can be used to provide secure but
- restricted access to the underlying tables. However, not all views are
- secure against tampering; see <xref linkend="rules-privileges"/> for
- details. Functions called in the view are treated the same as if they had
- been called directly from the query using the view. Therefore the user of
- a view must have permissions to call all functions used by the view.
+ By default, access to tables and functions referenced in the view is
+ determined by permissions of the view owner.

No, access to the functions is checked for the caller.

+ [...] Therefore the user of a view must have permissions

Comma after "therefore".

+ to call all functions used by the view. This also means that functions
+ are executed as the invoking user, not the view owner.
+ </para>
+
+ <para>
+ However, when using chained views, the <literal>CURRENT_USER</literal> user
+ will always stay the invoking user,

"However" would introduce something that is different from what came before,
which this doesn't seem to be.

Perhaps "In particular" or "moreover".

+ regardless of whether the query is run
+ as the view owner (the default) or the invoking user (when
+ <literal>run_as_owner</literal> is set to <literal>false</literal>)
+ and the depth of the current invocation.
+ </para>

The query is *always* run as the invoking user. Better:

regardless of whether relation permissions are checked as the view owner or ...

+ <para>
+ Be aware that <literal>USAGE</literal> privileges on schemas are not checked
+ when referencing the underlying base relations, even if they are part of a
+ different schema.
</para>

"referencing" is a bit unclear.
Perhaps "when checking permissions on the underlying base relations".

Otherwise, this looks good!

Yours,
Laurenz Albe