Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 15b1918

Browse files
peterepetere
committed
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
1 parent d933092 commit 15b1918

File tree

11 files changed

+39
-47
lines changed

11 files changed

+39
-47
lines changed

‎src/backend/access/common/tupdesc.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,7 @@ BuildDescForRelation(List *schema)
573573

574574
aclresult = pg_type_aclcheck(atttypid, GetUserId(), ACL_USAGE);
575575
if (aclresult != ACLCHECK_OK)
576-
aclcheck_error(aclresult, ACL_KIND_TYPE,
577-
format_type_be(atttypid));
576+
aclcheck_error_type(aclresult, atttypid);
578577

579578
attcollation = GetColumnDefCollation(NULL, entry, atttypid);
580579
attdim = list_length(entry->typeName->arrayBounds);

‎src/backend/catalog/aclchk.c

+13
Original file line numberDiff line numberDiff line change
@@ -3389,6 +3389,19 @@ aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind,
33893389
}
33903390

33913391

3392+
/*
3393+
* Special common handling for types: use element type instead of array type,
3394+
* and format nicely
3395+
*/
3396+
void
3397+
aclcheck_error_type(AclResult aclerr, Oid typeOid)
3398+
{
3399+
Oid element_type = get_element_type(typeOid);
3400+
3401+
aclcheck_error(aclerr, ACL_KIND_TYPE, format_type_be(element_type ? element_type : typeOid));
3402+
}
3403+
3404+
33923405
/* Check if given user has rolcatupdate privilege according to pg_authid */
33933406
static bool
33943407
has_rolcatupdate(Oid roleid)

‎src/backend/catalog/objectaddress.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -937,8 +937,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
937937
case OBJECT_DOMAIN:
938938
case OBJECT_ATTRIBUTE:
939939
if (!pg_type_ownercheck(address.objectId, roleid))
940-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
941-
format_type_be(address.objectId));
940+
aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
942941
break;
943942
case OBJECT_AGGREGATE:
944943
case OBJECT_FUNCTION:

‎src/backend/catalog/pg_aggregate.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -208,19 +208,16 @@ AggregateCreate(const char *aggName,
208208
{
209209
aclresult = pg_type_aclcheck(aggArgTypes[i], GetUserId(), ACL_USAGE);
210210
if (aclresult != ACLCHECK_OK)
211-
aclcheck_error(aclresult, ACL_KIND_TYPE,
212-
format_type_be(aggArgTypes[i]));
211+
aclcheck_error_type(aclresult, aggArgTypes[i]);
213212
}
214213

215214
aclresult = pg_type_aclcheck(aggTransType, GetUserId(), ACL_USAGE);
216215
if (aclresult != ACLCHECK_OK)
217-
aclcheck_error(aclresult, ACL_KIND_TYPE,
218-
format_type_be(aggTransType));
216+
aclcheck_error_type(aclresult, aggTransType);
219217

220218
aclresult = pg_type_aclcheck(finaltype, GetUserId(), ACL_USAGE);
221219
if (aclresult != ACLCHECK_OK)
222-
aclcheck_error(aclresult, ACL_KIND_TYPE,
223-
format_type_be(finaltype));
220+
aclcheck_error_type(aclresult, finaltype);
224221

225222

226223
/*

‎src/backend/commands/functioncmds.c

+4-8
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,7 @@ compute_return_type(TypeName *returnType, Oid languageOid,
154154

155155
aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE);
156156
if (aclresult != ACLCHECK_OK)
157-
aclcheck_error(aclresult, ACL_KIND_TYPE,
158-
format_type_be(rettype));
157+
aclcheck_error_type(aclresult, rettype);
159158

160159
*prorettype_p = rettype;
161160
*returnsSet_p = returnType->setof;
@@ -247,8 +246,7 @@ examine_parameter_list(List *parameters, Oid languageOid,
247246

248247
aclresult = pg_type_aclcheck(toid, GetUserId(), ACL_USAGE);
249248
if (aclresult != ACLCHECK_OK)
250-
aclcheck_error(aclresult, ACL_KIND_TYPE,
251-
format_type_be(toid));
249+
aclcheck_error_type(aclresult, toid);
252250

253251
if (t->setof)
254252
ereport(ERROR,
@@ -1510,13 +1508,11 @@ CreateCast(CreateCastStmt *stmt)
15101508

15111509
aclresult = pg_type_aclcheck(sourcetypeid, GetUserId(), ACL_USAGE);
15121510
if (aclresult != ACLCHECK_OK)
1513-
aclcheck_error(aclresult, ACL_KIND_TYPE,
1514-
format_type_be(sourcetypeid));
1511+
aclcheck_error_type(aclresult, sourcetypeid);
15151512

15161513
aclresult = pg_type_aclcheck(targettypeid, GetUserId(), ACL_USAGE);
15171514
if (aclresult != ACLCHECK_OK)
1518-
aclcheck_error(aclresult, ACL_KIND_TYPE,
1519-
format_type_be(targettypeid));
1515+
aclcheck_error_type(aclresult, targettypeid);
15201516

15211517
/* Domains are allowed for historical reasons, but we warn */
15221518
if (sourcetyptype == TYPTYPE_DOMAIN)

‎src/backend/commands/opclasscmds.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
414414
/* XXX this is unnecessary given the superuser check above */
415415
/* Check we have ownership of the datatype */
416416
if (!pg_type_ownercheck(typeoid, GetUserId()))
417-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
418-
format_type_be(typeoid));
417+
aclcheck_error_type(ACLCHECK_NOT_OWNER, typeoid);
419418
#endif
420419

