On Tue, 2022-01-18 at 16:16 +0100, Christoph Heiss wrote: > > I think that this should be a boolean reloption, for example > > "security_definer". > > If unset or set to "off", you would get the current behavior. > > A boolean option would have been indeed the better choice, I agree. > I haven't though of any specific other values for this enum, it was > rather a decision following a off-list discussion. > > I've changed the option to be boolean and renamed it to > "security_invoker". This puts it in line with how other systems (e.g. > MySQL) name their equivalent feature, so I think this should be an > appropriate choice. > > > > Also, I don't think that the documentation on > > RLS policies is the correct place for this. It should be on a page > > dedicated to views > > or permissions. > > > > The CREATE VIEW page already has a paragraph about this, starting with > > "Access to tables referenced in the view is determined by permissions of > > the view owner." > > This looks like the best place to me (and it would need to be adapted > > anyway). > It makes sense to put it there, thanks for the pointer! I wasn't really > that sure where to put the documentation to start with, and this seems > like a more appropriate place.
I gave the new patch a spin, and got a surprising result: CREATE TABLE tab (id integer); CREATE ROLE duff LOGIN; CREATE ROLE jock LOGIN; GRANT INSERT, UPDATE, DELETE ON tab TO jock; GRANT SELECT ON tab TO duff; CREATE VIEW v WITH (security_invoker = TRUE) AS SELECT * FROM tab; ALTER VIEW v OWNER TO jock; GRANT SELECT, INSERT, UPDATE, DELETE ON v TO duff; SET SESSION AUTHORIZATION duff; SELECT * FROM v; id ════ (0 rows) That's ok, "duff" has permissions to read "tab". INSERT INTO v VALUES (1); INSERT 0 1 Huh? "duff" has no permission to insert into "tab"! RESET SESSION AUTHORIZATION; ALTER VIEW v SET (security_invoker = FALSE); SET SESSION AUTHORIZATION duff; SELECT * FROM v; ERROR: permission denied for table tab As expected. INSERT INTO v VALUES (1); INSERT 0 1 As expected. About the documentation: --- a/doc/src/sgml/ref/create_view.sgml +++ b/doc/src/sgml/ref/create_view.sgml + <varlistentry> + <term><literal>security_invoker</literal> (<type>boolean</type>)</term> + <listitem> + <para> + If this option is set, it will cause all access to the underlying + tables to be checked as referenced by the invoking user, rather than + the view owner. This will only take effect when row level security is + enabled on the underlying tables (using <link linkend="sql-altertable"> + <command>ALTER TABLE ... ENABLE ROW LEVEL SECURITY</command></link>). + </para> Why should this *only* take effect if (not "when") RLS is enabled? The above test shows that there is an effect even without RLS. + <para>This option can be changed on existing views using <link + linkend="sql-alterview"><command>ALTER VIEW</command></link>. See + <xref linkend="ddl-rowsecurity"/> for more details on row level security. + </para> I don't think that it is necessary to mention that this can be changed with ALTER VIEW - all storage parameters can be. I guess you copied that from the "check_option" documentation, but I would say it need not be mentioned there either. + <para> + If the <firstterm>security_invoker</firstterm> option is set on the view, + access to tables is determined by permissions of the invoking user, rather + than the view owner. This can be used to provide stricter permission + checking to the underlying tables than by default. </para> Since you are talking about use cases here, RLS might deserve a mention. --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c + { + { + "security_invoker", + "View subquery in invoked within the current security context.", + RELOPT_KIND_VIEW, + AccessExclusiveLock + }, + false + }, That doesn't seem to be proper English. Yours, Laurenz Albe