[MISC] Sanitize PostgreSQL extra-user-roles arg (#876)

Author: sinclert-canonical

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -35,7 +35,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 43
+LIBPATCH = 44
 
 # Groups to distinguish database permissions
 PERMISSIONS_GROUP_ADMIN = "admin"
@@ -223,7 +223,7 @@ def create_user(
         user: str,
         password: Optional[str] = None,
         admin: bool = False,
-        extra_user_roles: Optional[str] = None,
+        extra_user_roles: Optional[list[str]] = None,
     ) -> None:
         """Creates a database user.
 
@@ -238,7 +238,6 @@ def create_user(
             admin_role = False
             roles = privileges = None
             if extra_user_roles:
-                extra_user_roles = tuple(extra_user_roles.lower().split(","))
                 admin_role = PERMISSIONS_GROUP_ADMIN in extra_user_roles
                 valid_privileges, valid_roles = self.list_valid_privileges_and_roles()
                 roles = [
@@ -572,7 +571,7 @@ def set_up_database(self) -> None:
                     )
                 self.create_user(
                     PERMISSIONS_GROUP_ADMIN,
-                    extra_user_roles="pg_read_all_data,pg_write_all_data",
+                    extra_user_roles=["pg_read_all_data", "pg_write_all_data"],
                 )
                 cursor.execute("GRANT CONNECT ON DATABASE postgres TO admin;")
         except psycopg2.Error as e:

src/charm.py

@@ -1093,7 +1093,7 @@ def _initialize_cluster(self, event: WorkloadEvent) -> bool:
             self.postgresql.create_user(
                 MONITORING_USER,
                 self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY),
-                extra_user_roles="pg_monitor",
+                extra_user_roles=["pg_monitor"],
             )
 
         self.postgresql.set_up_database()

src/relations/postgresql_provider.py

@@ -65,6 +65,14 @@ def __init__(self, charm: CharmBase, relation_name: str = "database") -> None:
             self.database_provides.on.database_requested, self._on_database_requested
         )
 
+    @staticmethod
+    def _sanitize_extra_roles(extra_roles: str | None) -> list[str]:
+        """Standardize and sanitize user extra-roles."""
+        if extra_roles is None:
+            return []
+
+        return [role.lower() for role in extra_roles.split(",")]
+
     def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
         """Handle the legacy postgresql-client relation changed event.
 
@@ -80,7 +88,9 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
 
         # Retrieve the database name and extra user roles using the charm library.
         database = event.database
-        extra_user_roles = event.extra_user_roles
+
+        # Make sure that certain groups are not in the list
+        extra_user_roles = self._sanitize_extra_roles(event.extra_user_roles)
 
         try:
             # Creates the user and the database for this specific relation.
@@ -268,9 +278,7 @@ def check_for_invalid_extra_user_roles(self, relation_id: int) -> bool:
                 continue
             for data in relation.data.values():
                 extra_user_roles = data.get("extra-user-roles")
-                if extra_user_roles is None:
-                    continue
-                extra_user_roles = extra_user_roles.lower().split(",")
+                extra_user_roles = self._sanitize_extra_roles(extra_user_roles)
                 for extra_user_role in extra_user_roles:
                     if (
                         extra_user_role not in valid_privileges

tests/unit/test_postgresql_provider.py

@@ -107,12 +107,17 @@ def test_on_database_requested(harness):
         # Assert that the correct calls were made.
         user = f"relation_id_{rel_id}"
         postgresql_mock.create_user.assert_called_once_with(
-            user, "test-password", extra_user_roles=EXTRA_USER_ROLES
+            user,
+            "test-password",
+            extra_user_roles=[role.lower() for role in EXTRA_USER_ROLES.split(",")],
         )
         database_relation = harness.model.get_relation(RELATION_NAME)
         client_relations = [database_relation]
         postgresql_mock.create_database.assert_called_once_with(
-            DATABASE, user, plugins=["pgaudit"], client_relations=client_relations
+            DATABASE,
+            user,
+            plugins=["pgaudit"],
+            client_relations=client_relations,
         )
         postgresql_mock.get_postgresql_version.assert_called_once()