Skip to content

Commit 9e1b204

Browse files
BohuTANGBohuTANG
authored
Merge pull request #9924 from yufan022/select_privilege
feat(query): add privilege check for select
2 parents 123d768 + 2864c20 commit 9e1b204

File tree

10 files changed

+124
-3
lines changed

10 files changed

+124
-3
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,24 @@ impl AccessChecker for PrivilegeAccess {
3838
let session = self.ctx.get_current_session();
3939

4040
match plan {
41-
Plan::Query { .. } => {}
41+
Plan::Query { metadata, .. } => {
42+
let metadata = metadata.read().clone();
43+
for table in metadata.tables() {
44+
if table.is_source_of_view() {
45+
continue;
46+
}
47+
session
48+
.validate_privilege(
49+
&GrantObject::Table(
50+
table.catalog().to_string(),
51+
table.database().to_string(),
52+
table.name().to_string(),
53+
),
54+
UserPrivilegeType::Select,
55+
)
56+
.await?
57+
}
58+
}
4259
Plan::Explain { .. } => {}
4360
Plan::Copy(_) => {}
4461
Plan::Call(_) => {}

‎src/query/service/src/interpreters/common/grant.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use common_catalog::table_context::TableContext;
1818
use common_exception::Result;
1919
use common_meta_app::principal::GrantObject;
2020

21+
use crate::procedures::ProcedureFactory;
2122
use crate::sessions::QueryContext;
2223

2324
pub async fn validate_grant_object_exists(
@@ -28,6 +29,13 @@ pub async fn validate_grant_object_exists(
2829

2930
match &object {
3031
GrantObject::Table(catalog_name, database_name, table_name) => {
32+
if ProcedureFactory::instance()
33+
.get(format!("{}${}", database_name, table_name))
34+
.is_ok()
35+
{
36+
return Ok(());
37+
}
38+
3139
let catalog = ctx.get_catalog(catalog_name)?;
3240
if !catalog
3341
.exists_table(tenant.as_str(), database_name, table_name)

‎src/query/service/tests/it/sql/planner/format/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ fn test_format() {
7575
"database".to_string(),
7676
Arc::new(DummyTable::new("table".to_string())),
7777
None,
78+
false,
7879
);
7980
let col1 = metadata.add_base_table_column(
8081
"col1".to_string(),

‎src/query/sql/src/planner/binder/bind_context.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ pub struct BindContext {
9494
pub in_grouping: bool,
9595

9696
pub ctes_map: Box<DashMap<String, CteInfo>>,
97+
98+
pub is_view: bool,
9799
}
98100

99101
#[derive(Clone, Debug)]
@@ -111,6 +113,7 @@ impl BindContext {
111113
aggregate_info: AggregateInfo::default(),
112114
in_grouping: false,
113115
ctes_map: Box::new(DashMap::new()),
116+
is_view: false,
114117
}
115118
}
116119

@@ -121,6 +124,7 @@ impl BindContext {
121124
aggregate_info: Default::default(),
122125
in_grouping: false,
123126
ctes_map: parent.ctes_map.clone(),
127+
is_view: false,
124128
}
125129
}
126130

‎src/query/sql/src/planner/binder/table.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ impl Binder {
9898
database.to_string(),
9999
table_meta,
100100
None,
101+
false,
101102
);
102103

103104
self.bind_base_table(bind_context, database, table_index)
@@ -167,9 +168,17 @@ impl Binder {
167168
let backtrace = Backtrace::new();
168169
let (stmt, _) = parse_sql(&tokens, Dialect::PostgreSQL, &backtrace)?;
169170
// For view, we need use a new context to bind it.
170-
let new_bind_context =
171+
let mut new_bind_context =
171172
BindContext::with_parent(Box::new(bind_context.clone()));
173+
new_bind_context.is_view = true;
172174
if let Statement::Query(query) = &stmt {
175+
self.metadata.write().add_table(
176+
catalog,
177+
database.clone(),
178+
table_meta,
179+
table_alias_name,
180+
false,
181+
);
173182
let (s_expr, mut new_bind_context) =
174183
self.bind_query(&new_bind_context, query).await?;
175184
if let Some(alias) = alias {
@@ -199,6 +208,7 @@ impl Binder {
199208
database.clone(),
200209
table_meta,
201210
table_alias_name,
211+
bind_context.is_view,
202212
);
203213

204214
let (s_expr, mut bind_context) = self
@@ -246,6 +256,7 @@ impl Binder {
246256
"system".to_string(),
247257
table.clone(),
248258
table_alias_name,
259+
false,
249260
);
250261

251262
let (s_expr, mut bind_context) = self
@@ -323,6 +334,7 @@ impl Binder {
323334
"system".to_string(),
324335
table.clone(),
325336
table_alias_name,
337+
false,
326338
);
327339

328340
let (s_expr, mut bind_context) = self

‎src/query/sql/src/planner/expression_parser.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ pub fn parse_exprs(
6161
"default".to_string(),
6262
table_meta,
6363
None,
64+
false,
6465
);
6566

6667
let columns = metadata.read().columns_by_table_index(table_index);

‎src/query/sql/src/planner/metadata.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ impl Metadata {
134134
database: String,
135135
table_meta: Arc<dyn Table>,
136136
table_alias_name: Option<String>,
137+
source_of_view: bool,
137138
) -> IndexType {
138139
let table_name = table_meta.name().to_string();
139140
let table_index = self.tables.len();
@@ -145,6 +146,7 @@ impl Metadata {
145146
catalog,
146147
table: table_meta.clone(),
147148
alias_name: table_alias_name,
149+
source_of_view,
148150
};
149151
self.tables.push(table_entry);
150152
let mut fields = VecDeque::new();
@@ -209,6 +211,7 @@ pub struct TableEntry {
209211
name: String,
210212
alias_name: Option<String>,
211213
index: IndexType,
214+
source_of_view: bool,
212215

213216
table: Arc<dyn Table>,
214217
}
@@ -240,6 +243,7 @@ impl TableEntry {
240243
database,
241244
table,
242245
alias_name,
246+
source_of_view: false,
243247
}
244248
}
245249

@@ -272,6 +276,11 @@ impl TableEntry {
272276
pub fn table(&self) -> Arc<dyn Table> {
273277
self.table.clone()
274278
}
279+
280+
/// Return true if it is source from view.
281+
pub fn is_source_of_view(&self) -> bool {
282+
self.source_of_view
283+
}
275284
}
276285

277286
#[derive(Clone, Debug)]

‎tests/suites/0_stateless/14_clickhouse_http_handler/14_0003_http_auth.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ curl -s -u root: -XPOST "http://localhost:${QUERY_CLICKHOUSE_HTTP_HANDLER_PORT}"
77

88
curl -s -u root: -XPOST "http://localhost:${QUERY_CLICKHOUSE_HTTP_HANDLER_PORT}" -d "create user user1 identified by 'abc123'"
99

10+
curl -s -u root: -XPOST "http://localhost:${QUERY_CLICKHOUSE_HTTP_HANDLER_PORT}" -d "grant select on *.* to 'user1'"
11+
1012
curl -s -u user1:abc123 -XPOST "http://localhost:${QUERY_CLICKHOUSE_HTTP_HANDLER_PORT}" -d 'select 1 FORMAT CSV'
1113

1214
curl -s -H 'X-ClickHouse-User: user1' -H 'X-ClickHouse-Key: abc123' -XPOST "http://localhost:${QUERY_CLICKHOUSE_HTTP_HANDLER_PORT}" -d 'select 1 FORMAT CSV'

‎tests/suites/0_stateless/20+_others/20_0012_privilege_access.result‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,17 @@ ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user
88
1
99
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SUPER privilege on 'default'.'default'.'t20_0012'.
1010
1
11+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default'.'t20_0012_a'.
12+
1
13+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default'.'t20_0012_b'.
14+
1
15+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default2'.'v_t20_0012'.
16+
1
17+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'clustering_information'.
18+
1
19+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_snapshot'.
20+
1
21+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_segment'.
22+
1
23+
ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_block'.
24+
1

‎tests/suites/0_stateless/20+_others/20_0012_privilege_access.sh‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,66 @@ echo "select count(*) = 0 from t20_0012 where c=2" | $TEST_USER_CONNECT
4343
echo "optimize table t20_0012 all" | $TEST_USER_CONNECT
4444
## grant user privilege
4545
echo "GRANT Super ON *.* TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
46+
echo "GRANT SELECT ON system.fuse_snapshot TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
4647
## optimize table
4748
echo "optimize table t20_0012 all" | $TEST_USER_CONNECT
4849
## verify
4950
echo "select count(*)=1 from fuse_snapshot('default', 't20_0012')" | $TEST_USER_CONNECT
51+
## revoke privilege
52+
echo "REVOKE SELECT ON system.fuse_snapshot FROM 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
53+
54+
## select data
55+
## Init tables
56+
echo "CREATE TABLE default.t20_0012_a(c int) CLUSTER BY(c)" | $MYSQL_CLIENT_CONNECT
57+
echo "GRANT INSERT ON default.t20_0012_a TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
58+
echo "INSERT INTO default.t20_0012_a values(1)" | $TEST_USER_CONNECT
59+
echo "CREATE TABLE default.t20_0012_b(c int)" | $MYSQL_CLIENT_CONNECT
60+
echo "GRANT INSERT ON default.t20_0012_b TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
61+
echo "INSERT INTO default.t20_0012_b values(1)" | $TEST_USER_CONNECT
62+
## Init privilege
63+
echo "REVOKE SELECT ON * FROM 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
64+
## Verify table privilege separately
65+
echo "select * from default.t20_0012_a order by c" | $TEST_USER_CONNECT
66+
echo "GRANT SELECT ON default.t20_0012_a TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
67+
echo "select * from default.t20_0012_a order by c" | $TEST_USER_CONNECT
68+
echo "select * from default.t20_0012_b order by c" | $TEST_USER_CONNECT
69+
echo "GRANT SELECT ON default.t20_0012_b TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
70+
echo "select * from default.t20_0012_b order by c" | $TEST_USER_CONNECT
71+
## Create view table
72+
echo "create database default2" | $MYSQL_CLIENT_CONNECT
73+
echo "create view default2.v_t20_0012 as select * from default.t20_0012_a" | $MYSQL_CLIENT_CONNECT
74+
## Verify view table privilege
75+
echo "select * from default2.v_t20_0012" | $TEST_USER_CONNECT
76+
## Only grant privilege for view table
77+
echo "GRANT SELECT ON default2.v_t20_0012 TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
78+
echo "REVOKE SELECT ON default.t20_0012_a FROM 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
79+
echo "REVOKE SELECT ON default.t20_0012_b FROM 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
80+
echo "select * from default2.v_t20_0012" | $TEST_USER_CONNECT
81+
82+
## select procedure
83+
## clustering_information
84+
echo "select count(*)=1 from clustering_information('default', 't20_0012_a')" | $TEST_USER_CONNECT
85+
echo "GRANT SELECT ON system.clustering_information TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
86+
echo "select count(*)=1 from clustering_information('default', 't20_0012_a')" | $TEST_USER_CONNECT
87+
## fuse_snapshot
88+
echo "select count(*)=1 from fuse_snapshot('default', 't20_0012_a')" | $TEST_USER_CONNECT
89+
echo "GRANT SELECT ON system.fuse_snapshot TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
90+
echo "select count(*)=1 from fuse_snapshot('default', 't20_0012_a')" | $TEST_USER_CONNECT
91+
## fuse_segment
92+
echo "select count(*)=0 from fuse_segment('default', 't20_0012_a', '')" | $TEST_USER_CONNECT
93+
echo "GRANT SELECT ON system.fuse_segment TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
94+
echo "select count(*)=0 from fuse_segment('default', 't20_0012_a', '')" | $TEST_USER_CONNECT
95+
## fuse_block
96+
echo "select count(*)=1 from fuse_block('default', 't20_0012_a')" | $TEST_USER_CONNECT
97+
echo "GRANT SELECT ON system.fuse_block TO 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
98+
echo "select count(*)=1 from fuse_block('default', 't20_0012_a')" | $TEST_USER_CONNECT
5099

51100
## Drop table.
52-
echo "drop table t20_0012 all" | $MYSQL_CLIENT_CONNECT
101+
echo "drop table default.t20_0012 all" | $MYSQL_CLIENT_CONNECT
102+
echo "drop table default.t20_0012_a all" | $MYSQL_CLIENT_CONNECT
103+
echo "drop table default.t20_0012_b all" | $MYSQL_CLIENT_CONNECT
104+
echo "drop view default2.v_t20_0012" | $MYSQL_CLIENT_CONNECT
105+
53106
## Drop user
54107
echo "drop user 'test-user'@'$QUERY_MYSQL_HANDLER_HOST'" | $MYSQL_CLIENT_CONNECT
55108
rm -rf password.out

0 commit comments

Comments
 (0)