Improve Http controller authentication filters log (#1686)

make it to log with the class name Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>
eclipse · Mar 15, 2024 · fca2e9b · fca2e9b
1 parent 60e25b4
commit fca2e9b
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 47 deletions.
diff --git a/...rc/main/java/org/eclipse/hawkbit/security/AbstractHttpControllerAuthenticationFilter.java b/...rc/main/java/org/eclipse/hawkbit/security/AbstractHttpControllerAuthenticationFilter.java
@@ -27,6 +27,7 @@
 import org.eclipse.hawkbit.security.DmfTenantSecurityToken.FileResource;
 import org.eclipse.hawkbit.tenancy.TenantAware;
 import org.eclipse.hawkbit.util.UrlUtils;
+import org.slf4j.Logger;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -39,7 +40,6 @@
  * name from the URL and the controller ID from the URL to do security checks
  * based on this information.
  */
-@Slf4j
 public abstract class AbstractHttpControllerAuthenticationFilter extends AbstractPreAuthenticatedProcessingFilter {
 
     private static final String TENANT_PLACE_HOLDER = "tenant";
@@ -64,14 +64,11 @@ public abstract class AbstractHttpControllerAuthenticationFilter extends Abstrac
     private PreAuthenticationFilter abstractControllerAuthenticationFilter;
 
     /**
-     * Constructor for sub-classes.
+     * Constructor for subclasses.
      * 
-     * @param tenantConfigurationManagement
-     *            the tenant configuration service
-     * @param tenantAware
-     *            the tenant aware service
-     * @param systemSecurityContext
-     *            the system secruity context
+     * @param tenantConfigurationManagement the tenant configuration service
+     * @param tenantAware the tenant aware service
+     * @param systemSecurityContext the system security context
      */
     protected AbstractHttpControllerAuthenticationFilter(
             final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware,
@@ -85,6 +82,11 @@ protected AbstractHttpControllerAuthenticationFilter(
     @Override
     public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain)
             throws IOException, ServletException {
+        if (SecurityContextHolder.getContext().getAuthentication() != null) {
+            log().trace("Request is already authenticated. Skip filter");
+            chain.doFilter(request, response);
+            return;
+        }
 
         if (!(request instanceof HttpServletRequest)) {
             chain.doFilter(request, response);
@@ -96,9 +98,10 @@ public void doFilter(final ServletRequest request, final ServletResponse respons
             chain.doFilter(request, response);
             return;
         }
+
         abstractControllerAuthenticationFilter = createControllerAuthenticationFilter();
-        if (abstractControllerAuthenticationFilter.isEnable(securityToken)
-                && SecurityContextHolder.getContext().getAuthentication() == null) {
+        if (abstractControllerAuthenticationFilter.isEnable(securityToken)) {
+            log().debug("Filter is disabled for the tenant {}", securityToken.getTenant());
             super.doFilter(request, response, chain);
         } else {
             chain.doFilter(request, response);
@@ -119,11 +122,12 @@ protected void successfulAuthentication(final HttpServletRequest request, final
         super.successfulAuthentication(request, response, authTokenWithGrantedAuthorities);
     }
 
+    protected abstract Logger log();
+
     /**
      * Extracts tenant and controllerId from the request URI as path variables.
      * 
-     * @param request
-     *            the Http request to extract the path variables.
+     * @param request the Http request to extract the path variables.
      * @return the extracted {@link DmfTenantSecurityToken} or {@code null} if the
      *         request does not match the pattern and no variables could be
      *         extracted
@@ -132,30 +136,23 @@ protected DmfTenantSecurityToken createTenantSecurityTokenVariables(final HttpSe
         final String requestURI = request.getRequestURI();
 
         if (pathExtractor.match(request.getContextPath() + CONTROLLER_REQUEST_ANT_PATTERN, requestURI)) {
-            log.debug("retrieving principal from URI request {}", requestURI);
+            log().debug("retrieving principal from URI request {}", requestURI);
             final Map<String, String> extractUriTemplateVariables = pathExtractor
                     .extractUriTemplateVariables(request.getContextPath() + CONTROLLER_REQUEST_ANT_PATTERN, requestURI);
             final String controllerId = UrlUtils.decodeUriValue(extractUriTemplateVariables.get(CONTROLLER_ID_PLACE_HOLDER));
             final String tenant = UrlUtils.decodeUriValue(extractUriTemplateVariables.get(TENANT_PLACE_HOLDER));
-            if (log.isTraceEnabled()) {
-                log.trace("Parsed tenant {} and controllerId {} from path request {}", tenant, controllerId,
-                        requestURI);
-            }
+            log().trace("Parsed tenant {} and controllerId {} from path request {}", tenant, controllerId, requestURI);
             return createTenantSecurityTokenVariables(request, tenant, controllerId);
         } else if (pathExtractor.match(request.getContextPath() + CONTROLLER_DL_REQUEST_ANT_PATTERN, requestURI)) {
-            log.debug("retrieving path variables from URI request {}", requestURI);
+            log().debug("retrieving path variables from URI request {}", requestURI);
             final Map<String, String> extractUriTemplateVariables = pathExtractor.extractUriTemplateVariables(
                     request.getContextPath() + CONTROLLER_DL_REQUEST_ANT_PATTERN, requestURI);
             final String tenant = UrlUtils.decodeUriValue(extractUriTemplateVariables.get(TENANT_PLACE_HOLDER));
-            if (log.isTraceEnabled()) {
-                log.trace("Parsed tenant {} from path request {}", tenant, requestURI);
-            }
+            log().trace("Parsed tenant {} from path request {}", tenant, requestURI);
             return createTenantSecurityTokenVariables(request, tenant, "anonymous");
         } else {
-            if (log.isTraceEnabled()) {
-                log.trace("request {} does not match the path pattern {}, request gets ignored", requestURI,
+            log().trace("request {} does not match the path pattern {}, request gets ignored", requestURI,
                         CONTROLLER_REQUEST_ANT_PATTERN);
-            }
             return null;
         }
     }

diff --git a/...va/org/eclipse/hawkbit/security/HttpControllerPreAuthenticateAnonymousDownloadFilter.java b/...va/org/eclipse/hawkbit/security/HttpControllerPreAuthenticateAnonymousDownloadFilter.java
@@ -9,29 +9,27 @@
  */
 package org.eclipse.hawkbit.security;
 
+import lombok.extern.slf4j.Slf4j;
 import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions;
 import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.tenancy.TenantAware;
+import org.slf4j.Logger;
 
 /**
  * An pre-authenticated processing filter which add the
  * {@link SpringEvalExpressions#CONTROLLER_DOWNLOAD_ROLE_ANONYMOUS} to the
  * security context in case the anonymous download is allowed through
  * configuration.
  */
+@Slf4j
 public class HttpControllerPreAuthenticateAnonymousDownloadFilter extends AbstractHttpControllerAuthenticationFilter {
 
     /**
      * Constructor.
      * 
-     * @param tenantConfigurationManagement
-     *            the system management service to retrieve configuration
-     *            properties
-     * @param tenantAware
-     *            the tenant aware service to get configuration for the specific
-     *            tenant
-     * @param systemSecurityContext
-     *            the system security context
+     * @param tenantConfigurationManagement the system management service to retrieve configuration properties
+     * @param tenantAware the tenant aware service to get configuration for the specific tenant
+     * @param systemSecurityContext the system security context
      */
     public HttpControllerPreAuthenticateAnonymousDownloadFilter(
             final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware,
@@ -45,4 +43,8 @@ protected PreAuthenticationFilter createControllerAuthenticationFilter() {
                 systemSecurityContext);
     }
 
-}
+    @Override
+    protected Logger log() {
+        return log;
+    }
+}
diff --git a/...n/java/org/eclipse/hawkbit/security/HttpControllerPreAuthenticateSecurityTokenFilter.java b/...n/java/org/eclipse/hawkbit/security/HttpControllerPreAuthenticateSecurityTokenFilter.java
@@ -9,9 +9,11 @@
  */
 package org.eclipse.hawkbit.security;
 
+import lombok.extern.slf4j.Slf4j;
 import org.eclipse.hawkbit.repository.ControllerManagement;
 import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.tenancy.TenantAware;
+import org.slf4j.Logger;
 
 /**
  * An pre-authenticated processing filter which extracts (if enabled through
@@ -25,10 +27,8 @@
  * custom headers which have then weird side-effects. Furthermore frameworks are
  * aware of the sensitivity of the Authorization header and do not log it and
  * store it somewhere.
- *
- *
- *
  */
+@Slf4j
 public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHttpControllerAuthenticationFilter {
 
     private final ControllerManagement controllerManagement;
@@ -61,4 +61,8 @@ protected PreAuthenticationFilter createControllerAuthenticationFilter() {
                 tenantAware, systemSecurityContext);
     }
 
-}
+    @Override
+    protected Logger log() {
+        return log;
+    }
+}
diff --git a/...rg/eclipse/hawkbit/security/HttpControllerPreAuthenticatedGatewaySecurityTokenFilter.java b/...rg/eclipse/hawkbit/security/HttpControllerPreAuthenticatedGatewaySecurityTokenFilter.java
@@ -9,19 +9,19 @@
  */
 package org.eclipse.hawkbit.security;
 
+import lombok.extern.slf4j.Slf4j;
 import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.tenancy.TenantAware;
+import org.slf4j.Logger;
 
 /**
  * Extract the {@code Authorization} header is a HTTP standard and reverse proxy
  * or other proxies will keep the Authorization headers untouched instead of
  * maybe custom headers which have then weird side-effects. Furthermore
  * frameworks are aware of the sensitivity of the Authorization header and do
  * not log it and store it somewhere.
- *
- *
- *
  */
+@Slf4j
 public class HttpControllerPreAuthenticatedGatewaySecurityTokenFilter
         extends AbstractHttpControllerAuthenticationFilter {
 
@@ -49,4 +49,8 @@ protected PreAuthenticationFilter createControllerAuthenticationFilter() {
                 systemSecurityContext);
     }
 
-}
+    @Override
+    protected Logger log() {
+        return log;
+    }
+}
diff --git a/...java/org/eclipse/hawkbit/security/HttpControllerPreAuthenticatedSecurityHeaderFilter.java b/...java/org/eclipse/hawkbit/security/HttpControllerPreAuthenticatedSecurityHeaderFilter.java
@@ -9,16 +9,16 @@
  */
 package org.eclipse.hawkbit.security;
 
+import lombok.extern.slf4j.Slf4j;
 import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.tenancy.TenantAware;
+import org.slf4j.Logger;
 
 /**
  * An pre-authenticated processing filter which extracts the principal from a
  * request URI and the credential from a request header.
- * 
- *
- *
  */
+@Slf4j
 public class HttpControllerPreAuthenticatedSecurityHeaderFilter extends AbstractHttpControllerAuthenticationFilter {
 
     private final String caCommonNameHeader;
@@ -60,4 +60,8 @@ protected PreAuthenticationFilter createControllerAuthenticationFilter() {
                 tenantConfigurationManagement, tenantAware, systemSecurityContext);
     }
 
-}
+    @Override
+    protected Logger log() {
+        return log;
+    }
+}
diff --git a/...src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedAnonymousFilter.java b/...src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedAnonymousFilter.java
@@ -42,5 +42,4 @@ public HeaderAuthentication getPreAuthenticatedCredentials(final DmfTenantSecuri
     public boolean isEnable(final DmfTenantSecurityToken securityToken) {
         return ddiSecurityConfiguration.getAuthentication().getAnonymous().isEnabled();
     }
-
-}
+}