From 1becdaad55517706a85ff5f2efeeac4d6cf42265 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 14 Mar 2016 15:40:57 +0100 Subject: [PATCH] SONAR-7396 Improve error message when base url is badly configured --- .../server/authentication/InitFilter.java | 23 +++++++++++------- .../authentication/OAuth2CallbackFilter.java | 24 +++++++++++++++---- .../authentication/OAuth2ContextFactory.java | 2 +- .../server/authentication/InitFilterTest.java | 11 ++++++++- .../OAuth2CallbackFilterTest.java | 13 +++++++++- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/InitFilter.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/InitFilter.java index dd09ebe9d7bc..d7b9e2a96063 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/InitFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/InitFilter.java @@ -40,7 +40,7 @@ public class InitFilter extends ServletFilter { - private static final String INIT_CONTEXT = "/sessions/init"; + private static final String INIT_CONTEXT = "/sessions/init/"; private final IdentityProviderRepository identityProviderRepository; private final BaseContextFactory baseContextFactory; @@ -54,19 +54,16 @@ public InitFilter(IdentityProviderRepository identityProviderRepository, BaseCon @Override public UrlPattern doGetPattern() { - return UrlPattern.create(INIT_CONTEXT + "/*"); + return UrlPattern.create(INIT_CONTEXT + "*"); } @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; - String requestUri = httpRequest.getRequestURI(); - final String keyProvider = requestUri.replace(INIT_CONTEXT + "/", ""); + String requestURI = httpRequest.getRequestURI(); + String keyProvider = ""; try { - if (isNullOrEmpty(keyProvider)) { - throw new IllegalArgumentException("A valid identity provider key is required"); - } - + keyProvider = extractKeyProvider(requestURI, INIT_CONTEXT); IdentityProvider provider = identityProviderRepository.getEnabledByKey(keyProvider); if (provider instanceof BaseIdentityProvider) { BaseIdentityProvider baseIdentityProvider = (BaseIdentityProvider) provider; @@ -84,6 +81,16 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } } + public static String extractKeyProvider(String requestUri, String context) { + if (requestUri.contains(context)) { + String key = requestUri.replace(context, ""); + if (!isNullOrEmpty(key)) { + return key; + } + } + throw new IllegalArgumentException("A valid identity provider key is required."); + } + @Override public void init(FilterConfig filterConfig) throws ServletException { // Nothing to do diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java index 4701be2bc5a3..3c708eb1a4d1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java @@ -27,18 +27,20 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.sonar.api.CoreProperties; import org.sonar.api.server.authentication.IdentityProvider; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.web.ServletFilter; +import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import static org.sonar.server.authentication.AuthenticationError.handleError; import static org.sonar.server.authentication.AuthenticationError.handleUnauthorizedError; public class OAuth2CallbackFilter extends ServletFilter { - public static final String CALLBACK_PATH = "/oauth2/callback"; + public static final String CALLBACK_PATH = "/oauth2/callback/"; private final IdentityProviderRepository identityProviderRepository; private final OAuth2ContextFactory oAuth2ContextFactory; @@ -50,16 +52,16 @@ public OAuth2CallbackFilter(IdentityProviderRepository identityProviderRepositor @Override public UrlPattern doGetPattern() { - return UrlPattern.create(CALLBACK_PATH + "/*"); + return UrlPattern.create(CALLBACK_PATH + "*"); } @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; String requestUri = httpRequest.getRequestURI(); - String keyProvider = requestUri.replace(CALLBACK_PATH + "/", ""); - + String keyProvider = ""; try { + keyProvider = extractKeyProvider(requestUri, CALLBACK_PATH); IdentityProvider provider = identityProviderRepository.getEnabledByKey(keyProvider); if (provider instanceof OAuth2IdentityProvider) { OAuth2IdentityProvider oauthProvider = (OAuth2IdentityProvider) provider; @@ -70,8 +72,20 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } catch (UnauthorizedException e) { handleUnauthorizedError(e, (HttpServletResponse) response); } catch (Exception e) { - handleError(e, (HttpServletResponse) response, format("Fail to callback authentication with %s", keyProvider)); + handleError(e, (HttpServletResponse) response, + keyProvider.isEmpty() ? "Fail to callback authentication" : + format("Fail to callback authentication with '%s'", keyProvider)); + } + } + + public static String extractKeyProvider(String requestUri, String context) { + if (requestUri.contains(context)) { + String key = requestUri.replace(context, ""); + if (!isNullOrEmpty(key)) { + return key; + } } + throw new IllegalArgumentException(String.format("A valid identity provider key is required. Please check that property '%s' is valid.", CoreProperties.SERVER_BASE_URL)); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java index b6bbffa62211..60cb6ff6fa93 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java @@ -69,7 +69,7 @@ public String getCallbackUrl() { if (publicRootUrl.startsWith("http:") && !server.isDev()) { throw MessageException.of(format("The server url should be configured in https, please update the property '%s'", SERVER_BASE_URL)); } - return publicRootUrl + CALLBACK_PATH + "/" + identityProvider.getKey(); + return publicRootUrl + CALLBACK_PATH + identityProvider.getKey(); } @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/InitFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/InitFilterTest.java index 532d45430c09..ef82a2def3e7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/InitFilterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/InitFilterTest.java @@ -109,7 +109,16 @@ public void fail_if_identity_provider_key_is_empty() throws Exception { } @Test - public void fail_if_identity_provider_class_is_unsuported() throws Exception { + public void fail_if_uri_doesnt_contains_callback() throws Exception { + when(request.getRequestURI()).thenReturn("/sessions/init"); + + underTest.doFilter(request, response, chain); + + assertError("Fail to initialize authentication with provider ''"); + } + + @Test + public void fail_if_identity_provider_class_is_unsupported() throws Exception { final String unsupportedKey = "unsupported"; when(request.getRequestURI()).thenReturn("/sessions/init/" + unsupportedKey); identityProviderRepository.addIdentityProvider(new IdentityProvider() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java index 04d58e6928e2..ef19c6a298b1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2CallbackFilterTest.java @@ -98,7 +98,7 @@ public void fail_on_disabled_provider() throws Exception { underTest.doFilter(request, response, chain); - assertError("Fail to callback authentication with github"); + assertError("Fail to callback authentication with 'github'"); } @Test @@ -114,6 +114,17 @@ public void redirect_when_failing_because_of_UnauthorizedExceptionException() th verify(response).sendRedirect("/sessions/unauthorized?message=Email+john%40email.com+is+already+used"); } + @Test + public void fail_when_no_oauth2_provider_provided() throws Exception { + String providerKey = "openid"; + when(request.getRequestURI()).thenReturn("/oauth2/callback"); + identityProviderRepository.addIdentityProvider(new FakeBasicIdentityProvider(providerKey, true)); + + underTest.doFilter(request, response, chain); + + assertError("Fail to callback authentication"); + } + private void assertCallbackCalled(){ assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); assertThat(oAuth2IdentityProvider.isCallbackCalled()).isTrue();