Refactor BasicAuthenticationFilter

src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfiguration.java

@@ -1,6 +1,8 @@
 package org.cloudfoundry.autoscaler.scheduler.conf;
 
 import jakarta.annotation.PostConstruct;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import lombok.AllArgsConstructor;
@@ -21,10 +23,16 @@ public class HealthServerConfiguration {
   private Integer port;
   private Set<String> unprotectedEndpoints;
 
+  final Map<String, Boolean> validProtectedEndpoints =
+      Map.of(
+          "/health/prometheus", true,
+          "/health/liveness", true);
+
   @PostConstruct
   public void init() {
 
     validatePort();
+    validateConfiguredEndpoints();
 
     boolean basicAuthEnabled =
         (unprotectedEndpoints != null || ObjectUtils.isEmpty(unprotectedEndpoints));
@@ -33,15 +41,38 @@ public void init() {
             || this.password == null
             || this.username.isEmpty()
             || this.password.isEmpty())) {
-      throw new IllegalArgumentException("Heath Server Basic Auth Username or password is not set");
+      throw new IllegalArgumentException(
+          "Health Server Basic Auth Username or password is not set");
     }
   }
 
   private void validatePort() {
     Optional<Integer> healthPortOptional = Optional.ofNullable(this.port);
     if (!healthPortOptional.isPresent() || healthPortOptional.get() == 0) {
       throw new IllegalArgumentException(
-          "Health Configuration: health server port not defined or set to unsupported port-number `0`");
+          "Health Configuration: health server port not defined or set to unsupported port-number"
+              + " `0`");
+    }
+  }
+
+  private void validateConfiguredEndpoints() {
+
+    Map<String, Boolean> invalidEndpointsMap = new HashMap<>();
+    if (unprotectedEndpoints == null) {
+      return;
+    }
+    for (String unprotectedEndpoint : unprotectedEndpoints) {
+      if (!validProtectedEndpoints.containsKey(unprotectedEndpoint)) {
+        invalidEndpointsMap.put(unprotectedEndpoint, true);
+      }
+    }
+    if (!ObjectUtils.isEmpty(invalidEndpointsMap)) {
+      throw new IllegalArgumentException(
+          "Health configuration: invalid unprotectedEndpoints provided: "
+              + invalidEndpointsMap
+              + "\n"
+              + "validate endpoints are: "
+              + validProtectedEndpoints);
     }
   }
 }

src/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilter.java

@@ -8,12 +8,8 @@
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.codec.binary.Base64;
 import org.cloudfoundry.autoscaler.scheduler.conf.HealthServerConfiguration;
