From b1bb7f5c17b0e3ba56b6cec2995bae1acaebd5b4 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 18 Dec 2014 15:54:29 +0100 Subject: [PATCH] SONAR-5934 Rewrite create users WS, SONAR-5961 Rewrite the update user WS --- .../server/platform/ServerComponents.java | 3 +- .../sonar/server/user/DefaultUserService.java | 21 ++ .../java/org/sonar/server/user/NewUser.java | 27 +- .../server/user/ReactivationException.java | 35 --- .../org/sonar/server/user/UpdateUser.java | 126 ++++++++ .../org/sonar/server/user/UserCreator.java | 229 -------------- .../org/sonar/server/user/UserService.java | 17 +- .../org/sonar/server/user/UserUpdater.java | 294 ++++++++++++++++++ .../sonar/server/user/ws/CreateAction.java | 58 ++-- .../sonar/server/user/ws/UpdateAction.java | 123 ++++++++ .../org/sonar/server/user/ws/UsersWs.java | 29 +- .../org/sonar/server/ws/ServletRequest.java | 5 + .../server/user/DefaultUserServiceTest.java | 42 +++ .../server/user/UserServiceMediumTest.java | 43 ++- ...rCreatorTest.java => UserUpdaterTest.java} | 263 ++++++++++++---- .../server/user/ws/CreateActionTest.java | 35 ++- .../server/user/ws/UpdateActionTest.java | 194 ++++++++++++ .../org/sonar/server/user/ws/UsersWsTest.java | 8 +- .../sonar/server/ws/WebServiceEngineTest.java | 5 + .../java/org/sonar/server/ws/WsTester.java | 7 +- ..._default_groups_when_reactivating_user.xml | 0 ...ate_default_groups_when_updating_user.xml} | 4 + ...ail_to_reactivate_user_if_not_disabled.xml | 6 + .../reactivate_user.xml | 0 .../user/UserUpdaterTest/update_user.xml | 6 + .../ws/CreateActionTest/reactivate_user.json | 14 + .../update_user.json} | 0 .../resources/org/sonar/l10n/core.properties | 1 + .../java/org/sonar/api/server/ws/Request.java | 5 + .../server/ws/internal/SimpleGetRequest.java | 5 + .../org/sonar/api/server/ws/RequestTest.java | 13 + .../ws/internal/SimpleGetRequestTest.java | 10 + 32 files changed, 1191 insertions(+), 437 deletions(-) delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/user/ReactivationException.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/user/UpdateUser.java delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/user/ws/UpdateAction.java rename server/sonar-server/src/test/java/org/sonar/server/user/{UserCreatorTest.java => UserUpdaterTest.java} (64%) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java rename server/sonar-server/src/test/resources/org/sonar/server/user/{UserCreatorTest => UserUpdaterTest}/associate_default_groups_when_reactivating_user.xml (100%) rename server/sonar-server/src/test/resources/org/sonar/server/user/{UserCreatorTest/fail_to_create_user_if_already_exists_but_inactive.xml => UserUpdaterTest/associate_default_groups_when_updating_user.xml} (67%) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_reactivate_user_if_not_disabled.xml rename server/sonar-server/src/test/resources/org/sonar/server/user/{UserCreatorTest => UserUpdaterTest}/reactivate_user.xml (100%) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/update_user.xml create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/reactivate_user.json rename server/sonar-server/src/test/resources/org/sonar/server/user/ws/{CreateActionTest/return_409_when_reactive_exception.json => UpdateActionTest/update_user.json} (100%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java index 26fb763fce5..374b8174cc6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java @@ -470,13 +470,14 @@ class ServerComponents { pico.addSingleton(DefaultUserService.class); pico.addSingleton(UsersWs.class); pico.addSingleton(org.sonar.server.user.ws.CreateAction.class); + pico.addSingleton(org.sonar.server.user.ws.UpdateAction.class); pico.addSingleton(FavoritesWs.class); pico.addSingleton(UserPropertiesWs.class); pico.addSingleton(UserIndexDefinition.class); pico.addSingleton(UserIndexer.class); pico.addSingleton(UserIndex.class); pico.addSingleton(UserService.class); - pico.addSingleton(UserCreator.class); + pico.addSingleton(UserUpdater.class); // groups pico.addSingleton(GroupMembershipService.class); diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java b/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java index d30ee2bbf86..df1cf9691c7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java @@ -82,6 +82,27 @@ public class DefaultUserService implements RubyUserService { userService.index(); } + public void create(Map params) { + NewUser newUser = NewUser.create() + .setLogin((String) params.get("login")) + .setName((String) params.get("name")) + .setEmail((String) params.get("email")) + .setScmAccounts((RubyUtils.toStrings(params.get("scm_accounts")))) + .setPassword((String) params.get("password")) + .setPasswordConfirmation((String) params.get("password_confirmation")); + userService.create(newUser); + } + + public void update(Map params) { + UpdateUser updateUser = UpdateUser.create((String) params.get("login")) + .setName((String) params.get("name")) + .setEmail((String) params.get("email")) + .setScmAccounts((RubyUtils.toStrings(params.get("scm_accounts")))) + .setPassword((String) params.get("password")) + .setPasswordConfirmation((String) params.get("password_confirmation")); + userService.update(updateUser); + } + public void index() { userService.index(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java b/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java index 833ecea8461..64a96f77890 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/NewUser.java @@ -35,26 +35,26 @@ public class NewUser { private String password; private String passwordConfirmation; - private boolean preventReactivation = false; - private NewUser() { // No direct call to this constructor } - public NewUser setLogin(String login) { + public NewUser setLogin(@Nullable String login) { this.login = login; return this; } + @Nullable public String login() { return login; } + @Nullable public String name() { return name; } - public NewUser setName(String name) { + public NewUser setName(@Nullable String name) { this.name = name; return this; } @@ -69,15 +69,17 @@ public class NewUser { return this; } + @Nullable public List scmAccounts() { return scmAccounts; } - public NewUser setScmAccounts(List scmAccounts) { + public NewUser setScmAccounts(@Nullable List scmAccounts) { this.scmAccounts = scmAccounts; return this; } + @Nullable public String password() { return password; } @@ -87,27 +89,16 @@ public class NewUser { return this; } + @Nullable public String passwordConfirmation() { return passwordConfirmation; } - public NewUser setPasswordConfirmation(String passwordConfirmation) { + public NewUser setPasswordConfirmation(@Nullable String passwordConfirmation) { this.passwordConfirmation = passwordConfirmation; return this; } - public boolean isPreventReactivation() { - return preventReactivation; - } - - /** - * When true, if the user already exists in status disabled, an {@link org.sonar.server.user.ReactivationException} will be thrown - */ - public NewUser setPreventReactivation(boolean preventReactivation) { - this.preventReactivation = preventReactivation; - return this; - } - public static NewUser create() { return new NewUser(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ReactivationException.java b/server/sonar-server/src/main/java/org/sonar/server/user/ReactivationException.java deleted file mode 100644 index 92b9fdebdba..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ReactivationException.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube 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. - * - * SonarQube 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.user; - -public class ReactivationException extends RuntimeException { - - private String login; - - public ReactivationException(String message, String login) { - super(message); - this.login = login; - } - - public String login() { - return login; - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UpdateUser.java b/server/sonar-server/src/main/java/org/sonar/server/user/UpdateUser.java new file mode 100644 index 00000000000..9df8c294f84 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UpdateUser.java @@ -0,0 +1,126 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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.user; + +import com.google.common.base.Preconditions; + +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + +import java.util.List; + +public class UpdateUser { + + private String login; + private String name; + private String email; + private List scmAccounts; + + private String password; + private String passwordConfirmation; + + boolean isNameChanged, isEmailChanged, isScmAccountsChanged, isPasswordChanged; + + private UpdateUser(String login) { + // No direct call to this constructor + this.login = login; + } + + public String login() { + return login; + } + + @CheckForNull + public String name() { + return name; + } + + public UpdateUser setName(@Nullable String name) { + this.name = name; + isNameChanged = true; + return this; + } + + @CheckForNull + public String email() { + return email; + } + + public UpdateUser setEmail(@Nullable String email) { + this.email = email; + isEmailChanged = true; + return this; + } + + @CheckForNull + public List scmAccounts() { + return scmAccounts; + } + + public UpdateUser setScmAccounts(@Nullable List scmAccounts) { + this.scmAccounts = scmAccounts; + isScmAccountsChanged = true; + return this; + } + + @CheckForNull + public String password() { + return password; + } + + public UpdateUser setPassword(@Nullable String password) { + this.password = password; + isPasswordChanged = true; + return this; + } + + @CheckForNull + public String passwordConfirmation() { + return passwordConfirmation; + } + + public UpdateUser setPasswordConfirmation(@Nullable String passwordConfirmation) { + this.passwordConfirmation = passwordConfirmation; + isPasswordChanged = true; + return this; + } + + public boolean isNameChanged() { + return isNameChanged; + } + + public boolean isEmailChanged() { + return isEmailChanged; + } + + public boolean isScmAccountsChanged() { + return isScmAccountsChanged; + } + + public boolean isPasswordChanged() { + return isPasswordChanged; + } + + public static UpdateUser create(String login) { + Preconditions.checkNotNull(login); + return new UpdateUser(login); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java deleted file mode 100644 index 0b4438a6713..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserCreator.java +++ /dev/null @@ -1,229 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube 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. - * - * SonarQube 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.user; - -import com.google.common.base.Predicate; -import com.google.common.base.Strings; -import com.google.common.collect.Iterables; -import org.apache.commons.codec.digest.DigestUtils; -import org.apache.commons.lang.StringUtils; -import org.sonar.api.CoreProperties; -import org.sonar.api.config.Settings; -import org.sonar.api.platform.NewUserHandler; -import org.sonar.api.utils.System2; -import org.sonar.api.utils.text.CsvWriter; -import org.sonar.core.persistence.DbSession; -import org.sonar.core.user.GroupDto; -import org.sonar.core.user.UserDto; -import org.sonar.core.user.UserGroupDto; -import org.sonar.server.db.DbClient; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.Message; -import org.sonar.server.user.db.UserGroupDao; -import org.sonar.server.util.Validation; - -import javax.annotation.CheckForNull; -import javax.annotation.Nullable; - -import java.io.StringWriter; -import java.security.SecureRandom; -import java.util.List; -import java.util.Random; - -import static com.google.common.collect.Lists.newArrayList; - -public class UserCreator { - - private static final String LOGIN_PARAM = "Login"; - private static final String PASSWORD_CONFIRMATION_PARAM = "Password confirmation"; - private static final String PASSWORD_PARAM = "Password"; - private static final String NAME_PARAM = "Name"; - private static final String EMAIL_PARAM = "Email"; - - private final NewUserNotifier newUserNotifier; - private final Settings settings; - private final UserGroupDao userGroupDao; - private final DbClient dbClient; - private final System2 system2; - - public UserCreator(NewUserNotifier newUserNotifier, Settings settings, UserGroupDao userGroupDao, DbClient dbClient, System2 system2) { - this.newUserNotifier = newUserNotifier; - this.settings = settings; - this.userGroupDao = userGroupDao; - this.dbClient = dbClient; - this.system2 = system2; - } - - public void create(NewUser newUser) { - validate(newUser); - - DbSession dbSession = dbClient.openSession(false); - try { - String login = newUser.login(); - UserDto existingUser = dbClient.userDao().selectNullableByLogin(dbSession, login); - if (existingUser != null) { - updateExistingUser(dbSession, newUser); - } else { - createNewUser(dbSession, newUser); - } - dbSession.commit(); - notifyNewUser(newUser); - } finally { - dbSession.close(); - } - } - - private static void validate(NewUser newUser) { - List messages = newArrayList(); - - validateLogin(newUser.login(), messages); - validateName(newUser.name(), messages); - validateEmail(newUser.email(), messages); - validatePassword(newUser, messages); - - if (!messages.isEmpty()) { - throw new BadRequestException(messages); - } - } - - private static void validateLogin(@Nullable String login, List messages) { - if (Strings.isNullOrEmpty(login)) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, LOGIN_PARAM)); - } else if (!login.matches("\\A\\w[\\w\\.\\-_@\\s]+\\z")) { - messages.add(Message.of("user.bad_login")); - } else if (login.length() <= 2) { - messages.add(Message.of(Validation.IS_TOO_SHORT_MESSAGE, LOGIN_PARAM, 2)); - } else if (login.length() >= 255) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, LOGIN_PARAM, 255)); - } - } - - private static void validateName(@Nullable String name, List messages) { - if (Strings.isNullOrEmpty(name)) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, NAME_PARAM)); - } else if (name.length() >= 200) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, NAME_PARAM, 200)); - } - } - - private static void validateEmail(@Nullable String email, List messages) { - if (!Strings.isNullOrEmpty(email) && email.length() >= 100) { - messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, EMAIL_PARAM, 100)); - } - } - - private static void validatePassword(NewUser newUser, List messages) { - if (Strings.isNullOrEmpty(newUser.password())) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD_PARAM)); - } - if (Strings.isNullOrEmpty(newUser.passwordConfirmation())) { - messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, PASSWORD_CONFIRMATION_PARAM)); - } - - if (!Strings.isNullOrEmpty(newUser.password()) && !Strings.isNullOrEmpty(newUser.passwordConfirmation()) - && !StringUtils.equals(newUser.password(), newUser.passwordConfirmation())) { - messages.add(Message.of("user.password_doesnt_match_confirmation")); - } - } - - private void createNewUser(DbSession dbSession, NewUser newUser) { - long now = system2.now(); - UserDto userDto = new UserDto() - .setLogin(newUser.login()) - .setName(newUser.name()) - .setEmail(newUser.email()) - .setActive(true) - .setScmAccounts(convertScmAccountsToCsv(newUser)) - .setCreatedAt(now) - .setUpdatedAt(now); - setEncryptedPassWord(newUser, userDto); - dbClient.userDao().insert(dbSession, userDto); - addDefaultGroup(dbSession, userDto); - } - - private void updateExistingUser(DbSession dbSession, NewUser newUser) { - String login = newUser.login(); - UserDto existingUser = dbClient.userDao().selectNullableByLogin(dbSession, login); - if (existingUser != null) { - if (!existingUser.isActive()) { - if (newUser.isPreventReactivation()) { - throw new ReactivationException(String.format("A disabled user with the login '%s' already exists", login), login); - } else { - existingUser.setActive(true); - existingUser.setUpdatedAt(system2.now()); - dbClient.userDao().update(dbSession, existingUser); - addDefaultGroup(dbSession, existingUser); - } - } else { - throw new IllegalArgumentException(String.format("A user with the login '%s' already exists", login)); - } - } - } - - private static void setEncryptedPassWord(NewUser newUser, UserDto userDto) { - Random random = new SecureRandom(); - byte[] salt = new byte[32]; - random.nextBytes(salt); - String saltHex = DigestUtils.sha1Hex(salt); - userDto.setSalt(saltHex); - userDto.setCryptedPassword(DigestUtils.sha1Hex("--" + saltHex + "--" + newUser.password() + "--")); - } - - @CheckForNull - private static String convertScmAccountsToCsv(NewUser newUser) { - List scmAccounts = newUser.scmAccounts(); - if (scmAccounts != null) { - int size = newUser.scmAccounts().size(); - StringWriter writer = new StringWriter(size); - CsvWriter csv = CsvWriter.of(writer); - csv.values(newUser.scmAccounts().toArray(new String[size])); - csv.close(); - return writer.toString(); - } - return null; - } - - private void notifyNewUser(NewUser newUser) { - newUserNotifier.onNewUser(NewUserHandler.Context.builder() - .setLogin(newUser.login()) - .setName(newUser.name()) - .setEmail(newUser.email()) - .build()); - } - - private void addDefaultGroup(DbSession dbSession, UserDto userDto) { - final String defaultGroup = settings.getString(CoreProperties.CORE_DEFAULT_GROUP); - if (defaultGroup == null) { - throw new IllegalStateException(String.format("The default group property '%s' is null", CoreProperties.CORE_DEFAULT_GROUP)); - } - List userGroups = dbClient.groupDao().findByUserLogin(dbSession, userDto.getLogin()); - if (!Iterables.any(userGroups, new Predicate() { - @Override - public boolean apply(@Nullable GroupDto input) { - return input != null && input.getKey().equals(defaultGroup); - } - })) { - GroupDto groupDto = dbClient.groupDao().getByKey(dbSession, defaultGroup); - userGroupDao.insert(dbSession, new UserGroupDto().setUserId(userDto.getId()).setGroupId(groupDto.getId())); - } - } - -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserService.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserService.java index 9f6945b7a10..1adfc5bb511 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/UserService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserService.java @@ -32,17 +32,24 @@ public class UserService implements ServerComponent { private final UserIndexer userIndexer; private final UserIndex userIndex; - private final UserCreator userCreator; + private final UserUpdater userUpdater; - public UserService(UserIndexer userIndexer, UserIndex userIndex, UserCreator userCreator) { + public UserService(UserIndexer userIndexer, UserIndex userIndex, UserUpdater userUpdater) { this.userIndexer = userIndexer; this.userIndex = userIndex; - this.userCreator = userCreator; + this.userUpdater = userUpdater; } - public void create(NewUser newUser) { + public boolean create(NewUser newUser) { checkPermission(); - userCreator.create(newUser); + boolean result = userUpdater.create(newUser); + userIndexer.index(); + return result; + } + + public void update(UpdateUser updateUser) { + checkPermission(); + userUpdater.update(updateUser); userIndexer.index(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java new file mode 100644 index 00000000000..d657020787e --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserUpdater.java @@ -0,0 +1,294 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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.user; + +import com.google.common.base.Predicate; +import com.google.common.base.Strings; +import com.google.common.collect.Iterables; +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.commons.lang.StringUtils; +import org.sonar.api.CoreProperties; +import org.sonar.api.ServerComponent; +import org.sonar.api.config.Settings; +import org.sonar.api.platform.NewUserHandler; +import org.sonar.api.utils.System2; +import org.sonar.api.utils.text.CsvWriter; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.user.GroupDto; +import org.sonar.core.user.UserDto; +import org.sonar.core.user.UserGroupDto; +import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.Message; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.user.db.UserGroupDao; +import org.sonar.server.util.Validation; + +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + +import java.io.StringWriter; +import java.security.SecureRandom; +import java.util.List; +import java.util.Random; + +import static com.google.common.collect.Lists.newArrayList; + +public class UserUpdater implements ServerComponent { + + private static final String LOGIN_PARAM = "Login"; + private static final String PASSWORD_CONFIRMATION_PARAM = "Password confirmation"; + private static final String PASSWORD_PARAM = "Password"; + private static final String NAME_PARAM = "Name"; + private static final String EMAIL_PARAM = "Email"; + + private final NewUserNotifier newUserNotifier; + private final Settings settings; + private final UserGroupDao userGroupDao; + private final DbClient dbClient; + private final System2 system2; + + public UserUpdater(NewUserNotifier newUserNotifier, Settings settings, UserGroupDao userGroupDao, DbClient dbClient, System2 system2) { + this.newUserNotifier = newUserNotifier; + this.settings = settings; + this.userGroupDao = userGroupDao; + this.dbClient = dbClient; + this.system2 = system2; + } + + /** + * Retuen true if the user has been reactivated + * @return + */ + public boolean create(NewUser newUser) { + UserDto userDto = createNewUserDto(newUser); + boolean isUserReactivated = false; + + DbSession dbSession = dbClient.openSession(false); + try { + String login = userDto.getLogin(); + UserDto existingUser = dbClient.userDao().selectNullableByLogin(dbSession, login); + if (existingUser == null) { + saveUser(dbSession, userDto); + } else { + if (existingUser.isActive()) { + throw new IllegalArgumentException(String.format("An active user with login '%s' already exists", login)); + } + UpdateUser updateUser = UpdateUser.create(login) + .setName(newUser.name()) + .setEmail(newUser.email()) + .setScmAccounts(newUser.scmAccounts()) + .setPassword(newUser.password()) + .setPasswordConfirmation(newUser.passwordConfirmation()); + updateUserDto(updateUser, existingUser); + updateUser(dbSession, existingUser); + isUserReactivated = true; + } + dbSession.commit(); + notifyNewUser(userDto.getLogin(), userDto.getName(), newUser.email()); + } finally { + dbSession.close(); + } + return isUserReactivated; + } + + public void update(UpdateUser updateUser) { + DbSession dbSession = dbClient.openSession(false); + try { + UserDto user = dbClient.userDao().selectNullableByLogin(dbSession, updateUser.login()); + if (user != null) { + updateUserDto(updateUser, user); + updateUser(dbSession, user); + } else { + throw new NotFoundException(String.format("User '%s' does not exists", updateUser.login())); + } + dbSession.commit(); + notifyNewUser(user.getLogin(), user.getName(), user.getEmail()); + } finally { + dbSession.close(); + } + } + + private static UserDto createNewUserDto(NewUser newUser) { + UserDto userDto = new UserDto(); + List messages = newArrayList(); + + String login = newUser.login(); + validateLoginFormat(login, messages); + userDto.setLogin(login); + + String name = newUser.name(); + validateNameFormat(name, messages); + userDto.setName(name); + + String email = newUser.email(); + if (email != null) { + validateEmailFormat(email, messages); + userDto.setEmail(email); + } + + String password = newUser.password(); + String passwordConfirmation = newUser.passwordConfirmation(); + validatePasswords(password, passwordConfirmation, messages); + setEncryptedPassWord(password, userDto); + + userDto.setScmAccounts(convertScmAccountsToCsv(newUser.scmAccounts())); + + if (!messages.isEmpty()) { + throw new BadRequestException(messages); + } + return userDto; + } + + private static void updateUserDto(UpdateUser updateUser, UserDto userDto) { + List messages = newArrayList(); + + String name = updateUser.name(); + if (updateUser.isNameChanged()) { + validateNameFormat(name, messages); + userDto.setName(name); + } + + String email = updateUser.email(); + if (updateUser.isEmailChanged()) { + validateEmailFormat(email, messages); + userDto.setEmail(email); + } + + String password = updateUser.password(); + String passwordConfirmation = updateUser.passwordConfirmation(); + if (updateUser.isPasswordChanged()) { + validatePasswords(password, passwordConfirmation, messages); + setEncryptedPassWord(password, userDto); + } + + if (updateUser.isScmAccountsChanged()) { + userDto.setScmAccounts(convertScmAccountsToCsv(updateUser.scmAccounts())); + } + + if (!messages.isEmpty()) { + throw new BadRequestException(messages); + } + } + + private static void checkNotEmptyParam(@Nullable String value, String param, List messages) { + if (Strings.isNullOrEmpty(value)) { + messages.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, param)); + } + } + + private static void validateLoginFormat(@Nullable String login, List messages) { + checkNotEmptyParam(login, LOGIN_PARAM, messages); + if (!Strings.isNullOrEmpty(login)) { + if (!login.matches("\\A\\w[\\w\\.\\-_@\\s]+\\z")) { + messages.add(Message.of("user.bad_login")); + } else if (login.length() <= 2) { + messages.add(Message.of(Validation.IS_TOO_SHORT_MESSAGE, LOGIN_PARAM, 2)); + } else if (login.length() >= 255) { + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, LOGIN_PARAM, 255)); + } + } + } + + private static void validateNameFormat(@Nullable String name, List messages) { + checkNotEmptyParam(name, NAME_PARAM, messages); + if (name != null && name.length() >= 200) { + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, NAME_PARAM, 200)); + } + } + + private static void validateEmailFormat(@Nullable String email, List messages) { + if (email != null) { + if (email.length() >= 100) { + messages.add(Message.of(Validation.IS_TOO_LONG_MESSAGE, EMAIL_PARAM, 100)); + } + } + } + + private static void validatePasswords(@Nullable String password, @Nullable String passwordConfirmation, List messages) { + checkNotEmptyParam(password, PASSWORD_PARAM, messages); + checkNotEmptyParam(passwordConfirmation, PASSWORD_CONFIRMATION_PARAM, messages); + if (!StringUtils.equals(password, passwordConfirmation)) { + messages.add(Message.of("user.password_doesnt_match_confirmation")); + } + } + + private void saveUser(DbSession dbSession, UserDto userDto) { + long now = system2.now(); + userDto.setActive(true).setCreatedAt(now).setUpdatedAt(now); + dbClient.userDao().insert(dbSession, userDto); + addDefaultGroup(dbSession, userDto); + } + + private void updateUser(DbSession dbSession, UserDto userDto) { + long now = system2.now(); + userDto.setActive(true).setUpdatedAt(now); + dbClient.userDao().update(dbSession, userDto); + addDefaultGroup(dbSession, userDto); + } + + private static void setEncryptedPassWord(String password, UserDto userDto) { + Random random = new SecureRandom(); + byte[] salt = new byte[32]; + random.nextBytes(salt); + String saltHex = DigestUtils.sha1Hex(salt); + userDto.setSalt(saltHex); + userDto.setCryptedPassword(DigestUtils.sha1Hex("--" + saltHex + "--" + password + "--")); + } + + @CheckForNull + private static String convertScmAccountsToCsv(@Nullable List scmAccounts) { + if (scmAccounts != null) { + int size = scmAccounts.size(); + StringWriter writer = new StringWriter(size); + CsvWriter csv = CsvWriter.of(writer); + csv.values(scmAccounts.toArray(new String[size])); + csv.close(); + return writer.toString(); + } + return null; + } + + private void notifyNewUser(String login, String name, String email) { + newUserNotifier.onNewUser(NewUserHandler.Context.builder() + .setLogin(login) + .setName(name) + .setEmail(email) + .build()); + } + + private void addDefaultGroup(DbSession dbSession, UserDto userDto) { + final String defaultGroup = settings.getString(CoreProperties.CORE_DEFAULT_GROUP); + if (defaultGroup == null) { + throw new IllegalStateException(String.format("The default group property '%s' is null", CoreProperties.CORE_DEFAULT_GROUP)); + } + List userGroups = dbClient.groupDao().findByUserLogin(dbSession, userDto.getLogin()); + if (!Iterables.any(userGroups, new Predicate() { + @Override + public boolean apply(@Nullable GroupDto input) { + return input != null && input.getKey().equals(defaultGroup); + } + })) { + GroupDto groupDto = dbClient.groupDao().getByKey(dbSession, defaultGroup); + userGroupDao.insert(dbSession, new UserGroupDto().setUserId(userDto.getId()).setGroupId(groupDto.getId())); + } + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java index cb5ea34f576..3977830d023 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/CreateAction.java @@ -20,19 +20,17 @@ package org.sonar.server.user.ws; +import org.sonar.api.i18n.I18n; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.text.JsonWriter; -import org.sonar.server.plugins.MimeTypes; import org.sonar.server.user.NewUser; -import org.sonar.server.user.ReactivationException; import org.sonar.server.user.UserService; +import org.sonar.server.user.UserSession; import org.sonar.server.user.index.UserDoc; -import java.io.OutputStreamWriter; - public class CreateAction implements RequestHandler { private static final String PARAM_LOGIN = "login"; @@ -41,12 +39,13 @@ public class CreateAction implements RequestHandler { private static final String PARAM_NAME = "name"; private static final String PARAM_EMAIL = "email"; private static final String PARAM_SCM_ACCOUNTS = "scm_accounts"; - private static final String PARAM_PREVENT_REACTIVATION = "prevent_reactivation"; private final UserService service; + private final I18n i18n; - public CreateAction(UserService service) { + public CreateAction(UserService service, I18n i18n) { this.service = service; + this.i18n = i18n; } void define(WebService.NewController controller) { @@ -81,13 +80,8 @@ public class CreateAction implements RequestHandler { .setExampleValue("myname@email.com"); action.createParam(PARAM_SCM_ACCOUNTS) - .setDescription("SCM accounts") + .setDescription("SCM accounts. This parameter has been added in 5.1") .setExampleValue("myscmaccount1, myscmaccount2"); - - action.createParam(PARAM_PREVENT_REACTIVATION) - .setDescription("If set to true and if the user has been removed, a status 409 will be returned") - .setDefaultValue(false) - .setBooleanPossibleValues(); } @Override @@ -99,37 +93,24 @@ public class CreateAction implements RequestHandler { .setEmail(request.param(PARAM_EMAIL)) .setScmAccounts(request.paramAsStrings(PARAM_SCM_ACCOUNTS)) .setPassword(request.mandatoryParam(PARAM_PASSWORD)) - .setPasswordConfirmation(request.mandatoryParam(PARAM_PASSWORD_CONFIRMATION)) - .setPreventReactivation(request.mandatoryParamAsBoolean(PARAM_PREVENT_REACTIVATION)); - - try { - service.create(newUser); - writeResponse(response, login); - } catch (ReactivationException e) { - write409(response, login); - } - } + .setPasswordConfirmation(request.mandatoryParam(PARAM_PASSWORD_CONFIRMATION)); - private void writeResponse(Response response, String login) { - UserDoc user = service.getByLogin(login); - JsonWriter json = response.newJsonWriter().beginObject().name("user"); - writeUser(json, user); - json.endObject().close(); + boolean isUserReactivated = service.create(newUser); + writeResponse(response, login, isUserReactivated); } - private void write409(Response response, String login) { + private void writeResponse(Response response, String login, boolean isUserReactivated) { UserDoc user = service.getByLogin(login); - - Response.Stream stream = response.stream(); - stream.setStatus(409); - stream.setMediaType(MimeTypes.JSON); - JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output())).beginObject().name("user"); + JsonWriter json = response.newJsonWriter().beginObject(); writeUser(json, user); + if (isUserReactivated) { + writeReactivationMessage(json, login); + } json.endObject().close(); } private void writeUser(JsonWriter json, UserDoc user) { - json.beginObject() + json.name("user").beginObject() .prop("login", user.login()) .prop("name", user.name()) .prop("email", user.email()) @@ -137,4 +118,13 @@ public class CreateAction implements RequestHandler { .name("scmAccounts").beginArray().values(user.scmAccounts()).endArray() .endObject(); } + + private void writeReactivationMessage(JsonWriter json, String login) { + json.name("infos").beginArray(); + json.beginObject(); + String text = i18n.message(UserSession.get().locale(), "user.reactivated", "user.reactivated", login); + json.prop("msg", text); + json.endObject(); + json.endArray(); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UpdateAction.java new file mode 100644 index 00000000000..4cb0526dcae --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UpdateAction.java @@ -0,0 +1,123 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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.user.ws; + +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.RequestHandler; +import org.sonar.api.server.ws.Response; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.utils.text.JsonWriter; +import org.sonar.server.user.UpdateUser; +import org.sonar.server.user.UserService; +import org.sonar.server.user.index.UserDoc; + +public class UpdateAction implements RequestHandler { + + private static final String PARAM_LOGIN = "login"; + private static final String PARAM_PASSWORD = "password"; + private static final String PARAM_PASSWORD_CONFIRMATION = "password_confirmation"; + private static final String PARAM_NAME = "name"; + private static final String PARAM_EMAIL = "email"; + private static final String PARAM_SCM_ACCOUNTS = "scm_accounts"; + + private final UserService service; + + public UpdateAction(UserService service) { + this.service = service; + } + + void define(WebService.NewController controller) { + WebService.NewAction action = controller.createAction("update") + .setDescription("Update a user. Requires Administer System permission") + .setSince("3.7") + .setPost(true) + .setHandler(this); + + action.createParam(PARAM_LOGIN) + .setDescription("User login") + .setRequired(true) + .setExampleValue("myuser"); + + action.createParam(PARAM_PASSWORD) + .setDescription("User password") + .setRequired(true) + .setExampleValue("mypassword"); + + action.createParam(PARAM_PASSWORD_CONFIRMATION) + .setDescription("Must be the same value as \"password\"") + .setRequired(true) + .setExampleValue("mypassword"); + + action.createParam(PARAM_NAME) + .setDescription("User name") + .setRequired(true) + .setExampleValue("My Name"); + + action.createParam(PARAM_EMAIL) + .setDescription("User email") + .setExampleValue("myname@email.com"); + + action.createParam(PARAM_SCM_ACCOUNTS) + .setDescription("SCM accounts. This parameter has been added in 5.1") + .setExampleValue("myscmaccount1, myscmaccount2"); + } + + @Override + public void handle(Request request, Response response) throws Exception { + String login = request.mandatoryParam(PARAM_LOGIN); + UpdateUser updateUser = UpdateUser.create(login); + if (request.hasParam(PARAM_NAME)) { + updateUser.setName(request.mandatoryParam(PARAM_NAME)); + } + if (request.hasParam(PARAM_EMAIL)) { + updateUser.setEmail(request.param(PARAM_EMAIL)); + } + if (request.hasParam(PARAM_SCM_ACCOUNTS)) { + updateUser.setScmAccounts(request.paramAsStrings(PARAM_SCM_ACCOUNTS)); + } + if (request.hasParam(PARAM_PASSWORD)) { + updateUser.setPassword(request.mandatoryParam(PARAM_PASSWORD)); + } + if (request.hasParam(PARAM_PASSWORD_CONFIRMATION)) { + updateUser.setPasswordConfirmation(request.mandatoryParam(PARAM_PASSWORD_CONFIRMATION)); + } + + service.update(updateUser); + writeResponse(response, login); + } + + private void writeResponse(Response response, String login) { + UserDoc user = service.getByLogin(login); + JsonWriter json = response.newJsonWriter().beginObject(); + writeUser(json, user); + json.endObject().close(); + } + + private void writeUser(JsonWriter json, UserDoc user) { + json.name("user").beginObject() + .prop("login", user.login()) + .prop("name", user.name()) + .prop("email", user.email()) + .prop("active", user.active()) + .name("scmAccounts").beginArray().values(user.scmAccounts()).endArray() + .endObject(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java index a5b7d1abdf2..4a44637627c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UsersWs.java @@ -27,9 +27,11 @@ import org.sonar.api.server.ws.WebService; public class UsersWs implements WebService { private final CreateAction createAction; + private final UpdateAction updateAction; - public UsersWs(CreateAction createAction) { + public UsersWs(CreateAction createAction, UpdateAction updateAction) { this.createAction = createAction; + this.updateAction = updateAction; } @Override @@ -40,7 +42,7 @@ public class UsersWs implements WebService { defineSearchAction(controller); createAction.define(controller); - defineUpdateAction(controller); + updateAction.define(controller); defineDeactivateAction(controller); controller.done(); @@ -65,29 +67,6 @@ public class UsersWs implements WebService { RailsHandler.addFormatParam(action); } - private void defineUpdateAction(NewController controller) { - NewAction action = controller.createAction("update") - .setDescription("Update a user. Requires Administer System permission") - .setSince("3.7") - .setPost(true) - .setHandler(RailsHandler.INSTANCE); - - action.createParam("login") - .setDescription("User login") - .setRequired(true) - .setExampleValue("myuser"); - - action.createParam("name") - .setDescription("User name") - .setExampleValue("My New Name"); - - action.createParam("email") - .setDescription("User email") - .setExampleValue("mynewname@email.com"); - - RailsHandler.addFormatParam(action); - } - private void defineDeactivateAction(NewController controller) { NewAction action = controller.createAction("deactivate") .setDescription("Deactivate a user. Requires Administer System permission") diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java index ca1e4e66224..6a863ee5d40 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java @@ -42,6 +42,11 @@ public class ServletRequest extends ValidatingRequest { return source.getMethod(); } + @Override + public boolean hasParam(String key) { + return source.getParameterMap().containsKey(key) || params.keySet().contains(key); + } + @Override protected String readParam(String key) { String value = source.getParameter(key); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java index 17b3107387f..4d87662e4e9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java @@ -24,7 +24,10 @@ import com.google.common.collect.Maps; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; +import org.mockito.runners.MockitoJUnitRunner; import org.sonar.api.user.UserFinder; import org.sonar.api.user.UserQuery; import org.sonar.core.permission.GlobalPermissions; @@ -32,11 +35,16 @@ import org.sonar.core.user.UserDao; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.*; +@RunWith(MockitoJUnitRunner.class) public class DefaultUserServiceTest { UserService userService = mock(UserService.class); @@ -122,6 +130,40 @@ public class DefaultUserServiceTest { } } + @Test + public void create() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("name", "John"); + params.put("email", "john@email.com"); + params.put("scm_accounts", newArrayList("jn")); + service.create(params); + + ArgumentCaptor newUserCaptor = ArgumentCaptor.forClass(NewUser.class); + verify(userService).create(newUserCaptor.capture()); + assertThat(newUserCaptor.getValue().login()).isEqualTo("john"); + assertThat(newUserCaptor.getValue().name()).isEqualTo("John"); + assertThat(newUserCaptor.getValue().email()).isEqualTo("john@email.com"); + assertThat(newUserCaptor.getValue().scmAccounts()).containsOnly("jn"); + } + + @Test + public void update() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("name", "John"); + params.put("email", "john@email.com"); + params.put("scm_accounts", newArrayList("jn")); + service.update(params); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); + verify(userService).update(userCaptor.capture()); + assertThat(userCaptor.getValue().login()).isEqualTo("john"); + assertThat(userCaptor.getValue().name()).isEqualTo("John"); + assertThat(userCaptor.getValue().email()).isEqualTo("john@email.com"); + assertThat(userCaptor.getValue().scmAccounts()).containsOnly("jn"); + } + @Test public void index() throws Exception { service.index(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserServiceMediumTest.java index 164a45f8439..5072bdee0f0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/UserServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserServiceMediumTest.java @@ -33,7 +33,6 @@ import org.sonar.server.db.DbClient; import org.sonar.server.es.EsClient; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.tester.ServerTester; -import org.sonar.server.user.db.UserDao; import org.sonar.server.user.index.UserIndexDefinition; import static com.google.common.collect.Lists.newArrayList; @@ -71,7 +70,7 @@ public class UserServiceMediumTest { dbClient.groupDao().insert(session, userGroup); session.commit(); - service.create(NewUser.create() + boolean result = service.create(NewUser.create() .setLogin("user") .setName("User") .setEmail("user@mail.com") @@ -79,7 +78,8 @@ public class UserServiceMediumTest { .setPasswordConfirmation("password") .setScmAccounts(newArrayList("u1", "u_1"))); - assertThat(tester.get(UserDao.class).selectNullableByLogin(session, "user")).isNotNull(); + assertThat(result).isFalse(); + assertThat(dbClient.userDao().selectNullableByLogin(session, "user")).isNotNull(); assertThat(esClient.prepareGet(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, "user").get().isExists()).isTrue(); } @@ -96,6 +96,43 @@ public class UserServiceMediumTest { .setScmAccounts(newArrayList("u1", "u_1"))); } + @Test + public void update_user() throws Exception { + MockUserSession.set().setLogin("john").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); + + dbClient.userDao().insert(session, new UserDto() + .setLogin("marius") + .setName("Marius") + .setEmail("marius@mail.com") + .setCryptedPassword("1234") + .setSalt("abcd") + .setCreatedAt(1000L) + ); + GroupDto userGroup = new GroupDto().setName(CoreProperties.CORE_DEFAULT_GROUP_DEFAULT_VALUE); + dbClient.groupDao().insert(session, userGroup); + session.commit(); + + service.update(UpdateUser.create("marius") + .setName("Marius2") + .setEmail("marius2@mail.com")); + + UserDto userDto = dbClient.userDao().selectNullableByLogin(session, "marius"); + assertThat(userDto.getName()).isEqualTo("Marius2"); + assertThat(userDto.getEmail()).isEqualTo("marius2@mail.com"); + } + + @Test(expected = ForbiddenException.class) + public void fail_to_update_user_without_sys_admin_permission() throws Exception { + MockUserSession.set().setLogin("john").setGlobalPermissions(GlobalPermissions.DASHBOARD_SHARING); + + service.update(UpdateUser.create("marius") + .setName("Marius2") + .setEmail("marius2@mail.com") + .setPassword("password2") + .setPasswordConfirmation("password2") + .setScmAccounts(newArrayList("ma2"))); + } + @Test public void get_nullable_by_login() throws Exception { MockUserSession.set().setLogin("john").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java similarity index 64% rename from server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java rename to server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java index e76ca213b37..582eaa1ca37 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/UserCreatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java @@ -44,6 +44,7 @@ import org.sonar.core.user.UserDto; import org.sonar.server.db.DbClient; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.Message; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.db.GroupDao; import org.sonar.server.user.db.UserDao; import org.sonar.server.user.db.UserGroupDao; @@ -58,7 +59,7 @@ import static org.mockito.Mockito.when; @Category(DbTests.class) @RunWith(MockitoJUnitRunner.class) -public class UserCreatorTest { +public class UserUpdaterTest { @Rule public DbTester db = new DbTester(); @@ -78,7 +79,7 @@ public class UserCreatorTest { GroupMembershipFinder groupMembershipFinder; DbSession session; - UserCreator userCreator; + UserUpdater userUpdater; @Before public void setUp() throws Exception { @@ -90,7 +91,7 @@ public class UserCreatorTest { GroupMembershipDao groupMembershipDao = new GroupMembershipDao(db.myBatis()); groupMembershipFinder = new GroupMembershipFinder(userDao, groupMembershipDao); - userCreator = new UserCreator(newUserNotifier, settings, userGroupDao, new DbClient(db.database(), db.myBatis(), userDao, groupDao), system2); + userUpdater = new UserUpdater(newUserNotifier, settings, userGroupDao, new DbClient(db.database(), db.myBatis(), userDao, groupDao), system2); } @After @@ -103,7 +104,7 @@ public class UserCreatorTest { when(system2.now()).thenReturn(1418215735482L); createDefaultGroup(); - userCreator.create(NewUser.create() + boolean result = userUpdater.create(NewUser.create() .setLogin("user") .setName("User") .setEmail("user@mail.com") @@ -123,6 +124,7 @@ public class UserCreatorTest { assertThat(dto.getCryptedPassword()).isNotNull(); assertThat(dto.getCreatedAt()).isEqualTo(1418215735482L); assertThat(dto.getUpdatedAt()).isEqualTo(1418215735482L); + assertThat(result).isFalse(); } @Test @@ -130,7 +132,7 @@ public class UserCreatorTest { when(system2.now()).thenReturn(1418215735482L); createDefaultGroup(); - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("user") .setName("User") .setPassword("password") @@ -139,45 +141,10 @@ public class UserCreatorTest { assertThat(userDao.selectNullableByLogin(session, "user")).isNotNull(); } - @Test - public void fail_to_create_user_if_login_already_exists() throws Exception { - db.prepareDbUnit(getClass(), "fail_to_create_user_if_already_exists.xml"); - - try { - userCreator.create(NewUser.create() - .setLogin("marius") - .setName("Marius") - .setEmail("marius@mail.com") - .setPassword("password") - .setPasswordConfirmation("password")); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("A user with the login 'marius' already exists"); - } - } - - @Test - public void fail_to_create_user_if_login_already_exists_but_inactive() throws Exception { - db.prepareDbUnit(getClass(), "fail_to_create_user_if_already_exists_but_inactive.xml"); - - try { - userCreator.create(NewUser.create() - .setLogin("marius") - .setName("Marius") - .setEmail("marius@mail.com") - .setPassword("password") - .setPasswordConfirmation("password") - .setPreventReactivation(true)); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(ReactivationException.class).hasMessage("A disabled user with the login 'marius' already exists"); - } - } - @Test public void fail_to_create_user_with_missing_login() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin(null) .setName("Marius") .setEmail("marius@mail.com") @@ -192,7 +159,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_invalid_login() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("/marius/") .setName("Marius") .setEmail("marius@mail.com") @@ -207,7 +174,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_too_short_login() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("ma") .setName("Marius") .setEmail("marius@mail.com") @@ -222,7 +189,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_too_long_login() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin(Strings.repeat("m", 256)) .setName("Marius") .setEmail("marius@mail.com") @@ -237,7 +204,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_missing_name() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName(null) .setEmail("marius@mail.com") @@ -252,7 +219,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_too_long_name() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName(Strings.repeat("m", 201)) .setEmail("marius@mail.com") @@ -267,7 +234,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_too_long_email() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName("Marius") .setEmail(Strings.repeat("m", 101)) @@ -282,7 +249,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_missing_password() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName("Marius") .setEmail("marius@mail.com") @@ -290,14 +257,14 @@ public class UserCreatorTest { .setPasswordConfirmation("password")); fail(); } catch (BadRequestException e) { - assertThat(e.errors().messages()).containsOnly(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Password")); + assertThat(e.errors().messages()).contains(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Password")); } } @Test public void fail_to_create_user_with_missing_password_confirmation() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName("Marius") .setEmail("marius@mail.com") @@ -305,14 +272,14 @@ public class UserCreatorTest { .setPasswordConfirmation(null)); fail(); } catch (BadRequestException e) { - assertThat(e.errors().messages()).containsOnly(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Password confirmation")); + assertThat(e.errors().messages()).contains(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Password confirmation")); } } @Test public void fail_to_create_user_with_password_not_matching_password_confirmation() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") .setName("Marius") .setEmail("marius@mail.com") @@ -327,7 +294,7 @@ public class UserCreatorTest { @Test public void fail_to_create_user_with_many_errors() throws Exception { try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("") .setName("") .setEmail("marius@mail.com") @@ -343,7 +310,7 @@ public class UserCreatorTest { public void notify_new_user() throws Exception { createDefaultGroup(); - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("user") .setName("User") .setEmail("user@mail.com") @@ -361,7 +328,7 @@ public class UserCreatorTest { public void associate_default_groups_when_creating_user() throws Exception { createDefaultGroup(); - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("user") .setName("User") .setEmail("user@mail.com") @@ -380,7 +347,7 @@ public class UserCreatorTest { settings.setProperty(CoreProperties.CORE_DEFAULT_GROUP, (String) null); try { - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("user") .setName("User") .setEmail("user@mail.com") @@ -393,29 +360,49 @@ public class UserCreatorTest { } @Test - public void reactivate_user() throws Exception { + public void reactivate_user_when_creating_user_with_existing_login() throws Exception { db.prepareDbUnit(getClass(), "reactivate_user.xml"); when(system2.now()).thenReturn(1418215735486L); createDefaultGroup(); - userCreator.create(NewUser.create() + boolean result = userUpdater.create(NewUser.create() .setLogin("marius") .setName("Marius2") .setEmail("marius2@mail.com") .setPassword("password2") - .setPasswordConfirmation("password2") - .setPreventReactivation(false)); + .setPasswordConfirmation("password2")); + session.commit(); UserDto dto = userDao.selectNullableByLogin(session, "marius"); assertThat(dto.isActive()).isTrue(); - assertThat(dto.getName()).isEqualTo("Marius"); - assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); - assertThat(dto.getScmAccounts()).contains("ma,marius33"); + assertThat(dto.getName()).isEqualTo("Marius2"); + assertThat(dto.getEmail()).isEqualTo("marius2@mail.com"); + assertThat(dto.getScmAccounts()).isNull(); - assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); - assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + assertThat(dto.getSalt()).isNotEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isNotEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); assertThat(dto.getCreatedAt()).isEqualTo(1418215735482L); assertThat(dto.getUpdatedAt()).isEqualTo(1418215735486L); + + assertThat(result).isTrue(); + } + + @Test + public void fail_to_reactivate_user_if_not_disabled() throws Exception { + db.prepareDbUnit(getClass(), "fail_to_reactivate_user_if_not_disabled.xml"); + createDefaultGroup(); + + try { + userUpdater.create(NewUser.create() + .setLogin("marius") + .setName("Marius2") + .setEmail("marius2@mail.com") + .setPassword("password2") + .setPasswordConfirmation("password2")); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("An active user with login 'marius' already exists"); + } } @Test @@ -423,13 +410,140 @@ public class UserCreatorTest { db.prepareDbUnit(getClass(), "associate_default_groups_when_reactivating_user.xml"); createDefaultGroup(); - userCreator.create(NewUser.create() + userUpdater.create(NewUser.create() .setLogin("marius") + .setName("Marius2") + .setEmail("marius2@mail.com") + .setPassword("password2") + .setPasswordConfirmation("password2")); + session.commit(); + + GroupMembershipFinder.Membership membership = groupMembershipFinder.find(GroupMembershipQuery.builder().login("marius").groupSearch("sonar-users").build()); + assertThat(membership.groups()).hasSize(1); + assertThat(membership.groups().get(0).name()).isEqualTo("sonar-users"); + assertThat(membership.groups().get(0).isMember()).isTrue(); + } + + @Test + public void update_user() throws Exception { + db.prepareDbUnit(getClass(), "update_user.xml"); + when(system2.now()).thenReturn(1418215735486L); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") + .setName("Marius2") + .setEmail("marius2@mail.com") + .setPassword("password2") + .setPasswordConfirmation("password2") + .setScmAccounts(newArrayList("ma2"))); + session.commit(); + session.clearCache(); + + UserDto dto = userDao.selectNullableByLogin(session, "marius"); + assertThat(dto.isActive()).isTrue(); + assertThat(dto.getName()).isEqualTo("Marius2"); + assertThat(dto.getEmail()).isEqualTo("marius2@mail.com"); + assertThat(dto.getScmAccounts()).contains("ma2"); + + assertThat(dto.getSalt()).isNotEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isNotEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + assertThat(dto.getCreatedAt()).isEqualTo(1418215735482L); + assertThat(dto.getUpdatedAt()).isEqualTo(1418215735486L); + } + + @Test + public void update_only_user_name() throws Exception { + db.prepareDbUnit(getClass(), "update_user.xml"); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") + .setName("Marius2")); + session.commit(); + session.clearCache(); + + UserDto dto = userDao.selectNullableByLogin(session, "marius"); + assertThat(dto.getName()).isEqualTo("Marius2"); + + // Following fields has not changed + assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); + assertThat(dto.getScmAccounts()).contains("ma,marius33"); + assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + } + + @Test + public void update_only_user_email() throws Exception { + db.prepareDbUnit(getClass(), "update_user.xml"); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") + .setEmail("marius2@mail.com")); + session.commit(); + session.clearCache(); + + UserDto dto = userDao.selectNullableByLogin(session, "marius"); + assertThat(dto.getEmail()).isEqualTo("marius2@mail.com"); + + // Following fields has not changed + assertThat(dto.getName()).isEqualTo("Marius"); + assertThat(dto.getScmAccounts()).contains("ma,marius33"); + assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + } + + @Test + public void update_only_scm_accounts_email() throws Exception { + db.prepareDbUnit(getClass(), "update_user.xml"); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") + .setScmAccounts(newArrayList("ma2"))); + session.commit(); + session.clearCache(); + + UserDto dto = userDao.selectNullableByLogin(session, "marius"); + assertThat(dto.getScmAccounts()).contains("ma2"); + + // Following fields has not changed + assertThat(dto.getName()).isEqualTo("Marius"); + assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); + assertThat(dto.getSalt()).isEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + } + + @Test + public void update_only_user_password() throws Exception { + db.prepareDbUnit(getClass(), "update_user.xml"); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") + .setPassword("password2") + .setPasswordConfirmation("password2")); + session.commit(); + session.clearCache(); + + UserDto dto = userDao.selectNullableByLogin(session, "marius"); + assertThat(dto.getSalt()).isNotEqualTo("79bd6a8e79fb8c76ac8b121cc7e8e11ad1af8365"); + assertThat(dto.getCryptedPassword()).isNotEqualTo("650d2261c98361e2f67f90ce5c65a95e7d8ea2fg"); + + // Following fields has not changed + assertThat(dto.getName()).isEqualTo("Marius"); + assertThat(dto.getScmAccounts()).contains("ma,marius33"); + assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr"); + } + + @Test + public void associate_default_groups_when_updating_user() throws Exception { + db.prepareDbUnit(getClass(), "associate_default_groups_when_updating_user.xml"); + createDefaultGroup(); + + userUpdater.update(UpdateUser.create("marius") .setName("Marius2") .setEmail("marius2@mail.com") .setPassword("password2") .setPasswordConfirmation("password2") - .setPreventReactivation(false)); + .setScmAccounts(newArrayList("ma2"))); + session.commit(); GroupMembershipFinder.Membership membership = groupMembershipFinder.find(GroupMembershipQuery.builder().login("marius").groupSearch("sonar-users").build()); assertThat(membership.groups()).hasSize(1); @@ -437,6 +551,21 @@ public class UserCreatorTest { assertThat(membership.groups().get(0).isMember()).isTrue(); } + @Test + public void fail_to_update_unknown_user() throws Exception { + try { + userUpdater.update(UpdateUser.create("marius") + .setName("Marius2") + .setEmail("marius2@mail.com") + .setPassword("password2") + .setPasswordConfirmation("password2") + .setScmAccounts(newArrayList("ma2"))); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("User 'marius' does not exists"); + } + } + private void createDefaultGroup() { settings.setProperty(CoreProperties.CORE_DEFAULT_GROUP, "sonar-users"); groupDao.insert(session, new GroupDto().setName("sonar-users").setDescription("Sonar Users")); diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java index ae92a794896..e9148ecb3c3 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java @@ -27,20 +27,23 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.sonar.api.i18n.I18n; import org.sonar.api.server.ws.WebService; +import org.sonar.server.user.MockUserSession; import org.sonar.server.user.NewUser; -import org.sonar.server.user.ReactivationException; import org.sonar.server.user.UserService; import org.sonar.server.user.index.UserDoc; import org.sonar.server.ws.WsTester; +import java.util.Locale; import java.util.Map; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newHashMap; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class CreateActionTest { @@ -52,12 +55,15 @@ public class CreateActionTest { @Mock UserService service; + @Mock + I18n i18n; + @Captor ArgumentCaptor newUserCaptor; @Before public void setUp() throws Exception { - tester = new WsTester(new UsersWs(new CreateAction(service))); + tester = new WsTester(new UsersWs(new CreateAction(service, i18n), new UpdateAction(service))); controller = tester.controller("api/users"); } @@ -89,13 +95,10 @@ public class CreateActionTest { assertThat(newUserCaptor.getValue().scmAccounts()).containsOnly("jn"); assertThat(newUserCaptor.getValue().password()).isEqualTo("1234"); assertThat(newUserCaptor.getValue().passwordConfirmation()).isEqualTo("1234"); - assertThat(newUserCaptor.getValue().isPreventReactivation()).isFalse(); } @Test - public void return_409_when_reactive_exception() throws Exception { - doThrow(new ReactivationException("Already exists", "john")).when(service).create(any(NewUser.class)); - + public void reactivate_user() throws Exception { Map userDocMap = newHashMap(); userDocMap.put("login", "john"); userDocMap.put("name", "John"); @@ -105,15 +108,19 @@ public class CreateActionTest { userDocMap.put("createdAt", 15000L); userDocMap.put("updatedAt", 15000L); when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + when(service.create(any(NewUser.class))).thenReturn(true); + + MockUserSession.set().setLogin("julien").setLocale(Locale.FRENCH); + when(i18n.message(Locale.FRENCH, "user.reactivated", "user.reactivated", "john")).thenReturn("The user 'john' has been reactivated."); tester.newPostRequest("api/users", "create") .setParam("login", "john") - .setParam("name", "John2") - .setParam("email", "john2@email.com") - .setParam("scm_accounts", "jn2") - .setParam("password", "12345") - .setParam("password_confirmation", "12345").execute() - .assertStatus(409) - .assertJson(getClass(), "return_409_when_reactive_exception.json"); + .setParam("name", "John") + .setParam("email", "john@email.com") + .setParam("scm_accounts", "jn") + .setParam("password", "1234") + .setParam("password_confirmation", "1234").execute() + .assertJson(getClass(), "reactivate_user.json"); } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java new file mode 100644 index 00000000000..7d4c7c2ab0f --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java @@ -0,0 +1,194 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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.user.ws; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.sonar.api.i18n.I18n; +import org.sonar.api.server.ws.WebService; +import org.sonar.server.user.UpdateUser; +import org.sonar.server.user.UserService; +import org.sonar.server.user.index.UserDoc; +import org.sonar.server.ws.WsTester; + +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.*; + +@RunWith(MockitoJUnitRunner.class) +public class UpdateActionTest { + + WebService.Controller controller; + + WsTester tester; + + @Mock + UserService service; + + @Captor + ArgumentCaptor userCaptor; + + @Before + public void setUp() throws Exception { + tester = new WsTester(new UsersWs(new CreateAction(service, mock(I18n.class)), new UpdateAction(service))); + controller = tester.controller("api/users"); + } + + @Test + public void update_user() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "update") + .setParam("login", "john") + .setParam("name", "John") + .setParam("email", "john@email.com") + .setParam("scm_accounts", "jn") + .setParam("password", "1234") + .setParam("password_confirmation", "1234") + .execute() + .assertJson(getClass(), "update_user.json"); + + verify(service).update(userCaptor.capture()); + assertThat(userCaptor.getValue().login()).isEqualTo("john"); + assertThat(userCaptor.getValue().name()).isEqualTo("John"); + assertThat(userCaptor.getValue().email()).isEqualTo("john@email.com"); + assertThat(userCaptor.getValue().scmAccounts()).containsOnly("jn"); + assertThat(userCaptor.getValue().password()).isEqualTo("1234"); + assertThat(userCaptor.getValue().passwordConfirmation()).isEqualTo("1234"); + } + + @Test + public void update_only_name() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "update") + .setParam("login", "john") + .setParam("name", "John") + .execute(); + + verify(service).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isTrue(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_email() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "update") + .setParam("login", "john") + .setParam("email", "john@email.com") + .execute(); + + verify(service).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isTrue(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_scm_accounts() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "update") + .setParam("login", "john") + .setParam("scm_accounts", "jn") + .execute(); + + verify(service).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isTrue(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_password() throws Exception { + Map userDocMap = newHashMap(); + userDocMap.put("login", "john"); + userDocMap.put("name", "John"); + userDocMap.put("email", "john@email.com"); + userDocMap.put("scmAccounts", newArrayList("jn")); + userDocMap.put("active", true); + userDocMap.put("createdAt", 15000L); + userDocMap.put("updatedAt", 15000L); + when(service.getByLogin("john")).thenReturn(new UserDoc(userDocMap)); + + tester.newPostRequest("api/users", "update") + .setParam("login", "john") + .setParam("password", "1234") + .setParam("password_confirmation", "1234") + .execute(); + + verify(service).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isTrue(); + assertThat(userCaptor.getValue().isPasswordChanged()).isTrue(); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java index 70ff4320a4e..3a3290019a1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java @@ -22,6 +22,7 @@ package org.sonar.server.user.ws; import org.junit.Before; import org.junit.Test; +import org.sonar.api.i18n.I18n; import org.sonar.api.server.ws.RailsHandler; import org.sonar.api.server.ws.WebService; import org.sonar.server.user.UserService; @@ -36,7 +37,7 @@ public class UsersWsTest { @Before public void setUp() throws Exception { - WsTester tester = new WsTester(new UsersWs(new CreateAction(mock(UserService.class)))); + WsTester tester = new WsTester(new UsersWs(new CreateAction(mock(UserService.class), mock(I18n.class)), new UpdateAction(mock(UserService.class)))); controller = tester.controller("api/users"); } @@ -63,7 +64,7 @@ public class UsersWsTest { WebService.Action action = controller.action("create"); assertThat(action).isNotNull(); assertThat(action.isPost()).isTrue(); - assertThat(action.params()).hasSize(7); + assertThat(action.params()).hasSize(6); } @Test @@ -71,8 +72,7 @@ public class UsersWsTest { WebService.Action action = controller.action("update"); assertThat(action).isNotNull(); assertThat(action.isPost()).isTrue(); - assertThat(action.handler()).isInstanceOf(RailsHandler.class); - assertThat(action.params()).hasSize(4); + assertThat(action.params()).hasSize(6); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index 35c1d39c556..f6cdb3c604b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -62,6 +62,11 @@ public class WebServiceEngineTest { return method; } + @Override + public boolean hasParam(String key) { + return params.keySet().contains(key); + } + @Override protected String readParam(String key) { return params.get(key); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java index 8737f8f2457..c4b55bb164f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java @@ -62,6 +62,11 @@ public class WsTester { return method; } + @Override + public boolean hasParam(String key) { + return params.keySet().contains(key); + } + public TestRequest setParams(Map m) { this.params = m; return this; @@ -148,7 +153,6 @@ public class WsTester { return stream; } - @Override public Response noContent() { stream().setStatus(HttpURLConnection.HTTP_NO_CONTENT); @@ -157,7 +161,6 @@ public class WsTester { } } - public static class Result { private final TestResponse response; diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/associate_default_groups_when_reactivating_user.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/associate_default_groups_when_reactivating_user.xml similarity index 100% rename from server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/associate_default_groups_when_reactivating_user.xml rename to server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/associate_default_groups_when_reactivating_user.xml diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/fail_to_create_user_if_already_exists_but_inactive.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/associate_default_groups_when_updating_user.xml similarity index 67% rename from server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/fail_to_create_user_if_already_exists_but_inactive.xml rename to server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/associate_default_groups_when_updating_user.xml index 24fd96bdfc7..d4b47674bdf 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/fail_to_create_user_if_already_exists_but_inactive.xml +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/associate_default_groups_when_updating_user.xml @@ -3,4 +3,8 @@ + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_reactivate_user_if_not_disabled.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_reactivate_user_if_not_disabled.xml new file mode 100644 index 00000000000..3c22b1ebf61 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/fail_to_reactivate_user_if_not_disabled.xml @@ -0,0 +1,6 @@ + + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/reactivate_user.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/reactivate_user.xml similarity index 100% rename from server/sonar-server/src/test/resources/org/sonar/server/user/UserCreatorTest/reactivate_user.xml rename to server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/reactivate_user.xml diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/update_user.xml b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/update_user.xml new file mode 100644 index 00000000000..3c22b1ebf61 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/UserUpdaterTest/update_user.xml @@ -0,0 +1,6 @@ + + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/reactivate_user.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/reactivate_user.json new file mode 100644 index 00000000000..43f484c95a4 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/reactivate_user.json @@ -0,0 +1,14 @@ +{ + "user": { + "login": "john", + "name": "John", + "email": "john@email.com", + "scmAccounts": ["jn"], + "active": true + }, + "infos": [ + { + "msg": "The user 'john' has been reactivated." + } + ] +} diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_user.json similarity index 100% rename from server/sonar-server/src/test/resources/org/sonar/server/user/ws/CreateActionTest/return_409_when_reactive_exception.json rename to server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_user.json diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index 9373041b351..2a3fa55a00b 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -1967,6 +1967,7 @@ events.name_required=Name (required) #------------------------------------------------------------------------------ user.bad_login=Use only letters, numbers, and .-_@ please. user.password_doesnt_match_confirmation=Password doesn't match confirmation. +user.reactivated=The user '{0}' has been reactivated. #------------------------------------------------------------------------------ diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java index 89e920e5338..2cbd0ef0b4d 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java @@ -42,6 +42,11 @@ public abstract class Request { */ public abstract String method(); + /** + * Return true of the parameter is set. + */ + public abstract boolean hasParam(String key); + /** * Returns a non-null value. To be used when parameter is required or has a default value. * diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java index 1053ebece29..d2ee7a65d9f 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java @@ -42,6 +42,11 @@ public class SimpleGetRequest extends Request { return "GET"; } + @Override + public boolean hasParam(String key) { + return params.keySet().contains(key); + } + @Override public String param(String key) { return params.get(key); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java index 6a86cc48e6f..ad7e00aea17 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java @@ -48,6 +48,11 @@ public class RequestTest { return "GET"; } + @Override + public boolean hasParam(String key) { + return params.keySet().contains(key); + } + public SimpleRequest setParam(String key, @Nullable String value) { if (value != null) { params.put(key, value); @@ -113,6 +118,14 @@ public class RequestTest { request.setAction(context.controller("my_controller").action("my_action")); } + @Test + public void has_param() throws Exception { + request.setParam("a_required_string", "foo"); + + assertThat(request.hasParam("a_required_string")).isTrue(); + assertThat(request.hasParam("unknown")).isFalse(); + } + @Test public void required_param_is_missing() throws Exception { try { diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/internal/SimpleGetRequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/internal/SimpleGetRequestTest.java index 26be2948d03..4ee4a6857e7 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/internal/SimpleGetRequestTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/internal/SimpleGetRequestTest.java @@ -34,4 +34,14 @@ public class SimpleGetRequestTest { assertThat(request.param("foo")).isEqualTo("bar"); assertThat(request.param("unknown")).isNull(); } + + @Test + public void has_param() throws Exception { + SimpleGetRequest request = new SimpleGetRequest(); + assertThat(request.method()).isEqualTo("GET"); + + request.setParam("foo", "bar"); + assertThat(request.hasParam("foo")).isTrue(); + assertThat(request.hasParam("unknown")).isFalse(); + } } -- 2.39.5