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

GRANT on unknown objects need to be rejected #15982

Open
jeeminso opened this issue May 10, 2024 · 7 comments
Open

GRANT on unknown objects need to be rejected #15982

jeeminso opened this issue May 10, 2024 · 7 comments

Comments

@jeeminso
Copy link
Contributor

CrateDB version

latest master

CrateDB setup information

No response

Problem description

Please see steps to reproduce.

Steps to Reproduce

cr> create table q (a int);                                                                                                                                           
CREATE OK, 1 row affected  (1.652 sec)
cr> create view qv as select * from q;                                                                                                                                
CREATE OK, 1 row affected  (0.493 sec)
cr> grant dql on table qv to john;  -- qv is not a table but a view but the stmt succeeded                                                                                                                                  
GRANT OK, 1 row affected  (0.085 sec)
cr> grant dql on schema q to john; -- schema q does not exist but the stmt scceeded                                                                                                                                   
GRANT OK, 1 row affected  (0.079 sec)
cr> select * from sys.privileges;                                                                                                                                     
+---------+---------+---------+--------+-------+------+
| class   | grantee | grantor | ident  | state | type |
+---------+---------+---------+--------+-------+------+
| SCHEMA  | john    | crate   | q      | GRANT | DQL  |
| TABLE   | john    | crate   | doc.qv | GRANT | DQL  |  -- qv is a view not a table
+---------+---------+---------+--------+-------+------+
SELECT 7 rows in set (0.002 sec)
cr> show schemas;                                                                                                                                                     
+--------------------+
| schema_name        |
+--------------------+
| blob               |
| doc                |
| information_schema |
| pg_catalog         |
| scheduler_issue    |
| sys                |
+--------------------+
-- schema 'q' does not exist

Actual Result

Privileges on unknown objects granted.

Expected Result

Such stmts need to be rejected.

@jeeminso jeeminso added triage An issue that needs to be triaged by a maintainer bug Clear identification of incorrect behaviour and removed triage An issue that needs to be triaged by a maintainer labels May 10, 2024
@jeeminso
Copy link
Contributor Author

Just by looking at the behaviours, the objects seem to be not analyzed. There will be more variations around this but I would presume a single fix will address all.

@proddata
Copy link
Member

I am pretty sure at least for schemas this is intentional, as they are not explicitly created and therefore a user would need to have access to a schema before the first object within it exists.

(Personally I'd prefer for schemas to also be explicitly created)

@matriv
Copy link
Contributor

matriv commented May 15, 2024

I don't have a clear opinion if this is a bug or a desired behavior.

A bit related, need to test, what happens if you attempt to restore users/roles & their privileges to a new cluster, if the tables/views/schemas are not still there? Maybe it can be handy for such cases to prepare the users/roles & privileges (manually, or with saved statements) before the DB objects are created. Additionally, agreeing with @proddata, since currently we don't have empty schemas, (always need to have the 1st table created within them, and we dont' have a DROP SCHEMA stmt), seems valid to be able to assign privileges to a schema that doesn't exist.

On the other, hand it is weird to select from sys.privileges and see privs for object not existing on the cluster.

@matriv
Copy link
Contributor

matriv commented May 15, 2024

I am pretty sure at least for schemas this is intentional, as they are not explicitly created and therefore a user would need to have access to a schema before the first object within it exists.

(Personally I'd prefer for schemas to also be explicitly created)

I'm guessing that after: #11939, we can consider implementing the explicit creation/dropping of schemas.

@matriv matriv added enhancement Enhancement that doesn't fit into a more specific feature label. Try avoid using this and removed bug Clear identification of incorrect behaviour labels May 21, 2024
@matriv
Copy link
Contributor

matriv commented May 21, 2024

After discussion we decided to treat this as an improvement, rather than a bug fix.

@matriv matriv added complexity: no estimate contributions welcome good first issue triage An issue that needs to be triaged by a maintainer and removed enhancement Enhancement that doesn't fit into a more specific feature label. Try avoid using this contributions welcome good first issue complexity: no estimate labels May 21, 2024
@matriv
Copy link
Contributor

matriv commented May 21, 2024

Some extra info, currently you can restore USERMANAGEMENT, and the privileges are there even though the objects are not:

cr> RESTORE SNAPSHOT repo.snap1 USERMANAGEMENT;
RESTORE OK, 1 row affected  (0.052 sec)
cr> select * from sys.privileges;
+-------+---------+---------+-------+-------+------+
| class | grantee | grantor | ident | state | type |
+-------+---------+---------+-------+-------+------+
| TABLE | matriv  | crate   | doc.t | GRANT | DML  |
+-------+---------+---------+-------+-------+------+
SELECT 1 row in set (0.003 sec)
cr> select * from t;
RelationUnknown[Relation 't' unknown]

If we go ahead to implement throw errors when trying to grant privs on non-existing objects, this will contradict with the restore privs behavior.

@mfussenegger mfussenegger added feature: security feature: ux feature: administration complexity: no estimate need refined description A maintainer should refine the description and clarify the scope and removed triage An issue that needs to be triaged by a maintainer labels May 21, 2024
@proddata
Copy link
Member

I would maintain the current behavior until we decide to explicitly create schemas. Once we do, we can more closely link privileges to objects and potentially drop privileges if the associated object no longer exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants