diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index ce8a4e927d..b8cbe6af86 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2612,6 +2612,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) { @@ -5333,6 +5340,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) { @@ -5352,13 +5369,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 70dbcb0756..562e3d3455 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -172,7 +172,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; @@ -224,7 +223,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)) @@ -670,7 +670,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; @@ -726,7 +725,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 */ @@ -950,8 +949,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)) { @@ -970,7 +967,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) snprintf(namebuf, sizeof(namebuf), "%u", objectId); 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 3c2b1661e0..1cdb7a9663 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2434,8 +2434,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_authid here) +RESET ROLE; +GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; +SET SESSION ROLE regress_reindexuser; +REINDEX TABLE pg_toast.pg_toast_1260; +ERROR: must be owner of table pg_toast_1260 +REINDEX INDEX pg_toast.pg_toast_1260_index; +ERROR: must be owner of index pg_toast_1260_index -- Clean up RESET ROLE; +REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; DROP ROLE regress_reindexuser; DROP SCHEMA schema_to_reindex CASCADE; NOTICE: drop cascades to 6 other objects diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 26640f019d..76598085f7 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1003,8 +1003,15 @@ REINDEX SCHEMA CONCURRENTLY schema_to_reindex; CREATE ROLE regress_reindexuser NOLOGIN; SET SESSION ROLE regress_reindexuser; REINDEX SCHEMA schema_to_reindex; +-- Permission failures with toast tables and indexes (pg_authid here) +RESET ROLE; +GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser; +SET SESSION ROLE regress_reindexuser; +REINDEX TABLE pg_toast.pg_toast_1260; +REINDEX INDEX pg_toast.pg_toast_1260_index; -- Clean up RESET ROLE; +REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser; DROP ROLE regress_reindexuser; DROP SCHEMA schema_to_reindex CASCADE;