diff options
| -rw-r--r-- | src/backend/catalog/objectaddress.c | 28 | ||||
| -rw-r--r-- | src/backend/commands/alter.c | 12 | ||||
| -rw-r--r-- | src/test/regress/expected/create_index.out | 9 | ||||
| -rw-r--r-- | src/test/regress/sql/create_index.sql | 7 |
4 files changed, 42 insertions, 14 deletions
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index f5c647a8645..8f6e8f05e03 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2564,6 +2564,13 @@ get_object_attnum_acl(Oid class_id) return prop->attnum_acl; } +/* + * get_object_type + * + * Return the object type associated with a given object. This routine + * is primarily used to determine the object type to mention in ACL check + * error messages, so it's desirable for it to avoid failing. + */ ObjectType get_object_type(Oid class_id, Oid object_id) { @@ -5274,6 +5281,16 @@ strlist_to_textarray(List *list) return arr; } +/* + * get_relkind_objtype + * + * Return the object type for the relkind given by the caller. + * + * If an unexpected relkind is passed, we say OBJECT_TABLE rather than + * failing. That's because this is mostly used for generating error messages + * for failed ACL checks on relations, and we'd rather produce a generic + * message saying "table" than fail entirely. + */ ObjectType get_relkind_objtype(char relkind) { @@ -5293,13 +5310,10 @@ get_relkind_objtype(char relkind) return OBJECT_MATVIEW; case RELKIND_FOREIGN_TABLE: return OBJECT_FOREIGN_TABLE; - - /* - * other relkinds are not supported here because they don't map to - * OBJECT_* values - */ + case RELKIND_TOASTVALUE: + return OBJECT_TABLE; default: - elog(ERROR, "unexpected relkind: %d", relkind); - return 0; + /* Per above, don't raise an error */ + return OBJECT_TABLE; } } diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index eff325cc7d0..4d443c13292 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -171,7 +171,6 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) AttrNumber Anum_name = get_object_attnum_name(classId); AttrNumber Anum_namespace = get_object_attnum_namespace(classId); AttrNumber Anum_owner = get_object_attnum_owner(classId); - ObjectType objtype = get_object_type(classId, objectId); HeapTuple oldtup; HeapTuple newtup; Datum datum; @@ -223,7 +222,8 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) ownerId = DatumGetObjectId(datum); if (!has_privs_of_role(GetUserId(), DatumGetObjectId(ownerId))) - aclcheck_error(ACLCHECK_NOT_OWNER, objtype, old_name); + aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objectId), + old_name); /* User must have CREATE privilege on the namespace */ if (OidIsValid(namespaceId)) @@ -663,7 +663,6 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) AttrNumber Anum_name = get_object_attnum_name(classId); AttrNumber Anum_namespace = get_object_attnum_namespace(classId); AttrNumber Anum_owner = get_object_attnum_owner(classId); - ObjectType objtype = get_object_type(classId, objid); Oid oldNspOid; Datum name, namespace; @@ -719,7 +718,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) ownerId = DatumGetObjectId(owner); if (!has_privs_of_role(GetUserId(), ownerId)) - aclcheck_error(ACLCHECK_NOT_OWNER, objtype, + aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objid), NameStr(*(DatumGetName(name)))); /* User must have CREATE privilege on new namespace */ @@ -942,8 +941,6 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) /* Superusers can bypass permission checks */ if (!superuser()) { - ObjectType objtype = get_object_type(classId, objectId); - /* must be owner */ if (!has_privs_of_role(GetUserId(), old_ownerId)) { @@ -963,7 +960,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) HeapTupleGetOid(oldtup)); objname = namebuf; } - aclcheck_error(ACLCHECK_NOT_OWNER, objtype, objname); + aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objectId), + objname); } /* Must be able to become new owner */ check_is_member_of_role(GetUserId(), new_ownerId); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index be25101db24..90b671f5b91 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -3134,8 +3134,17 @@ CREATE ROLE regress_reindexuser NOLOGIN; SET SESSION ROLE regress_reindexuser; REINDEX SCHEMA schema_to_reindex; ERROR: must be owner of schema schema_to_reindex +-- Permission failures with toast tables and indexes (pg_proc's toast here) +RESET ROLE; +GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; +SET SESSION ROLE regress_reindexuser; +REINDEX TABLE pg_toast.pg_toast_1255; +ERROR: must be owner of table pg_toast_1255 +REINDEX INDEX pg_toast.pg_toast_1255_index; +ERROR: must be owner of index pg_toast_1255_index -- Clean up RESET ROLE; +REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; DROP ROLE regress_reindexuser; \set VERBOSITY terse \\ -- suppress cascade details DROP SCHEMA schema_to_reindex CASCADE; diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f9e7118f0d3..6eee92bf973 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1126,9 +1126,16 @@ END; CREATE ROLE regress_reindexuser NOLOGIN; SET SESSION ROLE regress_reindexuser; REINDEX SCHEMA schema_to_reindex; +-- Permission failures with toast tables and indexes (pg_proc's toast here) +RESET ROLE; +GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; +SET SESSION ROLE regress_reindexuser; +REINDEX TABLE pg_toast.pg_toast_1255; +REINDEX INDEX pg_toast.pg_toast_1255_index; -- Clean up RESET ROLE; +REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; DROP ROLE regress_reindexuser; \set VERBOSITY terse \\ -- suppress cascade details DROP SCHEMA schema_to_reindex CASCADE; |