421420
/*
@@ -565,8 +564,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
565564
/* XXX this is unnecessary given the superuser check above */
566565
/* Check we have ownership of the datatype */
567566
if (!pg_type_ownercheck(storageoid, GetUserId()))
568-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
569-
format_type_be(storageoid));
567+
aclcheck_error_type(ACLCHECK_NOT_OWNER, storageoid);
570568
#endif
571569
break;
572570
default:

‎src/backend/commands/operatorcmds.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,14 @@ DefineOperator(List *names, List *parameters)
181181
{
182182
aclresult = pg_type_aclcheck(typeId1, GetUserId(), ACL_USAGE);
183183
if (aclresult != ACLCHECK_OK)
184-
aclcheck_error(aclresult, ACL_KIND_TYPE,
185-
format_type_be(typeId1));
184+
aclcheck_error_type(aclresult, typeId1);
186185
}
187186

188187
if (typeName2)
189188
{
190189
aclresult = pg_type_aclcheck(typeId2, GetUserId(), ACL_USAGE);
191190
if (aclresult != ACLCHECK_OK)
192-
aclcheck_error(aclresult, ACL_KIND_TYPE,
193-
format_type_be(typeId2));
191+
aclcheck_error_type(aclresult, typeId2);
194192
}
195193

196194
/*
@@ -227,8 +225,7 @@ DefineOperator(List *names, List *parameters)
227225
rettype = get_func_rettype(functionOid);
228226
aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE);
229227
if (aclresult != ACLCHECK_OK)
230-
aclcheck_error(aclresult, ACL_KIND_TYPE,
231-
format_type_be(rettype));
228+
aclcheck_error_type(aclresult, rettype);
232229

233230
/*
234231
* Look up restriction estimator if specified

‎src/backend/commands/tablecmds.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
526526

527527
aclresult = pg_type_aclcheck(ofTypeId, GetUserId(), ACL_USAGE);
528528
if (aclresult != ACLCHECK_OK)
529-
aclcheck_error(aclresult, ACL_KIND_TYPE,
530-
format_type_be(ofTypeId));
529+
aclcheck_error_type(aclresult, ofTypeId);
531530
}
532531
else
533532
ofTypeId = InvalidOid;
@@ -4500,8 +4499,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
45004499

45014500
aclresult = pg_type_aclcheck(typeOid, GetUserId(), ACL_USAGE);
45024501
if (aclresult != ACLCHECK_OK)
4503-
aclcheck_error(aclresult, ACL_KIND_TYPE,
4504-
format_type_be(typeOid));
4502+
aclcheck_error_type(aclresult, typeOid);
45054503

45064504
collOid = GetColumnDefCollation(NULL, colDef, typeOid);
45074505

@@ -7248,8 +7246,7 @@ ATPrepAlterColumnType(List **wqueue,
72487246

72497247
aclresult = pg_type_aclcheck(targettype, GetUserId(), ACL_USAGE);
72507248
if (aclresult != ACLCHECK_OK)
7251-
aclcheck_error(aclresult, ACL_KIND_TYPE,
7252-
format_type_be(targettype));
7249+
aclcheck_error_type(aclresult, targettype);
72537250

72547251
/* And the collation */
72557252
targetcollid = GetColumnDefCollation(NULL, def, targettype);

