Hardening
This commit is contained in:
committed by
DarioGii
parent
df58816baf
commit
a2ae30d260
@@ -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
|
||||
|
||||
+26
-26
@@ -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<Saml2Authentication> 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<Saml2Authentication> 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;
|
||||
}
|
||||
|
||||
|
||||
+3
-3
@@ -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,
|
||||
|
||||
+7
-7
@@ -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();
|
||||
|
||||
|
||||
@@ -251,17 +251,23 @@ describe('SpringAuthClient', () => {
|
||||
});
|
||||
|
||||
describe('signOut', () => {
|
||||
let originalLocation: Location;
|
||||
let mockForm: { method: string; action: string; style: { display: string }; submit: ReturnType<typeof vi.fn> };
|
||||
let appendChildSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 };
|
||||
|
||||
Reference in New Issue
Block a user