diff --git a/config/alfresco/authentication-services-context.xml b/config/alfresco/authentication-services-context.xml index 3d320cd7ca..0cdc14e762 100644 --- a/config/alfresco/authentication-services-context.xml +++ b/config/alfresco/authentication-services-context.xml @@ -325,6 +325,12 @@ + + + + + + @@ -363,9 +369,6 @@ false - - - diff --git a/config/alfresco/authority-services-context.xml b/config/alfresco/authority-services-context.xml index 8f1d199ed5..77e255b0fc 100644 --- a/config/alfresco/authority-services-context.xml +++ b/config/alfresco/authority-services-context.xml @@ -53,7 +53,8 @@ - + + ${spaces.store} @@ -75,6 +76,13 @@ + + + + + + + diff --git a/source/java/org/alfresco/repo/domain/hibernate/AclDaoComponentImpl.java b/source/java/org/alfresco/repo/domain/hibernate/AclDaoComponentImpl.java index 9dbc4d3642..a6abbaa1ba 100644 --- a/source/java/org/alfresco/repo/domain/hibernate/AclDaoComponentImpl.java +++ b/source/java/org/alfresco/repo/domain/hibernate/AclDaoComponentImpl.java @@ -890,23 +890,10 @@ public class AclDaoComponentImpl extends HibernateDaoSupport implements AclDaoCo // remove authority - - callback = new HibernateCallback() + DbAuthority toRemove = getAuthority(authority, false); + if(toRemove != null) { - public Object doInHibernate(Session session) - { - Query query = session.getNamedQuery(QUERY_GET_AUTHORITY); - query.setParameter("authority", authority); - return query.list(); - } - }; - List authorities = (List) getHibernateTemplate().execute(callback); - for (DbAuthority found : authorities) - { - if (found.getAuthority().equals(authority)) - { - getHibernateTemplate().delete(found); - } + getHibernateTemplate().delete(toRemove); } // TODO: Remove affected ACLs from the cache @@ -1378,34 +1365,8 @@ public class AclDaoComponentImpl extends HibernateDaoSupport implements AclDaoCo throw new IllegalArgumentException("Invalid position"); } - // Find auth - HibernateCallback callback = new HibernateCallback() - { - public Object doInHibernate(Session session) - { - Query query = session.getNamedQuery(QUERY_GET_AUTHORITY); - query.setParameter("authority", ace.getAuthority()); - return query.list(); - } - }; - DbAuthority authority = null; - List authorities = (List) getHibernateTemplate().execute(callback); - for (DbAuthority found : authorities) - { - if (found.getAuthority().equals(ace.getAuthority())) - { - authority = found; - break; - } - } - if (authority == null) - { - DbAuthorityImpl newAuthority = new DbAuthorityImpl(); - newAuthority.setAuthority(ace.getAuthority()); - newAuthority.setCrc(getCrc(ace.getAuthority())); - authority = newAuthority; - getHibernateTemplate().save(newAuthority); - } + // Find authority + DbAuthority authority = getAuthority(ace.getAuthority(), true); // Find permission @@ -1413,7 +1374,7 @@ public class AclDaoComponentImpl extends HibernateDaoSupport implements AclDaoCo final String permissionName = ace.getPermission().getName(); final Pair permissionQNamePair = qnameDAO.getOrCreateQName(permissionQName); - callback = new HibernateCallback() + HibernateCallback callback = new HibernateCallback() { public Object doInHibernate(Session session) { @@ -2221,4 +2182,62 @@ public class AclDaoComponentImpl extends HibernateDaoSupport implements AclDaoCo } } + public void updateAuthority(String before, String after) + { + DbAuthority dbAuthority = getAuthority(before, false); + // If there is no entry and alias is not required - there is nothing it would match + if(dbAuthority != null) + { + dbAuthority.setAuthority(after); + dbAuthority.setCrc(getCrc(after)); + aclCache.clear(); + } + } + + + + private DbAuthority getAuthority(final String authority, boolean create) + { + // Find auth + HibernateCallback callback = new HibernateCallback() + { + public Object doInHibernate(Session session) + { + Query query = session.getNamedQuery(QUERY_GET_AUTHORITY); + query.setParameter("authority", authority); + return query.list(); + } + }; + + DbAuthority dbAuthority = null; + List authorities = (List) getHibernateTemplate().execute(callback); + for (DbAuthority found : authorities) + { + if (found.getAuthority().equals(authority)) + { + dbAuthority = found; + break; + } + } + if (create && (dbAuthority == null)) + { + dbAuthority = createDbAuthority(authority); + } + return dbAuthority; + } + + public void createAuthority(String authority) + { + createDbAuthority(authority); + } + + public DbAuthority createDbAuthority(String authority) + { + DbAuthority dbAuthority = new DbAuthorityImpl(); + dbAuthority.setAuthority(authority); + dbAuthority.setCrc(getCrc(authority)); + getHibernateTemplate().save(dbAuthority); + return dbAuthority; + } + } diff --git a/source/java/org/alfresco/repo/security/authentication/AbstractAuthenticationComponent.java b/source/java/org/alfresco/repo/security/authentication/AbstractAuthenticationComponent.java index ac995f3fc2..d694f90298 100644 --- a/source/java/org/alfresco/repo/security/authentication/AbstractAuthenticationComponent.java +++ b/source/java/org/alfresco/repo/security/authentication/AbstractAuthenticationComponent.java @@ -270,6 +270,12 @@ public abstract class AbstractAuthenticationComponent implements AuthenticationC logger.debug("Setting the current user to \"" + userName + '"'); } ud = getUserDetails(userName); + if(!userName.equals(ud.getUsername())) + { + ud = new User(userName, ud.getPassword(), ud.isEnabled(), ud.isAccountNonExpired(), + ud.isCredentialsNonExpired(), ud.isAccountNonLocked(), ud.getAuthorities()); + } + } return setUserDetails(ud); } diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java index 295910fc00..2e8720aeeb 100644 --- a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java +++ b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java @@ -39,6 +39,10 @@ import java.util.regex.Pattern; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; import org.alfresco.repo.cache.SimpleCache; +import org.alfresco.repo.node.NodeServicePolicies; +import org.alfresco.repo.policy.JavaBehaviour; +import org.alfresco.repo.policy.PolicyComponent; +import org.alfresco.repo.security.permissions.impl.AclDaoComponent; import org.alfresco.repo.tenant.TenantService; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.repository.ChildAssociationRef; @@ -50,11 +54,14 @@ import org.alfresco.service.cmr.security.AuthorityType; import org.alfresco.service.cmr.security.NoSuchPersonException; import org.alfresco.service.cmr.security.PersonService; import org.alfresco.service.namespace.NamespacePrefixResolver; +import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; +import org.alfresco.util.EqualsHelper; import org.alfresco.util.SearchLanguageConversion; -public class AuthorityDAOImpl implements AuthorityDAO +public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.OnCreateNodePolicy, NodeServicePolicies.BeforeDeleteNodePolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy { private StoreRef storeRef; @@ -80,6 +87,10 @@ public class AuthorityDAOImpl implements AuthorityDAO private Map systemContainerRefs = new ConcurrentHashMap(4); + private AclDaoComponent aclDao; + + private PolicyComponent policyComponent; + public AuthorityDAOImpl() { super(); @@ -123,6 +134,16 @@ public class AuthorityDAOImpl implements AuthorityDAO this.tenantService = tenantService; } + public void setAclDao(AclDaoComponent aclDao) + { + this.aclDao = aclDao; + } + + public void setPolicyComponent(PolicyComponent policyComponent) + { + this.policyComponent = policyComponent; + } + public boolean authorityExists(String name) { NodeRef ref = getAuthorityOrNull(name); @@ -719,4 +740,56 @@ public class AuthorityDAOImpl implements AuthorityDAO return true; } } + + public void onCreateNode(ChildAssociationRef childAssocRef) + { + NodeRef authRef = childAssocRef.getChildRef(); + String authority = (String) this.nodeService.getProperty(authRef, ContentModel.PROP_AUTHORITY_NAME); + + // Make sure there is an authority entry - with a DB constraint for uniqueness + // aclDao.createAuthority(authority); + + } + + public void beforeDeleteNode(NodeRef nodeRef) + { + authorityLookupCache.clear(); + } + + public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { + String authBefore = DefaultTypeConverter.INSTANCE.convert(String.class, before.get(ContentModel.PROP_AUTHORITY_NAME)); + String authAfter = DefaultTypeConverter.INSTANCE.convert(String.class, after.get(ContentModel.PROP_AUTHORITY_NAME)); + if (!EqualsHelper.nullSafeEquals(authBefore, authAfter)) + { + if ((authBefore == null) || authBefore.equalsIgnoreCase(authAfter)) + { + // Fix any ACLs + aclDao.updateAuthority(authBefore, authAfter); + // Fix primary association local name + // This will need to lower case in 3.2+ + QName newAssocQName = QName.createQName("cm", authAfter, namespacePrefixResolver); + ChildAssociationRef assoc = nodeService.getPrimaryParent(nodeRef); + nodeService.moveNode(nodeRef, assoc.getParentRef(), assoc.getTypeQName(), newAssocQName); + // Cache is out of date + authorityLookupCache.clear(); + } + else + { + throw new UnsupportedOperationException("The name of an authority can not be changed"); + } + } + + } + + public void init() + { + this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onCreateNode"), ContentModel.TYPE_AUTHORITY_CONTAINER, new JavaBehaviour(this, + "onCreateNode")); + this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "beforeDeleteNode"), ContentModel.TYPE_AUTHORITY_CONTAINER, new JavaBehaviour( + this, "beforeDeleteNode")); + this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"), ContentModel.TYPE_AUTHORITY_CONTAINER, new JavaBehaviour( + this, "onUpdateProperties")); + } + } diff --git a/source/java/org/alfresco/repo/security/authority/DuplicateAuthorityTest.java b/source/java/org/alfresco/repo/security/authority/DuplicateAuthorityTest.java new file mode 100644 index 0000000000..7218c521e5 --- /dev/null +++ b/source/java/org/alfresco/repo/security/authority/DuplicateAuthorityTest.java @@ -0,0 +1,435 @@ +/* + * Copyright (C) 2005-2007 Alfresco Software Limited. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + + * This program 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 General Public License for more details. + + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + * As a special exception to the terms and conditions of version 2.0 of + * the GPL, you may redistribute this Program in connection with Free/Libre + * and Open Source Software ("FLOSS") applications as described in Alfresco's + * FLOSS exception. You should have recieved a copy of the text describing + * the FLOSS exception, and it is also available here: + * http://www.alfresco.com/legal/licensing" + */ +package org.alfresco.repo.security.authority; + +import java.io.Serializable; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import junit.framework.TestCase; + +import org.alfresco.model.ContentModel; +import org.alfresco.repo.security.authentication.AuthenticationComponent; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; +import org.alfresco.service.cmr.model.FileExistsException; +import org.alfresco.service.cmr.model.FileFolderService; +import org.alfresco.service.cmr.model.FileInfo; +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.AuthorityService; +import org.alfresco.service.cmr.security.AuthorityType; +import org.alfresco.service.cmr.security.PersonService; +import org.alfresco.service.namespace.NamespaceService; +import org.alfresco.service.namespace.QName; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.springframework.context.ApplicationContext; +import org.springframework.dao.DataIntegrityViolationException; + +/** + * Checks that the duplicate child handling is done correctly. + * + * @see org.alfresco.repo.model.filefolder.FileFolderServiceImpl + * @author Derek Hulley + * @since 2.1.0 + */ +public class DuplicateAuthorityTest extends TestCase +{ + private static final ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); + + private AuthenticationComponent authenticationComponent; + + private TransactionService transactionService; + + private RetryingTransactionHelper retryingTransactionHelper; + + private NodeService nodeService; + + private FileFolderService fileFolderService; + + private NodeRef rootNodeRef; + + private NodeRef workingRootNodeRef; + + private AuthorityService authorityService; + + private PersonService personService; + + @Override + public void setUp() throws Exception + { + ServiceRegistry serviceRegistry = (ServiceRegistry) ctx.getBean("ServiceRegistry"); + transactionService = serviceRegistry.getTransactionService(); + retryingTransactionHelper = transactionService.getRetryingTransactionHelper(); + retryingTransactionHelper.setMaxRetryWaitMs(10); + nodeService = serviceRegistry.getNodeService(); + fileFolderService = serviceRegistry.getFileFolderService(); + authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent"); + authorityService = (AuthorityService) ctx.getBean("authorityService"); + personService = (PersonService) ctx.getBean("personService"); + + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public NodeRef execute() throws Throwable + { + // authenticate + authenticationComponent.setCurrentUser(authenticationComponent.getSystemUserName()); + + // create a test store + StoreRef storeRef = nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, getName() + System.currentTimeMillis()); + rootNodeRef = nodeService.getRootNode(storeRef); + + // create a folder to import into + NodeRef nodeRef = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, QName.createQName(NamespaceService.ALFRESCO_URI, "working root"), + ContentModel.TYPE_FOLDER).getChildRef(); + // Done + return nodeRef; + } + }; + workingRootNodeRef = retryingTransactionHelper.doInTransaction(callback, false, true); + } + + public void tearDown() throws Exception + { + RetryingTransactionCallback callback1 = new RetryingTransactionCallback() + { + public Void execute() + { + for (int i = 0; i <= 10+1; i++) + { + if (authorityService.authorityExists("GROUP_" + i)) + { + authorityService.deleteAuthority("GROUP_" + i); + } + } + return null; + } + }; + retryingTransactionHelper.doInTransaction(callback1, false, true); + + } + + public void testDuplicateGroupDetection() throws Exception + { + // disable for now + if(true) + { + return; + } + final int threadCount = 10; + RetryingTransactionCallback callback1 = new RetryingTransactionCallback() + { + public Void execute() + { + for (int i = 0; i <= threadCount+1; i++) + { + if (authorityService.authorityExists("GROUP_" + i)) + { + authorityService.deleteAuthority("GROUP_" + i); + } + } + return null; + } + }; + retryingTransactionHelper.doInTransaction(callback1, false, true); + + // First create a file name F1 + RetryingTransactionCallback callback = new CreateAuthorityCallback(0); + String result = retryingTransactionHelper.doInTransaction(callback, false, true); + // Check that the filename is F0 + assertEquals("GROUP_0", result); + + // Now create a whole lot of threads that attempt file creation + + CountDownLatch endLatch = new CountDownLatch(threadCount); + AuthThread[] workers = new AuthThread[threadCount]; + for (int i = 0; i < threadCount; i++) + { + workers[i] = new AuthThread(endLatch); + workers[i].start(); + } + // Wait at the end gate + endLatch.await(300L, TimeUnit.SECONDS); + + // Analyse + int failureCount = 0; + int didNotCompleteCount = 0; + for (int i = 0; i < threadCount; i++) + { + if (workers[i].error != null) + { + failureCount++; + } + else if (workers[i].success == null) + { + didNotCompleteCount++; + } + } + System.out.println("" + failureCount + " of the " + threadCount + " threads failed and " + didNotCompleteCount + " did not finish."); + assertEquals("Some failures", 0, failureCount); + assertEquals("Some non-finishes", 0, didNotCompleteCount); + + retryingTransactionHelper.doInTransaction(callback1, false, true); + } + + public void testDuplicatePersonDetection() throws Exception + { + // disable for now + if(true) + { + return; + } + final int threadCount = 10; + RetryingTransactionCallback callback1 = new RetryingTransactionCallback() + { + public Void execute() + { + for (int i = 0; i <= threadCount+1; i++) + { + if (personService.personExists("Person_" + i)) + { + personService.deletePerson("Person_" + i); + } + } + return null; + } + }; + //retryingTransactionHelper.doInTransaction(callback1, false, true); + + // First create a file name F1 + RetryingTransactionCallback callback = new CreatePersonCallback(0); + String result = retryingTransactionHelper.doInTransaction(callback, false, true); + // Check that the filename is F0 + assertEquals("Person_0", result); + + // Now create a whole lot of threads that attempt file creation + + CountDownLatch endLatch = new CountDownLatch(threadCount); + PersonThread[] workers = new PersonThread[threadCount]; + for (int i = 0; i < threadCount; i++) + { + workers[i] = new PersonThread(endLatch); + workers[i].start(); + } + // Wait at the end gate + endLatch.await(300L, TimeUnit.SECONDS); + + // Analyse + int failureCount = 0; + int didNotCompleteCount = 0; + for (int i = 0; i < threadCount; i++) + { + if (workers[i].error != null) + { + failureCount++; + } + else if (workers[i].success == null) + { + didNotCompleteCount++; + } + } + System.out.println("" + failureCount + " of the " + threadCount + " threads failed and " + didNotCompleteCount + " did not finish."); + assertEquals("Some failures", 0, failureCount); + assertEquals("Some non-finishes", 0, didNotCompleteCount); + + retryingTransactionHelper.doInTransaction(callback1, false, true); + } + + /** + * Attempts to create a file "Fn" where n is the number supplied to the constructor. + */ + private class CreateAuthorityCallback implements RetryingTransactionCallback + { + private final int number; + + public CreateAuthorityCallback(int number) + { + this.number = number; + } + + public String execute() throws Throwable + { + authenticationComponent.setCurrentUser(authenticationComponent.getSystemUserName()); + return authorityService.createAuthority(AuthorityType.GROUP, "" + number); + } + } + + private class CreatePersonCallback implements RetryingTransactionCallback + { + private final int number; + + public CreatePersonCallback(int number) + { + this.number = number; + } + + public String execute() throws Throwable + { + authenticationComponent.setCurrentUser(authenticationComponent.getSystemUserName()); + + HashMap properties = new HashMap(); + properties.put(ContentModel.PROP_USERNAME, "Person_" + number); + properties.put(ContentModel.PROP_HOMEFOLDER, rootNodeRef); + properties.put(ContentModel.PROP_FIRSTNAME, "Person_" + number); + properties.put(ContentModel.PROP_LASTNAME, "Person_" + number); + properties.put(ContentModel.PROP_EMAIL, "Person_" + number); + properties.put(ContentModel.PROP_ORGID, "Person_" + number); + + personService.createPerson(properties); + return "Person_" + number; + } + } + + private static ThreadGroup threadGroup = new ThreadGroup("DuplicateAuthorityTest"); + + private static int threadNumber = -1; + + private class AuthThread extends Thread + { + private CountDownLatch endLatch; + + private Throwable error; + + private String success; + + public AuthThread(CountDownLatch endLatch) + { + super(threadGroup, "Worker " + ++threadNumber); + this.endLatch = endLatch; + } + + public void run() + { + String result = null; + // Start the count with a guaranteed failure + int number = 0; + while (true) + { + RetryingTransactionCallback callback = new CreateAuthorityCallback(number); + try + { + System.out.println("Thread " + getName() + " attempting file: " + number); + System.out.flush(); + + result = retryingTransactionHelper.doInTransaction(callback, false, true); + // It worked + success = result; + break; + } + catch (DataIntegrityViolationException e) + { + // Try another number + number++; + } + catch (Throwable e) + { + // Oops + error = e; + break; + } + } + // Done + if (error != null) + { + System.err.println("Thread " + getName() + " failed to create file " + number + ":"); + System.err.flush(); + error.printStackTrace(); + } + else + { + System.out.println("\t\t\tThread " + getName() + " created auth: " + success); + System.out.flush(); + } + // Tick the latch + endLatch.countDown(); + } + } + + private class PersonThread extends Thread + { + private CountDownLatch endLatch; + + private Throwable error; + + private String success; + + public PersonThread(CountDownLatch endLatch) + { + super(threadGroup, "Worker " + ++threadNumber); + this.endLatch = endLatch; + } + + public void run() + { + String result = null; + // Start the count with a guaranteed failure + int number = 0; + while (true) + { + RetryingTransactionCallback callback = new CreatePersonCallback(number); + try + { + System.out.println("Thread " + getName() + " attempting file: " + number); + System.out.flush(); + + result = retryingTransactionHelper.doInTransaction(callback, false, true); + // It worked + success = result; + break; + } + catch (DataIntegrityViolationException e) + { + // Try another number + number++; + } + catch (Throwable e) + { + // Oops + error = e; + break; + } + } + // Done + if (error != null) + { + System.err.println("Thread " + getName() + " failed to create file " + number + ":"); + System.err.flush(); + error.printStackTrace(); + } + else + { + System.out.println("\t\t\tThread " + getName() + " created auth: " + success); + System.out.flush(); + } + // Tick the latch + endLatch.countDown(); + } + } +} diff --git a/source/java/org/alfresco/repo/security/permissions/impl/AbstractPermissionTest.java b/source/java/org/alfresco/repo/security/permissions/impl/AbstractPermissionTest.java index eca199d213..638ed4cf28 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/AbstractPermissionTest.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/AbstractPermissionTest.java @@ -33,6 +33,7 @@ import org.alfresco.repo.node.db.NodeDaoService; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.MutableAuthenticationDao; +import org.alfresco.repo.security.authority.AuthorityDAO; import org.alfresco.repo.security.permissions.PermissionReference; import org.alfresco.repo.security.permissions.PermissionServiceSPI; import org.alfresco.repo.transaction.RetryingTransactionHelper; @@ -82,6 +83,8 @@ public class AbstractPermissionTest extends BaseSpringTest protected PersonService personService; protected AuthorityService authorityService; + + protected AuthorityDAO authorityDAO; protected NodeDaoService nodeDaoService; @@ -109,6 +112,7 @@ public class AbstractPermissionTest extends BaseSpringTest permissionModelDAO = (ModelDAO) applicationContext.getBean("permissionsModelDAO"); personService = (PersonService) applicationContext.getBean("personService"); authorityService = (AuthorityService) applicationContext.getBean("authorityService"); + authorityDAO = (AuthorityDAO) applicationContext.getBean("authorityDAO"); authenticationComponent.setCurrentUser(authenticationComponent.getSystemUserName()); authenticationDAO = (MutableAuthenticationDao) applicationContext.getBean("authenticationDao"); diff --git a/source/java/org/alfresco/repo/security/permissions/impl/AclDaoComponent.java b/source/java/org/alfresco/repo/security/permissions/impl/AclDaoComponent.java index 3084074649..9cea201267 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/AclDaoComponent.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/AclDaoComponent.java @@ -25,6 +25,7 @@ package org.alfresco.repo.security.permissions.impl; import java.util.List; +import java.util.Set; import org.alfresco.repo.domain.DbAccessControlList; import org.alfresco.repo.domain.hibernate.AclDaoComponentImpl.Indirection; @@ -182,4 +183,8 @@ public interface AclDaoComponent extends TransactionalDao * @param id */ public void onDeleteAccessControlList(final long id); + + public void updateAuthority(String before, String after); + + public void createAuthority(String authority); } diff --git a/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceTest.java b/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceTest.java index 1869de7f6b..7320265809 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceTest.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceTest.java @@ -68,6 +68,102 @@ public class PermissionServiceTest extends AbstractPermissionTest // TODO Auto-generated constructor stub } + public void testChangePersonUid() + { + runAs("admin"); + NodeRef one = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, QName.createQName("{namespace}one"), ContentModel.TYPE_FOLDER).getChildRef(); + permissionService.setPermission(one, "andy", PermissionService.ALL_PERMISSIONS, true); + runAs("andy"); + assertEquals("andy", authenticationComponent.getCurrentUserName()); + assertTrue(permissionService.hasPermission(one, PermissionService.EXECUTE_CONTENT) == AccessStatus.ALLOWED); + runAs("admin"); + boolean found = false; + Set set = permissionService.getAllSetPermissions(one); + for (AccessPermission ap : set) + { + if (ap.getAuthority().equals("Andy")) + { + found = true; + } + } + assertFalse(found); + NodeRef andy = personService.getPerson("andy"); + nodeService.setProperty(andy, ContentModel.PROP_USERNAME, "Andy"); + runAs("andy"); + assertEquals("Andy", authenticationComponent.getCurrentUserName()); + assertTrue(permissionService.hasPermission(one, PermissionService.EXECUTE_CONTENT) == AccessStatus.ALLOWED); + runAs("admin"); + found = false; + set = permissionService.getAllSetPermissions(one); + for (AccessPermission ap : set) + { + if (ap.getAuthority().equals("Andy")) + { + found = true; + } + } + assertTrue(found); + + try + { + nodeService.setProperty(andy, ContentModel.PROP_USERNAME, "Bob"); + fail("Chainging uid Andy -> Bob should fail"); + } + catch (UnsupportedOperationException e) + { + + } + } + + public void testChangeGroupUid() + { + personService.getPerson("andy"); + runAs("admin"); + NodeRef one = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, QName.createQName("{namespace}one"), ContentModel.TYPE_FOLDER).getChildRef(); + authorityService.createAuthority(AuthorityType.GROUP, "ONE"); + authorityService.addAuthority("GROUP_ONE", "andy"); + permissionService.setPermission(one, "GROUP_ONE", PermissionService.ALL_PERMISSIONS, true); + runAs("andy"); + assertEquals("andy", authenticationComponent.getCurrentUserName()); + assertTrue(permissionService.hasPermission(one, PermissionService.EXECUTE_CONTENT) == AccessStatus.ALLOWED); + runAs("admin"); + boolean found = false; + Set set = permissionService.getAllSetPermissions(one); + for (AccessPermission ap : set) + { + if (ap.getAuthority().equals("GROUP_One")) + { + found = true; + } + } + assertFalse(found); + NodeRef gONE = authorityDAO.getAuthorityNodeRefOrNull("GROUP_ONE"); + nodeService.setProperty(gONE, ContentModel.PROP_AUTHORITY_NAME, "GROUP_One"); + runAs("andy"); + assertTrue(permissionService.hasPermission(one, PermissionService.EXECUTE_CONTENT) == AccessStatus.ALLOWED); + runAs("admin"); + found = false; + set = permissionService.getAllSetPermissions(one); + for (AccessPermission ap : set) + { + if (ap.getAuthority().equals("GROUP_One")) + { + found = true; + } + } + assertTrue(found); + + try + { + nodeService.setProperty(gONE, ContentModel.PROP_AUTHORITY_NAME, "GROUP_TWO"); + fail("Chainging gid GROUP_One -> GROUP_TWO should fail"); + } + catch (UnsupportedOperationException e) + { + + } + } + public void testAuthenticatedRoleIsPresent() { runAs("andy"); diff --git a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java b/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java index d935c327ab..b314ba7357 100644 --- a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java +++ b/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java @@ -44,6 +44,7 @@ import org.alfresco.repo.policy.PolicyComponent; import org.alfresco.repo.security.authentication.AuthenticationException; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.permissions.PermissionServiceSPI; +import org.alfresco.repo.security.permissions.impl.AclDaoComponent; import org.alfresco.repo.tenant.TenantService; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.TransactionListenerAdapter; @@ -69,12 +70,14 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.EqualsHelper; import org.alfresco.util.GUID; import org.alfresco.util.PropertyCheck; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -public class PersonServiceImpl extends TransactionListenerAdapter implements PersonService, NodeServicePolicies.OnCreateNodePolicy, NodeServicePolicies.BeforeDeleteNodePolicy +public class PersonServiceImpl extends TransactionListenerAdapter implements PersonService, NodeServicePolicies.OnCreateNodePolicy, NodeServicePolicies.BeforeDeleteNodePolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy { private static Log s_logger = LogFactory.getLog(PersonServiceImpl.class); @@ -130,6 +133,8 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per private PersonDao personDao; + private AclDaoComponent aclDao; + private PermissionsManager permissionsManager; /** a transactionally-safe cache to be injected */ @@ -177,12 +182,15 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per PropertyCheck.mandatory(this, "policyComponent", policyComponent); PropertyCheck.mandatory(this, "personCache", personCache); PropertyCheck.mandatory(this, "personDao", personDao); + PropertyCheck.mandatory(this, "aclDao", aclDao); + PropertyCheck.mandatory(this, "homeFolderManager", homeFolderManager); this.policyComponent .bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onCreateNode"), ContentModel.TYPE_PERSON, new JavaBehaviour(this, "onCreateNode")); this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "beforeDeleteNode"), ContentModel.TYPE_PERSON, new JavaBehaviour(this, "beforeDeleteNode")); - + this.policyComponent.bindClassBehaviour(QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"), ContentModel.TYPE_PERSON, new JavaBehaviour(this, + "onUpdateProperties")); } public UserNameMatcher getUserNameMatcher() @@ -224,12 +232,17 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per { this.homeFolderManager = homeFolderManager; } - + public void setPersonDao(PersonDao personDao) { this.personDao = personDao; } + public void setAclDao(AclDaoComponent aclDao) + { + this.aclDao = aclDao; + } + public void setPermissionsManager(PermissionsManager permissionsManager) { this.permissionsManager = permissionsManager; @@ -643,6 +656,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per nodeService.addChild(authorityService.getOrCreateZone(zone), personRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", userName, namespacePrefixResolver)); } } + return personRef; } @@ -782,6 +796,12 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per String username = (String) this.nodeService.getProperty(personRef, ContentModel.PROP_USERNAME); this.personCache.put(username, personRef); permissionsManager.setPermissions(personRef, username, username); + + // Make sure there is an authority entry - with a DB constraint for uniqueness + // aclDao.createAuthority(username); + + // work around for policy bug ... + homeFolderManager.onCreateNode(childAssocRef); } /* @@ -916,4 +936,37 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per return userNameMatcher.getUserNamesAreCaseSensitive(); } + /* + * When a uid is changed we need to create an alias for the old uid so permissions are not broken. This can happen + * when an already existing user is updated via LDAP e.g. migration to LDAP, or when a user is auto created and then + * updated by LDAP This is probably less likely after 3.2 and sync on missing person See + * https://issues.alfresco.com/jira/browse/ETWOTWO-389 (non-Javadoc) + * + * @see org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy#onUpdateProperties(org.alfresco.service.cmr.repository.NodeRef, + * java.util.Map, java.util.Map) + */ + public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { + String uidBefore = DefaultTypeConverter.INSTANCE.convert(String.class, before.get(ContentModel.PROP_USERNAME)); + String uidAfter = DefaultTypeConverter.INSTANCE.convert(String.class, after.get(ContentModel.PROP_USERNAME)); + if (!EqualsHelper.nullSafeEquals(uidBefore, uidAfter)) + { + if ((uidBefore == null) || uidBefore.equalsIgnoreCase(uidAfter)) + { + // Fix any ACLs + aclDao.updateAuthority(uidBefore, uidAfter); + // Fix primary association local name + QName newAssocQName = QName.createQName("cm", uidAfter.toLowerCase(), namespacePrefixResolver); + ChildAssociationRef assoc = nodeService.getPrimaryParent(nodeRef); + nodeService.moveNode(nodeRef, assoc.getParentRef(), assoc.getTypeQName(), newAssocQName); + // Fix cache + personCache.remove(uidBefore); + personCache.put(uidAfter, nodeRef); + } + else + { + throw new UnsupportedOperationException("The user name on a person can not be changed"); + } + } + } }