feat(query): grant object visibility check #14458
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
pr-feature
|
cc @flaneur2020 wait auth check built in role |
TCeason
force-pushed
the
rewrite_show
branch
2 times, most recently
from
a89dbd9 to
bd8263c
Compare
TCeason
changed the title
TCeason
force-pushed
the
rewrite_show
branch
3 times, most recently
from
f84e4cb to
28c0486
Compare
IHMO, it's always showing databases/tables from effective roles. |
flaneur2020
reviewed
Jan 26, 2024
related PR: #14451 |
TCeason
force-pushed
the
rewrite_show
branch
2 times, most recently
from
45ac3d1 to
0c40999
Compare
|
cc @BohuTANG This pr can be review. And old query normal user need do this grant after upgrade. Note: Now create database need create database privilege. If you want to create a database, should excute grant create database on *.* to <user_name>Or grant create database on *.* to role <role_name>;
grant role <role_name> to <user_name>; |
|
Good. |
Create database will return err. Need root user do this Ref #14458 (comment) |
Docker Image for PR
|
SummaryI test on my local with standalone mode. The same error was repeated. Meta version v1.2.262 Query version : old-version(1.2.314) -> upgrade to pr version -> rollback to old-version(1.2.314). How to producemeta version: v1.2.262-37e04b9922-simd query version: MySQL [(none)]> create role test;
Query OK, 0 rows affected (0.033 sec)
MySQL [(none)]> grant all on *.* to role test;
Query OK, 0 rows affected (0.020 sec)
MySQL [(none)]> select version();
+----------------------------------------------------------------------------------------+
| version() |
+----------------------------------------------------------------------------------------+
| 8.0.26-v1.2.314-nightly-3ccec470ea(rust-1.77.0-nightly-2024-01-27T16:50:23.223949439Z) |
+----------------------------------------------------------------------------------------+
1 row in set (0.012 sec)
query version: MySQL [(none)]> create user u1 identified by '123';
Query OK, 0 rows affected (0.026 sec)
MySQL [(none)]> grant all on *.* to u1;
Query OK, 0 rows affected (0.023 sec)
rollback to rebuild query, and try to run it. failed: Analyzedebug log in pb : And this err is serde json err if deserialize struct err, will degrade to serje json. So we should focus on the pb deserialize err. It looks like an incompatible design here. At this point, this pr contains the value CreateDatabase, which was not recognized by older versions. So the judgment fails. |
Which PR and where is this incompatibility triggered? |
TCeason
force-pushed
the
rewrite_show
branch
2 times, most recently
from
3f05049 to
0503f85
Compare
TCeason
added
ci-cloud
|
In pr #14501, we modify deserialize bitflag way. Now use from_bits_truncate. So if this pr revert to before version https://github.com/datafuselabs/databend/releases/tag/v1.2.321-nightly, may cause a compatibility problem |
TCeason
force-pushed
the
rewrite_show
branch
2 times, most recently
from
2eebf14 to
2837ec7
Compare
Modify: 1. all users can see informatio_schema and system. 2, show databases/tables only display current role owned object. if set secondary role all, will display all effective roles owned object. 3. add new privilege type : CreateDatabase. It's a global object. 4. create udf and create index need super privilege. Note: Now create database need create database privilege. If you want to create a database, should excute `grant create database on *.* to <user_name> or ```sql grant create database on *.* to role <role_name>; grant role <role_name> to <user_name>; ```
TCeason
force-pushed
the
rewrite_show
branch
from
8ae1cb3 to
d3427f2
Compare
TCeason
force-pushed
the
rewrite_show
branch
from
d3427f2 to
726881e
Compare
drmingdrmer
reviewed
Feb 20, 2024
There was a problem hiding this comment.
Reviewed 2 of 6 files at r3, 1 of 6 files at r4, all commit messages.
Reviewable status: 3 of 14 files reviewed, 2 unresolved discussions (waiting on @flaneur2020, @JackTan25, and @TCeason)
src/meta/app/src/principal/user_privilege.rs line 77 at r4 (raw file):
Write = 1 << 19, // Privilege to Create database CreateDatabase = 1 << 20,
Is this new field compatible with old version of databend-query? i.e., what if an old query reads this value 1<<20 but it does not have this type CreateDatabase?
after this pr #14458 (comment) will ignore the unknown bit. |
drmingdrmer
approved these changes
Feb 20, 2024
There was a problem hiding this comment.
Reviewed 2 of 13 files at r1, 1 of 4 files at r2, 2 of 6 files at r3, 5 of 6 files at r4.
Reviewable status: 13 of 14 files reviewed, 2 unresolved discussions (waiting on @flaneur2020 and @JackTan25)
|
I test this pr on test-cloud env. And now ci has forward compat test already passed. Shell we merge this pr? cc @BohuTANG show warehouses;
│ pc-test_priv │ Suspended │ XSmall │ │ 600 │ NULL │ 2024-02-19 06:31:50 │
│ tai-test_priv │ Suspended │ XSmall │ pr-14458-8283a6f │ 600 │ NULL │ 2024-02-19 06:49:35 │ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Modify:
all users can see informatio_schema and system.
show databases/tables only display current role owned object. if set
secondary role all, will display all effective roles owned object.
add new privilege type : CreateDatabase. It's a global object.
create udf and create index need super privilege.
drop database need check Database level drop privilege
Note:
Now create database need create database privilege.
If you want to create a database, should excute
Or
Tests
Type of change
This change is