From 3219b99f2e675e9d14dd06ae68f4e005426f15b5 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 25 Jan 2016 16:23:06 +0100 Subject: [PATCH] SONAR-7217 Fail to authenticate user with existing email --- .../authentication/AuthenticationError.java | 7 ++- .../EmailAlreadyExistsException.java | 35 +++++++++++++ .../server/authentication/InitFilter.java | 3 ++ .../NotAllowUserToSignUpException.java | 2 + .../authentication/OAuth2CallbackFilter.java | 3 ++ .../UserIdentityAuthenticator.java | 6 +++ .../server/authentication/InitFilterTest.java | 51 +++++++++++++++++++ .../UserIdentityAuthenticatorTest.java | 10 ++++ .../views/sessions/email_already_exists.erb | 10 ++++ .../sessions/not_allowed_to_sign_up.html.erb | 2 +- .../main/java/org/sonar/db/user/UserDao.java | 7 +++ .../java/org/sonar/db/user/UserMapper.java | 2 + .../org/sonar/db/user/UserMapper.xml | 6 +++ .../java/org/sonar/db/user/UserDaoTest.java | 9 ++++ .../db/user/UserDaoTest/exists_by_email.xml | 8 +++ .../authentication/BaseIdentityProvider.java | 6 ++- .../OAuth2IdentityProvider.java | 3 +- 17 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/authentication/EmailAlreadyExistsException.java create mode 100644 server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/email_already_exists.erb create mode 100644 sonar-db/src/test/resources/org/sonar/db/user/UserDaoTest/exists_by_email.xml 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 35e4bcc3d74..c6628cdb055 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 @@ -25,13 +25,14 @@ import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import static java.lang.String.format; +import static org.sonar.server.authentication.EmailAlreadyExistsException.EMAIL_ALREADY_EXISTS_PATH; +import static org.sonar.server.authentication.NotAllowUserToSignUpException.NOT_ALLOWED_TO_SIGHNUP_PATH; public class AuthenticationError { private static final Logger LOGGER = Loggers.get(AuthenticationError.class); private static final String UNAUTHORIZED_PATH = "/sessions/unauthorized"; - private static final String NOT_ALLOWED_TO_SIGHNUP_PATH = "/sessions/not_allowed_to_sign_up?providerName=%s"; private AuthenticationError() { // Utility class @@ -51,6 +52,10 @@ public class AuthenticationError { redirectTo(response, format(NOT_ALLOWED_TO_SIGHNUP_PATH, e.getProvider().getName())); } + public static void handleEmailAlreadyExistsError(EmailAlreadyExistsException e, HttpServletResponse response) { + redirectTo(response, format(EMAIL_ALREADY_EXISTS_PATH, e.getEmail())); + } + private static void redirectToUnauthorized(HttpServletResponse response) { redirectTo(response, UNAUTHORIZED_PATH); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/EmailAlreadyExistsException.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/EmailAlreadyExistsException.java new file mode 100644 index 00000000000..67491a4822b --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/EmailAlreadyExistsException.java @@ -0,0 +1,35 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.authentication; + +public class EmailAlreadyExistsException extends RuntimeException { + + public static final String EMAIL_ALREADY_EXISTS_PATH = "/sessions/email_already_exists?email=%s"; + + private final String email; + + public EmailAlreadyExistsException(String email) { + this.email = email; + } + + public String getEmail() { + return email; + } +} 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 65b0eee5cb6..5111f8d3822 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 @@ -34,6 +34,7 @@ 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.handleEmailAlreadyExistsError; import static org.sonar.server.authentication.AuthenticationError.handleError; import static org.sonar.server.authentication.AuthenticationError.handleNotAllowedToSignUpError; @@ -78,6 +79,8 @@ public class InitFilter extends ServletFilter { } } catch (NotAllowUserToSignUpException e) { handleNotAllowedToSignUpError(e, (HttpServletResponse) response); + } catch (EmailAlreadyExistsException e) { + handleEmailAlreadyExistsError(e, (HttpServletResponse) response); } catch (Exception e) { handleError(e, (HttpServletResponse) response, String.format("Fail to initialize authentication with provider '%s'", keyProvider)); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/NotAllowUserToSignUpException.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/NotAllowUserToSignUpException.java index 02269964a45..2a1a2e7d9d7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/NotAllowUserToSignUpException.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/NotAllowUserToSignUpException.java @@ -23,6 +23,8 @@ import org.sonar.api.server.authentication.IdentityProvider; public class NotAllowUserToSignUpException extends RuntimeException { + public static final String NOT_ALLOWED_TO_SIGHNUP_PATH = "/sessions/not_allowed_to_sign_up?providerName=%s"; + private final IdentityProvider provider; public NotAllowUserToSignUpException(IdentityProvider provider) { 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 7240a936f7f..919a4302d76 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 @@ -31,6 +31,7 @@ import org.sonar.api.server.authentication.IdentityProvider; import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.web.ServletFilter; +import static org.sonar.server.authentication.AuthenticationError.handleEmailAlreadyExistsError; import static org.sonar.server.authentication.AuthenticationError.handleError; import static org.sonar.server.authentication.AuthenticationError.handleNotAllowedToSignUpError; @@ -67,6 +68,8 @@ public class OAuth2CallbackFilter extends ServletFilter { } } catch (NotAllowUserToSignUpException e) { handleNotAllowedToSignUpError(e, (HttpServletResponse) response); + } catch (EmailAlreadyExistsException e) { + handleEmailAlreadyExistsError(e, (HttpServletResponse) response); } catch (Exception e) { handleError(e, (HttpServletResponse) response, String.format("Fail to callback authentication with %s", keyProvider)); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java index 803b3fc39a8..7ef534e27b0 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserIdentityAuthenticator.java @@ -66,6 +66,12 @@ public class UserIdentityAuthenticator { if (!provider.allowsUsersToSignUp()) { throw new NotAllowUserToSignUpException(provider); } + + String email = user.getEmail(); + if (email != null && dbClient.userDao().doesEmailExist(dbSession, email)) { + throw new EmailAlreadyExistsException(email); + } + userUpdater.create(dbSession, NewUser.create() .setLogin(uuidFactory.create()) .setEmail(user.getEmail()) 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 04e3c74edce..adc9173d071 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 @@ -142,6 +142,28 @@ public class InitFilterTest { assertError("Fail to initialize authentication with provider 'unsupported'"); } + @Test + public void redirect_when_failing_because_of_NotAllowUserToSignUpException() throws Exception { + IdentityProvider identityProvider = new FailWithNotAllowUserToSignUpIdProvider("failing"); + when(request.getRequestURI()).thenReturn("/sessions/init/" + identityProvider.getKey()); + identityProviderRepository.addIdentityProvider(identityProvider); + + underTest.doFilter(request, response, chain); + + verify(response).sendRedirect("/sessions/not_allowed_to_sign_up?providerName=Failing provider"); + } + + @Test + public void redirect_when_failing_because_of_EmailAlreadyExistsException() throws Exception { + IdentityProvider identityProvider = new FailWithEmailAlreadyExistsExceptionIdProvider("failing"); + when(request.getRequestURI()).thenReturn("/sessions/init/" + identityProvider.getKey()); + identityProviderRepository.addIdentityProvider(identityProvider); + + underTest.doFilter(request, response, chain); + + verify(response).sendRedirect("/sessions/email_already_exists?email=john@email.com"); + } + private void assertOAuth2InitCalled(){ assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); assertThat(oAuth2IdentityProvider.isInitCalled()).isTrue(); @@ -157,4 +179,33 @@ public class InitFilterTest { verify(response).sendRedirect("/sessions/unauthorized"); assertThat(oAuth2IdentityProvider.isInitCalled()).isFalse(); } + + private static class FailWithNotAllowUserToSignUpIdProvider extends FakeBasicIdentityProvider { + + public FailWithNotAllowUserToSignUpIdProvider(String key) { + super(key, true); + } + + @Override + public String getName() { + return "Failing provider"; + } + + @Override + public void init(Context context) { + throw new NotAllowUserToSignUpException(this); + } + } + + private static class FailWithEmailAlreadyExistsExceptionIdProvider extends FakeBasicIdentityProvider { + + public FailWithEmailAlreadyExistsExceptionIdProvider(String key) { + super(key, true); + } + + @Override + public void init(Context context) { + throw new EmailAlreadyExistsException("john@email.com"); + } + } } 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 649ba0a35c5..ab3d706ca69 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 @@ -146,4 +146,14 @@ public class UserIdentityAuthenticatorTest { thrown.expect(NotAllowUserToSignUpException.class); underTest.authenticate(USER_IDENTITY, identityProvider, httpSession); } + + @Test + public void fail_to_authenticate_new_user_when_email_already_exists() throws Exception { + when(userDao.selectByExternalIdentity(dbSession, USER_IDENTITY.getId(), IDENTITY_PROVIDER.getKey())).thenReturn(Optional.absent()); + when(userDao.selectOrFailByExternalIdentity(dbSession, USER_IDENTITY.getId(), IDENTITY_PROVIDER.getKey())).thenReturn(ACTIVE_USER); + when(userDao.doesEmailExist(dbSession, USER_IDENTITY.getEmail())).thenReturn(true); + + thrown.expect(EmailAlreadyExistsException.class); + underTest.authenticate(USER_IDENTITY, IDENTITY_PROVIDER, httpSession); + } } diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/email_already_exists.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/email_already_exists.erb new file mode 100644 index 00000000000..ba75080a1c9 --- /dev/null +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/email_already_exists.erb @@ -0,0 +1,10 @@ + + + + +
+ +
+

Email <%= h params[:email] %> is already used by an existing user

+
+
diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/not_allowed_to_sign_up.html.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/not_allowed_to_sign_up.html.erb index dd8ddb1ce16..b2ab35dbc82 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/not_allowed_to_sign_up.html.erb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/views/sessions/not_allowed_to_sign_up.html.erb @@ -3,7 +3,7 @@
-

<%= params[:providerName] %> users are not allowed to signup

+

<%= h params[:providerName] %> users are not allowed to signup

diff --git a/sonar-db/src/main/java/org/sonar/db/user/UserDao.java b/sonar-db/src/main/java/org/sonar/db/user/UserDao.java index 8799754185a..8c01178d425 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/UserDao.java +++ b/sonar-db/src/main/java/org/sonar/db/user/UserDao.java @@ -187,6 +187,13 @@ public class UserDao implements Dao { throw new RowNotFoundException(String.format("User with identity provider '%s' and id '%s' has not been found", extIdentityProvider, extIdentity)); } + /** + * Please note that email is case insensitive, result for searching 'mail@email.com' or 'Mail@Email.com' will be the same + */ + public boolean doesEmailExist(DbSession dbSession, String email){ + return mapper(dbSession).countByEmail(email.toLowerCase()) > 0; + } + protected UserMapper mapper(DbSession session) { return session.getMapper(UserMapper.class); } diff --git a/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java b/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java index 605998588db..8f461e4f673 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/user/UserMapper.java @@ -57,6 +57,8 @@ public interface UserMapper { @CheckForNull GroupDto selectGroupByName(String name); + long countByEmail(String email); + void insert(UserDto userDto); void update(UserDto userDto); diff --git a/sonar-db/src/main/resources/org/sonar/db/user/UserMapper.xml b/sonar-db/src/main/resources/org/sonar/db/user/UserMapper.xml index a4d3b636448..066036e250c 100644 --- a/sonar-db/src/main/resources/org/sonar/db/user/UserMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/user/UserMapper.xml @@ -98,6 +98,12 @@ + +