dom-constraint-comments-v2.patch

application/octet-stream

Filename: dom-constraint-comments-v2.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #15833: defining a comment on a domain constraint fails with wrong OID

Patch

Format: format-patch
Series: patch v2
Subject: dom-constraint-comments-v2.patch
File+
src/backend/catalog/aclchk.c 34 0
src/backend/catalog/objectaddress.c 8 1
src/include/utils/acl.h 1 0
src/test/regress/input/constraints.source 15 0
src/test/regress/output/constraints.source 14 0
From 69854257d54962ebc2fef1abcab9edd4b9921fd4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Mon, 10 Jun 2019 11:47:35 +0200
Subject: [PATCH] dom-constraint-comments-v2.patch

https://postgr.es/m/0190610062855.GE2199@paquier.xyz
---
 src/backend/catalog/aclchk.c               | 34 ++++++++++++++++++++++++++++++
 src/backend/catalog/objectaddress.c        |  9 +++++++-
 src/include/utils/acl.h                    |  1 +
 src/test/regress/input/constraints.source  | 15 +++++++++++++
 src/test/regress/output/constraints.source | 14 ++++++++++++
 5 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 2797af35c3..54753a6499 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -33,6 +33,7 @@
 #include "catalog/pg_authid.h"
 #include "catalog/pg_cast.h"
 #include "catalog/pg_collation.h"
+#include "catalog/pg_constraint.h"
 #include "catalog/pg_conversion.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_default_acl.h"
@@ -4800,6 +4801,39 @@ pg_type_ownercheck(Oid type_oid, Oid roleid)
 	return has_privs_of_role(roleid, ownerId);
 }
 
+/*
+ * Ownership check for a domain constraint (specified by OID).
+ *
+ * Note that this relies on the ownership of the associated type.
+ */
+bool
+pg_domain_constraint_ownercheck(Oid con_oid, Oid roleid, Oid *contypid)
+{
+	HeapTuple	tuple;
+	Oid			typeId;
+
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
+	tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(con_oid));
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("constraint with OID %u does not exist", con_oid)));
+
+	typeId = ((Form_pg_constraint) GETSTRUCT(tuple))->contypid;
+	*contypid = typeId;
+
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Fallback to type ownership check in this case as this is what domain
+	 * constraints rely on.
+	 */
+	return pg_type_ownercheck(typeId, roleid);
+}
+
 /*
  * Ownership check for an operator (specified by OID).
  */
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 2235e5626f..b932fd462b 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2295,10 +2295,17 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 		case OBJECT_TYPE:
 		case OBJECT_DOMAIN:
 		case OBJECT_ATTRIBUTE:
-		case OBJECT_DOMCONSTRAINT:
 			if (!pg_type_ownercheck(address.objectId, roleid))
 				aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
 			break;
+		case OBJECT_DOMCONSTRAINT:
+			{
+				Oid			contypid;
+
+				if (!pg_domain_constraint_ownercheck(address.objectId, roleid, &contypid))
+					aclcheck_error_type(ACLCHECK_NOT_OWNER, contypid);
+			}
+			break;
 		case OBJECT_AGGREGATE:
 		case OBJECT_FUNCTION:
 		case OBJECT_PROCEDURE:
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index f4c160ee72..d8ff0cb515 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -287,6 +287,7 @@ extern void removeExtObjInitPriv(Oid objoid, Oid classoid);
 /* 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);
+extern bool pg_domain_constraint_ownercheck(Oid type_oid, Oid roleid, Oid *contypid);
 extern bool pg_oper_ownercheck(Oid oper_oid, Oid roleid);
 extern bool pg_proc_ownercheck(Oid proc_oid, Oid roleid);
 extern bool pg_language_ownercheck(Oid lan_oid, Oid roleid);
diff --git a/src/test/regress/input/constraints.source b/src/test/regress/input/constraints.source
index 0abf95577e..0d4c5b882d 100644
--- a/src/test/regress/input/constraints.source
+++ b/src/test/regress/input/constraints.source
@@ -518,6 +518,10 @@ ALTER TABLE deferred_excl ADD EXCLUDE (f1 WITH =);
 DROP TABLE deferred_excl;
 
 -- Comments
+-- Setup a low-level role to enforce non-superuser checks.
+CREATE ROLE regress_constraint_comments;
+SET SESSION AUTHORIZATION regress_constraint_comments;
+
 CREATE TABLE constraint_comments_tbl (a int CONSTRAINT the_constraint CHECK (a > 0));
 CREATE DOMAIN constraint_comments_dom AS int CONSTRAINT the_constraint CHECK (value > 0);
 
@@ -535,5 +539,16 @@ COMMENT ON CONSTRAINT the_constraint ON DOMAIN no_comments_dom IS 'another bad c
 COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS NULL;
 COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS NULL;
 
+-- unathorized user
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_constraint_comments_noaccess;
+SET SESSION AUTHORIZATION regress_constraint_comments_noaccess;
+COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS 'no, the comment';
+COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS 'no, another comment';
+RESET SESSION AUTHORIZATION;
+
 DROP TABLE constraint_comments_tbl;
 DROP DOMAIN constraint_comments_dom;
+
+DROP ROLE regress_constraint_comments;
+DROP ROLE regress_constraint_comments_noaccess;
diff --git a/src/test/regress/output/constraints.source b/src/test/regress/output/constraints.source
index 465d6da0e0..be151b7d60 100644
--- a/src/test/regress/output/constraints.source
+++ b/src/test/regress/output/constraints.source
@@ -704,6 +704,9 @@ ERROR:  could not create exclusion constraint "deferred_excl_f1_excl"
 DETAIL:  Key (f1)=(3) conflicts with key (f1)=(3).
 DROP TABLE deferred_excl;
 -- Comments
+-- Setup a low-level role to enforce non-superuser checks.
+CREATE ROLE regress_constraint_comments;
+SET SESSION AUTHORIZATION regress_constraint_comments;
 CREATE TABLE constraint_comments_tbl (a int CONSTRAINT the_constraint CHECK (a > 0));
 CREATE DOMAIN constraint_comments_dom AS int CONSTRAINT the_constraint CHECK (value > 0);
 COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS 'yes, the comment';
@@ -720,5 +723,16 @@ COMMENT ON CONSTRAINT the_constraint ON DOMAIN no_comments_dom IS 'another bad c
 ERROR:  type "no_comments_dom" does not exist
 COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS NULL;
 COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS NULL;
+-- unathorized user
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_constraint_comments_noaccess;
+SET SESSION AUTHORIZATION regress_constraint_comments_noaccess;
+COMMENT ON CONSTRAINT the_constraint ON constraint_comments_tbl IS 'no, the comment';
+ERROR:  must be owner of relation constraint_comments_tbl
+COMMENT ON CONSTRAINT the_constraint ON DOMAIN constraint_comments_dom IS 'no, another comment';
+ERROR:  must be owner of type constraint_comments_dom
+RESET SESSION AUTHORIZATION;
 DROP TABLE constraint_comments_tbl;
 DROP DOMAIN constraint_comments_dom;
+DROP ROLE regress_constraint_comments;
+DROP ROLE regress_constraint_comments_noaccess;
-- 
2.14.1.145.gb3622a4ee