Skip to content

Commit f6794ab

Browse files
TCeasonTCeason
authored
chore: use Usage privilege optimize USEDB Plan (#17679)
* chore: use Usage privilege optimize USEDB Plan * add forward test with revoke * no need to add test
1 parent d49b877 commit f6794ab

File tree

6 files changed

+123
-15
lines changed

6 files changed

+123
-15
lines changed

‎src/meta/app/src/principal/user_privilege.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ impl UserPrivilegeSet {
220220
/// Currently the privileges available to a database and a table are the same, it might becomes
221221
/// some differences in the future.
222222
pub fn available_privileges_on_database(available_ownership: bool) -> Self {
223-
UserPrivilegeSet::available_privileges_on_table(available_ownership)
223+
(UserPrivilegeSet::available_privileges_on_table(available_ownership).privileges
224+
| make_bitflags!(UserPrivilegeType::{ Usage }))
225+
.into()
224226
}
225227

226228
/// The all privileges global which available to the table object

‎src/query/service/src/interpreters/access/privilege_access.rs

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ impl AccessChecker for PrivilegeAccess {
757757
ObjectId::Database(db_id) => { (db_id, None) }
758758
};
759759

760-
if has_priv(&tenant, database, None, show_db_id, table_id, grant_set).await? {
760+
if has_priv(&tenant, database, None, show_db_id, table_id, grant_set, false).await? {
761761
return Ok(())
762762
}
763763

@@ -776,7 +776,7 @@ impl AccessChecker for PrivilegeAccess {
776776
ObjectId::Table(db_id, table_id) => { (db_id, Some(table_id)) }
777777
ObjectId::Database(db_id) => { (db_id, None) }
778778
};
779-
if has_priv(&tenant, database, None, show_db_id, table_id, grant_set).await? {
779+
if has_priv(&tenant, database, None, show_db_id, table_id, grant_set, true).await? {
780780
return Ok(());
781781
}
782782
let user_api = UserApiProvider::instance();
@@ -802,7 +802,7 @@ impl AccessChecker for PrivilegeAccess {
802802
ObjectId::Table(db_id, table_id) => { (db_id, Some(table_id)) }
803803
ObjectId::Database(db_id) => { (db_id, None) }
804804
};
805-
let has_priv = has_priv(&tenant, database, Some(table), db_id, table_id, grant_set).await?;
805+
let has_priv = has_priv(&tenant, database, Some(table), db_id, table_id, grant_set, false).await?;
806806
return if has_priv {
807807
Ok(())
808808
} else {
@@ -931,7 +931,7 @@ impl AccessChecker for PrivilegeAccess {
931931
ObjectId::Table(db_id, table_id) => { (db_id, Some(table_id)) }
932932
ObjectId::Database(db_id) => { (db_id, None) }
933933
};
934-
if has_priv(&tenant, &plan.database, None, show_db_id, None, grant_set).await? {
934+
if has_priv(&tenant, &plan.database, None, show_db_id, None, grant_set, true).await? {
935935
return Ok(());
936936
}
937937
let user_api = UserApiProvider::instance();
@@ -1433,6 +1433,7 @@ async fn has_priv(
14331433
db_id: u64,
14341434
table_id: Option<u64>,
14351435
grant_set: UserGrantSet,
1436+
valid_usage_priv: bool,
14361437
) -> Result<bool> {
14371438
if db_name.to_lowercase() == "information_schema" {
14381439
return Ok(true);
@@ -1460,13 +1461,38 @@ async fn has_priv(
14601461
if db_name.to_lowercase() == "system" {
14611462
return true;
14621463
}
1463-
e.privileges().iter().any(|privilege| {
1464-
UserPrivilegeSet::available_privileges_on_database(false)
1465-
.has_privilege(privilege)
1466-
})
1464+
if valid_usage_priv {
1465+
e.privileges().iter().any(|privilege| {
1466+
UserPrivilegeSet::available_privileges_on_database(false)
1467+
.has_privilege(privilege)
1468+
})
1469+
} else {
1470+
!(e.privileges().len() == 1
1471+
&& e.privileges().contains(UserPrivilegeType::Usage))
1472+
&& e.privileges().iter().any(|privilege| {
1473+
UserPrivilegeSet::available_privileges_on_database(false)
1474+
.has_privilege(privilege)
1475+
})
1476+
}
1477+
}
1478+
GrantObject::Database(_, ldb) => {
1479+
if valid_usage_priv {
1480+
*ldb == db_name
1481+
} else {
1482+
!(e.privileges().len() == 1
1483+
&& e.privileges().contains(UserPrivilegeType::Usage))
1484+
&& *ldb == db_name
1485+
}
1486+
}
1487+
GrantObject::DatabaseById(_, ldb) => {
1488+
if valid_usage_priv {
1489+
*ldb == db_id
1490+
} else {
1491+
!(e.privileges().len() == 1
1492+
&& e.privileges().contains(UserPrivilegeType::Usage))
1493+
&& *ldb == db_id
1494+
}
14671495
}
1468-
GrantObject::Database(_, ldb) => *ldb == db_name,
1469-
GrantObject::DatabaseById(_, ldb) => *ldb == db_id,
14701496
GrantObject::Table(_, ldb, ltab) => {
14711497
if let Some(table) = table_name {
14721498
*ldb == db_name && *ltab == table

‎src/query/users/src/visibility_checker.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,19 @@ impl GrantObjectVisibilityChecker {
133133
);
134134
}
135135
GrantObject::Database(catalog, db) => {
136-
granted_databases.insert((catalog.to_string(), db.to_string()));
136+
if !(ent.privileges().len() == 1
137+
&& ent.privileges().contains(UserPrivilegeType::Usage))
138+
{
139+
granted_databases.insert((catalog.to_string(), db.to_string()));
140+
}
137141
}
138142
GrantObject::DatabaseById(catalog, db) => {
139-
granted_databases_id.insert((catalog.to_string(), *db));
143+
// If only has db level usage privilege means only support use db.
144+
if !(ent.privileges().len() == 1
145+
&& ent.privileges().contains(UserPrivilegeType::Usage))
146+
{
147+
granted_databases_id.insert((catalog.to_string(), *db));
148+
}
140149
}
141150
GrantObject::Table(catalog, db, table) => {
142151
granted_tables.insert((

‎tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ OWNERSHIP s1 NULL ROLE account_admin (empty)
177177
query TT
178178
select * EXCLUDE(object_id) from show_grants('database', 'db1', 'default');
179179
----
180-
CREATE,SELECT,INSERT,UPDATE,DELETE,DROP,ALTER,GRANT db1 ROLE role2 (empty)
181-
CREATE,SELECT,INSERT,UPDATE,DELETE,DROP,ALTER,GRANT db1 ROLE role3 (empty)
180+
USAGE,CREATE,SELECT,INSERT,UPDATE,DELETE,DROP,ALTER,GRANT db1 ROLE role2 (empty)
181+
USAGE,CREATE,SELECT,INSERT,UPDATE,DELETE,DROP,ALTER,GRANT db1 ROLE role3 (empty)
182182
OWNERSHIP db1 ROLE account_admin (empty)
183183

184184
query TT

‎tests/suites/0_stateless/18_rbac/18_0014_use_db_priv_check.result

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
=== show db ===
2+
a
3+
b
4+
information_schema
5+
system
6+
=== use db ===
7+
=== show tables from a ===
8+
t1
9+
=== show tables from b ===
10+
t
11+
=== show columns ===
12+
Error: APIError: QueryFailed: [1063]Permission denied: User 'test'@'%' does not have the required privileges for table 'a.t'
13+
ida1 INT YES NULL NULL
14+
idb INT YES NULL NULL
15+
Error: APIError: QueryFailed: [1063]Permission denied: User 'test'@'%' does not have the required privileges for table 'b.t1'

‎tests/suites/0_stateless/18_rbac/18_0014_use_db_priv_check.sh

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#!/usr/bin/env bash
2+
3+
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
4+
. "$CURDIR"/../../../shell_env.sh
5+
6+
7+
export TEST_USER_PASSWORD="password"
8+
export USER_TEST_CONNECT="bendsql --user=test --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}"
9+
10+
11+
echo "drop user if exists test" | $BENDSQL_CLIENT_CONNECT
12+
echo "create user test identified by '$TEST_USER_PASSWORD'" | $BENDSQL_CLIENT_CONNECT
13+
14+
echo "create or replace database a" | $BENDSQL_CLIENT_CONNECT
15+
echo "create or replace database b" | $BENDSQL_CLIENT_CONNECT
16+
echo "create or replace database c" | $BENDSQL_CLIENT_CONNECT
17+
echo "create or replace table a.t(ida int)" | $BENDSQL_CLIENT_CONNECT
18+
echo "create or replace table a.t1(ida1 int)" | $BENDSQL_CLIENT_CONNECT
19+
echo "create or replace table b.t(idb int)" | $BENDSQL_CLIENT_CONNECT
20+
echo "create or replace table b.t1(idb1 int)" | $BENDSQL_CLIENT_CONNECT
21+
echo "drop role if exists test_role1" | $BENDSQL_CLIENT_CONNECT
22+
echo "drop role if exists test_role2" | $BENDSQL_CLIENT_CONNECT
23+
echo "create role test_role1" | $BENDSQL_CLIENT_CONNECT
24+
echo "create role test_role2" | $BENDSQL_CLIENT_CONNECT
25+
echo "grant usage on a.* to role test_role1" | $BENDSQL_CLIENT_CONNECT
26+
echo "grant usage on b.* to role test_role1" | $BENDSQL_CLIENT_CONNECT
27+
echo "grant ownership on b.t to role test_role1" | $BENDSQL_CLIENT_CONNECT
28+
29+
echo "grant role test_role1 to test" | $BENDSQL_CLIENT_CONNECT
30+
echo "grant select on a.t1 to test" | $BENDSQL_CLIENT_CONNECT
31+
32+
echo "=== show db ==="
33+
echo "show databases" | $USER_TEST_CONNECT
34+
35+
36+
echo "=== use db ==="
37+
echo "use a" | $USER_TEST_CONNECT
38+
echo "use b" | $USER_TEST_CONNECT
39+
40+
echo "=== show tables from a ==="
41+
echo "show tables from a" | $USER_TEST_CONNECT # only display a.t1
42+
echo "=== show tables from b ==="
43+
echo "show tables from b" | $USER_TEST_CONNECT # only display b.t
44+
45+
echo "=== show columns ==="
46+
echo "show columns from t from a" | $USER_TEST_CONNECT
47+
echo "show columns from t1 from a" | $USER_TEST_CONNECT
48+
echo "show columns from t from b" | $USER_TEST_CONNECT
49+
echo "show columns from t1 from b" | $USER_TEST_CONNECT
50+
51+
echo "drop user if exists test" | $BENDSQL_CLIENT_CONNECT
52+
echo "drop role if exists test_role1" | $BENDSQL_CLIENT_CONNECT
53+
echo "drop role if exists test_role2" | $BENDSQL_CLIENT_CONNECT
54+
echo "drop database a" | $BENDSQL_CLIENT_CONNECT
55+
echo "drop database b" | $BENDSQL_CLIENT_CONNECT
56+
echo "drop database c" | $BENDSQL_CLIENT_CONNECT

0 commit comments

Comments
 (0)