From 7def9837adf3080deecee4e46cc9c617f430dcc7 Mon Sep 17 00:00:00 2001 From: Roxana Lucanu Date: Tue, 31 Mar 2020 10:59:21 +0300 Subject: [PATCH 1/9] RM-7145 Fix for hasMoreItems --- .../capability/RMAfterInvocationProvider.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java index d9659e312e..f876e1954e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java @@ -563,6 +563,13 @@ public class RMAfterInvocationProvider extends RMSecurityCommon } } + if (maxSize != null) + { + LimitBy limitBy = returnedObject.length() > maxSize ? LimitBy.FINAL_SIZE : LimitBy.UNLIMITED; + filteringResultSet.setResultSetMetaData(new SimpleResultSetMetaData(limitBy, + PermissionEvaluationMode.EAGER, returnedObject.getResultSetMetaData().getSearchParameters())); + } + filteringResultSet.setNumberFound(returnedObject.getNumberFound()); return filteringResultSet; From 94b209e0ea8899678ab3d2dc607ce1b036dd810e Mon Sep 17 00:00:00 2001 From: Tom Page Date: Wed, 1 Apr 2020 15:28:40 +0100 Subject: [PATCH 2/9] RM-7145 Unit tests for processing ResultSet with RMAfterInvocationProviderUnitTest. --- .../capability/RMAfterInvocationProvider.java | 6 +- .../RMAfterInvocationProviderUnitTest.java | 178 ++++++++++++++++++ 2 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProviderUnitTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java index f876e1954e..e651ba7bb8 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java @@ -39,10 +39,10 @@ import java.util.StringTokenizer; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.module.org_alfresco_module_rm.util.AuthenticationUtil; import org.alfresco.repo.search.SimpleResultSetMetaData; import org.alfresco.repo.search.impl.lucene.PagingLuceneResultSet; import org.alfresco.repo.search.impl.querymodel.QueryEngineResults; -import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.permissions.PermissionCheckCollection; import org.alfresco.repo.security.permissions.PermissionCheckValue; import org.alfresco.repo.security.permissions.PermissionCheckedCollection.PermissionCheckedCollectionMixin; @@ -80,8 +80,8 @@ public class RMAfterInvocationProvider extends RMSecurityCommon private static final String AFTER_RM = "AFTER_RM"; + private AuthenticationUtil authenticationUtil; private int maxPermissionChecks; - private long maxPermissionCheckTimeMillis; public boolean supports(ConfigAttribute configAttribute) @@ -147,7 +147,7 @@ public class RMAfterInvocationProvider extends RMSecurityCommon } try { - if (AuthenticationUtil.isRunAsUserTheSystemUser()) + if (authenticationUtil.isRunAsUserTheSystemUser()) { if (logger.isDebugEnabled()) { diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProviderUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProviderUnitTest.java new file mode 100644 index 0000000000..36dc2efeb2 --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProviderUnitTest.java @@ -0,0 +1,178 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.module.org_alfresco_module_rm.capability; + +import static java.util.Arrays.asList; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import java.util.List; + +import net.sf.acegisecurity.Authentication; +import net.sf.acegisecurity.ConfigAttribute; +import net.sf.acegisecurity.ConfigAttributeDefinition; +import org.alfresco.module.org_alfresco_module_rm.util.AuthenticationUtil; +import org.alfresco.repo.security.permissions.impl.acegi.FilteringResultSet; +import org.alfresco.service.cmr.repository.ChildAssociationRef; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.cmr.search.LimitBy; +import org.alfresco.service.cmr.search.ResultSet; +import org.alfresco.service.cmr.search.ResultSetMetaData; +import org.alfresco.service.cmr.search.SearchParameters; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +/** Unit tests for {@link RMAfterInvocationProvider}. */ +public class RMAfterInvocationProviderUnitTest +{ + private static final NodeRef NODE_A = new NodeRef("test://node/a"); + + /** The class under test. */ + @InjectMocks + private RMAfterInvocationProvider rmAfterInvocationProvider; + @Mock + private Authentication authentication; + @Mock + Object object; + @Mock + ConfigAttributeDefinition config; + @Mock + AuthenticationUtil authenticationUtil; + @Mock + NodeService nodeService; + @Mock + ChildAssociationRef childAssocRefA; + + /** Set up the mocks and common test data. */ + @Before + public void setUp() + { + initMocks(this); + + // Set up the nodes and associations. + when(nodeService.exists(NODE_A)).thenReturn(true); + when(childAssocRefA.getParentRef()).thenReturn(NODE_A); + + // Create the config object for use by the tests. + ConfigAttribute configAttribute = mock(ConfigAttribute.class); + when(configAttribute.getAttribute()).thenReturn("AFTER_RM.test"); + List configAttributes = asList(configAttribute); + when(config.getConfigAttributes()).thenReturn(configAttributes.iterator()); + } + + /** Check that when all the results fit into a page then we get a response of "UNLIMITED". */ + @Test + public void testDecide_resultSet_unlimited() + { + // The returned object is a search result set. + ResultSet returnedObject = mock(ResultSet.class); + ResultSetMetaData resultSetMetaData = mock(ResultSetMetaData.class); + when(returnedObject.getResultSetMetaData()).thenReturn(resultSetMetaData); + + // Simulate a single result, and the user has access to it. + when(returnedObject.length()).thenReturn(1); + when(returnedObject.getNumberFound()).thenReturn(1L); + when(returnedObject.getNodeRef(0)).thenReturn(NODE_A); + when(returnedObject.getChildAssocRef(0)).thenReturn(childAssocRefA); + + // Set the page size to 1 and skip count to 0. + SearchParameters searchParameters = mock(SearchParameters.class); + when(searchParameters.getMaxItems()).thenReturn(1); + when(searchParameters.getSkipCount()).thenReturn(0); + when(resultSetMetaData.getSearchParameters()).thenReturn(searchParameters); + + // Call the method under test. + FilteringResultSet filteringResultSet = (FilteringResultSet) rmAfterInvocationProvider.decide(authentication, object, config, returnedObject); + + assertEquals("Expected total of one result.", 1, filteringResultSet.getNumberFound()); + assertEquals("Expected one result returned.", 1, filteringResultSet.length()); + assertEquals("Expected that results were not limited by the page size.", LimitBy.UNLIMITED, filteringResultSet.getResultSetMetaData().getLimitedBy()); + } + + /** Check that results can skipped due to the skip count. */ + @Test + public void testDecide_resultSet_skipped() + { + // The returned object is a search result set. + ResultSet returnedObject = mock(ResultSet.class); + ResultSetMetaData resultSetMetaData = mock(ResultSetMetaData.class); + when(returnedObject.getResultSetMetaData()).thenReturn(resultSetMetaData); + + // Simulate a single result that was skipped due to the skip count. + when(returnedObject.length()).thenReturn(0); + when(returnedObject.getNumberFound()).thenReturn(1L); + + // Set the page size to 1 and skip count to 1 (so the result is skipped). + SearchParameters searchParameters = mock(SearchParameters.class); + when(searchParameters.getMaxItems()).thenReturn(1); + when(searchParameters.getSkipCount()).thenReturn(1); + when(resultSetMetaData.getSearchParameters()).thenReturn(searchParameters); + + // Call the method under test. + FilteringResultSet filteringResultSet = (FilteringResultSet) rmAfterInvocationProvider.decide(authentication, object, config, returnedObject); + + assertEquals("Expected total of one result.", 1, filteringResultSet.getNumberFound()); + assertEquals("Expected no results returned.", 0, filteringResultSet.length()); + assertEquals("Expected that results were not limited by the page size.", LimitBy.UNLIMITED, filteringResultSet.getResultSetMetaData().getLimitedBy()); + } + + /** Check that results can be limited by the page size. */ + @Test + public void testDecide_resultSet_pageSize() + { + // The returned object is a search result set. + ResultSet returnedObject = mock(ResultSet.class); + ResultSetMetaData resultSetMetaData = mock(ResultSetMetaData.class); + when(returnedObject.getResultSetMetaData()).thenReturn(resultSetMetaData); + + // Simulate a single result, and the user has access to it. + when(returnedObject.length()).thenReturn(1); + when(returnedObject.getNumberFound()).thenReturn(1L); + when(returnedObject.getNodeRef(0)).thenReturn(NODE_A); + when(returnedObject.getChildAssocRef(0)).thenReturn(childAssocRefA); + + // Set the page size to 0 and skip count to 0 (so the result is not in page). + SearchParameters searchParameters = mock(SearchParameters.class); + when(searchParameters.getMaxItems()).thenReturn(0); + when(searchParameters.getSkipCount()).thenReturn(0); + when(resultSetMetaData.getSearchParameters()).thenReturn(searchParameters); + + // Call the method under test. + FilteringResultSet filteringResultSet = (FilteringResultSet) rmAfterInvocationProvider.decide(authentication, object, config, returnedObject); + + assertEquals("Expected total of one result.", 1, filteringResultSet.getNumberFound()); + assertEquals("Expected no results returned.", 0, filteringResultSet.length()); + assertEquals("Expected that results were limited by page size.", LimitBy.FINAL_SIZE, filteringResultSet.getResultSetMetaData().getLimitedBy()); + } +} From 98a1485baa8eaf0796e81ea3b400c210d00ce5f4 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Wed, 1 Apr 2020 15:34:25 +0100 Subject: [PATCH 3/9] RM-7145 Wire in RM AuthenticationUtil. --- .../rm-public-services-security-context.xml | 1 + .../capability/RMAfterInvocationProvider.java | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-services-security-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-services-security-context.xml index 6d90759e2b..c9856faeb8 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-services-security-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-public-services-security-context.xml @@ -156,6 +156,7 @@ ${system.acl.maxPermissionChecks} + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java index e651ba7bb8..c6ee235ae4 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java @@ -130,6 +130,16 @@ public class RMAfterInvocationProvider extends RMSecurityCommon this.maxPermissionCheckTimeMillis = maxPermissionCheckTimeMillis; } + /** + * Sets the authentication util + * + * @param authenticationUtil The authentication util to set + */ + public void setAuthenticationUtil(AuthenticationUtil authenticationUtil) + { + this.authenticationUtil = authenticationUtil; + } + @SuppressWarnings("rawtypes") public Object decide(Authentication authentication, Object object, ConfigAttributeDefinition config, Object returnedObject) { From efb33e672316905be084baa5fdef36738c14cffa Mon Sep 17 00:00:00 2001 From: Tom Page Date: Fri, 28 Feb 2020 08:06:16 +0000 Subject: [PATCH 4/9] Merge branch 'feature-2.7/RM-7119_ACS527StartUpFix' into 'release/V2.7' RM-7119 Fix startup with ACS 5.2.7. See merge request records-management/records-management!1393 --- .../RMMethodSecurityPostProcessor.java | 40 ++++---- ...RMMethodSecurityPostProcessorUnitTest.java | 91 +++++++++++++++++++ 2 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java index 41356e0cca..b6eac36746 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java @@ -33,8 +33,11 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import org.alfresco.error.AlfrescoRuntimeException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.PropertyValue; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; @@ -51,7 +54,7 @@ import org.springframework.beans.factory.config.TypedStringValue; */ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { - private static Log logger = LogFactory.getLog(RMMethodSecurityPostProcessor.class); + private static final Logger LOGGER = LoggerFactory.getLogger(RMMethodSecurityPostProcessor.class); public static final String PROP_OBJECT_DEFINITION_SOURCE = "objectDefinitionSource"; public static final String PROPERTY_PREFIX = "rm.methodsecurity."; @@ -94,10 +97,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { if (beanFactory.containsBeanDefinition(bean)) { - if (logger.isDebugEnabled()) - { - logger.debug("Adding RM method security definitions for " + bean); - } + LOGGER.debug("Adding RM method security definitions for {}", bean); BeanDefinition beanDef = beanFactory.getBeanDefinition(bean); PropertyValue beanValue = beanDef.getPropertyValues().getPropertyValue(PROP_OBJECT_DEFINITION_SOURCE); @@ -134,10 +134,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor String securityBeanName = split[index] + SECURITY_BEAN_POSTFIX; if (!securityBeanNameCache.contains(securityBeanName) && beanFactory.containsBean(securityBeanName)) { - if (logger.isDebugEnabled()) - { - logger.debug("Adding " + securityBeanName + " to list from properties."); - } + LOGGER.debug("Adding {} to list from properties.", securityBeanName); securityBeanNameCache.add(securityBeanName); } @@ -166,10 +163,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } else { - if (logger.isWarnEnabled()) - { - logger.warn("Missing RM security definition for method " + key); - } + LOGGER.warn("Missing RM security definition for method {}", key); } } @@ -177,16 +171,28 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } /** - * @param stringValue - * @return + * Convert the lines of a string to a map, separating keys from values by the first "=" sign. + * + * @param stringValue The multi-line string. + * @return The resulting map. + * @throws AlfrescoRuntimeException If a non-blank line does not contain an "=" sign. */ - private Map convertToMap(String stringValue) + protected Map convertToMap(String stringValue) { String[] values = stringValue.trim().split("\n"); Map map = new HashMap(values.length); for (String value : values) { - String[] pair = value.trim().split("="); + String trimmed = value.trim(); + if (trimmed.isEmpty()) + { + continue; + } + String[] pair = trimmed.split("=", 2); + if (pair.length != 2) + { + throw new AlfrescoRuntimeException("Could not convert string to map " + trimmed); + } map.put(pair[0], pair[1]); } return map; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java new file mode 100644 index 0000000000..bb996e6c4d --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java @@ -0,0 +1,91 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.security; + +import static java.util.Collections.emptyMap; + +import static org.junit.Assert.assertEquals; + +import java.util.Map; + +import com.google.common.collect.ImmutableMap; + +import org.alfresco.error.AlfrescoRuntimeException; +import org.junit.Test; + +/** + * Unit tests for {@link RMMethodSecurityPostProcessor}. + * + * See RM-7119. + */ +public class RMMethodSecurityPostProcessorUnitTest +{ + /** The class under test. */ + private RMMethodSecurityPostProcessor rmMethodSecurityPostProcessor = new RMMethodSecurityPostProcessor(); + + @Test + public void testConvertToMap_emptyString() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap(""); + assertEquals("Unexpectedly included empty string in output.", emptyMap(), actual); + } + + @Test + public void testConvertToMap_normalPairs() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\nc=d"); + assertEquals("Failed to handle multiline input string.", ImmutableMap.of("a", "b", "c", "d"), actual); + } + + @Test + public void testConvertToMap_stripWhitespace() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap(" \n \t a=b \n \t "); + assertEquals("Failed to strip whitespace.", ImmutableMap.of("a", "b"), actual); + } + + @Test + public void testConvertToMap_ignoreBlankLine() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\n\nc=d"); + assertEquals("Failed to ignore blank line.", ImmutableMap.of("a", "b", "c", "d"), actual); + } + + @Test + public void testConvertToMap_multipleEquals() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b=c\nd=e=f"); + assertEquals("Issue with handling of = symbol in value.", ImmutableMap.of("a", "b=c", "d", "e=f"), actual); + } + + /** Check that if a line is missing an equals sign then we get an exception. */ + @Test(expected = AlfrescoRuntimeException.class) + public void testConvertToMap_missingEquals() + { + rmMethodSecurityPostProcessor.convertToMap("a=b\ncd"); + } +} From 7e59b6aee0fecd529f3b336ef69981ef8c5dce76 Mon Sep 17 00:00:00 2001 From: Claudia Agache Date: Thu, 2 Apr 2020 16:30:29 +0300 Subject: [PATCH 5/9] Revert "Merge branch 'feature-2.7/RM-7119_ACS527StartUpFix' into 'release/V2.7'" This reverts commit efb33e67 --- .../RMMethodSecurityPostProcessor.java | 40 ++++---- ...RMMethodSecurityPostProcessorUnitTest.java | 91 ------------------- 2 files changed, 17 insertions(+), 114 deletions(-) delete mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java index b6eac36746..41356e0cca 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java @@ -33,11 +33,8 @@ import java.util.Map; import java.util.Properties; import java.util.Set; -import org.alfresco.error.AlfrescoRuntimeException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.PropertyValue; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; @@ -54,7 +51,7 @@ import org.springframework.beans.factory.config.TypedStringValue; */ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { - private static final Logger LOGGER = LoggerFactory.getLogger(RMMethodSecurityPostProcessor.class); + private static Log logger = LogFactory.getLog(RMMethodSecurityPostProcessor.class); public static final String PROP_OBJECT_DEFINITION_SOURCE = "objectDefinitionSource"; public static final String PROPERTY_PREFIX = "rm.methodsecurity."; @@ -97,7 +94,10 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { if (beanFactory.containsBeanDefinition(bean)) { - LOGGER.debug("Adding RM method security definitions for {}", bean); + if (logger.isDebugEnabled()) + { + logger.debug("Adding RM method security definitions for " + bean); + } BeanDefinition beanDef = beanFactory.getBeanDefinition(bean); PropertyValue beanValue = beanDef.getPropertyValues().getPropertyValue(PROP_OBJECT_DEFINITION_SOURCE); @@ -134,7 +134,10 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor String securityBeanName = split[index] + SECURITY_BEAN_POSTFIX; if (!securityBeanNameCache.contains(securityBeanName) && beanFactory.containsBean(securityBeanName)) { - LOGGER.debug("Adding {} to list from properties.", securityBeanName); + if (logger.isDebugEnabled()) + { + logger.debug("Adding " + securityBeanName + " to list from properties."); + } securityBeanNameCache.add(securityBeanName); } @@ -163,7 +166,10 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } else { - LOGGER.warn("Missing RM security definition for method {}", key); + if (logger.isWarnEnabled()) + { + logger.warn("Missing RM security definition for method " + key); + } } } @@ -171,28 +177,16 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } /** - * Convert the lines of a string to a map, separating keys from values by the first "=" sign. - * - * @param stringValue The multi-line string. - * @return The resulting map. - * @throws AlfrescoRuntimeException If a non-blank line does not contain an "=" sign. + * @param stringValue + * @return */ - protected Map convertToMap(String stringValue) + private Map convertToMap(String stringValue) { String[] values = stringValue.trim().split("\n"); Map map = new HashMap(values.length); for (String value : values) { - String trimmed = value.trim(); - if (trimmed.isEmpty()) - { - continue; - } - String[] pair = trimmed.split("=", 2); - if (pair.length != 2) - { - throw new AlfrescoRuntimeException("Could not convert string to map " + trimmed); - } + String[] pair = value.trim().split("="); map.put(pair[0], pair[1]); } return map; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java deleted file mode 100644 index bb996e6c4d..0000000000 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * #%L - * Alfresco Records Management Module - * %% - * Copyright (C) 2005 - 2020 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * - - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ -package org.alfresco.module.org_alfresco_module_rm.security; - -import static java.util.Collections.emptyMap; - -import static org.junit.Assert.assertEquals; - -import java.util.Map; - -import com.google.common.collect.ImmutableMap; - -import org.alfresco.error.AlfrescoRuntimeException; -import org.junit.Test; - -/** - * Unit tests for {@link RMMethodSecurityPostProcessor}. - * - * See RM-7119. - */ -public class RMMethodSecurityPostProcessorUnitTest -{ - /** The class under test. */ - private RMMethodSecurityPostProcessor rmMethodSecurityPostProcessor = new RMMethodSecurityPostProcessor(); - - @Test - public void testConvertToMap_emptyString() - { - Map actual = rmMethodSecurityPostProcessor.convertToMap(""); - assertEquals("Unexpectedly included empty string in output.", emptyMap(), actual); - } - - @Test - public void testConvertToMap_normalPairs() - { - Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\nc=d"); - assertEquals("Failed to handle multiline input string.", ImmutableMap.of("a", "b", "c", "d"), actual); - } - - @Test - public void testConvertToMap_stripWhitespace() - { - Map actual = rmMethodSecurityPostProcessor.convertToMap(" \n \t a=b \n \t "); - assertEquals("Failed to strip whitespace.", ImmutableMap.of("a", "b"), actual); - } - - @Test - public void testConvertToMap_ignoreBlankLine() - { - Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\n\nc=d"); - assertEquals("Failed to ignore blank line.", ImmutableMap.of("a", "b", "c", "d"), actual); - } - - @Test - public void testConvertToMap_multipleEquals() - { - Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b=c\nd=e=f"); - assertEquals("Issue with handling of = symbol in value.", ImmutableMap.of("a", "b=c", "d", "e=f"), actual); - } - - /** Check that if a line is missing an equals sign then we get an exception. */ - @Test(expected = AlfrescoRuntimeException.class) - public void testConvertToMap_missingEquals() - { - rmMethodSecurityPostProcessor.convertToMap("a=b\ncd"); - } -} From a18c22155fd1a7d79f38f7db7cbc78d42489ff11 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Wed, 29 Jan 2020 09:46:04 +0200 Subject: [PATCH 6/9] move the cmis query tests add some additional usecase (cherry picked from commit c20cfb3c6ffcb5ef82c58e61e006627723cf1aed) --- .../rm/community/search/CmisQueryTests.java | 218 ++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java new file mode 100644 index 0000000000..53795fce3f --- /dev/null +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java @@ -0,0 +1,218 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * License rights for this program may be obtained from Alfresco Software, Ltd. + * pursuant to a written agreement and any use of this program without such an + * agreement is prohibited. + * #L% + */ + +package org.alfresco.rest.rm.community.search; + +import static org.alfresco.rest.rm.community.model.user.UserRoles.ROLE_RM_MANAGER; +import static org.alfresco.rest.rm.community.util.CommonTestUtils.generateTestPrefix; +import static org.alfresco.utility.report.log.Step.STEP; +import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertFalse; +import static org.testng.AssertJUnit.assertTrue; + +import org.alfresco.dataprep.ContentActions; +import org.alfresco.rest.rm.community.base.BaseRMRestTest; +import org.alfresco.rest.rm.community.model.recordcategory.RecordCategoryChild; +import org.alfresco.rest.rm.community.model.user.UserPermissions; +import org.alfresco.test.AlfrescoTest; +import org.alfresco.utility.constants.UserRole; +import org.alfresco.utility.model.FileModel; +import org.alfresco.utility.model.FileType; +import org.alfresco.utility.model.SiteModel; +import org.alfresco.utility.model.UserModel; +import org.apache.chemistry.opencmis.client.api.ItemIterable; +import org.apache.chemistry.opencmis.client.api.OperationContext; +import org.apache.chemistry.opencmis.client.api.QueryResult; +import org.apache.chemistry.opencmis.client.runtime.OperationContextImpl; +import org.springframework.beans.factory.annotation.Autowired; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Test to check that RM doesn't break CMIS query + * + * @author jcule, Rodica Sutu + * @since 2.5.4 + * @since 3.3 + */ +public class CmisQueryTests extends BaseRMRestTest +{ + private static final String SEARCH_TERM = generateTestPrefix(CmisQueryTests.class); + private static final String sqlWithName = + "SELECT cmis:name FROM cmis:document where CONTAINS('cmis:name:*" + SEARCH_TERM + "*')"; + + private SiteModel collaborationSite; + private UserModel nonRMUser, rmUser; + private RecordCategoryChild recordFolder; + + @Autowired + private ContentActions contentActions; + + /** + * Create some test data: + *
+     *     - a collaboration site with documents
+     *     - in place records
+     *     - category with folder and records
+     *     - a user with no rm rights (no rights to see the record from file plan)
+     *     - a user with rights to see the records and the other documents created
+     * 
+ */ + @BeforeClass (alwaysRun = true) + public void setupCmisQuery() throws Exception + { + STEP("Create a collaboration site"); + collaborationSite = dataSite.usingAdmin().createPrivateRandomSite(); + + STEP("Create 10 documents ending with SEARCH_TERM"); + for (int i = 0; ++i <= 10; ) + { + FileModel fileModel = new FileModel(String.format("%s.%s", "Doc" + i + SEARCH_TERM, + FileType.TEXT_PLAIN.extention)); + dataContent.usingAdmin().usingSite(collaborationSite).createContent(fileModel); + } + + STEP("Create a collaborator user for the collaboration site"); + nonRMUser = getDataUser().createRandomTestUser(); + getDataUser().addUserToSite(nonRMUser, collaborationSite, UserRole.SiteManager); + + STEP("Create 10 documents and declare as records"); + for (int i = 0; ++i <= 10; ) + { + FileModel fileModel = new FileModel(String.format("%s.%s", "InPlace " + SEARCH_TERM + i, + FileType.TEXT_PLAIN.extention)); + fileModel = dataContent.usingUser(nonRMUser).usingSite(collaborationSite).createContent(fileModel); + getRestAPIFactory().getFilesAPI(nonRMUser).declareAsRecord(fileModel.getNodeRefWithoutVersion()); + } + + STEP("Create record folder and some records "); + recordFolder = createCategoryFolderInFilePlan(); + for (int i = 0; ++i <= 10; ) + { + createElectronicRecord(recordFolder.getId(), "Record " + SEARCH_TERM + i); + } + STEP("Create an rm user with read permission over the category created and contributor role within the " + + "collaboration site"); + rmUser = getDataUser().createRandomTestUser(); + getRestAPIFactory().getRMUserAPI().assignRoleToUser(rmUser.getUsername(), ROLE_RM_MANAGER); + getRestAPIFactory().getRMUserAPI().addUserPermission(recordFolder.getParentId(), rmUser, UserPermissions.PERMISSION_READ_RECORDS); + getDataUser().addUserToSite(rmUser, collaborationSite, UserRole.SiteContributor); + + + //do a cmis query to wait for solr indexing + long currentTime = System.currentTimeMillis(); + long endTime = 0; + do + { + try + { + endTime = System.currentTimeMillis(); + ItemIterable results = contentActions.getCMISSession(getAdminUser().getUsername(), + getAdminUser().getPassword()).query(sqlWithName, false); + assertEquals("Total number of items is not 30, got " + results.getTotalNumItems() + "total items", + 30, results.getTotalNumItems()); + break; + } + catch (AssertionError | Exception e) + { + if (endTime - currentTime > 30000) + { + throw new AssertionError("Maximum retry period reached, test failed.", e); + } + Thread.sleep(5000); + } + } while (true); + } + + /** + *
+     * Given the RM site created
+     * When I execute a cmis query to get all the documents names
+     * Then I get all documents names 100 per page
+     * 
+ */ + @Test + @AlfrescoTest (jira = "MNT-19442") + public void getAllDocumentsNamesCmisQuery() + { + // execute the cmis query + String cq = "SELECT cmis:name FROM cmis:document"; + ItemIterable results = + contentActions.getCMISSession(getAdminUser().getUsername(), getAdminUser().getPassword()).query(cq, + false); + + // check the total number of items is greater than 100 and has more items is true + assertTrue("Has more items not true.", results.getHasMoreItems()); + assertTrue("Total number of items is not greater than 100. Total number of items received" + results.getTotalNumItems(), + results.getTotalNumItems() > 100); + assertEquals("Expected 100 items per page and got " + results.getPageNumItems() + " per page.", 100, + results.getPageNumItems()); + } + + /** + *
+     * Given the RM site created
+     * When I execute a cmis query to get all the documents names with a particular name
+     * Then I get all documents names user has permission
+     * 
+ */ + @Test + @AlfrescoTest (jira = "MNT-19442") + public void getDocumentsWithSpecificNamesCmisQuery() throws Exception + { + // execute the cmis query + ItemIterable results = + contentActions.getCMISSession(nonRMUser.getUsername(), nonRMUser.getPassword()).query(sqlWithName, + false); + assertEquals("Total number of items is not 20, got " + results.getTotalNumItems() + " total items", + 20, results.getTotalNumItems()); + // check the has more items is false + assertFalse("Has more items not false.", results.getHasMoreItems()); + assertEquals("Expected 20 items per page and got " + results.getPageNumItems() + " per page.", 20, + results.getPageNumItems()); + } + + /** + *
+     * Given the RM site created
+     * When I execute a cmis query to get all the documents names with a specific number per page
+     * Then I get all documents names paged as requested that the user has permission
+     * 
+ */ + @Test + @AlfrescoTest (jira = "MNT-19442") + public void getDocumentsCmisQueryWithPagination() throws Exception + { + OperationContext oc = new OperationContextImpl(); + oc.setMaxItemsPerPage(10); + ItemIterable results = + contentActions.getCMISSession(rmUser.getUsername(), rmUser.getPassword()).query(sqlWithName, + false, oc); + + // check the total number of items and has more items is true + assertTrue("Has more items not true. ", results.getHasMoreItems()); + assertEquals("Total number of items is not 30 " + results.getTotalNumItems(), 30, + results.getTotalNumItems()); + assertEquals("Expected 10 items per page and got " + results.getPageNumItems() + " per page.", + 10, results.getPageNumItems()); + } + + @AfterClass + private void clearCmisQueryTests() + { + dataSite.usingAdmin().deleteSite(collaborationSite); + getRestAPIFactory().getRecordCategoryAPI().deleteRecordCategory(recordFolder.getParentId()); + getDataUser().usingAdmin().deleteUser(rmUser); + getDataUser().usingAdmin().deleteUser(nonRMUser); + } +} From 278699f1837015f25fed2d5ba8e264cd1ab2b393 Mon Sep 17 00:00:00 2001 From: Claudia Agache Date: Fri, 3 Apr 2020 09:40:46 +0300 Subject: [PATCH 7/9] fix header --- .../rm/community/search/CmisQueryTests.java | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java index 53795fce3f..33c83389ad 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java @@ -4,9 +4,24 @@ * %% * Copyright (C) 2005 - 2020 Alfresco Software Limited * %% - * License rights for this program may be obtained from Alfresco Software, Ltd. - * pursuant to a written agreement and any use of this program without such an - * agreement is prohibited. + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . * #L% */ From 604fc7785e4fa78a92743a55f115883167b3ddc3 Mon Sep 17 00:00:00 2001 From: Claudia Agache Date: Fri, 3 Apr 2020 14:51:02 +0300 Subject: [PATCH 8/9] code review comments --- .../alfresco/rest/rm/community/search/CmisQueryTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java index 33c83389ad..8be8ff293d 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java @@ -63,7 +63,7 @@ import org.testng.annotations.Test; public class CmisQueryTests extends BaseRMRestTest { private static final String SEARCH_TERM = generateTestPrefix(CmisQueryTests.class); - private static final String sqlWithName = + private static final String SQL_WITH_NAME = "SELECT cmis:name FROM cmis:document where CONTAINS('cmis:name:*" + SEARCH_TERM + "*')"; private SiteModel collaborationSite; @@ -133,7 +133,7 @@ public class CmisQueryTests extends BaseRMRestTest { endTime = System.currentTimeMillis(); ItemIterable results = contentActions.getCMISSession(getAdminUser().getUsername(), - getAdminUser().getPassword()).query(sqlWithName, false); + getAdminUser().getPassword()).query(SQL_WITH_NAME, false); assertEquals("Total number of items is not 30, got " + results.getTotalNumItems() + "total items", 30, results.getTotalNumItems()); break; @@ -187,7 +187,7 @@ public class CmisQueryTests extends BaseRMRestTest { // execute the cmis query ItemIterable results = - contentActions.getCMISSession(nonRMUser.getUsername(), nonRMUser.getPassword()).query(sqlWithName, + contentActions.getCMISSession(nonRMUser.getUsername(), nonRMUser.getPassword()).query(SQL_WITH_NAME, false); assertEquals("Total number of items is not 20, got " + results.getTotalNumItems() + " total items", 20, results.getTotalNumItems()); @@ -211,7 +211,7 @@ public class CmisQueryTests extends BaseRMRestTest OperationContext oc = new OperationContextImpl(); oc.setMaxItemsPerPage(10); ItemIterable results = - contentActions.getCMISSession(rmUser.getUsername(), rmUser.getPassword()).query(sqlWithName, + contentActions.getCMISSession(rmUser.getUsername(), rmUser.getPassword()).query(SQL_WITH_NAME, false, oc); // check the total number of items and has more items is true From d3206cf36326fecddea596ebc94131ee99810d1f Mon Sep 17 00:00:00 2001 From: Claudia Agache Date: Fri, 3 Apr 2020 16:26:03 +0300 Subject: [PATCH 9/9] fix the parameter --- .../org/alfresco/rest/rm/community/search/CmisQueryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java index 8be8ff293d..8624e2d00f 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/search/CmisQueryTests.java @@ -119,7 +119,7 @@ public class CmisQueryTests extends BaseRMRestTest STEP("Create an rm user with read permission over the category created and contributor role within the " + "collaboration site"); rmUser = getDataUser().createRandomTestUser(); - getRestAPIFactory().getRMUserAPI().assignRoleToUser(rmUser.getUsername(), ROLE_RM_MANAGER); + getRestAPIFactory().getRMUserAPI().assignRoleToUser(rmUser.getUsername(), ROLE_RM_MANAGER.roleId); getRestAPIFactory().getRMUserAPI().addUserPermission(recordFolder.getParentId(), rmUser, UserPermissions.PERMISSION_READ_RECORDS); getDataUser().addUserToSite(rmUser, collaborationSite, UserRole.SiteContributor);