Skip to content

Commit

Permalink
add table test and add some comment about why not modify get_ownerships
Browse files Browse the repository at this point in the history
  • Loading branch information
TCeason committed Feb 6, 2024
1 parent ecbbc26 commit 43455f1
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 5 deletions.
9 changes: 4 additions & 5 deletions src/query/service/src/sessions/session_privilege_mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ 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;

Expand Down Expand Up @@ -270,10 +269,10 @@ impl SessionPrivilegeManager for SessionPrivilegeManagerImpl {
async fn has_ownership(&self, object: &OwnershipObject) -> Result<bool> {
let role_mgr = RoleCacheManager::instance();
let tenant = self.session_ctx.get_current_tenant();
let owner_role_name = match role_mgr.find_object_owner(&tenant, object).await? {
Some(owner_role) => owner_role,
None => BUILTIN_ROLE_ACCOUNT_ADMIN.to_string(),
};
let owner_role_name = role_mgr
.find_object_owner(&tenant, object)
.await?
.unwrap_or_else(|| BUILTIN_ROLE_ACCOUNT_ADMIN.to_string());

let effective_roles = self.get_all_effective_roles().await?;
let exists = effective_roles.iter().any(|r| r.name == owner_role_name);
Expand Down
4 changes: 4 additions & 0 deletions src/query/users/src/role_mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ impl UserApiProvider {
if let Some(owner) = ownership {
// if object has ownership, but the owner role is not exists, set owner role to ACCOUNT_ADMIN,
// only account_admin can access this object.
// Note: get_ownerships no need to do this check.
// Because this can cause system.table queries to slow down
// The intention is that the account admin can grant ownership.
// So system.tables will display dropped role. It's by design.
if !self.exists_role(tenant, owner.role.clone()).await? {
Ok(Some(OwnershipInfo {
role: BUILTIN_ROLE_ACCOUNT_ADMIN.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ t
t1
=== fix_issue_14572: test drop role; grant ownership ===
a drop_role
t drop_role
a account_admin
t drop_role
a drop_role1
t drop_role1
GRANT OWNERSHIP ON 'default'.'a'.* TO ROLE `drop_role1`
GRANT OWNERSHIP ON 'default'.'a'.'t' TO ROLE `drop_role1`
5 changes: 5 additions & 0 deletions tests/suites/0_stateless/18_rbac/18_0002_ownership_cover.sh
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,16 @@ 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 "create table a.t(id int)" | $USER_U1_CONNECT
echo "select name, owner from system.databases where name='a'" | $USER_U1_CONNECT
echo "select name, owner from system.tables where database='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 "select name, owner from system.tables where database='a'" | $USER_U1_CONNECT
echo "grant ownership on a.* to role drop_role1;" | $BENDSQL_CLIENT_CONNECT
echo "grant ownership on a.t to role drop_role1;" | $BENDSQL_CLIENT_CONNECT
echo "select name, owner from system.databases where name='a'" | $BENDSQL_CLIENT_CONNECT
echo "select name, owner from system.tables where database='a'" | $USER_U1_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
Expand Down

0 comments on commit 43455f1

Please sign in to comment.