Pull request #125: [EDELIVERY-13944] Permission issue on private resource

Merge in EDELIVERY/smp from bugfix/EDELIVERY-13944-not-possible-to-get-a-private-resource-subresource to development

* commit '5a045308':
  [EDELIVERY-13944] Permission issue on private resource

smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/ResourceGuard.java

@@ -36,13 +36,17 @@ import eu.europa.ec.edelivery.smp.logging.SMPLoggerFactory;
 import eu.europa.ec.edelivery.smp.servlet.ResourceAction;
 import org.springframework.stereotype.Service;
 
+import java.util.Collections;
+
 /**
  * Service implements logic if user can activate action on the resource
  */
 
 @Service
 public class ResourceGuard {
+
     private static final SMPLogger LOG = SMPLoggerFactory.getLogger(ResourceGuard.class);
+    private static final String LOG_NOT_LOGGED_IN = "Anonymous users are not permitted to execute action [{}]!";
     DomainMemberDao domainMemberDao;
     GroupMemberDao groupMemberDao;
     ResourceMemberDao resourceMemberDao;
@@ -97,12 +101,23 @@ public class ResourceGuard {
         DBDomain domain = group.getDomain();
         DBUser dbuser = user == null ? null : user.getUser();
         // if domain is internal check if user is member of domain, or any internal resources, groups
+
+        if (resource.getVisibility() == VisibilityType.PRIVATE ) {
+            LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to read private resource [{}]", user, resource);
+            return dbuser!=null && resourceMemberDao.isUserResourceMember(dbuser, resource);
+        }
+
+        if (group.getVisibility() == VisibilityType.PRIVATE) {
+            LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to read public resource in a private group [{}]", user, group);
+            return dbuser!=null &&  (groupMemberDao.isUserGroupMember(dbuser, Collections.singletonList(group)) ||
+                    resourceMemberDao.isUserAnyGroupResourceMember(dbuser, group));
+        }
+
         if ((resource.getVisibility() == null || domain.getVisibility() == VisibilityType.PRIVATE)
                 && (dbuser == null ||
                 !(domainMemberDao.isUserDomainMember(dbuser, domain)
                         || groupMemberDao.isUserAnyDomainGroupResourceMember(dbuser, domain)
-                        || resourceMemberDao.isUserAnyDomainResourceMember(dbuser, domain)))
-        ) {
+                        || resourceMemberDao.isUserAnyDomainResourceMember(dbuser, domain)))) {
             LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is not authorized to read internal domain [{}] resources", user, domain);
             return false;
         }
