From: Steve Marion Date: Tue, 20 Sep 2022 15:05:22 +0000 (+0200) Subject: SONAR-17296 fix SAML validation page not being accessible with a user session. X-Git-Tag: 9.7.0.61563~197 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=02fed618e278701358310cac4018ab81f87d5c91;p=sonarqube.git SONAR-17296 fix SAML validation page not being accessible with a user session. --- diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SamlValidationRedirectionFilter.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SamlValidationRedirectionFilter.java index d28daf1b72a..9a76357e5f6 100644 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SamlValidationRedirectionFilter.java +++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/SamlValidationRedirectionFilter.java @@ -20,13 +20,19 @@ package org.sonar.server.authentication; +import com.google.common.io.Resources; import java.io.IOException; +import java.net.URL; +import java.nio.charset.StandardCharsets; import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; +import org.sonar.api.internal.apachecommons.lang.StringEscapeUtils; import org.sonar.api.platform.Server; import org.sonar.api.web.ServletFilter; @@ -36,6 +42,7 @@ public class SamlValidationRedirectionFilter extends ServletFilter { public static final String VALIDATION_RELAY_STATE = "validation-query"; public static final String SAML_VALIDATION_URL = "/saml/validation_callback"; + private String redirectionPageTemplate; private final Server server; public SamlValidationRedirectionFilter(Server server) { @@ -47,13 +54,35 @@ public class SamlValidationRedirectionFilter extends ServletFilter { return UrlPattern.create(CALLBACK_PATH + "saml"); } + @Override + public void init(FilterConfig filterConfig) throws ServletException { + super.init(filterConfig); + this.redirectionPageTemplate = extractTemplate("validation-redirection.html"); + } + + String extractTemplate(String templateLocation) { + try { + URL url = Resources.getResource(templateLocation); + return Resources.toString(url, StandardCharsets.UTF_8); + } catch (IOException | IllegalArgumentException e) { + throw new IllegalStateException("Cannot read the template " + templateLocation, e); + } + } + @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; if (isSamlValidation(httpRequest)) { HttpServletResponse httpResponse = (HttpServletResponse) response; - httpResponse.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); - httpResponse.setHeader("Location", server.getContextPath() + SAML_VALIDATION_URL); + + String samlResponse = StringEscapeUtils.escapeHtml(request.getParameter("SAMLResponse")); + + String template = StringUtils.replaceEachRepeatedly(redirectionPageTemplate, + new String[]{"%VALIDATION_URL%", "%SAML_RESPONSE%"}, + new String[]{server.getContextPath() + SAML_VALIDATION_URL, samlResponse}); + + httpResponse.setContentType("text/html"); + httpResponse.getWriter().print(template); return; } chain.doFilter(request, response); diff --git a/server/sonar-webserver-auth/src/main/resources/validation-redirection.html b/server/sonar-webserver-auth/src/main/resources/validation-redirection.html new file mode 100644 index 00000000000..df9a48a29d6 --- /dev/null +++ b/server/sonar-webserver-auth/src/main/resources/validation-redirection.html @@ -0,0 +1,19 @@ + + + + + + SAML Authentication Test + + + +

SAML Authentication Validation

+
+ + +
+ + + diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SamlValidationRedirectionFilterTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SamlValidationRedirectionFilterTest.java index ba54dddc33a..5dc41eea3cb 100644 --- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SamlValidationRedirectionFilterTest.java +++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/SamlValidationRedirectionFilterTest.java @@ -21,29 +21,36 @@ package org.sonar.server.authentication; import java.io.IOException; +import java.io.PrintWriter; import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.sonar.api.platform.Server; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; public class SamlValidationRedirectionFilterTest { SamlValidationRedirectionFilter underTest; @Before - public void setup() { + public void setup() throws ServletException { Server server = mock(Server.class); doReturn("").when(server).getContextPath(); underTest = new SamlValidationRedirectionFilter(server); + underTest.init(mock(FilterConfig.class)); } @Test @@ -60,11 +67,39 @@ public class SamlValidationRedirectionFilterTest { HttpServletResponse servletResponse = mock(HttpServletResponse.class); FilterChain filterChain = mock(FilterChain.class); - doReturn("validation-query").when(servletRequest).getParameter("RelayState"); + String validSample = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; + when(servletRequest.getParameter(matches("SAMLResponse"))).thenReturn(validSample); + when(servletRequest.getParameter(matches("RelayState"))).thenReturn("validation-query"); + PrintWriter pw = mock(PrintWriter.class); + when(servletResponse.getWriter()).thenReturn(pw); + underTest.doFilter(servletRequest, servletResponse, filterChain); - verify(servletResponse).setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); - verify(servletResponse).setHeader("Location", "/saml/validation_callback"); + verify(servletResponse).setContentType("text/html"); + ArgumentCaptor htmlProduced = ArgumentCaptor.forClass(String.class); + verify(pw).print(htmlProduced.capture()); + assertThat(htmlProduced.getValue()).contains(validSample); + } + + @Test + public void do_filter_validation_wrong_SAML_response() throws ServletException, IOException { + HttpServletRequest servletRequest = mock(HttpServletRequest.class); + HttpServletResponse servletResponse = mock(HttpServletResponse.class); + FilterChain filterChain = mock(FilterChain.class); + + String maliciousSaml = "test\" htmlProduced = ArgumentCaptor.forClass(String.class); + verify(pw).print(htmlProduced.capture()); + assertThat(htmlProduced.getValue()).doesNotContain(""); } @Test @@ -78,4 +113,9 @@ public class SamlValidationRedirectionFilterTest { verifyNoInteractions(servletResponse); } + + @Test + public void extract_nonexistant_template() { + assertThrows(IllegalStateException.class, () -> underTest.extractTemplate("not-there")); + } }