Fix for ALF-15257. Unable to delete sites if you are not the manager that created the site.

The problem here was that the deleteSite() method was deleting the group authorities associated with the site, before deleting the site itself. THerefore, site creators had permission to delete, but site managers who were not creators had their manager group membership deleted before the site delete was attempted and it therefore always failed.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@39796 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
Neil McErlean
2012-07-26 11:07:00 +00:00
parent 7ba6470483
commit 05bcd40a17
2 changed files with 59 additions and 19 deletions

View File

@@ -1405,6 +1405,23 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic
// Collection for recording the group memberships present on the site
final Map<String, Set<String>> groupsMemberships = new HashMap<String, Set<String>>();
// Save the group memberships so we can use them later
this.nodeService.setProperty(siteNodeRef, QName.createQName(null, "memberships"), (Serializable)groupsMemberships);
// The default behaviour is that sites cannot be deleted. But we disable that behaviour here
// in order to allow site deletion only via this service. Share calls this service for deletion.
//
// See ALF-7888 for some background on this issue
this.behaviourFilter.disableBehaviour(siteNodeRef, ContentModel.ASPECT_UNDELETABLE);
try
{
this.nodeService.deleteNode(siteNodeRef);
}
finally
{
this.behaviourFilter.enableBehaviour(siteNodeRef, ContentModel.ASPECT_UNDELETABLE);
}
// Delete the associated groups
AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork<Object>()
{
@@ -1435,23 +1452,6 @@ public class SiteServiceImpl extends AbstractLifecycleBean implements SiteServic
}
}, AuthenticationUtil.getSystemUserName());
// Save the group memberships so we can use them later
this.nodeService.setProperty(siteNodeRef, QName.createQName(null, "memberships"), (Serializable)groupsMemberships);
// The default behaviour is that sites cannot be deleted. But we disable that behaviour here
// in order to allow site deletion only via this service. Share calls this service for deletion.
//
// See ALF-7888 for some background on this issue
this.behaviourFilter.disableBehaviour(siteNodeRef, ContentModel.ASPECT_UNDELETABLE);
try
{
this.nodeService.deleteNode(siteNodeRef);
}
finally
{
this.behaviourFilter.enableBehaviour(siteNodeRef, ContentModel.ASPECT_UNDELETABLE);
}
logger.debug("site deleted :" + shortName);
}

View File

@@ -27,6 +27,7 @@ import java.util.Map;
import org.alfresco.query.PagingRequest;
import org.alfresco.query.PagingResults;
import org.alfresco.repo.security.authentication.AuthenticationUtil;
import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
import org.alfresco.repo.transaction.RetryingTransactionHelper;
import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
import org.alfresco.service.cmr.site.SiteInfo;
@@ -34,9 +35,11 @@ import org.alfresco.service.cmr.site.SiteService;
import org.alfresco.service.cmr.site.SiteVisibility;
import org.alfresco.service.namespace.NamespaceService;
import org.alfresco.service.namespace.QName;
import org.alfresco.util.test.junitrules.AlfrescoPerson;
import org.alfresco.util.test.junitrules.ApplicationContextInit;
import org.alfresco.util.test.junitrules.RunAsFullyAuthenticatedRule;
import org.alfresco.util.test.junitrules.TemporarySites;
import org.alfresco.util.test.junitrules.TemporarySites.TestSiteAndMemberInfo;
import org.alfresco.util.test.junitrules.TemporarySitesTest;
import org.junit.BeforeClass;
import org.junit.ClassRule;
@@ -64,10 +67,16 @@ public class SiteServiceImplMoreTest
public static TemporarySites STATIC_TEST_SITES = new TemporarySites(APP_CONTEXT_INIT);
// Tie them together in a static Rule Chain
@ClassRule public static RuleChain ruleChain = RuleChain.outerRule(APP_CONTEXT_INIT)
@ClassRule public static RuleChain staticRuleChain = RuleChain.outerRule(APP_CONTEXT_INIT)
.around(STATIC_TEST_SITES);
@Rule public RunAsFullyAuthenticatedRule runAllTestsAsAdmin = new RunAsFullyAuthenticatedRule(AuthenticationUtil.getAdminUserName());
public RunAsFullyAuthenticatedRule runAllTestsAsAdmin = new RunAsFullyAuthenticatedRule(AuthenticationUtil.getAdminUserName());
public AlfrescoPerson testUser = new AlfrescoPerson(APP_CONTEXT_INIT);
// Need to ensure the rules are in the correct order so that we're authenticated as admin before trying to create the person.
@Rule public RuleChain ruleChain = RuleChain.outerRule(runAllTestsAsAdmin)
.around(testUser);
// Various services
private static NamespaceService NAMESPACE_SERVICE;
@@ -75,6 +84,7 @@ public class SiteServiceImplMoreTest
private static RetryingTransactionHelper TRANSACTION_HELPER;
private static String TEST_SITE_NAME, TEST_SUB_SITE_NAME;
private static TestSiteAndMemberInfo TEST_SITE_WITH_MEMBERS;
@BeforeClass public static void initStaticData() throws Exception
{
@@ -92,6 +102,8 @@ public class SiteServiceImplMoreTest
STATIC_TEST_SITES.createSite("sitePreset", TEST_SITE_NAME, "siteTitle", "siteDescription", SiteVisibility.PUBLIC, admin);
STATIC_TEST_SITES.createSite("sitePreset", TEST_SUB_SITE_NAME, "siteTitle", "siteDescription", SiteVisibility.PUBLIC, subSiteType, admin);
TEST_SITE_WITH_MEMBERS = STATIC_TEST_SITES.createTestSiteWithUserPerRole(SiteServiceImplMoreTest.class.getSimpleName(), "sitePreset", SiteVisibility.PUBLIC, admin);
}
/**
@@ -118,4 +130,32 @@ public class SiteServiceImplMoreTest
}
});
}
/**
* This method ensures that sites can be deleted by any SiteManager, not just the Site Creator (ALF-15257).
*/
@Test public void anySiteManagerShouldBeAbleToDeleteASite() throws Exception
{
TRANSACTION_HELPER.doInTransaction(new RetryingTransactionCallback<Void>()
{
public Void execute() throws Throwable
{
// We already have a site with 4 users created by the TestSiteAndMemberInfo rule above.
// We'll use the testUser from the rule above - oh how I wish JUnit supported @Test-level rules, so I didn't have to create that user for all tests.
SITE_SERVICE.setMembership(TEST_SITE_WITH_MEMBERS.siteInfo.getShortName(), testUser.getUsername(), SiteModel.SITE_MANAGER);
// Now switch to run as that user and try to delete the site.
AuthenticationUtil.runAs(new RunAsWork<Void>()
{
@Override public Void doWork() throws Exception
{
SITE_SERVICE.deleteSite(TEST_SITE_WITH_MEMBERS.siteInfo.getShortName());
return null;
}
}, testUser.getUsername());
return null;
}
});
}
}