fix(scheduler): Read Health Credentials Correctly (#3948)

* fix scheduler health endpoint to read correct properties

* added acceptance test for scheduler health endpoint with basic auth

* test /health endpoint with basic authentication

* consider health configs from VCAP_Services and load in enviornment

* simplify CloudFOundryPostProcessor

* add required property in bosh based scheduler config

Author: asalan316

jobs/scheduler/templates/scheduler.yml.erb

@@ -174,6 +174,9 @@ server:
 cfserver:
   validOrgGuid: <%= p("autoscaler.scheduler.cf_server.xfcc.valid_org_guid") %>
   validSpaceGuid: <%= p("autoscaler.scheduler.cf_server.xfcc.valid_space_guid") %>
+  healthserver:
+    username: "<%=p('autoscaler.scheduler.health.username') %>"
+    password: "<%=p('autoscaler.scheduler.health.password') %>"
 
 
 #User added properties

src/autoscaler/integration/scheduler_application.template.yml

@@ -37,6 +37,9 @@ server:
 cfserver:
   validOrgGuid: some-org-guid
   validSpaceGuid: some-space-guid
+  healthserver:
+    username: health-test-user
+    password: health-test-password
 spring:
     aop:
         auto: false

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

@@ -1,5 +1,6 @@
 package org.cloudfoundry.autoscaler.scheduler.conf;
 
+import lombok.Getter;
 import lombok.Setter;
 import org.apache.catalina.connector.Connector;
 import org.springframework.boot.context.properties.ConfigurationProperties;
@@ -8,9 +9,11 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
+// TODO: may be move this to a better place e.g. CfServerConfiguration.java
 @Configuration
 @ConfigurationProperties(prefix = "server.http")
 @Setter
+@Getter
 public class CfHttpConfiguration {
 
   private int port;

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

@@ -0,0 +1,20 @@
+package org.cloudfoundry.autoscaler.scheduler.conf;
+
+import lombok.Data;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.annotation.Configuration;
+
+@Data
+@Configuration
+@ConfigurationProperties(prefix = "cfserver")
+public class CfServerConfiguration {
+  private String validOrgGuid;
+  private String validSpaceGuid;
+  private HealthServer healthserver;
+
+  @Data
+  public static class HealthServer {
+    private String username;
+    private String password;
+  }
+}

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

@@ -52,11 +52,13 @@ public void postProcessEnvironment(
         allConfigs.putAll(schedulerConfig);
       }
 
-      // Process VCAP_APPLICATION for org GUID (this should override scheduler-config)
-      Map<String, Object> vcapAppConfig = extractVcapApplicationConfig(environment);
-      if (vcapAppConfig != null && !vcapAppConfig.isEmpty()) {
-        logger.info("Found VCAP_APPLICATION, applying cfserver configuration overrides");
-        allConfigs.putAll(vcapAppConfig);
+      Map<String, Object> vcapOrgAndGuid = getOrgAndSpaceGuidFromVcap(environment);
+      Map<String, Object> cfServerConfig = (Map<String, Object>) allConfigs.get("cfserver");
+      if (cfServerConfig != null && !cfServerConfig.isEmpty()) {
+        logger.info(
+            "Found org and space guid service in VCAP_APPLICATIONS, applying configuration"
+                + " overrides");
+        cfServerConfig.putAll(vcapOrgAndGuid);
       }
 
       // Process database services
@@ -90,36 +92,28 @@ public void postProcessEnvironment(
     }
   }
 
-  private Map<String, Object> extractVcapApplicationConfig(ConfigurableEnvironment environment) {
+  private Map<String, Object> getOrgAndSpaceGuidFromVcap(ConfigurableEnvironment environment) {
+    Map<String, Object> orgSpaceMap;
     try {
       String vcapApplication = environment.getProperty(VCAP_APPLICATION);
 
       if (vcapApplication == null || vcapApplication.trim().isEmpty()) {
         logger.debug("VCAP_APPLICATION not found or empty, skipping org GUID extraction");
         return null;
       }
-
-      TypeReference<Map<String, Object>> typeRef = new TypeReference<Map<String, Object>>() {};
+      TypeReference<Map<String, Object>> typeRef = new TypeReference<>() {};
       Map<String, Object> vcapApp = objectMapper.readValue(vcapApplication, typeRef);
 
-      Map<String, Object> config = new java.util.HashMap<>();
-      boolean foundConfig = false;
+      orgSpaceMap = new java.util.HashMap<>();
 
       // Extract organization_id
       Object organizationId = vcapApp.get("organization_id");
       if (organizationId instanceof String && !((String) organizationId).trim().isEmpty()) {
         String orgGuid = (String) organizationId;
         logger.info(
             "Setting cfserver.validOrgGuid from VCAP_APPLICATION organization_id: {}", orgGuid);
+        orgSpaceMap.put("validOrgGuid", orgGuid);
 
-        // Create nested structure to match scheduler config format
-        Map<String, Object> cfserverConfig = (Map<String, Object>) config.get("cfserver");
-        if (cfserverConfig == null) {
-          cfserverConfig = new java.util.HashMap<>();
-          config.put("cfserver", cfserverConfig);
-        }
-        cfserverConfig.put("validOrgGuid", orgGuid);
-        foundConfig = true;
       } else {
         logger.warn("organization_id not found or empty in VCAP_APPLICATION");
       }
@@ -130,24 +124,15 @@ private Map<String, Object> extractVcapApplicationConfig(ConfigurableEnvironment
         String spaceGuid = (String) spaceId;
         logger.info(
             "Setting cfserver.validSpaceGuid from VCAP_APPLICATION space_id: {}", spaceGuid);
-
-        // Create nested structure to match scheduler config format
-        Map<String, Object> cfserverConfig = (Map<String, Object>) config.get("cfserver");
-        if (cfserverConfig == null) {
-          cfserverConfig = new java.util.HashMap<>();
-          config.put("cfserver", cfserverConfig);
-        }
-        cfserverConfig.put("validSpaceGuid", spaceGuid);
-        foundConfig = true;
+        orgSpaceMap.put("validSpaceGuid", spaceGuid);
       } else {
         logger.warn("space_id not found or empty in VCAP_APPLICATION");
       }
-
-      return foundConfig ? config : null;
     } catch (Exception e) {
       logger.error("Failed to parse VCAP_APPLICATION JSON", e);
       return null;
     }
+    return orgSpaceMap;
   }
 
   private Map<String, Object> extractSchedulerConfig(String vcapServices) {
@@ -420,11 +405,9 @@ private Map<String, Object> extractCfInstanceCertificates(ConfigurableEnvironmen
 
   private Map<String, Object> flattenConfiguration(String prefix, Map<String, Object> config) {
     Map<String, Object> flattened = new java.util.HashMap<>();
-
     config.forEach(
         (key, value) -> {
           String fullKey = prefix.isEmpty() ? key : prefix + "." + key;
-
           if (value instanceof Map) {
             @SuppressWarnings("unchecked")
             Map<String, Object> nestedMap = (Map<String, Object>) value;
@@ -450,6 +433,6 @@ private Map<String, Object> extractCredentialsFromService(Map<String, Object> se
       Map<String, Object> credentialsMap = (Map<String, Object>) credentials;
       return credentialsMap;
     }
-    return Map.<String, Object>of();
+    return Map.of();
   }
 }

src/autoscaler/scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/filter/HttpAuthFilter.java

@@ -6,145 +6,128 @@
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Base64;
-import lombok.Setter;
+import lombok.RequiredArgsConstructor;
+import org.cloudfoundry.autoscaler.scheduler.conf.CfServerConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Value;
-import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.core.annotation.Order;
 import org.springframework.stereotype.Component;
 import org.springframework.web.filter.OncePerRequestFilter;
 
 @Component
 @Order(0)
-@ConfigurationProperties(prefix = "cfserver")
-@Setter
+@RequiredArgsConstructor
 public class HttpAuthFilter extends OncePerRequestFilter {
-  private Logger logger = LoggerFactory.getLogger(this.getClass());
-
-  private String validSpaceGuid;
-  private String validOrgGuid;
-
-  @Value("${cfserver.healthserver.username}")
-  private String healthServerUsername;
 
-  @Value("${cfserver.healthserver.password}")
-  private String healthServerPassword;
-
-  public void setHealthServerUsername(String healthServerUsername) {
-    this.healthServerUsername = healthServerUsername;
-  }
+  private static final String AUTHORIZATION_HEADER = "Authorization";
+  private static final String BASIC_AUTH_PREFIX = "Basic ";
+  private static final String XFCC_HEADER = "X-Forwarded-Client-Cert";
+  private static final String HEALTH_ENDPOINT = "/health";
 
-  public void setHealthServerPassword(String healthServerPassword) {
-    this.healthServerPassword = healthServerPassword;
-  }
+  private Logger logger = LoggerFactory.getLogger(this.getClass());
+  private final CfServerConfiguration cfServerConfiguration;
 
   @Override
   protected void doFilterInternal(
       HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
       throws ServletException, IOException {
 
-    logger.info(
-        "Received request with request "
-            + request.getRequestURI()
-            + " method"
-            + request.getMethod());
-
-    // Debug logging
     String forwardedProto = request.getHeader("X-Forwarded-Proto");
-    boolean isHealthEndpoint = request.getRequestURI().contains("/health");
+    boolean isHealthEndpoint = request.getRequestURI().contains(HEALTH_ENDPOINT);
+
     logger.info(
-        "DEBUG: scheme={}, X-Forwarded-Proto={}, isHealthEndpoint={}, healthServerUsername={},"
-            + " healthServerPassword={}",
+        "Received {} request, scheme={},X-Forwarded-Proto={}, isHealthEndpoint={}, username={}",
+        request.getMethod(),
         request.getScheme(),
         forwardedProto,
         isHealthEndpoint,
-        healthServerUsername,
-        healthServerPassword);
-
-    // Skip filter if X-Forwarded-Client-Cert is missing and not a health request
-    String xfccHeader = request.getHeader("X-Forwarded-Client-Cert");
-    if ((xfccHeader == null || xfccHeader.isEmpty()) && !isHealthEndpoint) {
-      logger.info(
-          "DEBUG: Skipping request without X-Forwarded-Client-Cert - URI={}",
-          request.getRequestURI());
-      filterChain.doFilter(request, response);
-      return;
-    }
-
-    // handles /health endpoint with basic auth
-    if (request.getRequestURI().contains("/health")) {
-      logger.info("DEBUG: Processing health endpoint request");
-      // parse request basic auth header
-      String authHeader = request.getHeader("Authorization");
-      logger.info("DEBUG: Authorization header: {}", authHeader != null ? "present" : "missing");
-      if (authHeader == null || !authHeader.startsWith("Basic ")) {
-        logger.warn("Missing or invalid Authorization header for health check request");
-        response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
-        return;
-      }
-      String[] credentials =
-          new String(Base64.getDecoder().decode(authHeader.substring(6))).split(":");
-
-      if (credentials.length != 2) {
-        logger.warn("Invalid Authorization header format for health check request");
-        response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Bad Request");
-        return;
-      }
-      if (!credentials[0].equals(healthServerUsername)
-          || !credentials[1].equals(healthServerPassword)) {
-        logger.warn("Invalid credentials for health check request");
-        response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
-        return;
-      } else {
-        response.setStatus(HttpServletResponse.SC_OK);
-        response.setContentType("application/json");
-        response.getWriter().write("{\"status\":\"UP\"}");
-        response.getWriter().flush();
-      }
+        cfServerConfiguration.getHealthserver().getUsername());
 
+    if (isHealthEndpoint) {
+      handleHealthEndpoint(request, response);
       return;
     }
-
+    // Check for XFCC header
+    String xfccHeader = request.getHeader(XFCC_HEADER);
     if (xfccHeader == null || xfccHeader.isEmpty()) {
-      logger.warn("Missing X-Forwarded-Client-Cert header");
-      response.sendError(
-          HttpServletResponse.SC_BAD_REQUEST,
-          "Missing X-Forwarded-Client-Cert header in the request");
+      logger.warn("Missing X-Forwarded-Client-Cert header, URI={}", request.getRequestURI());
+      filterChain.doFilter(request, response);
       return;
     }
-    logger.info(
-        "X-Forwarded-Client-Cert header received ... checking authorized org and space in OU");
-    logger.info("X-Forwarded-Client-Cert header: " + xfccHeader);
+    validateOrganizationAndSpace(xfccHeader, response);
+    filterChain.doFilter(request, response);
+  }
 
+  private void validateOrganizationAndSpace(String xfccHeader, HttpServletResponse response)
+      throws IOException {
+    logger.info(
+        "X-Forwarded-Client-Cert header received ... checking authorized cf organization and space"
+            + " in Organizational Unit");
     try {
       String organizationalUnit = extractOrganizationalUnit(xfccHeader);
-
       // Validate both key-value pairs in OrganizationalUnit
       if (!isValidOrganizationalUnit(organizationalUnit)) {
         logger.warn("Unauthorized OrganizationalUnit: " + organizationalUnit);
         response.sendError(HttpServletResponse.SC_FORBIDDEN, "Unauthorized OrganizationalUnit");
-        return;
       }
-    } catch (Exception e) {
+    } catch (CertificateException e) {
       logger.warn("Invalid certificate: " + e.getMessage());
       response.sendError(
           HttpServletResponse.SC_BAD_REQUEST, "Invalid certificate: " + e.getMessage());
+    }
+  }
+
+  private void handleHealthEndpoint(HttpServletRequest request, HttpServletResponse response)
+      throws IOException {
+    logger.info("Handling health check request with Basic Auth");
+    String authHeader = request.getHeader(AUTHORIZATION_HEADER);
+    logger.info("Authorization header: {}", authHeader != null ? "present" : "missing");
+
+    if (authHeader == null || !authHeader.startsWith(BASIC_AUTH_PREFIX)) {
+      logger.warn("Missing or invalid Authorization header for health check request");
+      response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
       return;
     }
-    // Proceed with the request
-    filterChain.doFilter(request, response);
+    String[] credentials = decodeBasicAuth(authHeader);
+    if (credentials.length != 2) {
+      logger.warn("Invalid Authorization header format for health check request");
+      response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Bad Request");
+      return;
+    }
+    if (!credentials[0].equals(cfServerConfiguration.getHealthserver().getUsername())
+        || !credentials[1].equals(cfServerConfiguration.getHealthserver().getPassword())) {
+      logger.warn("Invalid credentials for health check request");
+      response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
+      return;
+    }
+
+    response.setStatus(HttpServletResponse.SC_OK);
+    response.setContentType("application/json");
+    response.getWriter().write("{\"status\":\"UP\"}");
+    response.getWriter().flush();
+  }
+
+  private String[] decodeBasicAuth(String authHeader) {
+    try {
+      return new String(
+              Base64.getDecoder().decode(authHeader.substring(BASIC_AUTH_PREFIX.length())))
+          .split(":");
+    } catch (IllegalArgumentException e) {
+      logger.warn("Failed to decode Basic Auth header: {}", e.getMessage());
+      return null;
+    }
   }
 
-  private String extractOrganizationalUnit(String certValue) throws Exception {
+  private String extractOrganizationalUnit(String certValue) throws CertificateException {
     X509Certificate certificate = parseCertificate(certValue);
     return certificate.getSubjectX500Principal().getName();
   }
 
-  private X509Certificate parseCertificate(String certValue) throws Exception {
+  private X509Certificate parseCertificate(String certValue) throws CertificateException {
     // Extract the base64-encoded certificate from the XFCC header
     String base64Cert =
         certValue
@@ -159,8 +142,10 @@ private X509Certificate parseCertificate(String certValue) throws Exception {
   }
 
   private boolean isValidOrganizationalUnit(String organizationalUnit) {
-    boolean isSpaceValid = organizationalUnit.contains("space:" + validSpaceGuid);
-    boolean isOrgValid = organizationalUnit.contains("organization:" + validOrgGuid);
+    boolean isSpaceValid =
+        organizationalUnit.contains("space:" + cfServerConfiguration.getValidSpaceGuid());
+    boolean isOrgValid =
+        organizationalUnit.contains("organization:" + cfServerConfiguration.getValidOrgGuid());
     return isSpaceValid && isOrgValid;
   }
 }