Flexible permissions for REFRESH MATERIALIZED VIEW

From: Isaac Morland <isaac(dot)morland(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Flexible permissions for REFRESH MATERIALIZED VIEW
Date: 2018-03-18 21:05:17
Message-ID: CAMsGm5d3wKaM2ZHV-ZdE_xBsMGOcFqY-BiyJretS0Pc582tr=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

This is a proposal for a Postgres feature enhancement. I've attached a
preliminary patch. However, the patch is extremely preliminary: there is no
documentation or testing change, and I think I actually want to make the
change itself in a different way from what this 2-line patch does.

Right now I'm really looking for whether anybody observes any problems with
the basic idea. If it's considered to be at least in principle a good idea
then I'll go and make a more complete patch.

The original idea was to allow access to REFRESH MATERIALIZED VIEW to be a
grantable permission, rather than being reserved to the table owner. I
found that permission checking is done in RangeVarCallbackOwnsTable(),
which is also used for CLUSTER and REINDEX.

My initial implementation was to duplicate RangeVarCallbackOwnsTable() and
make a new version just for REFRESH, but then I realized that allowing
CLUSTER and REINDEX to be grantable permissions also might make sense so
the attached patch just modifies RangeVarCallbackOwnsTable().

The patch so far uses TRUNCATE permission to control access as a
proof-of-concept. However, I am considering whether we could add a new
REFRESH permission ('R'?) on tables (including materialized views) to
control access. Of course, we would also rename RangeVarCallbackOwnsTable()
to accurately describe its new function.

When I first had the idea, one concern I had was what would happen to the
security context during REFRESH, but it turns out that checking whether the
operation is allowed and actually setting up the context are completely
separate, I think so that REFRESH triggered by superuser and by the owner
(or for that matter by a role which is a member of the owner) result in the
same security context. So the same should apply if we allow other users to
do a REFRESH.

I think backward compatibility is pretty good. If the feature is ignored
and no REFRESH permissions are granted, then it should work out to the same
as what we have now: owner and superuser are considered to have all table
permissions. pg_upgrade might need to upgrade explicit owner permissions -
I'm not yet absolutely clear on how those work. Anybody who wants the new
feature would be able to use it by granting the new permission, while for
anybody who doesn't need it things should continue working as before, with
the only difference being the exact error message resulting from a
permission violation.

I think the feature is pretty well justified in terms of the overall
Postgres authorization model too. DDL can in general be changed only by
object owners: e.g., renaming, altering columns, that sort of thing.
Changing relation contents, however, is a grantable privilege...but not
when it comes to refreshing materialized views or clustering or reindexing
indexes. So this just makes it so that more non-DDL changes are grantable.
Arguably CLUSTER should require the owner to change on which index the
table is clustered, but my inclination is not to have that additional
complication.

I welcome any comments, questions, and suggestions.

Attachment Content-Type Size
matview-permissions-1.patch application/octet-stream 1.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2018-03-18 21:26:30 Re: include/pgtar.h is missing include guards?
Previous Message Daniel Gustafsson 2018-03-18 20:56:12 Re: [HACKERS] AdvanceXLInsertBuffer vs. WAL segment compressibility