Merged 5.2.N (5.2.1) to HEAD (5.2)

128660 jkaabimofrad: Merged 5.1.N (5.1.2) to 5.2.N (5.2.1)
      128649 jkaabimofrad: Merged JAMAL/MNT-16371 (5.1.2) to 5.1.N (5.1.2)
         128626 jkaabimofrad: MNT-16371: Turned off Inherit Permissions and explicitly added the Site Manager role for the Site to the surf-config folder when it is first created. Also, revoked ownership privileges for surf-config contents.


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

config/alfresco/web-scripts-application-context.xml

@@ -308,7 +308,7 @@
    </bean>
    
    <!--  Remote Store service - ADM -->
-   <bean id="webscript.org.alfresco.repository.store.remoteadm.get" class="org.alfresco.repo.web.scripts.bean.ADMRemoteStore" parent="webscript">
+   <bean id="baseADMRemoteStore" abstract="true" class="org.alfresco.repo.web.scripts.bean.ADMRemoteStore" parent="webscript" >
       <property name="nodeService" ref="NodeService" />
       <property name="unprotectedNodeService" ref="nodeService" />
       <property name="mimetypeService" ref="MimetypeService" />
@@ -317,34 +317,22 @@
       <property name="siteService" ref="SiteService" />
       <property name="hiddenAspect" ref="hiddenAspect" />
       <property name="behaviourFilter" ref="policyBehaviourFilter" />
+      <property name="permissionService" ref="permissionService" />
+      <property name="ownableService" ref="OwnableService" />
    </bean>
-   <bean id="surfConfigCleaner" parent="webscript.org.alfresco.repository.store.remoteadm.get" class="org.alfresco.repo.surf.policy.SurfConfigCleaner"
-      init-method="init">
+   
+   <bean id="webscript.org.alfresco.repository.store.remoteadm.get"  parent="baseADMRemoteStore" />
+   
+   <bean id="surfConfigCleaner" parent="baseADMRemoteStore" class="org.alfresco.repo.surf.policy.SurfConfigCleaner" init-method="init">
       <property name="fileFolderService" ref="fileFolderService" />
       <property name="nodeService" ref="nodeService" />
       <property name="policyComponent" ref="policyComponent" />
       <property name="siteService" ref="siteService" />
    </bean>
-   <bean id="webscript.org.alfresco.repository.store.remoteadm.post" class="org.alfresco.repo.web.scripts.bean.ADMRemoteStore" parent="webscript">
-      <property name="nodeService" ref="NodeService" />
-      <property name="unprotectedNodeService" ref="nodeService" />
-      <property name="mimetypeService" ref="MimetypeService" />
-      <property name="fileFolderService" ref="FileFolderService"/>
-      <property name="contentService" ref="ContentService"/>
-      <property name="siteService" ref="SiteService" />
-      <property name="hiddenAspect" ref="hiddenAspect" />
-      <property name="behaviourFilter" ref="policyBehaviourFilter" />
-   </bean>
-   <bean id="webscript.org.alfresco.repository.store.remoteadm.delete" class="org.alfresco.repo.web.scripts.bean.ADMRemoteStore" parent="webscript">
-      <property name="nodeService" ref="NodeService" />
-      <property name="unprotectedNodeService" ref="nodeService" />
-      <property name="mimetypeService" ref="MimetypeService" />
-      <property name="fileFolderService" ref="FileFolderService"/>
-      <property name="contentService" ref="ContentService"/>
-      <property name="siteService" ref="SiteService" />
-      <property name="hiddenAspect" ref="hiddenAspect" />
-      <property name="behaviourFilter" ref="policyBehaviourFilter" />
-   </bean>
+   
+   <bean id="webscript.org.alfresco.repository.store.remoteadm.post" parent="baseADMRemoteStore" />
+   
+   <bean id="webscript.org.alfresco.repository.store.remoteadm.delete"  parent="baseADMRemoteStore" />
    
    <!-- Web-tier SSO authentication touch point -->
    <bean id="webscript.org.alfresco.repository.touch.get" class="org.alfresco.repo.web.scripts.bean.Touch" parent="webscript" />

source/java/org/alfresco/repo/web/scripts/bean/ADMRemoteStore.java

@@ -37,10 +37,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
+import java.util.Map;
+import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TimeZone;
 import java.util.regex.Matcher;
