From 9439639b40e8f8b3f784ea3bad7b86acb83d994e Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 21 Apr 2017 10:13:23 +0200 Subject: [PATCH] SONAR-9128 Return email only when sys admin and return avatar when logged in api/users/search --- .../sonar/server/user/ws/SearchAction.java | 14 +++- .../sonar/server/user/ws/UserJsonWriter.java | 5 +- .../sonar/server/user/ws/search-example.json | 6 +- .../server/user/ws/SearchActionTest.java | 71 +++++++++++++++++-- .../org/sonar/server/user/ws/UsersWsTest.java | 3 +- .../user/ws/SearchActionTest/five_users.json | 10 +-- .../user/ws/SearchActionTest/page_one.json | 40 ++--------- .../user/ws/SearchActionTest/page_two.json | 40 ++--------- .../user/ws/SearchActionTest/user_one.json | 2 +- 9 files changed, 102 insertions(+), 89 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java index 7e42599308b..753d388f67e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java @@ -35,6 +35,7 @@ import org.sonar.db.DbSession; import org.sonar.db.user.UserDto; import org.sonar.server.es.SearchOptions; import org.sonar.server.es.SearchResult; +import org.sonar.server.issue.ws.AvatarResolver; import org.sonar.server.user.UserSession; import org.sonar.server.user.index.UserDoc; import org.sonar.server.user.index.UserIndex; @@ -53,6 +54,7 @@ import static org.sonar.api.utils.Paging.forPageIndex; import static org.sonar.core.util.stream.MoreCollectors.toList; import static org.sonar.server.es.SearchOptions.MAX_LIMIT; import static org.sonar.server.user.ws.UserJsonWriter.FIELD_ACTIVE; +import static org.sonar.server.user.ws.UserJsonWriter.FIELD_AVATAR; import static org.sonar.server.user.ws.UserJsonWriter.FIELD_EMAIL; import static org.sonar.server.user.ws.UserJsonWriter.FIELD_EXTERNAL_IDENTITY; import static org.sonar.server.user.ws.UserJsonWriter.FIELD_EXTERNAL_PROVIDER; @@ -74,11 +76,13 @@ public class SearchAction implements UsersWsAction { private final UserSession userSession; private final UserIndex userIndex; private final DbClient dbClient; + private final AvatarResolver avatarResolver; - public SearchAction(UserSession userSession, UserIndex userIndex, DbClient dbClient) { + public SearchAction(UserSession userSession, UserIndex userIndex, DbClient dbClient, AvatarResolver avatarResolver) { this.userSession = userSession; this.userIndex = userIndex; this.dbClient = dbClient; + this.avatarResolver = avatarResolver; } @Override @@ -88,7 +92,10 @@ public class SearchAction implements UsersWsAction { "Administer System permission is required to show the 'groups' field.
" + "When accessed anonymously, only logins and names are returned.") .setSince("3.6") - .setChangelog(new Change("6.4", "Paging response fields moved to a Paging object")) + .setChangelog( + new Change("6.4", "Paging response fields moved to a Paging object"), + new Change("6.4", "Avatar has been added to the response"), + new Change("6.4", "Email is only returned when user has Administer System permission")) .setHandler(this) .setResponseExample(getClass().getResource("search-example.json")); @@ -137,7 +144,7 @@ public class SearchAction implements UsersWsAction { .setLogin(user.getLogin()); setIfNeeded(FIELD_NAME, fields, user.getName(), userBuilder::setName); if (userSession.isLoggedIn()) { - setIfNeeded(FIELD_EMAIL, fields, user.getEmail(), userBuilder::setEmail); + setIfNeeded(FIELD_AVATAR, fields, user.getEmail(), u -> userBuilder.setAvatar(avatarResolver.create(user))); setIfNeeded(FIELD_ACTIVE, fields, user.isActive(), userBuilder::setActive); setIfNeeded(FIELD_LOCAL, fields, user.isLocal(), userBuilder::setLocal); setIfNeeded(FIELD_EXTERNAL_IDENTITY, fields, user.getExternalIdentity(), userBuilder::setExternalIdentity); @@ -147,6 +154,7 @@ public class SearchAction implements UsersWsAction { scm -> userBuilder.setScmAccounts(ScmAccounts.newBuilder().addAllScmAccounts(scm))); } if (userSession.isSystemAdministrator()) { + setIfNeeded(FIELD_EMAIL, fields, user.getEmail(), userBuilder::setEmail); setIfNeeded(isNeeded(FIELD_GROUPS, fields) && !groups.isEmpty(), groups, g -> userBuilder.setGroups(Groups.newBuilder().addAllGroups(g))); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java index f2a6aa933bf..9bec5b04b1c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/UserJsonWriter.java @@ -36,6 +36,7 @@ public class UserJsonWriter { static final String FIELD_LOGIN = "login"; static final String FIELD_NAME = "name"; static final String FIELD_EMAIL = "email"; + static final String FIELD_AVATAR = "avatart"; static final String FIELD_SCM_ACCOUNTS = "scmAccounts"; static final String FIELD_GROUPS = "groups"; static final String FIELD_ACTIVE = "active"; @@ -44,8 +45,8 @@ public class UserJsonWriter { static final String FIELD_EXTERNAL_IDENTITY = "externalIdentity"; static final String FIELD_EXTERNAL_PROVIDER = "externalProvider"; - public static final Set FIELDS = ImmutableSet.of(FIELD_NAME, FIELD_EMAIL, FIELD_SCM_ACCOUNTS, FIELD_GROUPS, FIELD_ACTIVE, FIELD_LOCAL, FIELD_EXTERNAL_IDENTITY, - FIELD_EXTERNAL_PROVIDER); + public static final Set FIELDS = ImmutableSet.of(FIELD_NAME, FIELD_EMAIL, FIELD_AVATAR, FIELD_SCM_ACCOUNTS, FIELD_GROUPS, FIELD_ACTIVE, FIELD_LOCAL, + FIELD_EXTERNAL_IDENTITY, FIELD_EXTERNAL_PROVIDER); private static final Set CONCISE_FIELDS = ImmutableSet.of(FIELD_NAME, FIELD_EMAIL, FIELD_ACTIVE); private final UserSession userSession; diff --git a/server/sonar-server/src/main/resources/org/sonar/server/user/ws/search-example.json b/server/sonar-server/src/main/resources/org/sonar/server/user/ws/search-example.json index b7b28fa2b17..1666ecb556a 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/user/ws/search-example.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/user/ws/search-example.json @@ -17,7 +17,8 @@ "tokensCount": 1, "local": true, "externalIdentity": "fmallet", - "externalProvider": "sonarqube" + "externalProvider": "sonarqube", + "avatar": "2f9dff586d3f74f825b059e3798a3bbb" }, { "login": "sbrandhof", @@ -34,7 +35,8 @@ "tokensCount": 3, "local": false, "externalIdentity": "sbrandhof@ldap.com", - "externalProvider": "LDAP" + "externalProvider": "LDAP", + "avatar": "3930ad855bc7fe48db8e9a663174cdd3" } ] } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java index c431021da91..f03a1f887c8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java @@ -21,6 +21,7 @@ package org.sonar.server.user.ws; import com.google.common.collect.Lists; import java.util.List; +import java.util.function.Consumer; import org.junit.Rule; import org.junit.Test; import org.sonar.api.config.MapSettings; @@ -34,6 +35,7 @@ import org.sonar.db.user.UserDto; import org.sonar.db.user.UserGroupDto; import org.sonar.db.user.UserTesting; import org.sonar.server.es.EsTester; +import org.sonar.server.issue.ws.AvatarResolverImpl; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.user.index.UserDoc; import org.sonar.server.user.index.UserIndex; @@ -67,7 +69,7 @@ public class SearchActionTest { private DbSession dbSession = db.getSession(); private UserIndex index = new UserIndex(esTester.client()); private UserIndexer userIndexer = new UserIndexer(dbClient, esTester.client()); - private WsTester ws = new WsTester(new UsersWs(new SearchAction(userSession, index, dbClient))); + private WsTester ws = new WsTester(new UsersWs(new SearchAction(userSession, index, dbClient, new AvatarResolverImpl()))); @Test public void search_json_example() throws Exception { @@ -158,28 +160,28 @@ public class SearchActionTest { assertThat(ws.newGetRequest("api/users", "search").execute().outputAsString()) .contains("login") .contains("name") - .contains("email") + .contains("avatar") .contains("scmAccounts") .doesNotContain("groups"); assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "").execute().outputAsString()) .contains("login") .contains("name") - .contains("email") + .contains("avatar") .contains("scmAccounts") .doesNotContain("groups"); assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "scmAccounts").execute().outputAsString()) .contains("login") .doesNotContain("name") - .doesNotContain("email") + .doesNotContain("avatar") .contains("scmAccounts") .doesNotContain("groups"); assertThat(ws.newGetRequest("api/users", "search").setParam(Param.FIELDS, "groups").execute().outputAsString()) .contains("login") .doesNotContain("name") - .doesNotContain("email") + .doesNotContain("avatar") .doesNotContain("scmAccounts") .doesNotContain("groups"); @@ -189,6 +191,7 @@ public class SearchActionTest { .contains("login") .contains("name") .contains("email") + .contains("avatar") .contains("scmAccounts") .contains("groups"); @@ -196,6 +199,7 @@ public class SearchActionTest { .contains("login") .doesNotContain("name") .doesNotContain("email") + .doesNotContain("avatar") .doesNotContain("scmAccounts") .contains("groups"); } @@ -208,6 +212,57 @@ public class SearchActionTest { ws.newGetRequest("api/users", "search").execute().assertJson(getClass(), "user_with_groups.json"); } + @Test + public void does_not_return_email_when_not_when_system_administer() throws Exception { + loginAsSimpleUser(); + insertUser(user -> user.setLogin("john").setName("John").setEmail("john@email.com")); + + ws.newGetRequest("api/users", "search").execute().assertJson( + "{" + + " \"users\": [" + + " {" + + " \"login\": \"john\"," + + " \"name\": \"John\"," + + " \"avatar\": \"41193cdbffbf06be0cdf231b28c54b18\"" + + " }" + + " ]" + + "}"); + } + + @Test + public void return_email_and_avatar_when_system_administer() throws Exception { + loginAsSystemAdministrator(); + insertUser(user -> user.setLogin("john").setName("John").setEmail("john@email.com")); + + ws.newGetRequest("api/users", "search").execute().assertJson( + "{" + + " \"users\": [" + + " {" + + " \"login\": \"john\"," + + " \"name\": \"John\"," + + " \"email\": \"john@email.com\"," + + " \"avatar\": \"41193cdbffbf06be0cdf231b28c54b18\"" + + " }" + + " ]" + + "}"); + } + + @Test + public void does_not_fail_when_user_has_no_email() throws Exception { + loginAsSystemAdministrator(); + insertUser(user -> user.setLogin("john").setName("John").setEmail(null)); + + ws.newGetRequest("api/users", "search").execute().assertJson( + "{" + + " \"users\": [" + + " {" + + " \"login\": \"john\"," + + " \"name\": \"John\"" + + " }" + + " ]" + + "}"); + } + @Test public void only_return_login_and_name_when_not_logged() throws Exception { userSession.anonymous(); @@ -264,6 +319,12 @@ public class SearchActionTest { return userDtos; } + private UserDto insertUser(Consumer populateUserDto) { + UserDto user = db.users().insertUser(populateUserDto); + userIndexer.indexOnStartup(null); + return user; + } + private void loginAsSystemAdministrator() { userSession.logIn().setSystemAdministrator(); } 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 f336cdabaa6..24b0d526394 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 @@ -24,6 +24,7 @@ import org.junit.Rule; import org.junit.Test; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; +import org.sonar.server.issue.ws.AvatarResolver; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.user.UserUpdater; @@ -46,7 +47,7 @@ public class UsersWsTest { new UpdateAction(mock(UserUpdater.class), userSessionRule, mock(UserJsonWriter.class), mock(DbClient.class)), new CurrentAction(userSessionRule, mock(DbClient.class), mock(DefaultOrganizationProvider.class)), new ChangePasswordAction(mock(DbClient.class), mock(UserUpdater.class), userSessionRule), - new SearchAction(userSessionRule, mock(UserIndex.class), mock(DbClient.class)))); + new SearchAction(userSessionRule, mock(UserIndex.class), mock(DbClient.class), mock(AvatarResolver.class)))); controller = tester.controller("api/users"); } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json index 0f7676f95dc..ff3f77667da 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json @@ -8,7 +8,7 @@ { "login": "user-0", "name": "User 0", - "email": "user-0@mail.com", + "avatar": "7df9eb5cfa0b534812fd8e77c06bdccf", "scmAccounts": [ "user-0" ], @@ -18,7 +18,7 @@ { "login": "user-1", "name": "User 1", - "email": "user-1@mail.com", + "avatar": "071241157c57e21956c73081138ecb6e", "scmAccounts": [ "user-1" ], @@ -28,7 +28,7 @@ { "login": "user-2", "name": "User 2", - "email": "user-2@mail.com", + "avatar": "25c3b9b5609693983e1f9ed80820d7aa", "scmAccounts": [ "user-2" ], @@ -38,7 +38,7 @@ { "login": "user-3", "name": "User 3", - "email": "user-3@mail.com", + "avatar": "61061cc5edb4b771d22509bc47eeaccf", "scmAccounts": [ "user-3" ], @@ -48,7 +48,7 @@ { "login": "user-4", "name": "User 4", - "email": "user-4@mail.com", + "avatar": "07a317a07eb517ddf02bc592491ddf1b", "scmAccounts": [ "user-4" ], diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json index 908dbf0f620..3985d561cb1 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_one.json @@ -6,49 +6,19 @@ }, "users": [ { - "login": "user-0", - "name": "User 0", - "email": "user-0@mail.com", - "scmAccounts": [ - "user-0" - ], - "local": true + "login": "user-0" }, { - "login": "user-1", - "name": "User 1", - "email": "user-1@mail.com", - "scmAccounts": [ - "user-1" - ], - "local": true + "login": "user-1" }, { - "login": "user-2", - "name": "User 2", - "email": "user-2@mail.com", - "scmAccounts": [ - "user-2" - ], - "local": true + "login": "user-2" }, { - "login": "user-3", - "name": "User 3", - "email": "user-3@mail.com", - "scmAccounts": [ - "user-3" - ], - "local": true + "login": "user-3" }, { - "login": "user-4", - "name": "User 4", - "email": "user-4@mail.com", - "scmAccounts": [ - "user-4" - ], - "local": true + "login": "user-4" } ] } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json index 31d17a4591a..8d2c5f7d62d 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/page_two.json @@ -6,49 +6,19 @@ }, "users": [ { - "login": "user-5", - "name": "User 5", - "email": "user-5@mail.com", - "scmAccounts": [ - "user-5" - ], - "local": true + "login": "user-5" }, { - "login": "user-6", - "name": "User 6", - "email": "user-6@mail.com", - "scmAccounts": [ - "user-6" - ], - "local": true + "login": "user-6" }, { - "login": "user-7", - "name": "User 7", - "email": "user-7@mail.com", - "scmAccounts": [ - "user-7" - ], - "local": true + "login": "user-7" }, { - "login": "user-8", - "name": "User 8", - "email": "user-8@mail.com", - "scmAccounts": [ - "user-8" - ], - "local": true + "login": "user-8" }, { - "login": "user-9", - "name": "User 9", - "email": "user-9@mail.com", - "scmAccounts": [ - "user-9" - ], - "local": true + "login": "user-9" } ] } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json index f6d1966de62..383ea295686 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_one.json @@ -8,7 +8,7 @@ { "login": "user-%_%-login", "name": "user-name", - "email": "user@mail.com", + "avatar": "6ad193f57f79ac444c3621370da955e9", "active": true, "scmAccounts": [ "user1" -- 2.39.5