ACS-6928 handling restricted nodes in queries API (#3387)

remote-api/src/main/java/org/alfresco/rest/api/impl/QueriesImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Remote API
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2025 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software.
  * If the software was purchased under a paid Alfresco license, the terms of
@@ -38,14 +38,19 @@ import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.extensions.surf.util.I18NUtil;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.query.PagingRequest;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.site.SiteModel;
 import org.alfresco.rest.api.Nodes;
 import org.alfresco.rest.api.People;
@@ -89,6 +94,7 @@ import org.alfresco.util.SearchLanguageConversion;
  */
 public class QueriesImpl implements Queries, InitializingBean
 {
+    private static final Log LOGGER = LogFactory.getLog(QueriesImpl.class);
     private final static Map<String, QName> NODE_SORT_PARAMS_TO_QNAMES = sortParamsToQNames(
             PARAM_NAME, ContentModel.PROP_NAME,
             PARAM_CREATEDAT, ContentModel.PROP_CREATED,
@@ -247,7 +253,7 @@ public class QueriesImpl implements Queries, InitializingBean
                             {
                                 // first request for this namespace prefix, get and cache result
                                 Collection<String> prefixes = namespaceService.getPrefixes(qname.getNamespaceURI());
-                                prefix = prefixes.size() != 0 ? prefixes.iterator().next() : "";
+                                prefix = !prefixes.isEmpty() ? prefixes.iterator().next() : "";
                                 cache.put(qname.getNamespaceURI(), prefix);
                             }
                             buf.append('/').append(prefix).append(':').append(ISO9075.encode(qname.getLocalName()));
@@ -261,12 +267,6 @@ public class QueriesImpl implements Queries, InitializingBean
                 return buf.toString();
             }
 
-            @Override
-            protected List<Node> newList(int capacity)
-            {
-                return new ArrayList<Node>(capacity);
-            }
-
             @Override
             protected Node convert(NodeRef nodeRef, List<String> includeParam)
             {
@@ -303,18 +303,11 @@ public class QueriesImpl implements Queries, InitializingBean
                 query.append("*\")");
             }
 
-            @Override
-            protected List<Person> newList(int capacity)
-            {
-                return new ArrayList<Person>(capacity);
-            }
-
             @Override
             protected Person convert(NodeRef nodeRef, List<String> includeParam)
             {
                 String personId = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_USERNAME);
-                Person person = people.getPerson(personId);
+                return people.getPerson(personId);
-                return person;
             }
 
             // TODO Do the sort in the query on day. A comment in the code for the V0 API used for live people
@@ -341,28 +334,18 @@ public class QueriesImpl implements Queries, InitializingBean
                 query.append("*\")");
             }
 
-            @Override
-            protected List<Site> newList(int capacity)
-            {
-                return new ArrayList<>(capacity);
-            }
-
             @Override
             protected Site convert(NodeRef nodeRef, List<String> includeParam)
             {
-                return getSite(siteService.getSite(nodeRef), true);
+                return getSite(siteService.getSite(nodeRef));
             }
 
             // note: see also Sites.getSite
-            private Site getSite(SiteInfo siteInfo, boolean includeRole)
+            private Site getSite(SiteInfo siteInfo)
             {
                 // set the site id to the short name (to deal with case sensitivity issues with using the siteId from the url)
                 String siteId = siteInfo.getShortName();
-                String role = null;
+                String role = sites.getSiteRole(siteId);
-                if (includeRole)
-                {
-                    role = sites.getSiteRole(siteId);
-                }
                 return new Site(siteInfo, role);
             }
         }.find(parameters, PARAM_TERM, MIN_TERM_LENGTH_SITES, "_SITE", POST_QUERY_SORT, SITE_SORT_PARAMS_TO_QNAMES, new SortColumn(PARAM_SITE_TITLE, true));
@@ -412,34 +395,38 @@ public class QueriesImpl implements Queries, InitializingBean
             }
 
             ResultSet queryResults = null;
-            List<T> collection = null;
             try
             {
                 queryResults = searchService.query(sp);
 
                 List<NodeRef> nodeRefs = queryResults.getNodeRefs();
-
+                Map<NodeRef, T> collection = new LinkedHashMap<>(nodeRefs.size());
-                if (sort == POST_QUERY_SORT)
-                {
-                    nodeRefs = postQuerySort(parameters, sortParamsToQNames, defaultSortCols, nodeRefs);
-                }
-
-                collection = newList(nodeRefs.size());
                 List<String> includeParam = parameters.getInclude();
 
                 for (NodeRef nodeRef : nodeRefs)
                 {
-                    T t = convert(nodeRef, includeParam);
+                    try
-                    collection.add(t);
+                    {
+                        T t = convert(nodeRef, includeParam);
+                        collection.put(nodeRef, t);
+                    }
+                    catch (AccessDeniedException ade)
+                    {
+                        LOGGER.debug("Ignoring search result for nodeRef " + nodeRef + " due to access denied exception", ade);
+                    }
                 }
 
                 if (sort == POST_QUERY_SORT)
                 {
-                    return listPage(collection, paging);
+                    List<T> postQuerySortedCollection = postQuerySort(parameters, sortParamsToQNames, defaultSortCols, collection.keySet())
+                            .stream()
+                            .map(collection::get)
+                            .toList();
+                    return listPage(postQuerySortedCollection, paging);
                 }
                 else
                 {
-                    return CollectionWithPagingInfo.asPaged(paging, collection, queryResults.hasMore(), Long.valueOf(queryResults.getNumberFound()).intValue());
+                    return CollectionWithPagingInfo.asPaged(paging, collection.values(), queryResults.hasMore(), Long.valueOf(queryResults.getNumberFound()).intValue());
                 }
             }
             finally
