You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2018/11/20 14:08:46 UTC
[sling-org-apache-sling-capabilities] branch master updated:
SLING-8120 - pass the ResourceResolver to CapabilitiesSource
This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-capabilities.git
The following commit(s) were added to refs/heads/master by this push:
new c21a907 SLING-8120 - pass the ResourceResolver to CapabilitiesSource
c21a907 is described below
commit c21a90748de50c3ee07f82beb63df5697fc77b6d
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Nov 20 15:08:30 2018 +0100
SLING-8120 - pass the ResourceResolver to CapabilitiesSource
---
pom.xml | 6 ++++
.../sling/capabilities/CapabilitiesSource.java | 15 +++++++--
.../defaultsources/SlingServletsSource.java | 11 +++++--
.../capabilities/internal/CapabilitiesServlet.java | 9 ++++--
.../internal/JSONCapabilitiesWriter.java | 5 +--
.../defaultsources/SlingServletsSourceTest.java | 2 +-
.../internal/JSONCapabilitiesWriterTest.java | 36 +++++++++++++++++++---
.../sling/capabilities/internal/MockSource.java | 18 ++++++++---
.../capabilities/it/CapabilitiesBundleIT.java | 6 +++-
9 files changed, 86 insertions(+), 22 deletions(-)
diff --git a/pom.xml b/pom.xml
index 354a4e5..33637a5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -204,5 +204,11 @@
<version>${org.ops4j.pax.exam.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>2.23.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git a/src/main/java/org/apache/sling/capabilities/CapabilitiesSource.java b/src/main/java/org/apache/sling/capabilities/CapabilitiesSource.java
index e6ec9d5..62744e6 100644
--- a/src/main/java/org/apache/sling/capabilities/CapabilitiesSource.java
+++ b/src/main/java/org/apache/sling/capabilities/CapabilitiesSource.java
@@ -19,6 +19,7 @@
package org.apache.sling.capabilities;
import java.util.Map;
+import org.apache.sling.api.resource.ResourceResolver;
import org.osgi.annotation.versioning.ProviderType;
/** A CapabilitiesSource provides capabilities, as a Map of key/value
@@ -35,9 +36,17 @@ public interface CapabilitiesSource {
*/
String getNamespace();
- /** @return zero to N capabilities, each being represented by
- * a key/value pair.
+ /** Return zero to N capabilities, each being represented by
+ * a key/value pair.
+ *
+ * Services implementing this interface must be careful to
+ * avoid crossing trust boundaries. They should only expose data that
+ * is accessible to the ResourceResolver that's passed
+ * as a parameter.
+ *
+ * @return a Map of capabilities
+ * @param resolver used to establish the user's identity
* @throws Exception if the capabilities could not be computed.
*/
- Map<String, Object> getCapabilities() throws Exception;
+ Map<String, Object> getCapabilities(ResourceResolver resolver) throws Exception;
}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/capabilities/defaultsources/SlingServletsSource.java b/src/main/java/org/apache/sling/capabilities/defaultsources/SlingServletsSource.java
index bdb6e64..0de2c01 100644
--- a/src/main/java/org/apache/sling/capabilities/defaultsources/SlingServletsSource.java
+++ b/src/main/java/org/apache/sling/capabilities/defaultsources/SlingServletsSource.java
@@ -22,6 +22,7 @@ import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;
import javax.servlet.Servlet;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.capabilities.CapabilitiesSource;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceReference;
@@ -35,6 +36,10 @@ import org.osgi.service.metatype.annotations.ObjectClassDefinition;
/** Default CapabilitiesSource that provides information on available Sling
* Servlets, exposing their sling.servlet.* properties so that clients can
* find out which behaviors are available.
+ *
+ * This should only be used to expose servlets to which all users have access,
+ * generic functions such as searches etc., to avoid unwanted information
+ * disclosure.
*/
@Component(service = CapabilitiesSource.class)
@Designate(
@@ -50,7 +55,9 @@ public class SlingServletsSource implements CapabilitiesSource {
public static @interface Config {
@AttributeDefinition(
name = "LDAP filter",
- description = "OSGi LDAP filter to select servlets to consider for the provided capabilites"
+ description = "OSGi LDAP filter to select servlets to consider for the provided capabilites. "
+ + "This should only expose general purpose servlets to which all users have access, like "
+ + "search functionality and similar features."
)
String servletsLdapFilter() default "";
@@ -83,7 +90,7 @@ public class SlingServletsSource implements CapabilitiesSource {
}
@Override
- public Map<String, Object> getCapabilities() throws Exception {
+ public Map<String, Object> getCapabilities(ResourceResolver resolver) throws Exception {
final Map<String, Object> result = new HashMap<>();
final ServiceReference [] refs = bundleContext.getServiceReferences(Servlet.class.getName(), ldapFilter);
if(refs != null) {
diff --git a/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java b/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java
index b1a91a0..8e7b64c 100644
--- a/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java
+++ b/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java
@@ -26,6 +26,7 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletResponse;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
import org.apache.sling.capabilities.CapabilitiesSource;
@@ -78,12 +79,14 @@ public class CapabilitiesServlet extends SlingSafeMethodsServlet {
@Override
protected void doGet(SlingHttpServletRequest request, SlingHttpServletResponse response) throws ServletException, IOException {
+ final Resource resource = request.getResource();
+
// Resource Path must match a configurable set of patterns, to prevent
// users from getting this information by just creating a resource anywhere
// with our resource type. The idea is that those paths will have suitable
// access control (readonly for users) to control exactly which capabilities
// are exposed.
- final String path = request.getResource().getPath();
+ final String path = resource.getPath();
if(!pathFilter.accept(path)) {
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Invalid path " + path);
return;
@@ -92,7 +95,7 @@ public class CapabilitiesServlet extends SlingSafeMethodsServlet {
// Resource must define which namespaces are exposed, also for
// security reasons, to make sure administrators think about
// what's exposed
- final ValueMap m = request.getResource().adaptTo(ValueMap.class);
+ final ValueMap m = resource.adaptTo(ValueMap.class);
final String [] namespacePatterns = m.get(NAMESPACES_PROP, String[].class);
if(namespacePatterns == null) {
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Missing property " + NAMESPACES_PROP);
@@ -102,7 +105,7 @@ public class CapabilitiesServlet extends SlingSafeMethodsServlet {
// All good, get capabilities
response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");
- new JSONCapabilitiesWriter().writeJson(response.getWriter(), sources, new RegexFilter(namespacePatterns));
+ new JSONCapabilitiesWriter().writeJson(resource.getResourceResolver(), response.getWriter(), sources, new RegexFilter(namespacePatterns));
response.getWriter().flush();
}
diff --git a/src/main/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriter.java b/src/main/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriter.java
index bd85590..818ef28 100644
--- a/src/main/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriter.java
+++ b/src/main/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriter.java
@@ -26,6 +26,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.apache.felix.utils.json.JSONWriter;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.capabilities.CapabilitiesSource;
/** Create the JSON output of our servlet */
@@ -35,7 +36,7 @@ class JSONCapabilitiesWriter {
static final String DATA_KEY = "data";
/** Write JSON to the supplied Writer, using the supplied sources */
- void writeJson(Writer w, Collection<CapabilitiesSource> sources, RegexFilter namespacePatterns) throws IOException {
+ void writeJson(ResourceResolver resolver, Writer w, Collection<CapabilitiesSource> sources, RegexFilter namespacePatterns) throws IOException {
final Set<String> namespaces = new HashSet<>();
final JSONWriter jw = new JSONWriter(w);
@@ -58,7 +59,7 @@ class JSONCapabilitiesWriter {
namespaces.add(namespace);
try {
- values = s.getCapabilities();
+ values = s.getCapabilities(resolver);
} catch(Exception e) {
values = new HashMap<>();
values.put("_EXCEPTION_", e.getClass().getName() + ":" + e.getMessage());
diff --git a/src/test/java/org/apache/sling/capabilities/defaultsources/SlingServletsSourceTest.java b/src/test/java/org/apache/sling/capabilities/defaultsources/SlingServletsSourceTest.java
index 9503603..e95562a 100644
--- a/src/test/java/org/apache/sling/capabilities/defaultsources/SlingServletsSourceTest.java
+++ b/src/test/java/org/apache/sling/capabilities/defaultsources/SlingServletsSourceTest.java
@@ -85,7 +85,7 @@ public class SlingServletsSourceTest {
assertNotNull("Expecting a CapabilitiesSource", src);
assertEquals("Expecting namespace to match", "org.apache.sling.servlets.TEST_NS", src.getNamespace());
- final Map<String, Object> caps = src.getCapabilities();
+ final Map<String, Object> caps = src.getCapabilities(null);
assertNotNull("Expecting to get Capabilities", caps);
assertEquals("Expecting capabilities for 2 json servlets", 2, caps.size());
diff --git a/src/test/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriterTest.java b/src/test/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriterTest.java
index 27ee0aa..2cd2dfe 100644
--- a/src/test/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriterTest.java
+++ b/src/test/java/org/apache/sling/capabilities/internal/JSONCapabilitiesWriterTest.java
@@ -23,17 +23,43 @@ import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.List;
+import java.util.UUID;
import javax.json.Json;
import javax.json.JsonObject;
import javax.json.JsonReader;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.capabilities.CapabilitiesSource;
import static org.junit.Assert.assertEquals;
+import org.junit.BeforeClass;
import org.junit.Test;
+import org.mockito.Mockito;
/** Test the JSONCapabilitiesWriter */
public class JSONCapabilitiesWriterTest {
private final RegexFilter namespaceFilter = new RegexFilter(".*");
+ private static ResourceResolver resolver;
+ private static final String RESOLVER_STRING = "resolver-" + UUID.randomUUID();
+
+ @BeforeClass
+ public static void setupMocks() {
+ resolver = Mockito.mock(ResourceResolver.class);
+ Mockito.when(resolver.toString()).thenReturn(RESOLVER_STRING);
+ }
+
+ @Test
+ public void testResolverIsUsed() throws IOException {
+ final List<CapabilitiesSource> sources = new ArrayList<>();
+ sources.add(new MockSource("A", 2));
+
+ final StringWriter w = new StringWriter();
+ new JSONCapabilitiesWriter().writeJson(resolver, w, sources, namespaceFilter);
+ final JsonReader r = Json.createReader(new StringReader(w.toString()));
+ final JsonObject rootJson = r.readObject();
+ final JsonObject json = rootJson.getJsonObject(JSONCapabilitiesWriter.CAPS_KEY).getJsonObject("data");
+
+ assertEquals(RESOLVER_STRING, json.getJsonObject("A").getString(ResourceResolver.class.getSimpleName()));
+ }
@Test
public void testWithTwoSources() throws IOException {
@@ -42,7 +68,7 @@ public class JSONCapabilitiesWriterTest {
sources.add(new MockSource("B", 1));
final StringWriter w = new StringWriter();
- new JSONCapabilitiesWriter().writeJson(w, sources, namespaceFilter);
+ new JSONCapabilitiesWriter().writeJson(resolver, w, sources, namespaceFilter);
final JsonReader r = Json.createReader(new StringReader(w.toString()));
final JsonObject rootJson = r.readObject();
@@ -52,8 +78,8 @@ public class JSONCapabilitiesWriterTest {
assertEquals("VALUE_0_B", json.getJsonObject("B").getString("KEY_0_B"));
assertEquals("Expecting 1 root key", 1, rootJson.keySet().size());
- assertEquals("Expecting 2 keys at A", 2, json.getJsonObject("A").keySet().size());
- assertEquals("Expecting 1 key at B", 1, json.getJsonObject("B").keySet().size());
+ assertEquals("Expecting 3 keys at A", 3, json.getJsonObject("A").keySet().size());
+ assertEquals("Expecting 2 key at B", 2, json.getJsonObject("B").keySet().size());
}
@Test
@@ -64,7 +90,7 @@ public class JSONCapabilitiesWriterTest {
sources.add(new MockSource("B", 1));
final StringWriter w = new StringWriter();
- new JSONCapabilitiesWriter().writeJson(w, sources, namespaceFilter);
+ new JSONCapabilitiesWriter().writeJson(resolver, w, sources, namespaceFilter);
final JsonReader r = Json.createReader(new StringReader(w.toString()));
final JsonObject rootJson = r.readObject();
@@ -84,6 +110,6 @@ public class JSONCapabilitiesWriterTest {
sources.add(new MockSource("duplicate", 1));
final StringWriter w = new StringWriter();
- new JSONCapabilitiesWriter().writeJson(w, sources, namespaceFilter);
+ new JSONCapabilitiesWriter().writeJson(resolver, w, sources, namespaceFilter);
}
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/capabilities/internal/MockSource.java b/src/test/java/org/apache/sling/capabilities/internal/MockSource.java
index 50b8652..a5bb934 100644
--- a/src/test/java/org/apache/sling/capabilities/internal/MockSource.java
+++ b/src/test/java/org/apache/sling/capabilities/internal/MockSource.java
@@ -21,25 +21,33 @@ package org.apache.sling.capabilities.internal;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.capabilities.CapabilitiesSource;
class MockSource implements CapabilitiesSource {
private final String namespace;
- private final Map<String, Object> props = new HashMap<>();
+ private final int propsCount;
MockSource(String namespace, int propsCount) {
this.namespace = namespace;
- for (int i = 0; i < propsCount; i++) {
- props.put("KEY_" + i + "_" + namespace, "VALUE_" + i + "_" + namespace);
- }
+ this.propsCount = propsCount;
}
@Override
- public Map<String, Object> getCapabilities() throws Exception {
+ public Map<String, Object> getCapabilities(ResourceResolver resolver) throws Exception {
if (namespace.contains("EXCEPTION")) {
throw new IllegalArgumentException("Simulating a problem");
}
+ return getProps(resolver);
+ }
+
+ private Map<String, Object> getProps(ResourceResolver resolver) {
+ final Map<String, Object> props = new HashMap<>();
+ for (int i = 0; i < propsCount; i++) {
+ props.put("KEY_" + i + "_" + namespace, "VALUE_" + i + "_" + namespace);
+ }
+ props.put(ResourceResolver.class.getSimpleName(), resolver.toString());
return Collections.unmodifiableMap(props);
}
diff --git a/src/test/java/org/apache/sling/capabilities/it/CapabilitiesBundleIT.java b/src/test/java/org/apache/sling/capabilities/it/CapabilitiesBundleIT.java
index 610c2cf..abd2bfd 100644
--- a/src/test/java/org/apache/sling/capabilities/it/CapabilitiesBundleIT.java
+++ b/src/test/java/org/apache/sling/capabilities/it/CapabilitiesBundleIT.java
@@ -21,6 +21,7 @@ package org.apache.sling.capabilities.it;
import java.util.Map;
import javax.inject.Inject;
import javax.servlet.Servlet;
+import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.capabilities.CapabilitiesSource;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -47,8 +48,11 @@ public class CapabilitiesBundleIT extends CapabilitiesTestSupport {
private Bundle testBundle;
private static class TestCapabilitiesSource implements CapabilitiesSource {
+ @Override
public String getNamespace() { return null; }
- public Map<String, Object> getCapabilities() { return null; }
+
+ @Override
+ public Map<String, Object> getCapabilities(ResourceResolver unused) { return null; }
};
@Before