Pull request #143: [EDELIVERY-13937] fix group authorization for the delete resource

Merge in EDELIVERY/smp from bugfix/EDELIVERY-13937-delete to development

* commit '0d35e8d8':
  [EDELIVERY-13937] fix group authorization for the delete resource

smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/DomainGuard.java → smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/DomainGroupGuard.java

@@ -49,8 +49,8 @@ import java.util.stream.Collectors;
  * @author Joze RIHTARSIC
  */
 @Component
-public class DomainGuard {
-    private static final SMPLogger LOG = SMPLoggerFactory.getLogger(DomainGuard.class);
+public class DomainGroupGuard {
+    private static final SMPLogger LOG = SMPLoggerFactory.getLogger(DomainGroupGuard.class);
     public static final String NOT_DEFINED = "Not defined";
 
     final DomainResolverService domainResolverService;
@@ -59,11 +59,11 @@ public class DomainGuard {
     final GroupMemberDao groupMemberDao;
     final ResourceMemberDao resourceMemberDao;
 
-    public DomainGuard(DomainResolverService domainResolverService,
-                       DomainMemberDao domainMemberDao,
-                       GroupMemberDao groupMemberDao,
-                       ResourceMemberDao resourceMemberDao,
-                       GroupDao groupDao) {
+    public DomainGroupGuard(DomainResolverService domainResolverService,
+                            DomainMemberDao domainMemberDao,
+                            GroupMemberDao groupMemberDao,
+                            ResourceMemberDao resourceMemberDao,
+                            GroupDao groupDao) {
         this.domainResolverService = domainResolverService;
         this.domainMemberDao = domainMemberDao;
         this.groupMemberDao = groupMemberDao;
@@ -297,9 +297,11 @@ public class DomainGuard {
             LOG.warn(SMPLogger.SECURITY_MARKER, "Anonymous user [{}] is not authorized to delete resources on groups [{}]", userInfo, groupsInfo);
             return false;
         }
-        // allow only group admins to create/delete resources
-        boolean isAuthorized = groupMemberDao.isUserGroupMember(user.getUser(), groups)
-                || resourceMemberDao.isUserAnyGroupsResourceMember(user.getUser(), groups);
+        // allow only group admins to delete resources
+        List<Long> groupIds = groups.stream().map(DBGroup::getId).collect(Collectors.toList());
+        boolean isAuthorized =
+                resourceMemberDao.isUserAnyGroupsResourceMemberWithRole(userId, groupIds, MembershipRoleType.ADMIN)
+                        || groupMemberDao.isUserGroupMemberWithRole(userId, groupIds, MembershipRoleType.ADMIN);
         LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is authorized: [{}] to delete resources from groups [{}]", userInfo, isAuthorized, groupsInfo);
         return isAuthorized;
     }

smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/services/resource/ResourceResolverService.java

@@ -34,6 +34,7 @@ import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException;
 import eu.europa.ec.edelivery.smp.identifiers.Identifier;
 import eu.europa.ec.edelivery.smp.logging.SMPLogger;
 import eu.europa.ec.edelivery.smp.logging.SMPLoggerFactory;
+import eu.europa.ec.edelivery.smp.security.DomainGroupGuard;
 import eu.europa.ec.edelivery.smp.security.ResourceGuard;
 import eu.europa.ec.edelivery.smp.services.ConfigurationService;
 import eu.europa.ec.edelivery.smp.services.IdentifierService;
@@ -44,6 +45,7 @@ import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -65,6 +67,7 @@ public class ResourceResolverService {
     private static final SMPLogger LOG = SMPLoggerFactory.getLogger(ResourceResolverService.class);
 
     final ResourceGuard resourceGuard;
+    final DomainGroupGuard domainGroupGuard;
     final ConfigurationService configurationService;
     final IdentifierService identifierService;
     final DomainDao domainDao;
@@ -76,6 +79,7 @@ public class ResourceResolverService {
 
 
     public ResourceResolverService(ResourceGuard resourceGuard,
+                                      DomainGroupGuard domainGroupGuard,
                                    ConfigurationService configurationService,
                                    IdentifierService identifierService,
                                    DomainDao domainDao,
@@ -94,6 +98,7 @@ public class ResourceResolverService {
         this.resourceDefinitionDao = resourceDefinitionDao;
         this.resourceDao = resourceDao;
         this.subresourceDao = subresourceDao;
+        this.domainGroupGuard = domainGroupGuard;
     }
 
     @Transactional
@@ -144,6 +149,18 @@ public class ResourceResolverService {
                     trimToNull(resourceRequest.getResourceGroupParameter()));
             resource.setVisibility(resourceRequest.getResourceVisibilityParameter());
             resource.setGroup(group);
+        } else {
+            // initially the GroupGuard checked for all groups on the domain
+            // but now recheck if the user is authorized for the group
+            if (!domainGroupGuard.isUserAuthorizedForGroup(Collections.singletonList(resource.getGroup()),
+                    user, resourceRequest.getAction())) {
+
+                LOG.warn(SECURITY_MARKER, "User [{}] is NOT authorized for action [{}] on group [{}] in domain [{}]",
+                        resourceRequest.getAction(),
+                        getUsername(user),
+                        resource.getGroup().getGroupName(), domain.getDomainCode());
+                throw new SMPRuntimeException(ErrorCode.UNAUTHORIZED);
+            }
         }
 
         locationVector.setResource(resource);

smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/security/DomainGuardTest.java → smp-server-library/src/test/java/eu/europa/ec/edelivery/smp/security/DomainGroupGuardTest.java

@@ -37,10 +37,10 @@ import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.when;
 
-class DomainGuardTest extends AbstractJunit5BaseDao {
+class DomainGroupGuardTest extends AbstractJunit5BaseDao {
 
     @Autowired
-    DomainGuard testInstance;
+    DomainGroupGuard testInstance;
 
     ResourceRequest resourceRequest = Mockito.mock(ResourceRequest.class);
     SMPUserDetails userDetails = Mockito.mock(SMPUserDetails.class);
@@ -132,7 +132,7 @@ class DomainGuardTest extends AbstractJunit5BaseDao {
     }
 
     @Test
-    void testCanReadPrivateDomainAnonimous() {
+    void testCanReadPrivateDomainAnonymous() {
         DBDomain domain = Mockito.mock(DBDomain.class);
         when(domain.getVisibility()).thenReturn(VisibilityType.PRIVATE);
         when(userDetails.getUser()).thenReturn(null);

smp-webapp/src/main/java/eu/europa/ec/edelivery/smp/controllers/ResourceController.java

@@ -22,7 +22,7 @@ import eu.europa.ec.edelivery.smp.auth.SMPUserDetails;
 import eu.europa.ec.edelivery.smp.exceptions.SMPRuntimeException;
 import eu.europa.ec.edelivery.smp.logging.SMPLogger;
 import eu.europa.ec.edelivery.smp.logging.SMPLoggerFactory;
-import eu.europa.ec.edelivery.smp.security.DomainGuard;
+import eu.europa.ec.edelivery.smp.security.DomainGroupGuard;
 import eu.europa.ec.edelivery.smp.services.resource.ResourceService;
 import eu.europa.ec.edelivery.smp.servlet.ResourceAction;
 import eu.europa.ec.edelivery.smp.servlet.ResourceRequest;
@@ -74,10 +74,10 @@ public class ResourceController {
             lowerCase(HTTP_PARAM_RESOURCE_VISIBILITY),
             lowerCase(HTTP_PARAM_RESOURCE_TYPE));
     final ResourceService resourceService;
-    final DomainGuard domainGuard;
+    final DomainGroupGuard domainGuard;
 
 
-    public ResourceController(ResourceService resourceLocatorService, DomainGuard domainGuard) {
+    public ResourceController(ResourceService resourceLocatorService, DomainGroupGuard domainGuard) {
         this.resourceService = resourceLocatorService;
         this.domainGuard = domainGuard;
     }