New Model For Role Attributes and Fine Grained Permssions
Hi All,
This is a "proof-of-concept" patch for a new model around role attributes
and fine grained permissions meant to alleviate the current over dependence
on superuser.
This is not yet complete and only serves as a proof-of-concept at this
point, but I wanted to share it in the hopes of receiving comments,
suggestions and general feedback.
The general gist of this patch is as follows:
* New system catalog "pg_permission" that relates role id's to permissions.
* New syntax.
- GRANT <permission> TO <role>
- REVOKE <permission> FROM <role>
where, <permission> is one of an enumerated value, such as "CREATE ROLE" or
"CREATE DATABASE".
* Refactor CREATEDB and NOCREATEDB role attribute to "CREATE DATABASE"
permission set by GRANT or REVOKE.
* Refactor CREATEROLE and NOCREATEROLE role attribute to "CREATE ROLE"
permission set by GRANT or REVOKE.
Again, this is meant to serve as a proof-of-concept. It is not
comprehensive and only demonstrates how this might work with a few already
defined permissions.
I have attached the current patch based on master.
Any comments or feedback would be greatly appreciated.
Thanks,
Adam
--
Adam Brightwell - adam.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
Attachments:
superuser_8-18-2014.patchtext/x-patch; charset=US-ASCII; name=superuser_8-18-2014.patchDownload
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
new file mode 100644
index a974bd5..8e4ad25
*** a/src/backend/catalog/Makefile
--- b/src/backend/catalog/Makefile
*************** POSTGRES_BKI_SRCS = $(addprefix $(top_sr
*** 39,45 ****
pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \
pg_ts_parser.h pg_ts_template.h pg_extension.h \
pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \
! pg_foreign_table.h \
pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \
toasting.h indexing.h \
)
--- 39,45 ----
pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \
pg_ts_parser.h pg_ts_template.h pg_extension.h \
pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \
! pg_foreign_table.h pg_permission.h \
pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \
toasting.h indexing.h \
)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
new file mode 100644
index d9745ca..9f4b5df
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
***************
*** 42,53 ****
--- 42,55 ----
#include "catalog/pg_opclass.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_opfamily.h"
+ #include "catalog/pg_permission.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
#include "catalog/pg_ts_config.h"
#include "catalog/pg_ts_dict.h"
#include "commands/dbcommands.h"
+ #include "commands/permission.h"
#include "commands/proclang.h"
#include "commands/tablespace.h"
#include "foreign/foreign.h"
*************** bool
*** 5065,5082 ****
has_createrole_privilege(Oid roleid)
{
bool result = false;
- HeapTuple utup;
/* Superusers bypass all permission checking. */
if (superuser_arg(roleid))
return true;
! utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
! if (HeapTupleIsValid(utup))
! {
! result = ((Form_pg_authid) GETSTRUCT(utup))->rolcreaterole;
! ReleaseSysCache(utup);
! }
return result;
}
--- 5067,5079 ----
has_createrole_privilege(Oid roleid)
{
bool result = false;
/* Superusers bypass all permission checking. */
if (superuser_arg(roleid))
return true;
! result = HasPermission(roleid, PERM_CREATE_ROLE);
!
return result;
}
diff --git a/src/backend/commands/Makefile b/src/backend/commands/Makefile
new file mode 100644
index 22f116b..b98212a
*** a/src/backend/commands/Makefile
--- b/src/backend/commands/Makefile
*************** OBJS = aggregatecmds.o alter.o analyze.o
*** 17,23 ****
dbcommands.o define.o discard.o dropcmds.o \
event_trigger.o explain.o extension.o foreigncmds.o functioncmds.o \
indexcmds.o lockcmds.o matview.o operatorcmds.o opclasscmds.o \
! portalcmds.o prepare.o proclang.o \
schemacmds.o seclabel.o sequence.o tablecmds.o tablespace.o trigger.o \
tsearchcmds.o typecmds.o user.o vacuum.o vacuumlazy.o \
variable.o view.o
--- 17,23 ----
dbcommands.o define.o discard.o dropcmds.o \
event_trigger.o explain.o extension.o foreigncmds.o functioncmds.o \
indexcmds.o lockcmds.o matview.o operatorcmds.o opclasscmds.o \
! permission.o portalcmds.o prepare.o proclang.o \
schemacmds.o seclabel.o sequence.o tablecmds.o tablespace.o trigger.o \
tsearchcmds.o typecmds.o user.o vacuum.o vacuumlazy.o \
variable.o view.o
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
new file mode 100644
index f480be8..ecba8a5
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 36,45 ****
--- 36,47 ----
#include "catalog/pg_authid.h"
#include "catalog/pg_database.h"
#include "catalog/pg_db_role_setting.h"
+ #include "catalog/pg_permission.h"
#include "catalog/pg_tablespace.h"
#include "commands/comment.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
+ #include "commands/permission.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
#include "mb/pg_wchar.h"
*************** static bool
*** 1789,1806 ****
have_createdb_privilege(void)
{
bool result = false;
- HeapTuple utup;
/* Superusers can always do everything */
if (superuser())
return true;
! utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(GetUserId()));
! if (HeapTupleIsValid(utup))
! {
! result = ((Form_pg_authid) GETSTRUCT(utup))->rolcreatedb;
! ReleaseSysCache(utup);
! }
return result;
}
--- 1791,1803 ----
have_createdb_privilege(void)
{
bool result = false;
/* Superusers can always do everything */
if (superuser())
return true;
! result = HasPermission(GetUserId(), PERM_CREATE_DATABASE);
!
return result;
}
diff --git a/src/backend/commands/permission.c b/src/backend/commands/permission.c
new file mode 100644
index ...977b7d7
*** a/src/backend/commands/permission.c
--- b/src/backend/commands/permission.c
***************
*** 0 ****
--- 1,254 ----
+ /*-------------------------------------------------------------------------
+ *
+ * permission.c
+ * Commands for manipulating permissions.
+ *
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California*
+ *
+ * src/backend/commands/permission.c
+ *
+ *-------------------------------------------------------------------------
+ */
+ #include "postgres.h"
+
+ #include "access/genam.h"
+ #include "access/heapam.h"
+ #include "access/htup_details.h"
+ #include "access/sysattr.h"
+ #include "catalog/dependency.h"
+ #include "catalog/indexing.h"
+ #include "catalog/pg_authid.h"
+ #include "commands/permission.h"
+ #include "nodes/pg_list.h"
+ #include "parser/parse_node.h"
+ #include "utils/acl.h"
+ #include "utils/fmgroids.h"
+ #include "utils/rel.h"
+ #include "utils/syscache.h"
+
+ static void AddPermissionToRole(const char *role_name, Oid role_id,
+ List *permissions);
+ static void RemovePermissionFromRole(const char *role_name, Oid role_id,
+ List *permissions);
+
+ void
+ RemovePermissionById(Oid permission_id)
+ {
+ Relation catalog;
+ ScanKeyData skey;
+ SysScanDesc sscan;
+ HeapTuple tuple;
+
+ /* Find permission. */
+ catalog = heap_open(PermissionRelationId, RowExclusiveLock);
+
+ ScanKeyInit(&skey,
+ ObjectIdAttributeNumber,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(permission_id));
+
+ sscan = systable_beginscan(catalog, PermissionRelationId, true,
+ NULL, 1,&skey);
+
+ tuple = systable_getnext(sscan);
+
+ /* If permission exists, then delete it. */
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "could not find tuple for permission %u", permission_id);
+
+ simple_heap_delete(catalog, &tuple->t_self);
+
+ /* Invalidate Cache */
+ // CacheInvalidateRelcache(catalog);
+
+ /* Clean up */
+ heap_close(catalog, RowExclusiveLock);
+ }
+
+ /*
+ * Grant/Revoke permissions to/from roles.
+ */
+ void
+ GrantPermission(GrantPermissionStmt *stmt)
+ {
+ Oid role_id;
+ char *role_name;
+ ListCell *item;
+
+ foreach(item, stmt->roles)
+ {
+ role_name = strVal(lfirst(item));
+ role_id = get_role_oid(role_name, false);
+
+ if (stmt->is_grant)
+ AddPermissionToRole(role_name, role_id, stmt->permissions);
+ else
+ RemovePermissionFromRole(role_name, role_id, stmt->permissions);
+ }
+ }
+
+ /*
+ * HasPermission
+ * check if a user/role has a permission, true if role permission, otherwise
+ * false.
+ *
+ * role_id - the role to check.
+ * permission - the permission to check.
+ */
+ bool
+ HasPermission(Oid role_id, Permission permission)
+ {
+ bool result = false;
+ HeapTuple tuple;
+
+ tuple = SearchSysCache2(PERMROLEIDPERMID, ObjectIdGetDatum(role_id),
+ Int32GetDatum(permission));
+
+ if (HeapTupleIsValid(tuple))
+ {
+ result = true;
+ ReleaseSysCache(tuple);
+ }
+
+ return result;
+ }
+
+ /*
+ * AddPermissionToRole - Add given permissions to the specified role
+ *
+ * role_name: name of role to add permissions to, only used for error messages.
+ * role_id: OID of the role to add permissions to.
+ * permissions: list of permissions to add to the role.
+ *
+ */
+ static void
+ AddPermissionToRole(const char *role_name, Oid role_id, List *permissions)
+ {
+ Relation pg_permission_rel;
+ Oid permission_id;
+ ScanKeyData skeys[2];
+ SysScanDesc sscan;
+ HeapTuple tuple;
+ Datum new_record[Natts_pg_permission];
+ bool new_record_nulls[Natts_pg_permission];
+ ObjectAddress role_dependency;
+ ObjectAddress permission_dependee;
+ ListCell *item;
+
+ pg_permission_rel = heap_open(PermissionRelationId, RowExclusiveLock);
+
+ foreach(item, permissions)
+ {
+ int permission = lfirst_int(item);
+
+ /* Determine if permission is already set. */
+ ScanKeyInit(&skeys[0],
+ Anum_pg_permission_permroleid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(role_id));
+
+ ScanKeyInit(&skeys[1],
+ Anum_pg_permission_permpermission,
+ BTEqualStrategyNumber, F_INT4EQ,
+ Int32GetDatum(permission));
+
+ sscan = systable_beginscan(pg_permission_rel, PermissionRoleIdPermIndexId,
+ true, NULL, 2, skeys);
+
+ tuple = systable_getnext(sscan);
+
+ if (!HeapTupleIsValid(tuple))
+ {
+ memset(new_record, 0, sizeof(new_record));
+ memset(new_record_nulls, 0, sizeof(new_record_nulls));
+
+ new_record[Anum_pg_permission_permroleid - 1]
+ = ObjectIdGetDatum(role_id);
+ new_record[Anum_pg_permission_permpermission - 1]
+ = Int32GetDatum(permission);
+
+ tuple = heap_form_tuple(RelationGetDescr(pg_permission_rel),
+ new_record, new_record_nulls);
+
+ permission_id = simple_heap_insert(pg_permission_rel, tuple);
+
+ CatalogUpdateIndexes(pg_permission_rel, tuple);
+
+ /* Record dependencies on role */
+ role_dependency.classId = AuthIdRelationId;
+ role_dependency.objectId = role_id;
+ role_dependency.objectSubId = 0;
+
+ permission_dependee.classId = PermissionRelationId;
+ permission_dependee.objectId = permission_id;
+ permission_dependee.objectSubId = 0;
+
+ recordDependencyOn(&permission_dependee, &role_dependency, DEPENDENCY_AUTO);
+
+ heap_freetuple(tuple);
+ }
+ else
+ elog(NOTICE, "Permission already set for %s.", role_name);
+
+ systable_endscan(sscan);
+ }
+
+ /* Clean up. */
+ heap_close(pg_permission_rel, RowExclusiveLock);
+ }
+
+ /*
+ * RemovePermissionFromRole - Remove given permissions from the specified role
+ *
+ * role_name: name of role to add permissions to, only used for error messages.
+ * role_id: OID of the role to add permissions to.
+ * permissions: list of permissions to add to the role.
+ *
+ */
+ static void
+ RemovePermissionFromRole(const char *role_name, Oid role_id, List *permissions)
+ {
+ Relation pg_permission_rel;
+ ScanKeyData skeys[2];
+ SysScanDesc sscan;
+ HeapTuple tuple;
+ ListCell *item;
+
+ pg_permission_rel = heap_open(PermissionRelationId, RowExclusiveLock);
+
+ foreach(item, permissions)
+ {
+ int permission = lfirst_int(item);
+
+ /* Determine if permission is already set. */
+ ScanKeyInit(&skeys[0],
+ Anum_pg_permission_permroleid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(role_id));
+
+ ScanKeyInit(&skeys[1],
+ Anum_pg_permission_permpermission,
+ BTEqualStrategyNumber, F_INT4EQ,
+ Int32GetDatum(permission));
+
+ sscan = systable_beginscan(pg_permission_rel, PermissionRoleIdPermIndexId,
+ true, NULL, 2, skeys);
+
+ tuple = systable_getnext(sscan);
+
+ /* If the permission exists, remove it. */
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "Permission is not set for %s.", role_name);
+
+ simple_heap_delete(pg_permission_rel, &tuple->t_self);
+
+ CatalogUpdateIndexes(pg_permission_rel, tuple);
+
+ systable_endscan(sscan);
+ }
+
+ /* Clean up. */
+ heap_close(pg_permission_rel, RowExclusiveLock);
+ }
+
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
new file mode 100644
index 3088578..ed7c17b
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyGrantRoleStmt(const GrantRoleStmt *
*** 2701,2706 ****
--- 2701,2718 ----
return newnode;
}
+ static GrantPermissionStmt *
+ _copyGrantPermissionStmt(const GrantPermissionStmt *from)
+ {
+ GrantPermissionStmt *newnode = makeNode(GrantPermissionStmt);
+
+ COPY_NODE_FIELD(roles);
+ COPY_NODE_FIELD(permissions);
+ COPY_SCALAR_FIELD(is_grant);
+
+ return newnode;
+ }
+
static AlterDefaultPrivilegesStmt *
_copyAlterDefaultPrivilegesStmt(const AlterDefaultPrivilegesStmt *from)
{
*************** copyObject(const void *from)
*** 4291,4296 ****
--- 4303,4311 ----
case T_GrantRoleStmt:
retval = _copyGrantRoleStmt(from);
break;
+ case T_GrantPermissionStmt:
+ retval = _copyGrantPermissionStmt(from);
+ break;
case T_AlterDefaultPrivilegesStmt:
retval = _copyAlterDefaultPrivilegesStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
new file mode 100644
index 1b07db6..e261bbf
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalGrantRoleStmt(const GrantRoleStmt
*** 1045,1050 ****
--- 1045,1060 ----
}
static bool
+ _equalGrantPermissionStmt(const GrantPermissionStmt *a, const GrantPermissionStmt *b)
+ {
+ COMPARE_NODE_FIELD(roles);
+ COMPARE_NODE_FIELD(permissions);
+ COMPARE_SCALAR_FIELD(is_grant);
+
+ return true;
+ }
+
+ static bool
_equalAlterDefaultPrivilegesStmt(const AlterDefaultPrivilegesStmt *a, const AlterDefaultPrivilegesStmt *b)
{
COMPARE_NODE_FIELD(options);
*************** equal(const void *a, const void *b)
*** 2755,2760 ****
--- 2765,2773 ----
case T_GrantRoleStmt:
retval = _equalGrantRoleStmt(a, b);
break;
+ case T_GrantPermissionStmt:
+ retval = _equalGrantPermissionStmt(a, b);
+ break;
case T_AlterDefaultPrivilegesStmt:
retval = _equalAlterDefaultPrivilegesStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index a113809..d3b6d99
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 51,56 ****
--- 51,57 ----
#include "catalog/index.h"
#include "catalog/namespace.h"
+ #include "catalog/pg_permission.h"
#include "catalog/pg_trigger.h"
#include "commands/defrem.h"
#include "commands/trigger.h"
*************** static Node *makeRecursiveViewSelect(cha
*** 322,327 ****
--- 323,330 ----
%type <str> iso_level opt_encoding
%type <node> grantee
%type <list> grantee_list
+ %type <ival> permission
+ %type <list> permission_list
%type <accesspriv> privilege
%type <list> privileges privilege_list
%type <privtarget> privilege_target
*************** GrantRoleStmt:
*** 6072,6077 ****
--- 6075,6088 ----
n->grantor = $6;
$$ = (Node*)n;
}
+ | GRANT permission_list TO role_list
+ {
+ GrantPermissionStmt *n = makeNode(GrantPermissionStmt);
+ n->is_grant = true;
+ n->permissions = $2;
+ n->roles = $4;
+ $$ = (Node*)n;
+ }
;
RevokeRoleStmt:
*************** RevokeRoleStmt:
*** 6095,6100 ****
--- 6106,6119 ----
n->behavior = $9;
$$ = (Node*)n;
}
+ | REVOKE permission_list FROM role_list
+ {
+ GrantPermissionStmt *n = makeNode(GrantPermissionStmt);
+ n->is_grant = false;
+ n->permissions = $2;
+ n->roles = $4;
+ $$ = (Node*)n;
+ }
;
opt_grant_admin_option: WITH ADMIN OPTION { $$ = TRUE; }
*************** opt_granted_by: GRANTED BY RoleId {
*** 6105,6110 ****
--- 6124,6138 ----
| /*EMPTY*/ { $$ = NULL; }
;
+ permission_list: permission { $$ = list_make1_int($1); }
+ | permission_list ',' permission { $$ = lappend_int($1, $3); }
+ ;
+
+ permission: CREATE DATABASE { $$ = PERM_CREATE_DATABASE; }
+ | CREATE ROLE { $$ = PERM_CREATE_ROLE; }
+ | /*EMPTY*/ { $$ = PERM_INVALID; }
+ ;
+
/*****************************************************************************
*
* ALTER DEFAULT PRIVILEGES statement
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index 07e0b98..58cc8ee
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
***************
*** 39,44 ****
--- 39,45 ----
#include "commands/extension.h"
#include "commands/matview.h"
#include "commands/lockcmds.h"
+ #include "commands/permission.h"
#include "commands/portalcmds.h"
#include "commands/prepare.h"
#include "commands/proclang.h"
*************** check_xact_readonly(Node *parsetree)
*** 183,188 ****
--- 184,190 ----
case T_DropRoleStmt:
case T_GrantStmt:
case T_GrantRoleStmt:
+ case T_GrantPermissionStmt:
case T_AlterDefaultPrivilegesStmt:
case T_TruncateStmt:
case T_DropOwnedStmt:
*************** standard_ProcessUtility(Node *parsetree,
*** 561,566 ****
--- 563,572 ----
GrantRole((GrantRoleStmt *) parsetree);
break;
+ case T_GrantPermissionStmt:
+ GrantPermission((GrantPermissionStmt *) parsetree);
+ break;
+
case T_CreatedbStmt:
/* no event triggers for global objects */
PreventTransactionChain(isTopLevel, "CREATE DATABASE");
*************** CreateCommandTag(Node *parsetree)
*** 2002,2007 ****
--- 2008,2021 ----
}
break;
+ case T_GrantPermissionStmt:
+ {
+ GrantPermissionStmt *stmt = (GrantPermissionStmt *) parsetree;
+
+ tag = (stmt->is_grant) ? "GRANT PERMISSION" : "REVOKE PERMISSION";
+ }
+ break;
+
case T_GrantRoleStmt:
{
GrantRoleStmt *stmt = (GrantRoleStmt *) parsetree;
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
new file mode 100644
index 94d951c..2d5c7d4
*** a/src/backend/utils/cache/syscache.c
--- b/src/backend/utils/cache/syscache.c
***************
*** 47,52 ****
--- 47,53 ----
#include "catalog/pg_opclass.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_opfamily.h"
+ #include "catalog/pg_permission.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_range.h"
#include "catalog/pg_rewrite.h"
*************** static const struct cachedesc cacheinfo[
*** 565,570 ****
--- 566,582 ----
},
8
},
+ {PermissionRelationId, /* PERMROLEIDPERMID */
+ PermissionRoleIdPermIndexId,
+ 2,
+ {
+ Anum_pg_permission_permroleid,
+ Anum_pg_permission_permpermission,
+ 0,
+ 0,
+ },
+ 8
+ },
{ProcedureRelationId, /* PROCNAMEARGSNSP */
ProcedureNameArgsNspIndexId,
3,
diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h
new file mode 100644
index 59576fd..60e110d
*** a/src/include/catalog/indexing.h
--- b/src/include/catalog/indexing.h
*************** DECLARE_UNIQUE_INDEX(pg_extension_name_i
*** 299,304 ****
--- 299,310 ----
DECLARE_UNIQUE_INDEX(pg_range_rngtypid_index, 3542, on pg_range using btree(rngtypid oid_ops));
#define RangeTypidIndexId 3542
+ DECLARE_UNIQUE_INDEX(pg_permission_oid_index, 6001, on pg_permission using btree(oid oid_ops));
+ #define PermissionOidIndexId 6001
+
+ DECLARE_UNIQUE_INDEX(pg_permission_roleid_index, 6002, on pg_permission using btree(permroleid oid_ops, permpermission int4_ops));
+ #define PermissionRoleIdPermIndexId 6002
+
/* last step of initialization script: build the indexes declared above */
BUILD_INDICES
diff --git a/src/include/catalog/pg_permission.h b/src/include/catalog/pg_permission.h
new file mode 100644
index ...eee670b
*** a/src/include/catalog/pg_permission.h
--- b/src/include/catalog/pg_permission.h
***************
*** 0 ****
--- 1,51 ----
+ /*
+ * pg_permission.h
+ * definition of the system catalog for role permissions (pg_permission)
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ */
+ #ifndef PG_PERMISSION_H
+ #define PG_PERMISSION_H
+
+ #include "catalog/genbki.h"
+
+ /* ----------------
+ * pg_permission definition. cpp turns this into
+ * typedef struct FormData_pg_permission
+ * ----------------
+ */
+ #define PermissionRelationId 6000
+
+ CATALOG(pg_permission,6000)
+ {
+ Oid permroleid;
+ int32 permpermission;
+ } FormData_pg_permission;
+
+ /* ----------------
+ * Form_pg_permission corresponds to a pointer to a tuple with
+ * the format of pg_permission relation.
+ * ----------------
+ */
+ typedef FormData_pg_permission *Form_pg_permission;
+
+ /*
+ * ----------------
+ * compiler constants for pg_permission
+ * ----------------
+ */
+ #define Natts_pg_permission 2
+ #define Anum_pg_permission_permroleid 1
+ #define Anum_pg_permission_permpermission 2
+
+ typedef enum Permission
+ {
+ PERM_INVALID = -1, /* Invalid Permission */
+ PERM_CREATE_DATABASE = 0, /* CREATE DATABASE */
+ PERM_CREATE_ROLE /* CREATE ROLE */
+ } Permission;
+
+ #endif /* PG_PERMISSION_H */
+
diff --git a/src/include/commands/permission.h b/src/include/commands/permission.h
new file mode 100644
index ...0595e8c
*** a/src/include/commands/permission.h
--- b/src/include/commands/permission.h
***************
*** 0 ****
--- 1,27 ----
+ /*-------------------------------------------------------------------------
+ *
+ * permission.h
+ * prototypes for permission.c.
+ *
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/commands/permission.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+ #ifndef PERMISSION_H
+ #define PERMISSION_H
+
+ #include "catalog/pg_permission.h"
+ #include "nodes/parsenodes.h"
+
+ extern void RemovePermissionById(Oid permission_id);
+
+ extern void GrantPermission(GrantPermissionStmt *stmt);
+
+ extern bool HasPermission(Oid role_id, Permission permission);
+
+ #endif /* PERMISSION_H */
+
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
new file mode 100644
index 067c768..6b56981
*** a/src/include/nodes/nodes.h
--- b/src/include/nodes/nodes.h
*************** typedef enum NodeTag
*** 278,283 ****
--- 278,284 ----
T_AlterDomainStmt,
T_SetOperationStmt,
T_GrantStmt,
+ T_GrantPermissionStmt,
T_GrantRoleStmt,
T_AlterDefaultPrivilegesStmt,
T_ClosePortalStmt,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index 8364bef..2b20388
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct GrantRoleStmt
*** 1476,1481 ****
--- 1476,1493 ----
} GrantRoleStmt;
/* ----------------------
+ * Grant/Revoke Permission Statement
+ * ----------------------
+ */
+ typedef struct GrantPermissionStmt
+ {
+ NodeTag type;
+ List *permissions; /* list of permissions to be granted/revoked */
+ List *roles; /* list of roles to granted/revoked permission */
+ bool is_grant; /* true = GRANT, false = REVOKE */
+ } GrantPermissionStmt;
+
+ /* ----------------------
* Alter Default Privileges Statement
* ----------------------
*/
diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h
new file mode 100644
index f97229f..23e4d8a
*** a/src/include/utils/syscache.h
--- b/src/include/utils/syscache.h
*************** enum SysCacheIdentifier
*** 72,77 ****
--- 72,78 ----
OPEROID,
OPFAMILYAMNAMENSP,
OPFAMILYOID,
+ PERMROLEIDPERMID,
PROCNAMEARGSNSP,
PROCOID,
RANGETYPE,
On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
Hi All,
This is a "proof-of-concept" patch for a new model around role attributes
and fine grained permissions meant to alleviate the current over dependence
on superuser.
Hmm. How does this get us any closer to fine-grained permissions? I
guess we need this, so that you can grant/revoke the permissions, but I
thought the hard part is defining what the fine-grained permissions are,
in a way that you can't escalate yourself to full superuser through any
of them.
The new syntax could be useful even if we never get around to do
anything about current superuser checks, so I'm going to give this a
quick review just on its own merits.
Please add documentation. That's certainly required before this can be
committed, but it'll make reviewing the syntax much easier too.
This is not yet complete and only serves as a proof-of-concept at this
point, but I wanted to share it in the hopes of receiving comments,
suggestions and general feedback.The general gist of this patch is as follows:
* New system catalog "pg_permission" that relates role id's to permissions.
* New syntax.
- GRANT <permission> TO <role>
- REVOKE <permission> FROM <role>
where, <permission> is one of an enumerated value, such as "CREATE ROLE" or
"CREATE DATABASE".
The syntax for GRANT/REVOKE is quite complicated already. Do we want to
overload it even more? Also keep in mind that the SQL standard specifies
GRANT/REVOKE, so we have to be careful to not clash with the SQL
standard syntax, or any conceivable future syntax the SQL committee
might add to it (although we have plenty of PostgreSQL extensions in it
already). For example, this is currently legal:
GRANT CREATE ALL ON TABLESPACE <typename> TO <role>
Is that too close to the new syntax, to cause confusion? Does the new
syntax work for all the more fine-grained permissions you have in mind?
I'm particularly worried of conflicts with this existing syntax:
GRANT role_name [, ...] TO role_name [, ...] [ WITH ADMIN OPTION ]
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki,
* Heikki Linnakangas (hlinnakangas@vmware.com) wrote:
On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
This is a "proof-of-concept" patch for a new model around role attributes
and fine grained permissions meant to alleviate the current over dependence
on superuser.Hmm. How does this get us any closer to fine-grained permissions?
This patch, by itself, does not- but it adds the structure to allow us
to add more permissions without having to add more fields to pg_authid,
and we could build into pg_permission the "WITH ADMIN OPTION" and
"grantor" bits that the normal GRANT syntax already supports- but
without having to chew up additional bits for these granted permissions
which are applied to roles instead of objects in the system.
As for specific examples where this could be used beyond those
presented, consider things like:
CREATE EXTENSION
CREATE TABLESPACE
COPY (server-side)
The question which I've been kicking around is the possible additional
constraints which we may want/need for these- a list of extensions which
the role is allowed to create, a set of directories which the role is
allowed to create tablespaces within, similairly a set of directories
the role is allowed to use server-side COPY with (and perhaps a
distinction between COPY FROM and COPY TO).
I
guess we need this, so that you can grant/revoke the permissions,
but I thought the hard part is defining what the fine-grained
permissions are, in a way that you can't escalate yourself to full
superuser through any of them.
This is definitely a concern- which is why I mention the specific items
above as needing to be constrained in some fashion. CREATE EXTENSION
and CREATE TABLESPACE are, in a way, "naturally" constrained if you
imagine an environment where the user with those permissions doesn't
have direct access to modify the filesystem. COPY, on the other hand,
would allow the user to gain access to pg_authid through indirect means
and therefore gain access to an actual superuser role on the system,
potentially.
(Ok- it might be possible to do that with CREATE TABLESPACE too, but
it'd be a bit more interesting to work through that than being able to
just COPY to a bytea and download the result)
The syntax for GRANT/REVOKE is quite complicated already. Do we want
to overload it even more? Also keep in mind that the SQL standard
specifies GRANT/REVOKE, so we have to be careful to not clash with
the SQL standard syntax, or any conceivable future syntax the SQL
committee might add to it (although we have plenty of PostgreSQL
extensions in it already). For example, this is currently legal:
I agree that there are concerns in this area and I've not got any great
solutions. There are certainly pros and cons to using GRANT. It's
familiar and natural to DBAs, but it runs the risk of conflicting with
future SQL syntax, or even our own GRANT role. We can avoid the latter
by keeping to reserved keywords only, but that may lead to some rather
odd syntax (how would the "GRANT COPY ON (dir1, dir2, dir3)" work?).
Is there a good alternative to consider though..?
Thanks!
Stephen
On Thu, Aug 21, 2014 at 10:49 PM, Stephen Frost <sfrost@snowman.net> wrote:
* Heikki Linnakangas (hlinnakangas@vmware.com) wrote:
On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
This is a "proof-of-concept" patch for a new model around role attributes
and fine grained permissions meant to alleviate the current over dependence
on superuser.Hmm. How does this get us any closer to fine-grained permissions?
This patch, by itself, does not- but it adds the structure to allow us
to add more permissions without having to add more fields to pg_authid,
and we could build into pg_permission the "WITH ADMIN OPTION" and
"grantor" bits that the normal GRANT syntax already supports- but
without having to chew up additional bits for these granted permissions
which are applied to roles instead of objects in the system.As for specific examples where this could be used beyond those
presented, consider things like:CREATE EXTENSION
CREATE TABLESPACE
COPY (server-side)
I'm not opposed to this in theory, but I agree with Heikki that the
syntax you've picked (as well as the internal teminology of the patch)
is not sufficiently unambiguous. "Permission" could refer to a lot
of things, and the GRANT syntax does a lot of things already. Since
the patch appears to aim to supplant what we current do with ALTER
ROLE .. [NO] {CREATEDB | CREATEROLE }, how about something like:
ALTER ROLE role_name { GRANT | REVOKE } CAPABILITY capability_name;
In terms of catalog structure, I doubt that a
row-per-capability-per-user makes sense. Why not just add a new
rolcapability column to pg_authid? A single int64 interpreted as a
bitmask would give us room for 64 capabilities at a fraction of the
storage and lookup cost of a separate catalog. If that's not enough,
the column could be stored as an integer array or vector or something,
but a bigger problem is that Tom's head will likely explode if you
tell him you're going to have more than 64 of these things.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers