From 3cc5832bad03d6d41324bedef53f979d2e2f4033 Mon Sep 17 00:00:00 2001 From: taichong Date: Sun, 4 Feb 2024 16:44:15 +0800 Subject: [PATCH] fix(query): after drop role account_admin should support grant ownership to new role 1. if has ownership but owner role not exists, the owner role will be set to account_admin 2. has_ownership should get ownership and role from meta --- .../interpreter_privilege_grant.rs | 10 +++++++- .../src/sessions/session_privilege_mgr.rs | 19 ++++++++++----- .../18_rbac/18_0002_ownership_cover.result | 5 ++++ .../18_rbac/18_0002_ownership_cover.sh | 23 +++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/query/service/src/interpreters/interpreter_privilege_grant.rs b/src/query/service/src/interpreters/interpreter_privilege_grant.rs index 6cc1d6f17aa9..61678a92f680 100644 --- a/src/query/service/src/interpreters/interpreter_privilege_grant.rs +++ b/src/query/service/src/interpreters/interpreter_privilege_grant.rs @@ -24,6 +24,7 @@ use databend_common_meta_app::principal::UserPrivilegeType::Ownership; use databend_common_sql::plans::GrantPrivilegePlan; use databend_common_users::RoleCacheManager; use databend_common_users::UserApiProvider; +use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN; use log::debug; use log::error; use log::info; @@ -136,7 +137,14 @@ impl GrantPrivilegeInterpreter { // if the object's owner is None, it's considered as PUBLIC, everyone could access it let owner = user_mgr.get_ownership(tenant, owner_object).await?; if let Some(owner) = owner { - let can_grant_ownership = available_roles.iter().any(|r| r.name == owner.role); + // if object has ownership, but the owner role is not exists, set owner role to ACCOUNT_ADMIN, + // only account_admin can grant this object. + let role = if !user_mgr.exists_role(tenant, owner.role.clone()).await? { + BUILTIN_ROLE_ACCOUNT_ADMIN.to_string() + } else { + owner.role.clone() + }; + let can_grant_ownership = available_roles.iter().any(|r| r.name == role); log_msg = format!( "{}: grant ownership on {:?} from role {} to {}", ctx.get_id(), diff --git a/src/query/service/src/sessions/session_privilege_mgr.rs b/src/query/service/src/sessions/session_privilege_mgr.rs index d37fb78fed9c..1053edb26dff 100644 --- a/src/query/service/src/sessions/session_privilege_mgr.rs +++ b/src/query/service/src/sessions/session_privilege_mgr.rs @@ -23,6 +23,7 @@ use databend_common_meta_app::principal::UserInfo; use databend_common_meta_app::principal::UserPrivilegeType; use databend_common_users::GrantObjectVisibilityChecker; use databend_common_users::RoleCacheManager; +use databend_common_users::UserApiProvider; use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN; use databend_common_users::BUILTIN_ROLE_PUBLIC; @@ -267,14 +268,20 @@ impl SessionPrivilegeManager for SessionPrivilegeManagerImpl { #[async_backtrace::framed] async fn has_ownership(&self, object: &OwnershipObject) -> Result { - let role_mgr = RoleCacheManager::instance(); + let user_mgr = UserApiProvider::instance(); let tenant = self.session_ctx.get_current_tenant(); - - // if the object is not owned by any role, then considered as ACCOUNT_ADMIN, the normal users - // can not access it unless ACCOUNT_ADMIN grant relevant privileges to them. - let owner_role_name = match role_mgr.find_object_owner(&tenant, object).await? { - Some(owner_role) => owner_role.name, + let owner_role_name = match user_mgr.get_ownership(&tenant, object).await? { None => BUILTIN_ROLE_ACCOUNT_ADMIN.to_string(), + Some(owner) => match user_mgr.get_role(&tenant, owner.role).await { + Ok(owner_role) => owner_role.name, + Err(e) => { + if e.code() == ErrorCode::UNKNOWN_ROLE { + BUILTIN_ROLE_ACCOUNT_ADMIN.to_string() + } else { + return Err(e); + } + } + }, }; let effective_roles = self.get_all_effective_roles().await?; diff --git a/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.result b/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.result index 2c72afc3f894..a07db5162c01 100644 --- a/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.result +++ b/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.result @@ -37,3 +37,8 @@ t1 Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for database 'db_a' t t1 +=== fix_issue_14572: test drop role; grant ownership === +a drop_role +a drop_role +a drop_role1 +GRANT OWNERSHIP ON 'default'.'a'.* TO ROLE `drop_role1` diff --git a/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.sh b/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.sh index 3200ee75e482..2e827bb1d231 100755 --- a/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.sh +++ b/tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.sh @@ -126,3 +126,26 @@ echo "drop database db_a;" | $BENDSQL_CLIENT_CONNECT echo "drop role if exists role1;" | $BENDSQL_CLIENT_CONNECT echo "drop user if exists a;" | $BENDSQL_CLIENT_CONNECT echo "drop user if exists b;" | $BENDSQL_CLIENT_CONNECT + +echo "=== fix_issue_14572: test drop role; grant ownership ===" +echo "drop role if exists drop_role;" | $BENDSQL_CLIENT_CONNECT +echo "drop role if exists drop_role1;" | $BENDSQL_CLIENT_CONNECT +echo "drop user if exists u1;" | $BENDSQL_CLIENT_CONNECT +echo "drop database if exists a;" | $BENDSQL_CLIENT_CONNECT +echo "create role drop_role;" | $BENDSQL_CLIENT_CONNECT +echo "create role drop_role1;" | $BENDSQL_CLIENT_CONNECT +echo "create user u1 identified by '123' with DEFAULT_ROLE='drop_role'" | $BENDSQL_CLIENT_CONNECT +echo "grant role drop_role to u1;" | $BENDSQL_CLIENT_CONNECT +echo "grant create on *.* to u1;" | $BENDSQL_CLIENT_CONNECT +export USER_U1_CONNECT="bendsql --user=u1 --password=123 --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" + +echo "create database a" | $USER_U1_CONNECT +echo "select name, owner from system.databases where name='a'" | $USER_U1_CONNECT +echo "drop role drop_role;" | $BENDSQL_CLIENT_CONNECT +echo "select name, owner from system.databases where name='a'" | $BENDSQL_CLIENT_CONNECT +echo "grant ownership on a.* to role drop_role1;" | $BENDSQL_CLIENT_CONNECT +echo "select name, owner from system.databases where name='a'" | $BENDSQL_CLIENT_CONNECT +echo "show grants for role drop_role1" | $BENDSQL_CLIENT_CONNECT +echo "drop role drop_role1" | $BENDSQL_CLIENT_CONNECT +echo "drop user u1" | $BENDSQL_CLIENT_CONNECT +echo "drop database a" | $BENDSQL_CLIENT_CONNECT