Hi,
I updated the patch to support other has_*_privilege functions with some
refactoring, but tests for theses fixes are not included yet.
But, while running the regression test after this patch is applied, I found
that my patch doesn't pass privilege test. One of different behaviours
is as bellow.
-- Grant on all objects of given type in a schema
\c -
CREATE SCHEMA testns;
CREATE TABLE testns.t1 (f1 int);
CREATE TABLE testns.t2 (f1 int);
SELECT has_table_privilege('regress_priv_user1', 'testns.t1', 'SELECT'); --
false
GRANT ALL ON ALL TABLES IN SCHEMA testns TO regress_priv_user1;
SELECT has_table_privilege('regress_priv_user1', 'testns.t1', 'SELECT'); --
true
This is a part of src/test/regress/sql/privileges.sql. SELECT privilege on
testns.t1
is granted to regress_priv_user1, so the has_table_privilege of the original
implementation
returns true. On the other hand, after applied my patch, the function returns
false
because the regress_priv_user1 doesn't have USAGE privilege on schema testns.
Accually,
the user can not access to the table using SELECT statement.
Although the behavior of the original function would reflect pg_class.relacl,
it seems to
me that the function fixed in my patch is more useful for users because this
reflects
the actual accessibility during query execution.
Is there chance to change the behaviour of the functions as I am proposing?
If is is not allowed to change it to keep back-compatibility, I would like to
propose
to add a parameter to the function to consider the privilege of the schema, for
example as bellow. Assuming false as the default values will keep the
back-compatibility.
has_table_privilege(user, table, privilege[, consider_schema=false])
On Fri, 17 Aug 2018 12:02:57 +0900
Yugo Nagata <nag...@sraoss.co.jp> wrote:
> On Thu, 16 Aug 2018 19:37:42 -0400
> Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> > Yugo Nagata <nag...@sraoss.co.jp> writes:
> > > I found that has_table_privilege returns an error when a table is
> > > specified
> > > by schema-qualified name and the user doen't have privilege for its
> > > schema.
> >
> > > postgres=> select has_table_privilege('myschema.tbl','select');
> > > ERROR: permission denied for schema myschema
> >
> > > I think that this function should return false because the user doesn't
> > > have
> > > the privilege on this table eventually. It is more useful for users
> > > because
> > > it is not needed to parse the schema-qualified table name and check the
> > > privilege on the schema in advance.
> >
> > Sounds reasonable, but if we're going to do that, we should do it for
> > every one of these functions that concerns a schema-qualifiable object
> > type. Not just tables.
>
> OK. I will fix all of these functions that can take a schema-qualifiable
> object name as a parameter.
In addition to has_table_privilege, has_any_column_privilege,
has_column_privilege,
has_function_privilege, and has_sequence_privilege are fixed.
> >
> > Also, looking at the code, why are you bothering with
> > convert_table_schema_priv_string? ISTM what's relevant on the schema is
> > always going to be USAGE privilege, independently of the mode being
> > checked on the object. So you shouldn't need a bunch of duplicative
> > tables.
>
> I thought we needed to consider also USAGE GRANT OPTION, but I might be
> misunderstnding something. I will look into this again.
Fixed to check only USAGE privilege on the schema.
> > Plus, I don't think this implementation approach is going to work for
> > unqualified table names. You don't know which schema they're in until you
> > look them up. (Although I vaguely remember that the path search logic just
> > ignores unreadable schemas, so maybe all you have to do with unqualified
> > names is nothing. But that's not what this patch is doing now.)
>
> Oops. I overlooked these cases. I'll fix the patch to allow to handle
> unqualified table names.
Fixed.
Regards,
--
Yugo Nagata <nag...@sraoss.co.jp>
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index a45e093de7..262472e424 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -94,6 +94,7 @@ static AclMode convert_priv_string(text *priv_type_text);
static AclMode convert_any_priv_string(text *priv_type_text,
const priv_map *privileges);
+static bool check_schema_usage_privilege(text *objectname, Oid roleid);
static Oid convert_table_name(text *tablename);
static AclMode convert_table_priv_string(text *priv_type_text);
static AclMode convert_sequence_priv_string(text *priv_type_text);
@@ -1860,6 +1861,10 @@ has_table_privilege_name_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = get_role_oid_or_public(NameStr(*rolename));
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_table_priv_string(priv_type_text);
@@ -1885,6 +1890,10 @@ has_table_privilege_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_table_priv_string(priv_type_text);
@@ -1960,6 +1969,9 @@ has_table_privilege_id_name(PG_FUNCTION_ARGS)
AclMode mode;
AclResult aclresult;
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_table_priv_string(priv_type_text);
@@ -1996,6 +2008,30 @@ has_table_privilege_id_id(PG_FUNCTION_ARGS)
* Support routines for has_table_privilege family.
*/
+/*
+ * Given a possibly-qualified object name as a string, checking a user's
+ * usage privilege to the object's namespace. Return true if an unqualified
+ * name is specified because we don't have to consern its namespace.
+ */
+static bool
+check_schema_usage_privilege(text *objectname, Oid roleid)
+{
+ char *nspname = NULL;
+ char *objname = NULL;
+ Oid schemaoid;
+
+ DeconstructQualifiedName(textToQualifiedNameList(objectname), &nspname, &objname);
+
+ if (!nspname)
+ return true;
+
+ schemaoid = get_namespace_oid(nspname, false);
+ if (pg_namespace_aclcheck(schemaoid, roleid, ACL_USAGE) == ACLCHECK_OK)
+ return true;
+ else
+ return false;
+}
+
/*
* Given a table name expressed as a string, look it up and return Oid
*/
@@ -2068,6 +2104,10 @@ has_sequence_privilege_name_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = get_role_oid_or_public(NameStr(*rolename));
+
+ if (!check_schema_usage_privilege(sequencename, roleid))
+ PG_RETURN_BOOL(false);
+
mode = convert_sequence_priv_string(priv_type_text);
sequenceoid = convert_table_name(sequencename);
if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE)
@@ -2098,6 +2138,10 @@ has_sequence_privilege_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(sequencename, roleid))
+ PG_RETURN_BOOL(false);
+
mode = convert_sequence_priv_string(priv_type_text);
sequenceoid = convert_table_name(sequencename);
if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE)
@@ -2190,6 +2234,9 @@ has_sequence_privilege_id_name(PG_FUNCTION_ARGS)
AclMode mode;
AclResult aclresult;
+ if (!check_schema_usage_privilege(sequencename, roleid))
+ PG_RETURN_BOOL(false);
+
mode = convert_sequence_priv_string(priv_type_text);
sequenceoid = convert_table_name(sequencename);
if (get_rel_relkind(sequenceoid) != RELKIND_SEQUENCE)
@@ -2282,6 +2329,10 @@ has_any_column_privilege_name_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = get_role_oid_or_public(NameStr(*rolename));
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_column_priv_string(priv_type_text);
@@ -2311,6 +2362,10 @@ has_any_column_privilege_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_column_priv_string(priv_type_text);
@@ -2398,6 +2453,9 @@ has_any_column_privilege_id_name(PG_FUNCTION_ARGS)
AclMode mode;
AclResult aclresult;
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_column_priv_string(priv_type_text);
@@ -2524,6 +2582,10 @@ has_column_privilege_name_name_name(PG_FUNCTION_ARGS)
int privresult;
roleid = get_role_oid_or_public(NameStr(*rolename));
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
colattnum = convert_column_name(tableoid, column);
mode = convert_column_priv_string(priv_type_text);
@@ -2552,6 +2614,10 @@ has_column_privilege_name_name_attnum(PG_FUNCTION_ARGS)
int privresult;
roleid = get_role_oid_or_public(NameStr(*rolename));
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_column_priv_string(priv_type_text);
@@ -2630,6 +2696,9 @@ has_column_privilege_id_name_name(PG_FUNCTION_ARGS)
AclMode mode;
int privresult;
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
colattnum = convert_column_name(tableoid, column);
mode = convert_column_priv_string(priv_type_text);
@@ -2656,6 +2725,9 @@ has_column_privilege_id_name_attnum(PG_FUNCTION_ARGS)
AclMode mode;
int privresult;
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
mode = convert_column_priv_string(priv_type_text);
@@ -2732,6 +2804,10 @@ has_column_privilege_name_name(PG_FUNCTION_ARGS)
int privresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(tablename, roleid))
+ PG_RETURN_BOOL(false);
+
tableoid = convert_table_name(tablename);
colattnum = convert_column_name(tableoid, column);
mode = convert_column_priv_string(priv_type_text);
@@ -3276,6 +3352,10 @@ has_function_privilege_name_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = get_role_oid_or_public(NameStr(*username));
+
+ if (!check_schema_usage_privilege(functionname, roleid))
+ PG_RETURN_BOOL(false);
+
functionoid = convert_function_name(functionname);
mode = convert_function_priv_string(priv_type_text);
@@ -3301,6 +3381,10 @@ has_function_privilege_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(functionname, roleid))
+ PG_RETURN_BOOL(false);
+
functionoid = convert_function_name(functionname);
mode = convert_function_priv_string(priv_type_text);
@@ -3376,6 +3460,9 @@ has_function_privilege_id_name(PG_FUNCTION_ARGS)
AclMode mode;
AclResult aclresult;
+ if (!check_schema_usage_privilege(functionname, roleid))
+ PG_RETURN_BOOL(false);
+
functionoid = convert_function_name(functionname);
mode = convert_function_priv_string(priv_type_text);
@@ -4223,6 +4310,10 @@ has_type_privilege_name_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = get_role_oid_or_public(NameStr(*username));
+
+ if (!check_schema_usage_privilege(typename, roleid))
+ PG_RETURN_BOOL(false);
+
typeoid = convert_type_name(typename);
mode = convert_type_priv_string(priv_type_text);
@@ -4248,6 +4339,10 @@ has_type_privilege_name(PG_FUNCTION_ARGS)
AclResult aclresult;
roleid = GetUserId();
+
+ if (!check_schema_usage_privilege(typename, roleid))
+ PG_RETURN_BOOL(false);
+
typeoid = convert_type_name(typename);
mode = convert_type_priv_string(priv_type_text);
@@ -4323,6 +4418,9 @@ has_type_privilege_id_name(PG_FUNCTION_ARGS)
AclMode mode;
AclResult aclresult;
+ if (!check_schema_usage_privilege(typename, roleid))
+ PG_RETURN_BOOL(false);
+
typeoid = convert_type_name(typename);
mode = convert_type_priv_string(priv_type_text);
