Skip to content

Commit 43fda0d

Browse files
TCeasonTCeason
authored
fix(rbac): forbiden grant create-ownership-object privilege to user (#18987)
1 parent 4aa17c3 commit 43fda0d

File tree

19 files changed

+143
-94
lines changed

19 files changed

+143
-94
lines changed

โ€Žsrc/meta/app/src/principal/user_privilege.rsโ€Ž

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -97,36 +97,6 @@ pub enum UserPrivilegeType {
9797
CreateDataMask = 1 << 16,
9898
}
9999

100-
const ALL_PRIVILEGES: BitFlags<UserPrivilegeType> = make_bitflags!(
101-
UserPrivilegeType::{
102-
Create
103-
| Select
104-
| Insert
105-
| Update
106-
| Delete
107-
| Drop
108-
| Alter
109-
| Super
110-
| CreateUser
111-
| DropUser
112-
| CreateRole
113-
| DropRole
114-
| Grant
115-
| CreateStage
116-
| Set
117-
| CreateDataMask
118-
| CreateMaskingPolicy
119-
| ApplyMaskingPolicy
120-
| Ownership
121-
| Read
122-
| Write
123-
| CreateDatabase
124-
| CreateWarehouse
125-
| CreateConnection
126-
| AccessConnection
127-
}
128-
);
129-
130100
impl Display for UserPrivilegeType {
131101
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
132102
write!(f, "{}", match self {
@@ -345,26 +315,13 @@ impl UserPrivilegeSet {
345315
}
346316
}
347317

348-
// TODO: remove this, as ALL has different meanings on different objects
349-
pub fn all_privileges() -> Self {
350-
ALL_PRIVILEGES.into()
351-
}
352-
353318
pub fn set_privilege(&mut self, privilege: UserPrivilegeType) {
354319
self.privileges |= privilege;
355320
}
356321

357322
pub fn has_privilege(&self, privilege: UserPrivilegeType) -> bool {
358323
self.privileges.contains(privilege)
359324
}
360-
361-
pub fn set_all_privileges(&mut self) {
362-
self.privileges |= ALL_PRIVILEGES;
363-
}
364-
365-
pub fn is_all_privileges(&self) -> bool {
366-
self.privileges == ALL_PRIVILEGES
367-
}
368325
}
369326

370327
impl Display for UserPrivilegeSet {

โ€Žsrc/meta/app/tests/it/user_privilege.rsโ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn test_user_privilege() -> Result<()> {
2626
let r = privileges.has_privilege(UserPrivilegeType::Insert);
2727
assert!(r);
2828

29-
privileges.set_all_privileges();
29+
privileges |= UserPrivilegeSet::available_privileges_on_global();
3030
let r = privileges.has_privilege(UserPrivilegeType::Create);
3131
assert!(r);
3232

โ€Žsrc/query/service/src/interpreters/access/privilege_access.rsโ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1725,7 +1725,7 @@ impl AccessChecker for PrivilegeAccess {
17251725
.validate_access(
17261726
&GrantObject::Global,
17271727
UserPrivilegeType::CreateMaskingPolicy,
1728-
false,
1728+
true,
17291729
false,
17301730
)
17311731
.await?;

โ€Žsrc/query/service/src/interpreters/interpreter_privilege_grant.rsโ€Ž

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use databend_common_meta_app::principal::GrantObject;
2222
use databend_common_meta_app::principal::OwnershipObject;
2323
use databend_common_meta_app::principal::PrincipalIdentity;
2424
use databend_common_meta_app::principal::UserPrivilegeSet;
25+
use databend_common_meta_app::principal::UserPrivilegeType;
2526
use databend_common_meta_app::principal::UserPrivilegeType::Ownership;
2627
use databend_common_meta_app::tenant::Tenant;
2728
use databend_common_sql::plans::GrantPrivilegePlan;
@@ -216,7 +217,7 @@ impl Interpreter for GrantPrivilegeInterpreter {
216217

217218
let plan = self.plan.clone();
218219

219-
validate_grant_privileges(&plan.on, plan.priv_types)?;
220+
validate_grant_privileges(&plan.principal, &plan.on, plan.priv_types)?;
220221
validate_grant_object_exists(&self.ctx, &plan.on).await?;
221222

222223
// TODO: check user existence
@@ -273,7 +274,11 @@ impl Interpreter for GrantPrivilegeInterpreter {
273274
/// Check if there's any privilege which can not be granted to this GrantObject.
274275
/// Some global privileges can not be granted to a database or table, for example,
275276
/// a KILL statement is meaningless for a table.
276-
pub fn validate_grant_privileges(object: &GrantObject, privileges: UserPrivilegeSet) -> Result<()> {
277+
pub fn validate_grant_privileges(
278+
principal: &PrincipalIdentity,
279+
object: &GrantObject,
280+
privileges: UserPrivilegeSet,
281+
) -> Result<()> {
277282
let available_privileges = object.available_privileges(true);
278283
let ok = privileges
279284
.iter()
@@ -283,5 +288,26 @@ pub fn validate_grant_privileges(object: &GrantObject, privileges: UserPrivilege
283288
"Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used",
284289
));
285290
}
291+
if matches!(principal, PrincipalIdentity::User(_))
292+
&& privileges.iter().any(is_create_ownership_object_privilege)
293+
{
294+
return Err(ErrorCode::IllegalGrant(
295+
"CREATE-like privileges cannot be granted directly to USER; please grant them to a role"
296+
));
297+
}
286298
Ok(())
287299
}
300+
301+
fn is_create_ownership_object_privilege(privilege: UserPrivilegeType) -> bool {
302+
matches!(
303+
privilege,
304+
UserPrivilegeType::Create
305+
| UserPrivilegeType::CreateStage
306+
| UserPrivilegeType::CreateDatabase
307+
| UserPrivilegeType::CreateWarehouse
308+
| UserPrivilegeType::CreateConnection
309+
| UserPrivilegeType::CreateSequence
310+
| UserPrivilegeType::CreateProcedure
311+
| UserPrivilegeType::CreateMaskingPolicy
312+
)
313+
}

โ€Žsrc/query/users/tests/it/role_mgr.rsโ€Ž

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ async fn test_role_manager() -> Result<()> {
102102
&tenant,
103103
&role_name,
104104
GrantObject::Global,
105-
UserPrivilegeSet::all_privileges(),
105+
UserPrivilegeSet::available_privileges_on_global(),
106106
)
107107
.await?;
108108
let role = role_mgr.get_role(&tenant, role_name.clone()).await?;
@@ -118,7 +118,7 @@ async fn test_role_manager() -> Result<()> {
118118
&tenant,
119119
&role_name,
120120
GrantObject::Global,
121-
UserPrivilegeSet::all_privileges(),
121+
UserPrivilegeSet::available_privileges_on_global(),
122122
)
123123
.await?;
124124

โ€Žsrc/query/users/tests/it/user_mgr.rsโ€Ž

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ async fn test_user_manager() -> Result<()> {
162162
&tenant,
163163
user_info.identity(),
164164
GrantObject::Global,
165-
UserPrivilegeSet::all_privileges(),
165+
UserPrivilegeSet::available_privileges_on_global(),
166166
)
167167
.await?;
168168
let user_info = user_mgr.get_user(&tenant, user_info.identity()).await?;
@@ -173,7 +173,7 @@ async fn test_user_manager() -> Result<()> {
173173
&tenant,
174174
user_info.identity(),
175175
GrantObject::Global,
176-
UserPrivilegeSet::all_privileges(),
176+
UserPrivilegeSet::available_privileges_on_global(),
177177
)
178178
.await?;
179179
let user_info = user_mgr.get_user(&tenant, user_info.identity()).await?;

โ€Žtests/nox/java_client/prepare.pyโ€Ž

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from pathlib import Path
33
import requests
44
from requests.auth import HTTPBasicAuth
5+
import time
56

67

78
def main():
@@ -29,21 +30,62 @@ def download_jdbc(version):
2930
resp.raise_for_status()
3031
target.write_bytes(resp.content)
3132

32-
3333
def create_user():
3434
requests.post(
3535
"http://localhost:8000/v1/query/",
3636
auth=HTTPBasicAuth("root", ""),
3737
headers={"Content-Type": "application/json"},
38-
json={"sql": "CREATE USER IF NOT EXISTS databend IDENTIFIED BY 'databend'"},
38+
json={"sql": "DROP USER IF EXISTS databend"},
3939
).raise_for_status()
4040
requests.post(
4141
"http://localhost:8000/v1/query/",
4242
auth=HTTPBasicAuth("root", ""),
4343
headers={"Content-Type": "application/json"},
44-
json={"sql": "GRANT ALL ON *.* TO databend"},
44+
json={"sql": "DROP ROLE IF EXISTS test_jdbc"},
45+
).raise_for_status()
46+
requests.post(
47+
"http://localhost:8000/v1/query/",
48+
auth=HTTPBasicAuth("root", ""),
49+
headers={"Content-Type": "application/json"},
50+
json={"sql": "CREATE USER databend IDENTIFIED BY 'databend' with default_role='test_jdbc'"},
51+
).raise_for_status()
52+
requests.post(
53+
"http://localhost:8000/v1/query/",
54+
auth=HTTPBasicAuth("root", ""),
55+
headers={"Content-Type": "application/json"},
56+
json={"sql": "CREATE ROLE test_jdbc"},
57+
).raise_for_status()
58+
requests.post(
59+
"http://localhost:8000/v1/query/",
60+
auth=HTTPBasicAuth("root", ""),
61+
headers={"Content-Type": "application/json"},
62+
json={"sql": "GRANT ALL ON *.* TO ROLE test_jdbc"},
63+
).raise_for_status()
64+
requests.post(
65+
"http://localhost:8000/v1/query/",
66+
auth=HTTPBasicAuth("root", ""),
67+
headers={"Content-Type": "application/json"},
68+
json={"sql": "GRANT ROLE test_jdbc TO USER databend"},
69+
).raise_for_status()
70+
time.sleep(16)
71+
requests.post(
72+
"http://localhost:8001/v1/query/",
73+
auth=HTTPBasicAuth("root", ""),
74+
headers={"Content-Type": "application/json"},
75+
json={"sql": "SHOW GRANTS FOR USER databend"},
76+
).raise_for_status()
77+
requests.post(
78+
"http://localhost:8002/v1/query/",
79+
auth=HTTPBasicAuth("root", ""),
80+
headers={"Content-Type": "application/json"},
81+
json={"sql": "SHOW GRANTS FOR USER databend"},
82+
).raise_for_status()
83+
requests.post(
84+
"http://localhost:8003/v1/query/",
85+
auth=HTTPBasicAuth("root", ""),
86+
headers={"Content-Type": "application/json"},
87+
json={"sql": "SHOW GRANTS FOR USER databend"},
4588
).raise_for_status()
46-
4789

4890
def download_testng():
4991
urls = [

โ€Žtests/sqllogictests/suites/base/06_show/06_0007_show_roles.testโ€Ž

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ grant role b to role a;
6161
statement ok
6262
grant role a to a;
6363

64-
statement ok
65-
grant create database on *.* to a;
66-
6764
statement ok
6865
set enable_expand_roles=1;
6966

@@ -87,7 +84,7 @@ select grants from show_grants('user', 'a') order by object_id;
8784
GRANT SELECT ON 'default'.'default'.'t' TO 'a'@'%'
8885
GRANT OWNERSHIP ON 'default'.'default'.'t' TO 'a'@'%'
8986
GRANT OWNERSHIP ON 'default'.'default'.'t1' TO 'a'@'%'
90-
GRANT SELECT,INSERT,CREATE DATABASE ON *.* TO 'a'@'%'
87+
GRANT SELECT,INSERT ON *.* TO 'a'@'%'
9188

9289
statement ok
9390
set enable_expand_roles=0;
@@ -114,7 +111,6 @@ select grants from show_grants('user', 'a') order by object_id;
114111
GRANT ROLE a to 'a'@'%'
115112
GRANT ROLE b to 'a'@'%'
116113
GRANT ROLE public to 'a'@'%'
117-
GRANT CREATE DATABASE ON *.* TO 'a'@'%'
118114

119115
statement ok
120116
unset enable_expand_roles;

โ€Žtests/suites/0_stateless/18_rbac/18_0002_ownership_cover.shโ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ echo "create role drop_role;" | $BENDSQL_CLIENT_CONNECT
135135
echo "create role drop_role1;" | $BENDSQL_CLIENT_CONNECT
136136
echo "create user u1 identified by '123' with DEFAULT_ROLE='drop_role'" | $BENDSQL_CLIENT_CONNECT
137137
echo "grant role drop_role to u1;" | $BENDSQL_CLIENT_CONNECT
138-
echo "grant create database on *.* to u1;" | $BENDSQL_CLIENT_CONNECT
138+
echo "grant create database on *.* to role drop_role;" | $BENDSQL_CLIENT_CONNECT
139139
export USER_U1_CONNECT="bendsql --user=u1 --password=123 --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}"
140140

141141
echo "create database a" | $USER_U1_CONNECT

โ€Žtests/suites/0_stateless/18_rbac/18_0004_view_privilege.resultโ€Ž

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616
1
1717
>>>> revoke create on default.* from role role1
1818
need failed: with 1063
19-
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'t' for user 'owner'@'%' with roles [public,role1]
19+
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'t' for user 'owner'@'%' with roles [public,role1,role2]
2020
need failed: with 1063
21-
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'t' for user 'owner'@'%' with roles [public,role1]
21+
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'t' for user 'owner'@'%' with roles [public,role1,role2]
2222
>>>> grant ownership on default.v_t_owner to role role1
2323
>>>> create view v_t as select * from t
2424
>>>> create view v_t_union as select * from t union all select * from t_owner
2525
'select * from v_t order by id' failed.
26-
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'v_t' for user 'owner'@'%' with roles [public,role1]
26+
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'v_t' for user 'owner'@'%' with roles [public,role1,role2]
2727
>>>> grant select on default.v_t to owner
2828
>>>> grant select on default.v_t_union to owner
2929
1
@@ -43,7 +43,7 @@ Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is req
4343
=== create view as select view ===
4444
>>>> revoke select on default.v_t from owner
4545
>>>> grant select on default.t to owner
46-
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'v_t' for user 'owner'@'%' with roles [public,role1]
46+
Error: APIError: QueryFailed: [1063]Permission denied: privilege [Select] is required on 'default'.'default'.'v_t' for user 'owner'@'%' with roles [public,role1,role2]
4747
>>>> grant select on default.v_t to owner
4848
>>>> grant select on default.t to owner
4949
>>>> grant select on default.v_t1 to owner

0 commit comments

Comments
ย (0)