Skip to content

Commit a89505f

Browse files
sfrostsfrost
committed
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.
1 parent 6bd356c commit a89505f

File tree

11 files changed

+10
-74
lines changed

11 files changed

+10
-74
lines changed

‎src/backend/catalog/aclchk.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,6 @@ ExecuteGrantStmt(GrantStmt *stmt)
423423
grantee_uid = ACL_ID_PUBLIC;
424424
break;
425425
default:
426-
if (!IsBootstrapProcessingMode())
427-
check_rolespec_name((Node *) grantee,
428-
"Cannot GRANT or REVOKE privileges to or from a reserved role.");
429426
grantee_uid = get_rolespec_oid((Node *) grantee, false);
430427
break;
431428
}
@@ -921,8 +918,6 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt)
921918
grantee_uid = ACL_ID_PUBLIC;
922919
break;
923920
default:
924-
check_rolespec_name((Node *) grantee,
925-
"Cannot GRANT or REVOKE default privileges to or from a reserved role.");
926921
grantee_uid = get_rolespec_oid((Node *) grantee, false);
927922
break;
928923
}
@@ -1013,8 +1008,6 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt)
10131008
{
10141009
RoleSpec *rolespec = lfirst(rolecell);
10151010

1016-
check_rolespec_name((Node *) rolespec,
1017-
"Cannot alter default privileges for reserved role.");
10181011
iacls.roleid = get_rolespec_oid((Node *) rolespec, false);
10191012

10201013
/*

‎src/backend/commands/alter.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -747,9 +747,6 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
747747
{
748748
Oid newowner = get_rolespec_oid(stmt->newowner, false);
749749

750-
check_rolespec_name(stmt->newowner,
751-
"Cannot make reserved roles owners of objects.");
752-
753750
switch (stmt->objectType)
754751
{
755752
case OBJECT_DATABASE:

‎src/backend/commands/foreigncmds.c

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,10 +1148,6 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
11481148
else
11491149
useId = get_rolespec_oid(stmt->user, false);
11501150

1151-
/* Additional check to protect reserved role names */
1152-
check_rolespec_name(stmt->user,
1153-
"Cannot specify reserved role as mapping user.");
1154-
11551151
/* Check that the server exists. */
11561152
srv = GetForeignServerByName(stmt->servername, false);
11571153

@@ -1252,10 +1248,6 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
12521248
else
12531249
useId = get_rolespec_oid(stmt->user, false);
12541250

1255-
/* Additional check to protect reserved role names */
1256-
check_rolespec_name(stmt->user,
1257-
"Cannot alter reserved role mapping user.");
1258-
12591251
srv = GetForeignServerByName(stmt->servername, false);
12601252

12611253
umId = GetSysCacheOid2(USERMAPPINGUSERSERVER,
@@ -1345,11 +1337,6 @@ RemoveUserMapping(DropUserMappingStmt *stmt)
13451337
else
13461338
{
13471339
useId = get_rolespec_oid(stmt->user, stmt->missing_ok);
1348-
1349-
/* Additional check to protect reserved role names */
1350-
check_rolespec_name(stmt->user,
1351-
"Cannot remove reserved role mapping user.");
1352-
13531340
if (!OidIsValid(useId))
13541341
{
13551342
/*

‎src/backend/commands/policy.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,8 @@ policy_role_list_to_array(List *roles, int *num_roles)
176176
return role_oids;
177177
}
178178
else
179-
{
180-
/* Additional check to protect reserved role names */
181-
check_rolespec_name((Node *) spec,
182-
"Cannot specify reserved role as policy target");
183179
role_oids[i++] =
184180
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
185-
}
186181
}
187182

188183
return role_oids;

‎src/backend/commands/schemacmds.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,6 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
6565
else
6666
owner_uid = saved_uid;
6767

68-
/* Additional check to protect reserved role names */
69-
check_rolespec_name(stmt->authrole,
70-
"Cannot specify reserved role as owner.");
71-
7268
/* fill schema name with the user name if not specified */
7369
if (!schemaName)
7470
{

‎src/backend/commands/tablecmds.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3566,8 +3566,6 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
35663566
(List *) cmd->def, lockmode);
35673567
break;
35683568
case AT_ChangeOwner: /* ALTER OWNER */
3569-
check_rolespec_name(cmd->newowner,
3570-
"Cannot specify reserved role as owner.");
35713569
ATExecChangeOwner(RelationGetRelid(rel),
35723570
get_rolespec_oid(cmd->newowner, false),
35733571
false, lockmode);

‎src/backend/commands/tablespace.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,6 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
256256
else
257257
ownerId = GetUserId();
258258

259-
/* Additional check to protect reserved role names */
260-
check_rolespec_name(stmt->owner,
261-
"Cannot specify reserved role as owner.");
262-
263259
/* Unix-ify the offered path, and strip any trailing slashes */
264260
location = pstrdup(stmt->location);
265261
canonicalize_path(location);

‎src/backend/commands/user.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,18 +1262,10 @@ GrantRole(GrantRoleStmt *stmt)
12621262
ListCell *item;
12631263

12641264
if (stmt->grantor)
1265-
{
1266-
check_rolespec_name(stmt->grantor,
1267-
"Cannot specify reserved role as grantor.");
12681265
grantor = get_rolespec_oid(stmt->grantor, false);
1269-
}
12701266
else
12711267
grantor = GetUserId();
12721268

1273-
foreach(item, stmt->grantee_roles)
1274-
check_rolespec_name(lfirst(item),
1275-
"Cannot GRANT roles to a reserved role.");
1276-
12771269
grantee_ids = roleSpecsToIds(stmt->grantee_roles);
12781270

12791271
/* AccessShareLock is enough since we aren't modifying pg_authid */
@@ -1364,9 +1356,6 @@ ReassignOwnedObjects(ReassignOwnedStmt *stmt)
13641356
errmsg("permission denied to reassign objects")));
13651357
}
13661358

