SONAR-20921 Handle more configuration errors in SAML test page

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 d9f29b3b9b105f53c22f86277cabe49abc72ee68..0504d35e96164399e59e8cf6698371d544952b76 100644 (file)
--- 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 fb6e9f2cdeb911e54d415bc1c15139efe2b769a5..088bbad476e59dbbbc79862fb3f6a95a834f1910 100644 (file)
--- 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 9960cb09151e0d6fa836d241321dd7e84419fca6..12a01f7d213b47270de0b3e7f021ae9e8307ce41 100644 (file)
--- 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 5a6db55a32e92f1e67e51e4b8514615d130f35c6..3c2cc65e4fd6ea89396702553159b4d0338053be 100644 (file)
--- 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