Re: has_table_privilege for a table in unprivileged schema causes an error

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 <nagata(at)sraoss(dot)co(dot)jp> wrote:

> On Thu, 16 Aug 2018 19:37:42 -0400
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Yugo Nagata <nagata(at)sraoss(dot)co(dot)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 <nagata(at)sraoss(dot)co(dot)jp>