diff --git a/app/core/src/main/resources/application.properties b/app/core/src/main/resources/application.properties index e24c13f708..5b64e50537 100644 --- a/app/core/src/main/resources/application.properties +++ b/app/core/src/main/resources/application.properties @@ -4,7 +4,7 @@ logging.level.org.springframework.security=WARN logging.level.org.hibernate=WARN logging.level.org.eclipse.jetty=WARN #logging.level.org.springframework.security.oauth2=DEBUG -logging.level.org.springframework.security=DEBUG +#logging.level.org.springframework.security=DEBUG #logging.level.org.opensaml=DEBUG logging.level.stirling.software.proprietary.security=DEBUG logging.level.com.zaxxer.hikari=WARN 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 7a072282b2..f96edc173e 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 @@ -104,12 +104,12 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { private boolean handleSamlLogout( HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { - // Check if SAML SLO is enabled (samlLogoutHandler is only set when SLO is configured) - if (samlLogoutHandler != null) { - if (authentication instanceof Saml2Authentication samlAuthentication) { - CustomSaml2AuthenticatedPrincipal principal = - (CustomSaml2AuthenticatedPrincipal) samlAuthentication.getPrincipal(); - String nameId = principal.nameId(); + if (authentication instanceof Saml2Authentication samlAuthentication) { + CustomSaml2AuthenticatedPrincipal principal = + (CustomSaml2AuthenticatedPrincipal) samlAuthentication.getPrincipal(); + String nameId = principal.nameId(); + + if (securityProperties.getSaml2().getEnableSingleLogout()) { log.info("SAML user {} logging out via IdP SLO (session-based)", nameId); try { samlLogoutHandler.onLogoutSuccess(request, response, authentication); @@ -117,30 +117,30 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { log.error("SAML SLO failed, falling back to local logout", e); getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } - return true; + } else { + getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } - // Try to reconstruct Saml2Authentication from JWT claims for SLO - Optional reconstructedAuth = - reconstructSaml2AuthenticationFromJwt(request); + return true; + } - if (reconstructedAuth.isPresent()) { - Saml2Authentication samlAuth = reconstructedAuth.get(); - CustomSaml2AuthenticatedPrincipal principal = - (CustomSaml2AuthenticatedPrincipal) samlAuth.getPrincipal(); - log.info( - "SAML user {} logging out via IdP SLO (reconstructed from JWT)", - principal.nameId()); - try { - samlLogoutHandler.onLogoutSuccess(request, response, samlAuth); - } catch (Exception e) { - log.error("SAML SLO failed, falling back to local logout", e); - getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); - } - return true; + // Try to reconstruct Saml2Authentication from JWT claims for SLO + Optional reconstructedAuth = + reconstructSaml2AuthenticationFromJwt(request); + + if (reconstructedAuth.isPresent()) { + Saml2Authentication samlAuth = reconstructedAuth.get(); + CustomSaml2AuthenticatedPrincipal principal = + (CustomSaml2AuthenticatedPrincipal) samlAuth.getPrincipal(); + log.info( + "SAML user {} logging out via IdP SLO (reconstructed from JWT)", + principal.nameId()); + try { + samlLogoutHandler.onLogoutSuccess(request, response, samlAuth); + } catch (Exception e) { + log.error("SAML SLO failed, falling back to local logout", e); + getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } - } else { - getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); return true; } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java index 6fd7c84070..8d835fc213 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java @@ -30,7 +30,7 @@ import org.springframework.security.web.authentication.logout.LogoutFilter; import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; import org.springframework.security.web.authentication.rememberme.PersistentTokenRepository; import org.springframework.security.web.savedrequest.NullRequestCache; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; import org.springframework.web.cors.UrlBasedCorsConfigurationSource; @@ -246,9 +246,9 @@ public class SecurityConfiguration { http.logout( logout -> + // Require POST to prevent logout CSRF attacks logout.logoutRequestMatcher( - PathPatternRequestMatcher.withDefaults() - .matcher("/logout")) + new AntPathRequestMatcher("/logout", "POST")) .logoutSuccessHandler( new CustomLogoutSuccessHandler( securityProperties, diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/Saml2Configuration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/Saml2Configuration.java index dfcb277481..57fdad8041 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/Saml2Configuration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/Saml2Configuration.java @@ -79,7 +79,7 @@ public class Saml2Configuration { Resource privateKeyResource = samlConf.getPrivateKey(); Resource certificateResource = samlConf.getSpCert(); - log.info("Loading SP private key from: {}", privateKeyResource.getDescription()); + log.debug("Loading SP private key from: {}", privateKeyResource.getDescription()); if (!privateKeyResource.exists()) { log.error("SAML2 SP private key not found at: {}", privateKeyResource.getDescription()); throw new IllegalStateException( @@ -87,7 +87,7 @@ public class Saml2Configuration { + privateKeyResource.getDescription()); } - log.info("Loading SP certificate from: {}", certificateResource.getDescription()); + log.debug("Loading SP certificate from: {}", certificateResource.getDescription()); if (!certificateResource.exists()) { log.error( "SAML2 SP certificate not found at: {}", certificateResource.getDescription()); @@ -126,15 +126,17 @@ public class Saml2Configuration { String entityId = backendUrl + "/saml2/service-provider-metadata/" + samlConf.getRegistrationId(); String acsLocation = backendUrl + "/login/saml2/sso/{registrationId}"; - String sloResponseLocation = backendUrl + "/login"; + // SP's Single Logout Service endpoint (where SP receives logout requests/responses from + // IdP) + String spSloLocation = backendUrl + "/logout/saml2/slo"; RelyingPartyRegistration rp = RelyingPartyRegistration.withRegistrationId(samlConf.getRegistrationId()) .signingX509Credentials(c -> c.add(signingCredential)) .entityId(entityId) .singleLogoutServiceBinding(Saml2MessageBinding.POST) - .singleLogoutServiceLocation(idpSingleLogoutUrl) - .singleLogoutServiceResponseLocation(sloResponseLocation) + .singleLogoutServiceLocation(spSloLocation) + .singleLogoutServiceResponseLocation(spSloLocation) .assertionConsumerServiceBinding(Saml2MessageBinding.POST) .assertionConsumerServiceLocation(acsLocation) .authnRequestsSigned(true) @@ -149,8 +151,6 @@ public class Saml2Configuration { .singleLogoutServiceBinding( Saml2MessageBinding.POST) .singleLogoutServiceLocation(idpSingleLogoutUrl) - .singleLogoutServiceResponseLocation( - sloResponseLocation) .wantAuthnRequestsSigned(true)) .build(); diff --git a/frontend/src/proprietary/auth/springAuthClient.test.ts b/frontend/src/proprietary/auth/springAuthClient.test.ts index b19efcccfb..6b20e22a69 100644 --- a/frontend/src/proprietary/auth/springAuthClient.test.ts +++ b/frontend/src/proprietary/auth/springAuthClient.test.ts @@ -251,17 +251,23 @@ describe('SpringAuthClient', () => { }); describe('signOut', () => { - let originalLocation: Location; + let mockForm: { method: string; action: string; style: { display: string }; submit: ReturnType }; + let appendChildSpy: ReturnType; beforeEach(() => { - // Save and mock window.location to prevent actual navigation - originalLocation = window.location; - delete (window as any).location; - window.location = { ...originalLocation, href: '' } as Location; + // Mock form creation and submission + mockForm = { + method: '', + action: '', + style: { display: '' }, + submit: vi.fn(), + }; + vi.spyOn(document, 'createElement').mockReturnValue(mockForm as unknown as HTMLElement); + appendChildSpy = vi.spyOn(document.body, 'appendChild').mockImplementation(() => mockForm as unknown as HTMLFormElement); }); afterEach(() => { - window.location = originalLocation; + vi.restoreAllMocks(); }); it('should successfully sign out and clear JWT', async () => { @@ -272,23 +278,24 @@ describe('SpringAuthClient', () => { // JWT should be cleared from localStorage expect(localStorage.getItem('stirling_jwt')).toBeNull(); - // Should redirect to /logout for Spring Security logout handler - expect(window.location.href).toBe('/logout'); + // Should submit POST form to /logout for Spring Security logout handler + expect(mockForm.method).toBe('POST'); + expect(mockForm.action).toBe('/logout'); + expect(mockForm.submit).toHaveBeenCalled(); // Should return no error on successful signOut expect(result.error).toBeNull(); }); - it('should set logout cookie with JWT before redirect', async () => { + it('should create hidden form and submit POST to /logout', async () => { const mockToken = 'jwt-to-clear'; localStorage.setItem('stirling_jwt', mockToken); - // Clear any existing cookies - document.cookie = 'stirling_logout_token=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;'; - await springAuth.signOut(); - // Should have set the logout cookie with the token - expect(document.cookie).toContain('stirling_logout_token=' + encodeURIComponent(mockToken)); + // Verify form was created with correct attributes + expect(document.createElement).toHaveBeenCalledWith('form'); + expect(mockForm.style.display).toBe('none'); + expect(appendChildSpy).toHaveBeenCalled(); }); it('should handle signOut when no JWT is present', async () => { @@ -296,8 +303,10 @@ describe('SpringAuthClient', () => { const result = await springAuth.signOut(); - // Should still redirect to /logout - expect(window.location.href).toBe('/logout'); + // Should still submit POST form to /logout + expect(mockForm.method).toBe('POST'); + expect(mockForm.action).toBe('/logout'); + expect(mockForm.submit).toHaveBeenCalled(); expect(result.error).toBeNull(); }); }); diff --git a/frontend/src/proprietary/auth/springAuthClient.ts b/frontend/src/proprietary/auth/springAuthClient.ts index eede2a307c..9264a3597f 100644 --- a/frontend/src/proprietary/auth/springAuthClient.ts +++ b/frontend/src/proprietary/auth/springAuthClient.ts @@ -326,7 +326,8 @@ class SpringAuthClient { // it's sent with the redirect request if (token) { // Cookie expires in 30 seconds - just long enough for the logout redirect - document.cookie = `stirling_logout_token=${encodeURIComponent(token)}; path=/; max-age=30; SameSite=Lax`; + // Secure: only sent over HTTPS; Path=/logout: scoped to logout endpoint only + document.cookie = `stirling_logout_token=${encodeURIComponent(token)}; path=/logout; max-age=30; SameSite=Lax; Secure`; } // Clean up local storage @@ -364,9 +365,15 @@ class SpringAuthClient { // Notify listeners before redirect this.notifyListeners('SIGNED_OUT', null); - // Navigate to /logout to trigger Spring Security's logout handler - console.log('[SpringAuth] Redirecting to /logout for session invalidation'); - window.location.href = `${BASE_PATH}/logout`; + // Submit a POST form to /logout to trigger Spring Security's logout handler + // Using POST prevents logout CSRF attacks (GET-based logout can be triggered by any site) + console.log('[SpringAuth] Submitting POST to /logout for session invalidation'); + const form = document.createElement('form'); + form.method = 'POST'; + form.action = `${BASE_PATH}/logout`; + form.style.display = 'none'; + document.body.appendChild(form); + form.submit(); // This won't be reached if redirect happens, but return for type safety return { error: null };