diff --git a/config/alfresco/ownable-services-context.xml b/config/alfresco/ownable-services-context.xml index ba81a91c10..f003733745 100644 --- a/config/alfresco/ownable-services-context.xml +++ b/config/alfresco/ownable-services-context.xml @@ -24,5 +24,8 @@ + + + \ No newline at end of file diff --git a/source/java/org/alfresco/repo/ownable/impl/OwnableServiceImpl.java b/source/java/org/alfresco/repo/ownable/impl/OwnableServiceImpl.java index 6099481b80..a364652b22 100644 --- a/source/java/org/alfresco/repo/ownable/impl/OwnableServiceImpl.java +++ b/source/java/org/alfresco/repo/ownable/impl/OwnableServiceImpl.java @@ -1,14 +1,14 @@ /* - * #%L - * Alfresco Repository - * %% - * Copyright (C) 2005 - 2016 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: - * + * #%L + * Alfresco Repository + * %% + * Copyright (C) 2005 - 2016 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 @@ -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 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; } diff --git a/source/test-java/org/alfresco/repo/ownable/impl/OwnableServiceTest.java b/source/test-java/org/alfresco/repo/ownable/impl/OwnableServiceTest.java index 56b9d0ad37..23790dd7dd 100644 --- a/source/test-java/org/alfresco/repo/ownable/impl/OwnableServiceTest.java +++ b/source/test-java/org/alfresco/repo/ownable/impl/OwnableServiceTest.java @@ -1,14 +1,14 @@ /* - * #%L - * Alfresco Repository - * %% - * Copyright (C) 2005 - 2016 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: - * + * #%L + * Alfresco Repository + * %% + * Copyright (C) 2005 - 2016 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 @@ -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; @@ -58,6 +59,10 @@ import org.springframework.context.ApplicationContext; 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; @@ -113,18 +118,25 @@ public class OwnableServiceTest extends TestCase StoreRef storeRef = nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "Test_" + System.currentTimeMillis()); rootNodeRef = nodeService.getRootNode(storeRef); permissionService.setPermission(rootNodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.ADD_CHILDREN, true); - - if(authenticationDAO.userExists("andy")) - { - authenticationService.deleteAuthentication("andy"); - } - authenticationService.createAuthentication("andy", "andy".toCharArray()); - + + reCreateUser(USER_ANDY, USER_ANDY); + reCreateUser(USER_WOOF, USER_WOOF); + reCreateUser(USER_MUPPET, USER_MUPPET); + dynamicAuthority = new OwnerDynamicAuthority(); dynamicAuthority.setOwnableService(ownableService); 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); - assertEquals("andy", ownableService.getOwner(testNode)); + permissionService.setPermission(rootNodeRef, USER_ANDY, PermissionService.TAKE_OWNERSHIP, true); + 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"); -// assertEquals("woof", ownableService.getOwner(testNode)); +// nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, USER_WOOF); +// assertEquals(USER_WOOF, ownableService.getOwner(testNode)); // -// nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, "andy"); -// assertEquals("andy", ownableService.getOwner(testNode)); +// nodeService.setProperty(testNode, ContentModel.PROP_CREATOR, USER_ANDY); +// 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"); - assertEquals("woof", ownableService.getOwner(testNode)); - assertTrue(dynamicAuthority.hasAuthority(testNode, "woof")); + ownableService.setOwner(testNode, USER_WOOF); + assertEquals(USER_WOOF, ownableService.getOwner(testNode)); + 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"); - assertEquals("muppet", ownableService.getOwner(testNode)); - assertTrue(dynamicAuthority.hasAuthority(testNode, "muppet")); + ownableService.setOwner(testNode, USER_MUPPET); + assertEquals(USER_MUPPET, ownableService.getOwner(testNode)); + 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)); - assertTrue(dynamicAuthority.hasAuthority(testNode, "andy")); + assertEquals(USER_ANDY, ownableService.getOwner(testNode)); + 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"); - assertEquals("muppet", ownableService.getOwner(testNode)); + nodeService.setProperty(testNode, ContentModel.PROP_OWNER, USER_MUPPET); + assertEquals(USER_MUPPET, ownableService.getOwner(testNode)); nodeService.removeAspect(testNode, ContentModel.ASPECT_OWNABLE); - assertEquals("andy", ownableService.getOwner(testNode)); + assertEquals(USER_ANDY, ownableService.getOwner(testNode)); HashMap aspectProperties = new HashMap(); - 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"); - assertEquals("muppet", ownableService.getOwner(testNode)); + ownableService.setOwner(testNode, USER_MUPPET); + 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);