Skip to content
Open
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
119 changes: 119 additions & 0 deletions e2e_test/ddl/role_inherited_privilege.slt
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Test that actual query privilege checks honor role inheritance, not only
# pg_catalog.has_table_privilege.

statement ok
SET RW_IMPLICIT_FLUSH TO true;

statement ok
DROP TABLE IF EXISTS role_inherited_privilege_t;

statement ok
DROP ROLE IF EXISTS role_inherited_privilege_user;

statement ok
DROP ROLE IF EXISTS role_noinherit_privilege_user;

statement ok
DROP ROLE IF EXISTS role_inherited_privilege_app;

statement ok
DROP ROLE IF EXISTS role_inherited_privilege_select;

statement ok
DROP ROLE IF EXISTS role_inherited_privilege_update;

statement ok
CREATE TABLE role_inherited_privilege_t (id int PRIMARY KEY, v int);

statement ok
INSERT INTO role_inherited_privilege_t VALUES (1, 10), (2, 20);

statement ok
CREATE ROLE role_inherited_privilege_select;

statement ok
CREATE ROLE role_inherited_privilege_update;

statement ok
CREATE ROLE role_inherited_privilege_app;

statement ok
CREATE USER role_inherited_privilege_user WITH PASSWORD 'secret';

statement ok
CREATE USER role_noinherit_privilege_user WITH PASSWORD 'secret';

statement ok
GRANT SELECT ON TABLE role_inherited_privilege_t TO role_inherited_privilege_select;

statement ok
GRANT UPDATE ON TABLE role_inherited_privilege_t TO role_inherited_privilege_update;

statement ok
GRANT role_inherited_privilege_select TO role_inherited_privilege_app;

statement ok
GRANT role_inherited_privilege_update TO role_inherited_privilege_app;

statement ok
GRANT role_inherited_privilege_app TO role_inherited_privilege_user;

statement ok
GRANT role_inherited_privilege_app TO role_noinherit_privilege_user WITH INHERIT FALSE;

# Actual SELECT should also honor role-to-role inheritance after SET ROLE.
statement ok
SET ROLE role_inherited_privilege_app;

query II
SELECT id, v FROM role_inherited_privilege_t ORDER BY id;
----
1 10
2 20

statement ok
UPDATE role_inherited_privilege_t SET v = v + 1 WHERE id = 1;

statement ok
RESET ROLE;

query II
SELECT id, v FROM role_inherited_privilege_t ORDER BY id;
----
1 11
2 20

# Actual SELECT/UPDATE should honor role-to-user inheritance for a login user.
system ok
PGPASSWORD=secret psql -h localhost -p 4566 -d dev -U role_inherited_privilege_user -v ON_ERROR_STOP=1 -c "SELECT id, v FROM role_inherited_privilege_t ORDER BY id;" | grep -q "1.*11"

system ok
PGPASSWORD=secret psql -h localhost -p 4566 -d dev -U role_inherited_privilege_user -v ON_ERROR_STOP=1 -c "SET RW_IMPLICIT_FLUSH TO true; UPDATE role_inherited_privilege_t SET v = v + 1 WHERE id = 2;"

query II
SELECT id, v FROM role_inherited_privilege_t ORDER BY id;
----
1 11
2 21

# INHERIT FALSE remains a denial for implicit role-to-user privileges.
system ok
PGPASSWORD=secret psql -h localhost -p 4566 -d dev -U role_noinherit_privilege_user -v ON_ERROR_STOP=1 -c "SELECT id, v FROM role_inherited_privilege_t ORDER BY id;" 2>&1 | grep -q "Permission denied"

statement ok
DROP TABLE role_inherited_privilege_t;

statement ok
DROP ROLE role_inherited_privilege_user;

statement ok
DROP ROLE role_noinherit_privilege_user;

statement ok
DROP ROLE role_inherited_privilege_app;

statement ok
DROP ROLE role_inherited_privilege_select;

