From a70158e34fd339d3e4704133c7354c3e4140850c Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Fri, 16 Oct 2026 15:44:56 +0200 Subject: [PATCH] ESB-1296 Stop /api/health from creating sessions on every call --- .../apsadmin/ApsAdminBaseTestCase.java | 2 +- .../aps/util/ApsTenantApplicationUtils.java | 2 +- .../java/com/agiletec/aps/BaseTestCase.java | 8 +- .../util/ApsTenantApplicationUtilsTest.java | 22 +++- .../web/filter/MDCTenantFilterTest.java | 27 +++++ .../security/KeycloakSecurityConfig.java | 16 +++ .../servlet/security/BasicAuthFilterTest.java | 1 + .../KeycloakAuthenticationFilterTest.java | 4 +- .../security/KeycloakSecurityConfigTest.java | 103 ++++++++++++++++++ .../keycloak/filter/KeycloakFilterTest.java | 3 +- .../control/TestTenantController.java | 2 +- .../apsadmin/ApsAdminPluginBaseTestCase.java | 2 +- 12 files changed, 183 insertions(+), 9 deletions(-) diff --git a/admin-console/src/test/java/com/agiletec/apsadmin/ApsAdminBaseTestCase.java b/admin-console/src/test/java/com/agiletec/apsadmin/ApsAdminBaseTestCase.java index 69a83ea54..4fe1fd94e 100644 --- a/admin-console/src/test/java/com/agiletec/apsadmin/ApsAdminBaseTestCase.java +++ b/admin-console/src/test/java/com/agiletec/apsadmin/ApsAdminBaseTestCase.java @@ -74,7 +74,7 @@ protected static void setUp() throws Exception { refresh = true; } RequestContext reqCtx = BaseTestCase.createRequestContext(applicationContext, servletContext); - request = BaseTestCase.createRequest(); + request = BaseTestCase.createRequest(servletContext); reqCtx.setRequest(request); request.setAttribute(RequestContext.REQCTX, reqCtx); response = new MockHttpServletResponse(); diff --git a/engine/src/main/java/com/agiletec/aps/util/ApsTenantApplicationUtils.java b/engine/src/main/java/com/agiletec/aps/util/ApsTenantApplicationUtils.java index 394c86412..e130a0d17 100644 --- a/engine/src/main/java/com/agiletec/aps/util/ApsTenantApplicationUtils.java +++ b/engine/src/main/java/com/agiletec/aps/util/ApsTenantApplicationUtils.java @@ -43,7 +43,7 @@ public static Optional extractCurrentTenantCode(HttpServletRequest reque } else { logger.debug("the custom header:'{}' is empty or blank, skip it", UrlUtils.ENTANDO_TENANT_CODE_CUSTOM_HEADER); String domain = getDomainFromRequest(request); - ITenantManager tenantManager = ApsWebApplicationUtils.getBean(ITenantManager.class, request); + ITenantManager tenantManager = ApsWebApplicationUtils.getBean(ITenantManager.class, request.getServletContext()); String tenantCodeFromDomain = tenantManager.getTenantCodeByDomain(domain); logger.debug("the tenantCodeFromDomain is:'{}', return it", tenantCodeFromDomain); return Optional.ofNullable(tenantCodeFromDomain); diff --git a/engine/src/test/java/com/agiletec/aps/BaseTestCase.java b/engine/src/test/java/com/agiletec/aps/BaseTestCase.java index 0b56a6230..241d6333f 100644 --- a/engine/src/test/java/com/agiletec/aps/BaseTestCase.java +++ b/engine/src/test/java/com/agiletec/aps/BaseTestCase.java @@ -71,7 +71,7 @@ public static void setUp(ConfigTestUtils configTestUtils) throws Exception { refresh = true; } reqCtx = createRequestContext(applicationContext, servletContext); - request = createRequest(); + request = createRequest(servletContext); request.setAttribute(RequestContext.REQCTX, reqCtx); request.setSession(new MockHttpSession(servletContext)); reqCtx.setRequest(request); @@ -96,7 +96,11 @@ public static RequestContext createRequestContext(ApplicationContext application } public static MockHttpServletRequest createRequest() { - MockHttpServletRequest request = new MockHttpServletRequest(); + return createRequest(null); + } + + public static MockHttpServletRequest createRequest(ServletContext servletContext) { + MockHttpServletRequest request = new MockHttpServletRequest(servletContext); request.setScheme("http"); request.setServerName("www.entando.com"); request.addHeader("Host", "www.entando.com"); diff --git a/engine/src/test/java/com/agiletec/aps/util/ApsTenantApplicationUtilsTest.java b/engine/src/test/java/com/agiletec/aps/util/ApsTenantApplicationUtilsTest.java index 3952c0ff9..64599aa21 100644 --- a/engine/src/test/java/com/agiletec/aps/util/ApsTenantApplicationUtilsTest.java +++ b/engine/src/test/java/com/agiletec/aps/util/ApsTenantApplicationUtilsTest.java @@ -14,6 +14,7 @@ package com.agiletec.aps.util; import java.util.Optional; +import jakarta.servlet.ServletContext; import jakarta.servlet.http.HttpServletRequest; import org.assertj.core.api.Assertions; @@ -24,6 +25,9 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockServletContext; +import org.springframework.web.context.WebApplicationContext; @ExtendWith(MockitoExtension.class) class ApsTenantApplicationUtilsTest { @@ -37,7 +41,7 @@ class ApsTenantApplicationUtilsTest { void shouldExtractCorrectTenantDomainWithEveryInput(){ try(MockedStatic apsWebApplicationUtils = Mockito.mockStatic(ApsWebApplicationUtils.class)){ - apsWebApplicationUtils.when(() -> ApsWebApplicationUtils.getBean(ITenantManager.class,httpServletRequest)).thenReturn(tenantManager); + apsWebApplicationUtils.when(() -> ApsWebApplicationUtils.getBean(Mockito.eq(ITenantManager.class), Mockito.any())).thenReturn(tenantManager); Mockito.when(httpServletRequest.getHeader("X-ENTANDO-TENANTCODE")).thenReturn("tenant2"); Mockito.when(httpServletRequest.getServerName()).thenReturn("www.test.com"); @@ -72,6 +76,22 @@ void shouldExtractCorrectTenantDomainWithEveryInput(){ } } + @Test + void shouldNotCreateSessionWhenExtractingTenantFromDomain() { + MockServletContext servletContext = new MockServletContext(); + WebApplicationContext wac = Mockito.mock(WebApplicationContext.class); + Mockito.when(wac.getBean(ITenantManager.class)).thenReturn(tenantManager); + servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, wac); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext); + request.setServerName("www.test.com"); + Mockito.when(tenantManager.getTenantCodeByDomain("www.test.com")).thenReturn("wwwCode"); + + Optional res = ApsTenantApplicationUtils.extractCurrentTenantCode(request); + + Assertions.assertThat(res).contains("wwwCode"); + Assertions.assertThat(request.getSession(false)).isNull(); + } + @Test void shouldGetSetRemoveWorkCorrectTenantCodeWithEveryInput() { diff --git a/engine/src/test/java/org/entando/entando/web/filter/MDCTenantFilterTest.java b/engine/src/test/java/org/entando/entando/web/filter/MDCTenantFilterTest.java index 500b38ac8..cecae51c3 100644 --- a/engine/src/test/java/org/entando/entando/web/filter/MDCTenantFilterTest.java +++ b/engine/src/test/java/org/entando/entando/web/filter/MDCTenantFilterTest.java @@ -12,8 +12,13 @@ import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; +import org.entando.entando.aps.system.services.tenants.ITenantManager; import org.mockito.junit.jupiter.MockitoExtension; import org.slf4j.MDC; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletContext; +import org.springframework.web.context.WebApplicationContext; @ExtendWith(MockitoExtension.class) class MDCTenantFilterTest { @@ -54,6 +59,28 @@ void shouldSetPrimaryTenantToMDC() throws Exception { } } + @Test + void shouldNotCreateSessionWhenResolvingTenantFromDomain() throws Exception { + ITenantManager tenantManager = Mockito.mock(ITenantManager.class); + Mockito.when(tenantManager.getTenantCodeByDomain("tenant1.test.com")).thenReturn("tenant1"); + WebApplicationContext wac = Mockito.mock(WebApplicationContext.class); + Mockito.when(wac.getBean(ITenantManager.class)).thenReturn(tenantManager); + MockServletContext servletContext = new MockServletContext(); + servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, wac); + MockHttpServletRequest mockRequest = new MockHttpServletRequest(servletContext); + mockRequest.setServerName("tenant1.test.com"); + String[] tenantInChain = new String[1]; + Mockito.doAnswer(invocation -> { + tenantInChain[0] = MDC.get("tenant"); + return null; + }).when(chain).doFilter(Mockito.any(), Mockito.any()); + + filter.doFilter(mockRequest, new MockHttpServletResponse(), chain); + + Assertions.assertEquals("tenant1", tenantInChain[0]); + Assertions.assertNull(mockRequest.getSession(false)); + } + @Test void shouldRemoveMDCKeyInCaseOfException() throws Exception { try (MockedStatic mdc = Mockito.mockStatic(MDC.class); diff --git a/keycloak-plugin/src/main/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfig.java b/keycloak-plugin/src/main/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfig.java index 4f8e3ec06..9b900565e 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfig.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfig.java @@ -14,6 +14,7 @@ import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; import org.springframework.security.config.annotation.web.configurers.HeadersConfigurer; +import org.springframework.security.config.annotation.web.configurers.RequestCacheConfigurer; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; @@ -47,6 +48,21 @@ public KeycloakSecurityConfig(final KeycloakAuthenticationFilter keycloakAuthent this.authenticationProviderManager = authenticationProviderManager; } + @Bean + @Order(60) + public SecurityFilterChain healthSecurityFilterChain(HttpSecurity http) throws Exception { + http.securityMatcher(new AntPathRequestMatcher("/api/health")) + .authorizeHttpRequests(authorize -> authorize.anyRequest().permitAll()) + .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) + .requestCache(RequestCacheConfigurer::disable) + .headers(headers -> headers.frameOptions(iframeDisabled + ? HeadersConfigurer.FrameOptionsConfig::disable + : HeadersConfigurer.FrameOptionsConfig::sameOrigin)) + .anonymous(AbstractHttpConfigurer::disable) + .cors(cors -> cors.configurationSource(corsConfigurationSource())); + return http.build(); + } + @Bean @Order(70) public SecurityFilterChain keycloakSecurityFilterChain(HttpSecurity http) throws Exception { diff --git a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/BasicAuthFilterTest.java b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/BasicAuthFilterTest.java index c54a536a5..217b6ad4c 100644 --- a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/BasicAuthFilterTest.java +++ b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/BasicAuthFilterTest.java @@ -95,6 +95,7 @@ void setUp() { Mockito.lenient().when(request.getHeader(HttpHeaders.HOST)).thenReturn("localhost"); Mockito.lenient().when(request.getServerName()).thenReturn("localhost"); Mockito.lenient().when(session.getServletContext()).thenReturn(servletContext); + Mockito.lenient().when(request.getServletContext()).thenReturn(servletContext); Mockito.lenient().when(webApplicationContext.getBean(ITenantManager.class)).thenReturn(tenantManager); } diff --git a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakAuthenticationFilterTest.java b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakAuthenticationFilterTest.java index 19da64306..823c253fe 100644 --- a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakAuthenticationFilterTest.java +++ b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakAuthenticationFilterTest.java @@ -5,6 +5,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -88,6 +89,7 @@ public void setUp() { Mockito.lenient().when(request.getHeader(HttpHeaders.HOST)).thenReturn("dev.entando.org"); Mockito.lenient().when(request.getServerName()).thenReturn("dev.entando.org"); Mockito.lenient().when(session.getServletContext()).thenReturn(svCtx); + Mockito.lenient().when(request.getServletContext()).thenReturn(svCtx); Mockito.lenient().when(wac.getBean(ITenantManager.class)).thenReturn(tenantManager); } @@ -151,7 +153,7 @@ void apiAuthenticationShouldSetAttributeRequest() { wacUtil.when(() -> WebApplicationContextUtils.getWebApplicationContext(svCtx)).thenReturn(wac); keycloakAuthenticationFilter.attemptAuthentication(request, response); verify(request).setAttribute(eq("user"), any()); - verify(request, times(1)).getSession(); + verify(request, never()).getSession(); } } diff --git a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfigTest.java b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfigTest.java index e8f971ae8..2fb6d508b 100644 --- a/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfigTest.java +++ b/keycloak-plugin/src/test/java/org/entando/entando/aps/servlet/security/KeycloakSecurityConfigTest.java @@ -24,14 +24,28 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.entando.entando.aps.system.services.tenants.ITenantManager; +import org.springframework.context.annotation.Bean; +import org.springframework.core.annotation.Order; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletContext; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.web.DefaultSecurityFilterChain; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; +import org.springframework.web.cors.CorsConfiguration; +import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -214,4 +228,93 @@ void keycloakEnabled_shouldConfigureHttpSecurityCorrectly() throws Exception { verify(httpSecurity).build(); } + @Test + void healthProbeShouldNotCreateSession() throws Exception { + try (AnnotationConfigWebApplicationContext context = securityChainsContext()) { + MockHttpServletRequest request = apiRequest(context, "/health"); + MockFilterChain chain = new MockFilterChain(); + + context.getBean("springSecurityFilterChain", Filter.class).doFilter(request, new MockHttpServletResponse(), chain); + + assertNotNull(chain.getRequest()); + assertNull(request.getSession(false)); + } + } + + @Test + void healthPostWithoutCsrfTokenShouldBeRejected() throws Exception { + try (AnnotationConfigWebApplicationContext context = securityChainsContext()) { + MockHttpServletRequest request = apiRequest(context, "/health"); + request.setMethod("POST"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + context.getBean("springSecurityFilterChain", Filter.class).doFilter(request, response, chain); + + assertNull(chain.getRequest()); + assertEquals(403, response.getStatus()); + } + } + + @Test + void otherApiPathShouldStillCreateSession() throws Exception { + try (AnnotationConfigWebApplicationContext context = securityChainsContext()) { + MockHttpServletRequest request = apiRequest(context, "/other"); + MockFilterChain chain = new MockFilterChain(); + + context.getBean("springSecurityFilterChain", Filter.class).doFilter(request, new MockHttpServletResponse(), chain); + + assertNotNull(chain.getRequest()); + assertNotNull(request.getSession(false)); + } + } + + private static AnnotationConfigWebApplicationContext securityChainsContext() { + MockServletContext servletContext = new MockServletContext(); + AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); + context.setServletContext(servletContext); + context.register(SecurityChainsConfig.class); + context.refresh(); + servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, context); + return context; + } + + private static MockHttpServletRequest apiRequest(WebApplicationContext context, String pathInfo) { + MockHttpServletRequest request = new MockHttpServletRequest(context.getServletContext(), "GET", "/api" + pathInfo); + request.setServletPath("/api"); + request.setPathInfo(pathInfo); + return request; + } + + @EnableWebSecurity + @EnableWebMvc + static class SecurityChainsConfig { + + private final KeycloakSecurityConfig securityConfig = keycloakDisabledSecurityConfig(); + + @Bean + ITenantManager tenantManager() { + return mock(ITenantManager.class); + } + + @Bean + @Order(60) + SecurityFilterChain healthChain(HttpSecurity http) throws Exception { + return securityConfig.healthSecurityFilterChain(http); + } + + @Bean + @Order(70) + SecurityFilterChain defaultChain(HttpSecurity http) throws Exception { + return securityConfig.keycloakSecurityFilterChain(http); + } + + private static KeycloakSecurityConfig keycloakDisabledSecurityConfig() { + KeycloakSecurityConfig config = new KeycloakSecurityConfig(mock(KeycloakAuthenticationFilter.class), + mock(KeycloakConfiguration.class), mock(IUserManager.class), mock(IAuthenticationProviderManager.class)); + config.corsConfiguration = new CorsConfiguration(); + return config; + } + } + } diff --git a/keycloak-plugin/src/test/java/org/entando/entando/keycloak/filter/KeycloakFilterTest.java b/keycloak-plugin/src/test/java/org/entando/entando/keycloak/filter/KeycloakFilterTest.java index a540620f7..3a6c99d1a 100644 --- a/keycloak-plugin/src/test/java/org/entando/entando/keycloak/filter/KeycloakFilterTest.java +++ b/keycloak-plugin/src/test/java/org/entando/entando/keycloak/filter/KeycloakFilterTest.java @@ -93,6 +93,7 @@ public void setUp() { keycloakFilter = new KeycloakFilter(configuration, oidcService, providerManager, keycloakGroupManager, userManager); Mockito.lenient().when(request.getSession()).thenReturn(session); Mockito.lenient().when(session.getServletContext()).thenReturn(svCtx); + Mockito.lenient().when(request.getServletContext()).thenReturn(svCtx); Mockito.lenient().when(wac.getBean(ITenantManager.class)).thenReturn(tenantManager); Mockito.lenient().when(request.getServerName()).thenReturn("dev.entando.org"); Mockito.lenient().when(request.getScheme()).thenReturn("https"); @@ -421,7 +422,7 @@ void apiCallShouldNotSaveUserOnSession() throws Exception { } verify(filterChain, times(1)).doFilter(any(), any()); - verify(request, times(1)).getSession(); + verify(request, never()).getSession(); } @Test diff --git a/portal-ui/src/test/java/org/entando/entando/aps/system/services/controller/control/TestTenantController.java b/portal-ui/src/test/java/org/entando/entando/aps/system/services/controller/control/TestTenantController.java index ba1dca34b..358e6e44e 100644 --- a/portal-ui/src/test/java/org/entando/entando/aps/system/services/controller/control/TestTenantController.java +++ b/portal-ui/src/test/java/org/entando/entando/aps/system/services/controller/control/TestTenantController.java @@ -127,7 +127,7 @@ void testService_2() throws EntException { public RequestContext createExtRequestContext(String serverName, ApplicationContext applicationContext) { RequestContext reqCtx = this.getRequestContext(); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = new MockHttpServletRequest(reqCtx.getRequest().getServletContext()); request.setScheme("http"); request.setServerName(serverName); request.addHeader("Host", serverName); diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/apsadmin/ApsAdminPluginBaseTestCase.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/apsadmin/ApsAdminPluginBaseTestCase.java index 64881cf9a..1062e2a7b 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/apsadmin/ApsAdminPluginBaseTestCase.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/apsadmin/ApsAdminPluginBaseTestCase.java @@ -83,7 +83,7 @@ protected static void setUp() throws Exception { refresh = true; } RequestContext reqCtx = BaseTestCase.createRequestContext(applicationContext, servletContext); - request = new MockHttpServletRequest(); + request = new MockHttpServletRequest(servletContext); request.setAttribute(RequestContext.REQCTX, reqCtx); response = new MockHttpServletResponse(); request.setSession(new MockHttpSession(servletContext));