[DPE-7499] Remove admin and postgres roles

lib/charms/postgresql_k8s/v1/postgresql.py

@@ -55,9 +55,6 @@
 ROLE_BACKUP = "charmed_backup"
 ROLE_DBA = "charmed_dba"
 
-# Groups to distinguish database permissions
-PERMISSIONS_GROUP_ADMIN = "admin"
-
 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"
 
 REQUIRED_PLUGINS = {
@@ -246,7 +243,7 @@ def create_database(
                     Identifier(database)
                 )
             )
-            for user_to_grant_access in [user, PERMISSIONS_GROUP_ADMIN, *self.system_users]:
+            for user_to_grant_access in [user, *self.system_users]:
                 cursor.execute(
                     SQL("GRANT ALL PRIVILEGES ON DATABASE {} TO {};").format(
                         Identifier(database), Identifier(user_to_grant_access)
@@ -291,20 +288,18 @@ def create_user(
         """
         try:
             # Separate roles and privileges from the provided extra user roles.
-            admin_role = False
             roles = privileges = None
             if extra_user_roles:
-                admin_role = PERMISSIONS_GROUP_ADMIN in extra_user_roles
                 valid_privileges, valid_roles = self.list_valid_privileges_and_roles()
                 roles = [
                     role
                     for role in extra_user_roles
-                    if role in valid_roles and role != PERMISSIONS_GROUP_ADMIN
+                    if role in valid_roles
                 ]
                 privileges = {
                     extra_user_role
                     for extra_user_role in extra_user_roles
-                    if extra_user_role not in roles and extra_user_role != PERMISSIONS_GROUP_ADMIN
+                    if extra_user_role not in roles
                 }
                 invalid_privileges = [
                     privilege for privilege in privileges if privilege not in valid_privileges
@@ -321,8 +316,8 @@ def create_user(
                 if cursor.fetchone() is not None:
                     user_definition = "ALTER ROLE {}"
                 else:
-                    user_definition = "CREATE ROLE {}"
-                user_definition += f"WITH {'NOLOGIN' if user == 'admin' else 'LOGIN'}{' SUPERUSER' if admin else ''} ENCRYPTED PASSWORD '{password}'{'IN ROLE admin CREATEDB' if admin_role else ''}"
+                    user_definition = "CREATE ROLE {} "
+                user_definition += f"WITH LOGIN{' SUPERUSER' if admin else ''} ENCRYPTED PASSWORD '{password}'"
                 if privileges:
                     user_definition += f" {' '.join(privileges)}"
                 cursor.execute(SQL("BEGIN;"))
@@ -804,22 +799,15 @@ def set_up_database(self, temp_location: Optional[str] = None) -> None:
                     cursor.execute(f"CREATE TABLESPACE temp LOCATION '{temp_location}';")
                     cursor.execute("GRANT CREATE ON TABLESPACE temp TO public;")
 
-            cursor.execute("SELECT TRUE FROM pg_roles WHERE rolname='admin';")
-            if cursor.fetchone() is None:
-                # Allow access to the postgres database only to the system users.
-                cursor.execute("REVOKE ALL PRIVILEGES ON DATABASE postgres FROM PUBLIC;")
-                cursor.execute("REVOKE CREATE ON SCHEMA public FROM PUBLIC;")
-                for user in self.system_users:
-                    cursor.execute(
-                        SQL("GRANT ALL PRIVILEGES ON DATABASE postgres TO {};").format(
-                            Identifier(user)
-                        )
+            # Allow access to the postgres database only to the system users.
+            cursor.execute("REVOKE ALL PRIVILEGES ON DATABASE postgres FROM PUBLIC;")
+            cursor.execute("REVOKE CREATE ON SCHEMA public FROM PUBLIC;")
+            for user in self.system_users:
+                cursor.execute(
+                    SQL("GRANT ALL PRIVILEGES ON DATABASE postgres TO {};").format(
+                        Identifier(user)
                     )
-                self.create_user(
-                    PERMISSIONS_GROUP_ADMIN,
-                    extra_user_roles=[ROLE_READ, ROLE_DML],
                 )
-                cursor.execute("GRANT CONNECT ON DATABASE postgres TO admin;")
         except psycopg2.Error as e:
             logger.error(f"Failed to set up databases: {e}")
             raise PostgreSQLDatabasesSetupError() from e
@@ -907,7 +895,7 @@ def build_postgresql_group_map(group_map: Optional[str]) -> List[Tuple]:
             ldap_group = mapping_parts[0]
             psql_group = mapping_parts[1]
 
-            if psql_group in [*ACCESS_GROUPS, PERMISSIONS_GROUP_ADMIN]:
+            if psql_group in ACCESS_GROUPS:
                 logger.warning(f"Tried to assign LDAP users to forbidden group: {psql_group}")
                 continue
 

src/charm.py

@@ -1724,8 +1724,6 @@ def _start_primary(self, event: StartEvent) -> None:  # noqa: C901
             # This event can be run on a replica if the machines are restarted.
             # For that case, check whether the postgres user already exits.
             users = self.postgresql.list_users()
-            if "postgres" not in users:
-                self.postgresql.create_user("postgres", new_password(), admin=True)
             # Create the backup user.
             if BACKUP_USER not in users:
                 self.postgresql.create_user(

tests/integration/new_relations/test_new_relations_1.py

@@ -2,8 +2,6 @@
 # See LICENSE file for licensing details.
 import asyncio
 import logging
-import secrets
-import string
 from pathlib import Path
 
 import psycopg2
@@ -484,97 +482,10 @@ async def test_relation_with_no_database_name(ops_test: OpsTest):
         await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active", raise_on_blocked=True)
 
 
-async def test_admin_role(ops_test: OpsTest):
-    """Test that the admin role gives access to all the databases."""
-    all_app_names = [DATA_INTEGRATOR_APP_NAME]
-    all_app_names.extend(APP_NAMES)
+async def test_invalid_extra_user_roles(ops_test: OpsTest):
     async with ops_test.fast_forward():
         await ops_test.model.deploy(DATA_INTEGRATOR_APP_NAME, base=CHARM_BASE)
         await ops_test.model.wait_for_idle(apps=[DATA_INTEGRATOR_APP_NAME], status="blocked")
-        await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config({
-            "database-name": DATA_INTEGRATOR_APP_NAME.replace("-", "_"),
-            "extra-user-roles": "admin",
-        })
-        await ops_test.model.wait_for_idle(apps=[DATA_INTEGRATOR_APP_NAME], status="blocked")
-        await ops_test.model.add_relation(DATA_INTEGRATOR_APP_NAME, DATABASE_APP_NAME)
-        await ops_test.model.wait_for_idle(apps=all_app_names, status="active")
-
-    # Check that the user can access all the databases.
-    for database in [
-        DATABASE_DEFAULT_NAME,
-        f"{APPLICATION_APP_NAME.replace('-', '_')}_database",
-        "another_application_database",
-    ]:
-        logger.info(f"connecting to the following database: {database}")
-        connection_string = await build_connection_string(
-            ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", database=database
-        )
-        connection = None
-        should_fail = False
-        try:
-            with psycopg2.connect(connection_string) as connection, connection.cursor() as cursor:
-                # Check the version that the application received is the same on the
-                # database server.
-                cursor.execute("SELECT version();")
-                data = cursor.fetchone()[0].split(" ")[1]
-
-                # Get the version of the database and compare with the information that
-                # was retrieved directly from the database.
-                version = await get_application_relation_data(
-                    ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", "version"
-                )
-                assert version == data
-
-                # Write some data (it should fail in the default database name).
-                random_name = (
-                    f"test_{''.join(secrets.choice(string.ascii_lowercase) for _ in range(10))}"
-                )
-                should_fail = database == DATABASE_DEFAULT_NAME
-                cursor.execute(f"CREATE SCHEMA test; CREATE TABLE test.{random_name}(data TEXT);")
-                if should_fail:
-                    assert False, (
-                        f"failed to run a statement in the following database: {database}"
-                    )
-        except psycopg2.errors.InsufficientPrivilege as e:
-            if not should_fail:
-                logger.exception(e)
-                assert False, (
-                    f"failed to connect to or run a statement in the following database: {database}"
-                )
-        finally:
-            if connection is not None:
-                connection.close()
-
-    # Test the creation and deletion of databases.
-    connection_string = await build_connection_string(
-        ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", database=DATABASE_DEFAULT_NAME
-    )
-    connection = psycopg2.connect(connection_string)
-    connection.autocommit = True
-    cursor = connection.cursor()
-    random_name = f"test_{''.join(secrets.choice(string.ascii_lowercase) for _ in range(10))}"
-    cursor.execute(f"CREATE DATABASE {random_name};")
-    cursor.execute(f"DROP DATABASE {random_name};")
-    try:
-        cursor.execute(f"DROP DATABASE {DATABASE_DEFAULT_NAME};")
-        assert False, (
-            f"the admin extra user role was able to drop the `{DATABASE_DEFAULT_NAME}` system database"
-        )
-    except psycopg2.errors.InsufficientPrivilege:
-        # Ignore the error, as the admin extra user role mustn't be able to drop
-        # the "postgres" system database.
-        pass
-    finally:
-        connection.close()
-
-
-async def test_invalid_extra_user_roles(ops_test: OpsTest):
-    async with ops_test.fast_forward():
-        # Remove the relation between the database and the first data integrator.
-        await ops_test.model.applications[DATABASE_APP_NAME].remove_relation(
-            DATABASE_APP_NAME, DATA_INTEGRATOR_APP_NAME
-        )
-        await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active", raise_on_blocked=True)
 
         another_data_integrator_app_name = f"another-{DATA_INTEGRATOR_APP_NAME}"
         data_integrator_apps_names = [DATA_INTEGRATOR_APP_NAME, another_data_integrator_app_name]

tests/integration/new_relations/test_relations_coherence.py

@@ -82,40 +82,6 @@ async def test_relations(ops_test: OpsTest, charm):
             f"{DATABASE_APP_NAME}:database", f"{DATA_INTEGRATOR_APP_NAME}:postgresql"
         )
 
-        # Re-relation with admin role with checking permission
-        await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config({
-            "database-name": DATA_INTEGRATOR_APP_NAME.replace("-", "_"),
-            "extra-user-roles": "admin",
-        })
-        await ops_test.model.wait_for_idle(apps=[DATA_INTEGRATOR_APP_NAME], status="blocked")
-        await ops_test.model.add_relation(DATA_INTEGRATOR_APP_NAME, DATABASE_APP_NAME)
-        await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active")
-
-        connection_string = await build_connection_string(
-            ops_test,
-            DATA_INTEGRATOR_APP_NAME,
-            "postgresql",
-            database=DATA_INTEGRATOR_APP_NAME.replace("-", "_"),
-        )
-        try:
-            connection = psycopg2.connect(connection_string)
-            connection.autocommit = True
-            cursor = connection.cursor()
-            random_name = (
-                f"test_{''.join(secrets.choice(string.ascii_lowercase) for _ in range(10))}"
-            )
-            cursor.execute(f"CREATE DATABASE {random_name};")
-        except psycopg2.errors.InsufficientPrivilege:
-            assert False, (
-                f"failed connect to {random_name} or run a statement in the following database"
-            )
-        finally:
-            connection.close()
-
-        await ops_test.model.applications[DATABASE_APP_NAME].remove_relation(
-            f"{DATABASE_APP_NAME}:database", f"{DATA_INTEGRATOR_APP_NAME}:postgresql"
-        )
-
         # Re-relation again with user role and checking write data
         await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config({
             "database-name": DATA_INTEGRATOR_APP_NAME.replace("-", "_"),

tests/unit/test_charm.py

@@ -669,7 +669,7 @@ def test_on_start(harness):
         # Then test the event of a correct cluster bootstrapping.
         _restart_services_after_reboot.reset_mock()
         harness.charm.on.start.emit()
-        assert _postgresql.create_user.call_count == 4  # Considering the previous failed call.
+        assert _postgresql.create_user.call_count == 3  # Considering the previous failed call.
         _oversee_users.assert_called_once()
         _enable_disable_extensions.assert_called_once()
         _set_primary_status_message.assert_called_once()