Merge pull request #35 from FrendsPlatform/FSPES-2

Improved LDAP error handling by validating bind status and checking s…

Author: MatteoDelOmbra

.github/workflows/SearchObjects_build_and_test_on_main.yml

@@ -13,6 +13,24 @@ jobs:
     uses: FrendsPlatform/FrendsTasks/.github/workflows/linux_build_main.yml@main
     with:
       workdir: Frends.LDAP.SearchObjects
-      prebuild_command: docker run -d -i --rm -p 10389:10389 dwimberger/ldap-ad-it
+      prebuild_command: |
+        docker run -d -i --rm --name ldap1 -p 10389:10389 dwimberger/ldap-ad-it
+        docker run -d -i --rm --name ldap2 -p 20389:389 \
+          -e LDAP_ORGANISATION="Test Org" \
+          -e LDAP_DOMAIN="example.org" \
+          -e LDAP_ADMIN_PASSWORD="admin" \
+          osixia/openldap:1.5.0
+
+        echo "Waiting for osixia LDAP to be ready..."
+        for i in {1..30}; do
+        if docker logs ldap2 2>&1 | grep -q "slapd starting"; then
+            echo "osixia LDAP is ready."
+            break
+        fi
+        echo "Still waiting for osixia LDAP ($i)..."
+        sleep 1
+        done
+
+        echo "Prebuild finished."
     secrets:
       badge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}

.github/workflows/SearchObjects_build_and_test_on_push.yml

@@ -13,7 +13,25 @@ jobs:
     uses: FrendsPlatform/FrendsTasks/.github/workflows/linux_build_test.yml@main
     with:
       workdir: Frends.LDAP.SearchObjects
-      prebuild_command: docker run -d -i --rm -p 10389:10389 dwimberger/ldap-ad-it
+      prebuild_command: |
+        docker run -d -i --rm --name ldap1 -p 10389:10389 dwimberger/ldap-ad-it
+        docker run -d -i --rm --name ldap2 -p 20389:389 \
+          -e LDAP_ORGANISATION="Test Org" \
+          -e LDAP_DOMAIN="example.org" \
+          -e LDAP_ADMIN_PASSWORD="admin" \
+          osixia/openldap:1.5.0
+
+        echo "Waiting for osixia LDAP to be ready..."
+        for i in {1..30}; do
+        if docker logs ldap2 2>&1 | grep -q "slapd starting"; then
+            echo "osixia LDAP is ready."
+            break
+        fi
+        echo "Still waiting for osixia LDAP ($i)..."
+        sleep 1
+        done
+
+        echo "Prebuild finished."
     secrets:
       badge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}
       test_feed_api_key: ${{ secrets.TASKS_TEST_FEED_API_KEY }}

Frends.LDAP.SearchObjects/CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## [4.1.0] - 2025-06-17
+### Added
+- Improved LDAP error handling by validating bind status and checking search response codes.
+- Block negative values for MsLimit
+
 ## [4.0.0] - 2025-04-04
 ### Added
 - [Breaking] - Parameter for PageSize to control how many entries are fetched per page during an LDAP search.

Frends.LDAP.SearchObjects/Frends.LDAP.SearchObjects.Tests/UnitTestAnonymousBind .cs

