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

diff --git a/it/it-tests/src/test/java/it/user/SsoAuthenticationTest.java b/it/it-tests/src/test/java/it/user/SsoAuthenticationTest.java
index 78816596e2daa4317c58fcee1ff3079a0b773e05..d13ff117f46620527b7197415281858fcec04578 100644 (file)
--- a/it/it-tests/src/test/java/it/user/SsoAuthenticationTest.java
+++ b/it/it-tests/src/test/java/it/user/SsoAuthenticationTest.java
@@ -88,6 +88,16 @@ public class SsoAuthenticationTest {
     USER_RULE.verifyUserExists(USER_LOGIN, USER_LOGIN, null);
   }
 
+  @Test
+  public void update_user_when_headers_are_updated() {
+    call(USER_LOGIN, USER_NAME, USER_EMAIL, null);
+    USER_RULE.verifyUserExists(USER_LOGIN, USER_NAME, USER_EMAIL);
+
+    // As we don't keep the JWT token is the test, the user is updated
+    call(USER_LOGIN, "new name", "new email", null);
+    USER_RULE.verifyUserExists(USER_LOGIN, "new name", "new email");
+  }
+
   @Test
   public void authenticate_with_groups() {
     call(USER_LOGIN, null, null, GROUP_1);
@@ -116,12 +126,15 @@ public class SsoAuthenticationTest {
   }
 
   @Test
-  public void fail_when_login_is_invalid() throws Exception {
+  public void display_message_in_ui_but_not_in_log_when_unauthorized_exception() throws Exception {
     Response response = doCall("invalid login $", null, null, null);
 
-    assertThat(response.code()).isEqualTo(500);
+    assertThat(response.code()).isEqualTo(200);
+    assertThat(response.body().string()).contains("You're not authorized to access this page. Please contact the administrator");
+
     List<String> logsLines = FileUtils.readLines(orchestrator.getServer().getLogs(), Charsets.UTF_8);
-    assertThat(logsLines).contains("org.sonar.server.exceptions.BadRequestException: user.bad_login");
+    assertThat(logsLines).doesNotContain("org.sonar.server.exceptions.BadRequestException: user.bad_login");
+    USER_RULE.verifyUserDoesNotExist(USER_LOGIN);
   }
 
   private static Response call(String login, @Nullable String name, @Nullable String email, @Nullable String groups) {

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 239806115d9bb7d490121a0e53fbdab97c99d8cc..17aba9a45b4b75a19bd9d916072fa2bb1bf42b9f 100644 (file)
--- 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 73ca6d740d4be64d90ad31449957306e9968d910..271fa5e2f94f2a6e1cd942bb1c878f039ea74320 100644 (file)
--- 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 99c3bb511ead400a45d2c4f17a014c14b17dfe0f..310f9385d11af0f0b1dd3070fcfe75d1cebeb689 100644 (file)
--- 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 3768bfc388bf163dbb9b93e7625891012b6b772b..d04643132d1631f3fc77e67b31f976245f1aa258 100644 (file)
--- 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;
@@ -203,6 +204,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();