From b6f073d156edef859381361cd66d1520adc8a739 Mon Sep 17 00:00:00 2001 From: Matteo Mara Date: Tue, 31 Oct 2023 23:15:47 +0100 Subject: [PATCH] SONAR-20921 Handle more configuration errors in SAML test page --- .../sonar/auth/saml/SamlAuthenticator.java | 13 +++-- .../auth/saml/SamlAuthenticatorTest.java | 53 ++++++++++++++++++- .../server/saml/ws/ValidationInitAction.java | 4 +- .../saml/ws/ValidationInitActionTest.java | 2 +- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthenticator.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthenticator.java index d9f29b3b9b1..0504d35e961 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthenticator.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlAuthenticator.java @@ -35,13 +35,13 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.server.authentication.UserIdentity; import org.sonar.api.server.http.HttpRequest; import org.sonar.api.server.http.HttpResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.sonar.server.http.JavaxHttpRequest; import org.sonar.server.http.JavaxHttpResponse; @@ -103,7 +103,7 @@ public class SamlAuthenticator { HttpServletResponse httpServletResponse = ((JavaxHttpResponse) response).getDelegate(); return new Auth(initSettings(callbackUrl), httpServletRequest, httpServletResponse); - } catch (SettingsException e) { + } catch (Exception e) { throw new IllegalStateException("Failed to create a SAML Auth", e); } } @@ -135,7 +135,12 @@ public class SamlAuthenticator { var saml2Settings = new SettingsBuilder().fromValues(samlData).build(); if (samlSettings.getServiceProviderPrivateKey().isPresent() && saml2Settings.getSPkey() == null) { - LOGGER.error("Error in parsing service provider private key, please make sure that it is in PKCS 8 format."); + final String pkcs8ErrorMessage = "Error in parsing service provider private key, please make sure that it is in PKCS 8 format."; + LOGGER.error(pkcs8ErrorMessage); + // If signature is enabled then we need to throw an exception because the authentication will never work with a missing private key + if (samlSettings.isSignRequestsEnabled()) { + throw new IllegalStateException(pkcs8ErrorMessage); + } } return saml2Settings; } diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthenticatorTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthenticatorTest.java index fb6e9f2cdeb..088bbad476e 100644 --- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthenticatorTest.java +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthenticatorTest.java @@ -22,26 +22,75 @@ package org.sonar.auth.saml; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.junit.Test; +import org.sonar.api.config.PropertyDefinitions; +import org.sonar.api.config.internal.MapSettings; import org.sonar.api.server.http.HttpRequest; import org.sonar.api.server.http.HttpResponse; +import org.sonar.api.utils.System2; import org.sonar.server.http.JavaxHttpRequest; import org.sonar.server.http.JavaxHttpResponse; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class SamlAuthenticatorTest { + private MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE, SamlSettings.definitions())); + + private SamlSettings samlSettings = new SamlSettings(settings.asConfig()); + + private final SamlAuthenticator underTest = new SamlAuthenticator(samlSettings, mock(SamlMessageIdChecker.class)); + @Test public void authentication_status_with_errors_returned_when_init_fails() { - SamlAuthenticator samlAuthenticator = new SamlAuthenticator(mock(SamlSettings.class), mock(SamlMessageIdChecker.class)); HttpRequest request = new JavaxHttpRequest(mock(HttpServletRequest.class)); HttpResponse response = new JavaxHttpResponse(mock(HttpServletResponse.class)); when(request.getContextPath()).thenReturn("context"); - String authenticationStatus = samlAuthenticator.getAuthenticationStatusPage(request, response); + String authenticationStatus = underTest.getAuthenticationStatusPage(request, response); assertFalse(authenticationStatus.isEmpty()); } + + @Test + public void givenPrivateKeyIsNotPkcs8Encrypted_whenInitializingTheAuthentication_thenExceptionIsThrown() { + initBasicSamlSettings(); + + settings.setProperty("sonar.auth.saml.signature.enabled", true); + settings.setProperty("sonar.auth.saml.sp.certificate.secured", "CERTIFICATE"); + settings.setProperty("sonar.auth.saml.sp.privateKey.secured", "Not a PKCS8 key"); + + assertThatIllegalStateException() + .isThrownBy(() -> underTest.initLogin("","", mock(JavaxHttpRequest.class), mock(JavaxHttpResponse.class))) + .withMessage("Failed to create a SAML Auth") + .havingCause() + .withMessage("Error in parsing service provider private key, please make sure that it is in PKCS 8 format."); + } + + @Test + public void givenMissingSpCertificate_whenInitializingTheAuthentication_thenExceptionIsThrown() { + initBasicSamlSettings(); + + settings.setProperty("sonar.auth.saml.signature.enabled", true); + settings.setProperty("sonar.auth.saml.sp.privateKey.secured", "PRIVATE_KEY"); + + assertThatIllegalStateException() + .isThrownBy(() -> underTest.initLogin("","", mock(JavaxHttpRequest.class), mock(JavaxHttpResponse.class))) + .withMessage("Failed to create a SAML Auth") + .havingCause() + .withMessage("Service provider certificate is missing"); + } + + private void initBasicSamlSettings() { + settings.setProperty("sonar.auth.saml.applicationId", "MyApp"); + settings.setProperty("sonar.auth.saml.providerId", "http://localhost:8080/auth/realms/sonarqube"); + settings.setProperty("sonar.auth.saml.loginUrl", "http://localhost:8080/auth/realms/sonarqube/protocol/saml"); + settings.setProperty("sonar.auth.saml.certificate.secured", "ABCDEFG"); + settings.setProperty("sonar.auth.saml.user.login", "login"); + settings.setProperty("sonar.auth.saml.user.name", "name"); + settings.setProperty("sonar.auth.saml.enabled", true); + } + } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java index 9960cb09151..12a01f7d213 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java @@ -84,8 +84,8 @@ public class ValidationInitAction extends HttpFilter implements SamlAction { try { samlAuthenticator.initLogin(oAuth2ContextFactory.generateCallbackUrl(SamlIdentityProvider.KEY), VALIDATION_RELAY_STATE + "/" + csrfState, request, response); - } catch (IllegalStateException e) { - response.sendRedirect("/" + SAML_VALIDATION_CONTROLLER_CONTEXT + "/" + SAML_VALIDATION_KEY); + } catch (IllegalArgumentException | IllegalStateException e) { + response.sendRedirect("/" + SAML_VALIDATION_CONTROLLER_CONTEXT + "/" + SAML_VALIDATION_KEY + "?CSRFToken=" + csrfState); } } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java index 5a6db55a32e..3c2cc65e4fd 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java @@ -99,7 +99,7 @@ public class ValidationInitActionTest { underTest.doFilter(servletRequest, servletResponse, filterChain); - verify(servletResponse).sendRedirect("/saml/validation"); + verify(servletResponse).sendRedirect("/saml/validation?CSRFToken=CSRF_TOKEN"); } @Test -- 2.39.5