Merge pull request #5588 from chowc/bugfix-denyrootloginoutsidelocalh…

…ost-5555

fix(query): Deny the root login from others host

common/meta/types/src/user_identity.rs

@@ -31,6 +31,10 @@ impl UserIdentity {
     pub fn is_localhost(&self) -> bool {
         &self.hostname.to_lowercase() == "localhost" || &self.hostname == "127.0.0.1"
     }
+
+    pub fn is_root(&self) -> bool {
+        self.username.eq("root") || self.username.eq("default")
+    }
 }
 
 impl fmt::Display for UserIdentity {

query/src/servers/http/middleware.rs

@@ -74,6 +74,7 @@ fn get_credential(req: &Request) -> Result<Credential> {
         match Bearer::decode(value) {
             Some(bearer) => Ok(Credential::Jwt {
                 token: bearer.token().to_string(),
+                hostname: client_ip,
             }),
             None => Err(ErrorCode::AuthenticateFailure("bad Bearer auth header")),
         }

query/src/users/auth/auth_mgr.rs

@@ -17,7 +17,6 @@ use std::sync::Arc;
 use common_exception::ErrorCode;
 use common_exception::Result;
 use common_meta_types::AuthInfo;
-use common_meta_types::UserIdentity;
 use common_meta_types::UserInfo;
 
 use crate::users::auth::jwt::JwtAuthenticator;
@@ -33,6 +32,7 @@ pub struct AuthMgr {
 pub enum Credential {
     Jwt {
         token: String,
+        hostname: Option<String>,
     },
     Password {
         name: String,
@@ -52,7 +52,10 @@ impl AuthMgr {
 
     pub async fn auth(&self, credential: &Credential) -> Result<(Option<String>, UserInfo)> {
         match credential {
-            Credential::Jwt { token: t } => {
+            Credential::Jwt {
+                token: t,
+                hostname: h,
+            } => {
                 let jwt = match &self.jwt {
                     Some(j) => j.parse_jwt(t.as_str()).await?,
                     None => return Err(ErrorCode::AuthenticateFailure("jwt auth not configured.")),
@@ -71,14 +74,19 @@ impl AuthMgr {
                             user_info.grants.grant_role(role);
                         }
                     }
-                    self.user_mgr.add_user(&tenant, user_info, true).await?;
-                }
-                Ok((
-                    Some(tenant.clone()),
                     self.user_mgr
-                        .get_user(&tenant, UserIdentity::new(user_name, "%"))
-                        .await?,
-                ))
+                        .add_user(&tenant, user_info.clone(), true)
+                        .await?;
+                }
+                let user = self
+                    .user_mgr
+                    .get_user_with_client_ip(
+                        &tenant,
+                        user_name,
+                        h.as_ref().unwrap_or(&"%".to_string()),
+                    )
+                    .await?;
+                Ok((Some(tenant.clone()), user))
             }
             Credential::Password {
                 name: n,

query/src/users/user_mgr.rs

@@ -26,24 +26,25 @@ use crate::users::UserApiProvider;
 impl UserApiProvider {
     // Get one user from by tenant.
     pub async fn get_user(&self, tenant: &str, user: UserIdentity) -> Result<UserInfo> {
-        match user.username.as_str() {
-            // TODO(BohuTANG): Mock, need removed.
-            "default" | "" | "root" => {
-                let mut user_info = UserInfo::new_no_auth(&user.username, &user.hostname);
-                if user.is_localhost() {
-                    user_info.grants.grant_privileges(
-                        &GrantObject::Global,
-                        UserPrivilegeSet::available_privileges_on_global(),
-                    );
-                    user_info.option.set_all_flag();
-                }
-                Ok(user_info)
-            }
-            _ => {
-                let client = self.get_user_api_client(tenant)?;
-                let get_user = client.get_user(user, None);
-                Ok(get_user.await?.data)
+        if user.is_root() {
+            let mut user_info = UserInfo::new_no_auth(&user.username, &user.hostname);
+            if user.is_localhost() {
+                user_info.grants.grant_privileges(
+                    &GrantObject::Global,
+                    UserPrivilegeSet::available_privileges_on_global(),
+                );
+                user_info.option.set_all_flag();
+            } else {
+                return Err(ErrorCode::UnknownUser(format!(
+                    "only accept root from localhost '{}'@'{}'",
+                    user.username, user.hostname
+                )));
             }
+            Ok(user_info)
+        } else {
+            let client = self.get_user_api_client(tenant)?;
+            let get_user = client.get_user(user, None);
+            Ok(get_user.await?.data)
         }
     }
 
@@ -60,7 +61,7 @@ impl UserApiProvider {
             .await
             .map(Some)
             .or_else(|e| {
-                if e.code() == ErrorCode::unknown_user_code() {
+                if e.code() == ErrorCode::unknown_user_code() && !client_ip.eq("%") {
                     Ok(None)
                 } else {
                     Err(e)

query/tests/it/servers/http/http_query_handlers.rs

@@ -663,7 +663,7 @@ async fn test_auth_basic() -> Result<()> {
     post_sql_to_endpoint(&ep, &sql, 1).await?;
 
     let basic = headers::Authorization::basic(user_name, password);
-    test_auth_post(&ep, user_name, basic).await?;
+    test_auth_post(&ep, user_name, basic, &"%").await?;
     Ok(())
 }
 
@@ -715,11 +715,17 @@ async fn test_auth_jwt() -> Result<()> {
 
     let token = key_pair.sign(claims)?;
     let bear = headers::Authorization::bearer(&token).unwrap();
-    test_auth_post(&ep, user_name, bear).await?;
+    // root user can only login in localhost
+    test_auth_post(&ep, user_name, bear, "127.0.0.1").await?;
     Ok(())
 }
 
-async fn test_auth_post(ep: &EndpointType, user_name: &str, header: impl Header) -> Result<()> {
+async fn test_auth_post(
+    ep: &EndpointType,
+    user_name: &str,
+    header: impl Header,
+    host: &str,
+) -> Result<()> {
     let sql = "select current_user()";
 
     let json = serde_json::json!({"sql": sql.to_string()});
@@ -747,7 +753,7 @@ async fn test_auth_post(ep: &EndpointType, user_name: &str, header: impl Header)
     assert_eq!(v[0].len(), 1);
     assert_eq!(
         v[0][0],
-        serde_json::Value::String(format!("'{}'@'%'", user_name))
+        serde_json::Value::String(format!("'{}'@'{}'", user_name, host))
     );
     Ok(())
 }
@@ -804,7 +810,7 @@ async fn test_auth_jwt_with_create_user() -> Result<()> {
 
     let token = key_pair.sign(claims)?;
     let bear = headers::Authorization::bearer(&token).unwrap();
-    test_auth_post(&ep, user_name, bear).await?;
+    test_auth_post(&ep, user_name, bear, &"%").await?;
     Ok(())
 }
 

query/tests/it/servers/mysql/mysql_handler.rs

@@ -140,7 +140,7 @@ async fn test_rejected_session_with_parallel() -> Result<()> {
 }
 
 async fn create_connection(port: u16) -> Result<mysql_async::Conn> {
-    let uri = &format!("mysql://127.0.0.1:{}", port);
+    let uri = &format!("mysql://root@127.0.0.1:{}", port);
     let opts = mysql_async::Opts::from_url(uri).unwrap();
     mysql_async::Conn::new(opts)
         .await

query/tests/it/users/auth/auth_mgr.rs

@@ -64,7 +64,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
         let claims = Claims::create(Duration::from_hours(2));
         let token = key_pair.sign(claims)?;
 
-        let res = auth_mgr.auth(&Credential::Jwt { token }).await;
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await;
         assert!(res.is_err());
         assert_eq!(
             "Code: 1051, displayText = missing field `subject` in jwt.",
@@ -77,7 +82,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
         let claims = Claims::create(Duration::from_hours(2)).with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let res = auth_mgr.auth(&Credential::Jwt { token }).await;
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await;
         assert!(res.is_err());
         assert_eq!(
             "Code: 2201, displayText = unknown user 'test'@'%'.",
@@ -92,7 +102,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
             .with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let res = auth_mgr.auth(&Credential::Jwt { token }).await;
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await;
         assert!(res.is_err());
         assert_eq!(
             "Code: 2201, displayText = unknown user 'test'@'%'.",
@@ -110,7 +125,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
             .with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let (currnet_tenant, user_info) = auth_mgr.auth(&Credential::Jwt { token }).await?;
+        let (currnet_tenant, user_info) = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await?;
         assert!(currnet_tenant.is_some());
         assert_eq!(currnet_tenant.unwrap(), tenant.to_string());
         assert_eq!(user_info.grants.roles().len(), 0);
@@ -123,7 +143,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
             .with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let (_, user_info) = auth_mgr.auth(&Credential::Jwt { token }).await?;
+        let (_, user_info) = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await?;
         assert_eq!(user_info.grants.roles().len(), 0);
     }
 
@@ -136,7 +161,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
             .with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let (_, user_info) = auth_mgr.auth(&Credential::Jwt { token }).await?;
+        let (_, user_info) = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await?;
         assert_eq!(user_info.grants.roles().len(), 0);
     }
 
@@ -151,7 +181,12 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
             .with_subject(user_name.to_string());
         let token = key_pair.sign(claims)?;
 
-        let res = auth_mgr.auth(&Credential::Jwt { token }).await;
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: None,
+            })
+            .await;
         assert!(res.is_ok());
 
         let user_info = user_mgr
@@ -161,5 +196,38 @@ async fn test_auth_mgr_with_jwt() -> Result<()> {
         assert_eq!(user_info.grants.roles()[0], role_name.to_string());
     }
 
+    // root auth from localhost
+    {
+        let user_name = "root";
+
+        let claims = Claims::create(Duration::from_hours(2)).with_subject(user_name.to_string());
+        let token = key_pair.sign(claims)?;
+
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: Some("localhost".to_string()),
+            })
+            .await;
+        assert!(res.is_ok());
+    }
+
+    // root auth outside localhost
+    {
+        let claims = Claims::create(Duration::from_hours(2)).with_subject("root".to_string());
+        let token = key_pair.sign(claims)?;
+
+        let res = auth_mgr
+            .auth(&Credential::Jwt {
+                token,
+                hostname: Some("10.0.0.1".to_string()),
+            })
+            .await;
+        assert!(res.is_err());
+        assert_eq!(
+            "Code: 2201, displayText = only accept root from localhost 'root'@'%'.",
+            res.err().unwrap().to_string()
+        );
+    }
     Ok(())
 }

query/tests/it/users/user_mgr.rs

@@ -295,14 +295,16 @@ async fn test_user_manager_with_root_user() -> Result<()> {
             .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super));
     }
 
-    // Get user via username `default` and hostname `otherhost`.
+    // Get user via username `default` and hostname `otherhost` will be denied.
     {
-        let user = user_mgr
+        let res = user_mgr
             .get_user(tenant, UserIdentity::new(username1, hostname3))
-            .await?;
-        assert_eq!(user.name, username1);
-        assert_eq!(user.hostname, hostname3);
-        assert!(user.grants.entries().is_empty());
+            .await;
+        assert!(res.is_err());
+        assert_eq!(
+            "Code: 2201, displayText = only accept root from localhost 'default'@'otherhost'.",
+            res.err().unwrap().to_string()
+        );
     }
 
     // Get user via username `root` and hostname `127.0.0.1`.
@@ -347,14 +349,16 @@ async fn test_user_manager_with_root_user() -> Result<()> {
             .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super));
     }
 
-    // Get user via username `root` and hostname `otherhost`.
+    // Get user via username `root` and hostname `otherhost` will be denied.
     {
-        let user = user_mgr
+        let res = user_mgr
             .get_user(tenant, UserIdentity::new(username2, hostname3))
-            .await?;
-        assert_eq!(user.name, username2);
-        assert_eq!(user.hostname, hostname3);
-        assert!(user.grants.entries().is_empty());
+            .await;
+        assert!(res.is_err());
+        assert_eq!(
+            "Code: 2201, displayText = only accept root from localhost 'root'@'otherhost'.",
+            res.err().unwrap().to_string()
+        );
     }
 
     Ok(())