From 739a9f06737976738c6c48b6674602d6ca58ae89 Mon Sep 17 00:00:00 2001 From: Vikas Saurabh Date: Sat, 6 Oct 2018 09:10:05 +0530 Subject: [PATCH] OAK-7808: Incorrect facet counts when some results are inaccessible due to ACLs --- ...FilteredSortedSetDocValuesFacetCounts.java | 76 +++++++----- .../jackrabbit/oak/jcr/query/FacetTest.java | 109 +++++++++++++++++- 2 files changed, 152 insertions(+), 33 deletions(-) diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/FilteredSortedSetDocValuesFacetCounts.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/FilteredSortedSetDocValuesFacetCounts.java index 05340a5a82..eb04387110 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/FilteredSortedSetDocValuesFacetCounts.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/util/FilteredSortedSetDocValuesFacetCounts.java @@ -19,9 +19,9 @@ package org.apache.jackrabbit.oak.plugins.index.lucene.util; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; +import java.util.Set; +import com.google.common.collect.Sets; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.lucene.document.Document; @@ -34,9 +34,9 @@ import org.apache.lucene.facet.sortedset.SortedSetDocValuesFacetCounts; import org.apache.lucene.facet.sortedset.SortedSetDocValuesReaderState; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.TermsEnum; import org.apache.lucene.search.ScoreDoc; import org.apache.lucene.search.TopDocs; -import org.apache.lucene.util.BytesRef; /** * ACL filtered version of {@link SortedSetDocValuesFacetCounts} @@ -80,44 +80,56 @@ class FilteredSortedSetDocValuesFacetCounts extends SortedSetDocValuesFacetCount } private LabelAndValue[] filterFacet(int docId, String dimension, LabelAndValue[] labelAndValues) throws IOException { - boolean filterd = false; - Map newValues = new HashMap(); - Document document = reader.document(docId); - SortedSetDocValues docValues = state.getDocValues(); - docValues.setDocument(docId); // filter using doc values (avoiding requiring stored values) if (!filter.isAccessible(document.getField(FieldNames.PATH).stringValue() + "/" + dimension)) { - filterd = true; - for (LabelAndValue lv : labelAndValues) { - long existingCount = lv.value.longValue(); - - BytesRef key = new BytesRef(FacetsConfig.pathToString(dimension, new String[]{lv.label})); - long l = docValues.lookupTerm(key); - if (l >= 0) { - if (existingCount > 0) { - newValues.put(lv.label, existingCount - 1); - } else { - if (newValues.containsKey(lv.label)) { - newValues.remove(lv.label); - } + Set inaccessibleLabels = Sets.newHashSet(); + + SortedSetDocValues docValues = state.getDocValues(); + docValues.setDocument(docId); + TermsEnum termsEnum = docValues.termsEnum(); + + long ord = docValues.nextOrd(); + + while (ord != SortedSetDocValues.NO_MORE_ORDS) { + termsEnum.seekExact(ord); + String facetDVTerm = termsEnum.term().utf8ToString(); + String [] facetDVDimPaths = FacetsConfig.stringToPath(facetDVTerm); + + for (int i = 1; i < facetDVDimPaths.length; i++) { + inaccessibleLabels.add(facetDVDimPaths[i]); + } + + ord = docValues.nextOrd(); + } + + int numZeros = 0; + for (int i = 0; i < labelAndValues.length; i++) { + LabelAndValue lv = labelAndValues[i]; + if (inaccessibleLabels.contains(lv.label)) { + long originalValue = lv.value.longValue(); + labelAndValues[i] = new LabelAndValue(lv.label, originalValue- 1); + if (originalValue == 1) { + numZeros++; } } } - } - LabelAndValue[] filteredLVs; - if (filterd) { - filteredLVs = new LabelAndValue[newValues.size()]; - int i = 0; - for (Map.Entry entry : newValues.entrySet()) { - filteredLVs[i] = new LabelAndValue(entry.getKey(), entry.getValue()); - i++; + + if (numZeros > 0) { + LabelAndValue[] newArray = new LabelAndValue[labelAndValues.length - numZeros]; + + int i = 0; + for (LabelAndValue lv : labelAndValues) { + if (lv.value.longValue() > 0) { + newArray[i++] = lv; + } + } + + labelAndValues = newArray; } - } else { - filteredLVs = labelAndValues; } - return filteredLVs; + return labelAndValues; } } \ No newline at end of file diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java index 3b3fa2748b..0267f2f267 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/jcr/query/FacetTest.java @@ -26,10 +26,13 @@ import javax.jcr.query.Query; import javax.jcr.query.QueryManager; import javax.jcr.query.QueryResult; import javax.jcr.query.RowIterator; +import javax.jcr.security.Privilege; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; import org.apache.jackrabbit.core.query.AbstractQueryTest; -import org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants; import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants; import org.apache.jackrabbit.oak.query.facet.FacetResult; import org.junit.After; @@ -633,6 +636,110 @@ public class FacetTest extends AbstractQueryTest { assertFalse(rows.hasNext()); } + public void testNoFacetsIfNoAccess() throws Exception { + deny(testRootNode.addNode("test1")).setProperty("jcr:title", "test1"); + deny(testRootNode.addNode("test2")).addNode("child").setProperty("jcr:title", "test2"); + deny(testRootNode.addNode("test3").addNode("child")).setProperty("jcr:title", "test3"); + superuser.save(); + + Session anonUser = getHelper().getReadOnlySession(); + QueryManager queryManager = anonUser.getWorkspace().getQueryManager(); + Query q = queryManager.createQuery("//*[@jcr:title]/(rep:facet(jcr:title))", Query.XPATH); + QueryResult result = q.execute(); + FacetResult facetResult = new FacetResult(result); + + assertNotNull("facetResult is null", facetResult); + assertTrue(facetResult.getDimensions().isEmpty()); + } + + public void testOnlyAllowedFacetLabelsShowUp() throws Exception { + deny(testRootNode.addNode("test1")).setProperty("jcr:title", "test1"); + deny(testRootNode.addNode("test2")).addNode("child").setProperty("jcr:title", "test2"); + testRootNode.addNode("test3").addNode("child").setProperty("jcr:title", "test3"); + superuser.save(); + + Session anonUser = getHelper().getReadOnlySession(); + QueryManager queryManager = anonUser.getWorkspace().getQueryManager(); + Query q = queryManager.createQuery("//*[@jcr:title]/(rep:facet(jcr:title))", Query.XPATH); + QueryResult result = q.execute(); + FacetResult facetResult = new FacetResult(result); + + assertNotNull("facetResult is null", facetResult); + assertEquals("Unexpected number of dimension", 1, facetResult.getFacets("jcr:title").size()); + FacetResult.Facet facet = facetResult.getFacets("jcr:title").get(0); + assertEquals("Unexpected facet label", "test3", facet.getLabel()); + assertEquals("Unexpected facet count", 1, facet.getCount()); + } + + public void testInaccessibleFacetCounts() throws Exception { + deny(testRootNode.addNode("test1")).setProperty("jcr:title", "test"); + deny(testRootNode.addNode("test2")).addNode("child").setProperty("jcr:title", "test"); + testRootNode.addNode("test3").addNode("child").setProperty("jcr:title", "test"); + testRootNode.addNode("test4").addNode("child").setProperty("jcr:title", "another-test"); + superuser.save(); + + Session anonUser = getHelper().getReadOnlySession(); + QueryManager queryManager = anonUser.getWorkspace().getQueryManager(); + Query q = queryManager.createQuery("//*[@jcr:title]/(rep:facet(jcr:title))", Query.XPATH); + QueryResult result = q.execute(); + FacetResult facetResult = new FacetResult(result); + + assertNotNull("facetResult is null", facetResult); + assertEquals("Unexpected number of labels", 2, facetResult.getFacets("jcr:title").size()); + Map facets = facetResult.getFacets("jcr:title") + .stream().collect(Collectors.toMap(FacetResult.Facet::getLabel, FacetResult.Facet::getCount)); + assertEquals("Unexpected facet count for jcr:title", 1, (int)facets.get("test")); + assertEquals("Unexpected facet count for jcr:title", 1, (int)facets.get("another-test")); + } + + public void testAllowedSubNodeFacet() throws Exception { + allow( + deny(testRootNode.addNode("parent")).addNode("child") + ).setProperty("jcr:title", "test"); + superuser.save(); + + Session anonUser = getHelper().getReadOnlySession(); + QueryManager queryManager = anonUser.getWorkspace().getQueryManager(); + Query q = queryManager.createQuery("//*[@jcr:title]/(rep:facet(jcr:title))", Query.XPATH); + QueryResult result = q.execute(); + FacetResult facetResult = new FacetResult(result); + + assertNotNull("facetResult is null", facetResult); + assertEquals("Unexpected number of labels", 1, facetResult.getFacets("jcr:title").size()); + FacetResult.Facet facet = facetResult.getFacets("jcr:title").get(0); + assertEquals("Unexpected facet label", "test", facet.getLabel()); + assertEquals("Unexpected facet count", 1, facet.getCount()); + } + + public void testAcRelativeFacetsAccessControl() throws Exception { + deny(testRootNode.addNode("test1")).addNode("jc").setProperty("text", "test_1"); + deny(testRootNode.addNode("test2").addNode("jc")).setProperty("text", "test_2"); + testRootNode.addNode("test3").addNode("jc").setProperty("text", "test_3"); + superuser.save(); + + Session anonUser = getHelper().getReadOnlySession(); + QueryManager queryManager = anonUser.getWorkspace().getQueryManager(); + Query q = queryManager.createQuery("//*[jcr:contains(jc/@text, 'test')]/(rep:facet(jc/text))", Query.XPATH); + QueryResult result = q.execute(); + FacetResult facetResult = new FacetResult(result); + + assertNotNull("facetResult is null", facetResult); + assertEquals("Unexpected number of dimension", 1, facetResult.getFacets("jc/text").size()); + FacetResult.Facet facet = facetResult.getFacets("jc/text").get(0); + assertEquals("Unexpected facet label", "test_3", facet.getLabel()); + assertEquals("Unexpected facet count", 1, facet.getCount()); + } + + public Node deny(Node node) throws RepositoryException { + AccessControlUtils.deny(node, "anonymous", Privilege.JCR_ALL); + return node; + } + + public Node allow(Node node) throws RepositoryException { + AccessControlUtils.allow(node, "anonymous", Privilege.JCR_READ); + return node; + } + private void markIndexForReindex() throws RepositoryException { superuser.getNode("/oak:index/luceneGlobal").setProperty(REINDEX_PROPERTY_NAME, true); } -- 2.18.0