From 72bb6738e33ccc6e0a4efd4fdd08eb22278f8e55 Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Sat, 18 Nov 2023 19:14:02 -0500 Subject: [PATCH] GUACAMOLE-1855: Use common code for checking for IP in list. --- .../auth/duo/UserVerificationService.java | 47 +++++----------- .../totp/user/UserVerificationService.java | 53 ++++++------------- 2 files changed, 30 insertions(+), 70 deletions(-) diff --git a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java b/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java index 16b2c18160..39d1174b57 100644 --- a/extensions/guacamole-auth-duo/src/main/java/org/apache/guacamole/auth/duo/UserVerificationService.java +++ b/extensions/guacamole-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 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 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.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/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java index 413da7b502..66f390f7a6 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java +++ b/extensions/guacamole-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 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 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.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;