SONAR-6468 Restore unprivileged password change

* Check if external authentication is in use (currently done on UI side)
* Allow a user to change their own password

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/SecurityRealmFactory.java b/server/sonar-server/src/main/java/org/sonar/server/user/SecurityRealmFactory.java
index 78c4aa632a68e9034471826d074e319c5fb57c74..cf3a0ef340589fc8c1eaee658b113e9de17dc1b7 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/SecurityRealmFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/SecurityRealmFactory.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.user;
 
+import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
 import org.picocontainer.Startable;
 import org.sonar.api.CoreProperties;
@@ -97,10 +98,15 @@ public class SecurityRealmFactory implements Startable {
     // nothing
   }
 
+  @Nullable
   public SecurityRealm getRealm() {
     return realm;
   }
 
+  public boolean hasExternalAuthentication() {
+    return getRealm() != null;
+  }
+
   private static SecurityRealm selectRealm(SecurityRealm[] realms, String realmName) {
     for (SecurityRealm realm : realms) {
       if (StringUtils.equals(realmName, realm.getName())) {

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 9683ecc1485db20a19b7592278853233b21c7add..f10432b376973f87123db9013ec9e40498d47002 100644 (file)
--- 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
@@ -24,12 +24,18 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
+import java.security.SecureRandom;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.CoreProperties;
-import org.sonar.api.server.ServerSide;
 import org.sonar.api.config.Settings;
 import org.sonar.api.platform.NewUserHandler;
+import org.sonar.api.server.ServerSide;
 import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.user.GroupDto;
@@ -41,14 +47,6 @@ import org.sonar.server.exceptions.Message;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.util.Validation;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
-
-import java.security.SecureRandom;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Random;
-
 import static com.google.common.collect.Lists.newArrayList;
 
 @ServerSide
@@ -65,13 +63,15 @@ public class UserUpdater {
   private final DbClient dbClient;
   private final UserIndexer userIndexer;
   private final System2 system2;
+  private final SecurityRealmFactory realmFactory;
 
-  public UserUpdater(NewUserNotifier newUserNotifier, Settings settings, DbClient dbClient, UserIndexer userIndexer, System2 system2) {
+  public UserUpdater(NewUserNotifier newUserNotifier, Settings settings, DbClient dbClient, UserIndexer userIndexer, System2 system2, SecurityRealmFactory realmFactory) {
     this.newUserNotifier = newUserNotifier;
     this.settings = settings;
     this.dbClient = dbClient;
     this.userIndexer = userIndexer;
     this.system2 = system2;
+    this.realmFactory = realmFactory;
   }
 
   /**
@@ -182,6 +182,7 @@ public class UserUpdater {
     String password = updateUser.password();
     String passwordConfirmation = updateUser.passwordConfirmation();
     if (updateUser.isPasswordChanged()) {
+      checkPasswordChangeAllowed(messages);
       validatePasswords(password, passwordConfirmation, messages);
       setEncryptedPassWord(password, userDto);
     }
@@ -233,6 +234,12 @@ public class UserUpdater {
     }
   }
 
+  private void checkPasswordChangeAllowed(List<Message> messages) {
+    if (realmFactory.hasExternalAuthentication()) {
+      messages.add(Message.of("user.password_cant_be_changed_on_external_auth"));
+    }
+  }
+
   private static void validatePasswords(@Nullable String password, @Nullable String passwordConfirmation, List<Message> messages) {
     checkNotEmptyParam(password, PASSWORD_PARAM, messages);
     checkNotEmptyParam(passwordConfirmation, PASSWORD_CONFIRMATION_PARAM, messages);

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 76d2d61596e07ea294934e0d020579cc4737b7ad..c59560a786b9800c11591d5a77eeebf954fa82fa 100644 (file)
--- 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
@@ -45,6 +45,7 @@ public class ChangePasswordAction implements UsersWsAction {
   public void define(WebService.NewController controller) {
     WebService.NewAction action = controller.createAction("change_password")
       .setDescription("Update a user's password. Authenticated users can change their own password, " +
+        "provided that the account is not linked to an external authentication system. " +
         "Administer System permission is required to change another user's password.")
       .setSince("5.2")
       .setPost(true)

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/SecurityRealmFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/SecurityRealmFactoryTest.java
index d97ddbc314e07eb8261ec041975f3b53551298c6..ab23323e5e208c7f94079890850df3539b024071 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/SecurityRealmFactoryTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/SecurityRealmFactoryTest.java
@@ -46,6 +46,7 @@ public class SecurityRealmFactoryTest {
     SecurityRealmFactory factory = new SecurityRealmFactory(settings, new SecurityRealm[]{realm});
     factory.start();
     assertThat(factory.getRealm()).isSameAs(realm);
+    assertThat(factory.hasExternalAuthentication()).isTrue();
     verify(realm).init();
 
     factory.stop();
@@ -56,6 +57,7 @@ public class SecurityRealmFactoryTest {
     SecurityRealmFactory factory = new SecurityRealmFactory(settings);
     factory.start();
     assertThat(factory.getRealm()).isNull();
+    assertThat(factory.hasExternalAuthentication()).isFalse();
   }
 
   @Test

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
index 9f17e500e9ed1945646e608cea29747488c74455..1e9c7fa2b5de31d63843590b94aba3a4282e2d9b 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
@@ -78,6 +78,9 @@ public class UserUpdaterTest {
   @Mock
   NewUserNotifier newUserNotifier;
 
+  @Mock
+  SecurityRealmFactory realmFactory;
+
   @Captor
   ArgumentCaptor<NewUserHandler.Context> newUserHandler;
 
@@ -106,7 +109,7 @@ public class UserUpdaterTest {
     DbClient dbClient = new DbClient(db.database(), db.myBatis(), userDao, groupDao, userGroupDao);
     userIndexer = (UserIndexer) new UserIndexer(dbClient, es.client()).setEnabled(true);
     userUpdater = new UserUpdater(newUserNotifier, settings, dbClient,
-      userIndexer, system2);
+      userIndexer, system2, realmFactory);
   }
 
   @After
@@ -763,6 +766,23 @@ public class UserUpdaterTest {
     assertThat(dto.getEmail()).isEqualTo("marius@lesbronzes.fr");
   }
 
+  @Test
+  public void fail_to_update_password_when_external_auth_is_used() {
+    db.prepareDbUnit(getClass(), "update_user.xml");
+    createDefaultGroup();
+
+    when(realmFactory.hasExternalAuthentication()).thenReturn(true);
+
+    try {
+      userUpdater.update(UpdateUser.create("marius")
+        .setPassword("password2")
+        .setPasswordConfirmation("password2"));
+    } catch (BadRequestException e) {
+      assertThat(e.errors().messages()).containsOnly(Message.of("user.password_cant_be_changed_on_external_auth"));
+    }
+  }
+
+
   @Test
   public void associate_default_group_when_updating_user() {
     db.prepareDbUnit(getClass(), "associate_default_groups_when_updating_user.xml");

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 092762c4e2ded43d7ade34edf2003f1a31caedcc..38f507b27baade86331f6bc70e8254f08635e2ef 100644 (file)
--- 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
@@ -25,6 +25,9 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.config.Settings;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
@@ -35,10 +38,12 @@ 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.NewUserNotifier;
+import org.sonar.server.user.SecurityRealmFactory;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.db.GroupDao;
 import org.sonar.server.user.db.UserDao;
@@ -51,7 +56,9 @@ import org.sonar.server.ws.WsTester;
 import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+@RunWith(MockitoJUnitRunner.class)
 public class ChangePasswordActionTest {
 
   static final Settings settings = new Settings().setProperty("sonar.defaultGroup", "sonar-users");
@@ -76,6 +83,9 @@ public class ChangePasswordActionTest {
 
   DbSession session;
 
+  @Mock
+  SecurityRealmFactory realmFactory;
+
   @Before
   public void setUp() {
     dbTester.truncateTables();
@@ -92,7 +102,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), userSessionRule)));
+    tester = new WsTester(
+      new UsersWs(new ChangePasswordAction(new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, realmFactory), userSessionRule)));
     controller = tester.controller("api/users");
   }
 
@@ -154,6 +165,18 @@ public class ChangePasswordActionTest {
     assertThat(newPassword).isNotEqualTo(originalPassword);
   }
 
+  @Test(expected = BadRequestException.class)
+  public void fail_to_update_password_on_external_auth() throws Exception {
+    createUser();
+    session.clearCache();
+    when(realmFactory.hasExternalAuthentication()).thenReturn(true);
+
+    tester.newPostRequest("api/users", "change_password")
+      .setParam("login", "john")
+      .setParam("password", "Valar Morghulis")
+      .execute();
+  }
+
   private void createUser() {
     dbClient.userDao().insert(session, new UserDto()
       .setEmail("john@email.com")

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 02613a65adce5121d3e0410890f8f7544b4c17a6..9c99614d6d804c3632da9a13cfd1c2c682226785 100644 (file)
--- 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
@@ -20,6 +20,7 @@
 
 package org.sonar.server.user.ws;
 
+import java.util.Locale;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
@@ -42,6 +43,7 @@ import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.NewUserNotifier;
+import org.sonar.server.user.SecurityRealmFactory;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.db.GroupDao;
 import org.sonar.server.user.db.UserDao;
@@ -52,8 +54,6 @@ import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.ws.WsTester;
 
-import java.util.Locale;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -103,7 +103,7 @@ public class CreateActionTest {
     userIndexer = (UserIndexer) new UserIndexer(dbClient, esTester.client()).setEnabled(true);
     index = new UserIndex(esTester.client());
     tester = new WsTester(new UsersWs(new CreateAction(index,
-      new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2),
+      new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, mock(SecurityRealmFactory.class)),
       i18n, userSessionRule)));
     controller = tester.controller("api/users");
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
index c251ad45a03e20cbcd182741354acedcbca61b6d..871672759bb859cc5d6c44f67806177b4b677214 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
@@ -43,6 +43,7 @@ import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.NewUserNotifier;
+import org.sonar.server.user.SecurityRealmFactory;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.db.UserDao;
 import org.sonar.server.user.index.UserDoc;
@@ -95,7 +96,7 @@ public class DeactivateActionTest {
     userIndexer = (UserIndexer) new UserIndexer(dbClient, esTester.client()).setEnabled(true);
     index = new UserIndex(esTester.client());
     tester = new WsTester(new UsersWs(new DeactivateAction(index,
-      new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2), userSessionRule)));
+      new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, mock(SecurityRealmFactory.class)), userSessionRule)));
     controller = tester.controller("api/users");
 
   }

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
index 5bd83a3cec60d661d2bfcb56f8d2471714ec7793..2cce87b68ebc6b2a09085d22993744a381679f97 100644 (file)
--- 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
@@ -38,6 +38,7 @@ import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.user.NewUserNotifier;
+import org.sonar.server.user.SecurityRealmFactory;
 import org.sonar.server.user.UserUpdater;
 import org.sonar.server.user.db.GroupDao;
 import org.sonar.server.user.db.UserDao;
@@ -91,7 +92,7 @@ public class UpdateActionTest {
     userIndexer = (UserIndexer) new UserIndexer(dbClient, esTester.client()).setEnabled(true);
     index = new UserIndex(esTester.client());
     tester = new WsTester(new UsersWs(new UpdateAction(index,
-        new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2), userSessionRule)));
+      new UserUpdater(mock(NewUserNotifier.class), settings, dbClient, userIndexer, system2, mock(SecurityRealmFactory.class)), userSessionRule)));
     controller = tester.controller("api/users");
   }
 

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 83ca78d5b1f776b20c4bc44df90b784d015e1a33..07880341d8b2a9e8b856638465c9578f08d9f96a 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -2089,6 +2089,7 @@ user.reactivated=The user '{0}' has been reactivated.
 user.add_scm_account=Add SCM account
 user.scm_account_already_used=The scm account '{0}' is already used by user(s) : '{1}'
 user.login_or_email_used_as_scm_account=Login and email are automatically considered as SCM accounts
+user.password_cant_be_changed_on_external_auth=Password cannot be changed when external authentication is used
 
 
 #------------------------------------------------------------------------------