Re: WITH CHECK and Column-Level Privileges

Robert, all,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Mon, Sep 29, 2014 at 10:26 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > In the end, it sounds like we all agree that the right approach is to
> > simply remove this detail and avoid the issue entirely.
>
> Well, I think that's an acceptable approach from the point of view of
> fixing the security exposure, but it's far from ideal.  Good error
> messages are important for usability.  I can live with this as a
> short-term fix, but in the long run I strongly believe we should try
> to do better.

I've been working to try and address this.  Attached is a new patch
which moves the responsibility of figuring out what should be returned
down into the functions which build up the error detail which includes
the data (BuildIndexValueDescription and ExecBuildSlotValueDescription).

This allows us to avoid having to change any of the regression tests,
nor do we need to remove the information from the WITH CHECK option.
The patch is against master though it'd need to be back-patched, of
course.  This will return either the full and unchanged-from-previous
information, if the user has SELECT on the table or SELECT on the
columns which would be displayed, or "(unknown)" if the user does not
have access to view the entire key (in a key violation case), or the
subset of columns the user has access to (in a "Failing row contains"
case).  I'm not wedded to '(unknown)' by any means and welcome
suggestions.  If the user does not have table-level SELECT rights,
they'll see for the "Failing row contains" case, they'll get:

Failing row contains (col1, col2, col3) = (1, 2, 3).

Or, if they have no access to any columns:

Failing row contains () = ()

These could be changed, of course.  I don't consider this patch quite
ready to be committed and plan to do more testing and give it more
thought, but wanted to put it out there for others to comment on the
idea, the patch, and provide their own thoughts about what is safe and
sane to backpatch when it comes to error message changes like this.

As mentioned up-thread, another option would be to just omit the row
data detail completely unless the user has SELECT rights at the table
level.

    Thanks!

        Stephen