Merge pull request #2505 from gemini-hlsw/correct-ldap-query

Fix for LDAP query
gemini-hlsw · Oct 13, 2023 · 2d932b4 · 2d932b4
2 parents acc7bf2 + 4877adc
commit 2d932b4
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 11 deletions.
diff --git a/...web/server/src/main/scala/seqexec/web/server/security/FreeLDAPAuthenticationService.scala b/...web/server/src/main/scala/seqexec/web/server/security/FreeLDAPAuthenticationService.scala
@@ -19,6 +19,8 @@ object FreeLDAPAuthenticationService {
   import LdapConnectionOps._
   import UserDetails._
 
+  val Domain = "@ad.noirlab.edu"
+
   sealed trait LdapOp[A]
   // Operations on ldap
   object LdapOp {
@@ -60,10 +62,14 @@ object FreeLDAPAuthenticationService {
   def authenticate(u: String, p: String): LdapM[UID] = bind(u, p)
 
   // Authenticate and reads the display name
-  def authenticationAndName(u: String, p: String): LdapM[UserDetails] = for {
-    u <- bind(u, p)
-    d <- displayName(u)
-  } yield UserDetails(u, d)
+  def authenticationAndName(u: String, p: String): LdapM[UserDetails] = {
+    // We should include the domain
+    val usernameWithDomain = s"$u$Domain"
+    for {
+      lu <- bind(usernameWithDomain, p)
+      d  <- displayName(lu)
+    } yield UserDetails(lu, d)
+  }
 
   // Authenticate and reads the name, groups and photo
   def authNameGroupThumb(u: String, p: String): LdapM[(UserDetails, Groups, Option[Thumbnail])] =
@@ -83,7 +89,6 @@ class FreeLDAPAuthenticationService[F[_]: Sync: Logger](hosts: List[(String, Int
 
   // Shorten the default timeout
   private val Timeout = 1000
-  private val Domain  = "noirlab\\"
 
   lazy val ldapOptions: LDAPConnectionOptions = {
     val opts = new LDAPConnectionOptions()
@@ -96,15 +101,13 @@ class FreeLDAPAuthenticationService[F[_]: Sync: Logger](hosts: List[(String, Int
     new FailoverServerSet(hosts.map(_._1).toArray, hosts.map(_._2).toArray, ldapOptions)
 
   override def authenticateUser(username: String, password: String): F[AuthResult] = {
-    // We should always return the domain
-    val usernameWithDomain = if (username.startsWith(Domain)) username else s"$Domain$username"
 
     val rsrc =
       for {
         c <- Resource.make(Sync[F].delay(failoverServerSet.getConnection))(c =>
                Sync[F].delay(c.close())
              )
-        x <- Resource.eval(runF(authenticationAndName(usernameWithDomain, password), c).attempt)
+        x <- Resource.eval(runF(authenticationAndName(username, password), c).attempt)
         _ <- Resource.eval(
                Logger[F].info(s"LDAP authentication result on host $hosts for user '$username': $x")
              )

diff --git a/modules/web/server/src/main/scala/seqexec/web/server/security/LdapConnectionOps.scala b/modules/web/server/src/main/scala/seqexec/web/server/security/LdapConnectionOps.scala
@@ -75,7 +75,6 @@ object LdapConnectionOps {
         Filter.createEqualityFilter("objectClass", "user")
       )
 
-      // val attributes = List("displayName", "memberOf", "thumbnailPhoto")
       val search       = new SearchRequest(s"cn=users,$baseDN", SearchScope.SUB, filter, attributes: _*)
       // Search to read user data, it may throw an exception
       val searchResult = c.search(search)

diff --git a/...server/src/test/scala/seqexec/web/server/security/FreeLDAPAuthenticationServiceSpec.scala b/...server/src/test/scala/seqexec/web/server/security/FreeLDAPAuthenticationServiceSpec.scala
@@ -41,12 +41,16 @@ class FreeLDAPAuthenticationServiceSpec extends CatsSuite {
       runMock(authenticate(u, ""), db) shouldEqual u
     }
   }
+
   test("LDAP Auth Service: support auth and name") {
     forAll { (u: String, t: (String, String)) =>
-      val db = MockAuthDB(Map(u -> t), acceptEmptyPwd = true)
-      runMock(authenticationAndName(u, ""), db) shouldEqual UserDetails(u, t._2)
+      val username = s"$u${FreeLDAPAuthenticationService.Domain}"
+      val db       =
+        MockAuthDB(Map(username -> t), acceptEmptyPwd = true)
+      runMock(authenticationAndName(u, ""), db) shouldEqual UserDetails(username, t._2)
     }
   }
+
   test("LDAP Auth Service: support auth and name with a password") {
     forAll { (u: String, t: (String, String)) =>
       val db = MockAuthDB(Map(u -> t), acceptEmptyPwd = false)
@@ -55,6 +59,7 @@ class FreeLDAPAuthenticationServiceSpec extends CatsSuite {
       }
     }
   }
+
   test("LDAP Auth Service: handle exceptions") {
     forAll { (u: String, t: (String, String)) =>
       val db = MockAuthDB(Map(u -> t), acceptEmptyPwd = false)