chore: ldap name_attr optional and use user_id by default

Signed-off-by: Wei Zhang <[email protected]>
TabbyML · Jan 7, 2025 · 89ffd2d · 89ffd2d
1 parent 4c48509
commit 89ffd2d
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 27 deletions.
diff --git a/ee/tabby-schema/src/schema/auth.rs b/ee/tabby-schema/src/schema/auth.rs
@@ -425,12 +425,8 @@ pub struct UpdateLdapCredentialInput {
         message = "emailAttribute cannot be empty"
     ))]
     pub email_attribute: String,
-    #[validate(length(
-        min = 1,
-        code = "nameAttribute",
-        message = "nameAttribute cannot be empty"
-    ))]
-    pub name_attribute: String,
+    // if name_attribute is None, we will use username as name
+    pub name_attribute: Option<String>,
 }
 
 #[derive(GraphQLObject)]
@@ -443,7 +439,7 @@ pub struct LdapCredential {
     pub encryption: LdapEncryptionKind,
     pub skip_tls_verify: bool,
     pub email_attribute: String,
-    pub name_attribute: String,
+    pub name_attribute: Option<String>,
 
     pub created_at: DateTime<Utc>,
     pub updated_at: DateTime<Utc>,

diff --git a/ee/tabby-webserver/src/ldap.rs b/ee/tabby-webserver/src/ldap.rs
@@ -18,7 +18,7 @@ pub fn new_ldap_client(
     base_dn: String,
     user_filter: String,
     email_attr: String,
-    name_attr: String,
+    name_attr: Option<String>,
 ) -> impl LdapClient {
     let mut settings = LdapConnSettings::new();
     if encryption == "starttls" {
@@ -56,7 +56,7 @@ pub struct LdapClientImpl {
     user_filter: String,
 
     email_attr: String,
-    name_attr: String,
+    name_attr: Option<String>,
 
     settings: LdapConnSettings,
 }
@@ -79,12 +79,16 @@ impl LdapClient for LdapClientImpl {
             .await?
             .success()?;
 
+        let mut attrs = vec![&self.email_attr];
+        if let Some(name_attr) = &self.name_attr {
+            attrs.push(name_attr);
+        }
         let searched = client
             .search(
                 &self.base_dn,
                 Scope::OneLevel,
                 &self.user_filter.replace("%s", user),
-                vec![&self.name_attr, &self.email_attr],
+                attrs,
             )
             .await?;
 
@@ -94,15 +98,19 @@ impl LdapClient for LdapClientImpl {
             let email = entry
                 .attrs
                 .get(&self.email_attr)
-                .and_then(|v| v.get(0))
+                .and_then(|v| v.first())
                 .cloned()
                 .ok_or_else(|| CoreError::Other(anyhow!("email not found for user")))?;
-            let name = entry
-                .attrs
-                .get(&self.name_attr)
-                .and_then(|v| v.get(0))
-                .cloned()
-                .ok_or_else(|| CoreError::Other(anyhow!("name not found for user")))?;
+            let name = if let Some(name_attr) = &self.name_attr {
+                entry
+                    .attrs
+                    .get(name_attr)
+                    .and_then(|v| v.first())
+                    .cloned()
+                    .ok_or_else(|| CoreError::Other(anyhow!("name not found for user")))?
+            } else {
+                user.to_string()
+            };
 
             client.simple_bind(&user_dn, password).await?.success()?;
 

diff --git a/ee/tabby-webserver/src/service/auth.rs b/ee/tabby-webserver/src/service/auth.rs
@@ -653,10 +653,10 @@ impl AuthenticationService for AuthenticationServiceImpl {
                 password,
                 &input.base_dn,
                 &input.user_filter,
-                &input.encryption.as_enum_str(),
+                input.encryption.as_enum_str(),
                 input.skip_tls_verify,
                 &input.email_attribute,
-                &input.name_attribute,
+                input.name_attribute.as_deref(),
             )
             .await?;
         Ok(())
@@ -842,9 +842,10 @@ fn password_verify(raw: &str, hash: &str) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use crate::service::auth::testutils::FakeLdapClient;
     use tabby_schema::auth::LdapEncryptionKind;
 
+    use crate::service::auth::testutils::FakeLdapClient;
+
     struct MockLicenseService {
         status: LicenseStatus,
         seats: i32,
@@ -1694,7 +1695,7 @@ mod tests {
                 encryption: LdapEncryptionKind::None,
                 skip_tls_verify: false,
                 email_attribute: "mail".into(),
-                name_attribute: "cn".into(),
+                name_attribute: Some("cn".into()),
             })
             .await
             .unwrap();
@@ -1709,7 +1710,7 @@ mod tests {
         assert_eq!(cred.encryption, LdapEncryptionKind::None);
         assert!(!cred.skip_tls_verify);
         assert_eq!(cred.email_attribute, "mail");
-        assert_eq!(cred.name_attribute, "cn");
+        assert_eq!(cred.name_attribute, Some("cn".into()));
 
         service
             .update_ldap_credential(UpdateLdapCredentialInput {
@@ -1722,7 +1723,7 @@ mod tests {
                 encryption: LdapEncryptionKind::None,
                 skip_tls_verify: true,
                 email_attribute: "email".into(),
-                name_attribute: "name".into(),
+                name_attribute: Some("name".into()),
             })
             .await
             .unwrap();
@@ -1738,7 +1739,7 @@ mod tests {
         assert_eq!(cred.encryption, "none");
         assert!(cred.skip_tls_verify);
         assert_eq!(cred.email_attribute, "email");
-        assert_eq!(cred.name_attribute, "name");
+        assert_eq!(cred.name_attribute, Some("name".into()));
     }
 
     #[tokio::test]

diff --git a/ee/tabby-webserver/src/service/mod.rs b/ee/tabby-webserver/src/service/mod.rs
@@ -20,13 +20,12 @@ pub mod web_documents;
 
 use std::sync::Arc;
 
-#[cfg(test)]
-pub use auth::testutils::FakeAuthService;
-
 use answer::AnswerService;
 use anyhow::Context;
 use async_trait::async_trait;
 pub use auth::create as new_auth_service;
+#[cfg(test)]
+pub use auth::testutils::FakeAuthService;
 use axum::{
     body::Body,
     http::{HeaderName, HeaderValue, Request, StatusCode},