Site Service does not allow users 'admin' or 'system user' to set site membership if they are not Site Manager of the given site

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@11011 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

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 &&
+            if ((permissionService.hasPermission(siteNodeRef, PermissionService.CHANGE_PERMISSIONS) == AccessStatus.ALLOWED)
-                 SiteModel.SITE_MANAGER.equals(currentUserRole) == true)
                 || 
                 (isPublic == true &&
                  currentUserName.equals(userName) == true &&
@@ -702,6 +702,11 @@ public class SiteServiceImpl implements SiteService, SiteModel
                 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,7 +723,10 @@ 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)
+        {        
+            // Do nothing if the role of the user is not being changed
+            if (role.equals(currentRole) == false)
             {
                 // Determine whether the site is private or not
                 boolean isPublic = isSitePublic(siteNodeRef);
@@ -735,8 +743,7 @@ public class SiteServiceImpl implements SiteService, SiteModel
                 // ... then we can set the permissions as system user
                 final String currentUserName = AuthenticationUtil.getCurrentUserName();
                 final String currentUserRole = getMembersRole(shortName, currentUserName);
-            if  ((currentUserRole != null && 
+                if  ((permissionService.hasPermission(siteNodeRef, PermissionService.CHANGE_PERMISSIONS) == AccessStatus.ALLOWED)
-                  SiteModel.SITE_MANAGER.equals(currentUserRole) == true) 
                       || 
                      (isPublic == true &&
                       role.equals(SiteModel.SITE_CONSUMER) == true &&
@@ -809,6 +816,12 @@ public class SiteServiceImpl implements SiteService, SiteModel
                 }
             }
         }
+        else
+        {        
+            // Raise a permission exception
+            throw new AlfrescoRuntimeException("Access denied, user does not have permissions to modify membership details of the site '" + shortName + "'");
+        }
+    }    
 
     /**
      * @see org.alfresco.repo.site.SiteService#createContainer(java.lang.String, java.lang.String, org.alfresco.service.namespace.QName, java.util.Map)

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)
     {