Merged HEAD (5.2) to 5.2.N (5.2.1)

126530 jkaabimofrad: Merged FILE-FOLDER-API (5.2.0) to HEAD (5.2)
      123162 jvonka: ACE-5113: Platform - OwnableService - when setting owner check that username currently exists (at time of property update)


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@126874 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/ownable-services-context.xml

@@ -24,5 +24,8 @@
         <property name="renditionService">
             <ref bean="renditionService"/>
         </property>
+        <property name="personService">
+            <ref bean="personService"/>
+        </property>
     </bean>
 </beans>

source/java/org/alfresco/repo/ownable/impl/OwnableServiceImpl.java

@@ -51,7 +51,9 @@ import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
 import org.alfresco.service.cmr.security.AuthenticationService;
+import org.alfresco.service.cmr.security.NoSuchPersonException;
 import org.alfresco.service.cmr.security.OwnableService;
+import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.EqualsHelper;
 import org.alfresco.util.PropertyCheck;
@@ -76,6 +78,7 @@ public class OwnableServiceImpl implements
     private TenantService tenantService;
     private Set<String> storesToIgnorePolicies = Collections.emptySet();
     private RenditionService renditionService;
+    private PersonService personService;
 
     public OwnableServiceImpl()
     {
@@ -127,6 +130,11 @@ public class OwnableServiceImpl implements
         this.renditionService = renditionService;
     }
 
+    public void setPersonService(PersonService personService)
+    {
+        this.personService = personService;
+    }
+
     public void afterPropertiesSet() throws Exception
     {
         PropertyCheck.mandatory(this, "nodeService", nodeService);
@@ -134,6 +142,7 @@ public class OwnableServiceImpl implements
         PropertyCheck.mandatory(this, "nodeOwnerCache", nodeOwnerCache);
         PropertyCheck.mandatory(this, "policyComponent", policyComponent);
         PropertyCheck.mandatory(this, "renditionService", renditionService);
+        PropertyCheck.mandatory(this, "personService", personService);
     }
     
     public void init()
@@ -278,6 +287,25 @@ public class OwnableServiceImpl implements
 
         if (!EqualsHelper.nullSafeEquals(pb, pa))
         {
+            if (pa != null)
+            {
+                String username = (String) pa;
+                NodeRef personNodeRef = null;
+
+                // validate that user authentication exists
+                if (authenticationService.authenticationExists(username))
+                {
+                    // validate that person exists
+                    // note: will attempt to create missing person, if allowed - may throw NoSuchPersonException
+                    personNodeRef = personService.getPerson(username, true);
+                }
+
+                if (personNodeRef == null)
+                {
+                    throw new NoSuchPersonException(username);
+                }
+            }
+
             nodeOwnerCache.remove(nodeRef);
             return;
         }

source/test-java/org/alfresco/repo/ownable/impl/OwnableServiceTest.java

@@ -45,6 +45,7 @@ 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.MutableAuthenticationService;
+import org.alfresco.service.cmr.security.NoSuchPersonException;
 import org.alfresco.service.cmr.security.OwnableService;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.QName;
@@ -59,6 +60,10 @@ public class OwnableServiceTest extends TestCase
 {
     private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext();
     
+    private static final String USER_ANDY = "andy";
+    private static final String USER_WOOF = "woof";
+    private static final String USER_MUPPET = "muppet";
+
     private NodeService nodeService;
 
     private MutableAuthenticationService authenticationService;
@@ -114,11 +119,9 @@ public class OwnableServiceTest extends TestCase
         rootNodeRef = nodeService.getRootNode(storeRef);
         permissionService.setPermission(rootNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.ADD_CHILDREN, true);
 
-        if(authenticationDAO.userExists("andy"))
+        reCreateUser(USER_ANDY, USER_ANDY);
-        {
+        reCreateUser(USER_WOOF, USER_WOOF);
-            authenticationService.deleteAuthentication("andy");
+        reCreateUser(USER_MUPPET, USER_MUPPET);
-        }
-        authenticationService.createAuthentication("andy", "andy".toCharArray());
 
         dynamicAuthority = new OwnerDynamicAuthority();
         dynamicAuthority.setOwnableService(ownableService);
@@ -126,6 +129,15 @@ public class OwnableServiceTest extends TestCase
         authenticationComponent.clearCurrentSecurityContext();
     }
 
