This is an automated email from the ASF dual-hosted git repository. dklco pushed a commit to branch SLING-8913-multiple-instance-types in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git
commit ec1affbe693f97761cafd05a3613c2b70e422776 Author: Dan Klco <[email protected]> AuthorDate: Thu Aug 27 17:01:32 2020 -0400 Resolving issues where the security filter was not taking publication status into consideration and made it more configurable --- .../filters/CMSSecurityConfigInstance.java | 81 +++++++++++++++++++ .../core/internal/filters/CMSSecurityFilter.java | 93 ++++++++++------------ .../internal/filters/CMSSecurityFilterTest.java | 59 ++++++-------- 3 files changed, 148 insertions(+), 85 deletions(-) diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityConfigInstance.java b/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityConfigInstance.java new file mode 100644 index 0000000..377cf43 --- /dev/null +++ b/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityConfigInstance.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.cms.core.internal.filters; + +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.lang3.ArrayUtils; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Modified; +import org.osgi.service.metatype.annotations.Designate; + +@Component(service = { CMSSecurityConfigInstance.class }) +@Designate(ocd = CMSSecurityFilterConfig.class, factory = true) +public class CMSSecurityConfigInstance { + + private CMSSecurityFilterConfig config; + private final List<Pattern> patterns = new ArrayList<>(); + + @Modified + @Activate + public void activate(CMSSecurityFilterConfig config) { + this.config = config; + + if (config.allowedPatterns() != null) { + for (String p : config.allowedPatterns()) { + patterns.add(Pattern.compile(p)); + } + } + + } + + public boolean applies(HttpServletRequest request) { + return ArrayUtils.isEmpty(config.hostDomains()) + || ArrayUtils.contains(config.hostDomains(), request.getServerName()); + } + + /** + * @return the config + */ + public CMSSecurityFilterConfig getConfig() { + return config; + } + + public String getGroupName() { + return config.group(); + } + + /** + * @param config the config to set + */ + public void setConfig(CMSSecurityFilterConfig config) { + this.config = config; + } + + /** + * @return the patterns + */ + public List<Pattern> getPatterns() { + return patterns; + } + +} \ No newline at end of file diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilter.java b/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilter.java index 19fe002..f1f87de 100644 --- a/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilter.java +++ b/core/src/main/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilter.java @@ -17,8 +17,6 @@ package org.apache.sling.cms.core.internal.filters; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.regex.Pattern; @@ -33,7 +31,6 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.apache.jackrabbit.api.JackrabbitSession; import org.apache.jackrabbit.api.security.user.Authorizable; @@ -41,11 +38,13 @@ import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.api.security.user.User; import org.apache.jackrabbit.api.security.user.UserManager; import org.apache.sling.api.SlingHttpServletRequest; -import org.osgi.service.component.annotations.Activate; +import org.apache.sling.cms.PublishableResource; +import org.apache.sling.cms.publication.PUBLICATION_MODE; +import org.apache.sling.cms.publication.PublicationManagerFactory; import org.osgi.service.component.annotations.Component; -import org.osgi.service.component.annotations.ConfigurationPolicy; -import org.osgi.service.component.annotations.Modified; -import org.osgi.service.metatype.annotations.Designate; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicyOption; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,33 +52,16 @@ import org.slf4j.LoggerFactory; * Checks to ensure that the user is logged in for requests which otherwise * would be allowed when accessing through a CMS-specific domain name. */ -@Component(service = { Filter.class }, property = { - "sling.filter.scope=request" }, configurationPolicy = ConfigurationPolicy.REQUIRE) -@Designate(ocd = CMSSecurityFilterConfig.class) +@Component(service = { Filter.class }, property = { "sling.filter.scope=request" }, immediate = true) public class CMSSecurityFilter implements Filter { private static final Logger log = LoggerFactory.getLogger(CMSSecurityFilter.class); - private CMSSecurityFilterConfig config; + @Reference(cardinality = ReferenceCardinality.MULTIPLE, policyOption = ReferencePolicyOption.GREEDY) + private List<CMSSecurityConfigInstance> securityConfigInstances; - private List<Pattern> patterns = new ArrayList<>(); - - @Modified - @Activate - public void activate(CMSSecurityFilterConfig config) { - if (config.hostDomains() != null && config.hostDomains().length > 0) { - if (log.isInfoEnabled()) { - log.info("Applying CMS Security Filter for domains {}", Arrays.toString(config.hostDomains())); - } - this.config = config; - for (String p : config.allowedPatterns()) { - patterns.add(Pattern.compile(p)); - } - } else { - this.config = null; - log.info("No host domains supplied, CMS Security Filter not enabled"); - } - } + @Reference + private PublicationManagerFactory pubMgrFactory; @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -90,26 +72,33 @@ public class CMSSecurityFilter implements Filter { public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - SlingHttpServletRequest slingRequest = (SlingHttpServletRequest) request; - if (config != null && ArrayUtils.contains(config.hostDomains(), request.getServerName())) { - boolean allowed = checkAllowed(request, slingRequest); - - // permission checked failed, so return an unauthorized error - if (!allowed) { - log.trace("Request to {} not allowed for user {}", slingRequest.getRequestURI(), - slingRequest.getResourceResolver().getUserID()); - ((HttpServletResponse) response).sendError(401); - return; + if (pubMgrFactory.getPublicationMode() == PUBLICATION_MODE.STANDALONE) { + SlingHttpServletRequest slingRequest = (SlingHttpServletRequest) request; + for (CMSSecurityConfigInstance securityConfig : securityConfigInstances) { + log.trace("Checking to see if security config {} applies to request", securityConfig); + if (securityConfig.applies(slingRequest)) { + boolean allowed = checkAllowed(securityConfig, slingRequest); + // permission checked failed, so return an unauthorized error + if (!allowed) { + log.trace("Request to {} not allowed for user {}", slingRequest.getRequestURI(), + slingRequest.getResourceResolver().getUserID()); + ((HttpServletResponse) response).sendError(401); + return; + } + } } + } else { + log.trace("Publication mode {} is not standalone", pubMgrFactory.getPublicationMode()); } + chain.doFilter(request, response); } - private boolean checkAllowed(ServletRequest request, SlingHttpServletRequest slingRequest) { - log.trace("Filtering requests to host {}", request.getServerName()); + private boolean checkAllowed(CMSSecurityConfigInstance securityConfig, SlingHttpServletRequest slingRequest) { + log.trace("Filtering requests to host {}", slingRequest.getServerName()); String uri = slingRequest.getRequestURI(); boolean allowed = false; - for (Pattern p : this.patterns) { + for (Pattern p : securityConfig.getPatterns()) { if (p.matcher(uri).matches()) { log.trace("Allowing request matching pattern {}", p); allowed = true; @@ -117,15 +106,19 @@ public class CMSSecurityFilter implements Filter { } } + PublishableResource publishableResource = slingRequest.getResource().adaptTo(PublishableResource.class); + if (publishableResource.isPublished()) { + allowed = true; + } + // the uri isn't allowed automatically, so check user permissions if (!allowed) { log.trace("Request to {} not allowed, checking user permissions", uri); // check to see if the user is a member of the specified group - if (StringUtils.isNotBlank(config.group())) { - allowed = checkGroupMembership(slingRequest); - - // just check to make sure the user is logged in + if (StringUtils.isNotBlank(securityConfig.getGroupName())) { + allowed = checkGroupMembership(securityConfig, slingRequest); } else { + // just check to make sure the user is logged in if (!"anonymous".equals(slingRequest.getResourceResolver().getUserID())) { allowed = true; } @@ -136,7 +129,8 @@ public class CMSSecurityFilter implements Filter { return allowed; } - private boolean checkGroupMembership(SlingHttpServletRequest slingRequest) { + private boolean checkGroupMembership(CMSSecurityConfigInstance securityConfig, + SlingHttpServletRequest slingRequest) { boolean allowed = false; try { Session session = slingRequest.getResourceResolver().adaptTo(Session.class); @@ -157,10 +151,11 @@ public class CMSSecurityFilter implements Filter { return false; } - log.trace("Checking to see if user {} is in required group {}", auth.getID(), config.group()); + log.trace("Checking to see if user {} is in required group {}", auth.getID(), + securityConfig.getGroupName()); Iterator<Group> groups = ((User) auth).memberOf(); while (groups.hasNext()) { - if (groups.next().getID().equals(config.group())) { + if (groups.next().getID().equals(securityConfig.getGroupName())) { allowed = true; break; } diff --git a/core/src/test/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilterTest.java b/core/src/test/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilterTest.java index 793e1cc..7edc430 100644 --- a/core/src/test/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilterTest.java +++ b/core/src/test/java/org/apache/sling/cms/core/internal/filters/CMSSecurityFilterTest.java @@ -26,9 +26,9 @@ import javax.jcr.UnsupportedRepositoryOperationException; import javax.servlet.FilterChain; import javax.servlet.ServletException; -import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.cms.core.helpers.SlingCMSTestHelper; -import org.apache.sling.servlethelpers.MockSlingHttpServletRequest; +import org.apache.sling.cms.publication.PUBLICATION_MODE; +import org.apache.sling.cms.publication.PublicationManagerFactory; import org.apache.sling.testing.mock.sling.junit.SlingContext; import org.junit.Before; import org.junit.Rule; @@ -44,48 +44,31 @@ public class CMSSecurityFilterTest { @Before public void init() throws UnsupportedRepositoryOperationException, RepositoryException, IOException { SlingCMSTestHelper.initAuthContext(context); - securityFilter = new CMSSecurityFilter(); } @Test - public void testNotMatchingDomain() throws IOException, ServletException { - context.request().setServerName("localhost"); - securityFilter.activate(new CMSSecurityFilterConfig() { + public void testNotEnabled() throws IOException, ServletException { - @Override - public Class<? extends Annotation> annotationType() { - return null; - } + PublicationManagerFactory factory = Mockito.mock(PublicationManagerFactory.class); + Mockito.when(factory.getPublicationMode()).thenReturn(PUBLICATION_MODE.CONTENT_DISTRIBUTION); - @Override - public String[] hostDomains() { - return new String[] { "cms.danklco.com" }; - } + context.registerService(PublicationManagerFactory.class, factory); - @Override - public String[] allowedPatterns() { - return new String[0]; - } + securityFilter = context.registerInjectActivateService(new CMSSecurityFilter()); - @Override - public String group() { - return null; - } - - }); securityFilter.doFilter(context.request(), context.response(), Mockito.mock(FilterChain.class)); assertEquals(200, context.response().getStatus()); } @Test - public void testMatchingDomain() throws IOException, ServletException { - ResourceResolver resolver = Mockito.mock(ResourceResolver.class); - Mockito.when(resolver.getUserID()).thenReturn("anonymous"); - MockSlingHttpServletRequest request = new MockSlingHttpServletRequest(resolver); - request.setServerName("cms.danklco.com"); - request.setServletPath("/content/apache/sling-apache/org.html"); + public void testNotMatchingDomain() throws IOException, ServletException { + + PublicationManagerFactory factory = Mockito.mock(PublicationManagerFactory.class); + Mockito.when(factory.getPublicationMode()).thenReturn(PUBLICATION_MODE.STANDALONE); + context.registerService(PublicationManagerFactory.class, factory); - securityFilter.activate(new CMSSecurityFilterConfig() { + CMSSecurityConfigInstance config = new CMSSecurityConfigInstance(); + config.activate(new CMSSecurityFilterConfig() { @Override public Class<? extends Annotation> annotationType() { @@ -94,12 +77,12 @@ public class CMSSecurityFilterTest { @Override public String[] hostDomains() { - return new String[] { "cms.danklco.com" }; + return new String[] { "cms.apache.org" }; } @Override public String[] allowedPatterns() { - return new String[0]; + return null; } @Override @@ -108,8 +91,12 @@ public class CMSSecurityFilterTest { } }); - securityFilter.doFilter(request, context.response(), Mockito.mock(FilterChain.class)); - assertEquals(401, context.response().getStatus()); - } + context.registerService(CMSSecurityConfigInstance.class, config); + + securityFilter = context.registerInjectActivateService(new CMSSecurityFilter()); + context.request().setRemoteHost("www.apache.org"); + securityFilter.doFilter(context.request(), context.response(), Mockito.mock(FilterChain.class)); + assertEquals(200, context.response().getStatus()); + } } \ No newline at end of file
