Hi all, I noticed that we have no option to set default privileges for newly created schemas, other than calling GRANT explicitly. At work I use ALTER DEFAULT PRIVILEGE (ADP) command extensively, as the developers are permitted to manage DDL on the databases, and all work fine except for when a new schema is created. So,I'd like to propose this very simple patch (attached) that adds the capability of using SCHEMAS, adding the following syntax to ADP:
ALTER DEFAULT PRIVILEGES [ FOR { ROLE | USER } target_role [, ...] ] abbreviated_grant_or_revoke where abbreviated_grant_or_revoke is one of: GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] } ON SCHEMAS TO { [ GROUP ] role_name | PUBLIC } [, ...] [ WITH GRANT OPTION ] REVOKE [ GRANT OPTION FOR ] { USAGE | CREATE | ALL [ PRIVILEGES ] } ON SCHEMAS FROM { [ GROUP ] role_name | PUBLIC } [, ...] [ CASCADE | RESTRICT ] The patch itself is really straight forward (I'm new to sending patches, so I've chosen a simple one), and there is only one thing that concerns me (as in, if I did it right/good). The difference in syntax for SCHEMAS and the other objects is that IN SCHEMA option makes no sense here (as we don't have nested schemas), and to solve that I simple added the error "cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS". Does that look good to you? Also, should I add translations for that error message in other languages (I can do that without help of tools for pt_BR) or is that a latter process in the releasing? Other than that, I added a few regression tests (similar to others used for ADP), and patched the documentation (my English is not that good, so I'm open to suggestions). Anything else I forgot? While at this, I'd like to ask if you are interested in have all the other types we have in GRANT/REVOKE for ADP (I myself see few use for that at work, but the symmetry on those commands seems like a good idea). If you agree, I can take some time to do the others (looks very simple to do). I just wonder if that should be done as one patch for each, or just a single patch for all of them (perhaps send the sequence of patches in order, as certainly one will conflict with the other if done apart). Best regards, -- Matheus de Oliveira
diff --git a/doc/src/sgml/ref/alter_default_privileges.sgml b/doc/src/sgml/ref/alter_default_privileges.sgml index 04064d3..7745792 100644 *** a/doc/src/sgml/ref/alter_default_privileges.sgml --- b/doc/src/sgml/ref/alter_default_privileges.sgml *************** *** 46,51 **** GRANT { USAGE | ALL [ PRIVILEGES ] } --- 46,55 ---- ON TYPES TO { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ] + GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] } + ON SCHEMAS + TO { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ] + REVOKE [ GRANT OPTION FOR ] { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER } [, ...] | ALL [ PRIVILEGES ] } *************** *** 71,76 **** REVOKE [ GRANT OPTION FOR ] --- 75,86 ---- ON TYPES FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...] [ CASCADE | RESTRICT ] + + REVOKE [ GRANT OPTION FOR ] + { USAGE | CREATE | ALL [ PRIVILEGES ] } + ON SCHEMAS + FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...] + [ CASCADE | RESTRICT ] </synopsis> </refsynopsisdiv> *************** *** 125,130 **** REVOKE [ GRANT OPTION FOR ] --- 135,142 ---- are altered for objects later created in that schema. If <literal>IN SCHEMA</> is omitted, the global default privileges are altered. + <literal>IN SCHEMA</> is not allowed when using <literal>ON SCHEMAS</> + as schemas can't be nested. </para> </listitem> </varlistentry> diff --git a/src/backend/catalog/aclchk.c b/src/backeindex c0df671..e1256f1 100644 *** a/src/backend/catalog/aclchk.c --- b/src/backend/catalog/aclchk.c *************** *** 948,953 **** ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s --- 948,957 ---- all_privileges = ACL_ALL_RIGHTS_TYPE; errormsg = gettext_noop("invalid privilege type %s for type"); break; + case ACL_OBJECT_NAMESPACE: + all_privileges = ACL_ALL_RIGHTS_NAMESPACE; + errormsg = gettext_noop("invalid privilege type %s for schema"); + break; default: elog(ERROR, "unrecognized GrantStmt.objtype: %d", (int) action->objtype); *************** *** 1135,1140 **** SetDefaultACL(InternalDefaultACL *iacls) --- 1139,1154 ---- this_privileges = ACL_ALL_RIGHTS_TYPE; break; + case ACL_OBJECT_NAMESPACE: + if (OidIsValid(iacls->nspid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_GRANT_OPERATION), + errmsg("cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS"))); + objtype = DEFACLOBJ_NAMESPACE; + if (iacls->all_privs && this_privileges == ACL_NO_RIGHTS) + this_privileges = ACL_ALL_RIGHTS_NAMESPACE; + break; + default: elog(ERROR, "unrecognized objtype: %d", (int) iacls->objtype); *************** *** 1361,1366 **** RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid) --- 1375,1383 ---- case DEFACLOBJ_TYPE: iacls.objtype = ACL_OBJECT_TYPE; break; + case DEFACLOBJ_NAMESPACE: + iacls.objtype = ACL_OBJECT_NAMESPACE; + break; default: /* Shouldn't get here */ elog(ERROR, "unexpected default ACL type: %d", *************** *** 5189,5194 **** get_user_default_acl(GrantObjectType objtype, Oid ownerId, Oid nsp_oid) --- 5206,5215 ---- defaclobjtype = DEFACLOBJ_TYPE; break; + case ACL_OBJECT_NAMESPACE: + defaclobjtype = DEFACLOBJ_NAMESPACE; + break; + default: return NULL; } diff --git a/src/backend/catalog/obindex d531d17..23d6684 100644 *** a/src/backend/catalog/objectaddress.c --- b/src/backend/catalog/objectaddress.c *************** *** 1788,1798 **** get_object_address_defacl(List *objname, List *objargs, bool missing_ok) case DEFACLOBJ_TYPE: objtype_str = "types"; break; default: ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("unrecognized default ACL object type %c", objtype), ! errhint("Valid object types are \"r\", \"S\", \"f\", and \"T\"."))); } /* --- 1788,1801 ---- case DEFACLOBJ_TYPE: objtype_str = "types"; break; + case DEFACLOBJ_NAMESPACE: + objtype_str = "schemas"; + break; default: ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("unrecognized default ACL object type %c", objtype), ! errhint("Valid object types are \"r\", \"S\", \"f\", \"T\" and \"s\"."))); } /* *************** *** 3093,3098 **** getObjectDescription(const ObjectAddress *object) --- 3096,3106 ---- _("default privileges on new types belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; + case DEFACLOBJ_NAMESPACE: + appendStringInfo(&buffer, + _("default privileges on new schemas belonging to role %s"), + GetUserNameFromId(defacl->defaclrole, false)); + break; default: /* shouldn't get here */ appendStringInfo(&buffer, *************** *** 4547,4552 **** getObjectIdentityParts(const ObjectAddress *object, --- 4555,4564 ---- appendStringInfoString(&buffer, " on types"); break; + case DEFACLOBJ_NAMESPACE: + appendStringInfoString(&buffer, + " on schemas"); + break; } if (objname) diff --git a/src/backend/catalog/pg_namespindex e5eed79..34e957f 100644 *** a/src/backend/catalog/pg_namespace.c --- b/src/backend/catalog/pg_namespace.c *************** *** 31,40 **** * Create a namespace (schema) with the given name and owner OID. * * If isTemp is true, this schema is a per-backend schema for holding ! * temporary tables. Currently, the only effect of that is to prevent it ! * from being linked as a member of any active extension. (If someone ! * does CREATE TEMP TABLE in an extension script, we don't want the temp ! * schema to become part of the extension.) * --------------- */ Oid --- 31,41 ---- * Create a namespace (schema) with the given name and owner OID. * * If isTemp is true, this schema is a per-backend schema for holding ! * temporary tables. Currently, it is used to prevent it from being ! * linked as a member of any active extension. (If someone * does CREATE ! * TEMP TABLE in an extension script, we don't want the temp schema to ! * become part of the extension). And to avoid checking for default ACL ! * for temp namespace (as it is not necessary). * --------------- */ Oid *************** *** 49,54 **** NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp) --- 50,56 ---- TupleDesc tupDesc; ObjectAddress myself; int i; + Acl *nspacl; /* sanity checks */ if (!nspName) *************** *** 60,65 **** NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp) --- 62,73 ---- (errcode(ERRCODE_DUPLICATE_SCHEMA), errmsg("schema \"%s\" already exists", nspName))); + if (!isTemp) + nspacl = get_user_default_acl(ACL_OBJECT_NAMESPACE, ownerId, + InvalidOid); + else + nspacl = NULL; + /* initialize nulls and values */ for (i = 0; i < Natts_pg_namespace; i++) { *************** *** 69,75 **** NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp) namestrcpy(&nname, nspName); values[Anum_pg_namespace_nspname - 1] = NameGetDatum(&nname); values[Anum_pg_namespace_nspowner - 1] = ObjectIdGetDatum(ownerId); ! nulls[Anum_pg_namespace_nspacl - 1] = true; nspdesc = heap_open(NamespaceRelationId, RowExclusiveLock); tupDesc = nspdesc->rd_att; --- 77,86 ---- namestrcpy(&nname, nspName); values[Anum_pg_namespace_nspname - 1] = NameGetDatum(&nname); values[Anum_pg_namespace_nspowner - 1] = ObjectIdGetDatum(ownerId); ! if (nspacl != NULL) ! values[Anum_pg_namespace_nspacl - 1] = PointerGetDatum(nspacl); ! else ! nulls[Anum_pg_namespace_nspacl - 1] = true; nspdesc = heap_open(NamespaceRelationId, RowExclusiveLock); tupDesc = nspdesc->rd_att; diff --git a/src/backend/parser/gram.y b/index 0ec1cd3..b845733 100644 *** a/src/backend/parser/gram.y --- b/src/backend/parser/gram.y *************** *** 632,638 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP ROW ROWS RULE ! SAVEPOINT SCHEMA SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P START STATEMENT STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING --- 632,638 ---- RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP ROW ROWS RULE ! SAVEPOINT SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P START STATEMENT STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING *************** *** 6689,6694 **** defacl_privilege_target: --- 6689,6695 ---- | FUNCTIONS { $$ = ACL_OBJECT_FUNCTION; } | SEQUENCES { $$ = ACL_OBJECT_SEQUENCE; } | TYPES_P { $$ = ACL_OBJECT_TYPE; } + | SCHEMAS { $$ = ACL_OBJECT_NAMESPACE; } ; *************** *** 13923,13928 **** unreserved_keyword: --- 13924,13930 ---- | RULE | SAVEPOINT | SCHEMA + | SCHEMAS | SCROLL | SEARCH | SECOND_P diff --git a/src/bin/pg_dump/dumindex 0d51668..ff486ff 100644 *** a/src/bin/pg_dump/dumputils.c --- b/src/bin/pg_dump/dumputils.c *************** *** 511,517 **** do { \ CONVERT_PRIV('X', "EXECUTE"); else if (strcmp(type, "LANGUAGE") == 0) CONVERT_PRIV('U', "USAGE"); ! else if (strcmp(type, "SCHEMA") == 0) { CONVERT_PRIV('C', "CREATE"); CONVERT_PRIV('U', "USAGE"); --- 511,519 ---- CONVERT_PRIV('X', "EXECUTE"); else if (strcmp(type, "LANGUAGE") == 0) CONVERT_PRIV('U', "USAGE"); ! else if (strcmp(type, "SCHEMA") == 0 || ! strcmp(type, "SCHEMAS") == 0 ! ) { CONVERT_PRIV('C', "CREATE"); CONVERT_PRIV('U', "USAGE"); diff --git a/src/bin/pg_dump/pg_duindex 9f59f53..8f5d9c5 100644 *** a/src/bin/pg_dump/pg_dump.c --- b/src/bin/pg_dump/pg_dump.c *************** *** 13373,13378 **** dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo) --- 13373,13381 ---- case DEFACLOBJ_TYPE: type = "TYPES"; break; + case DEFACLOBJ_NAMESPACE: + type = "SCHEMAS"; + break; default: /* shouldn't get here */ exit_horribly(NULL, diff --git a/src/bin/psql/descriindex 1632104..15dd5f9 100644 *** a/src/bin/psql/describe.c --- b/src/bin/psql/describe.c *************** *** 984,990 **** listDefaultACLs(const char *pattern) printfPQExpBuffer(&buf, "SELECT pg_catalog.pg_get_userbyid(d.defaclrole) AS \"%s\",\n" " n.nspname AS \"%s\",\n" ! " CASE d.defaclobjtype WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' END AS \"%s\",\n" " ", gettext_noop("Owner"), gettext_noop("Schema"), --- 984,990 ---- printfPQExpBuffer(&buf, "SELECT pg_catalog.pg_get_userbyid(d.defaclrole) AS \"%s\",\n" " n.nspname AS \"%s\",\n" ! " CASE d.defaclobjtype WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' WHEN '%c' THEN '%s' END AS \"%s\",\n" " ", gettext_noop("Owner"), gettext_noop("Schema"), *************** *** 996,1001 **** listDefaultACLs(const char *pattern) --- 996,1003 ---- gettext_noop("function"), DEFACLOBJ_TYPE, gettext_noop("type"), + DEFACLOBJ_NAMESPACE, + gettext_noop("schema"), gettext_noop("Type")); printACLColumn(&buf, "d.defaclacl"); diff --git a/src/include/catalindex 06aaaba..7b7d7b8 100644 *** a/src/include/catalog/pg_default_acl.h --- b/src/include/catalog/pg_default_acl.h *************** *** 70,74 **** typedef FormData_pg_default_acl *Form_pg_default_acl; --- 70,75 ---- #define DEFACLOBJ_SEQUENCE 'S' /* sequence */ #define DEFACLOBJ_FUNCTION 'f' /* function */ #define DEFACLOBJ_TYPE 'T' /* type */ + #define DEFACLOBJ_NAMESPACE 'n' /* namespace */ #endif /* PG_DEFAULT_ACL_H */ diff --git a/src/include/parser/kwlist.h b/index 77d873b..5dc0244 100644 *** a/src/include/parser/kwlist.h --- b/src/include/parser/kwlist.h *************** *** 339,344 **** PG_KEYWORD("rows", ROWS, UNRESERVED_KEYWORD) --- 339,345 ---- PG_KEYWORD("rule", RULE, UNRESERVED_KEYWORD) PG_KEYWORD("savepoint", SAVEPOINT, UNRESERVED_KEYWORD) PG_KEYWORD("schema", SCHEMA, UNRESERVED_KEYWORD) + PG_KEYWORD("schemas", SCHEMAS, UNRESERVED_KEYWORD) PG_KEYWORD("scroll", SCROLL, UNRESERVED_KEYWORD) PG_KEYWORD("search", SEARCH, UNRESERVED_KEYWORD) PG_KEYWORD("second", SECOND_P, UNRESERVED_KEYWORD) diff --git a/src/test/regress/expeindex f66b443..449b0e3 100644 *** a/src/test/regress/expected/privileges.out --- b/src/test/regress/expected/privileges.out *************** *** 1337,1342 **** SELECT has_table_privilege('regress_user1', 'testns.acltest1', 'INSERT'); -- no --- 1337,1400 ---- (1 row) ALTER DEFAULT PRIVILEGES FOR ROLE regress_user1 REVOKE EXECUTE ON FUNCTIONS FROM public; + ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_user2; -- error + ERROR: cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS + ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_user2; + CREATE SCHEMA testns2; + SELECT has_schema_privilege('regress_user2', 'testns2', 'USAGE'); -- yes + has_schema_privilege + ---------------------- + t + (1 row) + + SELECT has_schema_privilege('regress_user2', 'testns2', 'CREATE'); -- no + has_schema_privilege + ---------------------- + f + (1 row) + + ALTER DEFAULT PRIVILEGES REVOKE USAGE ON SCHEMAS FROM regress_user2; + CREATE SCHEMA testns3; + SELECT has_schema_privilege('regress_user2', 'testns3', 'USAGE'); -- no + has_schema_privilege + ---------------------- + f + (1 row) + + SELECT has_schema_privilege('regress_user2', 'testns3', 'CREATE'); -- no + has_schema_privilege + ---------------------- + f + (1 row) + + ALTER DEFAULT PRIVILEGES GRANT ALL ON SCHEMAS TO regress_user2; + CREATE SCHEMA testns4; + SELECT has_schema_privilege('regress_user2', 'testns4', 'USAGE'); -- yes + has_schema_privilege + ---------------------- + t + (1 row) + + SELECT has_schema_privilege('regress_user2', 'testns4', 'CREATE'); -- yes + has_schema_privilege + ---------------------- + t + (1 row) + + ALTER DEFAULT PRIVILEGES REVOKE ALL ON SCHEMAS FROM regress_user2; + CREATE SCHEMA testns5; + SELECT has_schema_privilege('regress_user2', 'testns5', 'USAGE'); -- no + has_schema_privilege + ---------------------- + f + (1 row) + + SELECT has_schema_privilege('regress_user2', 'testns5', 'CREATE'); -- no + has_schema_privilege + ---------------------- + f + (1 row) + SET ROLE regress_user1; CREATE FUNCTION testns.foo() RETURNS int AS 'select 1' LANGUAGE sql; SELECT has_function_privilege('regress_user2', 'testns.foo()', 'EXECUTE'); -- no *************** *** 1384,1389 **** SELECT count(*) --- 1442,1451 ---- DROP SCHEMA testns CASCADE; NOTICE: drop cascades to table testns.acltest1 + DROP SCHEMA testns2 CASCADE; + DROP SCHEMA testns3 CASCADE; + DROP SCHEMA testns4 CASCADE; + DROP SCHEMA testns5 CASCADE; SELECT d.* -- check that entries went away FROM pg_default_acl d LEFT JOIN pg_namespace n ON defaclnamespace = n.oid WHERE nspname IS NULL AND defaclnamespace != 0; diff --git a/src/test/regress/sql/privileges.sqindex 00dc7bd..c2c1629 100644 *** a/src/test/regress/sql/privileges.sql --- b/src/test/regress/sql/privileges.sql *************** *** 807,812 **** SELECT has_table_privilege('regress_user1', 'testns.acltest1', 'INSERT'); -- no --- 807,842 ---- ALTER DEFAULT PRIVILEGES FOR ROLE regress_user1 REVOKE EXECUTE ON FUNCTIONS FROM public; + ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_user2; -- error + + ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_user2; + + CREATE SCHEMA testns2; + + SELECT has_schema_privilege('regress_user2', 'testns2', 'USAGE'); -- yes + SELECT has_schema_privilege('regress_user2', 'testns2', 'CREATE'); -- no + + ALTER DEFAULT PRIVILEGES REVOKE USAGE ON SCHEMAS FROM regress_user2; + + CREATE SCHEMA testns3; + + SELECT has_schema_privilege('regress_user2', 'testns3', 'USAGE'); -- no + SELECT has_schema_privilege('regress_user2', 'testns3', 'CREATE'); -- no + + ALTER DEFAULT PRIVILEGES GRANT ALL ON SCHEMAS TO regress_user2; + + CREATE SCHEMA testns4; + + SELECT has_schema_privilege('regress_user2', 'testns4', 'USAGE'); -- yes + SELECT has_schema_privilege('regress_user2', 'testns4', 'CREATE'); -- yes + + ALTER DEFAULT PRIVILEGES REVOKE ALL ON SCHEMAS FROM regress_user2; + + CREATE SCHEMA testns5; + + SELECT has_schema_privilege('regress_user2', 'testns5', 'USAGE'); -- no + SELECT has_schema_privilege('regress_user2', 'testns5', 'CREATE'); -- no + SET ROLE regress_user1; CREATE FUNCTION testns.foo() RETURNS int AS 'select 1' LANGUAGE sql; *************** *** 844,849 **** SELECT count(*) --- 874,883 ---- WHERE nspname = 'testns'; DROP SCHEMA testns CASCADE; + DROP SCHEMA testns2 CASCADE; + DROP SCHEMA testns3 CASCADE; + DROP SCHEMA testns4 CASCADE; + DROP SCHEMA testns5 CASCADE; SELECT d.* -- check that entries went away FROM pg_default_acl d LEFT JOIN pg_namespace n ON defaclnamespace = n.oid
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers