diff --git a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/ResourceGuard.java b/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/ResourceGuard.java index ddb816aec16a52e9b42637e753f4abec12312ad0..2bc138e2ae6245fffc0468744a87d852ab754289 100644 --- a/smp-server-library/src/main/java/eu/europa/ec/edelivery/smp/security/ResourceGuard.java +++ b/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); } diff --git a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/controllers/ResourceControllerSubResourceTest.java b/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/controllers/ResourceControllerSubResourceTest.java index 5c881147a7863b585914d785a88be9db2cd2b141..4d8da09bf69d7403555dcb24b3879ae64798235e 100644 --- a/smp-webapp/src/test/java/eu/europa/ec/edelivery/smp/controllers/ResourceControllerSubResourceTest.java +++ b/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