+    private void reCreateUser(String username, String password)
+    {
+        if(authenticationDAO.userExists(username))
+        {
+            authenticationService.deleteAuthentication(username);
+        }
+        authenticationService.createAuthentication(username, password.toCharArray());
+    }
+    
     @Override
     protected void tearDown() throws Exception
     {
@@ -156,22 +168,22 @@ public class OwnableServiceTest extends TestCase
     
     public void testCMObject()
     {
-        authenticationService.authenticate("andy", "andy".toCharArray());
+        authenticationService.authenticate(USER_ANDY, USER_ANDY.toCharArray());
         NodeRef testNode = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_PERSON, ContentModel.TYPE_CMOBJECT, null).getChildRef();
-        permissionService.setPermission(rootNodeRef, "andy", PermissionService.TAKE_OWNERSHIP, true);
+        permissionService.setPermission(rootNodeRef, USER_ANDY, PermissionService.TAKE_OWNERSHIP, true);
-        assertEquals("andy", ownableService.getOwner(testNode));
+        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
         assertTrue(ownableService.hasOwner(testNode));
         assertTrue(nodeService.hasAspect(testNode, ContentModel.ASPECT_AUDITABLE));
         assertFalse(nodeService.hasAspect(testNode, ContentModel.ASPECT_OWNABLE));
-        assertTrue(dynamicAuthority.hasAuthority(testNode, "andy"));
+        assertTrue(dynamicAuthority.hasAuthority(testNode, USER_ANDY));
         
-        assertEquals("andy", ownableService.getOwner(testNode));
+        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
         
-//        nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, "woof");
+//        nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, USER_WOOF);
-//        assertEquals("woof", ownableService.getOwner(testNode));
+//        assertEquals(USER_WOOF, ownableService.getOwner(testNode));
 //        
-//        nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, "andy");
+//        nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, USER_ANDY);
-//        assertEquals("andy", ownableService.getOwner(testNode));
+//        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
 //        
         permissionService.setInheritParentPermissions(testNode, false);
         
@@ -181,7 +193,7 @@ public class OwnableServiceTest extends TestCase
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(testNode, PermissionService.TAKE_OWNERSHIP));
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(testNode, PermissionService.SET_OWNER));
         
-        permissionService.setPermission(rootNodeRef, "andy", PermissionService.WRITE_PROPERTIES, true);
+        permissionService.setPermission(rootNodeRef, USER_ANDY, PermissionService.WRITE_PROPERTIES, true);
         
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(rootNodeRef, PermissionService.TAKE_OWNERSHIP));
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(rootNodeRef, PermissionService.SET_OWNER));
@@ -190,23 +202,23 @@ public class OwnableServiceTest extends TestCase
         
        
         
-        ownableService.setOwner(testNode, "woof");
+        ownableService.setOwner(testNode, USER_WOOF);
-        assertEquals("woof", ownableService.getOwner(testNode));
+        assertEquals(USER_WOOF, ownableService.getOwner(testNode));
-        assertTrue(dynamicAuthority.hasAuthority(testNode, "woof"));
+        assertTrue(dynamicAuthority.hasAuthority(testNode, USER_WOOF));
         assertEquals(AccessStatus.DENIED, permissionService.hasPermission(testNode, PermissionService.TAKE_OWNERSHIP));
         assertEquals(AccessStatus.DENIED, permissionService.hasPermission(testNode, PermissionService.SET_OWNER));
         
         
-        ownableService.setOwner(testNode, "muppet");
+        ownableService.setOwner(testNode, USER_MUPPET);
-        assertEquals("muppet", ownableService.getOwner(testNode));
+        assertEquals(USER_MUPPET, ownableService.getOwner(testNode));
-        assertTrue(dynamicAuthority.hasAuthority(testNode, "muppet"));
+        assertTrue(dynamicAuthority.hasAuthority(testNode, USER_MUPPET));
         assertEquals(AccessStatus.DENIED, permissionService.hasPermission(testNode, PermissionService.TAKE_OWNERSHIP));
         assertEquals(AccessStatus.DENIED, permissionService.hasPermission(testNode, PermissionService.SET_OWNER));
         
         
         ownableService.takeOwnership(testNode);