@@ -27,14 +23,6 @@
 @Order(2)
 public class BasicAuthenticationFilter implements Filter {
   private static final String WWW_AUTHENTICATE_VALUE = "Basic";
-  private static final Map<String, Boolean> validProtectedEndpointsMap;
-
-  static {
-    validProtectedEndpointsMap =
-        Map.of(
-            "/health/prometheus", true,
-            "/health/liveness", true);
-  }
 
   final HealthServerConfiguration healthServerConfiguration;
 
@@ -55,144 +43,108 @@ public void doFilter(
       chain.doFilter(servletRequest, servletResponse);
       return;
     }
-
     final boolean allEndpointsRequireAuthorization =
         ObjectUtils.isEmpty(unprotectedEndpointsConfig);
-    if (allEndpointsRequireAuthorization) {
-      isUserAuthenticatedOrSendError(chain, httpRequest, httpResponse);
+
+    if (allEndpointsRequireAuthorization) { // case 1 ; config []
+      allowAuthenticatedRequest(chain, httpRequest, httpResponse);
+
     } else if (!ObjectUtils.isEmpty(unprotectedEndpointsConfig)) {
-      Map<String, Boolean> validateMap = checkValidEndpoints(unprotectedEndpointsConfig);
-      if (!ObjectUtils.isEmpty(validateMap)) {
-        log.warn(
-            "Health configuration: invalid unprotectedEndpoints provided: "
-                + validateMap.get("invalidEndpoints"));
+      /*
+      // case 2.1 ; config ["/health/liveness"]
+      here is – by configuration – one protected endpoint "/health/prometheus" and one unprotected "/health/liveness".
+      The user is authenticated.
+       The user queries on "/health/prometheus".
+       Expected behaviour: The request will be handled successfully.
+       */
+
+      // IMPORTANT: Match the configured health endpoints with the allowed list of endpoints to
+      // cover
+      // BasicAuthenticationFilterTest#denyHealthRequestWithWrongUnprotectedEndpoints()
+      // Suggestion: THe following block should be part of HealthConfiguration.
+      // Move this block to Health Configuration/Or adjust test
+      if (!healthConfigsExists()) {
+        log.error("Health Configuration: Invalid endpoints defined");
         httpResponse.setHeader(HttpHeaders.WWW_AUTHENTICATE, WWW_AUTHENTICATE_VALUE);
         httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
         return;
       }
-      Map<String, Boolean> unprotectedConfig = getMapFromList(unprotectedEndpointsConfig);
-      List<String> allowedEndpointsWithoutBasicAuth =
-          areEndpointsAuthorized(unprotectedConfig, httpRequest.getRequestURI());
-
-      if (!allowedEndpointsWithoutBasicAuth.contains(httpRequest.getRequestURI())) {
-        log.error(
-            "Health configuration: Basic auth is required to access protectedEndpoints: "
-                + httpRequest.getRequestURI()
-                + " \nValid unprotected endpoints are: "
-                + allowedEndpointsWithoutBasicAuth);
-        httpResponse.setHeader("WWW-Authenticate", WWW_AUTHENTICATE_VALUE);
-        httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
-        return;
+
+      boolean isEndpointRequireAuthentication =
+          isEndpointRequireAuthentication(httpRequest.getRequestURI());
+      if (isEndpointRequireAuthentication) {
+        allowAuthenticatedRequest(chain, httpRequest, httpResponse);
+      } else { // RequestURI() does not need authentication (
+        // Case 2.2 ; config  ["/health/liveness", "/health/prometheus"]
+        chain.doFilter(servletRequest, servletResponse);
       }
-      chain.doFilter(servletRequest, servletResponse);
     }
   }
 
-  private void isUserAuthenticatedOrSendError(
+  private boolean healthConfigsExists() {
+    boolean found = false;
+    Map<String, Boolean> validProtectedEndpoints =
+        healthServerConfiguration.getValidProtectedEndpoints();
+    for (String configuredEndpoint : healthServerConfiguration.getUnprotectedEndpoints()) {
+      found = validProtectedEndpoints.containsKey(configuredEndpoint);
+    }
+    if (!found) {
+      return false;
+    }
+    return true;
+  }
+
+  private void allowAuthenticatedRequest(
       FilterChain filterChain, HttpServletRequest httpRequest, HttpServletResponse httpResponse)
       throws IOException, ServletException {
     final String authorizationHeader = httpRequest.getHeader("Authorization");
 
-    if (healthServerConfiguration.getUsername() == null
-        || healthServerConfiguration.getPassword() == null) {
-      log.error("Health configuration: username || password not set");
-      httpResponse.setHeader(HttpHeaders.WWW_AUTHENTICATE, WWW_AUTHENTICATE_VALUE);
-      httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
-      return;
-    }
     if (authorizationHeader == null) {
       log.error("Basic authentication not provided with the request");
       httpResponse.setHeader(HttpHeaders.WWW_AUTHENTICATE, WWW_AUTHENTICATE_VALUE);
       httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
       return;
     }
 
-    String base64Credentials =
-        authorizationHeader.substring(WWW_AUTHENTICATE_VALUE.length()).trim();
-    byte[] credDecoded = Base64.decodeBase64(base64Credentials);
-    String credentials = new String(credDecoded);
-    String[] tokens = credentials.split(":");
+    String[] tokens = decodeAndGetAuthTokens(authorizationHeader);
     if (tokens.length != 2) {
       log.error("Malformed authorization header");
       httpResponse.sendError(HttpServletResponse.SC_BAD_REQUEST);
       return;
     }
-    String username = tokens[0];
-    String password = tokens[1];
-
-    if (!areBasicAuthCredentialsCorrect(username, password)) {
-      httpResponse.setHeader(HttpHeaders.WWW_AUTHENTICATE, WWW_AUTHENTICATE_VALUE);
-      httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
-      return;
-    }
 
     if (isUserAuthenticated(authorizationHeader)) {
       // allow access to health endpoints
       filterChain.doFilter(httpRequest, httpResponse);
     } else {
+      log.error(
+          "Health configuration: Basic auth is required to access protectedEndpoints: "
+              + httpRequest.getRequestURI()
+              + " \nValid unprotected endpoints are: "
+              + healthServerConfiguration.getUnprotectedEndpoints());
       httpResponse.setHeader(HttpHeaders.WWW_AUTHENTICATE, WWW_AUTHENTICATE_VALUE);
       httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
     }
   }
 
-  private Map<String, Boolean> checkValidEndpoints(Set<String> unprotectedEndpointsConfig) {
-
-    Map<String, Boolean> invalidEndpointsMap = new HashMap<>();
-    for (String unprotectedEndpoint : unprotectedEndpointsConfig) {
-      if (!validProtectedEndpointsMap.containsKey(unprotectedEndpoint)) {
-        invalidEndpointsMap.put(unprotectedEndpoint, true);
-      }
-    }
-    return invalidEndpointsMap;
-  }
-
-  private static Map<String, Boolean> getMapFromList(Set<String> unprotectedEndpointsConfig) {
-    return unprotectedEndpointsConfig.stream()
-        .collect(Collectors.toMap(endpoint -> endpoint, endpoint -> true, (a, b) -> b));
-  }
-
-  private List<String> areEndpointsAuthorized(
-      Map<String, Boolean> unprotectedEndpointsConfig, String requestURI) {
-    Map<String, Boolean> resultUnprotectedEndpoints = new HashMap<>();
-    for (Map.Entry<String, Boolean> protectedEndpoint : validProtectedEndpointsMap.entrySet()) {
-      if (unprotectedEndpointsConfig.containsKey(protectedEndpoint.getKey())) {
-        resultUnprotectedEndpoints.put(protectedEndpoint.getKey(), false);
-      }
-    }
-    List<String> allowedEndpointsWithoutBasicAuth =
-        keys(resultUnprotectedEndpoints, false).toList();
-    if (isBasicAuthRequired(requestURI, allowedEndpointsWithoutBasicAuth)) {
-      log.debug("Endpoints allowed without basic auth: + " + allowedEndpointsWithoutBasicAuth);
-      return allowedEndpointsWithoutBasicAuth;
-    }
-    return allowedEndpointsWithoutBasicAuth;
-  }
-
-  private static boolean isBasicAuthRequired(String requestURI, List<String> allowedEndpoints) {
-    return !ObjectUtils.isEmpty(allowedEndpoints) && allowedEndpoints.contains(requestURI);
-  }
+  private boolean isEndpointRequireAuthentication(String requestURI) {
+    Map<String, Boolean> protectedEndpoints =
+        healthServerConfiguration.getValidProtectedEndpoints();
+    boolean isProtected = protectedEndpoints.containsKey(requestURI);
+    boolean isUnprotectedByConfiguration =
+        healthServerConfiguration.getUnprotectedEndpoints().contains(requestURI);
 
-  private <K, V> Stream<K> keys(Map<K, V> map, V val) {
-    return map.entrySet().stream()
-        .filter(entry -> val.equals(entry.getValue()))
-        .map(Map.Entry::getKey);
+    return isProtected && !isUnprotectedByConfiguration;
   }
 
   private boolean isUserAuthenticated(String authorization) {
-    if (healthServerConfiguration.getUsername() == null
-        || healthServerConfiguration.getPassword() == null) {
-      log.error("Health configuration: username || password not set");
-      return false;
-    }
+
     if (authorization == null) {
       log.error("Basic authentication not provided with the request");
       return false;
     }
 
-    String base64Credentials = authorization.substring("Basic".length()).trim();
-    byte[] credDecoded = Base64.decodeBase64(base64Credentials);
-    String credentials = new String(credDecoded);
-    String[] tokens = credentials.split(":");
+    String[] tokens = decodeAndGetAuthTokens(authorization);
     if (tokens.length != 2) {
       log.error("Malformed authorization header");
       return false;
@@ -202,6 +154,14 @@ private boolean isUserAuthenticated(String authorization) {
     return areBasicAuthCredentialsCorrect(username, password);
   }
 
+  private static String[] decodeAndGetAuthTokens(String authorization) {
+    String base64Credentials = authorization.substring(WWW_AUTHENTICATE_VALUE.length()).trim();
+    byte[] credDecoded = Base64.decodeBase64(base64Credentials);
+    String credentials = new String(credDecoded);
+    String[] tokens = credentials.split(":");
+    return tokens;
+  }
+
   private boolean areBasicAuthCredentialsCorrect(String username, String password) {
     return healthServerConfiguration.getUsername().equals(username)
         && healthServerConfiguration.getPassword().equals(password);

src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/conf/HealthServerConfigurationTest.java

@@ -77,4 +77,20 @@ void givenEmptyPortThrowsException() {
         IllegalArgumentException.class,
         () -> new HealthServerConfiguration("", "", null, Set.of()).init());
   }
+
+  @Test
+  void givenInvalidConfiguredEndpointsThrowsException() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> new HealthServerConfiguration("", "", 8888, Set.of("/info")).init());
+  }
+
+  @Test
+  void givenValidConfiguredEndpoints() {
+    assertDoesNotThrow(
+        () ->
+            new HealthServerConfiguration(
+                    "some-user", "some-password", 8888, Set.of("/health/prometheus"))
+                .init());
+  }
 }

src/scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/rest/BasicAuthenticationFilterTest.java

@@ -51,15 +51,15 @@ public void allowRequestIfPort8081andURIContainHealthWithoutUnprotectedEndpoints
   }
 
   @Test
-  public void denyHealthRequestWithoutUnprotectedEndpointsAndInvalidCredentials()
+  public void denyHealthRequesWithAllSecuredEndpointsAndInvalidCredentials()
       throws ServletException, IOException {
 
     req.setRequestURI("some/health/uri");
 
     String auth = username + ":" + password;
     req.addHeader("Authorization", "Basic " + Base64.encodeBase64String(auth.getBytes()));
     HealthServerConfiguration healthServerConfig =
-        new HealthServerConfiguration(null, null, 8081, Set.of());
+        new HealthServerConfiguration("", "", 8081, Set.of());
     BasicAuthenticationFilter userPwNullFilter = new BasicAuthenticationFilter(healthServerConfig);
     userPwNullFilter.doFilter(req, res, filterChainMock);