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

fix(catalog): grant view when grant table #16699

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions e2e_test/batch/catalog/has_privilege.slt.part
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ GRANT ALL PRIVILEGES ON foo TO test_user GRANTED BY root;
statement ok
GRANT INSERT ON bar TO test_user WITH GRANT OPTION GRANTED BY root;

statement ok
GRANT INSERT ON foo_view TO test_user WITH GRANT OPTION GRANTED BY root;

statement ok
GRANT SELECT ON ALL TABLES IN SCHEMA public TO test_user WITH GRANT OPTION GRANTED BY root;

Expand All @@ -44,6 +47,9 @@ GRANT SELECT ON ALL SOURCES IN SCHEMA public TO test_user WITH GRANT OPTION GRAN
statement ok
GRANT CREATE ON SCHEMA test_schema TO test_user;

query error table not found: bar_err
GRANT INSERT ON bar_err TO test_user WITH GRANT OPTION GRANTED BY root;

query error Invalid parameter user: User test_user_err not found
SELECT has_table_privilege('test_user_err', 'foo', 'SELECT');

Expand Down Expand Up @@ -101,21 +107,25 @@ SELECT has_table_privilege('test_user', 'foo', 'DELETE WITH GRANT OPTION, INSERT
----
f

# FIXME(Kexiang): Currently, RW's grant privilege on all table doesn't apply to VIEWS.
query I
SELECT has_table_privilege('test_user', 'foo_view', 'SELECT');
----
f
t

query I
SELECT has_table_privilege('test_user', 'foo_view'::regclass, 'INSERT');
----
t

query I
SELECT has_table_privilege('test_user', 'foo_view'::regclass, 'UPDATE');
----
f

query I
SELECT has_any_column_privilege('test_user', 'foo_view'::regclass, 'INSERT');
----
f
t

query I
SELECT has_table_privilege('test_user', 'foo_mv', 'SELECT');
Expand Down Expand Up @@ -203,6 +213,32 @@ SELECT has_schema_privilege('test_user', 'test_schema', 'CREATE');
----
t

statement ok
REVOKE SELECT ON ALL TABLES IN SCHEMA public FROM test_user GRANTED BY root;

query I
SELECT has_table_privilege('test_user', 'bar'::regclass, 'SELECT');
----
f

query I
SELECT has_table_privilege('test_user', 'foo_view', 'SELECT');
----
f

query I
SELECT has_table_privilege('test_user', 'foo_view', 'INSERT');
----
t

statement ok
REVOKE INSERT ON foo_view FROM test_user GRANTED BY root;

query I
SELECT has_table_privilege('test_user', 'foo_view', 'INSERT');
----
f

statement ok
DROP SOURCE foo_source;

Expand Down
2 changes: 1 addition & 1 deletion proto/user.proto
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@

uint32 all_tables_schema_id = 11;
uint32 all_sources_schema_id = 12;
uint32 all_dml_tables_schema_id = 13;
uint32 all_dml_relations_schema_id = 13;

Check failure on line 69 in proto/user.proto

View workflow job for this annotation

GitHub Actions / Check breaking changes in Protobuf files

Field "13" with name "all_dml_relations_schema_id" on message "GrantPrivilege" changed option "json_name" from "allDmlTablesSchemaId" to "allDmlRelationsSchemaId".

Check failure on line 69 in proto/user.proto

View workflow job for this annotation

GitHub Actions / Check breaking changes in Protobuf files

Field "13" on message "GrantPrivilege" changed name from "all_dml_tables_schema_id" to "all_dml_relations_schema_id".
uint32 subscription_id = 14;
}
repeated ActionWithGrantOption action_with_opts = 7;
Expand Down
2 changes: 1 addition & 1 deletion src/common/src/acl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ pub static ALL_AVAILABLE_DATABASE_MODES: LazyLock<AclModeSet> =
LazyLock::new(|| make_bitflags!(AclMode::{Create | Connect}).into());
pub static ALL_AVAILABLE_SCHEMA_MODES: LazyLock<AclModeSet> =
LazyLock::new(|| make_bitflags!(AclMode::{Create | Usage}).into());
// Including TABLES and VIEWS
pub static ALL_AVAILABLE_TABLE_MODES: LazyLock<AclModeSet> =
LazyLock::new(|| make_bitflags!(AclMode::{Select | Insert | Update | Delete}).into());
pub static ALL_AVAILABLE_SOURCE_MODES: LazyLock<AclModeSet> = LazyLock::new(AclModeSet::readonly);
pub static ALL_AVAILABLE_MVIEW_MODES: LazyLock<AclModeSet> = LazyLock::new(AclModeSet::readonly);
pub static ALL_AVAILABLE_VIEW_MODES: LazyLock<AclModeSet> = LazyLock::new(AclModeSet::readonly);
pub static ALL_AVAILABLE_SINK_MODES: LazyLock<AclModeSet> = LazyLock::new(AclModeSet::empty);
pub static ALL_AVAILABLE_SUBSCRIPTION_MODES: LazyLock<AclModeSet> =
LazyLock::new(AclModeSet::empty);
Expand Down
35 changes: 25 additions & 10 deletions src/frontend/src/handler/handle_privilege.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use super::RwPgResponse;
use crate::binder::Binder;
use crate::catalog::root_catalog::SchemaPath;
use crate::catalog::table_catalog::TableType;
use crate::catalog::CatalogError;
use crate::error::{ErrorCode, Result};
use crate::handler::HandlerArgs;
use crate::session::SessionImpl;
Expand Down Expand Up @@ -93,17 +94,31 @@ fn make_prost_privilege(
Binder::resolve_schema_qualified_name(db_name, name)?;
let schema_path = SchemaPath::new(schema_name.as_deref(), &search_path, user_name);

let (table, _) = reader.get_table_by_name(db_name, schema_path, &table_name)?;
match table.table_type() {
TableType::Table => {}
_ => {
return Err(ErrorCode::InvalidInputSyntax(format!(
"{table_name} is not a table",
))
.into());
match reader.get_table_by_name(db_name, schema_path, &table_name) {
Ok((table, _)) => {
match table.table_type() {
TableType::Table => {
grant_objs.push(PbObject::TableId(table.id().table_id));
continue;
}
_ => {
return Err(ErrorCode::InvalidInputSyntax(format!(
"{table_name} is not a table",
))
.into());
}
};
}
Err(CatalogError::NotFound("table", _)) => {
let (view, _) = reader
.get_view_by_name(db_name, schema_path, &table_name)
.map_err(|_| CatalogError::NotFound("table", table_name))?;
grant_objs.push(PbObject::ViewId(view.id));
}
Err(e) => {
return Err(e.into());
}
}
grant_objs.push(PbObject::TableId(table.id().table_id));
}
}
GrantObjects::Sources(sources) => {
Expand Down Expand Up @@ -138,7 +153,7 @@ fn make_prost_privilege(
for schema in schemas {
let schema_name = Binder::resolve_schema_name(schema)?;
let schema = reader.get_schema_by_name(session.database(), &schema_name)?;
grant_objs.push(PbObject::AllDmlTablesSchemaId(schema.id()));
grant_objs.push(PbObject::AllDmlRelationsSchemaId(schema.id()));
}
}
o => {
Expand Down
1 change: 0 additions & 1 deletion src/frontend/src/user/user_catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ impl UserCatalog {

for (&key, found) in &mut action_map {
let (required_action, required_grant_option) = *key;

if action == required_action && (!required_grant_option | with_grant_option) {
*found = true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/user/user_privilege.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub fn available_prost_privilege(object: PbObject, for_dml_table: bool) -> PbGra
&acl::ALL_AVAILABLE_MVIEW_MODES
}
}
PbObject::ViewId(_) => &acl::ALL_AVAILABLE_VIEW_MODES,
PbObject::ViewId(_) => &acl::ALL_AVAILABLE_TABLE_MODES,
PbObject::SinkId(_) => &acl::ALL_AVAILABLE_SINK_MODES,
PbObject::SubscriptionId(_) => &acl::ALL_AVAILABLE_SUBSCRIPTION_MODES,
PbObject::FunctionId(_) => &acl::ALL_AVAILABLE_FUNCTION_MODES,
Expand Down
23 changes: 22 additions & 1 deletion src/meta/service/src/user_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl UserServiceImpl {
}
expanded_privileges.push(privilege);
}
} else if let Some(Object::AllDmlTablesSchemaId(schema_id)) = &privilege.object {
} else if let Some(Object::AllDmlRelationsSchemaId(schema_id)) = &privilege.object {
let tables = match &self.metadata_manager {
MetadataManager::V1(mgr) => {
mgr.catalog_manager.list_dml_table_ids(*schema_id).await
Expand All @@ -89,6 +89,16 @@ impl UserServiceImpl {
.map(|id| id as _)
.collect(),
};
let views = match &self.metadata_manager {
MetadataManager::V1(mgr) => mgr.catalog_manager.list_view_ids(*schema_id).await,
MetadataManager::V2(mgr) => mgr
.catalog_controller
.list_view_ids(*schema_id as _)
.await?
.into_iter()
.map(|id| id as _)
.collect(),
};
for table_id in tables {
let mut privilege = privilege.clone();
privilege.object = Some(Object::TableId(table_id));
Expand All @@ -100,6 +110,17 @@ impl UserServiceImpl {
}
expanded_privileges.push(privilege);
}
for view_id in views {
let mut privilege = privilege.clone();
privilege.object = Some(Object::ViewId(view_id));
if let Some(true) = with_grant_option {
privilege
.action_with_opts
.iter_mut()
.for_each(|p| p.with_grant_option = true);
}
expanded_privileges.push(privilege);
}
} else if let Some(Object::AllSourcesSchemaId(schema_id)) = &privilege.object {
let sources = match &self.metadata_manager {
MetadataManager::V1(mgr) => {
Expand Down
15 changes: 14 additions & 1 deletion src/meta/src/controller/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use risingwave_meta_model_v2::{
ActorUpstreamActors, ColumnCatalogArray, ConnectionId, CreateType, DatabaseId, FragmentId,
FunctionId, I32Array, IndexId, JobStatus, ObjectId, PrivateLinkService, Property, SchemaId,
SinkId, SourceId, StreamNode, StreamSourceInfo, StreamingParallelism, SubscriptionId, TableId,
UserId,
UserId, ViewId,
};
use risingwave_pb::catalog::subscription::SubscriptionState;
use risingwave_pb::catalog::table::PbTableType;
Expand Down Expand Up @@ -2428,6 +2428,19 @@ impl CatalogController {
Ok(table_ids)
}

pub async fn list_view_ids(&self, schema_id: SchemaId) -> MetaResult<Vec<ViewId>> {
let inner = self.inner.read().await;
let view_ids: Vec<ViewId> = View::find()
.select_only()
.column(view::Column::ViewId)
.join(JoinType::InnerJoin, view::Relation::Object.def())
.filter(object::Column::SchemaId.eq(schema_id))
.into_tuple()
.all(&inner.db)
.await?;
Ok(view_ids)
}

pub async fn list_tables_by_type(&self, table_type: TableType) -> MetaResult<Vec<PbTable>> {
let inner = self.inner.read().await;
let table_objs = Table::find()
Expand Down
3 changes: 1 addition & 2 deletions src/meta/src/controller/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,12 +754,11 @@ where
let obj = match object.obj_type {
ObjectType::Database => PbObject::DatabaseId(oid),
ObjectType::Schema => PbObject::SchemaId(oid),
ObjectType::Table => PbObject::TableId(oid),
ObjectType::Table | ObjectType::Index => PbObject::TableId(oid),
ObjectType::Source => PbObject::SourceId(oid),
ObjectType::Sink => PbObject::SinkId(oid),
ObjectType::View => PbObject::ViewId(oid),
ObjectType::Function => PbObject::FunctionId(oid),
ObjectType::Index => unreachable!("index is not supported yet"),
ObjectType::Connection => unreachable!("connection is not supported yet"),
ObjectType::Subscription => PbObject::SubscriptionId(oid),
};
Expand Down
8 changes: 8 additions & 0 deletions src/meta/src/manager/catalog/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,14 @@ impl DatabaseManager {
.collect_vec()
}

pub fn list_view_ids(&self, schema_id: SchemaId) -> Vec<ViewId> {
self.views
.values()
.filter(|view| view.schema_id == schema_id)
.map(|view| view.id)
.collect_vec()
}

pub fn list_sources(&self) -> Vec<Source> {
self.sources.values().cloned().collect_vec()
}
Expand Down
4 changes: 4 additions & 0 deletions src/meta/src/manager/catalog/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3608,6 +3608,10 @@ impl CatalogManager {
.list_dml_table_ids(schema_id)
}

pub async fn list_view_ids(&self, schema_id: SchemaId) -> Vec<TableId> {
self.core.lock().await.database.list_view_ids(schema_id)
}

pub async fn list_sources(&self) -> Vec<Source> {
self.core.lock().await.database.list_sources()
}
Expand Down
Loading