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

6 files changed, 92 insertions, 4 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 584d95950e9..dba3c5b170f 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
@@ -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 337f061a299..d47394582e5 100644
--- 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
index 00000000000..84ca8b0f333
--- /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 f76529d3095..fd286fbae4d 100644
--- 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
@@ -65,6 +65,16 @@ public class SamlStatusCheckerTest {
   }
 
   @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 f882827cd07..4181fe6d83f 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
@@ -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 618dce62ef0..c284e4defec 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
@@ -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;
@@ -81,6 +82,23 @@ public class ValidationInitActionTest {
   }
 
   @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();
     HttpServletRequest servletRequest = mock(HttpServletRequest.class);