@@ -464,15 +451,6 @@ public class QueriesImpl implements Queries, InitializingBean
          */
         protected abstract void buildQuery(StringBuilder query, String term, SearchParameters sp, String queryTemplateName);
 
-        /**
-         * Returns a list of the correct type.
-         * 
-         * @param capacity
-         *            of the list
-         * @return a new list.
-         */
-        protected abstract List<T> newList(int capacity);
-
         /**
          * Converts a nodeRef into the an object of the required type.
          * 
@@ -551,7 +529,7 @@ public class QueriesImpl implements Queries, InitializingBean
         private List<SortColumn> getSorting(Parameters parameters, List<SortColumn> defaultSortCols)
         {
             List<SortColumn> sortCols = parameters.getSorting();
-            if (sortCols == null || sortCols.size() == 0)
+            if (sortCols == null || sortCols.isEmpty())
             {
                 sortCols = defaultSortCols == null ? Collections.emptyList() : defaultSortCols;
             }
@@ -559,62 +537,66 @@ public class QueriesImpl implements Queries, InitializingBean
         }
 
         protected List<NodeRef> postQuerySort(Parameters parameters, Map<String, QName> sortParamsToQNames,
-                List<SortColumn> defaultSortCols, List<NodeRef> nodeRefs)
+                List<SortColumn> defaultSortCols, Set<NodeRef> unsortedNodeRefs)
         {
             final List<SortColumn> sortCols = getSorting(parameters, defaultSortCols);
             int sortColCount = sortCols.size();
-            if (sortColCount > 0)
-            {
-                // make copy of nodeRefs because it can be unmodifiable list.
-                nodeRefs = new ArrayList<NodeRef>(nodeRefs);
 
-                List<QName> sortPropQNames = new ArrayList<>(sortColCount);
+            if (sortColCount == 0)
-                for (SortColumn sortCol : sortCols)
+            {
+                return new ArrayList<>(unsortedNodeRefs);
+            }
+
+            // make copy of nodeRefs because it can be unmodifiable list.
+            List<NodeRef> sortedNodeRefs = new ArrayList<>(unsortedNodeRefs);
+
+            List<QName> sortPropQNames = new ArrayList<>(sortColCount);
+            for (SortColumn sortCol : sortCols)
+            {
+                QName sortPropQName = sortParamsToQNames.get(sortCol.column);
+                if (sortPropQName == null)
                 {
-                    QName sortPropQName = sortParamsToQNames.get(sortCol.column);
+                    throw new InvalidArgumentException("Invalid sort field: " + sortCol.column);
-                    if (sortPropQName == null)
+                }
+                sortPropQNames.add(sortPropQName);
+            }
+
+            final Collator col = AlfrescoCollator.getInstance(I18NUtil.getLocale());
+            Collections.sort(sortedNodeRefs, new Comparator<NodeRef>() {
+                @Override
+                public int compare(NodeRef n1, NodeRef n2)
+                {
+                    int result = 0;
+                    for (int i = 0; i < sortCols.size(); i++)
                     {
-                        throw new InvalidArgumentException("Invalid sort field: " + sortCol.column);
+                        SortColumn sortCol = sortCols.get(i);
+                        QName sortPropQName = sortPropQNames.get(i);
+
+                        Serializable p1 = getProperty(n1, sortPropQName);
+                        Serializable p2 = getProperty(n2, sortPropQName);
+
+                        result = ((p1 instanceof Long) && (p2 instanceof Long)
+                                ? Long.compare((Long) p1, (Long) p2)
+                                : col.compare(p1.toString(), p2.toString()))
+                                * (sortCol.asc ? 1 : -1);
+
+                        if (result != 0)
+                        {
+                            break;
+                        }
                     }
-                    sortPropQNames.add(sortPropQName);
+                    return result;
                 }
 
-                final Collator col = AlfrescoCollator.getInstance(I18NUtil.getLocale());
+                private Serializable getProperty(NodeRef nodeRef, QName sortPropQName)
-                Collections.sort(nodeRefs, new Comparator<NodeRef>() {
+                {
-                    @Override
+                    Serializable result = nodeService.getProperty(nodeRef, sortPropQName);
-                    public int compare(NodeRef n1, NodeRef n2)
+                    return result == null ? "" : result;
-                    {
+                }
-                        int result = 0;
-                        for (int i = 0; i < sortCols.size(); i++)
-                        {
-                            SortColumn sortCol = sortCols.get(i);
-                            QName sortPropQName = sortPropQNames.get(i);
 
-                            Serializable p1 = getProperty(n1, sortPropQName);
+            });
-                            Serializable p2 = getProperty(n2, sortPropQName);
 
-                            result = ((p1 instanceof Long) && (p2 instanceof Long)
+            return sortedNodeRefs;
-                                    ? Long.compare((Long) p1, (Long) p2)
-                                    : col.compare(p1.toString(), p2.toString()))
-                                    * (sortCol.asc ? 1 : -1);
-
-                            if (result != 0)
-                            {
-                                break;
-                            }
-                        }
-                        return result;
-                    }
-
-                    private Serializable getProperty(NodeRef nodeRef, QName sortPropQName)
-                    {
-                        Serializable result = nodeService.getProperty(nodeRef, sortPropQName);
-                        return result == null ? "" : result;
-                    }
-
-                });
-            }
-            return nodeRefs;
         }
 
         // note: see also AbstractNodeRelation

remote-api/src/test/java/org/alfresco/rest/api/tests/QueriesSitesApiTest.java

@@ -130,6 +130,7 @@ public class QueriesSitesApiTest extends AbstractSingleNetworkSiteTest
     public void testLiveSearchSites() throws Exception
     {
         setRequestContext(user1);
+        AuthenticationUtil.setFullyAuthenticatedUser(user1);
 
         int sCount = 5;
         assertTrue(sCount > 4); // as relied on by test below
@@ -231,7 +232,11 @@ public class QueriesSitesApiTest extends AbstractSingleNetworkSiteTest
 
     private NodeRef getNodeRef(String createdSiteId)
     {
-        AuthenticationUtil.setFullyAuthenticatedUser(user1);
+        // Created sites do not return NodeRefs to the caller so we need to get the NodeRef from the siteService.
+        // Temporarily as admin we will get NodeRefs to handle ACL authorization.
+        String userUnderTest = AuthenticationUtil.getFullyAuthenticatedUser();
+        AuthenticationUtil.setFullyAuthenticatedUser(DEFAULT_ADMIN);
+
         // The following call to siteService.getSite(createdSiteId).getNodeRef() returns a NodeRef like:
         // workspace://SpacesStore/9db76769-96de-4de4-bdb4-a127130af362
         // We call tenantService.getName(nodeRef) to get a fully qualified NodeRef as Solr returns this.
@@ -239,6 +244,8 @@ public class QueriesSitesApiTest extends AbstractSingleNetworkSiteTest
         // workspace://@org.alfresco.rest.api.tests.queriespeopleapitest@SpacesStore/9db76769-96de-4de4-bdb4-a127130af362
         NodeRef nodeRef = siteService.getSite(createdSiteId).getNodeRef();
         nodeRef = tenantService.getName(nodeRef);
+
+        AuthenticationUtil.setFullyAuthenticatedUser(userUnderTest);
         return nodeRef;
     }
 
@@ -246,6 +253,7 @@ public class QueriesSitesApiTest extends AbstractSingleNetworkSiteTest
     public void testLiveSearchSites_SortPage() throws Exception
     {
         setRequestContext(user1);
+        AuthenticationUtil.setFullyAuthenticatedUser(user1);
 
         List<String> siteIds = new ArrayList<>(5);
 
@@ -306,6 +314,51 @@ public class QueriesSitesApiTest extends AbstractSingleNetworkSiteTest
         }
     }
 
+    /**
+     * If the search service do not support ACL filtering, then the Queries API should handle the response to exclude private sites and potential unauthorized error when building response.
+     */
+    @Test
+    public void testLiveSearchExcludesPrivateSites() throws Exception
+    {
+        String publicSiteId = null;
+        String privateSiteId = null;
+        try
+        {
+            // given
+            setRequestContext(null, DEFAULT_ADMIN, DEFAULT_ADMIN_PWD);
+            createUser("bartender");
+
+            publicSiteId = createSite("samePrefixPublicSite", "samePrefixPublicSite", "Visible to all users", SiteVisibility.PUBLIC, 201).getId();
+            privateSiteId = createSite("samePrefixPrivateSite", "samePrefixPrivateSite", "Hidden from bartender", SiteVisibility.PRIVATE, 201).getId();
+
+            String[] searchResults = {publicSiteId, privateSiteId};
+            String[] expectedSites = {publicSiteId};
+
+            // when
+            setRequestContext(null, "bartender", "password");
+            AuthenticationUtil.setFullyAuthenticatedUser("bartender");
+
+            // then
+            checkApiCall("samePrefix", null, getPaging(0, 100), 200, expectedSites, searchResults);
+        }
+        finally
+        {
+            // cleanup
+            AuthenticationUtil.setFullyAuthenticatedUser(DEFAULT_ADMIN);
+            setRequestContext(null, DEFAULT_ADMIN, DEFAULT_ADMIN_PWD);
+            if (publicSiteId != null)
+            {
+                deleteSite(publicSiteId, true, 204);
+            }
+            if (privateSiteId != null)
+            {
+                deleteSite(privateSiteId, true, 204);
+            }
+            deleteUser("bartender", null);
+        }
+
+    }
+
     @Override
     public String getScope()
     {