Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't try to sync Postgres schemas or tables current user has no perms for #10139

Closed
camsaul opened this issue Jun 12, 2019 · 7 comments · Fixed by #10892
Closed

Don't try to sync Postgres schemas or tables current user has no perms for #10139

camsaul opened this issue Jun 12, 2019 · 7 comments · Fixed by #10892
Assignees
Labels
Administration/Metadata & Sync Database/Postgres Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Security related Type:Bug Product defects
Milestone

Comments

@camsaul
Copy link
Member

camsaul commented Jun 12, 2019

As far as I know Postgres does not have a way to configure permissions to hide schemas entirely from users. You can revoke their USAGE permission of those schemas, but they can still see that it exists.

Metabase tries to sync all the schemas it sees, and when some of them fail it results in noisy logs. Everything still works as expected, but the useless error messages clutter things up.

@camsaul camsaul changed the title Don't try to sync Postgres schemas current user has no perms for Don't try to sync Postgres schemas or tables current user has no perms for Jun 28, 2019
@camsaul
Copy link
Member Author

camsaul commented Jun 28, 2019

Mentioned in #8405:

  • In Postgres I create a "metabase_reader" user, which can:
    • connect to Database X,
    • read from all tables in Schema A
  • do nothing for Schema B
  • In Metabase I create database connection for Database A
  • Metabase inspects Database A, and shows Schema A and Schema B as being queryable

@camsaul
Copy link
Member Author

camsaul commented Jun 28, 2019

I'm guessing what's happening with #8045 is that by do nothing for Schema B they mean the users are actually still granted USAGE permissions on the schema, but do not have any permissions for any of the tables inside that schema. So they will still be able to see the tables in the schema, which is why Metabase syncs them. The fix is to revoke USAGE permissions for the schema, and Metabase will not be able to sync it.

@TicklesTheBrain
Copy link

The problem, at least in our use case, is that the user can still see the schema B (as described by @lindsay-stevens in #8045) and all the tables inside them. There is no way in postgres to configure the permissions in a way that hides the schema and tables from users.
It would be nice if Metabase allowed us to specify which schema's should be synced and visible to the user or, alternatively, check if user has permission for the data (not the structure) and if not, do not display that table/schema in Metabase.

@lindsay-stevens
Copy link
Contributor

In #8045, by 'do nothing' I meant the user had no permissions for anything in the schema (the schema itself, tables, columns, views, etc). It was a non-public schema so there were no default permissions at play e.g.

db=# begin; create schema test; create role test; create table test.test (); set role test;
db=> select pg_catalog.has_schema_privilege('test', 'test'::regnamespace, 'USAGE');
 has_schema_privilege
----------------------
 f
(1 row)
db=> select * from test.test;
ERROR:  permission denied for schema test
LINE 1: select * from test.test;
                      ^
db=# rollback;

As already mentioned, in Postgres it's not possible to hide the catalog. Even if all object permissions are revoked on schemas/tables/columns, if a user can connect to the database then they can 'see' the objects via the pg_catalog tables. Like how you can look around in the garage at a mall but not get into any of the cars. The manual justifies this more or less because Postgres must report that the object doesn't exist, or that it does exists and the user has insufficient permissions, so objects can be enumerated anyway e.g. the following error vs. the above error.

db=# select pg_catalog.has_schema_privilege('test', 'test'::regnamespace, 'USAGE');
ERROR:  schema "test" does not exist
LINE 1: select pg_catalog.has_schema_privilege('test', 'test'::regna...

So in terms of this ticket and #8045 Metabase probably should look at permissions as well as the catalog. Postgres has a tidy set of permission inspection functions that could be called on each object found during sync, to determine whether it's active/relevant or not:

https://www.postgresql.org/docs/current/functions-info.html#FUNCTIONS-INFO-ACCESS-TABLE

  • has_schema_privilege(user, schema, privilege) for USAGE.
  • has_table_privilege(user, table, privilege) for SELECT (if parent schema USAGE is true).
  • has_any_column_privilege(user, table, privilege) for SELECT (if parent table SELECT is true).

Being able to specify which schemas to sync / ignore would be very useful too, but there's #5500 for that.

@tlrobinson tlrobinson added .Security related Type:Bug Product defects Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness labels Aug 21, 2019
@flyingmachine
Copy link
Contributor

@camsaul @sbelak I'm closing this issue since #10892 fixes it

@mazameli
Copy link
Contributor

mazameli commented Oct 8, 2019

@flyingmachine Point of order: we typically wait to close an issue until the PR that fixes it actually gets merged (I see that #10892 hasn't been merged yet). It's also super helpful if the issue is marked with a Milestone when it gets closed — that's how we know what items to put into release notes.

@salsakran salsakran reopened this Jan 27, 2020
@sbelak sbelak removed their assignment Feb 3, 2020
@Limess
Copy link

Limess commented Apr 24, 2020

This is having a huge negative affect on our metabase usage.

We store all our 'end analytics' tables in a single Redshift schema (analytics), we then have a number of other staging/sensitive/other schema which metabase should not have access to. This is exacibated by using dbt and having developer schemas, meaning we have ~30 schemas with all tables reproduced in each schema, with only the 1 or 2 which metabase should have access to.

Denying USAGE permissions to metabase is also not a useful option here, as we need to allow usage so that views in our analytics schema can be placed on top of other tables which metabase does not have SELECT permissions for.

It'd be an incredibly useful feature for us to have a whitelist of schemas which are synced, rather than automatically discovering them, if #10892 isn't likely to land anytime soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Administration/Metadata & Sync Database/Postgres Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Security related Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.