@@ -0,0 +1,71 @@
+using Frends.LDAP.SearchObjects.Definitions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Frends.LDAP.SearchObjects.Tests;
+
+[TestClass]
+public class UnitTestAnonymousBind
+{
+    /*
+        Create a simple LDAP server to docker for anonymous bind testing
+        docker run --rm -p 20389:389 -e LDAP_ORGANISATION="Test Org" -e LDAP_DOMAIN="example.org" -e LDAP_ADMIN_PASSWORD="admin" osixia/openldap:1.5.0
+    */
+
+    private readonly string? _host = "127.0.0.1";
+    private readonly int _port = 20389;
+    private readonly string? _user = "cn=admin,dc=example,dc=org";
+    private readonly string? _pw = "admin";
+    private readonly string _path = "dc=example,dc=org";
+
+    Connection? connection;
+
+    [TestInitialize]
+    public void Setup()
+    {
+        connection = new()
+        {
+            Host = _host,
+            User = _user,
+            Password = _pw,
+            SecureSocketLayer = false,
+            Port = _port,
+            TLS = false,
+            LDAPProtocolVersion = LDAPVersion.V3
+        };
+    }
+
+    [TestMethod]
+    public void Search_AnonymousBind_ShouldFailAndReturnError()
+    {
+        var badConnection = new Connection
+        {
+            Host = _host,
+            Port = _port,
+            SecureSocketLayer = false,
+            TLS = false,
+            LDAPProtocolVersion = LDAPVersion.V3,
+            AnonymousBind = true,
+            ThrowExceptionOnError = false
+        };
+
+        var badInput = new Input
+        {
+            SearchBase = _path,
+            Scope = Scopes.ScopeSub,
+            Filter = "(objectClass=*)",
+            PageSize = 2,
+            MaxResults = 5
+        };
+
+        var result = LDAP.SearchObjects(badInput, badConnection, CancellationToken.None);
+
+        Assert.IsFalse(result.Success, "Expected Success to be false when anonymous bind fails.");
+        Assert.IsTrue(!string.IsNullOrEmpty(result.Error), "Expected an error message.");
+        Assert.IsTrue(result.Error.Contains("bind"), "Expected error message to mention bind.");
+    }
+}

Frends.LDAP.SearchObjects/Frends.LDAP.SearchObjects/Frends.LDAP.SearchObjects.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
 	<TargetFrameworks>net6.0</TargetFrameworks>
-	<Version>4.0.0</Version>
+	<Version>4.1.0</Version>
 	<Authors>Frends</Authors>
 	<Copyright>Frends</Copyright>
 	<Company>Frends</Company>

Frends.LDAP.SearchObjects/Frends.LDAP.SearchObjects/SearchObjects.cs

@@ -35,6 +35,9 @@ public static Result SearchObjects([PropertyTab] Input input, [PropertyTab] Conn
         if (string.IsNullOrEmpty(connection.Password) && !connection.AnonymousBind)
             throw new Exception("Password is missing.");
 
+        if (input.MsLimit < 0)
+            throw new ArgumentException("MsLimit must be a non-negative value (0 for no limit).");
+
         LdapConnectionOptions ldco = new LdapConnectionOptions();
 
         var encoding = GetEncoding(input.ContentEncoding, input.ContentEncodingString, input.EnableBom);
@@ -91,6 +94,13 @@ public static Result SearchObjects([PropertyTab] Input input, [PropertyTab] Conn
 
             byte[] cookie = null;
 
+            if (!conn.Bound)
+            {
+                if (connection.ThrowExceptionOnError)
+                    throw new LdapException("LDAP bind failed: connection is not bound.");
+                return new Result(false, $"LdapException: LDAP bind failed: connection is not bound.", null);
+            }
+
             do
             {
                 if (input.PageSize > 0)
@@ -150,6 +160,18 @@ private static void ProcessSearchResults(LdapSearchQueue queue, List<SearchResul
             }
             else if (message is LdapResponse ldapResponse)
             {
+                if (ldapResponse.ResultCode != LdapException.Success &&
+                    ldapResponse.ResultCode != LdapException.SizeLimitExceeded &&
+                    ldapResponse.ResultCode != LdapException.TimeLimitExceeded &&
+                    ldapResponse.ResultCode != LdapException.LdapPartialResults)
+                {
+                    throw new LdapException(
+                        $"LDAP search failed with error: {ldapResponse.ErrorMessage}",
+                        ldapResponse.ResultCode,
+                        ldapResponse.MatchedDn
+                    );
+                }
+
                 var controls = ldapResponse.Controls;
                 if (controls != null)
                 {