LdapFacade to verify LDAP servers (#3198)

fixes #3196

Author: Vladimir Kotal

plugins/src/main/java/opengrok/auth/plugin/ldap/LdapFacade.java

@@ -100,7 +100,7 @@ public class LdapFacade extends AbstractLdapProvider {
     private WebHooks webHooks;
 
     private SearchControls controls;
-    private int actualServer = 0;
+    private int actualServer = -1;
     private long errorTimestamp = 0;
     private boolean reported = false;
 
@@ -187,14 +187,13 @@ private void setWebHooks(WebHooks webHooks) {
     }
 
     /**
-     * Finds first working server in the pool.
+     * Go through all servers in the pool and record the first working.
      */
-    private void prepareServers() {
+    void prepareServers() {
         for (int i = 0; i < servers.size(); i++) {
             LdapServer server = servers.get(i);
-            if (server.isWorking()) {
+            if (server.isWorking() && actualServer == -1) {
                 actualServer = i;
-                return;
             }
         }
     }
@@ -245,7 +244,7 @@ public void setSearchBase(String base) {
 
     @Override
     public boolean isConfigured() {
-        return servers != null && servers.size() > 0 && LDAP_FILTER != null && searchBase != null;
+        return servers != null && servers.size() > 0 && LDAP_FILTER != null && searchBase != null && actualServer != -1;
     }
 
     /**

plugins/src/main/java/opengrok/auth/plugin/ldap/LdapServer.java

@@ -22,7 +22,14 @@
  */
 package opengrok.auth.plugin.ldap;
 
+import java.io.IOException;
 import java.io.Serializable;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.UnknownHostException;
 import java.util.Hashtable;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -64,12 +71,12 @@ public LdapServer() {
 
     public LdapServer(String server) {
         this(prepareEnv());
-        this.url = server;
+        setName(server);
     }
 
     public LdapServer(String server, String username, String password) {
         this(prepareEnv());
-        this.url = server;
+        setName(server);
         this.username = username;
         this.password = password;
     }
@@ -122,13 +129,95 @@ public void setInterval(int interval) {
         this.interval = interval;
     }
 
+    private String urlToHostname(String urlStr) throws URISyntaxException {
+        URI uri = new URI(urlStr);
+        return uri.getHost();
+    }
+
+    /**
+     * This method converts the scheme from URI to port number.
+     * It is limited to the ldap/ldaps schemes.
+     * The method could be static however then it cannot be easily mocked in testing.
+     * @return port number or -1 if the scheme in given URI is not known
+     * @throws URISyntaxException if the URI is not valid
+     */
+    public int getPort() throws URISyntaxException {
+        URI uri = new URI(getUrl());
+        switch (uri.getScheme()) {
+            case "ldaps":
+                return 636;
+            case "ldap":
+                return 389;
+        }
+
+        return -1;
+    }
+
+    private boolean isReachable(InetAddress addr, int port, int timeOutMillis) {
+        try {
+            try (Socket soc = new Socket()) {
+                soc.connect(new InetSocketAddress(addr, port), timeOutMillis);
+            }
+            return true;
+        } catch (IOException e) {
+            return false;
+        }
+    }
+
+    /**
+     * Wraps InetAddress.getAllByName() so that it can be mocked in testing.
+     * (mocking static methods is not really possible with Mockito)
+     * @param hostname hostname string
+     * @return array of InetAddress objects
+     * @throws UnknownHostException if the host cannot be resolved to any IP address
+     */
+    public InetAddress[] getAddresses(String hostname) throws UnknownHostException {
+        return InetAddress.getAllByName(hostname);
+    }
+
     /**
-     * The LDAP server is working only when its connection is not null. This
-     * tries to establish the connection if it is not established already.
+     * Go through all IP addresses and find out if they are reachable.
+     * @return true if all IP addresses are reachable, false otherwise
+     */
+    public boolean isReachable() {
+        try {
+            InetAddress[] addresses = getAddresses(urlToHostname(getUrl()));
+            if (addresses.length == 0) {
+                LOGGER.log(Level.WARNING, "LDAP server {0} does not resolve to any IP address", this);
+                return false;
+            }
+
+            for (InetAddress addr : addresses) {
+                // InetAddr.isReachable() is not sufficient as it can only check ICMP and TCP echo.
+                int port = getPort();
+                if (!isReachable(addr, port, getConnectTimeout())) {
+                    LOGGER.log(Level.WARNING, "LDAP server {0} is not reachable on {1}:{2}",
+                            new Object[]{this, addr, Integer.toString(port)});
+                    return false;
+                }
+            }
+        } catch (UnknownHostException e) {
+            LOGGER.log(Level.SEVERE, String.format("cannot get IP addresses for LDAP server %s", this), e);
+            return false;
+        } catch (URISyntaxException e) {
+            LOGGER.log(Level.SEVERE, String.format("not a valid URI: %s", getUrl()), e);
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * The LDAP server is working only when it is reachable and its connection is not null.
+     * This method tries to establish the connection if it is not established already.
      *
      * @return true if it is working
      */
     public synchronized boolean isWorking() {
+        if (!isReachable()) {
+            return false;
+        }
+
         if (ctx == null) {
             ctx = connect();
         }
@@ -144,7 +233,7 @@ private synchronized LdapContext connect() {
         LOGGER.log(Level.INFO, "Connecting to LDAP server {0} ", this.toString());
 
         if (errorTimestamp > 0 && errorTimestamp + interval > System.currentTimeMillis()) {
-            LOGGER.log(Level.INFO, "LDAP server {0} is down", this.url);
+            LOGGER.log(Level.WARNING, "LDAP server {0} is down", this.url);
             close();
             return null;
         }
@@ -169,7 +258,7 @@ private synchronized LdapContext connect() {
                 LOGGER.log(Level.INFO, "Connected to LDAP server {0}", this.toString());
                 errorTimestamp = 0;
             } catch (NamingException ex) {
-                LOGGER.log(Level.INFO, "LDAP server {0} is not responding", env.get(Context.PROVIDER_URL));
+                LOGGER.log(Level.WARNING, "LDAP server {0} is not responding", env.get(Context.PROVIDER_URL));
                 errorTimestamp = System.currentTimeMillis();
                 close();
                 return ctx = null;
@@ -194,7 +283,7 @@ public NamingEnumeration<SearchResult> search(String name, String filter, Search
     }
 
     /**
-     * Lookups the LDAP server.
+     * Perform LDAP search.
      *
      * @param name base dn for the search
      * @param filter LDAP filter

plugins/src/test/java/opengrok/auth/plugin/LdapServerTest.java

@@ -0,0 +1,110 @@
+package opengrok.auth.plugin;
+
+import opengrok.auth.plugin.ldap.LdapServer;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.URISyntaxException;
+import java.net.UnknownHostException;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+
+public class LdapServerTest {
+
+    @Test
+    public void testInvalidURI() {
+        LdapServer server = new LdapServer("foo:/\\/\\foo.bar");
+        assertFalse(server.isReachable());
+    }
+
+    @Test
+    public void testGetPort() throws URISyntaxException {
+        LdapServer server = new LdapServer("ldaps://foo.bar");
+        assertEquals(636, server.getPort());
+
+        server = new LdapServer("ldap://foo.bar");
+        assertEquals(389, server.getPort());
+
+        server = new LdapServer("crumble://foo.bar");
+        assertEquals(-1, server.getPort());
+    }
+
+    @Test
+    public void testSetGetUsername() {
+        LdapServer server = new LdapServer();
+
+        assertNull(server.getUsername());
+        assertNull(server.getPassword());
+
+        final String testUsername = "foo";
+        server.setUsername(testUsername);
+        assertEquals(testUsername, server.getUsername());
+
+        final String testPassword = "bar";
+        server.setPassword(testPassword);
+        assertEquals(testPassword, server.getPassword());
+    }
+
+    @Test
+    public void testIsReachable() throws IOException, InterruptedException, URISyntaxException {
+        // Start simple TCP server on port 6336. It has to be > 1024 to avoid BindException
+        // due to permission denied.
+        int testPort = 6336;
+        InetAddress localhostAddr = InetAddress.getLocalHost();
+        ServerSocket serverSocket = new ServerSocket(testPort, 1, localhostAddr);
+        Thread thread = new Thread(() -> {
+            try {
+                while (true) {
+                    Socket client = serverSocket.accept();
+                    client.close();
+                }
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+        });
+
+        thread.start();
+        Socket socket = null;
+        for (int i = 0; i < 3; i++) {
+            try {
+                socket = new Socket(localhostAddr, testPort);
+            } catch (IOException e) {
+                Thread.sleep(1000);
+            }
+        }
+
+        assertNotNull(socket);
+        assertTrue(socket.isConnected());
+
+        // Mock getAddresses() to return single localhost IP address.
+        LdapServer server = new LdapServer("ldaps://foo.bar.com");
+        LdapServer serverSpy = Mockito.spy(server);
+        Mockito.when(serverSpy.getAddresses(any())).thenReturn(new InetAddress[]{localhostAddr});
+        doReturn(testPort).when(serverSpy).getPort();
+
+        // Test reachability.
+        boolean reachable = serverSpy.isReachable();
+        serverSocket.close();
+        thread.join(5000);
+        thread.interrupt();
+        assertTrue(reachable);
+
+        // Test non-reachability.
+        reachable = serverSpy.isReachable();
+        assertFalse(reachable);
+    }
+
+    @Test
+    public void testEmptyAddressArray() throws UnknownHostException {
+        LdapServer server = new LdapServer("ldaps://foo.bar.com");
+        LdapServer serverSpy = Mockito.spy(server);
+        Mockito.when(serverSpy.getAddresses(any())).thenReturn(new InetAddress[]{});
+        assertFalse(serverSpy.isReachable());
+    }
+}

plugins/src/test/java/opengrok/auth/plugin/LdapFacadeTest.java renamed to plugins/src/test/java/opengrok/auth/plugin/ldap/LdapFacadeTest.java

@@ -1,18 +1,23 @@
-package opengrok.auth.plugin;
+package opengrok.auth.plugin.ldap;
 
 import opengrok.auth.plugin.configuration.Configuration;
 import opengrok.auth.plugin.ldap.LdapFacade;
 import opengrok.auth.plugin.ldap.LdapServer;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 import javax.naming.directory.SearchControls;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.mockito.ArgumentMatchers.any;
 
 public class LdapFacadeTest {
     @Test
@@ -52,4 +57,23 @@ public void testToString() {
         assertEquals("{servers=http://foo.foo,http://bar.bar, searchBase=dc=foo,dc=com}",
                 facade.toString());
     }
+
+    @Test
+    public void testPrepareServersNegative() throws UnknownHostException {
+        Configuration config = new Configuration();
+
+        LdapServer server1 = new LdapServer("ldap://foo.com");
+        LdapServer serverSpy1 = Mockito.spy(server1);
+        Mockito.when(serverSpy1.getAddresses(any())).thenReturn(new InetAddress[]{InetAddress.getLocalHost()});
+        Mockito.when(serverSpy1.isReachable()).thenReturn(false);
+
+        LdapServer server2 = new LdapServer("ldap://bar.com");
+        LdapServer serverSpy2 = Mockito.spy(server2);
+        Mockito.when(serverSpy2.getAddresses(any())).thenReturn(new InetAddress[]{});
+
+        config.setServers(Arrays.asList(serverSpy1, serverSpy2));
+        LdapFacade facade = new LdapFacade(config);
+        facade.prepareServers();
+        assertFalse(facade.isConfigured());
+    }
 }