SONAR-5430 Improve error handling

When a functional error is generated, the user is redirect to a page where he can see the error, and not errors in logs are generated

4 files changed, 39 insertions, 0 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/SsoAuthenticator.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/SsoAuthenticator.java
index 239806115d9..17aba9a45b4 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/SsoAuthenticator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/SsoAuthenticator.java
@@ -35,9 +35,11 @@ import javax.servlet.http.HttpServletResponse;
 import org.sonar.api.config.Settings;
 import org.sonar.api.server.authentication.Display;
 import org.sonar.api.server.authentication.IdentityProvider;
+import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.api.utils.System2;
 import org.sonar.db.user.UserDto;
+import org.sonar.server.exceptions.BadRequestException;
 
 import static org.apache.commons.lang.StringUtils.defaultIfBlank;
 import static org.apache.commons.lang.time.DateUtils.addMinutes;
@@ -86,6 +88,14 @@ public class SsoAuthenticator {
   }
 
   public Optional<UserDto> authenticate(HttpServletRequest request, HttpServletResponse response) {
+    try {
+      return doAuthenticate(request, response);
+    } catch (BadRequestException e) {
+      throw new UnauthorizedException(e.getMessage(), e);
+    }
+  }
+
+  private Optional<UserDto> doAuthenticate(HttpServletRequest request, HttpServletResponse response) {
     if (!settings.getBoolean(ENABLE_PARAM)) {
       return Optional.empty();
     }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserSessionFilter.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserSessionFilter.java
index 73ca6d740d4..271fa5e2f94 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/user/UserSessionFilter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserSessionFilter.java
@@ -30,11 +30,14 @@ import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.server.authentication.UserSessionInitializer;
 import org.sonar.server.organization.DefaultOrganizationCache;
 import org.sonar.server.platform.Platform;
 import org.sonar.server.setting.ThreadLocalSettings;
 
+import static org.sonar.server.authentication.AuthenticationError.handleUnauthorizedError;
+
 public class UserSessionFilter implements Filter {
   private final Platform platform;
 
@@ -64,6 +67,10 @@ public class UserSessionFilter implements Filter {
       } finally {
         settings.unload();
       }
+    } catch (UnauthorizedException e) {
+      // Functional exceptions generated by Identity provider API (for instance, when authenticating with SSO and a bad login is given)
+      // should redirect the user to a unauthorized page with the error message
+      handleUnauthorizedError(e, response);
     } finally {
       defaultOrganizationCache.unload();
     }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/SsoAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/SsoAuthenticatorTest.java
index 99c3bb511ea..310f9385d11 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/authentication/SsoAuthenticatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/SsoAuthenticatorTest.java
@@ -34,6 +34,7 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.config.MapSettings;
 import org.sonar.api.config.Settings;
+import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.api.utils.System2;
 import org.sonar.api.utils.internal.AlwaysIncreasingSystem2;
 import org.sonar.core.util.stream.Collectors;
@@ -315,6 +316,16 @@ public class SsoAuthenticatorTest {
     verifyZeroInteractions(jwtHttpHandler);
   }
 
+  @Test
+  public void throw_UnauthorizedException_when_BadRequestException_is_generated() throws Exception {
+    enableSso();
+    setNotUserInToken();
+
+    expectedException.expect(UnauthorizedException.class);
+    expectedException.expectMessage("user.bad_login");
+    underTest.authenticate(createRequest("invalid login", DEFAULT_NAME, DEFAULT_EMAIL, GROUPS), response);
+  }
+
   private void enableSso() {
     settings.setProperty("sonar.sso.enable", true);
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserSessionFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserSessionFilterTest.java
index 3768bfc388b..d04643132d1 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserSessionFilterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserSessionFilterTest.java
@@ -29,6 +29,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InOrder;
 import org.mockito.Mockito;
+import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.server.authentication.UserSessionInitializer;
 import org.sonar.server.organization.DefaultOrganizationCache;
 import org.sonar.server.platform.Platform;
@@ -204,6 +205,16 @@ public class UserSessionFilterTest {
   }
 
   @Test
+  public void send_redirect_when_catching_functional_unauthorized_errors() throws Exception {
+    mockUserSessionInitializer(true);
+    doThrow(new UnauthorizedException("bad login")).when(userSessionInitializer).initUserSession(request, response);
+
+    underTest.doFilter(request, response, chain);
+
+    verify(response).sendRedirect("/sessions/unauthorized?message=bad+login");
+  }
+
+  @Test
   public void just_for_fun_and_coverage() throws ServletException {
     UserSessionFilter filter = new UserSessionFilter();
     filter.init(mock(FilterConfig.class));