From 2eb7fe5ea6fc38a62485ab2e5b90145acf86f696 Mon Sep 17 00:00:00 2001 From: Thomas Wiese Date: Thu, 9 Apr 2020 16:01:36 +0200 Subject: [PATCH] Bug 4822: /usergroups/readable: passing username by query paramater instead of path parameter to allow null for username to get readable groups of the currently authenticated user --- .../selenium/api/event/UserGroupApi.java | 15 ++++++++++++--- .../selenium/api/test/UserGroupApiTest.java | 17 +++++++++++++---- .../gateway/jaxrs/api/UserGroupsResource.java | 6 +++--- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/event/UserGroupApi.java b/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/event/UserGroupApi.java index 00f9276ccc7..85c7ee10337 100644 --- a/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/event/UserGroupApi.java +++ b/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/event/UserGroupApi.java @@ -23,10 +23,12 @@ public class UserGroupApi { private static final String KEY_ROLES = "roles"; private static final String KEY_FOR_ALL = "forAll"; private static final String KEY_TENANT_GROUP_UUID = "tenantGroup"; + private static final String KEY_READABLE_GROUPS = "readableGroups"; private static final String USERGROUP_URL = "/api/restsecurity/usergroup/"; private static final String USERGROUPS_URL = "/api/v1/usergroups/"; - private static final String SET_DEFAULT_TENANT_FOR_CURRENT_USER = USERGROUPS_URL + "setDefaultTenantForCurrentServerAndUser"; + private static final String SET_DEFAULT_TENANT_FOR_CURRENT_USER = USERGROUPS_URL + + "setDefaultTenantForCurrentServerAndUser"; private static final String ADD_USER_TO_USERGROUP_URL = USERGROUPS_URL + "addAnyUserToGroup"; public UserGroup getUserGroup(ApiContext ctx, UUID groupId) { @@ -58,8 +60,15 @@ public class UserGroupApi { } public Iterable getReadableGroupsOfUser(ApiContext ctx, String userName) { - JSONObject result = ctx.get(USERGROUPS_URL + "readable/" + userName); - return ((JSONArray) result.get("readableGroups")).stream().map(UserGroup::new).collect(Collectors.toList()); + final Map queryParams = new HashMap<>(); + queryParams.put(KEY_USER_NAME, userName); + JSONObject result = ctx.get(USERGROUPS_URL + "readable", queryParams); + return ((JSONArray) result.get(KEY_READABLE_GROUPS)).stream().map(UserGroup::new).collect(Collectors.toList()); + } + + public Iterable getReadableGroupsOfCurrentUser(ApiContext ctx) { + JSONObject result = ctx.get(USERGROUPS_URL + "readable/"); + return ((JSONArray) result.get(KEY_READABLE_GROUPS)).stream().map(UserGroup::new).collect(Collectors.toList()); } public void addRoleToGroup(ApiContext ctx, UUID groupId, UUID roleId, boolean forAll) { diff --git a/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/test/UserGroupApiTest.java b/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/test/UserGroupApiTest.java index 1db8655beea..d39962c22fc 100644 --- a/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/test/UserGroupApiTest.java +++ b/java/com.sap.sailing.selenium.test/src/com/sap/sailing/selenium/api/test/UserGroupApiTest.java @@ -128,9 +128,9 @@ public class UserGroupApiTest extends AbstractSeleniumTest { "password"); assertEquals(1, Util.size(userGroupApi.getReadableGroupsOfUser(user1Ctx, user1Name))); - // admin creates new group and adds user -> does not mean the group is allowed to read + // admin creates new group and adds user -> does not mean the user is allowed to read the group final LongAdder counter = new LongAdder(); - UserGroup group1ToAdd = userGroupApi.createUserGroup(adminSecurityCtx, "group1ToAdd"); + final UserGroup group1ToAdd = userGroupApi.createUserGroup(adminSecurityCtx, "group1ToAdd"); userGroupApi.addUserToGroup(adminSecurityCtx, group1ToAdd.getGroupId(), user1Name); userGroupApi.getReadableGroupsOfUser(user1Ctx, user1Name).forEach(ug -> { if (group1ToAdd.getGroupId().equals(ug.getGroupId())) { @@ -139,15 +139,24 @@ public class UserGroupApiTest extends AbstractSeleniumTest { }); assertEquals(0, counter.intValue()); - // user creates new group + // user1 creates new group -> this group is readable by user1 counter.reset(); - UserGroup group2ToAdd = userGroupApi.createUserGroup(user1SecurityCtx, "group2ToAdd"); + final UserGroup group2ToAdd = userGroupApi.createUserGroup(user1SecurityCtx, "group2ToAdd"); userGroupApi.getReadableGroupsOfUser(user1Ctx, user1Name).forEach(ug -> { if (group2ToAdd.getGroupId().equals(ug.getGroupId())) { counter.increment(); } }); assertEquals(1, counter.intValue()); + + // if passing null for the username, implying the current user, should return the same result + counter.reset(); + userGroupApi.getReadableGroupsOfCurrentUser(user1Ctx).forEach(ug -> { + if (group2ToAdd.getGroupId().equals(ug.getGroupId())) { + counter.increment(); + } + }); + assertEquals(1, counter.intValue()); } @Test diff --git a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/UserGroupsResource.java b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/UserGroupsResource.java index df1bd25ae9a..6bac05adace 100644 --- a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/UserGroupsResource.java +++ b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/UserGroupsResource.java @@ -53,9 +53,9 @@ public class UserGroupsResource extends AbstractSailingServerResource { } @GET - @Path("readable/{userName}") + @Path("readable") @Produces("application/json;charset=UTF-8") - public Response getReadableUserGroupsForCurrentUser(@PathParam("userName") String userName) { + public Response getReadableUserGroupsForCurrentUser(@QueryParam("userName") String userName) { Response response = null; final User user = userName != null ? getSecurityService().getUserByName(userName) : getSecurityService().getCurrentUser(); @@ -63,7 +63,7 @@ public class UserGroupsResource extends AbstractSailingServerResource { final List userGroups = new ArrayList<>(); getSecurityService().getUserGroupList().forEach(ug -> { if (getSecurityService().getSecurityManager().isPermitted( - new SimplePrincipalCollection(userName, userName), + new SimplePrincipalCollection(user.getName(), user.getName()), ug.getIdentifier().getPermission(DefaultActions.READ).toString())) { userGroups.add(ug); }