SONAR-7396 Improve error message when base url is badly configured

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 dd09ebe9d7b..d7b9e2a9606 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 @@ import static org.sonar.server.authentication.AuthenticationError.handleUnauthor
 
 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 class InitFilter extends ServletFilter {
 
   @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 class InitFilter extends ServletFilter {
     }
   }
 
+  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 4701be2bc5a..3c708eb1a4d 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.ServletRequest;
 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 class OAuth2CallbackFilter extends ServletFilter {
 
   @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 class OAuth2CallbackFilter extends ServletFilter {
     } 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 b6bbffa6221..60cb6ff6fa9 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 class OAuth2ContextFactory {
       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 532d45430c0..ef82a2def3e 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 class InitFilterTest {
   }
 
   @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 04d58e6928e..ef19c6a298b 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 class OAuth2CallbackFilterTest {
 
     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 class OAuth2CallbackFilterTest {
     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();