From ec5d3d86508242b24f94151073eabf1cb1f9dd20 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 29 Nov 2016 18:04:01 +0100 Subject: [PATCH] SONAR-8416 add event log on error in OAuth2 and Base authent --- .../authentication/AuthenticationError.java | 20 +-- .../server/authentication/InitFilter.java | 92 ++++++++++--- .../authentication/OAuth2CallbackFilter.java | 80 ++++++++--- .../authentication/SsoAuthenticator.java | 7 +- .../event/AuthenticationEvent.java | 3 +- .../event/AuthenticationException.java | 14 ++ .../sonar/server/user/UserSessionFilter.java | 7 - .../BasicAuthenticatorTest.java | 6 +- .../CredentialsAuthenticatorTest.java | 8 +- .../FakeBasicIdentityProvider.java | 1 + .../server/authentication/InitFilterTest.java | 129 +++++++++++------- .../authentication/JwtCsrfVerifierTest.java | 12 +- .../authentication/JwtSerializerTest.java | 8 +- .../OAuth2CallbackFilterTest.java | 43 ++++-- .../authentication/OAuthCsrfVerifierTest.java | 8 +- .../RealmAuthenticatorTest.java | 6 +- .../authentication/SsoAuthenticatorTest.java | 6 +- .../UserIdentityAuthenticatorTest.java | 4 +- .../event/AuthenticationExceptionMatcher.java | 47 +++++-- .../server/user/UserSessionFilterTest.java | 11 -- 20 files changed, 339 insertions(+), 173 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationError.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationError.java index c17c7028e3f..dc830b0ad73 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationError.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationError.java @@ -23,14 +23,14 @@ import com.google.common.base.Charsets; import java.io.IOException; import java.io.UnsupportedEncodingException; import javax.servlet.http.HttpServletResponse; -import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.server.authentication.event.AuthenticationException; import static java.lang.String.format; import static java.net.URLEncoder.encode; -public class AuthenticationError { +final class AuthenticationError { private static final String UNAUTHORIZED_PATH = "/sessions/unauthorized"; private static final String UNAUTHORIZED_PATH_WITH_MESSAGE = UNAUTHORIZED_PATH + "?message=%s"; @@ -40,22 +40,26 @@ public class AuthenticationError { // Utility class } - public static void handleError(Exception e, HttpServletResponse response, String message) { + static void handleError(Exception e, HttpServletResponse response, String message) { LOGGER.error(message, e); redirectToUnauthorized(response); } - public static void handleError(HttpServletResponse response, String message) { + static void handleError(HttpServletResponse response, String message) { LOGGER.error(message); redirectToUnauthorized(response); } - public static void handleUnauthorizedError(UnauthorizedException e, HttpServletResponse response) { + static void handleAuthenticationError(AuthenticationException e, HttpServletResponse response) { redirectTo(response, getPath(e)); } - private static String getPath(UnauthorizedException e) { - return format(UNAUTHORIZED_PATH_WITH_MESSAGE, encodeMessage(e.getMessage())); + private static String getPath(AuthenticationException e) { + String publicMessage = e.getPublicMessage(); + if (publicMessage == null || publicMessage.isEmpty()) { + return UNAUTHORIZED_PATH; + } + return format(UNAUTHORIZED_PATH_WITH_MESSAGE, encodeMessage(publicMessage)); } private static String encodeMessage(String message) { @@ -66,7 +70,7 @@ public class AuthenticationError { } } - private static void redirectToUnauthorized(HttpServletResponse response) { + public static void redirectToUnauthorized(HttpServletResponse response) { redirectTo(response, UNAUTHORIZED_PATH); } 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 00e4a5b6e46..a95f788b810 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 @@ -20,6 +20,7 @@ package org.sonar.server.authentication; import java.io.IOException; +import javax.annotation.CheckForNull; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; @@ -33,11 +34,14 @@ 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 org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; +import static org.sonar.server.authentication.AuthenticationError.handleAuthenticationError; import static org.sonar.server.authentication.AuthenticationError.handleError; -import static org.sonar.server.authentication.AuthenticationError.handleUnauthorizedError; +import static org.sonar.server.authentication.event.AuthenticationEvent.Source; public class InitFilter extends ServletFilter { @@ -47,12 +51,15 @@ public class InitFilter extends ServletFilter { private final BaseContextFactory baseContextFactory; private final OAuth2ContextFactory oAuth2ContextFactory; private final Server server; + private final AuthenticationEvent authenticationEvent; - public InitFilter(IdentityProviderRepository identityProviderRepository, BaseContextFactory baseContextFactory, OAuth2ContextFactory oAuth2ContextFactory, Server server) { + public InitFilter(IdentityProviderRepository identityProviderRepository, BaseContextFactory baseContextFactory, + OAuth2ContextFactory oAuth2ContextFactory, Server server, AuthenticationEvent authenticationEvent) { this.identityProviderRepository = identityProviderRepository; this.baseContextFactory = baseContextFactory; this.oAuth2ContextFactory = oAuth2ContextFactory; this.server = server; + this.authenticationEvent = authenticationEvent; } @Override @@ -63,35 +70,80 @@ public class InitFilter extends ServletFilter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; - String requestURI = httpRequest.getRequestURI(); - String keyProvider = ""; + HttpServletResponse httpResponse = (HttpServletResponse) response; + + IdentityProvider provider = resolveProviderOrHandleResponse(httpRequest, httpResponse); + if (provider != null) { + handleProvider(httpRequest, httpResponse, provider); + } + } + + @CheckForNull + private IdentityProvider resolveProviderOrHandleResponse(HttpServletRequest request, HttpServletResponse response) { + String requestURI = request.getRequestURI(); + String providerKey = extractKeyProvider(requestURI, server.getContextPath() + INIT_CONTEXT); + if (providerKey == null) { + handleError(response, "No provider key found in URI"); + return null; + } try { - keyProvider = extractKeyProvider(requestURI, server.getContextPath() + INIT_CONTEXT); - IdentityProvider provider = identityProviderRepository.getEnabledByKey(keyProvider); - if (provider instanceof BaseIdentityProvider) { - BaseIdentityProvider baseIdentityProvider = (BaseIdentityProvider) provider; - baseIdentityProvider.init(baseContextFactory.newContext(httpRequest, (HttpServletResponse) response, baseIdentityProvider)); - } else if (provider instanceof OAuth2IdentityProvider) { - OAuth2IdentityProvider oAuth2IdentityProvider = (OAuth2IdentityProvider) provider; - oAuth2IdentityProvider.init(oAuth2ContextFactory.newContext(httpRequest, (HttpServletResponse) response, oAuth2IdentityProvider)); - } else { - throw new UnsupportedOperationException(format("Unsupported IdentityProvider class: %s ", provider.getClass())); - } - } catch (UnauthorizedException e) { - handleUnauthorizedError(e, (HttpServletResponse) response); + return identityProviderRepository.getEnabledByKey(providerKey); } catch (Exception e) { - handleError(e, (HttpServletResponse) response, format("Fail to initialize authentication with provider '%s'", keyProvider)); + handleError(e, response, format("Failed to retrieve IdentityProvider for key '%s'", providerKey)); + return null; } } - public static String extractKeyProvider(String requestUri, String context) { + @CheckForNull + private 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."); + return null; + } + + private void handleProvider(HttpServletRequest request, HttpServletResponse response, IdentityProvider provider) { + try { + if (provider instanceof BaseIdentityProvider) { + handleBaseIdentityProvider(request, response, (BaseIdentityProvider) provider); + } else if (provider instanceof OAuth2IdentityProvider) { + handleOAuth2IdentityProvider(request, response, (OAuth2IdentityProvider) provider); + } else { + handleError(response, format("Unsupported IdentityProvider class: %s", provider.getClass())); + } + } catch (AuthenticationException e) { + authenticationEvent.failure(request, e); + handleAuthenticationError(e, response); + } catch (Exception e) { + handleError(e, response, format("Fail to initialize authentication with provider '%s'", provider.getKey())); + } + } + + private void handleBaseIdentityProvider(HttpServletRequest request, HttpServletResponse response, BaseIdentityProvider provider) { + try { + provider.init(baseContextFactory.newContext(request, response, provider)); + } catch (UnauthorizedException e) { + throw AuthenticationException.newBuilder() + .setSource(Source.external(provider)) + .setMessage(e.getMessage()) + .setPublicMessage(e.getMessage()) + .build(); + } + } + + private void handleOAuth2IdentityProvider(HttpServletRequest request, HttpServletResponse response, OAuth2IdentityProvider provider) { + try { + provider.init(oAuth2ContextFactory.newContext(request, response, provider)); + } catch (UnauthorizedException e) { + throw AuthenticationException.newBuilder() + .setSource(Source.oauth2(provider)) + .setMessage(e.getMessage()) + .setPublicMessage(e.getMessage()) + .build(); + } } @Override 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 b208b9aa281..d376634d3db 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 @@ -28,7 +28,6 @@ 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.platform.Server; import org.sonar.api.server.authentication.IdentityProvider; import org.sonar.api.server.authentication.OAuth2IdentityProvider; @@ -36,11 +35,12 @@ import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.server.authentication.UserIdentity; import org.sonar.api.web.ServletFilter; import org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; +import static org.sonar.server.authentication.AuthenticationError.handleAuthenticationError; import static org.sonar.server.authentication.AuthenticationError.handleError; -import static org.sonar.server.authentication.AuthenticationError.handleUnauthorizedError; import static org.sonar.server.authentication.event.AuthenticationEvent.Source; public class OAuth2CallbackFilter extends ServletFilter { @@ -68,29 +68,31 @@ public class OAuth2CallbackFilter extends ServletFilter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; - String requestUri = httpRequest.getRequestURI(); - String keyProvider = ""; + HttpServletResponse httpResponse = (HttpServletResponse) response; + + IdentityProvider provider = resolveProviderOrHandleResponse(httpRequest, httpResponse); + if (provider != null) { + handleProvider(httpRequest, (HttpServletResponse) response, provider); + } + } + + @CheckForNull + private IdentityProvider resolveProviderOrHandleResponse(HttpServletRequest request, HttpServletResponse response) { + String requestUri = request.getRequestURI(); + String providerKey = extractKeyProvider(requestUri, server.getContextPath() + CALLBACK_PATH); + if (providerKey == null) { + handleError(response, "No provider key found in URI"); + return null; + } try { - keyProvider = extractKeyProvider(requestUri, server.getContextPath() + CALLBACK_PATH); - IdentityProvider provider = identityProviderRepository.getEnabledByKey(keyProvider); - if (provider instanceof OAuth2IdentityProvider) { - OAuth2IdentityProvider oauthProvider = (OAuth2IdentityProvider) provider; - WrappedContext context = new WrappedContext(oAuth2ContextFactory.newCallback(httpRequest, (HttpServletResponse) response, oauthProvider)); - oauthProvider.callback(context); - if (context.isAuthenticated()) { - authenticationEvent.login(httpRequest, context.getLogin(), Source.oauth2(oauthProvider)); - } - } else { - handleError((HttpServletResponse) response, format("Not an OAuth2IdentityProvider: %s", provider.getClass())); - } - } catch (UnauthorizedException e) { - handleUnauthorizedError(e, (HttpServletResponse) response); + return identityProviderRepository.getEnabledByKey(providerKey); } catch (Exception e) { - handleError(e, (HttpServletResponse) response, - keyProvider.isEmpty() ? "Fail to callback authentication" : format("Fail to callback authentication with '%s'", keyProvider)); + handleError(e, response, format("Failed to retrieve IdentityProvider for key '%s'", providerKey)); + return null; } } + @CheckForNull private static String extractKeyProvider(String requestUri, String context) { if (requestUri.contains(context)) { String key = requestUri.replace(context, ""); @@ -98,7 +100,43 @@ public class OAuth2CallbackFilter extends ServletFilter { 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)); + return null; + } + + private void handleProvider(HttpServletRequest request, HttpServletResponse response, IdentityProvider provider) { + try { + if (provider instanceof OAuth2IdentityProvider) { + handleOAuth2Provider(response, request, (OAuth2IdentityProvider) provider); + } else { + handleError(response, format("Not an OAuth2IdentityProvider: %s", provider.getClass())); + } + } catch (AuthenticationException e) { + authenticationEvent.failure(request, e); + handleAuthenticationError(e, response); + } catch (Exception e) { + handleError(e, response, format("Fail to callback authentication with '%s'", provider.getKey())); + } + } + + private void handleOAuth2Provider(HttpServletResponse response, HttpServletRequest httpRequest, OAuth2IdentityProvider oAuth2Provider) { + OAuth2CallbackFilter.WrappedContext context = new OAuth2CallbackFilter.WrappedContext(oAuth2ContextFactory.newCallback(httpRequest, response, oAuth2Provider)); + try { + oAuth2Provider.callback(context); + } catch (UnauthorizedException e) { + throw AuthenticationException.newBuilder() + .setSource(Source.oauth2(oAuth2Provider)) + .setMessage(e.getMessage()) + .setPublicMessage(e.getMessage()) + .build(); + } + if (context.isAuthenticated()) { + authenticationEvent.login(httpRequest, context.getLogin(), Source.oauth2(oAuth2Provider)); + } else { + throw AuthenticationException.newBuilder() + .setSource(Source.oauth2(oAuth2Provider)) + .setMessage("Plugin did not call authenticate") + .build(); + } } @Override 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 9cac78e0f90..cd451874871 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 @@ -36,13 +36,13 @@ import org.sonar.api.Startable; 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.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.db.user.UserDto; import org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; import org.sonar.server.exceptions.BadRequestException; import static org.apache.commons.lang.StringUtils.defaultIfBlank; @@ -119,7 +119,10 @@ public class SsoAuthenticator implements Startable { try { return doAuthenticate(request, response); } catch (BadRequestException e) { - throw new UnauthorizedException(e.getMessage(), e); + throw AuthenticationException.newBuilder() + .setSource(Source.sso()) + .setMessage(e.getMessage()) + .build(); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationEvent.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationEvent.java index 497bf058b0b..d0eeac4f932 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationEvent.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationEvent.java @@ -24,6 +24,7 @@ import java.util.Objects; import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; import org.sonar.api.server.authentication.BaseIdentityProvider; +import org.sonar.api.server.authentication.IdentityProvider; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import static com.google.common.base.Preconditions.checkArgument; @@ -127,7 +128,7 @@ public interface AuthenticationEvent { return JWT_INSTANCE; } - public static Source external(BaseIdentityProvider identityProvider) { + public static Source external(IdentityProvider identityProvider) { return new Source( Method.EXTERNAL, Provider.EXTERNAL, requireNonNull(identityProvider, "identityProvider can't be null").getName()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationException.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationException.java index 64ae64ea40a..f60c3f5b04a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationException.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/event/AuthenticationException.java @@ -38,11 +38,13 @@ public class AuthenticationException extends RuntimeException { private final AuthenticationEvent.Source source; @CheckForNull private final String login; + private final String publicMessage; private AuthenticationException(Builder builder) { super(builder.message); this.source = requireNonNull(builder.source, "source can't be null"); this.login = builder.login; + this.publicMessage = builder.publicMessage; } public AuthenticationEvent.Source getSource() { @@ -54,6 +56,11 @@ public class AuthenticationException extends RuntimeException { return login; } + @CheckForNull + public String getPublicMessage() { + return publicMessage; + } + public static Builder newBuilder() { return new Builder(); } @@ -65,6 +72,8 @@ public class AuthenticationException extends RuntimeException { private String login; @CheckForNull private String message; + @CheckForNull + private String publicMessage; private Builder() { // use static factory method @@ -85,6 +94,11 @@ public class AuthenticationException extends RuntimeException { return this; } + public Builder setPublicMessage(String publicMessage) { + this.publicMessage = publicMessage; + return this; + } + public AuthenticationException build() { return new AuthenticationException(this); } 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 271fa5e2f94..73ca6d740d4 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,14 +30,11 @@ 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; @@ -67,10 +64,6 @@ 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/BasicAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/BasicAuthenticatorTest.java index 9191da50352..2cdb832e8de 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/BasicAuthenticatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/BasicAuthenticatorTest.java @@ -121,7 +121,7 @@ public class BasicAuthenticatorTest { public void fail_to_authenticate_when_no_login() throws Exception { when(request.getHeader("Authorization")).thenReturn("Basic " + toBase64(":" + PASSWORD)); - expectedException.expect(authenticationException().from(Source.local(BASIC)).withoutLogin()); + expectedException.expect(authenticationException().from(Source.local(BASIC)).withoutLogin().andNoPublicMessage()); try { underTest.authenticate(request); } finally { @@ -157,7 +157,7 @@ public class BasicAuthenticatorTest { when(userTokenAuthenticator.authenticate("token")).thenReturn(Optional.empty()); when(request.getHeader("Authorization")).thenReturn("Basic " + toBase64("token:")); - expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withoutLogin()); + expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withoutLogin().andNoPublicMessage()); try { underTest.authenticate(request); } finally { @@ -171,7 +171,7 @@ public class BasicAuthenticatorTest { when(userTokenAuthenticator.authenticate("token")).thenReturn(Optional.of("Unknown user")); when(request.getHeader("Authorization")).thenReturn("Basic " + toBase64("token:")); - expectedException.expect(authenticationException().from(Source.local(Method.BASIC_TOKEN)).withoutLogin()); + expectedException.expect(authenticationException().from(Source.local(Method.BASIC_TOKEN)).withoutLogin().andNoPublicMessage()); try { underTest.authenticate(request); } finally { diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/CredentialsAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/CredentialsAuthenticatorTest.java index cd5f1d13b6c..2ba2a6fb839 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/CredentialsAuthenticatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/CredentialsAuthenticatorTest.java @@ -87,7 +87,7 @@ public class CredentialsAuthenticatorTest { .setSalt("Wrong salt") .setLocal(true)); - expectedException.expect(authenticationException().from(Source.local(BASIC)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.local(BASIC)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("wrong password"); try { executeAuthenticate(BASIC); @@ -116,7 +116,7 @@ public class CredentialsAuthenticatorTest { .setLogin(LOGIN) .setLocal(false)); - expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("User is not local"); try { executeAuthenticate(BASIC_TOKEN); @@ -133,7 +133,7 @@ public class CredentialsAuthenticatorTest { .setSalt(SALT) .setLocal(true)); - expectedException.expect(authenticationException().from(Source.local(BASIC)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.local(BASIC)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("null password in DB"); try { executeAuthenticate(BASIC); @@ -150,7 +150,7 @@ public class CredentialsAuthenticatorTest { .setSalt(null) .setLocal(true)); - expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.local(BASIC_TOKEN)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("null salt"); try { executeAuthenticate(BASIC_TOKEN); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/FakeBasicIdentityProvider.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/FakeBasicIdentityProvider.java index a0ab806fb10..a6495c25a1f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/FakeBasicIdentityProvider.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/FakeBasicIdentityProvider.java @@ -27,6 +27,7 @@ class FakeBasicIdentityProvider extends TestIdentityProvider implements BaseIden public FakeBasicIdentityProvider(String key, boolean enabled) { setKey(key); + setName("name of " + key); setEnabled(enabled); } 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 e4b147416fe..10dabe5f749 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 @@ -19,11 +19,6 @@ */ package org.sonar.server.authentication; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import javax.servlet.FilterChain; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -31,6 +26,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; import org.sonar.api.platform.Server; import org.sonar.api.server.authentication.BaseIdentityProvider; import org.sonar.api.server.authentication.Display; @@ -39,36 +35,47 @@ import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; +import org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; public class InitFilterTest { - static String OAUTH2_PROVIDER_KEY = "github"; - static String BASIC_PROVIDER_KEY = "openid"; + private static final String OAUTH2_PROVIDER_KEY = "github"; + private static final String BASIC_PROVIDER_KEY = "openid"; @Rule public LogTester logTester = new LogTester(); @Rule public ExpectedException thrown = ExpectedException.none(); - @Rule public IdentityProviderRepositoryRule identityProviderRepository = new IdentityProviderRepositoryRule(); - BaseContextFactory baseContextFactory = mock(BaseContextFactory.class); - OAuth2ContextFactory oAuth2ContextFactory = mock(OAuth2ContextFactory.class); - Server server = mock(Server.class); + private BaseContextFactory baseContextFactory = mock(BaseContextFactory.class); + private OAuth2ContextFactory oAuth2ContextFactory = mock(OAuth2ContextFactory.class); + private Server server = mock(Server.class); - HttpServletRequest request = mock(HttpServletRequest.class); - HttpServletResponse response = mock(HttpServletResponse.class); - FilterChain chain = mock(FilterChain.class); + private HttpServletRequest request = mock(HttpServletRequest.class); + private HttpServletResponse response = mock(HttpServletResponse.class); + private FilterChain chain = mock(FilterChain.class); - FakeOAuth2IdentityProvider oAuth2IdentityProvider = new FakeOAuth2IdentityProvider(OAUTH2_PROVIDER_KEY, true); - OAuth2IdentityProvider.InitContext oauth2Context = mock(OAuth2IdentityProvider.InitContext.class); + private FakeOAuth2IdentityProvider oAuth2IdentityProvider = new FakeOAuth2IdentityProvider(OAUTH2_PROVIDER_KEY, true); + private OAuth2IdentityProvider.InitContext oauth2Context = mock(OAuth2IdentityProvider.InitContext.class); - FakeBasicIdentityProvider baseIdentityProvider = new FakeBasicIdentityProvider(BASIC_PROVIDER_KEY, true); - BaseIdentityProvider.Context baseContext = mock(BaseIdentityProvider.Context.class); + private FakeBasicIdentityProvider baseIdentityProvider = new FakeBasicIdentityProvider(BASIC_PROVIDER_KEY, true); + private BaseIdentityProvider.Context baseContext = mock(BaseIdentityProvider.Context.class); + private AuthenticationEvent authenticationEvent = mock(AuthenticationEvent.class); - InitFilter underTest = new InitFilter(identityProviderRepository, baseContextFactory, oAuth2ContextFactory, server); + private ArgumentCaptor authenticationExceptionCaptor = ArgumentCaptor.forClass(AuthenticationException.class); + + private InitFilter underTest = new InitFilter(identityProviderRepository, baseContextFactory, oAuth2ContextFactory, server, authenticationEvent); @Before public void setUp() throws Exception { @@ -91,6 +98,7 @@ public class InitFilterTest { underTest.doFilter(request, response, chain); assertOAuth2InitCalled(); + verifyZeroInteractions(authenticationEvent); } @Test @@ -101,6 +109,7 @@ public class InitFilterTest { underTest.doFilter(request, response, chain); assertOAuth2InitCalled(); + verifyZeroInteractions(authenticationEvent); } @Test @@ -111,6 +120,7 @@ public class InitFilterTest { underTest.doFilter(request, response, chain); assertBasicInitCalled(); + verifyZeroInteractions(authenticationEvent); } @Test @@ -119,52 +129,32 @@ public class InitFilterTest { underTest.doFilter(request, response, chain); - assertError("Fail to initialize authentication with provider ''"); + assertError("No provider key found in URI"); + verifyZeroInteractions(authenticationEvent); } @Test - public void fail_if_uri_doesnt_contains_callback() throws Exception { + public void fail_if_uri_does_not_contains_callback() throws Exception { when(request.getRequestURI()).thenReturn("/sessions/init"); underTest.doFilter(request, response, chain); - assertError("Fail to initialize authentication with provider ''"); + assertError("No provider key found in URI"); + verifyZeroInteractions(authenticationEvent); } @Test public void fail_if_identity_provider_class_is_unsupported() throws Exception { - final String unsupportedKey = "unsupported"; + String unsupportedKey = "unsupported"; when(request.getRequestURI()).thenReturn("/sessions/init/" + unsupportedKey); - identityProviderRepository.addIdentityProvider(new IdentityProvider() { - @Override - public String getKey() { - return unsupportedKey; - } - - @Override - public String getName() { - return null; - } - - @Override - public Display getDisplay() { - return null; - } - - @Override - public boolean isEnabled() { - return true; - } - - @Override - public boolean allowsUsersToSignUp() { - return false; - } - }); + IdentityProvider identityProvider = new UnsupportedIdentityProvider(unsupportedKey); + identityProviderRepository.addIdentityProvider(identityProvider); underTest.doFilter(request, response, chain); - assertError("Fail to initialize authentication with provider 'unsupported'"); + assertError("Unsupported IdentityProvider class: class org.sonar.server.authentication.InitFilterTest$UnsupportedIdentityProvider"); + verifyZeroInteractions(authenticationEvent); + } @Test @@ -176,6 +166,12 @@ public class InitFilterTest { underTest.doFilter(request, response, chain); verify(response).sendRedirect("/sessions/unauthorized?message=Email+john%40email.com+is+already+used"); + verify(authenticationEvent).failure(eq(request), authenticationExceptionCaptor.capture()); + AuthenticationException authenticationException = authenticationExceptionCaptor.getValue(); + assertThat(authenticationException).hasMessage("Email john@email.com is already used"); + assertThat(authenticationException.getSource()).isEqualTo(AuthenticationEvent.Source.external(identityProvider)); + assertThat(authenticationException.getLogin()).isNull(); + assertThat(authenticationException.getPublicMessage()).isEqualTo("Email john@email.com is already used"); } private void assertOAuth2InitCalled() { @@ -205,4 +201,37 @@ public class InitFilterTest { throw new UnauthorizedException("Email john@email.com is already used"); } } + + private static class UnsupportedIdentityProvider implements IdentityProvider { + private final String unsupportedKey; + + public UnsupportedIdentityProvider(String unsupportedKey) { + this.unsupportedKey = unsupportedKey; + } + + @Override + public String getKey() { + return unsupportedKey; + } + + @Override + public String getName() { + return null; + } + + @Override + public Display getDisplay() { + return null; + } + + @Override + public boolean isEnabled() { + return true; + } + + @Override + public boolean allowsUsersToSignUp() { + return false; + } + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java index 96e1fe51347..d3044aa6375 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java @@ -80,7 +80,7 @@ public class JwtCsrfVerifierTest { mockRequestCsrf("other value"); mockPostJavaWsRequest(); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("wrong CSFR in request"); underTest.verifyState(request, CSRF_STATE, LOGIN); } @@ -90,7 +90,7 @@ public class JwtCsrfVerifierTest { mockRequestCsrf(CSRF_STATE); mockPostJavaWsRequest(); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("missing reference CSRF value"); underTest.verifyState(request, null, LOGIN); } @@ -100,7 +100,7 @@ public class JwtCsrfVerifierTest { mockRequestCsrf(CSRF_STATE); mockPostJavaWsRequest(); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("missing reference CSRF value"); underTest.verifyState(request, "", LOGIN); } @@ -111,7 +111,7 @@ public class JwtCsrfVerifierTest { when(request.getRequestURI()).thenReturn(JAVA_WS_URL); when(request.getMethod()).thenReturn("POST"); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("wrong CSFR in request"); underTest.verifyState(request, CSRF_STATE, LOGIN); } @@ -122,7 +122,7 @@ public class JwtCsrfVerifierTest { when(request.getRequestURI()).thenReturn(JAVA_WS_URL); when(request.getMethod()).thenReturn("PUT"); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("wrong CSFR in request"); underTest.verifyState(request, CSRF_STATE, LOGIN); } @@ -133,7 +133,7 @@ public class JwtCsrfVerifierTest { when(request.getRequestURI()).thenReturn(JAVA_WS_URL); when(request.getMethod()).thenReturn("DELETE"); - thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN)); + thrown.expect(authenticationException().from(Source.local(Method.JWT)).withLogin(LOGIN).andNoPublicMessage()); thrown.expectMessage("wrong CSFR in request"); underTest.verifyState(request, CSRF_STATE, LOGIN); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtSerializerTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtSerializerTest.java index d54f1519dc6..357c586b060 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtSerializerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtSerializerTest.java @@ -156,7 +156,7 @@ public class JwtSerializerTest { .signWith(SignatureAlgorithm.HS256, decodeSecretKey(A_SECRET_KEY)) .compact(); - expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN)); + expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN).andNoPublicMessage()); expectedException.expectMessage("Token id hasn't been found"); underTest.decode(token); } @@ -174,7 +174,7 @@ public class JwtSerializerTest { .signWith(SignatureAlgorithm.HS256, decodeSecretKey(A_SECRET_KEY)) .compact(); - expectedException.expect(authenticationException().from(Source.jwt()).withoutLogin()); + expectedException.expect(authenticationException().from(Source.jwt()).withoutLogin().andNoPublicMessage()); expectedException.expectMessage("Token subject hasn't been found"); underTest.decode(token); } @@ -192,7 +192,7 @@ public class JwtSerializerTest { .signWith(SignatureAlgorithm.HS256, decodeSecretKey(A_SECRET_KEY)) .compact(); - expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN)); + expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN).andNoPublicMessage()); expectedException.expectMessage("Token expiration date hasn't been found"); underTest.decode(token); } @@ -209,7 +209,7 @@ public class JwtSerializerTest { .signWith(SignatureAlgorithm.HS256, decodeSecretKey(A_SECRET_KEY)) .compact(); - expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN)); + expectedException.expect(authenticationException().from(Source.jwt()).withLogin(USER_LOGIN).andNoPublicMessage()); expectedException.expectMessage("Token creation date hasn't been found"); underTest.decode(token); } 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 bc1c8b935ce..defff6598ca 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 @@ -26,6 +26,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; import org.sonar.api.platform.Server; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; @@ -33,8 +34,10 @@ import org.sonar.api.server.authentication.UserIdentity; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -63,6 +66,8 @@ public class OAuth2CallbackFilterTest { private FakeOAuth2IdentityProvider oAuth2IdentityProvider = new WellbehaveFakeOAuth2IdentityProvider(OAUTH2_PROVIDER_KEY, true, LOGIN); private AuthenticationEvent authenticationEvent = mock(AuthenticationEvent.class); + private ArgumentCaptor authenticationExceptionCaptor = ArgumentCaptor.forClass(AuthenticationException.class); + private OAuth2CallbackFilter underTest = new OAuth2CallbackFilter(identityProviderRepository, oAuth2ContextFactory, server, authenticationEvent); @Before @@ -84,7 +89,8 @@ public class OAuth2CallbackFilterTest { underTest.doFilter(request, response, chain); - assertCallbackCalled(oAuth2IdentityProvider, true); + assertCallbackCalled(oAuth2IdentityProvider); + verify(authenticationEvent).login(request, LOGIN, Source.oauth2(oAuth2IdentityProvider)); } @Test @@ -96,7 +102,13 @@ public class OAuth2CallbackFilterTest { underTest.doFilter(request, response, chain); - assertCallbackCalled(identityProvider, false); + assertCallbackCalled(identityProvider); + verify(authenticationEvent).failure(eq(request), authenticationExceptionCaptor.capture()); + AuthenticationException authenticationException = authenticationExceptionCaptor.getValue(); + assertThat(authenticationException).hasMessage("Plugin did not call authenticate"); + assertThat(authenticationException.getSource()).isEqualTo(Source.oauth2(identityProvider)); + assertThat(authenticationException.getLogin()).isNull(); + assertThat(authenticationException.getPublicMessage()).isNull(); } @Test @@ -106,7 +118,8 @@ public class OAuth2CallbackFilterTest { underTest.doFilter(request, response, chain); - assertCallbackCalled(oAuth2IdentityProvider, true); + assertCallbackCalled(oAuth2IdentityProvider); + verify(authenticationEvent).login(request, LOGIN, Source.oauth2(oAuth2IdentityProvider)); } @Test @@ -128,14 +141,16 @@ public class OAuth2CallbackFilterTest { underTest.doFilter(request, response, chain); - assertError("Fail to callback authentication with 'github'"); + assertError("Failed to retrieve IdentityProvider for key 'github'"); verifyZeroInteractions(authenticationEvent); } @Test public void redirect_when_failing_because_of_UnauthorizedExceptionException() throws Exception { - TestIdentityProvider identityProvider = new FailWithUnauthorizedExceptionIdProvider() + FailWithUnauthorizedExceptionIdProvider identityProvider = new FailWithUnauthorizedExceptionIdProvider(); + identityProvider .setKey("failing") + .setName("name of failing") .setEnabled(true); when(request.getRequestURI()).thenReturn("/oauth2/callback/" + identityProvider.getKey()); identityProviderRepository.addIdentityProvider(identityProvider); @@ -143,29 +158,27 @@ public class OAuth2CallbackFilterTest { underTest.doFilter(request, response, chain); verify(response).sendRedirect("/sessions/unauthorized?message=Email+john%40email.com+is+already+used"); - verifyZeroInteractions(authenticationEvent); + verify(authenticationEvent).failure(eq(request), authenticationExceptionCaptor.capture()); + AuthenticationException authenticationException = authenticationExceptionCaptor.getValue(); + assertThat(authenticationException).hasMessage("Email john@email.com is already used"); + assertThat(authenticationException.getSource()).isEqualTo(Source.oauth2(identityProvider)); + assertThat(authenticationException.getLogin()).isNull(); + assertThat(authenticationException.getPublicMessage()).isEqualTo("Email john@email.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"); + assertError("No provider key found in URI"); verifyZeroInteractions(authenticationEvent); } - private void assertCallbackCalled(FakeOAuth2IdentityProvider oAuth2IdentityProvider, boolean expectLoginLog) { + private void assertCallbackCalled(FakeOAuth2IdentityProvider oAuth2IdentityProvider) { assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); assertThat(oAuth2IdentityProvider.isCallbackCalled()).isTrue(); - if (expectLoginLog) { - verify(authenticationEvent).login(request, LOGIN, Source.oauth2(oAuth2IdentityProvider)); - } else { - verifyZeroInteractions(authenticationEvent); - } } private void assertError(String expectedError) throws Exception { diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java index 5f1825d6b1e..9704a0932fe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java @@ -91,7 +91,7 @@ public class OAuthCsrfVerifierTest { when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))}); when(request.getParameter("state")).thenReturn("other value"); - thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin()); + thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin().andNoPublicMessage()); thrown.expectMessage("CSRF state value is invalid"); underTest.verifyState(request, response, identityProvider); } @@ -101,7 +101,7 @@ public class OAuthCsrfVerifierTest { when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", null)}); when(request.getParameter("state")).thenReturn("state"); - thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin()); + thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin().andNoPublicMessage()); thrown.expectMessage("CSRF state value is invalid"); underTest.verifyState(request, response, identityProvider); } @@ -111,7 +111,7 @@ public class OAuthCsrfVerifierTest { when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))}); when(request.getParameter("state")).thenReturn(""); - thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin()); + thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin().andNoPublicMessage()); thrown.expectMessage("CSRF state value is invalid"); underTest.verifyState(request, response, identityProvider); } @@ -120,7 +120,7 @@ public class OAuthCsrfVerifierTest { public void fail_with_AuthenticationException_when_cookie_is_missing() throws Exception { when(request.getCookies()).thenReturn(new Cookie[] {}); - thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin()); + thrown.expect(authenticationException().from(AuthenticationEvent.Source.oauth2(identityProvider)).withoutLogin().andNoPublicMessage()); thrown.expectMessage("Cookie 'OAUTHSTATE' is missing"); underTest.verifyState(request, response, identityProvider); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/RealmAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/RealmAuthenticatorTest.java index 7651e4cdd6e..1256494838e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/RealmAuthenticatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/RealmAuthenticatorTest.java @@ -239,7 +239,7 @@ public class RealmAuthenticatorTest { when(externalUsersProvider.doGetUserDetails(any(ExternalUsersProvider.Context.class))).thenReturn(null); - expectedException.expect(authenticationException().from(Source.realm(BASIC, REALM_NAME)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.realm(BASIC, REALM_NAME)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("No user details"); try { underTest.authenticate(LOGIN, PASSWORD, request, BASIC); @@ -255,7 +255,7 @@ public class RealmAuthenticatorTest { when(authenticator.doAuthenticate(any(Authenticator.Context.class))).thenReturn(false); - expectedException.expect(authenticationException().from(Source.realm(BASIC, REALM_NAME)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.realm(BASIC, REALM_NAME)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage("realm returned authenticate=false"); try { underTest.authenticate(LOGIN, PASSWORD, request, BASIC); @@ -272,7 +272,7 @@ public class RealmAuthenticatorTest { when(externalUsersProvider.doGetUserDetails(any(ExternalUsersProvider.Context.class))).thenReturn(new UserDetails()); - expectedException.expect(authenticationException().from(Source.realm(BASIC_TOKEN, REALM_NAME)).withLogin(LOGIN)); + expectedException.expect(authenticationException().from(Source.realm(BASIC_TOKEN, REALM_NAME)).withLogin(LOGIN).andNoPublicMessage()); expectedException.expectMessage(expectedMessage); try { underTest.authenticate(LOGIN, PASSWORD, request, BASIC_TOKEN); 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 ab974915556..da40bd0bc47 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,7 +34,6 @@ 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; @@ -61,6 +60,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.db.user.UserTesting.newUserDto; import static org.sonar.server.authentication.event.AuthenticationEvent.Source; +import static org.sonar.server.authentication.event.AuthenticationExceptionMatcher.authenticationException; public class SsoAuthenticatorTest { @@ -339,11 +339,11 @@ public class SsoAuthenticatorTest { } @Test - public void throw_UnauthorizedException_when_BadRequestException_is_generated() throws Exception { + public void throw_AuthenticationException_when_BadRequestException_is_generated() throws Exception { startWithSso(); setNotUserInToken(); - expectedException.expect(UnauthorizedException.class); + expectedException.expect(authenticationException().from(Source.sso()).withoutLogin().andNoPublicMessage()); expectedException.expectMessage("user.bad_login"); try { underTest.authenticate(createRequest("invalid login", DEFAULT_NAME, DEFAULT_EMAIL, GROUPS), response); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java index a0b7e682a0e..6c649243caa 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserIdentityAuthenticatorTest.java @@ -361,7 +361,7 @@ public class UserIdentityAuthenticatorTest { .setAllowsUsersToSignUp(false); Source source = Source.realm(Method.FORM, identityProvider.getName()); - thrown.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getLogin())); + thrown.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getLogin()).andNoPublicMessage()); thrown.expectMessage("'github' users are not allowed to sign up"); underTest.authenticate(USER_IDENTITY, identityProvider, source); } @@ -374,7 +374,7 @@ public class UserIdentityAuthenticatorTest { .setEmail("john@email.com")); Source source = Source.realm(Method.FORM, IDENTITY_PROVIDER.getName()); - thrown.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getLogin())); + thrown.expect(authenticationException().from(source).withLogin(USER_IDENTITY.getLogin()).andNoPublicMessage()); thrown.expectMessage("You can't sign up because email 'john@email.com' is already used by an existing user. " + "This means that you probably already registered with another account."); underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, source); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/event/AuthenticationExceptionMatcher.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/event/AuthenticationExceptionMatcher.java index a0f1be6d976..2c64be7ceca 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/event/AuthenticationExceptionMatcher.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/event/AuthenticationExceptionMatcher.java @@ -33,7 +33,7 @@ import static org.sonar.server.authentication.event.AuthenticationEvent.Source; *

* Usage: * 

- * expectedException.expect(authenticationException().from(Source.local(Method.BASIC_TOKEN)).withoutLogin());
+ * expectedException.expect(authenticationException().from(Source.local(Method.BASIC_TOKEN)).withLogin("foo").andNoPublicMessage());
  *
*

*/ @@ -41,10 +41,13 @@ public class AuthenticationExceptionMatcher extends TypeSafeMatcher { private final Source source; @CheckForNull private final String login; + @CheckForNull + private final String publicMessage; - private AuthenticationExceptionMatcher(Source source, @Nullable String login) { + private AuthenticationExceptionMatcher(Source source, @Nullable String login, @Nullable String publicMessage) { this.source = requireNonNull(source, "source can't be null"); this.login = login; + this.publicMessage = publicMessage; } public static Builder authenticationException() { @@ -53,19 +56,29 @@ public class AuthenticationExceptionMatcher extends TypeSafeMatcher { public static class Builder { private Source source; + private String login; public Builder from(Source source) { this.source = checkSource(source); return this; } - public AuthenticationExceptionMatcher withLogin(String login) { - requireNonNull(login, "expected login can't be null"); - return new AuthenticationExceptionMatcher(checkSource(source), login); + public Builder withLogin(String login) { + this.login = requireNonNull(login, "expected login can't be null"); + return this; + } + + public Builder withoutLogin() { + this.login = null; + return this; + } + + public AuthenticationExceptionMatcher andNoPublicMessage() { + return new AuthenticationExceptionMatcher(this.source, this.login, null); } - public AuthenticationExceptionMatcher withoutLogin() { - return new AuthenticationExceptionMatcher(checkSource(source), null); + public AuthenticationExceptionMatcher andPublicMessage(String publicMessage){ + return new AuthenticationExceptionMatcher(this.source, this.login, requireNonNull(publicMessage)); } private static Source checkSource(Source source) { @@ -98,6 +111,17 @@ public class AuthenticationExceptionMatcher extends TypeSafeMatcher { return "login is \"" + login + "\""; } + String publicMessage = authenticationException.getPublicMessage(); + if (this.publicMessage == null) { + if (publicMessage != null) { + return "publicMessage is \"" + publicMessage + "\""; + } + } else if (publicMessage == null) { + return "publicMessage is null"; + } else if (!this.publicMessage.equals(publicMessage)) { + return "publicMessage is \"" + publicMessage + "\""; + } + return null; } @@ -105,9 +129,14 @@ public class AuthenticationExceptionMatcher extends TypeSafeMatcher { public void describeTo(Description description) { description.appendText("AuthenticationException with source ").appendText(source.toString()); if (login == null) { - description.appendText(" and no login"); + description.appendText(", no login"); + } else { + description.appendText(", login \"").appendText(login).appendText("\""); + } + if (publicMessage == null) { + description.appendText(" and no publicMessage"); } else { - description.appendText(" and login \"").appendText(login).appendText("\""); + description.appendText(" and publicMessage \"").appendText(publicMessage).appendText("\""); } } 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 d04643132d1..3768bfc388b 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,7 +29,6 @@ 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,16 +203,6 @@ 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(); -- 2.39.5