GUACAMOLE-1855: Use common code for checking for IP in list.

apache · Jan 18, 2024 · 10d6aae · 10d6aae
1 parent c71f490
commit 10d6aae
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 70 deletions.
diff --git a/...acamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java b/...acamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java
@@ -35,6 +35,7 @@
 import org.apache.guacamole.net.auth.AuthenticatedUser;
 import org.apache.guacamole.net.auth.Credentials;
 import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import org.apache.guacamole.properties.IPAddressListProperty;
 
 /**
  * Service for verifying the identity of a user against Duo.
@@ -80,52 +81,30 @@ public void verifyAuthenticatedUser(AuthenticatedUser authenticatedUser)
         if (authenticatedUser.getIdentifier().equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER))
             return;
 
-        // We enforce by default
-        boolean enforceHost = true;
-
-        // Check for a list of addresses that should be bypassed and iterate
+        // Pull address lists to check from configuration. Note that the enforce
+        // list will override the bypass list, which means that, if the client
+        // address happens to be in both lists, Duo MFA will be enforced.
         List<IPAddress> bypassAddresses = confService.getBypassHosts();
-        for (IPAddress bypassAddr : bypassAddresses) {
-
-            // If the address contains current client address, flip enforce flag
-            // and break out
-            if (clientAddr != null && clientAddr.isIPAddress()
-                    && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion())
-                    && bypassAddr.contains(clientAddr)) {
-                enforceHost = false;
-                break;
-            }
-        }
-
-        // Check for a list of addresses that should be enforced and iterate
         List<IPAddress> enforceAddresses = confService.getEnforceHosts();
 
+        // Check if the bypass list contains the client address, and set the
+        // enforce flag to the opposite.
+        boolean enforceHost = !(IPAddressListProperty.addressListContains(bypassAddresses, clientAddr));
+
         // Only continue processing if the list is not empty
         if (enforceAddresses != null && !enforceAddresses.isEmpty()) {
 
             // If client address is not available or invalid, MFA will
             // be enforced.
-            if (clientAddr == null || !clientAddr.isIPAddress()) {
+            if (clientAddr == null || !clientAddr.isIPAddress())
                 enforceHost = true;
-            }
 
-            else {
-                // With addresses set, this default changes to false.
-                enforceHost = false;
-
-                for (IPAddress enforceAddr : enforceAddresses) {
-
-                    // If there's a match, flip the enforce flag and break out of the loop
-                    if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion())
-                            && enforceAddr.contains(clientAddr)) {
-                        enforceHost = true;
-                        break;
-                    }
-                }
-            }
+            // Check the enforce list for the client address and set enforcement flag.
+            else
+                enforceHost = IPAddressListProperty.addressListContains(enforceAddresses, clientAddr);
         }
 
-        // If the enforce flag has been changed, exit, bypassing Duo MFA.
+        // If the enforce flag is not true, bypass Duo MFA.
         if (!enforceHost)
             return;
 

diff --git a/...-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/...-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java
@@ -47,6 +47,7 @@
 import org.apache.guacamole.net.auth.UserContext;
 import org.apache.guacamole.net.auth.UserGroup;
 import org.apache.guacamole.net.auth.credentials.CredentialsInfo;
+import org.apache.guacamole.properties.IPAddressListProperty;
 import org.apache.guacamole.totp.TOTPGenerator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -296,57 +297,37 @@ public void verifyIdentity(UserContext context,
         HttpServletRequest request = credentials.getRequest();
 
         // Get the current client address
-        IPAddressString clientAddr = new IPAddressString(request.getRemoteAddr());
+        IPAddress clientAddr = new IPAddressString(request.getRemoteAddr()).getAddress();
 
         // Ignore anonymous users
         if (authenticatedUser.getIdentifier().equals(AuthenticatedUser.ANONYMOUS_IDENTIFIER))
             return;
 
-        // We enforce by default
-        boolean enforceHost = true;
-
-        // Check for a list of addresses that should be bypassed and iterate
+        // Pull address lists to check from configuration. Note that the enforce
+        // list will override the bypass list, which means that, if the client
+        // address happens to be in both lists, Duo MFA will be enforced.
         List<IPAddress> bypassAddresses = confService.getBypassHosts();
-        for (IPAddress bypassAddr : bypassAddresses) {
-            // If the address contains current client address, flip enforce flag
-            // and break out
-            if (clientAddr != null && clientAddr.isIPAddress()
-                    && bypassAddr.getIPVersion().equals(clientAddr.getIPVersion())
-                    && bypassAddr.contains(clientAddr.getAddress())) {
-                enforceHost = false;
-                break;
-            }
-        }
-
-        // Check for a list of addresses that should be enforced and iterate
         List<IPAddress> enforceAddresses = confService.getEnforceHosts();
 
+        // Check the bypass list for the client address, and set the enforce
+        // flag to the opposite.
+        boolean enforceHost = !(IPAddressListProperty.addressListContains(bypassAddresses, clientAddr));
+
         // Only continue processing if the list is not empty
         if (enforceAddresses != null && !enforceAddresses.isEmpty()) {
 
-            if (clientAddr == null || !clientAddr.isIPAddress()) {
-                logger.warn("Client address is not valid, "
-                + "MFA will be enforced.");
+            // If client address is not available or invalid, MFA will
+            // be enforced.
+            if (clientAddr == null || !clientAddr.isIPAddress())
                 enforceHost = true;
-            }
 
-            else {
-                // With addresses set, this default changes to false.
-                enforceHost = false;
-
-                for (IPAddress enforceAddr : enforceAddresses) {
-
-                    // If there's a match, flip the enforce flag and break out of the loop
-                    if (enforceAddr.getIPVersion().equals(clientAddr.getIPVersion())
-                            && enforceAddr.contains(clientAddr.getAddress())) {
-                        enforceHost = true;
-                        break;
-                    }
-                }
-            }
+            // Check the enforce list and set the flag if the client address
+            // is found in the list.
+            else
+                enforceHost = IPAddressListProperty.addressListContains(enforceAddresses, clientAddr);
         }
 
-        // If the enforce flag has been changed, exit, bypassing TOTP MFA.
+        // If the enforce flag is not true, bypass TOTP MFA.
         if (!enforceHost)
             return;