diff --git a/source/java/org/alfresco/repo/site/SiteServiceImpl.java b/source/java/org/alfresco/repo/site/SiteServiceImpl.java index 59e7d18ddc..63ff6d3223 100644 --- a/source/java/org/alfresco/repo/site/SiteServiceImpl.java +++ b/source/java/org/alfresco/repo/site/SiteServiceImpl.java @@ -47,6 +47,7 @@ import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AccessPermission; +import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.AuthorityService; import org.alfresco.service.cmr.security.AuthorityType; import org.alfresco.service.cmr.security.PermissionService; @@ -660,13 +661,12 @@ public class SiteServiceImpl implements SiteService, SiteModel } // If ... - // -- the current user is a site manager + // -- the current user has change permissions // or // -- the site is public and // -- the user is ourselves and // -- the users current role is consumer - if ((currentUserRole != null && - SiteModel.SITE_MANAGER.equals(currentUserRole) == true) + if ((permissionService.hasPermission(siteNodeRef, PermissionService.CHANGE_PERMISSIONS) == AccessStatus.ALLOWED) || (isPublic == true && currentUserName.equals(userName) == true && @@ -701,6 +701,11 @@ public class SiteServiceImpl implements SiteService, SiteModel // Throw a permission exception throw new AlfrescoRuntimeException("Access denied, user does not have permissions to delete membership details of the site '" + shortName + "'"); } + } + else + { + // Throw a permission exception + throw new AlfrescoRuntimeException("Access denied, user does not have permissions to delete membership details of the site '" + shortName + "'"); } } @@ -718,96 +723,104 @@ public class SiteServiceImpl implements SiteService, SiteModel // Get the user's current role final String currentRole = getMembersRole(shortName, userName); - if (currentRole == null || role.equals(currentRole) == false) + if (currentRole == null) { - // Determine whether the site is private or not - boolean isPublic = isSitePublic(siteNodeRef); - - // TODO if this is the only site manager do not downgrade their permissions - - // If we are ... - // -- the site manager - // or we are ... - // -- refering to a public site and - // -- the role being set is consumer and - // -- the user being added is ourselves and - // -- the member does not already have permissions - // ... then we can set the permissions as system user - final String currentUserName = AuthenticationUtil.getCurrentUserName(); - final String currentUserRole = getMembersRole(shortName, currentUserName); - if ((currentUserRole != null && - SiteModel.SITE_MANAGER.equals(currentUserRole) == true) - || - (isPublic == true && - role.equals(SiteModel.SITE_CONSUMER) == true && - userName.equals(currentUserName) == true && - currentRole == null)) + // Do nothing if the role of the user is not being changed + if (role.equals(currentRole) == false) { - // Check that we are not about to remove the last site manager - if (SiteModel.SITE_MANAGER.equals(currentRole) == true) - { - Set siteMangers = this.authorityService.getContainedAuthorities( - AuthorityType.USER, - getSiteRoleGroup(shortName, SITE_MANAGER, true), - true); - if (siteMangers.size() == 1) - { - throw new AlfrescoRuntimeException("A site requires at least one site manager. You can not change '" + userName + "' role from the site memebership because they are currently the only site manager."); - } - } + // Determine whether the site is private or not + boolean isPublic = isSitePublic(siteNodeRef); - // Run as system user - AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork() + // TODO if this is the only site manager do not downgrade their permissions + + // If we are ... + // -- the site manager + // or we are ... + // -- refering to a public site and + // -- the role being set is consumer and + // -- the user being added is ourselves and + // -- the member does not already have permissions + // ... then we can set the permissions as system user + final String currentUserName = AuthenticationUtil.getCurrentUserName(); + final String currentUserRole = getMembersRole(shortName, currentUserName); + if ((permissionService.hasPermission(siteNodeRef, PermissionService.CHANGE_PERMISSIONS) == AccessStatus.ALLOWED) + || + (isPublic == true && + role.equals(SiteModel.SITE_CONSUMER) == true && + userName.equals(currentUserName) == true && + currentRole == null)) { - public Object doWork() throws Exception + // Check that we are not about to remove the last site manager + if (SiteModel.SITE_MANAGER.equals(currentRole) == true) { - if (currentRole != null) + Set siteMangers = this.authorityService.getContainedAuthorities( + AuthorityType.USER, + getSiteRoleGroup(shortName, SITE_MANAGER, true), + true); + if (siteMangers.size() == 1) { - // Remove the user from the current permission group - String currentGroup = getSiteRoleGroup(shortName, currentRole, true); - authorityService.removeAuthority(currentGroup, userName); + throw new AlfrescoRuntimeException("A site requires at least one site manager. You can not change '" + userName + "' role from the site memebership because they are currently the only site manager."); } - - // Add the user to the new permission group - String newGroup = getSiteRoleGroup(shortName, role, true); - authorityService.addAuthority(newGroup, userName); - - return null; } - - }, AuthenticationUtil.SYSTEM_USER_NAME); - - if (currentRole == null) - { - if (AuthorityType.getAuthorityType(userName) == AuthorityType.USER) + + // Run as system user + AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork() { - activityService.postActivity(ActivityType.SITE_USER_JOINED, shortName, ACTIVITY_TOOL, getActivityData(userName, role)); + public Object doWork() throws Exception + { + if (currentRole != null) + { + // Remove the user from the current permission group + String currentGroup = getSiteRoleGroup(shortName, currentRole, true); + authorityService.removeAuthority(currentGroup, userName); + } + + // Add the user to the new permission group + String newGroup = getSiteRoleGroup(shortName, role, true); + authorityService.addAuthority(newGroup, userName); + + return null; + } + + }, AuthenticationUtil.SYSTEM_USER_NAME); + + if (currentRole == null) + { + if (AuthorityType.getAuthorityType(userName) == AuthorityType.USER) + { + activityService.postActivity(ActivityType.SITE_USER_JOINED, shortName, ACTIVITY_TOOL, getActivityData(userName, role)); + } + else + { + // TODO - update this, if sites support groups + logger.error("setMembership - failed to post activity: unexpected authority type: " + AuthorityType.getAuthorityType(userName)); + } } else { - // TODO - update this, if sites support groups - logger.error("setMembership - failed to post activity: unexpected authority type: " + AuthorityType.getAuthorityType(userName)); - } + if (AuthorityType.getAuthorityType(userName) == AuthorityType.USER) + { + activityService.postActivity(ActivityType.SITE_USER_ROLE_UPDATE, shortName, ACTIVITY_TOOL, getActivityData(userName, role)); + } + else + { + // TODO - update this, if sites support groups + logger.error("setMembership - failed to post activity: unexpected authority type: " + AuthorityType.getAuthorityType(userName)); + } + } } else - { - if (AuthorityType.getAuthorityType(userName) == AuthorityType.USER) - { - activityService.postActivity(ActivityType.SITE_USER_ROLE_UPDATE, shortName, ACTIVITY_TOOL, getActivityData(userName, role)); - } - else - { - // TODO - update this, if sites support groups - logger.error("setMembership - failed to post activity: unexpected authority type: " + AuthorityType.getAuthorityType(userName)); - } - } - } - else - { - // Raise a permission exception - throw new AlfrescoRuntimeException("Access denied, user does not have permissions to modify membership details of the site '" + shortName + "'"); + { + // Raise a permission exception + throw new AlfrescoRuntimeException("Access denied, user does not have permissions to modify membership details of the site '" + shortName + "'"); + } } } + else + { + // Raise a permission exception + throw new AlfrescoRuntimeException("Access denied, user does not have permissions to modify membership details of the site '" + shortName + "'"); + } } /** diff --git a/source/java/org/alfresco/repo/site/SiteServiceImplTest.java b/source/java/org/alfresco/repo/site/SiteServiceImplTest.java index 469d03b200..218281d653 100644 --- a/source/java/org/alfresco/repo/site/SiteServiceImplTest.java +++ b/source/java/org/alfresco/repo/site/SiteServiceImplTest.java @@ -134,6 +134,42 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest } } + public void testETHREEOH_15() throws Exception + { + SiteInfo siteInfo = this.siteService.createSite(TEST_SITE_PRESET, "mySiteTest", TEST_TITLE, TEST_DESCRIPTION, true); + checkSiteInfo(siteInfo, TEST_SITE_PRESET, "mySiteTest", TEST_TITLE, TEST_DESCRIPTION, true); + + authenticationComponent.setCurrentUser("admin"); + this.siteService.setMembership(siteInfo.getShortName(), USER_TWO, SiteModel.SITE_MANAGER); + + authenticationComponent.setCurrentUser(USER_TWO); + this.siteService.setMembership(siteInfo.getShortName(), USER_THREE, SiteModel.SITE_CONTRIBUTOR); + this.siteService.removeMembership(siteInfo.getShortName(), USER_THREE); + + authenticationComponent.setCurrentUser("admin"); + this.siteService.removeMembership(siteInfo.getShortName(), USER_TWO); + + authenticationComponent.setSystemUserAsCurrentUser(); + this.siteService.setMembership(siteInfo.getShortName(), USER_THREE, SiteModel.SITE_CONTRIBUTOR); + + authenticationComponent.setCurrentUser(USER_THREE); + try + { + this.siteService.setMembership(siteInfo.getShortName(), USER_TWO, SiteModel.SITE_CONTRIBUTOR); + fail("Shouldn't be able to do this cos you don't have permissions"); + } + catch (Exception exception) {} + try + { + this.siteService.removeMembership(siteInfo.getShortName(), USER_THREE); + fail("Shouldn't be able to do this cos you don't have permissions"); + } + catch (Exception exception) {} + + authenticationComponent.setSystemUserAsCurrentUser(); + this.siteService.removeMembership(siteInfo.getShortName(), USER_THREE); + } + private void checkSiteInfo( SiteInfo siteInfo, String expectedSitePreset, String expectedShortName, String expectedTitle, String expectedDescription, boolean expectedIsPublic) {