@@ -131,7 +146,7 @@ public class ResourceGuard {
     public boolean canUpdate(SMPUserDetails user, DBResource resource) {
         LOG.info(SMPLogger.SECURITY_MARKER, "User [{}] is trying to update resource [{}]", user, resource);
         if (user == null || user.getUser() == null) {
-            LOG.warn("Not user is logged in!");
+            LOG.warn(LOG_NOT_LOGGED_IN, "UPDATE");
             return false;
         }
         // only resource member with admin rights can update resource
@@ -147,7 +162,7 @@ public class ResourceGuard {
     public boolean canCreate(SMPUserDetails user, DBResource resource, DBDomain domain) {
         LOG.info(SMPLogger.SECURITY_MARKER, "User [{}] is trying to create resource [{}]", user, resource);
         if (user == null || user.getUser() == null) {
-            LOG.warn("Not user is logged in!");
+            LOG.warn(LOG_NOT_LOGGED_IN, "CREATE");
             return false;
         }
         return groupMemberDao.isUserAnyDomainGroupResourceMemberWithRole(user.getUser(),
@@ -160,20 +175,20 @@ public class ResourceGuard {
         LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to delete resource [{}]", user, resource);
         // same as for create
         if (user == null || user.getUser() == null) {
-            LOG.warn("Not user is logged in!");
+            LOG.warn(LOG_NOT_LOGGED_IN, "DELETE");
             return false;
         }
         return canCreate(user, resource, domain);
     }
 
     public boolean canDelete(SMPUserDetails user, DBSubresource subresource) {
-        LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to delete resource [{}]", user, subresource);
+        LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to delete subresource [{}]", user, subresource);
         // Subresource can be created by the resource admin, the same as for update
         return canUpdate(user, subresource);
     }
 
     public boolean canCreateUpdate(SMPUserDetails user, DBSubresource subresource) {
-        LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to delete resource [{}]", user, subresource);
+        LOG.debug(SMPLogger.SECURITY_MARKER, "User [{}] is trying to create/update subresource [{}]", user, subresource);
         // Subresource can be created by the resource admin, the same as for update
         return canUpdate(user, subresource);
     }

smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/controllers/ResourceControllerSubResourceTest.java

@@ -18,6 +18,7 @@
  */
 package eu.europa.ec.edelivery.smp.controllers;
 
+import eu.europa.ec.edelivery.smp.data.enums.VisibilityType;
 import eu.europa.ec.edelivery.smp.servlet.WebConstants;
 import eu.europa.ec.edelivery.smp.ui.AbstractControllerTest;
 import org.junit.jupiter.api.BeforeEach;
@@ -49,10 +50,58 @@ public class ResourceControllerSubResourceTest extends AbstractControllerTest {
 
 
     @BeforeEach
-    public void setup() throws IOException {
+    public void setupController() throws IOException {
         super.setup();
     }
 
+
+    /**
+     * Test get permissions for resource with different creation parameters. The user data match
+     * the data in the database: webapp_integration_test_data.sql
+     */
+    @ParameterizedTest
+    @CsvSource({"'Get with same user as resource admin: OK', 200, pat_smp_admin, 123456, pat_smp_admin, 123456, ''",
+            "'Non resource memeber is trying to get it ', 401, pat_smp_admin, 123456, test_pat_hashed_pass, 123456,''",
+            "'Resource member with bad password, bad credentials: Fail', 401, pat_smp_admin, 123456, pat_smp_admin, 000000,''",
+            "'Set same Owner as admin user: OK', 200, pat_smp_admin, 123456, pat_smp_admin, 123456,'pat_smp_admin'",
+            "'Set resource owner user: OK', 200, test_pat_hashed_pass, 123456, test_pat_hashed_pass, 123456,'test_user_hashed_pass'",
+            "'Legacy: Set owner user, but default admin owner deletes: OK', 200, test_pat_hashed_pass, 123456, pat_smp_admin, 123456, 'test_pat_hashed_pass'",
+    })
+    void getPrivateSubResourcePermissions(String desc, int expectedStatus,
+                                      String resourceAdminCreateATId, String resourceCreateATSecret,
+                                      String getUserATId, String getUserPassword,
+                                      String resourceOwnerId) throws Exception {
+        LOG.info(desc);
+
+        String xmlSG = getSampleServiceGroupBody(IDENTIFIER_SCHEME, PARTICIPANT_ID);
+        String xmlMD = generateServiceMetadata(PARTICIPANT_ID, IDENTIFIER_SCHEME, DOCUMENT_ID, DOCUMENT_SCHEME, "test");
+        // owner headers
+        HttpHeaders httpHeaders = new HttpHeaders();
+        httpHeaders.add(WebConstants.HTTP_PARAM_RESOURCE_VISIBILITY, VisibilityType.PRIVATE.name());
+        if (StringUtils.isNotBlank(resourceOwnerId)) {
+            httpHeaders.add(WebConstants.HTTP_PARAM_OWNER, resourceOwnerId);
+        }
+        // crate service group
+        mvc.perform(put(URL_PATH)
+                        .with(ADMIN_CREDENTIALS)
+                        .headers(httpHeaders)
+                        .contentType(APPLICATION_XML_VALUE)
+                        .content(xmlSG))
+                .andExpect(status().isCreated());
+        // add subresource/service-metadata with appropriate owner
+        mvc.perform(put(URL_DOC_PATH)
+                        .with(httpBasic(resourceAdminCreateATId, resourceCreateATSecret))
+                        .contentType(APPLICATION_XML_VALUE)
+                        .content(xmlMD))
+                .andExpect(status().isCreated());
+
+        // get subresource/service-metadata with test owner
+        mvc.perform(get(URL_DOC_PATH)
+                        .with(httpBasic(getUserATId, getUserPassword)))
+                .andExpect(status().is(expectedStatus));
+    }
+
+
     /**
      * Test update permissions for resource with different creation parameters. The user data match
      * the data in the database: webapp_integration_test_data.sql