-        assertEquals("andy", ownableService.getOwner(testNode));
+        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
-        assertTrue(dynamicAuthority.hasAuthority(testNode, "andy"));
+        assertTrue(dynamicAuthority.hasAuthority(testNode, USER_ANDY));
         assertTrue(nodeService.hasAspect(testNode, ContentModel.ASPECT_AUDITABLE));
         assertTrue(nodeService.hasAspect(testNode, ContentModel.ASPECT_OWNABLE));
       
@@ -215,41 +227,50 @@ public class OwnableServiceTest extends TestCase
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(testNode, PermissionService.TAKE_OWNERSHIP));
         assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(testNode, PermissionService.SET_OWNER));
         
-        nodeService.setProperty(testNode, ContentModel.PROP_OWNER, "muppet");
+        nodeService.setProperty(testNode, ContentModel.PROP_OWNER, USER_MUPPET);
-        assertEquals("muppet", ownableService.getOwner(testNode));
+        assertEquals(USER_MUPPET, ownableService.getOwner(testNode));
         nodeService.removeAspect(testNode, ContentModel.ASPECT_OWNABLE);
-        assertEquals("andy", ownableService.getOwner(testNode));
+        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
         
         HashMap<QName, Serializable> aspectProperties = new HashMap<QName, Serializable>();
-        aspectProperties.put(ContentModel.PROP_OWNER, "muppet");
+        aspectProperties.put(ContentModel.PROP_OWNER, USER_MUPPET);
         nodeService.addAspect(testNode, ContentModel.ASPECT_OWNABLE, aspectProperties);
-        assertEquals("muppet", ownableService.getOwner(testNode));
+        assertEquals(USER_MUPPET, ownableService.getOwner(testNode));
-        
 
+        // -ve test
+        try
+        {
+            ownableService.setOwner(testNode, "unknownuserdoesnotexist");
+            fail("Unexpected - should not be able to set owner as a non-existent user");
+        }
+        catch (NoSuchPersonException nspe)
+        {
+            // ignore
+        }
     }
     
     public void testContainer()
     {  
-        authenticationService.authenticate("andy", "andy".toCharArray());
+        authenticationService.authenticate(USER_ANDY, USER_ANDY.toCharArray());
         NodeRef testNode = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, ContentModel.TYPE_PERSON, ContentModel.TYPE_CONTAINER, null).getChildRef();
         assertNull(ownableService.getOwner(testNode));
         assertFalse(ownableService.hasOwner(testNode));
         assertFalse(nodeService.hasAspect(testNode, ContentModel.ASPECT_AUDITABLE));
         assertFalse(nodeService.hasAspect(testNode, ContentModel.ASPECT_OWNABLE));
-        assertFalse(dynamicAuthority.hasAuthority(testNode, "andy"));
+        assertFalse(dynamicAuthority.hasAuthority(testNode, USER_ANDY));
         
         assertFalse(permissionService.hasPermission(testNode, PermissionService.READ) == AccessStatus.ALLOWED);
         assertFalse(permissionService.hasPermission(testNode, permissionService.getAllPermission()) == AccessStatus.ALLOWED);
         
         permissionService.setPermission(rootNodeRef, permissionService.getOwnerAuthority(), permissionService.getAllPermission(), true);
         
-        ownableService.setOwner(testNode, "muppet");
+        ownableService.setOwner(testNode, USER_MUPPET);
-        assertEquals("muppet", ownableService.getOwner(testNode));
+        assertEquals(USER_MUPPET, ownableService.getOwner(testNode));
         ownableService.takeOwnership(testNode);
-        assertEquals("andy", ownableService.getOwner(testNode));
+        assertEquals(USER_ANDY, ownableService.getOwner(testNode));
         assertFalse(nodeService.hasAspect(testNode, ContentModel.ASPECT_AUDITABLE));
         assertTrue(nodeService.hasAspect(testNode, ContentModel.ASPECT_OWNABLE));
-        assertTrue(dynamicAuthority.hasAuthority(testNode, "andy"));
+        assertTrue(dynamicAuthority.hasAuthority(testNode, USER_ANDY));
         
         assertTrue(permissionService.hasPermission(testNode, PermissionService.READ) == AccessStatus.ALLOWED);
         assertTrue(permissionService.hasPermission(testNode, permissionService.getAllPermission())== AccessStatus.ALLOWED);