statement ok
DROP ROLE role_inherited_privilege_update;
6 changes: 6 additions & 0 deletions src/frontend/src/binder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub use relation::{
// Re-export common types
pub use risingwave_common::gap_fill::FillStrategy;
use risingwave_common::id::ObjectId;
use risingwave_pb::user::RoleMembership;
pub use select::{BoundDistinct, BoundSelect};
pub use set_expr::*;
pub use statement::BoundStatement;
Expand All @@ -73,6 +74,7 @@ use crate::catalog::schema_catalog::SchemaCatalog;
use crate::catalog::{CatalogResult, DatabaseId, SecretId, ViewId};
use crate::error::ErrorCode;
use crate::session::{AuthContext, SessionImpl, StagingCatalogManager, TemporarySourceManager};
use crate::user::effective_privilege::role_memberships_snapshot;
use crate::user::user_service::UserInfoReadGuard;

pub type ShareId = usize;
Expand All @@ -99,6 +101,7 @@ pub struct Binder {
session_id: SessionId,
context: BindContext,
auth_context: Arc<AuthContext>,
role_memberships: Vec<RoleMembership>,
/// A stack holding contexts of outer queries when binding a subquery.
/// It also holds all of the lateral contexts for each respective
/// subquery.
Expand Down Expand Up @@ -252,6 +255,9 @@ impl Binder {
session_id: session.id(),
context: BindContext::new(),
auth_context: session.auth_context(),
role_memberships: role_memberships_snapshot(
session.env().role_membership_info_reader(),
),
upper_subquery_contexts: vec![],
lateral_contexts: vec![],
next_subquery_id: 0,
Expand Down
38 changes: 25 additions & 13 deletions src/frontend/src/binder/relation/table_or_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::catalog::{CatalogError, DatabaseId, IndexCatalog, TableId};
use crate::error::ErrorCode::PermissionDenied;
use crate::error::{ErrorCode, Result, RwError};
use crate::handler::privilege::ObjectCheckItem;
use crate::user::effective_privilege::catalog_user_has_privilege;

#[derive(Debug, Clone)]
pub struct BoundBaseTable {
Expand Down Expand Up @@ -299,23 +300,34 @@ impl Binder {
.into());
}
if let Some(user) = self.user.get_user_by_name(&self.auth_context.user_name) {
if user.is_super || user.id == item.owner {
return Ok(());
}
if !user.has_privilege(item.object, item.mode) {
if !catalog_user_has_privilege(
&self.user,
user,
&self.role_memberships,
item.owner,
item.object,
item.mode,
) {
return Err(PermissionDenied(item.error_message()).into());
}

// check CONNECT privilege for cross-db access
if self.database_id != database_id
&& !user.has_privilege(database_id, AclMode::Connect)
{
let db_name = self.catalog.get_database_by_id(database_id)?.name.clone();

return Err(PermissionDenied(format!(
"permission denied for database \"{db_name}\""
))
.into());
if self.database_id != database_id {
let db = self.catalog.get_database_by_id(database_id)?;
if !catalog_user_has_privilege(
&self.user,
user,
&self.role_memberships,
db.owner,
database_id,
AclMode::Connect,
) {
return Err(PermissionDenied(format!(
"permission denied for database \"{}\"",
db.name
))
.into());
}
}
} else {
return Err(PermissionDenied("Session user is invalid".to_owned()).into());
Expand Down
28 changes: 17 additions & 11 deletions src/frontend/src/handler/alter_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,22 @@ fn alter_prost_user_info(
user_info.can_create_db = false;
update_fields.insert(UpdateField::CreateDb);
}
UserOption::CreateUser => {
UserOption::CreateRole | UserOption::CreateUser => {
user_info.can_create_user = true;
update_fields.insert(UpdateField::CreateUser);
}
UserOption::NoCreateUser => {
UserOption::NoCreateRole | UserOption::NoCreateUser => {
user_info.can_create_user = false;
update_fields.insert(UpdateField::CreateUser);
}
UserOption::Inherit => {
user_info.can_inherit = true;
update_fields.insert(UpdateField::Inherit);
}
UserOption::NoInherit => {
user_info.can_inherit = false;
update_fields.insert(UpdateField::Inherit);
}
UserOption::Login => {
user_info.can_login = true;
update_fields.insert(UpdateField::Login);
Expand All @@ -119,15 +127,6 @@ fn alter_prost_user_info(
user_info.is_admin = false;
update_fields.insert(UpdateField::Admin);
}
UserOption::CreateRole
| UserOption::NoCreateRole
| UserOption::Inherit
| UserOption::NoInherit => {
return Err(ErrorCode::InvalidParameterValue(
"role options are not supported yet".to_owned(),
)
.into());
}
UserOption::EncryptedPassword(p) => {
if !p.0.is_empty() {
user_info.auth_info = encrypted_password(&user_info.name, &p.0);
Expand Down Expand Up @@ -253,6 +252,13 @@ pub async fn handle_alter_user(
Ok(response_builder.into())
}

pub async fn handle_alter_role(
handler_args: HandlerArgs,
stmt: AlterUserStatement,
) -> Result<RwPgResponse> {
handle_alter_user(handler_args, stmt).await
}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
Expand Down
32 changes: 21 additions & 11 deletions src/frontend/src/handler/create_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,16 @@ pub async fn handle_create_user(
UserOption::NoSuperUser => user_info.is_super = false,
UserOption::CreateDB => user_info.can_create_db = true,
UserOption::NoCreateDB => user_info.can_create_db = false,
UserOption::CreateUser => user_info.can_create_user = true,
UserOption::NoCreateUser => user_info.can_create_user = false,
UserOption::CreateRole | UserOption::CreateUser => user_info.can_create_user = true,
UserOption::NoCreateRole | UserOption::NoCreateUser => {
user_info.can_create_user = false
}
UserOption::Inherit => user_info.can_inherit = true,
UserOption::NoInherit => user_info.can_inherit = false,
UserOption::Login => user_info.can_login = true,
UserOption::NoLogin => user_info.can_login = false,
UserOption::Admin => user_info.is_admin = true,
UserOption::NoAdmin => user_info.is_admin = false,
UserOption::CreateRole
| UserOption::NoCreateRole
| UserOption::Inherit
| UserOption::NoInherit => {
return Err(ErrorCode::InvalidParameterValue(
"role options are not supported yet".to_owned(),
)
.into());
}
UserOption::EncryptedPassword(password) => {
if !password.0.is_empty() {
user_info.auth_info = encrypted_password(&user_info.name, &password.0);
Expand Down Expand Up @@ -168,6 +163,21 @@ pub async fn handle_create_user(
Ok(response_builder.into())
}

pub async fn handle_create_role(
handler_args: HandlerArgs,
mut stmt: CreateUserStatement,
) -> Result<RwPgResponse> {
let has_explicit_login = stmt
.with_options
.0
.iter()
.any(|option| matches!(option, UserOption::Login | UserOption::NoLogin));
if !has_explicit_login {
stmt.with_options.0.push(UserOption::NoLogin);
}
handle_create_user(handler_args, stmt).await
}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
Expand Down
Loading
Loading