[DPE-6752] Implement predefined roles #837
Conversation
…ly removed database_owner role creation
…per database + increase upgrade test timeout
shayancanonical
requested review from
dragomirp,
marceloneppel and
taurus-forever
…tabase privileges for users
…ions created by db_owner
…mplement integration tests for predefined roles
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 16/edge #837 +/- ##
===========================================
- Coverage 71.03% 70.63% -0.40%
===========================================
Files 15 15
Lines 3870 3903 +33
Branches 571 572 +1
===========================================
+ Hits 2749 2757 +8
- Misses 938 959 +21
- Partials 183 187 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| cur_user := (SELECT current_user); | ||
| EXECUTE 'SELECT current_database()' INTO db_name; |
There was a problem hiding this comment.
Out of curiosity: does this work in replicas? I had some errors in the past where I tried to use INTO variable in the replicas. I'm not sure if it was really related to the unit being a replica or another reason.
| if "superuser" in roles: | ||
| logger.warning("superuser privileges not allowed via extra-user-roles") | ||
| roles.remove("superuser") |
There was a problem hiding this comment.
Won't this break PgBouncer? I believe PgBouncer needs the SUPERUSER role to grant the SUPERUSER role to Landscape or another application when it connects to the PgBouncer legacy relation called db-admin. I know that it might not apply to this charm version (PG 16), but we may need to handle this on PgBouncer side, to avoid it breaking.
There was a problem hiding this comment.
The PGB auth user also can't be created, since it needs access to user password hashes.
There was a problem hiding this comment.
I was wondering if in PG 16 only, we can translate SUPERUSER extra-user-role to CREATEDB,CREATEROLE in postgresql charm. if pgbouncer does not need superuser, it can continue to request superuser, and we can just provide it with what it needs in PG 16. we will also add support for pgbouncer extra-user-role which pgbouncer can request once we build in support for sharing PG versions through data interfaces
| """Grant the specified priviliges on the provided database for the user.""" | ||
| try: | ||
| with self._connect_to_database() as connection, connection.cursor() as cursor: | ||
| cursor.execute(SQL("GRANT {} ON DATABASE {} TO {};").format(Identifier(", ".join(privileges)), Identifier(database), Identifier(user))) |
There was a problem hiding this comment.
PGB and create role users will need to be able to regrant at least connect privileges to created users.
3 participants
Issue
We would like to introduce pre-defined roles with the introduction of postgres 16 track in our charm
These pre-defined roles not only will help users be more expressive of the
extra-user-rolesthat they can request, but also serve as a means to start hardening the security in our charm. Furthermore, the changes proposed in this PR take a step in the direction of supporting creation of custom roles via data interfaces.Solution
Introduce the following instance level predefined roles:
Introduce, for every database created, the following roles:
Additionally, create a function
set_user_<database_name>_owner()which can be used by<database_name>_adminto escalate to<database_name>_ownerusingset_userextensionAll resources created in a database will be owned by
<database_name>_owner-> or reassigned to<database_name>_ownerwhen the provider relation is broken or when the first provider relation is formed (in the case that there is an existing database already)Backup users have much less privileges than previously (we do not grant all privileges to all databases to the
backupuser anymore)The following
extra-user-rolesstill need to be maintained until we determine how to cleanly deprecate them:charmed_dmlwith all privileges to all databases. additionally, will also havecreatedbattribute enabled for created userThe
superuserrole is being deprecated (albeit, silently. there will be no errors thrown when requested, the charm will simply not comply)TODO
<database_name>_adminto<database_name>_ownerupon login to ensure that all resources created by the user are correctly owned by the owner roleChecklist