From 8d6d0b0cdd62824ded0bf81562e9cf19f8d47433 Mon Sep 17 00:00:00 2001 From: DarioGii Date: Thu, 22 Jan 2026 22:06:12 +0000 Subject: [PATCH] pr comments --- .../src/main/resources/application.properties | 5 ++- .../api/ProprietaryUIDataController.java | 5 ++- .../security/CustomLogoutSuccessHandler.java | 34 ++++++++++++------- .../CustomLogoutSuccessHandlerTest.java | 34 +++++++++++++------ build.gradle | 2 +- .../tests/convert/ConvertIntegration.test.tsx | 9 ----- 6 files changed, 53 insertions(+), 36 deletions(-) diff --git a/app/core/src/main/resources/application.properties b/app/core/src/main/resources/application.properties index e24c13f708..de05ddf24a 100644 --- a/app/core/src/main/resources/application.properties +++ b/app/core/src/main/resources/application.properties @@ -1,12 +1,11 @@ multipart.enabled=true logging.level.org.springframework=WARN -logging.level.org.springframework.security=WARN logging.level.org.hibernate=WARN logging.level.org.eclipse.jetty=WARN +#logging.level.org.springframework.security=DEBUG #logging.level.org.springframework.security.oauth2=DEBUG -logging.level.org.springframework.security=DEBUG #logging.level.org.opensaml=DEBUG -logging.level.stirling.software.proprietary.security=DEBUG +#logging.level.stirling.software.proprietary.security=DEBUG logging.level.com.zaxxer.hikari=WARN logging.level.stirling.software.SPDF.service.PdfJsonConversionService=INFO logging.level.stirling.software.common.service.JobExecutorService=INFO diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java index c903458c04..af5324c4f5 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java @@ -210,7 +210,10 @@ public class ProprietaryUIDataController { SAML2 saml2 = securityProps.getSaml2(); // Only add SAML2 providers if loginMethod allows it if (securityProps.isSaml2Active() && applicationProperties.getPremium().isEnabled()) { - String samlIdp = saml2.getIdpIssuer(); + String samlIdp = + saml2.getProvider().getEntityId() != null + ? saml2.getProvider().getEntityId() + : saml2.getIdpIssuer(); // legacy fallback String saml2AuthenticationPath = "/saml2/authenticate/" + saml2.getRegistrationId(); // For SAML, we need to use the backend URL directly, not a relative path diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java index 193ad91ff1..0f356f3492 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java @@ -99,7 +99,8 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { String samlResponse = request.getParameter("SAMLResponse"); if (samlResponse != null && !samlResponse.isBlank()) { if (samlResponse.contains("/saml2/slo")) { - log.info("Received SAML LogoutResponse at /logout endpoint, completing logout locally"); + log.info( + "Received SAML LogoutResponse at /logout endpoint, completing logout locally"); getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); return true; } @@ -109,10 +110,16 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { log.info("SP-initiated SLO detected, logging out via IdP"); if (authentication instanceof Saml2Authentication samlAuthentication) { - try { - samlLogoutHandler.onLogoutSuccess(request, response, samlAuthentication); - } catch (Exception e) { - log.error("SP-initiated SLO failed, falling back to local logout", e); + if (samlLogoutHandler != null) { + try { + samlLogoutHandler.onLogoutSuccess(request, response, samlAuthentication); + } catch (Exception e) { + log.error("SP-initiated SLO failed, falling back to local logout", e); + getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); + } + } else { + log.warn( + "SAML SLO enabled but handler not configured, performing local logout only"); getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } @@ -125,19 +132,22 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { if (reconstructedAuth.isPresent()) { Saml2Authentication samlAuth = reconstructedAuth.get(); - try { - samlLogoutHandler.onLogoutSuccess(request, response, samlAuth); - } catch (Exception e) { - log.error("SP-initiated SLO failed, falling back to local logout", e); + if (samlLogoutHandler != null) { + try { + samlLogoutHandler.onLogoutSuccess(request, response, samlAuth); + } catch (Exception e) { + log.error("SP-initiated SLO failed, falling back to local logout", e); + getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); + } + } else { + log.warn( + "SAML SLO enabled but handler not configured, performing local logout only"); getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } return true; } } - } else { - getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); - return true; } return false; diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java index 96d9b2809e..12aa4e95c1 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java @@ -7,7 +7,6 @@ import static org.mockito.Mockito.when; import java.io.IOException; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -71,7 +70,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testSuccessfulLogoutViaOAuth2() throws IOException { HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); @@ -80,6 +78,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getScheme()).thenReturn("http"); @@ -95,7 +96,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testUserIsDisabledRedirect() throws IOException { String error = "userIsDisabled"; String url = "http://localhost:8080"; @@ -106,6 +106,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getParameter("oAuth2AutoCreateDisabled")).thenReturn(null); @@ -127,7 +130,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testUserAlreadyExistsWebRedirect() throws IOException { String error = "oAuth2AuthenticationErrorWeb"; String errorPath = "userAlreadyExistsWeb"; @@ -139,6 +141,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter(error)).thenReturn("true"); when(request.getScheme()).thenReturn("http"); when(request.getServerName()).thenReturn("localhost"); @@ -153,7 +158,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testErrorOAuthRedirect() throws IOException { String error = "testError"; String url = "http://localhost:8080"; @@ -164,6 +168,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn("!!!" + error + "!!!"); when(request.getScheme()).thenReturn("http"); @@ -179,7 +186,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testOAuth2AutoCreateDisabled() throws IOException { String error = "oAuth2AutoCreateDisabled"; String url = "http://localhost:8080"; @@ -190,10 +196,12 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getParameter(error)).thenReturn("true"); - when(request.getContextPath()).thenReturn(url); when(request.getScheme()).thenReturn("http"); when(request.getServerName()).thenReturn("localhost"); when(request.getServerPort()).thenReturn(8080); @@ -207,7 +215,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testOAuth2Error() throws IOException { String error = "test"; String url = "http://localhost:8080"; @@ -218,6 +225,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getParameter("oAuth2AutoCreateDisabled")).thenReturn(null); @@ -240,7 +250,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testOAuth2BadCredentialsError() throws IOException { String error = "badCredentials"; String url = "http://localhost:8080"; @@ -251,6 +260,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getParameter("oAuth2AutoCreateDisabled")).thenReturn(null); @@ -274,7 +286,6 @@ class CustomLogoutSuccessHandlerTest { } @Test - @Disabled("TODO: Fix OAuth2 logout tests - need to properly mock UrlUtils.getOrigin") void testOAuth2AdminBlockedUser() throws IOException { String error = "oAuth2AdminBlockedUser"; String url = "http://localhost:8080"; @@ -285,6 +296,9 @@ class CustomLogoutSuccessHandlerTest { mock(ApplicationProperties.Security.OAUTH2.class); when(response.isCommitted()).thenReturn(false); + when(request.getParameter("SAMLResponse")).thenReturn(null); + when(securityProperties.getSaml2()).thenReturn(saml2); + when(saml2.getEnableSingleLogout()).thenReturn(false); when(request.getParameter("oAuth2AuthenticationErrorWeb")).thenReturn(null); when(request.getParameter("errorOAuth")).thenReturn(null); when(request.getParameter("oAuth2AutoCreateDisabled")).thenReturn(null); diff --git a/build.gradle b/build.gradle index 404639a4f3..2366c6a926 100644 --- a/build.gradle +++ b/build.gradle @@ -537,4 +537,4 @@ tasks.register('buildRestartHelper', Jar) { doLast { println "restart-helper.jar created at: ${destinationDirectory.get()}/restart-helper.jar" } -} \ No newline at end of file +} diff --git a/frontend/src/core/tests/convert/ConvertIntegration.test.tsx b/frontend/src/core/tests/convert/ConvertIntegration.test.tsx index f034f2a371..87715f3a15 100644 --- a/frontend/src/core/tests/convert/ConvertIntegration.test.tsx +++ b/frontend/src/core/tests/convert/ConvertIntegration.test.tsx @@ -100,15 +100,6 @@ const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( ); -// Helper to create mock API responses with full response structure -const _createMockResponse = (data: T, status = 200, statusText = 'OK') => ({ - data, - status, - statusText, - headers: {}, - config: {}, -}); - describe('Convert Tool Integration Tests', () => { beforeEach(() => {