Revert "feat(rbac): ensure the builtin roles on auth" (#14497)

src/meta/app/src/principal/user_grant.rs

@@ -232,16 +232,6 @@ impl UserGrantSet {
         self.roles.remove(role);
     }
 
-    pub fn find_object_granted_privileges(&self, object: &GrantObject) -> UserPrivilegeSet {
-        let mut privileges = UserPrivilegeSet::empty();
-        for entry in self.entries.iter() {
-            if entry.matches_entry(object) {
-                privileges |= entry.privileges.into();
-            }
-        }
-        privileges
-    }
-
     pub fn verify_privilege(
         &self,
         object: &GrantObject,

src/meta/app/tests/it/user_grant.rs

@@ -185,9 +185,6 @@ fn test_user_grant_entry() -> Result<()> {
 fn test_user_grant_set() -> Result<()> {
     let mut grants = UserGrantSet::empty();
 
-    // GRANT CREATE ON *.* TO 'user1';
-    // GRANT INSERT ON *.* TO 'user1';
-    // GRANT SELECT, CREATE ON db1.table1 TO 'user1';
     grants.grant_privileges(
         &GrantObject::Global,
         make_bitflags!(UserPrivilegeType::{Create}).into(),
@@ -206,7 +203,6 @@ fn test_user_grant_set() -> Result<()> {
     );
     assert_eq!(2, grants.entries().len());
 
-    // REVOKE INSERT ON *.* FROM 'user1';
     grants.revoke_privileges(
         &GrantObject::Global,
         make_bitflags!(UserPrivilegeType::{Insert}).into(),

src/query/management/src/role/role_api.rs

@@ -36,7 +36,7 @@ pub trait RoleApi: Sync + Send {
     ///
     /// Seq number ensures there is no other write happens between get and set.
     #[allow(clippy::ptr_arg)]
-    async fn update_role_with<F>(&self, role: &str, seq: MatchSeq, f: F) -> Result<Option<u64>>
+    async fn update_role_with<F>(&self, role: &String, seq: MatchSeq, f: F) -> Result<Option<u64>>
     where F: FnOnce(&mut RoleInfo) + Send;
 
     /// Grant ownership would transfer ownership of a object from one role to another role

src/query/management/src/role/role_mgr.rs

@@ -249,7 +249,7 @@ impl RoleApi for RoleMgr {
     #[minitrace::trace]
     async fn update_role_with<F>(
         &self,
-        role: &str,
+        role: &String,
         seq: MatchSeq,
         f: F,
     ) -> Result<Option<u64>, ErrorCode>
@@ -260,7 +260,7 @@ impl RoleApi for RoleMgr {
             seq,
             data: mut role_info,
             ..
-        } = self.get_role(&role.to_string(), seq).await?;
+        } = self.get_role(role, seq).await?;
 
         f(&mut role_info);
 

src/query/service/src/auth.rs

@@ -121,9 +121,6 @@ impl AuthMgr {
                     }
                 };
 
-                UserApiProvider::instance()
-                    .ensure_builtin_roles(&tenant)
-                    .await?;
                 session.set_authed_user(user, jwt.custom.role).await?;
             }
             Credential::Password {
@@ -164,13 +161,9 @@ impl AuthMgr {
 
                 authed?;
 
-                UserApiProvider::instance()
-                    .ensure_builtin_roles(&tenant)
-                    .await?;
                 session.set_authed_user(user, None).await?;
             }
         };
-
         Ok(())
     }
 }

src/query/service/src/interpreters/interpreter_role_grant.rs

@@ -56,7 +56,7 @@ impl Interpreter for GrantRoleInterpreter {
         // TODO: check privileges
 
         // Check if the grant role exists.
-        user_mgr.get_role(&tenant, &plan.role).await?;
+        user_mgr.get_role(&tenant, plan.role.clone()).await?;
         match plan.principal {
             PrincipalIdentity::User(user) => {
                 user_mgr

src/query/service/src/interpreters/interpreter_setting.rs

@@ -15,12 +15,9 @@
 use std::sync::Arc;
 
 use chrono_tz::Tz;
-use databend_common_config::GlobalConfig;
 use databend_common_exception::ErrorCode;
 use databend_common_exception::Result;
 use databend_common_sql::plans::SettingPlan;
-use databend_common_sql::plans::VarValue;
-use databend_common_users::UserApiProvider;
 
 use crate::interpreters::Interpreter;
 use crate::pipelines::PipelineBuildResult;
@@ -37,19 +34,6 @@ impl SettingInterpreter {
     pub fn try_create(ctx: Arc<QueryContext>, set: SettingPlan) -> Result<Self> {
         Ok(SettingInterpreter { ctx, set })
     }
-
-    async fn set_setting_by_var(&self, var: &VarValue, value: String) -> Result<()> {
-        let settings = self.ctx.get_shared_settings();
-
-        match var.is_global {
-            true => {
-                settings
-                    .set_global_setting(var.variable.clone(), value)
-                    .await
-            }
-            false => settings.set_setting(var.variable.clone(), value).await,
-        }
-    }
 }
 
 #[async_trait::async_trait]
@@ -74,26 +58,39 @@ impl Interpreter for SettingInterpreter {
                     let _ = tz.parse::<Tz>().map_err(|_| {
                         ErrorCode::InvalidTimezone(format!("Invalid Timezone: {}", var.value))
                     })?;
+                    let settings = self.ctx.get_shared_settings();
 
-                    self.set_setting_by_var(&var, tz.to_string()).await?;
-                    true
-                }
-                "sandbox_tenant" => {
-                    // only used in sqlogictest, it will create a sandbox tenant on every sqlogictest cases
-                    // and switch to it by SET sandbox_tenant = xxx;
-                    let config = GlobalConfig::instance();
-                    let tenant = var.value.clone();
-                    if config.query.internal_enable_sandbox_tenant && !tenant.is_empty() {
-                        UserApiProvider::instance()
-                            .ensure_builtin_roles(&tenant)
-                            .await?;
-                    }
+                    match var.is_global {
+                        true => {
+                            settings
+                                .set_global_setting(var.variable.clone(), tz.to_string())
+                                .await
+                        }
+                        false => {
+                            settings
+                                .set_setting(var.variable.clone(), tz.to_string())
+                                .await
+                        }
+                    }?;
 
-                    self.set_setting_by_var(&var, var.value.clone()).await?;
                     true
                 }
                 _ => {
-                    self.set_setting_by_var(&var, var.value.clone()).await?;
+                    let settings = self.ctx.get_shared_settings();
+
+                    match var.is_global {
+                        true => {
+                            settings
+                                .set_global_setting(var.variable.clone(), var.value.clone())
+                                .await
+                        }
+                        false => {
+                            settings
+                                .set_setting(var.variable.clone(), var.value.clone())
+                                .await
+                        }
+                    }?;
+
                     true
                 }
             };

src/query/service/src/interpreters/interpreter_show_grants.rs

@@ -65,7 +65,9 @@ impl Interpreter for ShowGrantsInterpreter {
                     (user.identity().to_string(), user.grants)
                 }
                 PrincipalIdentity::Role(role) => {
-                    let role = UserApiProvider::instance().get_role(&tenant, role).await?;
+                    let role = UserApiProvider::instance()
+                        .get_role(&tenant, role.clone())
+                        .await?;
                     (format!("ROLE `{}`", role.identity()), role.grants)
                 }
             },

src/query/settings/src/settings_getter_setter.rs

@@ -13,9 +13,11 @@
 // limitations under the License.
 
 use databend_common_ast::Dialect;
+use databend_common_config::GlobalConfig;
 use databend_common_exception::ErrorCode;
 use databend_common_exception::Result;
 use databend_common_meta_app::principal::UserSettingValue;
+use databend_common_users::UserApiProvider;
 
 use crate::settings::Settings;
 use crate::settings_default::DefaultSettings;
@@ -117,7 +119,12 @@ impl Settings {
         let (key, value) = DefaultSettings::convert_value(k.clone(), v)?;
 
         if key == "sandbox_tenant" {
-            log::info!("switch sandbox tenant to {}", value);
+            let config = GlobalConfig::instance();
+            let tenant = value.as_string();
+            if config.query.internal_enable_sandbox_tenant && !tenant.is_empty() {
+                UserApiProvider::try_create_simple(config.meta.to_meta_grpc_client_conf(), &tenant)
+                    .await?;
+            }
         }
 
         self.changes.insert(key, ChangeValue {

src/query/users/src/role_cache_mgr.rs

@@ -125,7 +125,7 @@ impl RoleCacheManager {
         };
 
         // cache manager would not look into built-in roles.
-        let role = self.user_manager.get_role(tenant, &owner.role).await?;
+        let role = self.user_manager.get_role(tenant, owner.role).await?;
         Ok(Some(role))
     }
 

src/query/users/src/role_mgr.rs

@@ -23,7 +23,6 @@ use databend_common_meta_app::principal::OwnershipObject;
 use databend_common_meta_app::principal::RoleInfo;
 use databend_common_meta_app::principal::UserPrivilegeSet;
 use databend_common_meta_types::MatchSeq;
-use log::info;
 
 use crate::role_util::find_all_related_roles;
 use crate::UserApiProvider;
@@ -34,29 +33,12 @@ pub const BUILTIN_ROLE_PUBLIC: &str = "public";
 impl UserApiProvider {
     // Get one role from by tenant.
     #[async_backtrace::framed]
-    pub async fn get_role(&self, tenant: &str, role: &str) -> Result<RoleInfo> {
+    pub async fn get_role(&self, tenant: &str, role: String) -> Result<RoleInfo> {
         let client = self.get_role_api_client(tenant)?;
-        let role_data = client
-            .get_role(&role.to_string(), MatchSeq::GE(0))
-            .await?
-            .data;
+        let role_data = client.get_role(&role, MatchSeq::GE(0)).await?.data;
         Ok(role_data)
     }
 
-    #[async_backtrace::framed]
-    pub async fn get_role_optional(&self, tenant: &str, role: &str) -> Result<Option<RoleInfo>> {
-        match self.get_role(tenant, role).await {
-            Ok(r) => Ok(Some(r)),
-            Err(err) => {
-                if err.code() == ErrorCode::UNKNOWN_ROLE {
-                    Ok(None)
-                } else {
-                    return Err(err);
-                }
-            }
-        }
-    }
-
     // Get the tenant all roles list.
     #[async_backtrace::framed]
     pub async fn get_roles(&self, tenant: &str) -> Result<Vec<RoleInfo>> {
@@ -70,55 +52,6 @@ impl UserApiProvider {
         Ok(roles)
     }
 
-    /// Ensure the builtin roles account_admin and public exists. Please note that there's
-    /// a corner case that if we added another privilege type, we should add it to the
-    /// existed account_admin role.
-    ///
-    /// This function have two calling places:
-    /// 1. when the server starts
-    /// 2. when the user is authenticated
-    #[async_backtrace::framed]
-    pub async fn ensure_builtin_roles(&self, tenant: &str) -> Result<()> {
-        // CREATE ROLE IF NOT EXISTS public;
-        let public = RoleInfo::new(BUILTIN_ROLE_PUBLIC);
-        self.add_role(tenant, public, true).await?;
-
-        // if not exists, create account_admin.
-        // if new privilege type on Global added, grant it to account_admin.
-        let existed_account_admin = self
-            .get_role_optional(tenant, BUILTIN_ROLE_ACCOUNT_ADMIN)
-            .await?;
-        if existed_account_admin.is_none() {
-            let account_admin = {
-                let mut r = RoleInfo::new(BUILTIN_ROLE_ACCOUNT_ADMIN);
-                r.grants.grant_privileges(
-                    &GrantObject::Global,
-                    UserPrivilegeSet::available_privileges_on_global(),
-                );
-                r
-            };
-            self.add_role(tenant, account_admin, true).await?;
-        } else if existed_account_admin
-            .unwrap()
-            .grants
-            .find_object_granted_privileges(&GrantObject::Global)
-            != UserPrivilegeSet::available_privileges_on_global()
-        {
-            info!(
-                "new privilege type on GrantObject::Global detected, sync it to role account_admin"
-            );
-            self.grant_privileges_to_role(
-                tenant,
-                BUILTIN_ROLE_ACCOUNT_ADMIN,
-                GrantObject::Global,
-                UserPrivilegeSet::available_privileges_on_global(),
-            )
-            .await?;
-        }
-
-        Ok(())
-    }
-
     #[async_backtrace::framed]
     pub async fn get_ownerships(&self, tenant: &str) -> Result<HashMap<OwnershipObject, String>> {
         let seq_owns = self
@@ -135,7 +68,7 @@ impl UserApiProvider {
     }
 
     #[async_backtrace::framed]
-    pub async fn exists_role(&self, tenant: &str, role: &str) -> Result<bool> {
+    pub async fn exists_role(&self, tenant: &str, role: String) -> Result<bool> {
         match self.get_role(tenant, role).await {
             Ok(_) => Ok(true),
             Err(e) => {
@@ -156,7 +89,7 @@ impl UserApiProvider {
         role_info: RoleInfo,
         if_not_exists: bool,
     ) -> Result<u64> {
-        if if_not_exists && self.exists_role(tenant, &role_info.name).await? {
+        if if_not_exists && self.exists_role(tenant, role_info.name.clone()).await? {
             return Ok(0);
         }
 
@@ -182,7 +115,7 @@ impl UserApiProvider {
         new_role: &str,
     ) -> Result<()> {
         // from and to role must exists
-        self.get_role(tenant, new_role).await?;
+        self.get_role(tenant, new_role.to_string()).await?;
 
         let client = self.get_role_api_client(tenant)?;
         client
@@ -209,7 +142,7 @@ impl UserApiProvider {
     pub async fn grant_privileges_to_role(
         &self,
         tenant: &str,
-        role: &str,
+        role: &String,
         object: GrantObject,
         privileges: UserPrivilegeSet,
     ) -> Result<Option<u64>> {
@@ -226,7 +159,7 @@ impl UserApiProvider {
     pub async fn revoke_privileges_from_role(
         &self,
         tenant: &str,
-        role: &str,
+        role: &String,
         object: GrantObject,
         privileges: UserPrivilegeSet,
     ) -> Result<Option<u64>> {
@@ -271,7 +204,7 @@ impl UserApiProvider {
     pub async fn revoke_role_from_role(
         &self,
         tenant: &str,
-        role: &str,
+        role: &String,
         revoke_role: &String,
     ) -> Result<Option<u64>> {
         let client = self.get_role_api_client(tenant)?;