]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-6467 Change behavior of user update WS
authorJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Mon, 4 May 2015 09:47:51 +0000 (11:47 +0200)
committerJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Thu, 7 May 2015 12:41:36 +0000 (14:41 +0200)
server/sonar-server/src/main/java/org/sonar/server/user/ws/UpdateAction.java
server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-update.json [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/user/ws/UpdateActionTest.java
server/sonar-server/src/test/java/org/sonar/server/user/ws/UsersWsTest.java
server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_email.json [new file with mode: 0644]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_name.json [new file with mode: 0644]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_scm_accounts.json [new file with mode: 0644]
server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_user.json

index 565781e35d01577581499a453101bd09516272b3..1ab8413d5367aa6c13c0c81170032ce987d7a5e8 100644 (file)
@@ -34,8 +34,6 @@ import org.sonar.server.user.index.UserIndex;
 public class UpdateAction implements BaseUsersWsAction {
 
   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";
@@ -51,29 +49,19 @@ public class UpdateAction implements BaseUsersWsAction {
   @Override
   public void define(WebService.NewController controller) {
     WebService.NewAction action = controller.createAction("update")
-      .setDescription("Update a user. Requires Administer System permission")
+      .setDescription("Update a user. Requires Administer System permission. Since 5.2, a user's password can only be changed using the 'change_password' action.")
       .setSince("3.7")
       .setPost(true)
-      .setHandler(this);
+      .setHandler(this)
+      .setResponseExample(getClass().getResource("example-update.json"));
 
     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)
@@ -100,12 +88,6 @@ public class UpdateAction implements BaseUsersWsAction {
     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));
-    }
 
     userUpdater.update(updateUser);
     writeResponse(response, login);
diff --git a/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-update.json b/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-update.json
new file mode 100644 (file)
index 0000000..f423f9e
--- /dev/null
@@ -0,0 +1,9 @@
+{
+  "user": {
+    "login": "ada.lovelace",
+    "name": "Ada Lovelace",
+    "email": "ada.lovelace@noteg.com",
+    "scmAccounts": ["ada.lovelace"],
+    "active": true
+  }
+}
index 4a6ed1ee9538889153c0775076a4ce7f71b9768f..450dd4e2371c33c2bae638894ce39044b34ab96c 100644 (file)
 
 package org.sonar.server.user.ws;
 
+import org.junit.After;
 import org.junit.Before;
+import org.junit.ClassRule;
 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.config.Settings;
 import org.sonar.api.server.ws.WebService;
+import org.sonar.api.utils.System2;
 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.ForbiddenException;
 import org.sonar.server.user.MockUserSession;
-import org.sonar.server.user.UpdateUser;
+import org.sonar.server.user.NewUserNotifier;
 import org.sonar.server.user.UserUpdater;
-import org.sonar.server.user.index.UserDoc;
+import org.sonar.server.user.db.GroupDao;
+import org.sonar.server.user.db.UserDao;
+import org.sonar.server.user.db.UserGroupDao;
 import org.sonar.server.user.index.UserIndex;
+import org.sonar.server.user.index.UserIndexDefinition;
+import org.sonar.server.user.index.UserIndexer;
 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.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.mock;
 
-@RunWith(MockitoJUnitRunner.class)
 public class UpdateActionTest {
 
+  static final Settings settings = new Settings().setProperty("sonar.defaultGroup", "sonar-users");
+
+  @ClassRule
+  public static final DbTester dbTester = new DbTester();
+
+  @ClassRule
+  public static final EsTester esTester = new EsTester().addDefinitions(new UserIndexDefinition(settings));
+
   WebService.Controller controller;
 
   WsTester tester;
 
-  @Mock
   UserIndex index;
 
-  @Mock
-  UserUpdater updater;
+  DbClient dbClient;
 
-  @Captor
-  ArgumentCaptor<UpdateUser> userCaptor;
+  UserIndexer userIndexer;
+
+  DbSession session;
 
   @Before
   public void setUp() throws Exception {
-    tester = new WsTester(new UsersWs(new UpdateAction(index, updater)));
+    dbTester.truncateTables();
+    esTester.truncateIndices();
+
+    System2 system2 = new System2();
+    UserDao userDao = new UserDao(dbTester.myBatis(), system2);
+    UserGroupDao userGroupDao = new UserGroupDao();
+    GroupDao groupDao = new GroupDao();
+    dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), userDao, userGroupDao, groupDao);
+    session = dbClient.openSession(false);
+    groupDao.insert(session, new GroupDto().setName("sonar-users"));
+    session.commit();
+
+    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))));
     controller = tester.controller("api/users");
+
     MockUserSession.set().setLogin("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
   }
 
