feat(graphQL): add is_sso_user and forbid update name, password for s… (

#3733) * feat(graphQL): add is_sso_user and forbid update name, password for sso users Signed-off-by: Wei Zhang <[email protected]> * chore: users should return is_sso_user Signed-off-by: Wei Zhang <[email protected]> * [autofix.ci] apply automated fixes * chore: fix tests Signed-off-by: Wei Zhang <[email protected]> --------- Signed-off-by: Wei Zhang <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
TabbyML · Jan 20, 2025 · 059cf2d · 059cf2d
1 parent 3247ee3
commit 059cf2d
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 3 deletions.
diff --git a/ee/tabby-db/src/users.rs b/ee/tabby-db/src/users.rs
@@ -16,6 +16,9 @@ pub struct UserDAO {
     pub id: i64,
     pub email: String,
     pub name: Option<String>,
+
+    // when the user is created with password, this field is set and will never be changed to None
+    // when the user is created with SSO, this field is None and will never be set
     pub password_encrypted: Option<String>,
     pub is_admin: bool,
 

diff --git a/ee/tabby-schema/graphql/schema.graphql b/ee/tabby-schema/graphql/schema.graphql
@@ -296,6 +296,7 @@ interface User {
   isAdmin: Boolean!
   isOwner: Boolean!
   active: Boolean!
+  isSsoUser: Boolean!
 }
 
 """
@@ -965,6 +966,7 @@ type UserSecured implements User {
   active: Boolean!
   authToken: String!
   isPasswordSet: Boolean!
+  isSsoUser: Boolean!
 }
 
 type WebContextSource implements ContextSourceId & ContextSource {

diff --git a/ee/tabby-schema/src/schema/auth.rs b/ee/tabby-schema/src/schema/auth.rs
@@ -198,6 +198,11 @@ pub struct UserSecured {
     pub auth_token: String,
     pub is_password_set: bool,
 
+    // is_sso_user is used to indicate if the user is created by SSO
+    // and should not be able to change Name and Password
+    // e.g. LDAP, OAuth users
+    pub is_sso_user: bool,
+
     #[graphql(skip)]
     pub policy: AccessPolicy,
 }

diff --git a/ee/tabby-schema/src/schema/interface.rs b/ee/tabby-schema/src/schema/interface.rs
@@ -14,6 +14,7 @@ pub struct User {
     pub is_admin: bool,
     pub is_owner: bool,
     pub active: bool,
+    pub is_sso_user: bool,
 }
 
 impl relay::NodeType for UserValue {

diff --git a/ee/tabby-webserver/src/service/auth.rs b/ee/tabby-webserver/src/service/auth.rs
@@ -161,6 +161,11 @@ impl AuthenticationService for AuthenticationServiceImpl {
     }
 
     async fn generate_reset_password_url(&self, id: &ID) -> Result<String> {
+        let user = self.get_user(id).await?;
+        if user.is_sso_user {
+            bail!("Cannot generate reset password url for SSO users");
+        }
+
         let external_url = self.setting.read_network_setting().await?.external_url;
         let id = id.as_rowid()?;
         let user = self.db.get_user(id).await?.context("User doesn't exits")?;
@@ -179,6 +184,10 @@ impl AuthenticationService for AuthenticationServiceImpl {
             return Ok(None);
         };
 
+        if user.is_sso_user {
+            bail!("Cannot request password reset for SSO users");
+        }
+
         let id = user.id.as_rowid()?;
 
         // request_password_reset_email is invoked by the user, so we need to check for existing password reset requests to prevent spamming
@@ -200,6 +209,11 @@ impl AuthenticationService for AuthenticationServiceImpl {
         let password_encrypted = password_hash(password).map_err(|_| anyhow!("Unknown error"))?;
 
         let user_id = self.db.verify_password_reset(code).await?;
+        let user = self.get_user(&user_id.as_id()).await?;
+        if user.is_sso_user {
+            bail!("Password cannot be reset for SSO users");
+        }
+
         let old_pass_encrypted = self
             .db
             .get_user(user_id)
@@ -227,6 +241,11 @@ impl AuthenticationService for AuthenticationServiceImpl {
             bail!("Changing passwords is disabled in demo mode");
         }
 
+        let user = self.get_user(id).await?;
+        if user.is_sso_user {
+            bail!("Password cannot be changed for SSO users");
+        }
+
         let user = self
             .db
             .get_user(id.as_rowid()?)
@@ -280,6 +299,12 @@ impl AuthenticationService for AuthenticationServiceImpl {
         if is_demo_mode() {
             bail!("Changing profile data is disabled in demo mode");
         }
+
+        let user = self.get_user(id).await?;
+        if user.is_sso_user {
+            bail!("Name cannot be changed for SSO users");
+        }
+
         let id = id.as_rowid()?;
         self.db.update_user_name(id, name).await?;
         Ok(())
@@ -1602,19 +1627,24 @@ mod tests {
         let service = test_authentication_service().await;
         let id = service
             .db
-            .create_user("[email protected]".into(), None, true, None)
+            .create_user(
+                "[email protected]".into(),
+                password_hash("pass").ok(),
+                true,
+                None,
+            )
             .await
             .unwrap();
 
         let id = id.as_id();
 
         assert!(service
-            .update_user_password(&id, None, "newpass")
+            .update_user_password(&id, Some("pass"), "newpass")
             .await
             .is_ok());
 
         assert!(service
-            .update_user_password(&id, None, "newpass2")
+            .update_user_password(&id, Some("wrong"), "newpass2")
             .await
             .is_err());
 
@@ -1624,6 +1654,68 @@ mod tests {
             .is_ok());
     }
 
+    #[tokio::test]
+    async fn test_sso_user_forbid_update_password() {
+        let service = test_authentication_service().await;
+        let id = service
+            .db
+            .create_user("[email protected]".into(), None, true, None)
+            .await
+            .unwrap();
+
+        let id = id.as_id();
+
+        assert!(service
+            .update_user_password(&id, None, "newpass2")
+            .await
+            .is_err());
+    }
+
+    #[tokio::test]
+    async fn test_sso_user_forbid_update_name() {
+        let service = test_authentication_service().await;
+        let id = service
+            .db
+            .create_user("[email protected]".into(), None, true, None)
+            .await
+            .unwrap();
+
+        assert!(service
+            .update_user_name(&id.as_id(), "newname".into())
+            .await
+            .is_err());
+    }
+
+    #[tokio::test]
+    async fn test_sso_user_forbid_generate_password_reset_url() {
+        let service = test_authentication_service().await;
+        let id = service
+            .db
+            .create_user("[email protected]".into(), None, true, None)
+            .await
+            .unwrap();
+
+        assert!(service
+            .generate_reset_password_url(&id.as_id())
+            .await
+            .is_err());
+    }
+
+    #[tokio::test]
+    async fn test_sso_user_forbid_request_password_reset_email() {
+        let service = test_authentication_service().await;
+        let id = service
+            .db
+            .create_user("[email protected]".into(), None, true, None)
+            .await
+            .unwrap();
+
+        assert!(service
+            .request_password_reset_email("[email protected]".into())
+            .await
+            .is_err());
+    }
+
     #[tokio::test]
     async fn test_cannot_reset_same_password() {
         let (service, _mail) = test_authentication_service_with_mail().await;

diff --git a/ee/tabby-webserver/src/service/mod.rs b/ee/tabby-webserver/src/service/mod.rs
@@ -463,6 +463,11 @@ impl UserSecuredExt for tabby_schema::auth::UserSecured {
             created_at: val.created_at,
             active: val.active,
             is_password_set: val.password_encrypted.is_some(),
+
+            // when a user created by registration, password_encrypted is set
+            // when a user created by SSO, password_encrypted is not set
+            // so, we can determine if a user is SSO user by checking if password_encrypted is set
+            is_sso_user: val.password_encrypted.is_none(),
         }
     }
 }