From 4df3987054a7cef88322713b9c4a0e3b1a706131 Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Mon, 28 Oct 2024 18:21:45 -0500 Subject: [PATCH] Get role name when not a C string We will only have a C string if the specified role is a string. Otherwise, we need to resolve references to public, current_role, current_user, and session_user. Fixes: https://github.com/neondatabase/cloud/issues/19323 Signed-off-by: Tristan Partin --- pgxn/neon/control_plane_connector.c | 12 +++++++- test_runner/regress/test_ddl_forwarding.py | 32 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pgxn/neon/control_plane_connector.c b/pgxn/neon/control_plane_connector.c index 4713103909..b47b22cd20 100644 --- a/pgxn/neon/control_plane_connector.c +++ b/pgxn/neon/control_plane_connector.c @@ -18,6 +18,7 @@ * *------------------------------------------------------------------------- */ + #include "postgres.h" #include @@ -508,6 +509,8 @@ NeonXactCallback(XactEvent event, void *arg) static bool RoleIsNeonSuperuser(const char *role_name) { + Assert(role_name); + return strcmp(role_name, "neon_superuser") == 0; } @@ -670,7 +673,7 @@ HandleCreateRole(CreateRoleStmt *stmt) static void HandleAlterRole(AlterRoleStmt *stmt) { - const char *role_name = stmt->role->rolename; + char *role_name; DefElem *dpass; ListCell *option; bool found = false; @@ -678,6 +681,7 @@ HandleAlterRole(AlterRoleStmt *stmt) InitRoleTableIfNeeded(); + role_name = get_rolespec_name(stmt->role); if (RoleIsNeonSuperuser(role_name) && !superuser()) elog(ERROR, "can't ALTER neon_superuser"); @@ -689,9 +693,13 @@ HandleAlterRole(AlterRoleStmt *stmt) if (strcmp(defel->defname, "password") == 0) dpass = defel; } + /* We only care about updates to the password */ if (!dpass) + { + pfree(role_name); return; + } entry = hash_search(CurrentDdlTable->role_table, role_name, @@ -704,6 +712,8 @@ HandleAlterRole(AlterRoleStmt *stmt) else entry->password = NULL; entry->type = Op_Set; + + pfree(role_name); } static void diff --git a/test_runner/regress/test_ddl_forwarding.py b/test_runner/regress/test_ddl_forwarding.py index 96657b3ce4..e517e83e6f 100644 --- a/test_runner/regress/test_ddl_forwarding.py +++ b/test_runner/regress/test_ddl_forwarding.py @@ -7,6 +7,7 @@ import psycopg2 import pytest from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnv, VanillaPostgres +from psycopg2.errors import UndefinedObject from pytest_httpserver import HTTPServer from werkzeug.wrappers.request import Request from werkzeug.wrappers.response import Response @@ -335,3 +336,34 @@ def test_ddl_forwarding_invalid_db(neon_simple_env: NeonEnv): if not result: raise AssertionError("Could not count databases") assert result[0] == 0, "Database 'failure' still exists after restart" + + +def test_ddl_forwarding_role_specs(neon_simple_env: NeonEnv): + """ + Postgres has a concept of role specs: + + ROLESPEC_CSTRING: ALTER ROLE xyz + ROLESPEC_CURRENT_USER: ALTER ROLE current_user + ROLESPEC_CURRENT_ROLE: ALTER ROLE current_role + ROLESPEC_SESSION_USER: ALTER ROLE session_user + ROLESPEC_PUBLIC: ALTER ROLE public + + The extension is required to serialize these special role spec into + usernames for the purpose of DDL forwarding. + """ + env = neon_simple_env + + endpoint = env.endpoints.create_start("main") + + with endpoint.cursor() as cur: + # ROLESPEC_CSTRING + cur.execute("ALTER ROLE cloud_admin WITH PASSWORD 'york'") + # ROLESPEC_CURRENT_USER + cur.execute("ALTER ROLE current_user WITH PASSWORD 'pork'") + # ROLESPEC_CURRENT_ROLE + cur.execute("ALTER ROLE current_role WITH PASSWORD 'cork'") + # ROLESPEC_SESSION_USER + cur.execute("ALTER ROLE session_user WITH PASSWORD 'bork'") + # ROLESPEC_PUBLIC + with pytest.raises(UndefinedObject): + cur.execute("ALTER ROLE public WITH PASSWORD 'dork'")