Re: New patch for Column-level privileges - Mailing list pgsql-hackers

Hello Stephen,

Stephen Frost wrote:
> ..in the attached patch.

Thanks, I've reviewed this patch again.

> Please find attached an updated patch for column-level privileges
> which incorporates Alvaro's suggested changes and is updated to the
> latest CVS HEAD.

Cool, applies cleanly and compiles without any error or warning on my
Debian box. Regression tests pass fine as well.

> Regression tests have been added as well as
> documentation (though this could probably be improved).

Sorry I didn't get around writing docu. Looks good so far. Some more
hints on its usage wouldn't hurt, especially because the error messages
aren't overly verbose ('permission denied..' doesn't tell you much).

> Currently,
> column-level privileges are not honored when JOINs are involved (you
> must have the necessary table-level privileges, as you do today). It
> would really be great to have that working and mainly involves
> modifying the rewriter to add on to the appropriate range table column
> list entries the columns which are used in the joins and output from
> joins.

Understood. Do you plan to work on that? Or do you think the patch
should go into 8.4 even without support for JOINs?


Experimenting with relation vs column level privileges, I've discovered
a strange behavior:

test=# GRANT SELECT ON test TO joe;
GRANT
test=# GRANT SELECT(id) ON test TO joe;
GRANT
test=# REVOKE SELECT ON test FROM joe;
ERROR:  tuple already updated by self

That's odd. Maybe you need to increment the command counter in between
two updates of that tuple?

Also note, that I'm unsure about what to expect from this REVOKE. Is it
intended to remove the column level privilege as well or not?

Removing the column level privilege first, then the relation level one
works:

test=# REVOKE SELECT(id) ON test FROM joe;
REVOKE
test=# REVOKE SELECT ON test FROM joe;
REVOKE



I've also tested column level privileges on hidden attributes (xmin,
xmax, ctid, tableoid, cmin), which works fine for SELECTs. However, you
might want to filter out INSERT and UPDATE privileges on those, as those
don't make much sense:

test=# GRANT UPDATE(xmin) ON test TO joe;
GRANT
test=# GRANT INSERT(xmin) ON test TO joe;
GRANT

[ Note that user joe can INSERT or UPDATE tuples of relation test even
without those column level privileges, as long as he is allowed to
INSERT or UPDATE the affected non-hidden columns. ]


Some minor nit-picks: some lines exceed 80 columns, multi-line comments
don't follow coding standards.

BTW: how are long constant strings expected to be formatted? Are those
allowed to exceed 80 columns, or are they expected to be split like so
(i.e. for errmsg):
 "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed " "do eiusmod tempor incididunt ut labore et dolore
magnaaliqua."
 


Nice work! I'd like to see this shipping with 8.4. The above mentioned
bugs (the "updated by self" and the hidden columns) should be easy
enough to fix, I think. Respecting columns level privileges for JOINs is
probably going to be more work, but is required as well, IMO.

Regards

Markus Wanner