SONAR-17296 improved handling of SAML Authentication errors preventing the handshake...

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 584d95950e9e4f075a21baf7d1bc8298680b430f..dba3c5b170f55dc3b978c36016b22641ae559461 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
@@ -209,8 +209,12 @@ public class SamlAuthenticator {
   }
 
   public String getAuthenticationStatusPage(HttpServletRequest request, HttpServletResponse response) {
-    Auth auth = this.initSamlAuth(request, response);
-    return getSamlAuthStatusHtml(getSamlAuthenticationStatus(auth, samlSettings));
+    try {
+      Auth auth = this.initSamlAuth(request, response);
+      return getSamlAuthStatusHtml(getSamlAuthenticationStatus(auth, samlSettings));
+    } catch (IllegalStateException e) {
+      return getSamlAuthStatusHtml(getSamlAuthenticationStatus(String.format("%s due to: %s", e.getMessage(), e.getCause().getMessage())));
+    }
   }
 }
 

diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
index 337f061a299ba83e61760773fbac4b855ae8c69f..d47394582e5388620d0efdf7a1d7f707daf55307 100644 (file)
--- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
+++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlStatusChecker.java
@@ -67,6 +67,14 @@ public final class SamlStatusChecker {
 
   }
 
+  public static SamlAuthenticationStatus getSamlAuthenticationStatus(String errorMessage) {
+    SamlAuthenticationStatus samlAuthenticationStatus = new SamlAuthenticationStatus();
+    samlAuthenticationStatus.getErrors().add(errorMessage);
+    samlAuthenticationStatus.setStatus("error");
+
+    return samlAuthenticationStatus;
+  }
+
   private static Map<String, Collection<String>> getAttributesMapping(Auth auth, SamlSettings samlSettings) {
     Map<String, Collection<String>> attributesMapping = new HashMap<>();
 

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
new file mode 100644 (file)
index 0000000..84ca8b0
--- /dev/null
+++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlAuthenticatorTest.java
@@ -0,0 +1,41 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.auth.saml;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+
+public class SamlAuthenticatorTest {
+
+  @Test
+  public void authentication_status_with_errors_returned_when_init_fails() {
+    SamlAuthenticator samlAuthenticator = new SamlAuthenticator(mock(SamlSettings.class), mock(SamlMessageIdChecker.class));
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    HttpServletResponse response = mock(HttpServletResponse.class);
+
+    String authenticationStatus = samlAuthenticator.getAuthenticationStatusPage(request, response);
+
+    assertFalse(authenticationStatus.isEmpty());
+  }
+}

diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
index f76529d30951e0802d21e6d4e4d12888e754a942..fd286fbae4d5135559878ed3910b177d76913814 100644 (file)
--- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
+++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlStatusCheckerTest.java
@@ -64,6 +64,16 @@ public class SamlStatusCheckerTest {
     when(auth.getAttributes()).thenReturn(getResponseAttributes());
   }
 
+  @Test
+  public void authentication_status_has_errors_when_no_idp_certificate_is_provided() {
+
+    samlAuthenticationStatus = getSamlAuthenticationStatus("error message");
+
+    assertEquals("error", samlAuthenticationStatus.getStatus());
+    assertFalse(samlAuthenticationStatus.getErrors().isEmpty());
+    assertEquals("error message", samlAuthenticationStatus.getErrors().get(0));
+  }
+
   @Test
   public void authentication_status_is_success_when_no_errors() {
     setSettings();

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 f882827cd0796fa3bff01721b78c7a19b6bfb238..4181fe6d83f62b78abc685c24f9a284703821381 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
@@ -36,6 +36,9 @@ import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.user.UserSession;
 import org.sonar.server.ws.ServletFilterHandler;
 
+import static org.sonar.server.authentication.SamlValidationRedirectionFilter.SAML_VALIDATION_CONTROLLER_CONTEXT;
+import static org.sonar.server.authentication.SamlValidationRedirectionFilter.SAML_VALIDATION_KEY;
+
 public class ValidationInitAction extends ServletFilter implements SamlAction {
 
   public static final String VALIDATION_RELAY_STATE = "validation-query";
@@ -79,7 +82,11 @@ public class ValidationInitAction extends ServletFilter implements SamlAction {
       return;
     }
 
-    samlAuthenticator.initLogin(oAuth2ContextFactory.generateCallbackUrl(SamlIdentityProvider.KEY),
-      VALIDATION_RELAY_STATE, request, response);
+    try {
+      samlAuthenticator.initLogin(oAuth2ContextFactory.generateCallbackUrl(SamlIdentityProvider.KEY),
+        VALIDATION_RELAY_STATE, request, response);
+    } catch (IllegalStateException e) {
+      response.sendRedirect("/" + SAML_VALIDATION_CONTROLLER_CONTEXT + "/" + SAML_VALIDATION_KEY);
+    }
   }
 }

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 618dce62ef0da9f242de56406060a70d5581b9e2..c284e4defec7275f5922a79fc263eca19e857842 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
@@ -36,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.matches;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -80,6 +81,23 @@ public class ValidationInitActionTest {
       any(), any());
   }
 
+  @Test
+  public void do_filter_as_admin_with_init_issues() throws IOException, ServletException {
+    userSession.logIn().setSystemAdministrator();
+    HttpServletRequest servletRequest = mock(HttpServletRequest.class);
+    HttpServletResponse servletResponse = mock(HttpServletResponse.class);
+    FilterChain filterChain = mock(FilterChain.class);
+    String callbackUrl = "http://localhost:9000/api/validation_test";
+    when(oAuth2ContextFactory.generateCallbackUrl(anyString()))
+      .thenReturn(callbackUrl);
+
+    doThrow(new IllegalStateException()).when(samlAuthenticator).initLogin(any(), any(), any(), any());
+
+    underTest.doFilter(servletRequest, servletResponse, filterChain);
+
+    verify(servletResponse).sendRedirect("/saml/validation");
+  }
+
   @Test
   public void do_filter_as_not_admin() throws IOException, ServletException {
     userSession.logIn();