‎src/backend/commands/typecmds.c

+6-12
Original file line numberDiff line numberDiff line change
@@ -758,8 +758,7 @@ DefineDomain(CreateDomainStmt *stmt)
758758

759759
aclresult = pg_type_aclcheck(basetypeoid, GetUserId(), ACL_USAGE);
760760
if (aclresult != ACLCHECK_OK)
761-
aclcheck_error(aclresult, ACL_KIND_TYPE,
762-
format_type_be(basetypeoid));
761+
aclcheck_error_type(aclresult, basetypeoid);
763762

764763
/*
765764
* Identify the collation if any
@@ -1208,8 +1207,7 @@ checkEnumOwner(HeapTuple tup)
12081207

12091208
/* Permission check: must own type */
12101209
if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
1211-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
1212-
format_type_be(HeapTupleGetOid(tup)));
1210+
aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
12131211
}
12141212

12151213

@@ -2809,8 +2807,7 @@ checkDomainOwner(HeapTuple tup)
28092807

28102808
/* Permission check: must own type */
28112809
if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
2812-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
2813-
format_type_be(HeapTupleGetOid(tup)));
2810+
aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
28142811
}
28152812

28162813
/*
@@ -3116,8 +3113,7 @@ RenameType(RenameStmt *stmt)
31163113

31173114
/* check permissions on type */
31183115
if (!pg_type_ownercheck(typeOid, GetUserId()))
3119-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
3120-
format_type_be(typeOid));
3116+
aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
31213117

31223118
/* ALTER DOMAIN used on a non-domain? */
31233119
if (stmt->renameType == OBJECT_DOMAIN && typTup->typtype != TYPTYPE_DOMAIN)
@@ -3238,8 +3234,7 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
32383234
{
32393235
/* Otherwise, must be owner of the existing object */
32403236
if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
3241-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
3242-
format_type_be(HeapTupleGetOid(tup)));
3237+
aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
32433238

32443239
/* Must be able to become new owner */
32453240
check_is_member_of_role(GetUserId(), newOwnerId);
@@ -3367,8 +3362,7 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid)
33673362

33683363
/* check permissions on type */
33693364
if (!pg_type_ownercheck(typeOid, GetUserId()))
3370-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
3371-
format_type_be(typeOid));
3365+
aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
33723366

33733367
/* don't allow direct alteration of array types */
33743368
elemOid = get_element_type(typeOid);

‎src/include/utils/acl.h

+2
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ extern void aclcheck_error(AclResult aclerr, AclObjectKind objectkind,
302302
extern void aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind,
303303
const char *objectname, const char *colname);
304304

305+
extern void aclcheck_error_type(AclResult aclerr, Oid typeOid);
306+
305307
/* ownercheck routines just return true (owner) or false (not) */
306308
extern bool pg_class_ownercheck(Oid class_oid, Oid roleid);
307309
extern bool pg_type_ownercheck(Oid type_oid, Oid roleid);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ ERROR: permission denied for type testdomain1
547547
CREATE TABLE test6a OF testtype1;
548548
ERROR: permission denied for type testtype1
549549
CREATE TABLE test10a (a int[], b testtype1[]);
550-
ERROR: permission denied for type testtype1[]
550+
ERROR: permission denied for type testtype1
551551
CREATE TABLE test9a (a int, b int);
552552
ALTER TABLE test9a ADD COLUMN c testdomain1;
553553
ERROR: permission denied for type testdomain1

0 commit comments

Comments
 (0)