Improve Http controller authentication filters log (#1686)

make it to log with the class name

Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-03-15 10:21:25 +02:00
committed by GitHub
parent 60e25b47ec
commit fca2e9b0ae
6 changed files with 57 additions and 47 deletions

View File

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

View File

@@ -9,9 +9,11 @@
*/ */
package org.eclipse.hawkbit.security; package org.eclipse.hawkbit.security;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.TenantAware;
import org.slf4j.Logger;
/** /**
* An pre-authenticated processing filter which add the * An pre-authenticated processing filter which add the
@@ -19,19 +21,15 @@ import org.eclipse.hawkbit.tenancy.TenantAware;
* security context in case the anonymous download is allowed through * security context in case the anonymous download is allowed through
* configuration. * configuration.
*/ */
@Slf4j
public class HttpControllerPreAuthenticateAnonymousDownloadFilter extends AbstractHttpControllerAuthenticationFilter { public class HttpControllerPreAuthenticateAnonymousDownloadFilter extends AbstractHttpControllerAuthenticationFilter {
/** /**
* Constructor. * Constructor.
* *
* @param tenantConfigurationManagement * @param tenantConfigurationManagement the system management service to retrieve configuration properties
* the system management service to retrieve configuration * @param tenantAware the tenant aware service to get configuration for the specific tenant
* properties * @param systemSecurityContext the system security context
* @param tenantAware
* the tenant aware service to get configuration for the specific
* tenant
* @param systemSecurityContext
* the system security context
*/ */
public HttpControllerPreAuthenticateAnonymousDownloadFilter( public HttpControllerPreAuthenticateAnonymousDownloadFilter(
final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware, final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware,
@@ -45,4 +43,8 @@ public class HttpControllerPreAuthenticateAnonymousDownloadFilter extends Abstra
systemSecurityContext); systemSecurityContext);
} }
@Override
protected Logger log() {
return log;
}
} }

View File

@@ -9,9 +9,11 @@
*/ */
package org.eclipse.hawkbit.security; package org.eclipse.hawkbit.security;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.repository.ControllerManagement; import org.eclipse.hawkbit.repository.ControllerManagement;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.TenantAware;
import org.slf4j.Logger;
/** /**
* An pre-authenticated processing filter which extracts (if enabled through * An pre-authenticated processing filter which extracts (if enabled through
@@ -25,10 +27,8 @@ import org.eclipse.hawkbit.tenancy.TenantAware;
* custom headers which have then weird side-effects. Furthermore frameworks are * 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 * aware of the sensitivity of the Authorization header and do not log it and
* store it somewhere. * store it somewhere.
*
*
*
*/ */
@Slf4j
public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHttpControllerAuthenticationFilter { public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHttpControllerAuthenticationFilter {
private final ControllerManagement controllerManagement; private final ControllerManagement controllerManagement;
@@ -61,4 +61,8 @@ public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHt
tenantAware, systemSecurityContext); tenantAware, systemSecurityContext);
} }
@Override
protected Logger log() {
return log;
}
} }

View File

@@ -9,8 +9,10 @@
*/ */
package org.eclipse.hawkbit.security; package org.eclipse.hawkbit.security;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.TenantAware;
import org.slf4j.Logger;
/** /**
* Extract the {@code Authorization} header is a HTTP standard and reverse proxy * Extract the {@code Authorization} header is a HTTP standard and reverse proxy
@@ -18,10 +20,8 @@ import org.eclipse.hawkbit.tenancy.TenantAware;
* maybe custom headers which have then weird side-effects. Furthermore * maybe custom headers which have then weird side-effects. Furthermore
* frameworks are aware of the sensitivity of the Authorization header and do * frameworks are aware of the sensitivity of the Authorization header and do
* not log it and store it somewhere. * not log it and store it somewhere.
*
*
*
*/ */
@Slf4j
public class HttpControllerPreAuthenticatedGatewaySecurityTokenFilter public class HttpControllerPreAuthenticatedGatewaySecurityTokenFilter
extends AbstractHttpControllerAuthenticationFilter { extends AbstractHttpControllerAuthenticationFilter {
@@ -49,4 +49,8 @@ public class HttpControllerPreAuthenticatedGatewaySecurityTokenFilter
systemSecurityContext); systemSecurityContext);
} }
@Override
protected Logger log() {
return log;
}
} }

View File

@@ -9,16 +9,16 @@
*/ */
package org.eclipse.hawkbit.security; package org.eclipse.hawkbit.security;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.TenantAware;
import org.slf4j.Logger;
/** /**
* An pre-authenticated processing filter which extracts the principal from a * An pre-authenticated processing filter which extracts the principal from a
* request URI and the credential from a request header. * request URI and the credential from a request header.
*
*
*
*/ */
@Slf4j
public class HttpControllerPreAuthenticatedSecurityHeaderFilter extends AbstractHttpControllerAuthenticationFilter { public class HttpControllerPreAuthenticatedSecurityHeaderFilter extends AbstractHttpControllerAuthenticationFilter {
private final String caCommonNameHeader; private final String caCommonNameHeader;
@@ -60,4 +60,8 @@ public class HttpControllerPreAuthenticatedSecurityHeaderFilter extends Abstract
tenantConfigurationManagement, tenantAware, systemSecurityContext); tenantConfigurationManagement, tenantAware, systemSecurityContext);
} }
@Override
protected Logger log() {
return log;
}
} }

View File

@@ -42,5 +42,4 @@ public class ControllerPreAuthenticatedAnonymousFilter implements PreAuthenticat
public boolean isEnable(final DmfTenantSecurityToken securityToken) { public boolean isEnable(final DmfTenantSecurityToken securityToken) {
return ddiSecurityConfiguration.getAuthentication().getAnonymous().isEnabled(); return ddiSecurityConfiguration.getAuthentication().getAnonymous().isEnabled();
} }
} }