From 15b1918e7d3532f0e4ec3455ae6ce45fae09c86f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 15 Jun 2012 22:55:03 +0300 Subject: [PATCH] Improve reporting of permission errors for array types Because permissions are assigned to element types, not array types, complaining about permission denied on an array type would be misleading to users. So adjust the reporting to refer to the element type instead. In order not to duplicate the required logic in two dozen places, refactor the permission denied reporting for types a bit. pointed out by Yeb Havinga during the review of the type privilege feature --- src/backend/access/common/tupdesc.c | 3 +-- src/backend/catalog/aclchk.c | 13 +++++++++++++ src/backend/catalog/objectaddress.c | 3 +-- src/backend/catalog/pg_aggregate.c | 9 +++------ src/backend/commands/functioncmds.c | 12 ++++-------- src/backend/commands/opclasscmds.c | 6 ++---- src/backend/commands/operatorcmds.c | 9 +++------ src/backend/commands/tablecmds.c | 9 +++------ src/backend/commands/typecmds.c | 18 ++++++------------ src/include/utils/acl.h | 2 ++ src/test/regress/expected/privileges.out | 2 +- 11 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index 1f40b7c660..aa1ce800d4 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -573,8 +573,7 @@ BuildDescForRelation(List *schema) aclresult = pg_type_aclcheck(atttypid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(atttypid)); + aclcheck_error_type(aclresult, atttypid); attcollation = GetColumnDefCollation(NULL, entry, atttypid); attdim = list_length(entry->typeName->arrayBounds); diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 45cd0808ce..56c40b14e0 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -3389,6 +3389,19 @@ aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind, } +/* + * Special common handling for types: use element type instead of array type, + * and format nicely + */ +void +aclcheck_error_type(AclResult aclerr, Oid typeOid) +{ + Oid element_type = get_element_type(typeOid); + + aclcheck_error(aclerr, ACL_KIND_TYPE, format_type_be(element_type ? element_type : typeOid)); +} + + /* Check if given user has rolcatupdate privilege according to pg_authid */ static bool has_rolcatupdate(Oid roleid) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 5a06fcbf41..19bde9f476 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -937,8 +937,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, case OBJECT_DOMAIN: case OBJECT_ATTRIBUTE: if (!pg_type_ownercheck(address.objectId, roleid)) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(address.objectId)); + aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId); break; case OBJECT_AGGREGATE: case OBJECT_FUNCTION: diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 0b393cf6ce..403c6f03a6 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -208,19 +208,16 @@ AggregateCreate(const char *aggName, { aclresult = pg_type_aclcheck(aggArgTypes[i], GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(aggArgTypes[i])); + aclcheck_error_type(aclresult, aggArgTypes[i]); } aclresult = pg_type_aclcheck(aggTransType, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(aggTransType)); + aclcheck_error_type(aclresult, aggTransType); aclresult = pg_type_aclcheck(finaltype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(finaltype)); + aclcheck_error_type(aclresult, finaltype); /* diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 13e30f4a55..9ba6dd8fcf 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -154,8 +154,7 @@ compute_return_type(TypeName *returnType, Oid languageOid, aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(rettype)); + aclcheck_error_type(aclresult, rettype); *prorettype_p = rettype; *returnsSet_p = returnType->setof; @@ -247,8 +246,7 @@ examine_parameter_list(List *parameters, Oid languageOid, aclresult = pg_type_aclcheck(toid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(toid)); + aclcheck_error_type(aclresult, toid); if (t->setof) ereport(ERROR, @@ -1510,13 +1508,11 @@ CreateCast(CreateCastStmt *stmt) aclresult = pg_type_aclcheck(sourcetypeid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(sourcetypeid)); + aclcheck_error_type(aclresult, sourcetypeid); aclresult = pg_type_aclcheck(targettypeid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(targettypeid)); + aclcheck_error_type(aclresult, targettypeid); /* Domains are allowed for historical reasons, but we warn */ if (sourcetyptype == TYPTYPE_DOMAIN) diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index 460b1d9ae2..f5642447f1 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -414,8 +414,7 @@ DefineOpClass(CreateOpClassStmt *stmt) /* XXX this is unnecessary given the superuser check above */ /* Check we have ownership of the datatype */ if (!pg_type_ownercheck(typeoid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(typeoid)); + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeoid); #endif /* @@ -565,8 +564,7 @@ DefineOpClass(CreateOpClassStmt *stmt) /* XXX this is unnecessary given the superuser check above */ /* Check we have ownership of the datatype */ if (!pg_type_ownercheck(storageoid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(storageoid)); + aclcheck_error_type(ACLCHECK_NOT_OWNER, storageoid); #endif break; default: diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index afae5e406e..410c708975 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -181,16 +181,14 @@ DefineOperator(List *names, List *parameters) { aclresult = pg_type_aclcheck(typeId1, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(typeId1)); + aclcheck_error_type(aclresult, typeId1); } if (typeName2) { aclresult = pg_type_aclcheck(typeId2, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(typeId2)); + aclcheck_error_type(aclresult, typeId2); } /* @@ -227,8 +225,7 @@ DefineOperator(List *names, List *parameters) rettype = get_func_rettype(functionOid); aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(rettype)); + aclcheck_error_type(aclresult, rettype); /* * Look up restriction estimator if specified diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5c69cfb85a..70753e33e4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -526,8 +526,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId) aclresult = pg_type_aclcheck(ofTypeId, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(ofTypeId)); + aclcheck_error_type(aclresult, ofTypeId); } else ofTypeId = InvalidOid; @@ -4500,8 +4499,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, aclresult = pg_type_aclcheck(typeOid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(typeOid)); + aclcheck_error_type(aclresult, typeOid); collOid = GetColumnDefCollation(NULL, colDef, typeOid); @@ -7248,8 +7246,7 @@ ATPrepAlterColumnType(List **wqueue, aclresult = pg_type_aclcheck(targettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(targettype)); + aclcheck_error_type(aclresult, targettype); /* And the collation */ targetcollid = GetColumnDefCollation(NULL, def, targettype); diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index fdb5bdbc11..30850b2b22 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -758,8 +758,7 @@ DefineDomain(CreateDomainStmt *stmt) aclresult = pg_type_aclcheck(basetypeoid, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, ACL_KIND_TYPE, - format_type_be(basetypeoid)); + aclcheck_error_type(aclresult, basetypeoid); /* * Identify the collation if any @@ -1208,8 +1207,7 @@ checkEnumOwner(HeapTuple tup) /* Permission check: must own type */ if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(HeapTupleGetOid(tup))); + aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup)); } @@ -2809,8 +2807,7 @@ checkDomainOwner(HeapTuple tup) /* Permission check: must own type */ if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(HeapTupleGetOid(tup))); + aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup)); } /* @@ -3116,8 +3113,7 @@ RenameType(RenameStmt *stmt) /* check permissions on type */ if (!pg_type_ownercheck(typeOid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(typeOid)); + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid); /* ALTER DOMAIN used on a non-domain? */ if (stmt->renameType == OBJECT_DOMAIN && typTup->typtype != TYPTYPE_DOMAIN) @@ -3238,8 +3234,7 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype) { /* Otherwise, must be owner of the existing object */ if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(HeapTupleGetOid(tup))); + aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup)); /* Must be able to become new owner */ check_is_member_of_role(GetUserId(), newOwnerId); @@ -3367,8 +3362,7 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid) /* check permissions on type */ if (!pg_type_ownercheck(typeOid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, - format_type_be(typeOid)); + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid); /* don't allow direct alteration of array types */ elemOid = get_element_type(typeOid); diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 6de39b21cf..2d1cccbf66 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -302,6 +302,8 @@ extern void aclcheck_error(AclResult aclerr, AclObjectKind objectkind, extern void aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind, const char *objectname, const char *colname); +extern void aclcheck_error_type(AclResult aclerr, Oid typeOid); + /* ownercheck routines just return true (owner) or false (not) */ extern bool pg_class_ownercheck(Oid class_oid, Oid roleid); extern bool pg_type_ownercheck(Oid type_oid, Oid roleid); diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index dfaf3a7a7e..f99ebde33c 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -547,7 +547,7 @@ ERROR: permission denied for type testdomain1 CREATE TABLE test6a OF testtype1; ERROR: permission denied for type testtype1 CREATE TABLE test10a (a int[], b testtype1[]); -ERROR: permission denied for type testtype1[] +ERROR: permission denied for type testtype1 CREATE TABLE test9a (a int, b int); ALTER TABLE test9a ADD COLUMN c testdomain1; ERROR: permission denied for type testdomain1 -- 2.39.5