From 56f847a7e4a6d4adff95b52e1db62c830d804660 Mon Sep 17 00:00:00 2001 From: Steve Marion Date: Tue, 20 Sep 2022 16:31:42 +0200 Subject: [PATCH] SONAR-17296 move validation callback under /saml controller alongside init action --- .../authentication/SamlAuthentication.tsx | 2 +- .../SamlValidationRedirectionFilter.java | 9 +++++-- .../SamlValidationRedirectionFilterTest.java | 1 + .../server/saml/ws/SamlValidationModule.java | 4 +-- .../server/saml/ws/SamlValidationWs.java | 3 ++- ...lbackFilter.java => ValidationAction.java} | 27 +++++++++++++++---- ...tAction.java => ValidationInitAction.java} | 9 ++++--- ...terTest.java => ValidationActionTest.java} | 26 +++++++++++++++--- ...est.java => ValidationInitActionTest.java} | 13 ++++----- 9 files changed, 69 insertions(+), 25 deletions(-) rename server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/{SamlValidationCallbackFilter.java => ValidationAction.java} (70%) rename server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/{SamlValidationInitAction.java => ValidationInitAction.java} (87%) rename server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/{SamlValidationCallbackFilterTest.java => ValidationActionTest.java} (78%) rename server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/{SamlValidationInitActionTest.java => ValidationInitActionTest.java} (91%) diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx index e0ccf340bfa..f6070383c10 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx @@ -49,7 +49,7 @@ interface SamlAuthenticationState { success?: boolean; } -const CONFIG_TEST_PATH = '/api/saml/validation_init'; +const CONFIG_TEST_PATH = '/saml/validation_init'; const SAML_ENABLED_FIELD = 'sonar.auth.saml.enabled'; 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 9a76357e5f6..7d66c9047f0 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 @@ -22,6 +22,7 @@ package org.sonar.server.authentication; import com.google.common.io.Resources; import java.io.IOException; +import java.net.URI; import java.net.URL; import java.nio.charset.StandardCharsets; import javax.servlet.FilterChain; @@ -41,7 +42,8 @@ import static org.sonar.server.authentication.AuthenticationFilter.CALLBACK_PATH public class SamlValidationRedirectionFilter extends ServletFilter { public static final String VALIDATION_RELAY_STATE = "validation-query"; - public static final String SAML_VALIDATION_URL = "/saml/validation_callback"; + public static final String SAML_VALIDATION_CONTROLLER_CONTEXT = "saml"; + public static final String SAML_VALIDATION_KEY = "validation"; private String redirectionPageTemplate; private final Server server; @@ -76,10 +78,13 @@ public class SamlValidationRedirectionFilter extends ServletFilter { HttpServletResponse httpResponse = (HttpServletResponse) response; String samlResponse = StringEscapeUtils.escapeHtml(request.getParameter("SAMLResponse")); + URI redirectionEndpointUrl = URI.create(server.getContextPath() + "/") + .resolve(SAML_VALIDATION_CONTROLLER_CONTEXT + "/") + .resolve(SAML_VALIDATION_KEY); String template = StringUtils.replaceEachRepeatedly(redirectionPageTemplate, new String[]{"%VALIDATION_URL%", "%SAML_RESPONSE%"}, - new String[]{server.getContextPath() + SAML_VALIDATION_URL, samlResponse}); + new String[]{redirectionEndpointUrl.toString(), samlResponse}); httpResponse.setContentType("text/html"); httpResponse.getWriter().print(template); 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 5dc41eea3cb..b27c68a5f1f 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 @@ -100,6 +100,7 @@ public class SamlValidationRedirectionFilterTest { ArgumentCaptor htmlProduced = ArgumentCaptor.forClass(String.class); verify(pw).print(htmlProduced.capture()); assertThat(htmlProduced.getValue()).doesNotContain(""); + assertThat(htmlProduced.getValue()).contains("action=\"/saml/validation\""); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationModule.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationModule.java index 602ed55959a..ce51a4a06fc 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationModule.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationModule.java @@ -26,8 +26,8 @@ public class SamlValidationModule extends Module { protected void configureModule() { add( SamlValidationWs.class, - SamlValidationInitAction.class, - SamlValidationCallbackFilter.class + ValidationInitAction.class, + ValidationAction.class ); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationWs.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationWs.java index 52529579be9..198e24d614b 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationWs.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationWs.java @@ -21,10 +21,11 @@ package org.sonar.server.saml.ws; import java.util.List; import org.sonar.api.server.ws.WebService; +import org.sonar.server.authentication.SamlValidationRedirectionFilter; public class SamlValidationWs implements WebService { - public static final String SAML_VALIDATION_CONTROLLER = "api/saml"; + public static final String SAML_VALIDATION_CONTROLLER = SamlValidationRedirectionFilter.SAML_VALIDATION_CONTROLLER_CONTEXT; private final List actions; public SamlValidationWs(List actions) { diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationCallbackFilter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationAction.java similarity index 70% rename from server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationCallbackFilter.java rename to server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationAction.java index 439a1b312ca..4b58458513c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationCallbackFilter.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationAction.java @@ -28,22 +28,24 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; +import org.sonar.api.server.ws.WebService; import org.sonar.api.web.ServletFilter; import org.sonar.auth.saml.SamlAuthenticator; import org.sonar.auth.saml.SamlIdentityProvider; import org.sonar.server.authentication.AuthenticationError; import org.sonar.server.authentication.OAuth2ContextFactory; +import org.sonar.server.authentication.SamlValidationRedirectionFilter; import org.sonar.server.user.ThreadLocalUserSession; +import org.sonar.server.ws.ServletFilterHandler; -import static org.sonar.server.authentication.SamlValidationRedirectionFilter.SAML_VALIDATION_URL; - -public class SamlValidationCallbackFilter extends ServletFilter { +public class ValidationAction extends ServletFilter implements SamlAction { + static final String VALIDATION_CALLBACK_KEY = SamlValidationRedirectionFilter.SAML_VALIDATION_KEY; private final ThreadLocalUserSession userSession; private final SamlAuthenticator samlAuthenticator; private final OAuth2ContextFactory oAuth2ContextFactory; - public SamlValidationCallbackFilter(ThreadLocalUserSession userSession, SamlAuthenticator samlAuthenticator, OAuth2ContextFactory oAuth2ContextFactory) { + public ValidationAction(ThreadLocalUserSession userSession, SamlAuthenticator samlAuthenticator, OAuth2ContextFactory oAuth2ContextFactory) { this.samlAuthenticator = samlAuthenticator; this.userSession = userSession; this.oAuth2ContextFactory = oAuth2ContextFactory; @@ -51,7 +53,7 @@ public class SamlValidationCallbackFilter extends ServletFilter { @Override public UrlPattern doGetPattern() { - return UrlPattern.create(SAML_VALIDATION_URL); + return UrlPattern.create("/" + SamlValidationWs.SAML_VALIDATION_CONTROLLER + "/" + VALIDATION_CALLBACK_KEY); } @Override @@ -73,4 +75,19 @@ public class SamlValidationCallbackFilter extends ServletFilter { httpResponse.setContentType("text/html"); httpResponse.getWriter().print(samlAuthenticator.getAuthenticationStatusPage(httpRequest, httpResponse)); } + + @Override + public void define(WebService.NewController controller) { + WebService.NewAction action = controller + .createAction(VALIDATION_CALLBACK_KEY) + .setInternal(true) + .setPost(true) + .setHandler(ServletFilterHandler.INSTANCE) + .setDescription("Handle the callback of a SAML assertion from the identity Provider and produces " + + "a HTML page with all information available in the assertion.") + .setSince("9.7"); + action.createParam("SAMLResponse") + .setDescription("SAML assertion value") + .setRequired(true); + } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationInitAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java similarity index 87% rename from server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationInitAction.java rename to server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java index 6837be75755..f882827cd07 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/SamlValidationInitAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/saml/ws/ValidationInitAction.java @@ -36,15 +36,16 @@ import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.user.UserSession; import org.sonar.server.ws.ServletFilterHandler; -public class SamlValidationInitAction extends ServletFilter implements SamlAction { +public class ValidationInitAction extends ServletFilter implements SamlAction { public static final String VALIDATION_RELAY_STATE = "validation-query"; + public static final String VALIDATION_INIT_KEY = "validation_init"; private final SamlAuthenticator samlAuthenticator; private final OAuth2ContextFactory oAuth2ContextFactory; private final UserSession userSession; - public SamlValidationInitAction(SamlAuthenticator samlAuthenticator, OAuth2ContextFactory oAuth2ContextFactory, UserSession userSession) { + public ValidationInitAction(SamlAuthenticator samlAuthenticator, OAuth2ContextFactory oAuth2ContextFactory, UserSession userSession) { this.samlAuthenticator = samlAuthenticator; this.oAuth2ContextFactory = oAuth2ContextFactory; this.userSession = userSession; @@ -52,13 +53,13 @@ public class SamlValidationInitAction extends ServletFilter implements SamlActio @Override public UrlPattern doGetPattern() { - return UrlPattern.create("/api/saml/validation_init"); + return UrlPattern.create("/" + SamlValidationWs.SAML_VALIDATION_CONTROLLER + "/" + VALIDATION_INIT_KEY); } @Override public void define(WebService.NewController controller) { controller - .createAction("validation_init") + .createAction(VALIDATION_INIT_KEY) .setInternal(true) .setPost(false) .setHandler(ServletFilterHandler.INSTANCE) diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationCallbackFilterTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationActionTest.java similarity index 78% rename from server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationCallbackFilterTest.java rename to server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationActionTest.java index 27fd9764302..a00371db1d6 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationCallbackFilterTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationActionTest.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; +import org.sonar.api.server.ws.WebService; import org.sonar.auth.saml.SamlAuthenticator; import org.sonar.server.authentication.OAuth2ContextFactory; import org.sonar.server.user.ThreadLocalUserSession; @@ -41,9 +42,9 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -public class SamlValidationCallbackFilterTest { +public class ValidationActionTest { - private SamlValidationCallbackFilter underTest; + private ValidationAction underTest; private SamlAuthenticator samlAuthenticator; private ThreadLocalUserSession userSession; @@ -52,12 +53,14 @@ public class SamlValidationCallbackFilterTest { samlAuthenticator = mock(SamlAuthenticator.class); userSession = mock(ThreadLocalUserSession.class); var oAuth2ContextFactory = mock(OAuth2ContextFactory.class); - underTest = new SamlValidationCallbackFilter(userSession, samlAuthenticator, oAuth2ContextFactory); + underTest = new ValidationAction(userSession, samlAuthenticator, oAuth2ContextFactory); } @Test public void do_get_pattern() { - assertThat(underTest.doGetPattern().matches("/saml/validation_callback")).isTrue(); + assertThat(underTest.doGetPattern().matches("/saml/validation")).isTrue(); + assertThat(underTest.doGetPattern().matches("/saml/validation2")).isFalse(); + assertThat(underTest.doGetPattern().matches("/api/saml/validation")).isFalse(); assertThat(underTest.doGetPattern().matches("/saml/validation_callback2")).isFalse(); assertThat(underTest.doGetPattern().matches("/saml/")).isFalse(); } @@ -94,4 +97,19 @@ public class SamlValidationCallbackFilterTest { verifyNoInteractions(samlAuthenticator); } + + @Test + public void verify_definition() { + String controllerKey = "foo"; + WebService.Context context = new WebService.Context(); + WebService.NewController newController = context.createController(controllerKey); + underTest.define(newController); + newController.done(); + + WebService.Action validationInitAction = context.controller(controllerKey) + .action(ValidationAction.VALIDATION_CALLBACK_KEY); + assertThat(validationInitAction).isNotNull(); + assertThat(validationInitAction.description()).isNotEmpty(); + assertThat(validationInitAction.handler()).isNotNull(); + } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationInitActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java similarity index 91% rename from server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationInitActionTest.java rename to server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java index fde2872433d..618dce62ef0 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/SamlValidationInitActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/saml/ws/ValidationInitActionTest.java @@ -41,10 +41,10 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -public class SamlValidationInitActionTest { +public class ValidationInitActionTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); - private SamlValidationInitAction underTest; + private ValidationInitAction underTest; private SamlAuthenticator samlAuthenticator; private OAuth2ContextFactory oAuth2ContextFactory; @@ -52,14 +52,15 @@ public class SamlValidationInitActionTest { public void setUp() throws Exception { samlAuthenticator = mock(SamlAuthenticator.class); oAuth2ContextFactory = mock(OAuth2ContextFactory.class); - underTest = new SamlValidationInitAction(samlAuthenticator, oAuth2ContextFactory, userSession); + underTest = new ValidationInitAction(samlAuthenticator, oAuth2ContextFactory, userSession); } @Test public void do_get_pattern() { - assertThat(underTest.doGetPattern().matches("/api/saml/validation_init")).isTrue(); + assertThat(underTest.doGetPattern().matches("/saml/validation_init")).isTrue(); assertThat(underTest.doGetPattern().matches("/api/saml")).isFalse(); - assertThat(underTest.doGetPattern().matches("/api/saml/validation_init2")).isFalse(); + assertThat(underTest.doGetPattern().matches("/api/saml/validation_init")).isFalse(); + assertThat(underTest.doGetPattern().matches("/saml/validation_init2")).isFalse(); } @Test @@ -75,7 +76,7 @@ public class SamlValidationInitActionTest { underTest.doFilter(servletRequest, servletResponse, filterChain); verify(samlAuthenticator).initLogin(matches(callbackUrl), - matches(SamlValidationInitAction.VALIDATION_RELAY_STATE), + matches(ValidationInitAction.VALIDATION_RELAY_STATE), any(), any()); } -- 2.39.5