From a89505fd21da337b81172871d8f65d9a4fa22a8b Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Fri, 6 May 2016 14:06:50 -0400 Subject: [PATCH] Remove various special checks around default roles Default roles really should be like regular roles, for the most part. This removes a number of checks that were trying to make default roles extra special by not allowing them to be used as regular roles. We still prevent users from creating roles in the "pg_" namespace or from altering roles which exist in that namespace via ALTER ROLE, as we can't preserve such changes, but otherwise the roles are very much like regular roles. Based on discussion with Robert and Tom. --- src/backend/catalog/aclchk.c | 7 ------- src/backend/commands/alter.c | 3 --- src/backend/commands/foreigncmds.c | 13 ------------- src/backend/commands/policy.c | 5 ----- src/backend/commands/schemacmds.c | 4 ---- src/backend/commands/tablecmds.c | 2 -- src/backend/commands/tablespace.c | 4 ---- src/backend/commands/user.c | 11 ----------- src/backend/commands/variable.c | 7 ------- src/test/regress/expected/rolenames.out | 18 +++++------------- src/test/regress/sql/rolenames.sql | 10 +++++----- 11 files changed, 10 insertions(+), 74 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 7d656d5c6d..d074e85b27 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -423,9 +423,6 @@ ExecuteGrantStmt(GrantStmt *stmt) grantee_uid = ACL_ID_PUBLIC; break; default: - if (!IsBootstrapProcessingMode()) - check_rolespec_name((Node *) grantee, - "Cannot GRANT or REVOKE privileges to or from a reserved role."); grantee_uid = get_rolespec_oid((Node *) grantee, false); break; } @@ -921,8 +918,6 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) grantee_uid = ACL_ID_PUBLIC; break; default: - check_rolespec_name((Node *) grantee, - "Cannot GRANT or REVOKE default privileges to or from a reserved role."); grantee_uid = get_rolespec_oid((Node *) grantee, false); break; } @@ -1013,8 +1008,6 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) { RoleSpec *rolespec = lfirst(rolecell); - check_rolespec_name((Node *) rolespec, - "Cannot alter default privileges for reserved role."); iacls.roleid = get_rolespec_oid((Node *) rolespec, false); /* diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 47a5c50132..4b08cb832e 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -747,9 +747,6 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt) { Oid newowner = get_rolespec_oid(stmt->newowner, false); - check_rolespec_name(stmt->newowner, - "Cannot make reserved roles owners of objects."); - switch (stmt->objectType) { case OBJECT_DATABASE: diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index 88cefb7f95..804bab2e1f 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -1148,10 +1148,6 @@ CreateUserMapping(CreateUserMappingStmt *stmt) else useId = get_rolespec_oid(stmt->user, false); - /* Additional check to protect reserved role names */ - check_rolespec_name(stmt->user, - "Cannot specify reserved role as mapping user."); - /* Check that the server exists. */ srv = GetForeignServerByName(stmt->servername, false); @@ -1252,10 +1248,6 @@ AlterUserMapping(AlterUserMappingStmt *stmt) else useId = get_rolespec_oid(stmt->user, false); - /* Additional check to protect reserved role names */ - check_rolespec_name(stmt->user, - "Cannot alter reserved role mapping user."); - srv = GetForeignServerByName(stmt->servername, false); umId = GetSysCacheOid2(USERMAPPINGUSERSERVER, @@ -1345,11 +1337,6 @@ RemoveUserMapping(DropUserMappingStmt *stmt) else { useId = get_rolespec_oid(stmt->user, stmt->missing_ok); - - /* Additional check to protect reserved role names */ - check_rolespec_name(stmt->user, - "Cannot remove reserved role mapping user."); - if (!OidIsValid(useId)) { /* diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 146b36c2fa..93d15e477a 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -176,13 +176,8 @@ policy_role_list_to_array(List *roles, int *num_roles) return role_oids; } else - { - /* Additional check to protect reserved role names */ - check_rolespec_name((Node *) spec, - "Cannot specify reserved role as policy target"); role_oids[i++] = ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false)); - } } return role_oids; diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index dea3299ced..a60ceb8eba 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -65,10 +65,6 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString) else owner_uid = saved_uid; - /* Additional check to protect reserved role names */ - check_rolespec_name(stmt->authrole, - "Cannot specify reserved role as owner."); - /* fill schema name with the user name if not specified */ if (!schemaName) { diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 45a5144643..86e98148c1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3566,8 +3566,6 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, (List *) cmd->def, lockmode); break; case AT_ChangeOwner: /* ALTER OWNER */ - check_rolespec_name(cmd->newowner, - "Cannot specify reserved role as owner."); ATExecChangeOwner(RelationGetRelid(rel), get_rolespec_oid(cmd->newowner, false), false, lockmode); diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index fe7f25337d..7902d433d5 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -256,10 +256,6 @@ CreateTableSpace(CreateTableSpaceStmt *stmt) else ownerId = GetUserId(); - /* Additional check to protect reserved role names */ - check_rolespec_name(stmt->owner, - "Cannot specify reserved role as owner."); - /* Unix-ify the offered path, and strip any trailing slashes */ location = pstrdup(stmt->location); canonicalize_path(location); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index cc3d564534..f0ac636b9b 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1262,18 +1262,10 @@ GrantRole(GrantRoleStmt *stmt) ListCell *item; if (stmt->grantor) - { - check_rolespec_name(stmt->grantor, - "Cannot specify reserved role as grantor."); grantor = get_rolespec_oid(stmt->grantor, false); - } else grantor = GetUserId(); - foreach(item, stmt->grantee_roles) - check_rolespec_name(lfirst(item), - "Cannot GRANT roles to a reserved role."); - grantee_ids = roleSpecsToIds(stmt->grantee_roles); /* AccessShareLock is enough since we aren't modifying pg_authid */ @@ -1364,9 +1356,6 @@ ReassignOwnedObjects(ReassignOwnedStmt *stmt) errmsg("permission denied to reassign objects"))); } - check_rolespec_name(stmt->newrole, - "Cannot specify reserved role as owner."); - /* Must have privileges on the receiving side too */ newrole = get_rolespec_oid(stmt->newrole, false); diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index 05e59a6e09..f801faacd2 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -794,10 +794,6 @@ check_session_authorization(char **newval, void **extra, GucSource source) return false; } - /* Do not allow setting role to a reserved role. */ - if (strncmp(*newval, "pg_", 3) == 0) - return false; - /* Look up the username */ roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(*newval)); if (!HeapTupleIsValid(roleTup)) @@ -858,9 +854,6 @@ check_role(char **newval, void **extra, GucSource source) roleid = InvalidOid; is_superuser = false; } - /* Do not allow setting role to a reserved role. */ - else if (strncmp(*newval, "pg_", 3) == 0) - return false; else { if (!IsTransactionState()) diff --git a/src/test/regress/expected/rolenames.out b/src/test/regress/expected/rolenames.out index 15a97abe19..a1f039422f 100644 --- a/src/test/regress/expected/rolenames.out +++ b/src/test/regress/expected/rolenames.out @@ -816,19 +816,11 @@ LINE 1: DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9; DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9; -- error NOTICE: role "nonexistent" does not exist, skipping -- GRANT/REVOKE -GRANT testrol0 TO pg_abc; -- error -ERROR: role "pg_abc" is reserved -DETAIL: Cannot GRANT roles to a reserved role. -GRANT pg_abc TO pg_abcdef; -- error -ERROR: role "pg_abcdef" is reserved -DETAIL: Cannot GRANT roles to a reserved role. -SET ROLE pg_testrole; -- error -ERROR: invalid value for parameter "role": "pg_testrole" -SET ROLE pg_signal_backend; --error -ERROR: invalid value for parameter "role": "pg_signal_backend" -CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --error -ERROR: role "pg_signal_backend" is reserved -DETAIL: Cannot specify reserved role as owner. +GRANT testrol0 TO pg_signal_backend; -- success +SET ROLE pg_signal_backend; --success +RESET ROLE; +CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --success +SET ROLE testrol2; UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_'; SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_'; proname | proacl diff --git a/src/test/regress/sql/rolenames.sql b/src/test/regress/sql/rolenames.sql index b58a16359b..6c831b8b9f 100644 --- a/src/test/regress/sql/rolenames.sql +++ b/src/test/regress/sql/rolenames.sql @@ -381,12 +381,12 @@ DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9; --error DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9; -- error -- GRANT/REVOKE -GRANT testrol0 TO pg_abc; -- error -GRANT pg_abc TO pg_abcdef; -- error +GRANT testrol0 TO pg_signal_backend; -- success -SET ROLE pg_testrole; -- error -SET ROLE pg_signal_backend; --error -CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --error +SET ROLE pg_signal_backend; --success +RESET ROLE; +CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --success +SET ROLE testrol2; UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_'; SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_'; -- 2.40.0