@@ -62,7 +64,8 @@ import org.alfresco.repo.model.filefolder.HiddenAspect;
 import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
-import org.alfresco.repo.security.permissions.AccessDeniedException;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
+import org.alfresco.repo.site.SiteModel;
 import org.alfresco.service.cmr.model.FileExistsException;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.model.FileFolderUtil;
@@ -74,7 +77,9 @@ import org.alfresco.service.cmr.repository.ContentReader;
 import org.alfresco.service.cmr.repository.ContentService;
 import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.security.OwnableService;
+import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.cmr.site.SiteInfo;
 import org.alfresco.service.cmr.site.SiteService;
 import org.alfresco.service.namespace.NamespaceService;
@@ -130,7 +135,9 @@ public class ADMRemoteStore extends BaseRemoteStore
     protected NamespaceService namespaceService;
     protected SiteService siteService;
     protected ContentService contentService;
-    protected HiddenAspect hiddenAspect;
+    protected HiddenAspect hiddenAspect;
+    protected PermissionService permissionService;
+    protected OwnableService ownableService;
     private BehaviourFilter behaviourFilter;
     
     /**
@@ -197,7 +204,17 @@ public class ADMRemoteStore extends BaseRemoteStore
     {
         this.behaviourFilter = behaviourFilter;
     }
-    
+
+    public void setPermissionService(PermissionService permissionService)
+    {
+        this.permissionService = permissionService;
+    }
+
+    public void setOwnableService(OwnableService ownableService)
+    {
+        this.ownableService = ownableService;
+    }
+
     /**
      * Gets the last modified timestamp for the document.
      * <p>
@@ -483,12 +500,15 @@ public class ADMRemoteStore extends BaseRemoteStore
                         if (nodeService.getChildByName(parentFolderRef, ContentModel.ASSOC_CONTAINS, name) == null)
                         {
                             FileInfo fileInfo = fileFolderService.create(
-                                    parentFolderRef, name, ContentModel.TYPE_CONTENT);
+                                    parentFolderRef, name, ContentModel.TYPE_CONTENT);
+                            final NodeRef nodeRef = fileInfo.getNodeRef();
+                            // MNT-16371: Revoke ownership privileges for surf-config folder contents, to tighten access for former SiteManagers.
+                            ownableService.setOwner(nodeRef, AuthenticationUtil.getAdminUserName());
+
                             Map<QName, Serializable> aspectProperties = new HashMap<QName, Serializable>(1, 1.0f);
                             aspectProperties.put(ContentModel.PROP_IS_INDEXED, false);
-                            unprotNodeService.addAspect(fileInfo.getNodeRef(), ContentModel.ASPECT_INDEX_CONTROL, aspectProperties);
-                            ContentWriter writer = contentService.getWriter(
-                                    fileInfo.getNodeRef(), ContentModel.PROP_CONTENT, true);
+                            unprotNodeService.addAspect(nodeRef, ContentModel.ASPECT_INDEX_CONTROL, aspectProperties);
+                            ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true);
                             writer.guessMimetype(fileInfo.getName());
                             writer.putContent(content);
                             if (logger.isDebugEnabled())
@@ -803,8 +823,10 @@ public class ADMRemoteStore extends BaseRemoteStore
     private FileInfo resolveNodePath(final String path, final String pattern, final boolean create, final boolean isFolder)
     {
         if (logger.isDebugEnabled())
-            logger.debug("Resolving path: " + path);
-        
+            logger.debug("Resolving path: " + path);
+
+        final String adminUserName = AuthenticationUtil.getAdminUserName();
+
         FileInfo result = null;
         if (path != null)
         {
@@ -841,14 +863,21 @@ public class ADMRemoteStore extends BaseRemoteStore
                                 folderDetails.add(new FileFolderUtil.PathElementDetails(element, aspects));
                             }
                             // ensure folders exist down to the specified parent
-                            // ALF-17729 / ALF-17796 - disable auditable on parent folders
+                            // ALF-17729 / ALF-17796 - disable auditable on parent folders
+                            Set<NodeRef> allCreatedFolders = new LinkedHashSet<>();
                             result = FileFolderUtil.makeFolders(
                                     this.fileFolderService,nodeService,
                                     surfConfigRef,
                                     folderDetails,
                                     ContentModel.TYPE_FOLDER,
                                     behaviourFilter,
-                                    new HashSet<QName>(Arrays.asList(new QName[]{ContentModel.ASPECT_AUDITABLE})));
+                                    new HashSet<QName>(Arrays.asList(new QName[]{ContentModel.ASPECT_AUDITABLE})), allCreatedFolders);
+
+                            // MNT-16371: Revoke ownership privileges for surf-config folder, to tighten access for former SiteManagers.
+                            for(NodeRef nodeRef : allCreatedFolders)
+                            {
+                                ownableService.setOwner(nodeRef, adminUserName);
+                            }
                         }
                         else
                         {
@@ -984,7 +1013,19 @@ public class ADMRemoteStore extends BaseRemoteStore
                     rootRef, ContentModel.ASSOC_CONTAINS, assocQName, ContentModel.TYPE_FOLDER, properties);
             surfConfigRef = ref.getChildRef();
             // surf-config needs to be hidden - applies index control aspect as part of the hidden aspect
-            hiddenAspect.hideNode(ref.getChildRef(), false, false, false);
+            hiddenAspect.hideNode(ref.getChildRef(), false, false, false);
+
+            // MNT-16371: Revoke inherited permission
+            permissionService.setInheritParentPermissions(surfConfigRef, false);
+            String siteName = siteService.getSiteShortName(rootRef);
+            if (siteName != null)
+            {
+                // Revoke ownership privileges for surf-config folder, to tighten access for former SiteManagers.
+                ownableService.setOwner(surfConfigRef, AuthenticationUtil.getAdminUserName());
+                // Set site manager group permission
+                String siteManagerGroup = siteService.getSiteRoleGroup(siteName, SiteModel.SITE_MANAGER);
+                permissionService.setPermission(surfConfigRef, siteManagerGroup, SiteModel.SITE_MANAGER, true);
+            }
         }
         return surfConfigRef;
     }

source/test-java/org/alfresco/repo/web/scripts/WebScriptTestSuite.java

@@ -53,7 +53,8 @@ import org.alfresco.repo.web.scripts.rating.RatingRestApiTest;
 import org.alfresco.repo.web.scripts.replication.ReplicationRestApiTest;
 import org.alfresco.repo.web.scripts.rule.RuleServiceTest;
 import org.alfresco.repo.web.scripts.search.PersonSearchTest;
-import org.alfresco.repo.web.scripts.site.SiteServiceTest;
+import org.alfresco.repo.web.scripts.site.SiteServiceTest;
+import org.alfresco.repo.web.scripts.site.SurfConfigTest;
 import org.alfresco.repo.web.scripts.solr.SOLRWebScriptTest;
 import org.alfresco.repo.web.scripts.subscriptions.SubscriptionServiceRestApiTest;
 import org.alfresco.repo.web.scripts.tagging.TaggingServiceTest;
@@ -112,7 +113,8 @@ public class WebScriptTestSuite extends TestSuite
         suite.addTestSuite( LinksRestApiTest.class );
         suite.addTestSuite( RemoteFileFolderLoaderTest.class );
         suite.addTestSuite( ReadOnlyTransactionInGetRestApiTest.class );
-        suite.addTestSuite( CustomModelImportTest.class );
+        suite.addTestSuite( CustomModelImportTest.class );
+        suite.addTestSuite( SurfConfigTest.class );
         // This uses a slightly different context
         // As such, we can't run it in the same suite as the others,
         //  due to finalisers closing caches when we're not looking

source/test-java/org/alfresco/repo/web/scripts/site/SurfConfigTest.java

@@ -0,0 +1,224 @@
+/*
+ * #%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
+ * (at your option) any later version.
+ * 
+ * Alfresco 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 Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.repo.web.scripts.site;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.query.CannedQueryPageDetails;
+import org.alfresco.query.PagingRequest;
+import org.alfresco.query.PagingResults;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
+import org.alfresco.repo.site.SiteModel;
+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.security.AccessPermission;
+import org.alfresco.service.cmr.security.AccessStatus;
+import org.alfresco.service.cmr.security.PermissionService;
+import org.alfresco.service.cmr.site.SiteService;
+import org.alfresco.service.cmr.site.SiteVisibility;
+import org.json.JSONObject;
+import org.springframework.extensions.webscripts.TestWebScriptServer.PostRequest;
+import org.springframework.extensions.webscripts.TestWebScriptServer.PutRequest;
+import org.springframework.extensions.webscripts.TestWebScriptServer.Response;
+
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+
+/**
+ * @author Jamal Kaabi-Mofrad
+ */
+public class SurfConfigTest extends AbstractSiteServiceTest
+{
+    private SiteService siteService;
+    private NodeService nodeService;
+    private PermissionService permissionService;
+    private FileFolderService fileFolderService;
+    private static final long RD = System.currentTimeMillis();
+    private static final String USER_ONE = "SiteUserOne" + RD;
+    private static final String USER_TWO = "SiteUserTwo" + RD;
+    private static final String USER_THREE = "SiteUserThree" + RD;
+    private static final String URL_SITES = "/api/sites";
+    private static final String URL_MEMBERSHIPS = "/memberships";
+    private static final String URL_ADM = "/remoteadm/";
+
+    @Override
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        this.siteService = (SiteService) getServer().getApplicationContext().getBean("SiteService");
+        this.nodeService = (NodeService) getServer().getApplicationContext().getBean("NodeService");
+        this.permissionService = (PermissionService) getServer().getApplicationContext().getBean("PermissionService");
+        this.fileFolderService = (FileFolderService) getServer().getApplicationContext().getBean("FileFolderService");
+
+        createUser(USER_ONE);
+        createUser(USER_TWO);
+        createUser(USER_THREE);
+
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE);
+    }
+
+    @Override
+    protected void tearDown() throws Exception
+    {
+        super.tearDown();
+        AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser();
+        deleteUser(USER_ONE);
+        deleteUser(USER_TWO);
+        deleteUser(USER_THREE);
+
+        //Delete the sites
+        deleteSites();
+
+        AuthenticationUtil.clearCurrentSecurityContext();
+    }
+
+    //MNT-16371
+    public void testSurfConfigPermissions() throws Exception
+    {
+        // Create a site as USER_ONE
+        String shortName = UUID.randomUUID().toString();
+        JSONObject result = createSite("myPreset", shortName, "myTitle", "myDescription", SiteVisibility.PUBLIC, 200);
+        assertEquals("myPreset", result.get("sitePreset"));
+        assertEquals(shortName, result.get("shortName"));
+        assertEquals("myTitle", result.get("title"));
+        assertEquals("myDescription", result.get("description"));
+        assertEquals(SiteVisibility.PUBLIC.toString(), result.get("visibility"));
+
+        // Make ADMRemoteStore to create the surf-config folder and the dashboard.xml file.
+        sendRequest(new PostRequest(URL_ADM + "CREATE/alfresco/site-data/pages/site/" + shortName + "/dashboard.xml?s=sitestore",
+                    new JSONObject().toString(), "application/json"), 200);
+
+        // {siteName}/cm:surf-config/
+        NodeRef surfConfigFolderRef = nodeService
+                    .getChildByName(siteService.getSite(shortName).getNodeRef(), ContentModel.ASSOC_CONTAINS, "surf-config");
+        assertEquals("surf-config", nodeService.getProperty(surfConfigFolderRef, ContentModel.PROP_NAME));
+
+        String owner = (String) nodeService.getProperty(surfConfigFolderRef, ContentModel.PROP_OWNER);
+        assertFalse(USER_ONE.equalsIgnoreCase(owner));
+        assertEquals(AuthenticationUtil.getAdminUserName(), owner);
+
+        assertFalse("Inherit Permissions should be off.", permissionService.getInheritParentPermissions(surfConfigFolderRef));
+
+        Set<AccessPermission> permissions = permissionService.getAllSetPermissions(surfConfigFolderRef);
+        assertEquals(1, permissions.size());
+        String siteManagerGroup = siteService.getSiteRoleGroup(shortName, SiteModel.SITE_MANAGER);
+        AccessPermission accessPermission = permissions.iterator().next();
+        assertEquals(siteManagerGroup, accessPermission.getAuthority());
+        assertEquals(SiteModel.SITE_MANAGER, accessPermission.getPermission());
+        assertTrue(accessPermission.getAccessStatus() == AccessStatus.ALLOWED);
+
+        // This is the method that finally gets called when ALF-21643 steps are followed.
+        PagingResults<FileInfo> pageResults = fileFolderService
+                    .list(surfConfigFolderRef, true, true, null, null, null, new PagingRequest(CannedQueryPageDetails.DEFAULT_PAGE_SIZE));
+        List<FileInfo> fileInfos = pageResults.getPage();
+        assertNotNull(fileInfos);
+        assertEquals(1, fileInfos.size());
+        // {siteName}/cm:surf-config/pages
+        assertEquals("pages", fileInfos.get(0).getName());
+
+        // Add USER_TWO as a site collaborator
+        JSONObject membership = new JSONObject();
+        membership.put("role", SiteModel.SITE_COLLABORATOR);
+        JSONObject person = new JSONObject();
+        person.put("userName", USER_TWO);
+        membership.put("person", person);
+        // Post the membership
+        Response response = sendRequest(new PostRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS, membership.toString(), "application/json"),
+                    200);
+        result = new JSONObject(response.getContentAsString());
+        assertEquals(SiteModel.SITE_COLLABORATOR, result.get("role"));
+        assertEquals(USER_TWO, result.getJSONObject("authority").get("userName"));
+
+        // Add USER_THREE as a site manager
+        membership.put("role", SiteModel.SITE_MANAGER);
+        person.put("userName", USER_THREE);
+        membership.put("person", person);
+        // Post the membership
+        response = sendRequest(new PostRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS, membership.toString(), "application/json"), 200);
+        result = new JSONObject(response.getContentAsString());
+        assertEquals(SiteModel.SITE_MANAGER, result.get("role"));
+        assertEquals(USER_THREE, result.getJSONObject("authority").get("userName"));
+
+        // USER_TWO is a site collaborator so he should not be able to access the surf-config folder
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_TWO);
+        try
+        {
+            fileFolderService.list(surfConfigFolderRef, true, true, null, null, null, new PagingRequest(CannedQueryPageDetails.DEFAULT_PAGE_SIZE));
+            fail("USER_TWO dose not have the appropriate permissions to perform this operation.");
+        }
+        catch (AccessDeniedException ex)
+        {
+            //expected
+        }
+
+        // USER_THREE is a site manager so he is able to access the surf-config folder
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_THREE);
+        pageResults = fileFolderService
+                    .list(surfConfigFolderRef, true, true, null, null, null, new PagingRequest(CannedQueryPageDetails.DEFAULT_PAGE_SIZE));
+        fileInfos = pageResults.getPage();
+        assertNotNull(fileInfos);
+        assertEquals(1, fileInfos.size());
+        // {siteName}/cm:surf-config/pages
+        assertEquals("pages", fileInfos.get(0).getName());
+
+        // Update USER_ONE role from SiteManager to SiteContributor.
+        membership.put("role", SiteModel.SITE_CONTRIBUTOR);
+        person.put("userName", USER_ONE);
+        membership.put("person", person);
+        response = sendRequest(new PutRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS, membership.toString(), "application/json"), 200);
+        result = new JSONObject(response.getContentAsString());
+        assertEquals(SiteModel.SITE_CONTRIBUTOR, result.get("role"));
+        assertEquals(USER_ONE, result.getJSONObject("authority").get("userName"));
+
+        // USER_ONE is no longer a site manager
+        // USER_ONE tries to access "{siteName}/cm:surf-config" children
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE);
+        try
+        {
+            fileFolderService.list(surfConfigFolderRef, true, true, null, null, null, new PagingRequest(CannedQueryPageDetails.DEFAULT_PAGE_SIZE));
+            fail("USER_ONE is not the owner and he is no longer a site manager, so does not have the appropriate permissions to perform this operation");
+        }
+        catch (AccessDeniedException ex)
+        {
+            //expected
+        }
+
+        // USER_ONE tries to access "{siteName}/cm:surf-config/pages" children
+        try
+        {
+            fileFolderService.list(fileInfos.get(0).getNodeRef(), true, true, null, null, null,
+                        new PagingRequest(CannedQueryPageDetails.DEFAULT_PAGE_SIZE));
+            fail("USER_ONE is not the owner and he is no longer a site manager, so does not have the appropriate permissions to perform this operation");
+        }
+        catch (AccessDeniedException ex)
+        {
+            //expected
+        }
+    }
+}