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:31:48 UTC
[sling-org-apache-sling-capabilities] branch master updated
(3c644c5 -> da49ea5)
This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-capabilities.git.
from 3c644c5 SLING-8120 - refactor tests
new ba9c4e6 SLING-8120 - update README for the modified behaviors
new da49ea5 SLING-8120 - remove path restrictions
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
README.md | 46 ++++++++--------------
.../capabilities/internal/CapabilitiesServlet.java | 34 +---------------
.../internal/CapabilitesServletTest.java | 40 ++-----------------
3 files changed, 22 insertions(+), 98 deletions(-)
[sling-org-apache-sling-capabilities] 01/02: SLING-8120 - update
README for the modified behaviors
Posted by bd...@apache.org.
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
commit ba9c4e6b3463c721c67ffffb8e8d0652ff0443c5
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Nov 20 15:26:39 2018 +0100
SLING-8120 - update README for the modified behaviors
---
README.md | 46 ++++++++++++++++------------------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/README.md b/README.md
index a8cc77e..b6bd142 100644
--- a/README.md
+++ b/README.md
@@ -27,19 +27,23 @@ Services that implement the `CapabilitiesSource` interface provide capabilities.
Each service must have its own unique namespace, used to split the capabilities in
categories that can be provided separately (see below):
- @ProviderType
- public interface CapabilitiesSource {
- /** @return the namespace to use to group our capabilities.
- * That name must be unique in a given Sling instance.
- */
- String getNamespace();
- /** @return zero to N capabilities, each being represented by
- * a key/value pair.
- * @throws Exception if the capabilities could not be computed.
- */
- Map<String, Object> getCapabilities() throws Exception;
+ @ProviderType
+ public interface CapabilitiesSource {
+ /** 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(ResourceResolver resolver) throws Exception;
}
The sling/capabilities resource type
@@ -51,21 +55,6 @@ resource type.
A required property named `namespace_patterns` must be present, containing 1..N Java
regexp patterns to select which capabilities namespaces are exposed by this resource.
-Write access to such resources should be strictly controlled to avoid leaking unwanted
-information, along with the CapabilitiesServlet path restrictions described below.
-
-CapabilitiesServlet configuration
----------------------------------
-To restrict access to capabilities, the `CapabilitiesServlet` requires a configuration
-(see example below) that specifies which paths patterns are acceptable for `sling/capabilities`
-resources.
-
-The idea is to strictly control write access to these paths, so that even if users can create
-`sling/capabilities` resources elsewhere they will not expose capabilities data.
-
-If the path of a `sling/capabilities` resource does not match any of the configured patterns,
-the servlet returns a 403 status code saying `Invalid path`.
-
Example configuration
---------------------
This module does not provide any active `CapabilitiesSource` out of the box, but it provides a
@@ -73,7 +62,7 @@ This module does not provide any active `CapabilitiesSource` out of the box, but
`sling.servlet.*` properties for reference.
With the example configuration below a `sling/capabilities` resource with
-`namespace_patterns='org\.apache\.sling\.servlets\.test[A|B]'` and a path that matches `/var/capabilities/.*`
+`namespace_patterns='org\.apache\.sling\.servlets\.test[A|B]'`
produces the following output at `/var/capabilities/caps.json` with the resource shown
below:
@@ -106,9 +95,6 @@ The configured `testC` namespace is omitted due to the `namespace_patterns` prop
Here's the required configuration, excerpted from `/system/console/status-Configurations`:
- PID = org.apache.sling.capabilities.internal.CapabilitiesServlet
- resourcePathPatterns = [/var/capabilities/.*]
-
Factory PID = org.apache.sling.capabilities.defaultsources.SlingServletsSource
capabilitiesNamespaceSuffix = testA
servletsLdapFilter = (&(sling.servlet.extensions=json)(sling.servlet.selectors=acl))
[sling-org-apache-sling-capabilities] 02/02: SLING-8120 - remove
path restrictions
Posted by bd...@apache.org.
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
commit da49ea5616ac18ae3f7e21f929f5a856ab11f843
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Nov 20 15:31:30 2018 +0100
SLING-8120 - remove path restrictions
---
.../capabilities/internal/CapabilitiesServlet.java | 34 ++----------------
.../internal/CapabilitesServletTest.java | 40 +++-------------------
2 files changed, 6 insertions(+), 68 deletions(-)
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 8e7b64c..4e0ab9e 100644
--- a/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java
+++ b/src/main/java/org/apache/sling/capabilities/internal/CapabilitiesServlet.java
@@ -47,30 +47,11 @@ property = {
"sling.servlet.methods=GET",
"sling.servlet.extensions=json"
})
-@Designate(ocd = CapabilitiesServlet.Config.class)
public class CapabilitiesServlet extends SlingSafeMethodsServlet {
- @ObjectClassDefinition(
- name = "Apache Sling Capabilities Servlet",
- description = "Provides information about Sling capabilities"
- )
- public static @interface Config {
- @AttributeDefinition(
- name = "Resource Path Patterns",
- description = "A set of (java) regular expression patterns that the resource path must match for this servlet to execute"
- )
- String [] resourcePathPatterns() default {};
- }
-
private final List<CapabilitiesSource> sources = new CopyOnWriteArrayList<>();
- private RegexFilter pathFilter;
public static final String NAMESPACES_PROP = "namespace_patterns";
- @Activate
- protected void activate(Config cfg, ComponentContext ctx) {
- pathFilter = new RegexFilter(cfg.resourcePathPatterns());
- }
-
@Override
public String toString() {
return getClass().getSimpleName() + ": " + sources.size() + " " + CapabilitiesSource.class.getSimpleName() + " active";
@@ -81,19 +62,8 @@ public class CapabilitiesServlet extends SlingSafeMethodsServlet {
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 = resource.getPath();
- if(!pathFilter.accept(path)) {
- response.sendError(HttpServletResponse.SC_FORBIDDEN, "Invalid path " + path);
- return;
- }
-
- // Resource must define which namespaces are exposed, also for
- // security reasons, to make sure administrators think about
+ // Resource must define which namespaces are exposed,
+ // to make sure administrators think about
// what's exposed
final ValueMap m = resource.adaptTo(ValueMap.class);
final String [] namespacePatterns = m.get(NAMESPACES_PROP, String[].class);
diff --git a/src/test/java/org/apache/sling/capabilities/internal/CapabilitesServletTest.java b/src/test/java/org/apache/sling/capabilities/internal/CapabilitesServletTest.java
index 1d3546d..9534083 100644
--- a/src/test/java/org/apache/sling/capabilities/internal/CapabilitesServletTest.java
+++ b/src/test/java/org/apache/sling/capabilities/internal/CapabilitesServletTest.java
@@ -60,22 +60,12 @@ public class CapabilitesServletTest {
private BundleContext bundleContext;
private ResourceResolver resourceResolver;
- // The CapabilitiesServlet must reject resource paths which are outside of this
- private static final String [] AUTHORIZED_PATHS_PATTERNS = {
- ".*/ok$",
- "/var/.*"
- };
-
// The CapabilitiesServlet must omit capabilities outside of these namespaces
private static final String [] NAMESPACE_PATTERNS = {
"[EF]",
"G"
};
- private final String DENIED_PATH = "/denied";
- private final String OK_PATH = "/denied/but/ok";
- private final String VAR_PATH = "/var/something";
-
private static final CapabilitiesSource [] SOURCES = {
new MockSource("F", 2),
new MockSource("G", 43),
@@ -85,14 +75,6 @@ public class CapabilitesServletTest {
@Before
public void setup() throws IOException {
- // Configure allowed path patterns
- final ConfigurationAdmin ca = context.getService(ConfigurationAdmin.class);
- assertNotNull("Expecting a ConfigurationAdmin service", ca);
- final Configuration cfg = ca.getConfiguration(CapabilitiesServlet.class.getName());
- final Dictionary<String, Object> props = new Hashtable<>();
- props.put("resourcePathPatterns", AUTHORIZED_PATHS_PATTERNS);
- cfg.update(props);
-
servlet = new CapabilitiesServlet();
bundleContext = MockOsgi.newBundleContext();
resourceResolver = MockSling.newResourceResolver(bundleContext);
@@ -107,13 +89,13 @@ public class CapabilitesServletTest {
context.registerInjectActivateService(servlet);
}
- private MockSlingHttpServletRequest requestFor(String path, boolean withNamespacePatterns) {
+ private MockSlingHttpServletRequest testRequest(boolean withNamespacePatterns) {
final MockSlingHttpServletRequest req = new MockSlingHttpServletRequest(resourceResolver);
final Map<String, Object> props = new HashMap<>();
if(withNamespacePatterns) {
props.put(CapabilitiesServlet.NAMESPACES_PROP, NAMESPACE_PATTERNS);
}
- final MockResource res = new MockResource(path, props, resourceResolver);
+ final MockResource res = new MockResource("/", props, resourceResolver);
req.setResource(res);
return req;
}
@@ -132,30 +114,16 @@ public class CapabilitesServletTest {
}
@Test
- public void testDeniedPath() throws ServletException, IOException {
- MockSlingHttpServletResponse resp = new MockSlingHttpServletResponse();
- servlet.service(requestFor(DENIED_PATH, true), resp);
- assertEquals(403, resp.getStatus());
- }
-
- @Test
- public void testOkPath() throws ServletException, IOException {
- MockSlingHttpServletResponse resp = new MockSlingHttpServletResponse();
- servlet.service(requestFor(OK_PATH, true), resp);
- assertEquals(200, resp.getStatus());
- }
-
- @Test
public void testMissingNamespaceProperty() throws ServletException, IOException {
MockSlingHttpServletResponse resp = new MockSlingHttpServletResponse();
- servlet.service(requestFor(OK_PATH, false), resp);
+ servlet.service(testRequest(false), resp);
assertEquals(403, resp.getStatus());
}
@Test
public void testServletResponse() throws ServletException, IOException {
MockSlingHttpServletResponse resp = new MockSlingHttpServletResponse();
- servlet.service(requestFor(VAR_PATH, true), resp);
+ servlet.service(testRequest(true), resp);
assertEquals(200, resp.getStatus());
// Just verify that both sources are taken into account