From 0b847cba696b0426694e5f48aa95455269624e87 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
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 1f40b7c6604..aa1ce800d40 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 45cd0808ce8..56c40b14e00 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 5a06fcbf41d..19bde9f4761 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 0b393cf6cee..403c6f03a63 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 13e30f4a556..9ba6dd8fcf1 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 460b1d9ae28..f5642447f12 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 afae5e406e8..410c708975f 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 5c69cfb85a2..70753e33e43 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 fdb5bdbc11b..30850b2b22e 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 6de39b21cf4..2d1cccbf66e 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 dfaf3a7a7e0..f99ebde33cc 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
--
GitLab