Thread: Misleading error "permission denied for table"

Misleading error "permission denied for table"

From
Ashutosh Bapat
Date:
Hi hackers,
In privileges.sql there are tests for column level privileges e.g.

INSERT INTO atest5(two) VALUES (6) ON CONFLICT (two) DO UPDATE set
three = 10 RETURNING atest5.three;
ERROR:  permission denied for table atest5

In the above case the current user regress_priv_user4, doesn't have
privileges to access atest5.three. But the error does not mention
atest5.three anywhere. In fact, if the same query were to be changed
to return atest5.four, it would succeed since the user has privileges
to access column atest5.four.

Shouldn't we report "permission defined for column atest5.three?

-- 
Best Wishes,
Ashutosh Bapat



Re: Misleading error "permission denied for table"

From
Nathan Bossart
Date:
On Wed, Oct 16, 2024 at 07:36:29PM +0530, Ashutosh Bapat wrote:
> In privileges.sql there are tests for column level privileges e.g.
> 
> INSERT INTO atest5(two) VALUES (6) ON CONFLICT (two) DO UPDATE set
> three = 10 RETURNING atest5.three;
> ERROR:  permission denied for table atest5
> 
> In the above case the current user regress_priv_user4, doesn't have
> privileges to access atest5.three. But the error does not mention
> atest5.three anywhere. In fact, if the same query were to be changed
> to return atest5.four, it would succeed since the user has privileges
> to access column atest5.four.
> 
> Shouldn't we report "permission defined for column atest5.three?

We do have "permission denied for column" messages in aclchk.c (e.g.,
aclcheck_error_col()), but I don't see them actually used anywhere (or at
least they don't show up in any expected regression test output).  I'm
inclined to agree that a more specific error would be nice, but I worry
there's some hidden complexity that's prevented it thus far...

-- 
nathan



Re: Misleading error "permission denied for table"

From
Tom Lane
Date:
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Wed, Oct 16, 2024 at 07:36:29PM +0530, Ashutosh Bapat wrote:
>> Shouldn't we report "permission defined for column atest5.three?

> We do have "permission denied for column" messages in aclchk.c (e.g.,
> aclcheck_error_col()), but I don't see them actually used anywhere (or at
> least they don't show up in any expected regression test output).  I'm
> inclined to agree that a more specific error would be nice, but I worry
> there's some hidden complexity that's prevented it thus far...

See ExecCheckOneRelPerms, which seems to regard this as a TODO.
I think the hard part is how to report the cases that involve
pg_attribute_aclcheck_all(..., ACLMASK_ANY), which means

 * If 'how' is ACLMASK_ANY, then returns ACLCHECK_OK if user has any of the
 * privileges identified by 'mode' on any non-dropped column in the relation;

so that you can't really finger a specific column as being in
violation.  Maybe we could leave those cases as just mentioning the
rel; not sure if that leads to failing to move the goalposts much.
Otherwise, we could extend ExecCheckOneRelPerms and
pg_attribute_aclcheck_all to pass back a column number, and
then modify the error reporting in ExecCheckPermissions.

            regards, tom lane