diff --git a/gms-ui/src/components/MembersPanel.vue b/gms-ui/src/components/MembersPanel.vue
index 34af9ce145a42062404fd6dfcea7f10aeb933fce..335d23d85902ddb77901d0f295cae69c601b499d 100644
--- a/gms-ui/src/components/MembersPanel.vue
+++ b/gms-ui/src/components/MembersPanel.vue
@@ -1,5 +1,5 @@
<template>
-<b-tab title="Members" v-if="model.permission === 'ADMIN' || model.permission === 'MANAGE_MEMBERS' || model.permission === 'VIEW_MEMBERS'">
+<b-tab title="Members" :title-link-class="{ 'd-none': (model.permission === 'TRAVERSE') }">
<div v-if="model.membersPanel !== null">
<b-list-group v-for="member in model.membersPanel.items" id="members-list" v-bind:key="member.memberId">
<b-list-group-item href="#" @click.prevent="openUser(member)">
diff --git a/gms-ui/src/components/PermissionsPanel.vue b/gms-ui/src/components/PermissionsPanel.vue
index 8a30a8c42b294b8032896872e8bffdc28e2be30a..c6631b857c27be3925217b1628d9e9f33099e448 100644
--- a/gms-ui/src/components/PermissionsPanel.vue
+++ b/gms-ui/src/components/PermissionsPanel.vue
@@ -1,5 +1,5 @@
<template>
-<b-tab title="Permissions" v-if="model.permission === 'ADMIN'">
+<b-tab title="Permissions" :title-link-class="{ 'd-none': (model.permission !== 'ADMIN') }">
<div v-if="model.permissionsPanel !== null">
<table class="table b-table table-striped table-hover">
<thead>
diff --git a/gms/src/main/java/it/inaf/ia2/gms/controller/MembersController.java b/gms/src/main/java/it/inaf/ia2/gms/controller/MembersController.java
index 792e2d521e5e141437181aad7b7ce5cfa2b7b51f..f31966fc7c63369f672c1a745c4c310c9eae9691 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/controller/MembersController.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/controller/MembersController.java
@@ -4,7 +4,6 @@ import it.inaf.ia2.gms.manager.MembershipManager;
import it.inaf.ia2.gms.manager.PermissionsManager;
import it.inaf.ia2.gms.model.request.AddMemberRequest;
import it.inaf.ia2.gms.model.response.PaginatedData;
-import it.inaf.ia2.gms.model.Permission;
import it.inaf.ia2.gms.model.RapUser;
import it.inaf.ia2.gms.model.request.PaginatedModelRequest;
import it.inaf.ia2.gms.model.request.RemoveMemberRequest;
@@ -52,16 +51,7 @@ public class MembersController {
GroupEntity group = groupsService.getGroupById(request.getGroupId());
membershipManager.addMember(group, request.getUserId());
-
- Permission currentUserPermission = permissionsManager.getCurrentUserPermission(group);
-
- if (currentUserPermission == Permission.MANAGE_MEMBERS) {
- // Automatically assign the VIEW_MEMBERS permission ("Add collaborator" feature)
- permissionsManager.addPermission(group, request.getUserId(), Permission.VIEW_MEMBERS);
- } else if (request.getPermission() != null) {
- // Admin users can specify a permission
- permissionsManager.addPermission(group, request.getUserId(), request.getPermission());
- }
+ permissionsManager.addPermission(group, request.getUserId(), request.getPermission());
return new ResponseEntity<>(getMembersPanel(group, request), HttpStatus.CREATED);
}
@@ -73,20 +63,7 @@ public class MembersController {
membershipManager.removeMember(group, request.getUserId());
- Permission currentUserPermission = permissionsManager.getCurrentUserPermission(group);
-
- // For users having the MANAGE_MEMBERS permission, the VIEW_MEMBERS permission
- // is automatically assigned when they add a member ("Add collaborator" feature).
- // We want to keep also the reverse behavior.
- // If the member permission is not VIEW_MEMBERS that means that it has been
- // changed by an ADMIN user, so we don't remove it.
- boolean removeCollaborator = currentUserPermission == Permission.MANAGE_MEMBERS
- && permissionsManager.getUserPermission(group, request.getUserId()) == Permission.VIEW_MEMBERS;
-
- // ADMIN users can choose if delete also the permission or not.
- boolean adminRemovePermission = currentUserPermission == Permission.ADMIN && request.isRemoveAlsoPermission();
-
- if (removeCollaborator || adminRemovePermission) {
+ if (request.isRemoveAlsoPermission()) {
permissionsManager.removePermission(group, request.getUserId());
}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/manager/PermissionsManager.java b/gms/src/main/java/it/inaf/ia2/gms/manager/PermissionsManager.java
index 238797c65ab6282fa6d11917dec90a31b298a50b..1a22842e280843cc7782167080beae1457ca57a6 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/manager/PermissionsManager.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/manager/PermissionsManager.java
@@ -15,6 +15,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
@@ -72,15 +73,18 @@ public class PermissionsManager extends UserAwareComponent {
return null;
}
- public Permission getUserPermission(GroupEntity group, String userId) {
- verifyUserCanManagePermissions(group);
- List<PermissionEntity> permissions = permissionsService.findUserPermissions(group, userId);
- return PermissionUtils.getGroupPermission(group, permissions).orElse(null);
- }
-
public PermissionEntity addPermission(GroupEntity group, String userId, Permission permission) {
- verifyUserCanManagePermissions(group);
- return permissionsService.addPermission(group, userId, permission);
+
+ Permission currentUserPermission = getCurrentUserPermission(group);
+
+ if (currentUserPermission == Permission.MANAGE_MEMBERS && permission == Permission.VIEW_MEMBERS) {
+ // Automatically assign the VIEW_MEMBERS permission ("Add collaborator" feature)
+ return permissionsService.addPermission(group, userId, Permission.VIEW_MEMBERS);
+ } else if (currentUserPermission == Permission.ADMIN) {
+ // Admin users can specify a permission
+ return permissionsService.addPermission(group, userId, permission);
+ }
+ throw unauthorizedExceptionSupplier(group).get();
}
public PermissionEntity updatePermission(GroupEntity group, String userId, Permission permission) {
@@ -89,18 +93,49 @@ public class PermissionsManager extends UserAwareComponent {
}
public void removePermission(GroupEntity group, String userId) {
- verifyUserCanManagePermissions(group);
- permissionsService.removePermission(group, userId);
+
+ Permission currentUserPermission = getCurrentUserPermission(group);
+
+ // For users having the MANAGE_MEMBERS permission, the VIEW_MEMBERS permission
+ // is automatically assigned when they add a member ("Add collaborator" feature).
+ // We want to keep also the reverse behavior.
+ if (currentUserPermission == Permission.MANAGE_MEMBERS) {
+ if (getUserPermission(group, userId, false) == Permission.VIEW_MEMBERS) {
+ permissionsService.removePermission(group, userId);
+ }
+ // If the member permission is not VIEW_MEMBERS that means that it has been
+ // changed by an ADMIN user, so we don't remove it.
+ } else if (currentUserPermission == Permission.ADMIN) {
+ permissionsService.removePermission(group, userId);
+ } else {
+ throw unauthorizedExceptionSupplier(group).get();
+ }
+ }
+
+ public Permission getUserPermission(GroupEntity group, String userId) {
+ return getUserPermission(group, userId, true);
+ }
+
+ private Permission getUserPermission(GroupEntity group, String userId, boolean verify) {
+ if (verify) {
+ verifyUserCanManagePermissions(group);
+ }
+ List<PermissionEntity> permissions = permissionsService.findUserPermissions(group, userId);
+ return PermissionUtils.getGroupPermission(group, permissions).orElse(null);
}
private void verifyUserCanManagePermissions(GroupEntity group) {
Permission permission = getCurrentUserPermission(group);
if (permission != Permission.ADMIN) {
- loggingDAO.logAction("Unauthorized attempt to manage permissions");
- throw new UnauthorizedException("Only admin users can handle permissions");
+ throw unauthorizedExceptionSupplier(group).get();
}
}
+ private Supplier<UnauthorizedException> unauthorizedExceptionSupplier(GroupEntity group) {
+ loggingDAO.logAction("Unauthorized attempt to manage permissions [group_id=" + group.getId() + "]");
+ return () -> new UnauthorizedException("You don't have the privileges for managing the requested permission");
+ }
+
public List<PermissionEntity> getCurrentUserPermissions(GroupEntity group) {
return permissionsService.findUserPermissions(group, getCurrentUserId());
}
diff --git a/gms/src/main/java/it/inaf/ia2/gms/model/request/AddMemberRequest.java b/gms/src/main/java/it/inaf/ia2/gms/model/request/AddMemberRequest.java
index 14d24eda556cb6a3624c51de867e51bbd7117493..d514da063eaa0c2dc3e60ff70c7272679577f511 100644
--- a/gms/src/main/java/it/inaf/ia2/gms/model/request/AddMemberRequest.java
+++ b/gms/src/main/java/it/inaf/ia2/gms/model/request/AddMemberRequest.java
@@ -1,14 +1,11 @@
package it.inaf.ia2.gms.model.request;
import it.inaf.ia2.gms.model.Permission;
+import javax.validation.constraints.NotNull;
public class AddMemberRequest extends MemberRequest {
- /**
- * When adding a member it is possible to assign also a permission. This
- * field can be null (in that case the user is member of the group but
- * he/she can't do nothing (not even seeing him/her group membership).
- */
+ @NotNull
private Permission permission;
public Permission getPermission() {
diff --git a/gms/src/test/java/it/inaf/ia2/gms/controller/MembersControllerTest.java b/gms/src/test/java/it/inaf/ia2/gms/controller/MembersControllerTest.java
index fdf853c33e1ddba6b17e6e99a19716c6c784e110..be3de511601ad8e360b5f297749f2fba681f4d83 100644
--- a/gms/src/test/java/it/inaf/ia2/gms/controller/MembersControllerTest.java
+++ b/gms/src/test/java/it/inaf/ia2/gms/controller/MembersControllerTest.java
@@ -1,7 +1,6 @@
package it.inaf.ia2.gms.controller;
import com.fasterxml.jackson.databind.ObjectMapper;
-import it.inaf.ia2.gms.authn.SessionData;
import it.inaf.ia2.gms.manager.MembershipManager;
import it.inaf.ia2.gms.manager.PermissionsManager;
import it.inaf.ia2.gms.model.request.AddMemberRequest;
@@ -57,7 +56,6 @@ public class MembersControllerTest {
group.setPath("parent_id.group_id");
when(groupsService.getGroupById(eq("group_id"))).thenReturn(group);
- when(permissionsManager.getCurrentUserPermission(eq(group))).thenReturn(Permission.ADMIN);
}
@Test
@@ -68,10 +66,11 @@ public class MembersControllerTest {
request.setUserId("user_id");
request.setPaginatorPage(1);
request.setPaginatorPageSize(10);
+ request.setPermission(Permission.VIEW_MEMBERS);
mockMvc.perform(post("/member")
.content(mapper.writeValueAsString(request))
- .contentType(MediaType.APPLICATION_JSON_UTF8))
+ .contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.currentPage", is(1)));
diff --git a/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerIntegrationTest.java b/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerIntegrationTest.java
index e5508ee7a2a6fd9d19bf7ebe34a0178095f54c88..9a25bc6eaf540ce6f9465fdfe44374f014eb56cd 100644
--- a/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerIntegrationTest.java
+++ b/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerIntegrationTest.java
@@ -1,7 +1,6 @@
package it.inaf.ia2.gms.manager;
import it.inaf.ia2.gms.DataSourceConfig;
-import it.inaf.ia2.gms.authn.RapPrincipal;
import it.inaf.ia2.gms.model.Permission;
import it.inaf.ia2.gms.model.RapUser;
import it.inaf.ia2.gms.model.UserPermission;
@@ -13,9 +12,7 @@ import it.inaf.ia2.gms.persistence.model.PermissionEntity;
import it.inaf.ia2.gms.rap.RapClient;
import it.inaf.ia2.gms.service.PermissionsService;
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import javax.sql.DataSource;
import static org.junit.Assert.assertEquals;
diff --git a/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerTest.java b/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..e1459f0e72206a3ad52e69b11d7331b09b37c0b4
--- /dev/null
+++ b/gms/src/test/java/it/inaf/ia2/gms/manager/PermissionsManagerTest.java
@@ -0,0 +1,155 @@
+package it.inaf.ia2.gms.manager;
+
+import it.inaf.ia2.gms.exception.UnauthorizedException;
+import it.inaf.ia2.gms.model.Permission;
+import it.inaf.ia2.gms.persistence.LoggingDAO;
+import it.inaf.ia2.gms.persistence.model.GroupEntity;
+import it.inaf.ia2.gms.persistence.model.PermissionEntity;
+import it.inaf.ia2.gms.service.PermissionsService;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class PermissionsManagerTest {
+
+ private static final String USER_ID = "USER_ID";
+ private static final String TARGET_USER_ID = "TARGET_USER_ID";
+
+ @Mock
+ private PermissionsService permissionsService;
+ @Mock
+ private HttpServletRequest request;
+ @Mock
+ private LoggingDAO loggingDAO;
+
+ @InjectMocks
+ private PermissionsManager permissionsManager;
+
+ @Before
+ public void setUp() {
+ UserAwareComponentTestUtil.setUser(permissionsManager, USER_ID);
+ }
+
+ @Test
+ public void addPermissionFromManager() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.MANAGE_MEMBERS));
+
+ permissionsManager.addPermission(group, TARGET_USER_ID, Permission.VIEW_MEMBERS);
+
+ verify(permissionsService, times(1)).addPermission(eq(group), eq(TARGET_USER_ID), eq(Permission.VIEW_MEMBERS));
+ }
+
+ @Test
+ public void addPermissionFromAdmin() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.ADMIN));
+
+ permissionsManager.addPermission(group, TARGET_USER_ID, Permission.MANAGE_MEMBERS);
+
+ verify(permissionsService, times(1)).addPermission(eq(group), eq(TARGET_USER_ID), eq(Permission.MANAGE_MEMBERS));
+ }
+
+ @Test(expected = UnauthorizedException.class)
+ public void addPermissionFromUnauthorized() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(new ArrayList<>());
+
+ permissionsManager.addPermission(group, TARGET_USER_ID, Permission.MANAGE_MEMBERS);
+ }
+
+ @Test
+ public void removePermissionFromManager() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.MANAGE_MEMBERS));
+
+ when(permissionsService.findUserPermissions(eq(group), eq(TARGET_USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.VIEW_MEMBERS));
+
+ permissionsManager.removePermission(group, TARGET_USER_ID);
+
+ verify(permissionsService, times(1)).removePermission(eq(group), eq(TARGET_USER_ID));
+ }
+
+ @Test
+ public void removePermissionFromManagerAddedByAdmin() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.MANAGE_MEMBERS));
+
+ when(permissionsService.findUserPermissions(eq(group), eq(TARGET_USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.MANAGE_MEMBERS));
+
+ permissionsManager.removePermission(group, TARGET_USER_ID);
+
+ verify(permissionsService, never()).removePermission(any(), any());
+ }
+
+ @Test
+ public void removePermissionFromAdmin() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.ADMIN));
+
+ permissionsManager.removePermission(group, TARGET_USER_ID);
+
+ verify(permissionsService, times(1)).removePermission(eq(group), eq(TARGET_USER_ID));
+ }
+
+ @Test(expected = UnauthorizedException.class)
+ public void removePermissionFromUnauthorized() {
+
+ GroupEntity group = getGroup();
+
+ when(permissionsService.findUserPermissions(eq(group), eq(USER_ID)))
+ .thenReturn(getUserPermissions(group, Permission.VIEW_MEMBERS));
+
+ permissionsManager.removePermission(group, TARGET_USER_ID);
+ }
+
+ private List<PermissionEntity> getUserPermissions(GroupEntity group, Permission permission) {
+ PermissionEntity entity = new PermissionEntity();
+ entity.setPermission(permission);
+ entity.setGroupId(group.getId());
+ entity.setGroupPath(group.getPath());
+ entity.setUserId(USER_ID);
+ return Collections.singletonList(entity);
+ }
+
+ private GroupEntity getGroup() {
+ GroupEntity group = new GroupEntity();
+ group.setId("group_id");
+ group.setPath("parent_id.group_id");
+ return group;
+ }
+}