1367-
check_rolespec_name(stmt->newrole,
1368-
"Cannot specify reserved role as owner.");
1369-
13701359
/* Must have privileges on the receiving side too */
13711360
newrole = get_rolespec_oid(stmt->newrole, false);
13721361

‎src/backend/commands/variable.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -794,10 +794,6 @@ check_session_authorization(char **newval, void **extra, GucSource source)
794794
return false;
795795
}
796796

797-
/* Do not allow setting role to a reserved role. */
798-
if (strncmp(*newval, "pg_", 3) == 0)
799-
return false;
800-
801797
/* Look up the username */
802798
roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(*newval));
803799
if (!HeapTupleIsValid(roleTup))
@@ -858,9 +854,6 @@ check_role(char **newval, void **extra, GucSource source)
858854
roleid = InvalidOid;
859855
is_superuser = false;
860856
}
861-
/* Do not allow setting role to a reserved role. */
862-
else if (strncmp(*newval, "pg_", 3) == 0)
863-
return false;
864857
else
865858
{
866859
if (!IsTransactionState())

‎src/test/regress/expected/rolenames.out

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -816,19 +816,11 @@ LINE 1: DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9;
816816
DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9; -- error
817817
NOTICE: role "nonexistent" does not exist, skipping
818818
-- GRANT/REVOKE
819-
GRANT testrol0 TO pg_abc; -- error
820-
ERROR: role "pg_abc" is reserved
821-
DETAIL: Cannot GRANT roles to a reserved role.
822-
GRANT pg_abc TO pg_abcdef; -- error
823-
ERROR: role "pg_abcdef" is reserved
824-
DETAIL: Cannot GRANT roles to a reserved role.
825-
SET ROLE pg_testrole; -- error
826-
ERROR: invalid value for parameter "role": "pg_testrole"
827-
SET ROLE pg_signal_backend; --error
828-
ERROR: invalid value for parameter "role": "pg_signal_backend"
829-
CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --error
830-
ERROR: role "pg_signal_backend" is reserved
831-
DETAIL: Cannot specify reserved role as owner.
819+
GRANT testrol0 TO pg_signal_backend; -- success
820+
SET ROLE pg_signal_backend; --success
821+
RESET ROLE;
822+
CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --success
823+
SET ROLE testrol2;
832824
UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_';
833825
SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';
834826
proname | proacl

‎src/test/regress/sql/rolenames.sql

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,12 @@ DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9; --error
381381
DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9; -- error
382382

383383
-- GRANT/REVOKE
384-
GRANT testrol0 TO pg_abc; -- error
385-
GRANT pg_abc TO pg_abcdef; -- error
384+
GRANT testrol0 TO pg_signal_backend; -- success
386385

387-
SET ROLE pg_testrole; -- error
388-
SET ROLE pg_signal_backend; --error
389-
CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --error
386+
SET ROLE pg_signal_backend; --success
387+
RESET ROLE;
388+
CREATE SCHEMA test_schema AUTHORIZATION pg_signal_backend; --success
389+
SET ROLE testrol2;
390390

391391
UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_';
392392
SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';

0 commit comments

Comments
 (0)