SONAR-6468 Check old password when changing password on self

4 files changed, 63 insertions, 10 deletions

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
index 6e849793bbc..e0e1080911e 100644
--- 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
@@ -134,6 +134,19 @@ public class UserUpdater {
     userIndexer.index();
   }
 
+  public void checkCurrentPassword(String login, String password) {
+    DbSession dbSession = dbClient.openSession(false);
+    try {
+      UserDto user = dbClient.userDao().selectByLogin(dbSession, login);
+      String cryptedPassword = encryptPassword(password, user.getSalt());
+      if (!cryptedPassword.equals(user.getCryptedPassword())) {
+        throw new IllegalArgumentException("Incorrect password");
+      }
+    } finally {
+      dbSession.close();
+    }
+  }
+
   private UserDto createNewUserDto(DbSession dbSession, NewUser newUser) {
     UserDto userDto = new UserDto();
     List<Message> messages = newArrayList();
@@ -301,7 +314,11 @@ public class UserUpdater {
     random.nextBytes(salt);
     String saltHex = DigestUtils.sha1Hex(salt);
     userDto.setSalt(saltHex);
-    userDto.setCryptedPassword(DigestUtils.sha1Hex("--" + saltHex + "--" + password + "--"));
+    userDto.setCryptedPassword(encryptPassword(password, saltHex));
+  }
+
+  private static String encryptPassword(String password, String salt) {
+    return DigestUtils.sha1Hex("--" + salt + "--" + password + "--");
   }
 
   private void notifyNewUser(String login, String name, String email) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java
index c59560a786b..0fcb4ee58b2 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/ChangePasswordAction.java
@@ -32,6 +32,7 @@ public class ChangePasswordAction implements UsersWsAction {
 
   private static final String PARAM_LOGIN = "login";
   private static final String PARAM_PASSWORD = "password";
+  private static final String PARAM_PREVIOUS_PASSWORD = "previousPassword";
 
   private final UserUpdater userUpdater;
   private final UserSession userSession;
@@ -60,6 +61,11 @@ public class ChangePasswordAction implements UsersWsAction {
       .setDescription("New password")
       .setRequired(true)
       .setExampleValue("mypassword");
+
+    action.createParam(PARAM_PREVIOUS_PASSWORD)
+      .setDescription("Previous password. Required when changing one's own password.")
+      .setRequired(false)
+      .setExampleValue("oldpassword");
   }
 
   @Override
@@ -67,7 +73,10 @@ public class ChangePasswordAction implements UsersWsAction {
     userSession.checkLoggedIn();
 
     String login = request.mandatoryParam(PARAM_LOGIN);
-    if (!login.equals(userSession.getLogin())) {
+    if (login.equals(userSession.getLogin())) {
+      String previousPassword = request.mandatoryParam(PARAM_PREVIOUS_PASSWORD);
+      userUpdater.checkCurrentPassword(login, previousPassword);
+    } else {
       userSession.checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN);
     }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
index 83b484554cb..c0c419bbd95 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
@@ -32,13 +32,13 @@ import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.DbTester;
 import org.sonar.core.user.GroupDto;
-import org.sonar.core.user.UserDto;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
+import org.sonar.server.user.NewUser;
 import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.SecurityRealmFactory;
 import org.sonar.server.user.UserUpdater;
@@ -76,6 +76,8 @@ public class ChangePasswordActionTest {
 
   DbClient dbClient;
 
+  UserUpdater userUpdater;
+
   UserIndexer userIndexer;
 
   DbSession session;
@@ -98,8 +100,8 @@ public class ChangePasswordActionTest {
 
     userIndexer = (UserIndexer) new UserIndexer(dbClient, esTester.client()).setEnabled(true);
     index = new UserIndex(esTester.client());
-    tester = new WsTester(
-      new UsersWs(new ChangePasswordAction(new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, realmFactory), userSessionRule)));
+    userUpdater = new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, realmFactory);
+    tester = new WsTester(new UsersWs(new ChangePasswordAction(userUpdater, userSessionRule)));
     controller = tester.controller("api/users");
   }
 
@@ -152,6 +154,7 @@ public class ChangePasswordActionTest {
     userSessionRule.login("john");
     tester.newPostRequest("api/users", "change_password")
       .setParam("login", "john")
+      .setParam("previousPassword", "Valar Dohaeris")
       .setParam("password", "Valar Morghulis")
       .execute()
       .assertNoContent();
@@ -161,6 +164,31 @@ public class ChangePasswordActionTest {
     assertThat(newPassword).isNotEqualTo(originalPassword);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void fail_to_update_password_on_self_without_old_password() throws Exception {
+    createUser();
+    session.clearCache();
+
+    userSessionRule.login("john");
+    tester.newPostRequest("api/users", "change_password")
+      .setParam("login", "john")
+      .setParam("password", "Valar Morghulis")
+      .execute();
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void fail_to_update_password_on_self_with_bad_old_password() throws Exception {
+    createUser();
+    session.clearCache();
+
+    userSessionRule.login("john");
+    tester.newPostRequest("api/users", "change_password")
+      .setParam("login", "john")
+      .setParam("previousPassword", "I dunno")
+      .setParam("password", "Valar Morghulis")
+      .execute();
+  }
+
   @Test(expected = BadRequestException.class)
   public void fail_to_update_password_on_external_auth() throws Exception {
     createUser();
@@ -174,13 +202,12 @@ public class ChangePasswordActionTest {
   }
 
   private void createUser() {
-    dbClient.userDao().insert(session, new UserDto()
+    userUpdater.create(NewUser.create()
       .setEmail("john@email.com")
       .setLogin("john")
       .setName("John")
       .setScmAccounts(newArrayList("jn"))
-      .setActive(true));
-    session.commit();
-    userIndexer.index();
+      .setPassword("Valar Dohaeris")
+      .setPasswordConfirmation("Valar Dohaeris"));
   }
 }
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 fdbe1d595f7..d40dfe87961 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
@@ -89,7 +89,7 @@ public class UsersWsTest {
     WebService.Action action = controller.action("change_password");
     assertThat(action).isNotNull();
     assertThat(action.isPost()).isTrue();
-    assertThat(action.params()).hasSize(2);
+    assertThat(action.params()).hasSize(3);
   }
 
   @Test