RM-999: Hold and Transfers are displayed for ExtendedReaders, ExtendedWriters

* remove in-place roles from the 'all roles' group .. now in-place readers and writers can't gain access to items just because they have a role!
 * patch to remove in-place roles from all group in existing installations
 * unit test for patch



git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@73532 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
Roy Wetherall
2014-06-11 05:31:06 +00:00
parent c1477ad783
commit e6591dd9fe
13 changed files with 325 additions and 52 deletions

View File

@@ -9,7 +9,7 @@
<property name="description" value="RM patch executer"/>
<property name="sinceVersion" value="2.2"/>
<property name="executeOnceOnly" value="false"/>
<property name="moduleSchema" value="1007"/>
<property name="moduleSchema" value="1008"/>
<property name="attributeService" ref="AttributeService" />
<property name="dependsOn">
<list>
@@ -34,7 +34,7 @@
<!-- compatibility beans -->
<!-- @depracted since 2.2 -->
<!-- @deprecated since 2.2 -->
<bean id="rm.baseModulePatch" abstract="true" parent="module.baseComponent" init-method="init">
<property name="retryingTransactionHelper" ref="retryingTransactionHelper"/>
<property name="behaviourFilter" ref="policyBehaviourFilter" />

View File

@@ -81,4 +81,15 @@
<property name="capabilityService" ref="CapabilityService"/>
</bean>
<bean id="rm.removeInPlaceRolesFromAllPatch"
parent="rm.parentModulePatch"
class="org.alfresco.module.org_alfresco_module_rm.patch.v22.RMv22RemoveInPlaceRolesFromAllPatch">
<property name="description" value="Remove in-place roles from 'all roles' group."/>
<property name="fixesToSchema" value="1007"/>
<property name="targetSchema" value="1008"/>
<property name="filePlanService" ref="FilePlanService"/>
<property name="filePlanRoleService" ref="FilePlanRoleService"/>
<property name="authorityService" ref="AuthorityService"/>
</bean>
</beans>

View File

@@ -34,6 +34,7 @@ public enum FilePlanComponentKind
RECORD_FOLDER,
RECORD,
TRANSFER,
TRANSFER_CONTAINER,
HOLD,
HOLD_CONTAINER,
DISPOSITION_SCHEDULE,

View File

@@ -27,6 +27,8 @@ import org.alfresco.service.cmr.repository.NodeRef;
/**
* Freeze Service Interface
*
* TODO should be deprecated and methods moved to the HoldService with "hold, held, etc" style names
*
* @author Roy Wetherall
* @since 2.0
*/

View File

@@ -0,0 +1,97 @@
/*
* Copyright (C) 2005-2014 Alfresco Software Limited.
*
* This file is part of Alfresco
*
* 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 <http://www.gnu.org/licenses/>.
*/
package org.alfresco.module.org_alfresco_module_rm.patch.v22;
import java.util.Set;
import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
import org.alfresco.module.org_alfresco_module_rm.patch.AbstractModulePatch;
import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
import org.alfresco.module.org_alfresco_module_rm.role.Role;
import org.alfresco.service.cmr.repository.NodeRef;
import org.alfresco.service.cmr.security.AuthorityService;
/**
* Removes the in-place groups from the all roles group.
*
* @author Roy Wetherall
* @since 2.2
*/
public class RMv22RemoveInPlaceRolesFromAllPatch extends AbstractModulePatch
{
/** file plan service */
private FilePlanService filePlanService;
/** file plan role service */
private FilePlanRoleService filePlanRoleService;
/** authority service */
private AuthorityService authorityService;
/**
* @param filePlanService file plan service
*/
public void setFilePlanService(FilePlanService filePlanService)
{
this.filePlanService = filePlanService;
}
/**
* @param filePlanRoleService file plan role service
*/
public void setFilePlanRoleService(FilePlanRoleService filePlanRoleService)
{
this.filePlanRoleService = filePlanRoleService;
}
/**
* @param authorityService authority service
*/
public void setAuthorityService(AuthorityService authorityService)
{
this.authorityService = authorityService;
}
/**
* @see org.alfresco.module.org_alfresco_module_rm.patch.AbstractModulePatch#applyInternal()
*/
@Override
public void applyInternal()
{
// get all file plans
Set<NodeRef> filePlans = filePlanService.getFilePlans();
for (NodeRef filePlan : filePlans)
{
Role extendedReaders = filePlanRoleService.getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_READERS);
Role extendedWriters = filePlanRoleService.getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_WRITERS);
// remove extended readers and writers roles from the all roles group
String allRolesGroup = filePlanRoleService.getAllRolesContainerGroup(filePlan);
Set<String> members = authorityService.getContainedAuthorities(null, allRolesGroup, true);
if (members.contains(extendedReaders.getRoleGroupName()))
{
authorityService.removeAuthority(allRolesGroup, extendedReaders.getRoleGroupName());
}
if (members.contains(extendedWriters.getRoleGroupName()))
{
authorityService.removeAuthority(allRolesGroup, extendedWriters.getRoleGroupName());
}
}
}
}

