Postgres: fix sync tables with partial select perm

[ericcj]

Fixes #40338

Description

As part of CSV uploads introduced in 0.47 via #33415 metabase began excluding postgresql tables that have only a subset of their columns selectable from the schema for simple questions (you can still query them via native sql). This appears to have been unintentional since scanning for permissions looks to have been to determine which tables are writable by csv upload and tables that aren't even in the list, like the known limitation for foreign tables, are still included in the database schema sync for building simple questions on. It'd be even nicer if columns without select permission were automatically hidden so simple questions would work by default on them but that's beyond the scope of this bugfix.

This is a serious security limitation as it's the safest way to remove access for particular columns from even metabase users constructing native sql, and it is the only way to achieve this with postgres:

https://www.postgresql.org/docs/current/sql-grant.html
"Granting the privilege at the table level and then revoking it for one column will not do what one might wish: the table-level grant is unaffected by a column-level operation."

The has_any_column_privilege function has been around since postgresql 8.4 so doesn't seem to introduce any new compatibility restriction.

I believe they could be dug out of the column_privileges information_schema or pg_attribute.attacl ala https://dba.stackexchange.com/a/285632/113307 but that seems more complex and this was a smaller change.

column grants don't appear to be supported by the same code used in the redshift driver https://docs.aws.amazon.com/redshift/latest/dg/r_System_information_functions.html

How to verify

1. Instead of granting whole table permission grant it on a set of columns like the test does, e.g. GRANT SELECT (id) ON foo TO bar
2. See that the table is available for simple questions.

I also ran the query current-user-table-privileges uses with this modification and saw that our tables that are this way now appear in it:

       with table_privileges as (
        select
          NULL as role,
          t.schemaname as schema,
          t.objectname as table,
          pg_catalog.has_any_column_privilege(current_user, '"' || t.schemaname || '"' || '.' || '"' || t.objectname || '"',  'update') as update,
          pg_catalog.has_any_column_privilege(current_user, '"' || t.schemaname || '"' || '.' || '"' || t.objectname || '"',  'select') as select,
          pg_catalog.has_any_column_privilege(current_user, '"' || t.schemaname || '"' || '.' || '"' || t.objectname || '"',  'insert') as insert,
          pg_catalog.has_table_privilege(current_user, '"' || t.schemaname || '"' || '.' || '"' || t.objectname || '"',  'delete') as delete
        from (
          select schemaname, tablename as objectname from pg_catalog.pg_tables
          union
          select schemaname, viewname as objectname from pg_catalog.pg_views
          union
          select schemaname, matviewname as objectname from pg_catalog.pg_matviews
        ) t
        where t.schemaname !~ '^pg_'
          and t.schemaname <> 'information_schema'
          and pg_catalog.has_schema_privilege(current_user, t.schemaname, 'usage')
       )
       select t.*
       from table_privileges t

Checklist

• Tests have been added/updated to cover changes in this PR

[qnkhuat]

Instead of modifying this test, could you add a new table and a new column grant instead?
I'd also add some test for "update" and "insert" grant too.

[qnkhuat]

Hi Eric, Thank you for your contribution; while the fix works, I'd like to request adding more test cases.

Detail in the comment, please check it out!

[qnkhuat]

CI PR: #40351

[ericcj]

Thanks for the review. I'm not sure when we'll get around to writing tests for every permutation of this significant regression introduced in 0.47. Perhaps the author of the regression would be better positioned to thoroughly test it?

Also FYI I wasn't able to figure out how to rebuild and get the tests to run my changes is there documentation for the build command to run to affect unit tests? Or can I run them by CI somehow? I wasn't clear from https://www.metabase.com/docs/master/developers-guide/devenv#building-drivers whether I should build-drivers for postgres or build the uberjar but neither seemed to affect the code run by clojure -X:dev:test

[qnkhuat]

@ericcj on testing.
I'd only add 3 more cases:

• a table with select permission on an ID column
• same but with update permission
• same but with insert permission

I can write those tests; just lmk if you want me to.

On executing the test, since it's a driver test, you'll need to have set the env var DRIVERS=postgres. This is to make driver tests executed for Postgres.

DRIVERS=postgres clj -X:dev:drivers:drivers-dev:test :only metabase.driver.postgres-test/table-privileges-test

This will run only the table-privileges-test for Postgres.

It expects a local Postgres server running at localhost:5432.
Tips: you can use this script to get a postgres server running.

[qnkhuat]

CI is green and the fix works, I'll update the test after this is merged.