+  @After
+  public void tearDown() throws Exception {
+    session.close();
+  }
+
+  @Test(expected = ForbiddenException.class)
+  public void fail_on_missing_permission() throws Exception {
+    createUser();
+
+    MockUserSession.set().setLogin("polop");
+    tester.newPostRequest("api/users", "update")
+      .setParam("login", "john")
+      .execute();
+  }
+
   @Test
   public void update_user() throws Exception {
-    Map<String, Object> 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(index.getByLogin("john")).thenReturn(new UserDoc(userDocMap));
+    createUser();
 
     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")
+      .setParam("name", "Jon Snow")
+      .setParam("email", "jon.snow@thegreatw.all")
+      .setParam("scm_accounts", "jon.snow")
       .execute()
       .assertJson(getClass(), "update_user.json");
-
-    verify(updater).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<String, Object> 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(index.getByLogin("john")).thenReturn(new UserDoc(userDocMap));
+    createUser();
 
     tester.newPostRequest("api/users", "update")
       .setParam("login", "john")
-      .setParam("name", "John")
-      .execute();
-
-    verify(updater).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();
+      .setParam("name", "Jon Snow")
+      .execute()
+      .assertJson(getClass(), "update_name.json");
   }
 
   @Test
   public void update_only_email() throws Exception {
-    Map<String, Object> 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(index.getByLogin("john")).thenReturn(new UserDoc(userDocMap));
+    createUser();
 
     tester.newPostRequest("api/users", "update")
       .setParam("login", "john")
-      .setParam("email", "john@email.com")
-      .execute();
-
-    verify(updater).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();
+      .setParam("email", "jon.snow@thegreatw.all")
+      .execute()
+      .assertJson(getClass(), "update_email.json");
   }
 
   @Test
   public void update_only_scm_accounts() throws Exception {
-    Map<String, Object> 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(index.getByLogin("john")).thenReturn(new UserDoc(userDocMap));
+    createUser();
 
     tester.newPostRequest("api/users", "update")
       .setParam("login", "john")
-      .setParam("scm_accounts", "jn")
-      .execute();
-
-    verify(updater).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();
+      .setParam("scm_accounts", "jon.snow")
+      .execute()
+      .assertJson(getClass(), "update_scm_accounts.json");
   }
 
-  @Test
-  public void update_only_password() throws Exception {
-    Map<String, Object> 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(index.getByLogin("john")).thenReturn(new UserDoc(userDocMap));
-
-    tester.newPostRequest("api/users", "update")
-      .setParam("login", "john")
-      .setParam("password", "1234")
-      .setParam("password_confirmation", "1234")
-      .execute();
-
-    verify(updater).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();
+  private void createUser() {
+    dbClient.userDao().insert(session, new UserDto()
+      .setEmail("john@email.com")
+      .setLogin("john")
+      .setName("John")
+      .setScmAccounts(newArrayList("jn"))
+      .setActive(true));
+    session.commit();
+    userIndexer.index();
   }
 }
index 75f3f8ffc2bb2223bbf69ed61610a8bc77f9c393..3ab08ea313e241409d87ae7581497d5b5d77d85b 100644 (file)
@@ -77,7 +77,7 @@ public class UsersWsTest {
     WebService.Action action = controller.action("update");
     assertThat(action).isNotNull();
     assertThat(action.isPost()).isTrue();
-    assertThat(action.params()).hasSize(6);
+    assertThat(action.params()).hasSize(4);
   }
 
   @Test
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_email.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_email.json
new file mode 100644 (file)
index 0000000..32461eb
--- /dev/null
@@ -0,0 +1,11 @@
+{
+  "user": {
+    "login": "john",
+    "name": "John",
+    "email": "jon.snow@thegreatw.all",
+    "active": true,
+    "scmAccounts": [
+      "jn"
+    ]
+  }
+}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_name.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_name.json
new file mode 100644 (file)
index 0000000..f0d7b92
--- /dev/null
@@ -0,0 +1,11 @@
+{
+  "user": {
+    "login": "john",
+    "name": "Jon Snow",
+    "email": "john@email.com",
+    "active": true,
+    "scmAccounts": [
+      "jn"
+    ]
+  }
+}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_scm_accounts.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/UpdateActionTest/update_scm_accounts.json
new file mode 100644 (file)
index 0000000..311a8ca
--- /dev/null
@@ -0,0 +1,11 @@
+{
+  "user": {
+    "login": "john",
+    "name": "John",
+    "email": "john@email.com",
+    "active": true,
+    "scmAccounts": [
+      "jon.snow"
+    ]
+  }
+}
index 0feda7c9c1cdc10449ea7300dfb07f06123c249e..7293894355708b4331fdf3dd71c89105c7e07e40 100644 (file)
@@ -1,9 +1,9 @@
 {
   "user": {
     "login": "john",
-    "name": "John",
-    "email": "john@email.com",
-    "scmAccounts": ["jn"],
+    "name": "Jon Snow",
+    "email": "jon.snow@thegreatw.all",
+    "scmAccounts": ["jon.snow"],
     "active": true
   }
 }