View File

@@ -82,6 +82,12 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
/** Location of bootstrap role JSON */
private static final String BOOTSTRAP_ROLE_JSON_LOCATION = "alfresco/module/org_alfresco_module_rm/security/rm-default-roles-bootstrap.json";
/** JSON names */
private static final String JSON_NAME = "name";
private static final String JSON_DISPLAY_LABEL = "displayLabel";
private static final String JSON_IS_ADMIN = "isAdmin";
private static final String JSON_CAPABILITIES = "capabilities";
/** Capability service */
private CapabilityService capabilityService;
@@ -111,9 +117,7 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
/** Records management role zone */
public static final String RM_ROLE_ZONE_PREFIX = "rmRoleZone";
/**
* Records Management Config Node
*/
/** Records Management Config Node */
private static final String CONFIG_NODEID = "rm_config_folder";
/** Logger */
@@ -286,9 +290,10 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
}
/**
* Bootstraps the default roles
*
* @param rmRootNode
* @param unfiledContainer
* @param filePlan file plan
* @param systemContainers system containers
*/
private void bootstrapDefaultRoles(final NodeRef filePlan, final List<NodeRef> systemContainers)
{
@@ -321,9 +326,9 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
// Get the name of the role
String name = null;
if (object.has("name"))
if (object.has(JSON_NAME))
{
name = object.getString("name");
name = object.getString(JSON_NAME);
if (existsRole(filePlan, name))
{
throw new AlfrescoRuntimeException("The bootstrap role " + name + " already exists on the rm root node " + filePlan.toString());
@@ -337,23 +342,23 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
// Get the role's display label
String displayLabel = name;
if (object.has("displayLabel"))
if (object.has(JSON_DISPLAY_LABEL))
{
displayLabel = object.getString("displayLabel");
displayLabel = object.getString(JSON_DISPLAY_LABEL);
}
// Determine whether the role is an admin role or not
boolean isAdmin = false;
if (object.has("isAdmin"))
if (object.has(JSON_IS_ADMIN))
{
isAdmin = object.getBoolean("isAdmin");
isAdmin = object.getBoolean(JSON_IS_ADMIN);
}
// Get the roles capabilities
Set<Capability> capabilities = new HashSet<Capability>(30);
if (object.has("capabilities"))
if (object.has(JSON_CAPABILITIES))
{
JSONArray arrCaps = object.getJSONArray("capabilities");
JSONArray arrCaps = object.getJSONArray(JSON_CAPABILITIES);
for (int index = 0; index < arrCaps.length(); index++)
{
String capName = arrCaps.getString(index);
@@ -686,23 +691,23 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
/**
* @see org.alfresco.module.org_alfresco_module_rm.security.RecordsManagementSecurityService#createRole(org.alfresco.service.cmr.repository.NodeRef, java.lang.String, java.lang.String, java.util.Set)
*/
public Role createRole(final NodeRef rmRootNode, final String role, final String roleDisplayLabel, final Set<Capability> capabilities)
public Role createRole(final NodeRef filePlan, final String role, final String roleDisplayLabel, final Set<Capability> capabilities)
{
return AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork<Role>()
{
public Role doWork()
{
String fullRoleName = getFullRoleName(role, rmRootNode);
String fullRoleName = getFullRoleName(role, filePlan);
// Check that the role does not already exist for the rm root node
if (authorityService.authorityExists(authorityService.getName(AuthorityType.GROUP, fullRoleName)))
{
throw new AlfrescoRuntimeException("The role " + role + " already exists for root rm node " + rmRootNode.getId());
throw new AlfrescoRuntimeException("The role " + role + " already exists for root rm node " + filePlan.getId());
}
// Create a group that relates to the records management role
Set<String> zones = new HashSet<String>(2);
zones.add(getZoneName(rmRootNode));
zones.add(getZoneName(filePlan));
zones.add(RMAuthority.ZONE_APP_RM);
// Look up string, default to passed value if none found
@@ -714,9 +719,13 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
String roleGroup = authorityService.createAuthority(AuthorityType.GROUP, fullRoleName, groupDisplayLabel, zones);
// do not add system roles to "all"
if (!isSystemRole(role))
{
// Add the roleGroup to the "all" role group
String allRoleGroup = authorityService.getName(AuthorityType.GROUP, getAllRolesGroupShortName(rmRootNode));
String allRoleGroup = authorityService.getName(AuthorityType.GROUP, getAllRolesGroupShortName(filePlan));
authorityService.addAuthority(allRoleGroup, roleGroup);
}
// TODO .. we should be creating a permission set containing all the capabilities and then assigning that
// single permission group to the file plan .. would be tidier
@@ -726,7 +735,7 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
{
for (Capability capability : capabilities)
{
permissionService.setPermission(rmRootNode, roleGroup, capability.getName(), true);
permissionService.setPermission(filePlan, roleGroup, capability.getName(), true);
}
}

View File

@@ -140,7 +140,6 @@ public class AuditLogPost extends BaseAuditRetrievalWebScript
* @param record {@link NodeRef} The audit trail as record
* @return Response text as {@link String}
*/
@SuppressWarnings("null")
private String createResponse(NodeRef record)
{
JSONObject responseJSON = new JSONObject();

View File

@@ -192,7 +192,7 @@ public class FilePlanPermissionServiceImpl extends ServiceBaseImpl
)
public void onCreateTransfer(final ChildAssociationRef childAssocRef)
{
setupPermissions(childAssocRef.getParentRef(), childAssocRef.getChildRef());
setupPermissions(childAssocRef.getParentRef(), childAssocRef.getChildRef(), false);
}
/**
@@ -205,11 +205,22 @@ public class FilePlanPermissionServiceImpl extends ServiceBaseImpl
{
ParameterCheck.mandatory("parent", parent);
ParameterCheck.mandatory("nodeRef", nodeRef);
setupPermissions(parent, nodeRef, true);
}
/**
* Helper method to setup permissions.
*
* @param parent parent node reference
* @param nodeRef child node reference
* @param includeInPlace true if in-place permissions should be included, false otherwise
*/
private void setupPermissions(final NodeRef parent, final NodeRef nodeRef, final boolean includeInPlace)
{
if (nodeService.exists(nodeRef))
{
// initialise permissions
initPermissions(nodeRef);
initPermissions(nodeRef, includeInPlace);
if (nodeService.exists(parent))
{
@@ -357,8 +368,9 @@ public class FilePlanPermissionServiceImpl extends ServiceBaseImpl
* Init the permissions for the given node.
*
* @param nodeRef node reference
* @param includeInPlace true if in-place
*/
private void initPermissions(final NodeRef nodeRef)
private void initPermissions(final NodeRef nodeRef, final boolean includeInPlace)
{
if (nodeService.exists(nodeRef))
{
@@ -372,9 +384,12 @@ public class FilePlanPermissionServiceImpl extends ServiceBaseImpl
// clear all existing permissions
permissionService.clearPermission(nodeRef, null);
if (includeInPlace)
{
// set extended reader permissions
permissionService.setPermission(nodeRef, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS, true);
permissionService.setPermission(nodeRef, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true);
}
// remove owner
ownableService.setOwner(nodeRef, OwnableService.NO_OWNER);

View File

@@ -116,6 +116,10 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte
{
result = FilePlanComponentKind.HOLD;
}
else if (instanceOf(nodeRef, TYPE_TRANSFER_CONTAINER))
{
result = FilePlanComponentKind.TRANSFER_CONTAINER;
}
else if (isTransfer(nodeRef))
{
result = FilePlanComponentKind.TRANSFER;

View File

@@ -0,0 +1,127 @@
/*
* Copyright (C) 2005-2014 Alfresco Software Limited.
*
* This file is part of Alfresco
*
* 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 <http://www.gnu.org/licenses/>.
*/
package org.alfresco.module.org_alfresco_module_rm.patch.v22;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.mock;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
import org.alfresco.module.org_alfresco_module_rm.role.Role;
import org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest;
import org.junit.Test;
import org.mockito.InjectMocks;
/**
* Unit test for remove in-place roles from 'all roles' group patch unit test.
*
* @author Roy Wetherall
* @since 2.2
*/
public class RMv22RemoveInPlaceRolesFromAllPatchUnitTest extends BaseUnitTest
{
/** test data */
private static final String ALL_ROLES = "allroles";
/** patch */
@InjectMocks private RMv22RemoveInPlaceRolesFromAllPatch patch;
/**
* Given there are no file plans to update then the 'all roles' group should not
* be changed.
*/
@Test
public void noFilePlans()
{
// given
doReturn(Collections.EMPTY_SET).when(mockedFilePlanService).getFilePlans();
// when
patch.applyInternal();
// then
verifyZeroInteractions(mockedAuthorityService);
}
/**
* Given that there is one file plan whose 'all roles' group does not contain the
* in-place roles the 'all roles' groups should not be changed.
*/
@Test
public void rolesDontNeedRemovingFromGroup()
{
// given
doReturn(Collections.singleton(filePlan)).when(mockedFilePlanService).getFilePlans();
doReturn(getMockedRole(FilePlanRoleService.ROLE_EXTENDED_READERS)).when(mockedFilePlanRoleService).getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_READERS);
doReturn(getMockedRole(FilePlanRoleService.ROLE_EXTENDED_WRITERS)).when(mockedFilePlanRoleService).getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_WRITERS);
doReturn(ALL_ROLES).when(mockedFilePlanRoleService).getAllRolesContainerGroup(filePlan);
doReturn(Collections.EMPTY_SET).when(mockedAuthorityService).getContainedAuthorities(null, ALL_ROLES, true);
// when
patch.applyInternal();
// then
verify(mockedAuthorityService, times(1)).getContainedAuthorities(null, ALL_ROLES, true);
verifyNoMoreInteractions(mockedAuthorityService);
}
/**
* Given that there is one file plan whose 'all roles' group contains the in-place
* roles then they should be revoved.
*/
@Test
public void removeRolesFromGroup()
{
// given
doReturn(Collections.singleton(filePlan)).when(mockedFilePlanService).getFilePlans();
doReturn(getMockedRole(FilePlanRoleService.ROLE_EXTENDED_READERS)).when(mockedFilePlanRoleService).getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_READERS);
doReturn(getMockedRole(FilePlanRoleService.ROLE_EXTENDED_WRITERS)).when(mockedFilePlanRoleService).getRole(filePlan, FilePlanRoleService.ROLE_EXTENDED_WRITERS);
doReturn(ALL_ROLES).when(mockedFilePlanRoleService).getAllRolesContainerGroup(filePlan);
Set<String> contains = new HashSet<String>(2);
contains.add(FilePlanRoleService.ROLE_EXTENDED_READERS);
contains.add(FilePlanRoleService.ROLE_EXTENDED_WRITERS);
doReturn(contains).when(mockedAuthorityService).getContainedAuthorities(null, ALL_ROLES, true);
// when
patch.applyInternal();
// then
verify(mockedAuthorityService, times(1)).getContainedAuthorities(null, ALL_ROLES, true);
verify(mockedAuthorityService, times(1)).removeAuthority(ALL_ROLES, FilePlanRoleService.ROLE_EXTENDED_READERS);
verify(mockedAuthorityService, times(1)).removeAuthority(ALL_ROLES, FilePlanRoleService.ROLE_EXTENDED_WRITERS);
verifyNoMoreInteractions(mockedAuthorityService);
}
/**
* Helper method to create a mocked role.
*/
private Role getMockedRole(String name)
{
Role mockedRole = mock(Role.class);
doReturn(name).when(mockedRole).getRoleGroupName();
return mockedRole;
}
}

View File

@@ -26,6 +26,7 @@ import org.alfresco.module.org_alfresco_module_rm.job.DispositionLifecycleJobExe
import org.alfresco.module.org_alfresco_module_rm.jscript.app.evaluator.FrozenEvaluatorUnitTest;
import org.alfresco.module.org_alfresco_module_rm.jscript.app.evaluator.TransferEvaluatorUnitTest;
import org.alfresco.module.org_alfresco_module_rm.model.compatibility.DictionaryBootstrapPostProcessorUnitTest;
import org.alfresco.module.org_alfresco_module_rm.patch.v22.RMv22RemoveInPlaceRolesFromAllPatchUnitTest;
import org.alfresco.module.org_alfresco_module_rm.record.RecordMetadataBootstrapUnitTest;
import org.alfresco.module.org_alfresco_module_rm.record.RecordServiceImplUnitTest;
import org.alfresco.module.org_alfresco_module_rm.script.hold.HoldPostUnitTest;
@@ -70,7 +71,10 @@ import org.junit.runners.Suite.SuiteClasses;
HoldCapabilityConditionUnitTest.class,
// action implementations
FileReportActionUnitTest.class
FileReportActionUnitTest.class,
// patches
RMv22RemoveInPlaceRolesFromAllPatchUnitTest.class
})
public class AllUnitTestSuite
{

View File

@@ -38,6 +38,7 @@ import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
import org.alfresco.module.org_alfresco_module_rm.report.ReportService;
import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
import org.alfresco.repo.transaction.RetryingTransactionHelper;
@@ -48,6 +49,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
import org.alfresco.service.cmr.repository.NodeService;
import org.alfresco.service.cmr.repository.StoreRef;
import org.alfresco.service.cmr.search.SearchService;
import org.alfresco.service.cmr.security.AuthorityService;
import org.alfresco.service.cmr.security.OwnableService;
import org.alfresco.service.cmr.security.PermissionService;
import org.alfresco.service.namespace.NamespaceService;
@@ -91,6 +93,7 @@ public class BaseUnitTest implements RecordsManagementModel
@Mock(name="ownableService") protected OwnableService mockedOwnableService;
@Mock(name="searchService") protected SearchService mockedSearchService;
@Mock(name="retryingTransactionHelper") protected RetryingTransactionHelper mockedRetryingTransactionHelper;
@Mock(name="authorityService") protected AuthorityService mockedAuthorityService;
/** rm service mocks */
@Mock(name="filePlanService") protected FilePlanService mockedFilePlanService;
@@ -99,6 +102,7 @@ public class BaseUnitTest implements RecordsManagementModel
@Mock(name="holdService") protected HoldService mockedHoldService;
@Mock(name="recordsManagementActionService") protected RecordsManagementActionService mockedRecordsManagementActionService;
@Mock(name="reportService") protected ReportService mockedReportService;
@Mock(name="filePlanRoleService") protected FilePlanRoleService mockedFilePlanRoleService;
/** application context mock */
@Mock(name="applicationContext") protected ApplicationContext mockedApplicationContext;