MBS-11962: Update privileges for all database users on schema changes

[mwiencek]

Problem

MBS-11962

When new tables or schemas are added, production database users like musicbrainz_ro may not be granted access to them.

Solution

Adds a new script, admin/UpdateDatabasePrivileges.pl, which is invoked from upgrade.sh and updates privileges for the following users if they exist:

• musicbrainz_ro
• caa_redirect
• sir

The script works by revoking all write privileges from musicbrainz_ro, and then granting USAGE on all schemas, and SELECT privileges on all tables in those schemas.

Next, for the last two schemas, it revokes all privileges from them (including SELECT and USAGE), and then simply grants them membership in the musicbrainz_ro role.

Testing

I added the three roles in question to my local database and ran admin/UpdateDatabasePrivileges.pl. I observed that each of the three users could SELECT from any table in our schemas, but not modify them.

[reosarevok]

The idea seems great. Can we add a test for the script, please? :)

[reosarevok]

Tests seem to be failing

[mwiencek]

Tests seem to be failing

Seems to be because the WITH INHERIT clause I'm using was added in PostgreSQL 16.

[reosarevok]

LGTMBDNT

[yvanzo]

I created the roles musicbrainz_ro, caa_redirect and sir using admin/psql then ran the new script and obtained the following error message:

`LINE 3: WITH INHERIT TRUE;`

Failed query:
        'GRANT "musicbrainz_ro"
   TO "caa_redirect"
 WITH INHERIT TRUE;
'
        ()
42601 DBD::Pg::st execute failed: ERROR:  syntax error at or near "INHERIT"
LINE 3:  WITH INHERIT TRUE;
              ^ [for Statement "GRANT "musicbrainz_ro"
   TO "caa_redirect"
 WITH INHERIT TRUE;
"]
 at /musicbrainz-server/admin/../lib/Sql.pm line 123.
        Sql::catch {...} (MusicBrainz::Server::Exceptions::DatabaseError=HASH(0x5acb49453658)) called at /musicbrainz-server/perl_modules/lib/perl5/Try/Tiny.pm line 123
        Try::Tiny::try(CODE(0x5acb49453dd8), Try::Tiny::Catch=REF(0x5acb49353978)) called at /musicbrainz-server/admin/../lib/Sql.pm line 124
        Sql::do(Sql=HASH(0x5acb48ebf288), "GRANT \"musicbrainz_ro\"\x{a}   TO \"caa_redirect\"\x{a} WITH INHERIT TRUE;\x{a}") called at ./admin/UpdateDatabasePrivileges.pl line 119

Edit: I just realized from your above comment that PG 16 is required here. I can’t test it at the moment, but my other comments probably still stand.

[mwiencek]

@yvanzo I think I've addressed all of your comments (and it should also work on PG<16 now).

[yvanzo]

Thank you!

💯 agreed that WITH INHERIT is better set once for all for roles. The other new syntax in PG 16 is probably for more casual use case.

---

I made a couple of (also minor) ready-to-pick suggestions for improving the output.

---

Last, when running this script (without --nogrant) two times in a row, two additional lines are logged:

NOTICE:  role "caa_redirect" is already a member of role "musicbrainz_ro"

NOTICE:  role "sir" is already a member of role "musicbrainz_ro"

This doesn’t seem to be an issue, mentioning it just in case.

[mwiencek]

Thanks!

Last, when running this script (without --nogrant) two times in a row, two additional lines are logged:

Yeah, those are logged by PG if you try to grant a privilege that already exists. It's harmless though and may be useful information (if it wasn't expected for them to exist).