From 8c870f8c340fa5eba60832ed229f26c26100c259 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Wed, 24 May 2006 07:13:56 +0000 Subject: [PATCH] Fixed AR-598: Restore not working for non-admin user Fixed AR-579: Node ownership not changing for archived and restored nodes git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@2964 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/model/systemModel.xml | 4 + .../java/org/alfresco/model/ContentModel.java | 1 + .../node/archive/ArchiveAndRestoreTest.java | 103 +++++++++++------- .../repo/node/db/DbNodeServiceImpl.java | 27 ++++- 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/config/alfresco/model/systemModel.xml b/config/alfresco/model/systemModel.xml index b30ed8c1ec..1408b50f04 100644 --- a/config/alfresco/model/systemModel.xml +++ b/config/alfresco/model/systemModel.xml @@ -139,6 +139,10 @@ d:datetime true + + d:text + true + diff --git a/source/java/org/alfresco/model/ContentModel.java b/source/java/org/alfresco/model/ContentModel.java index 8dfdc291ec..56cdd9afdb 100644 --- a/source/java/org/alfresco/model/ContentModel.java +++ b/source/java/org/alfresco/model/ContentModel.java @@ -44,6 +44,7 @@ public interface ContentModel static final QName PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedOriginalParentAssoc"); static final QName PROP_ARCHIVED_BY = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedBy"); static final QName PROP_ARCHIVED_DATE = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedDate"); + static final QName PROP_ARCHIVED_ORIGINAL_OWNER = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedOriginalOwner"); static final QName ASPECT_ARCHIVED_ASSOCS = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archived-assocs"); static final QName PROP_ARCHIVED_PARENT_ASSOCS = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedParentAssocs"); static final QName PROP_ARCHIVED_CHILD_ASSOCS = QName.createQName(NamespaceService.SYSTEM_MODEL_1_0_URI, "archivedChildAssocs"); diff --git a/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java b/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java index ca65ea6679..489b65a72f 100644 --- a/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java +++ b/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java @@ -37,7 +37,9 @@ 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.repository.StoreRef; +import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.AuthenticationService; +import org.alfresco.service.cmr.security.OwnableService; import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; @@ -70,6 +72,7 @@ public class ArchiveAndRestoreTest extends TestCase private PermissionService permissionService; private AuthenticationComponent authenticationComponent; private AuthenticationService authenticationService; + private OwnableService ownableService; private TransactionService transactionService; private UserTransaction txn; @@ -104,6 +107,7 @@ public class ArchiveAndRestoreTest extends TestCase permissionService = serviceRegistry.getPermissionService(); authenticationService = serviceRegistry.getAuthenticationService(); authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent"); + ownableService = (OwnableService) ctx.getBean("ownableService"); transactionService = serviceRegistry.getTransactionService(); // Start a transaction @@ -138,12 +142,6 @@ public class ArchiveAndRestoreTest extends TestCase PermissionService.ALL_PERMISSIONS, true); - // grant everyone rights to the archive store - permissionService.setPermission( - archiveStoreRootNodeRef, - PermissionService.ALL_AUTHORITIES, - PermissionService.ALL_PERMISSIONS, - true); } finally { @@ -322,6 +320,17 @@ public class ArchiveAndRestoreTest extends TestCase assertEquals("Mapping of archived store is not correct", archiveStoreRootNodeRef, archiveNodeRef); } + public void testArchivedAspect() throws Exception + { + // delete 'a' + nodeService.deleteNode(a); + // check that it has the aspect and that the properties are correct + assertTrue("Archived aspect not present", nodeService.hasAspect(a_, ContentModel.ASPECT_ARCHIVED)); + Map properties = nodeService.getProperties(a_); + assertNotNull("Original owner property not present", properties.get(ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER)); + assertEquals("Original owner property is incorrect", USER_A, properties.get(ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER)); + } + public void testArchiveAndRestoreNodeBB() throws Exception { // delete a child @@ -568,6 +577,10 @@ public class ArchiveAndRestoreTest extends TestCase nodeService.deleteNode(b); commitAndBeginNewTransaction(); + // check that archived nodes are visible + verifyNodeExistence(a_, true); + verifyNodeExistence(b_, true); + nodeArchiveService.purgeAllArchivedNodes(workStoreRef); commitAndBeginNewTransaction(); @@ -581,35 +594,51 @@ public class ArchiveAndRestoreTest extends TestCase verifyNodeExistence(aa_, false); verifyNodeExistence(bb_, false); } -// -// public void testPermissionsForRestore() throws Exception -// { -// // user A deletes 'a' -// authenticationService.authenticate(USER_A, USER_A.toCharArray()); -// nodeService.deleteNode(a); -// // user B deletes 'b' -// authenticationService.authenticate(USER_B, USER_B.toCharArray()); -// nodeService.deleteNode(b); -// -// // user B can't see archived 'a' -// List restoredByB = nodeArchiveService.restoreAllArchivedNodes(workStoreRef); -// assertEquals("User B should not have seen A's delete", 1, restoredByB.size()); -// } -// -// /** -// * Deny the current user the rights to write to the destination location -// * and ensure that the use-case is handled properly. -// */ -// public void testPermissionsLackingOnDestination() throws Exception -// { -// // remove 'b', deny permissions to workspace root and attempt a restore -// nodeService.deleteNode(b); -// permissionService.setPermission(workStoreRootNodeRef, USER_B, PermissionService.ADD_CHILDREN, false); -// commitAndBeginNewTransaction(); -// -// // the restore of b should fail for user B -// authenticationService.authenticate(USER_B, USER_B.toCharArray()); -// RestoreNodeReport report = nodeArchiveService.restoreArchivedNode(b_); -// assertEquals("Expected permission denied status", RestoreStatus.FAILURE_PERMISSION, report.getStatus()); -// } + + public void testDeletedOwnership() throws Exception + { + // check that A is the current owner of 'b' + String bOwner = ownableService.getOwner(b); + assertEquals("User A must own 'b'", USER_A, bOwner); + // user B deletes 'b' + authenticationService.authenticate(USER_B, USER_B.toCharArray()); + nodeService.deleteNode(b); + // check that B is the owner of 'b_' + String b_Owner = ownableService.getOwner(b_); + assertEquals("User B must own 'b_'", USER_B, b_Owner); + } + + /** + * Check that node ownership changes correctly + */ + public void testPermissionsForRestore() throws Exception + { + // user A deletes 'a' + authenticationService.authenticate(USER_A, USER_A.toCharArray()); + nodeService.deleteNode(a); + // user B deletes 'b' + authenticationService.authenticate(USER_B, USER_B.toCharArray()); + nodeService.deleteNode(b); + + // user B can't see archived 'a' + List restoredByB = nodeArchiveService.restoreAllArchivedNodes(workStoreRef); + assertEquals("User B should be able to see only B's delete", 1, restoredByB.size()); + } + + /** + * Deny the current user the rights to write to the destination location + * and ensure that the use-case is handled properly. + */ + public void testPermissionsLackingOnDestination() throws Exception + { + // remove 'b', deny permissions to workspace root and attempt a restore + nodeService.deleteNode(b); + permissionService.setPermission(workStoreRootNodeRef, USER_B, PermissionService.ADD_CHILDREN, false); + commitAndBeginNewTransaction(); + + // the restore of b should fail for user B + authenticationService.authenticate(USER_B, USER_B.toCharArray()); + RestoreNodeReport report = nodeArchiveService.restoreArchivedNode(b_); + assertEquals("Expected permission denied status", RestoreStatus.FAILURE_PERMISSION, report.getStatus()); + } } diff --git a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java index add21c98f5..97278c4072 100644 --- a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java +++ b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java @@ -1310,7 +1310,8 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl ChildAssoc primaryParentAssoc = nodeDaoService.getPrimaryParentAssoc(node); // add the aspect - node.getAspects().add(ContentModel.ASPECT_ARCHIVED); + Set aspects = node.getAspects(); + aspects.add(ContentModel.ASPECT_ARCHIVED); Map properties = node.getProperties(); PropertyValue archivedByProperty = makePropertyValue( dictionaryService.getProperty(ContentModel.PROP_ARCHIVED_BY), @@ -1324,6 +1325,21 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl dictionaryService.getProperty(ContentModel.PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC), primaryParentAssoc.getChildAssocRef()); properties.put(ContentModel.PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC, archivedPrimaryParentNodeRefProperty); + PropertyValue originalOwnerProperty = properties.get(ContentModel.PROP_OWNER); + PropertyValue originalCreatorProperty = properties.get(ContentModel.PROP_CREATOR); + if (originalOwnerProperty != null || originalCreatorProperty != null) + { + properties.put( + ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER, + originalOwnerProperty != null ? originalOwnerProperty : originalCreatorProperty); + } + + // change the node ownership + aspects.add(ContentModel.ASPECT_OWNABLE); + PropertyValue newOwnerProperty = makePropertyValue( + dictionaryService.getProperty(ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER), + AuthenticationUtil.getCurrentUserName()); + properties.put(ContentModel.PROP_OWNER, newOwnerProperty); // move the node NodeRef archiveStoreRootNodeRef = getRootNode(archiveStoreRef); @@ -1557,11 +1573,20 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl ChildAssociationRef originalPrimaryParentAssocRef = (ChildAssociationRef) makeSerializableValue( dictionaryService.getProperty(ContentModel.PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC), properties.get(ContentModel.PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC)); + PropertyValue originalOwnerProperty = properties.get(ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER); // remove the aspect archived aspect aspects.remove(ContentModel.ASPECT_ARCHIVED); properties.remove(ContentModel.PROP_ARCHIVED_ORIGINAL_PARENT_ASSOC); properties.remove(ContentModel.PROP_ARCHIVED_BY); properties.remove(ContentModel.PROP_ARCHIVED_DATE); + properties.remove(ContentModel.PROP_ARCHIVED_ORIGINAL_OWNER); + + // restore the original ownership + if (originalOwnerProperty != null) + { + aspects.add(ContentModel.ASPECT_OWNABLE); + properties.put(ContentModel.PROP_OWNER, originalOwnerProperty); + } if (destinationParentNodeRef == null) {