Make default ACL be consistent --- ie, starting point for ChangeAcl

is the same as the access permissions granted when a relation's relacl
field is NULL, ie, owner=all rights, world=no rights.

src/backend/catalog/aclchk.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.40 2000/09/06 14:15:15 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.41 2000/10/02 04:49:28 tgl Exp $
  *
  * NOTES
  *	  See acl.h.
@@ -36,28 +36,8 @@
 static int32 aclcheck(char *relname, Acl *acl, AclId id,
 					  AclIdType idtype, AclMode mode);
 
-/*
- * Enable use of user relations in place of real system catalogs.
- */
-/*#define ACLDEBUG*/
-
-#ifdef ACLDEBUG
-/*
- * Fool the code below into thinking that "pgacls" is pg_class.
- * relname and relowner are in the same place, happily.
- */
-#undef	Anum_pg_class_relacl
-#define Anum_pg_class_relacl			3
-#undef	Natts_pg_class
-#define Natts_pg_class					3
-#undef	Name_pg_class
-#define Name_pg_class					"pgacls"
-#undef	Name_pg_group
-#define Name_pg_group					"pggroup"
-#endif
-
 /* warning messages, now more explicit. */
-/* should correspond to the order of the ACLCHK_* result codes above. */
+/* MUST correspond to the order of the ACLCHK_* result codes in acl.h. */
 char	   *aclcheck_error_strings[] = {
 	"No error.",
 	"Permission denied.",
@@ -65,6 +45,7 @@ char	   *aclcheck_error_strings[] = {
 	"Must be table owner."
 };
 
+
 #ifdef ACLDEBUG_TRACE
 static
 dumpacl(Acl *acl)
@@ -84,7 +65,7 @@ dumpacl(Acl *acl)
 #endif
 
 /*
- *
+ * ChangeAcl
  */
 void
 ChangeAcl(char *relname,
@@ -96,12 +77,12 @@ ChangeAcl(char *relname,
 			   *new_acl;
 	Relation	relation;
 	HeapTuple	tuple;
+	Datum		aclDatum;
 	Datum		values[Natts_pg_class];
 	char		nulls[Natts_pg_class];
 	char		replaces[Natts_pg_class];
 	Relation	idescs[Num_pg_class_indices];
 	bool		isNull;
-	bool		free_old_acl = false;
 
 	/*
 	 * Find the pg_class tuple matching 'relname' and extract the ACL. If
@@ -118,29 +99,20 @@ ChangeAcl(char *relname,
 			 relname);
 	}
 
-	old_acl = (Acl *) heap_getattr(tuple,
-								   Anum_pg_class_relacl,
-								   RelationGetDescr(relation),
-								   &isNull);
+	aclDatum = SysCacheGetAttr(RELNAME, tuple, Anum_pg_class_relacl,
+							   &isNull);
 	if (isNull)
 	{
-#ifdef ACLDEBUG_TRACE
-		elog(DEBUG, "ChangeAcl: using default ACL");
-#endif
-		old_acl = acldefault(relname);
-		free_old_acl = true;
-	}
-
-	/* Need to detoast the old ACL for modification */
-	old_acl = DatumGetAclP(PointerGetDatum(old_acl));
+		/* No ACL, so build default ACL for rel */
+		AclId		ownerId;
 
-	if (ACL_NUM(old_acl) < 1)
+		ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
+		old_acl = acldefault(relname, ownerId);
+	}
+	else
 	{
-#ifdef ACLDEBUG_TRACE
-		elog(DEBUG, "ChangeAcl: old ACL has zero length");
-#endif
-		old_acl = acldefault(relname);
-		free_old_acl = true;
+		/* get a detoasted copy of the rel's ACL */
+		old_acl = DatumGetAclPCopy(aclDatum);
 	}
 
 #ifdef ACLDEBUG_TRACE
@@ -173,8 +145,8 @@ ChangeAcl(char *relname,
 	CatalogCloseIndices(Num_pg_class_indices, idescs);
 
 	heap_close(relation, RowExclusiveLock);
-	if (free_old_acl)
-		pfree(old_acl);
+
+	pfree(old_acl);
 	pfree(new_acl);
 }
 
@@ -264,9 +236,15 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
 	unsigned	num,
 				found_group;
 
-	/* if no acl is found, use world default */
+	/*
+	 * If ACL is null, default to "OK" --- this should not happen,
+	 * since caller should have inserted appropriate default
+	 */
 	if (!acl)
-		acl = acldefault(relname);
+	{
+		elog(DEBUG, "aclcheck: null ACL, returning 1");
+		return ACLCHECK_OK;
+	}
 
 	num = ACL_NUM(acl);
 	aidat = ACL_DAT(acl);
@@ -278,9 +256,7 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
 	 */
 	if (num < 1)
 	{
-#if defined(ACLDEBUG_TRACE) || 1
 		elog(DEBUG, "aclcheck: zero-length ACL, returning 1");
-#endif
 		return ACLCHECK_OK;
 	}
 
@@ -357,11 +333,12 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
 int32
 pg_aclcheck(char *relname, Oid userid, AclMode mode)
 {
-	HeapTuple	tuple;
-	Acl		   *acl = (Acl *) NULL;
 	int32		result;
+	HeapTuple	tuple;
 	char       *usename;
-	Relation	relation;
+	Datum		aclDatum;
+	bool		isNull;
+	Acl		   *acl;
 
 	tuple = SearchSysCacheTuple(SHADOWSYSID,
 								ObjectIdGetDatum(userid),
@@ -399,53 +376,31 @@ pg_aclcheck(char *relname, Oid userid, AclMode mode)
 		return ACLCHECK_OK;
 	}
 
-#ifndef ACLDEBUG
-	relation = heap_openr(RelationRelationName, RowExclusiveLock);
+	/*
+	 * Normal case: get the relation's ACL from pg_class
+	 */
 	tuple = SearchSysCacheTuple(RELNAME,
 								PointerGetDatum(relname),
 								0, 0, 0);
 	if (!HeapTupleIsValid(tuple))
-	{
-		elog(ERROR, "pg_aclcheck: class \"%s\" not found",
-			 relname);
-	}
-	if (!heap_attisnull(tuple, Anum_pg_class_relacl))
-	{
-		/* get a detoasted copy of the ACL */
-		acl = DatumGetAclPCopy(heap_getattr(tuple,
-											Anum_pg_class_relacl,
-											RelationGetDescr(relation),
-											(bool *) NULL));
-	}
-	else
-	{
+		elog(ERROR, "pg_aclcheck: class \"%s\" not found", relname);
 
-		/*
-		 * if the acl is null, by default the owner can do whatever he
-		 * wants to with it
-		 */
+	aclDatum = SysCacheGetAttr(RELNAME, tuple, Anum_pg_class_relacl,
+							   &isNull);
+	if (isNull)
+	{
+		/* No ACL, so build default ACL for rel */
 		AclId		ownerId;
 
 		ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
-		acl = aclownerdefault(relname, ownerId);
+		acl = acldefault(relname, ownerId);
 	}
-	heap_close(relation, RowExclusiveLock);
-#else
-	relation = heap_openr(RelationRelationName, RowExclusiveLock);
-	tuple = SearchSysCacheTuple(RELNAME,
-								PointerGetDatum(relname),
-								0, 0, 0);
-	if (HeapTupleIsValid(tuple) &&
-		!heap_attisnull(tuple, Anum_pg_class_relacl))
+	else
 	{
-		/* get a detoasted copy of the ACL */
-		acl = DatumGetAclPCopy(heap_getattr(tuple,
-											Anum_pg_class_relacl,
-											RelationGetDescr(relation),
-											(bool *) NULL));
+		/* get a detoasted copy of the rel's ACL */
+		acl = DatumGetAclPCopy(aclDatum);
 	}
-	heap_close(relation, RowExclusiveLock);
-#endif
+
 	result = aclcheck(relname, acl, userid, (AclIdType) ACL_IDTYPE_UID, mode);
 	if (acl)
 		pfree(acl);

src/backend/utils/adt/acl.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.48 2000/07/31 22:39:09 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.49 2000/10/02 04:49:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -334,12 +334,23 @@ aclitemgt(AclItem *a1, AclItem *a2)
 			(a1->ai_idtype == a2->ai_idtype && a1->ai_id > a2->ai_id));
 }
 
+
+/*
+ * acldefault()  --- create an ACL describing default access permissions
+ *
+ * Change this routine if you want to alter the default access policy for
+ * newly-created tables (or any table with a NULL acl entry in pg_class)
+ */
 Acl *
-aclownerdefault(char *relname, AclId ownerid)
+acldefault(char *relname, AclId ownerid)
 {
 	Acl		   *acl;
 	AclItem    *aip;
 
+#define ACL_WORLD_DEFAULT		(ACL_NO)
+/* #define		ACL_WORLD_DEFAULT		(ACL_RD|ACL_WR|ACL_AP|ACL_RU) */
+#define ACL_OWNER_DEFAULT		(ACL_RD|ACL_WR|ACL_AP|ACL_RU)
+
 	acl = makeacl(2);
 	aip = ACL_DAT(acl);
 	aip[0].ai_idtype = ACL_IDTYPE_WORLD;
@@ -351,19 +362,6 @@ aclownerdefault(char *relname, AclId ownerid)
 	return acl;
 }
 
-Acl *
-acldefault(char *relname)
-{
-	Acl		   *acl;
-	AclItem    *aip;
-
-	acl = makeacl(1);
-	aip = ACL_DAT(acl);
-	aip[0].ai_idtype = ACL_IDTYPE_WORLD;
-	aip[0].ai_id = ACL_ID_WORLD;
-	aip[0].ai_mode = IsSystemRelationName(relname) ? ACL_RD : ACL_WORLD_DEFAULT;
-	return acl;
-}
 
 /*
  * Add or replace an item in an ACL array.

src/include/utils/acl.h

@@ -7,12 +7,12 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: acl.h,v 1.27 2000/09/06 14:15:31 petere Exp $
+ * $Id: acl.h,v 1.28 2000/10/02 04:49:27 tgl Exp $
  *
  * NOTES
  *	  For backward-compatibility purposes we have to allow there
  *	  to be a null ACL in a pg_class tuple.  This will be defined as
- *	  meaning "no protection" (i.e., old catalogs get old semantics).
+ *	  meaning "default protection" (i.e., whatever acldefault() returns).
  *
  *	  The AclItems in an ACL array are currently kept in sorted order.
  *	  Things will break hard if you change that without changing the
@@ -32,7 +32,7 @@
  */
 typedef uint32 AclId;
 
-#define ACL_ID_WORLD	0		/* XXX only idtype should be checked */
+#define ACL_ID_WORLD	0		/* placeholder for id in a WORLD acl item */
 
 /*
  * AclIdType	tag that describes if the AclId is a user, group, etc.
@@ -58,15 +58,6 @@ typedef uint8 AclMode;
 #define ACL_RU			(1<<3)	/* place rules */
 #define N_ACL_MODES		4
 
-#define ACL_MODECHG_ADD			1
-#define ACL_MODECHG_DEL			2
-#define ACL_MODECHG_EQL			3
-
-/* change this line if you want to set the default acl permission  */
-#define ACL_WORLD_DEFAULT		(ACL_NO)
-/* #define		ACL_WORLD_DEFAULT		(ACL_RD|ACL_WR|ACL_AP|ACL_RU) */
-#define ACL_OWNER_DEFAULT		(ACL_RD|ACL_WR|ACL_AP|ACL_RU)
-
 /*
  * AclItem
  */
@@ -143,6 +134,13 @@ typedef ArrayType IdList;
 #define PG_RETURN_IDLIST_P(x)      PG_RETURN_POINTER(x)
 
 
+/*
+ * ACL modification opcodes
+ */
+#define ACL_MODECHG_ADD			1
+#define ACL_MODECHG_DEL			2
+#define ACL_MODECHG_EQL			3
+
 /* mode indicators for I/O */
 #define ACL_MODECHG_STR			"+-="	/* list of valid characters */
 #define ACL_MODECHG_ADD_CHR		'+'
@@ -171,8 +169,8 @@ extern char *aclcheck_error_strings[];
 /*
  * routines used internally (parser, etc.)
  */
-extern Acl *aclownerdefault(char *relname, AclId ownerid);
-extern Acl *acldefault(char *relname);
+extern Acl *acldefault(char *relname, AclId ownerid);
+
 extern Acl *aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg);
 
 extern char *aclmakepriv(char *old_privlist, char new_priv);