This is an automated email from the ASF dual-hosted git repository. sagarmiglani pushed a commit to branch revert-6-SLING-12814 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceaccesssecurity.git
commit d72c87682916a6e38aab57cd202dd53ea8e3feb9 Author: Sagar Miglani <[email protected]> AuthorDate: Tue Jun 3 15:16:17 2025 +0530 Revert "SLING-12814 - Update to Parent 62" --- .git-blame-ignore-revs | 2 - .sling-module.json | 5 - pom.xml | 21 ++-- .../AllowingResourceAccessGate.java | 8 +- .../resourceaccesssecurity/ResourceAccessGate.java | 20 ++-- .../impl/AccessGateResourceWrapper.java | 20 ++-- .../ApplicationResourceAccessSecurityImpl.java | 13 +-- .../impl/ProviderResourceAccessSecurityImpl.java | 12 +- .../impl/ReadOnlyValueMapWrapper.java | 4 +- .../impl/ResourceAccessGateHandler.java | 33 +++--- .../impl/ResourceAccessSecurityImpl.java | 127 +++++++++------------ .../sling/resourceaccesssecurity/package-info.java | 1 + .../impl/ResourceAccessGateHandlerTest.java | 8 +- .../impl/ResourceAccessSecurityImplTests.java | 77 +++++++------ 14 files changed, 154 insertions(+), 197 deletions(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs deleted file mode 100644 index 3bf6666..0000000 --- a/.git-blame-ignore-revs +++ /dev/null @@ -1,2 +0,0 @@ -# spotless:apply formatting changes -3abf31760e9961804016916694201b1c52983089 \ No newline at end of file diff --git a/.sling-module.json b/.sling-module.json deleted file mode 100644 index cfad4d2..0000000 --- a/.sling-module.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "jenkins": { - "jdks": [17, 21] - } -} \ No newline at end of file diff --git a/pom.xml b/pom.xml index eaea71b..b636ce2 100644 --- a/pom.xml +++ b/pom.xml @@ -1,4 +1,4 @@ -<?xml version="1.0" encoding="UTF-8"?> +<?xml version="1.0" encoding="ISO-8859-1"?> <!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file @@ -22,7 +22,7 @@ <parent> <groupId>org.apache.sling</groupId> <artifactId>sling-bundle-parent</artifactId> - <version>62</version> + <version>46</version> <relativePath /> </parent> @@ -30,14 +30,16 @@ <version>1.1.1-SNAPSHOT</version> <name>Apache Sling Resource Access Security</name> - <description>This bundle provides an implementation of the ResourceAccessSecurity service</description> + <description> + This bundle provides an implementation of the ResourceAccessSecurity service + </description> <scm> <connection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceaccesssecurity.git</connection> <developerConnection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceaccesssecurity.git</developerConnection> - <tag>HEAD</tag> <url>https://github.com/apache/sling-org-apache-sling-resourceaccesssecurity.git</url> - </scm> + <tag>HEAD</tag> + </scm> <properties> <site.javadoc.exclude>**.internal.**</site.javadoc.exclude> @@ -49,32 +51,26 @@ <dependency> <groupId>javax.jcr</groupId> <artifactId>jcr</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.framework</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.annotation.versioning</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.service.component</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.service.component.annotations</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-api</artifactId> - <scope>provided</scope> </dependency> <dependency> <groupId>org.apache.sling</groupId> @@ -92,7 +88,6 @@ <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> - <scope>test</scope> </dependency> <dependency> <groupId>org.mockito</groupId> @@ -100,5 +95,5 @@ <version>4.0.0</version> <scope>test</scope> </dependency> - </dependencies> + </dependencies> </project> diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/AllowingResourceAccessGate.java b/src/main/java/org/apache/sling/resourceaccesssecurity/AllowingResourceAccessGate.java index 34ab259..86c2662 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/AllowingResourceAccessGate.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/AllowingResourceAccessGate.java @@ -36,7 +36,8 @@ public abstract class AllowingResourceAccessGate implements ResourceAccessGate { } @Override - public GateResult canCreate(final String absPathName, final ResourceResolver resourceResolver) { + public GateResult canCreate(final String absPathName, + final ResourceResolver resourceResolver) { return GateResult.CANT_DECIDE; } @@ -80,9 +81,10 @@ public abstract class AllowingResourceAccessGate implements ResourceAccessGate { return GateResult.CANT_DECIDE; } + @Override - public String transformQuery(final String query, final String language, final ResourceResolver resourceResolver) - throws AccessSecurityException { + public String transformQuery(final String query, final String language, + final ResourceResolver resourceResolver) throws AccessSecurityException { return query; } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/ResourceAccessGate.java b/src/main/java/org/apache/sling/resourceaccesssecurity/ResourceAccessGate.java index 772e12a..c809a37 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/ResourceAccessGate.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/ResourceAccessGate.java @@ -122,18 +122,12 @@ public interface ResourceAccessGate { * </ul> */ public enum GateResult { - GRANTED, - DENIED, - CANT_DECIDE + GRANTED, DENIED, CANT_DECIDE }; public enum Operation { - READ("read"), - CREATE("create"), - UPDATE("update"), - DELETE("delete"), - EXECUTE("execute"), - ORDER_CHILDREN("order-children"); + READ("read"), CREATE("create"), UPDATE("update"), DELETE("delete"), EXECUTE( + "execute"), ORDER_CHILDREN("order-children"); private String text; @@ -161,7 +155,8 @@ public interface ResourceAccessGate { public GateResult canRead(Resource resource); - public GateResult canCreate(String absPathName, ResourceResolver resourceResolver); + public GateResult canCreate(String absPathName, + ResourceResolver resourceResolver); public default GateResult canOrderChildren(Resource resource) { return GateResult.CANT_DECIDE; @@ -199,8 +194,8 @@ public interface ResourceAccessGate { * took place. This method should never return <code>null</code> * @throws AccessSecurityException */ - public String transformQuery(String query, String language, ResourceResolver resourceResolver) - throws AccessSecurityException; + public String transformQuery(String query, String language, + ResourceResolver resourceResolver) throws AccessSecurityException; /* for convenience (and performance) */ public boolean hasReadRestrictions(ResourceResolver resourceResolver); @@ -224,4 +219,5 @@ public interface ResourceAccessGate { public boolean canUpdateAllValues(Resource resource); public boolean canDeleteAllValues(Resource resource); + } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/AccessGateResourceWrapper.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/AccessGateResourceWrapper.java index b2f394d..f57c1c2 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/AccessGateResourceWrapper.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/AccessGateResourceWrapper.java @@ -43,17 +43,16 @@ public class AccessGateResourceWrapper extends ResourceWrapper { * Creates a new wrapper instance delegating all method calls to the given * <code>resource</code>, but intercepts the calls with checks to the * applied ResourceAccessGate instances for read and/or update values. - * + * * @param resource resource to protect - * @param accessGatesForReadForValues list of access gates to ask when reading values. If + * @param accessGatesForReadForValues list of access gates to ask when reading values. If * the list is <code>null</code> or empty there are no read restrictions * @param modifiable if <code>true</code> the resource can be updated */ - public AccessGateResourceWrapper( - @NotNull final Resource resource, - final List<ResourceAccessGate> accessGatesForReadForValues, - final boolean modifiable) { - super(resource); + public AccessGateResourceWrapper(@NotNull final Resource resource, + final List<ResourceAccessGate> accessGatesForReadForValues, + final boolean modifiable ) { + super( resource ); this.modifiable = modifiable; } @@ -70,7 +69,8 @@ public class AccessGateResourceWrapper extends ResourceWrapper { if (adapter != null && !modifiable) { if (type == ModifiableValueMap.class) { adapter = null; - } else if (type == Map.class || type == ValueMap.class) { + } + else if (type == Map.class || type == ValueMap.class) { // protect also against accidental modifications when changes are done in an adapted map adapter = (AdapterType) new ReadOnlyValueMapWrapper((Map) adapter); } @@ -78,5 +78,9 @@ public class AccessGateResourceWrapper extends ResourceWrapper { // TODO: restrict reading of certain values return adapter; + + } + + } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ApplicationResourceAccessSecurityImpl.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ApplicationResourceAccessSecurityImpl.java index 5712885..794cb25 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ApplicationResourceAccessSecurityImpl.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ApplicationResourceAccessSecurityImpl.java @@ -30,22 +30,15 @@ import org.osgi.service.component.annotations.Reference; import org.osgi.service.component.annotations.ReferenceCardinality; import org.osgi.service.component.annotations.ReferencePolicyOption; -@Component( - service = ResourceAccessSecurity.class, - property = ResourceAccessSecurity.CONTEXT + "=" + ResourceAccessSecurity.APPLICATION_CONTEXT) +@Component(service = ResourceAccessSecurity.class, property = ResourceAccessSecurity.CONTEXT+"="+ResourceAccessSecurity.APPLICATION_CONTEXT) public class ApplicationResourceAccessSecurityImpl extends ResourceAccessSecurityImpl { private static final String RESOURCE_ACCESS_GATE_REFERENCE_NAME = "resourceAccessGates"; @Activate public ApplicationResourceAccessSecurityImpl( - @Reference( - name = RESOURCE_ACCESS_GATE_REFERENCE_NAME, - cardinality = ReferenceCardinality.AT_LEAST_ONE, - policyOption = ReferencePolicyOption.GREEDY, - target = "(" + ResourceAccessGate.CONTEXT + "=" + ResourceAccessGate.APPLICATION_CONTEXT - + ")") - List<ServiceReference<ResourceAccessGate>> resourceAccessGateRefs, + @Reference(name = RESOURCE_ACCESS_GATE_REFERENCE_NAME, cardinality = ReferenceCardinality.AT_LEAST_ONE, policyOption = ReferencePolicyOption.GREEDY, target ="(" + ResourceAccessGate.CONTEXT + "=" + ResourceAccessGate.APPLICATION_CONTEXT + ")") + List<ServiceReference<ResourceAccessGate>> resourceAccessGateRefs, ComponentContext componentContext) { super(false, resourceAccessGateRefs, componentContext, RESOURCE_ACCESS_GATE_REFERENCE_NAME); } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ProviderResourceAccessSecurityImpl.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ProviderResourceAccessSecurityImpl.java index f6c2348..e3fad67 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ProviderResourceAccessSecurityImpl.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ProviderResourceAccessSecurityImpl.java @@ -30,21 +30,15 @@ import org.osgi.service.component.annotations.Reference; import org.osgi.service.component.annotations.ReferenceCardinality; import org.osgi.service.component.annotations.ReferencePolicyOption; -@Component( - service = ResourceAccessSecurity.class, - property = ResourceAccessSecurity.CONTEXT + "=" + ResourceAccessSecurity.PROVIDER_CONTEXT) +@Component(service = ResourceAccessSecurity.class, property = ResourceAccessSecurity.CONTEXT + "=" + ResourceAccessSecurity.PROVIDER_CONTEXT) public class ProviderResourceAccessSecurityImpl extends ResourceAccessSecurityImpl { private static final String RESOURCE_ACCESS_GATE_REFERENCE_NAME = "resourceAccessGates"; @Activate public ProviderResourceAccessSecurityImpl( - @Reference( - name = RESOURCE_ACCESS_GATE_REFERENCE_NAME, - cardinality = ReferenceCardinality.AT_LEAST_ONE, - policyOption = ReferencePolicyOption.GREEDY, - target = "(" + ResourceAccessGate.CONTEXT + "=" + ResourceAccessGate.PROVIDER_CONTEXT + ")") - List<ServiceReference<ResourceAccessGate>> resourceAccessGates, + @Reference(name = RESOURCE_ACCESS_GATE_REFERENCE_NAME, cardinality = ReferenceCardinality.AT_LEAST_ONE, policyOption = ReferencePolicyOption.GREEDY, target ="(" + ResourceAccessGate.CONTEXT + "=" + ResourceAccessGate.PROVIDER_CONTEXT + ")") + List<ServiceReference<ResourceAccessGate>> resourceAccessGates, ComponentContext componentContext) { super(false, resourceAccessGates, componentContext, RESOURCE_ACCESS_GATE_REFERENCE_NAME); } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ReadOnlyValueMapWrapper.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ReadOnlyValueMapWrapper.java index 31fe652..43ac8db 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ReadOnlyValueMapWrapper.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ReadOnlyValueMapWrapper.java @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ + package org.apache.sling.resourceaccesssecurity.impl; import java.util.Map; @@ -26,7 +27,8 @@ import org.apache.sling.api.wrappers.ValueMapDecorator; /** * Wrapper class that does protect the underlying map from modifications. */ -public class ReadOnlyValueMapWrapper extends ValueMapDecorator implements ValueMap { +public class ReadOnlyValueMapWrapper extends ValueMapDecorator + implements ValueMap { /** * Creates a new wrapper around a given map. diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandler.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandler.java index 80e51e5..1dbea1a 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandler.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandler.java @@ -42,50 +42,47 @@ public class ResourceAccessGateHandler { /** * constructor */ - public ResourceAccessGateHandler( - @NotNull final ServiceReference<ResourceAccessGate> resourceAccessGateRef, - @NotNull final ResourceAccessGate resourceAccessGate) { + public ResourceAccessGateHandler( @NotNull final ServiceReference<ResourceAccessGate> resourceAccessGateRef, @NotNull final ResourceAccessGate resourceAccessGate ) { this.reference = resourceAccessGateRef; this.resourceAccessGate = resourceAccessGate; // extract the service property "path" final String path = (String) resourceAccessGateRef.getProperty(ResourceAccessGate.PATH); - if (path != null) { + if ( path != null ) { pathPattern = Pattern.compile(path); } else { pathPattern = Pattern.compile(".*"); } // extract the service property "operations" - final String[] ops = - PropertiesUtil.toStringArray(resourceAccessGateRef.getProperty(ResourceAccessGate.OPERATIONS)); - if (ops != null && ops.length > 0) { + final String[] ops = PropertiesUtil.toStringArray( resourceAccessGateRef.getProperty(ResourceAccessGate.OPERATIONS) ); + if ( ops != null && ops.length > 0) { for (final String opAsString : ops) { final ResourceAccessGate.Operation operation = ResourceAccessGate.Operation.fromString(opAsString); - if (operation != null) { + if ( operation != null ) { operations.add(operation); } } } else { - Collections.addAll(operations, ResourceAccessGate.Operation.values()); + Collections.addAll(operations, ResourceAccessGate.Operation.values()); } // extract the service property "finaloperations" - final String[] finalOps = - PropertiesUtil.toStringArray(resourceAccessGateRef.getProperty(ResourceAccessGate.FINALOPERATIONS)); - if (finalOps != null) { + final String[] finalOps = PropertiesUtil.toStringArray( resourceAccessGateRef.getProperty(ResourceAccessGate.FINALOPERATIONS) ); + if ( finalOps != null ) { for (final String opAsString : finalOps) { final ResourceAccessGate.Operation operation = ResourceAccessGate.Operation.fromString(opAsString); - if (operation != null) { + if ( operation != null ) { finalOperations.add(operation); } } } + } - public boolean matches(final String path, final ResourceAccessGate.Operation operation) { + public boolean matches( final String path, final ResourceAccessGate.Operation operation ) { boolean returnValue = false; - if (operations.contains(operation)) { + if ( operations.contains( operation ) ) { if (path != null) { final Matcher match = pathPattern.matcher(path); returnValue = match.matches(); @@ -99,7 +96,7 @@ public class ResourceAccessGateHandler { return returnValue; } - public boolean isFinalOperation(final ResourceAccessGate.Operation operation) { + public boolean isFinalOperation( final ResourceAccessGate.Operation operation ) { return finalOperations.contains(operation); } @@ -109,8 +106,8 @@ public class ResourceAccessGateHandler { @Override public boolean equals(final Object obj) { - if (obj instanceof ResourceAccessGateHandler) { - return ((ResourceAccessGateHandler) obj).reference.equals(this.reference); + if ( obj instanceof ResourceAccessGateHandler ) { + return ((ResourceAccessGateHandler)obj).reference.equals(this.reference); } return false; } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImpl.java b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImpl.java index 12080cb..14c60cf 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImpl.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImpl.java @@ -42,20 +42,14 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri private final boolean defaultAllowIfNoGateMatches; - protected ResourceAccessSecurityImpl( - final boolean defaultAllowIfNoGateMatches, - List<ServiceReference<ResourceAccessGate>> resourceAccessGateRefs, - ComponentContext componentContext, - String resourceAccessGateReferenceName) { + protected ResourceAccessSecurityImpl(final boolean defaultAllowIfNoGateMatches, List<ServiceReference<ResourceAccessGate>> resourceAccessGateRefs, + ComponentContext componentContext, String resourceAccessGateReferenceName) { this.defaultAllowIfNoGateMatches = defaultAllowIfNoGateMatches; // sort from highest ranked service to lowest ranked (opposite of default sorting of ServiceReference) - this.allHandlers = resourceAccessGateRefs.stream() - .sorted(Collections.reverseOrder()) - .map(ref -> new ResourceAccessGateHandler( - ref, componentContext.locateService(resourceAccessGateReferenceName, ref))) - .collect(Collectors.toList()); + this.allHandlers = resourceAccessGateRefs.stream().sorted(Collections.reverseOrder()).map(ref -> new ResourceAccessGateHandler(ref, componentContext.locateService(resourceAccessGateReferenceName, ref))).collect(Collectors.toList()); } + /** * This method returns either an iterator delivering the matching handlers * or <code>null</code>. @@ -80,7 +74,7 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri private void peek() { this.next = null; - while (iter.hasNext() && next == null) { + while ( iter.hasNext() && next == null ) { final ResourceAccessGateHandler handler = iter.next(); if (handler.matches(path, operation)) { next = handler; @@ -95,7 +89,7 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri @Override public ResourceAccessGateHandler next() { - if (next == null) { + if ( next == null ) { throw new NoSuchElementException(); } final ResourceAccessGateHandler handler = this.next; @@ -117,26 +111,26 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri public Resource getReadableResource(final Resource resource) { Resource returnValue = null; - final Iterator<ResourceAccessGateHandler> accessGateHandlers = - getMatchingResourceAccessGateHandlerIterator(resource.getPath(), ResourceAccessGate.Operation.READ); + final Iterator<ResourceAccessGateHandler> accessGateHandlers = getMatchingResourceAccessGateHandlerIterator( + resource.getPath(), ResourceAccessGate.Operation.READ); GateResult finalGateResult = null; List<ResourceAccessGate> accessGatesForReadValues = null; boolean canReadAllValues = false; - if (accessGateHandlers != null) { - boolean noGateMatched = true; + if ( accessGateHandlers != null ) { - while (accessGateHandlers.hasNext()) { + boolean noGateMatched = true; + + while ( accessGateHandlers.hasNext() ) { noGateMatched = false; - final ResourceAccessGateHandler resourceAccessGateHandler = accessGateHandlers.next(); + final ResourceAccessGateHandler resourceAccessGateHandler = accessGateHandlers.next(); final GateResult gateResult = !resourceAccessGateHandler - .getResourceAccessGate() - .hasReadRestrictions(resource.getResourceResolver()) - ? GateResult.GRANTED - : resourceAccessGateHandler.getResourceAccessGate().canRead(resource); + .getResourceAccessGate().hasReadRestrictions(resource.getResourceResolver()) ? GateResult.GRANTED + : resourceAccessGateHandler.getResourceAccessGate() + .canRead(resource); if (!canReadAllValues && gateResult == GateResult.GRANTED) { if (resourceAccessGateHandler.getResourceAccessGate().canReadAllValues(resource)) { canReadAllValues = true; @@ -152,18 +146,19 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri finalGateResult = gateResult; } // stop checking if the operation is final and the result not GateResult.CANT_DECIDE - if (gateResult != GateResult.CANT_DECIDE - && resourceAccessGateHandler.isFinalOperation(ResourceAccessGate.Operation.READ)) { + if (gateResult != GateResult.CANT_DECIDE && resourceAccessGateHandler.isFinalOperation(ResourceAccessGate.Operation.READ)) { break; } } + // return null if access is denied or no ResourceAccessGate is present if (finalGateResult == GateResult.DENIED) { returnValue = null; - } else if (finalGateResult == GateResult.GRANTED) { + } else if (finalGateResult == GateResult.GRANTED ) { returnValue = resource; - } else if (noGateMatched && this.defaultAllowIfNoGateMatches) { + } else if (noGateMatched && this.defaultAllowIfNoGateMatches) + { returnValue = resource; } } @@ -172,93 +167,70 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri // wrap Resource if read access is not or partly (values) not granted if (returnValue != null) { - if (!canReadAllValues || !canUpdateResource) { - returnValue = new AccessGateResourceWrapper(returnValue, accessGatesForReadValues, canUpdateResource); + if( !canReadAllValues || !canUpdateResource ) { + returnValue = new AccessGateResourceWrapper(returnValue, + accessGatesForReadValues, + canUpdateResource); } } return returnValue; } - private boolean canDoOperation( - ResourceAccessGate.Operation operation, - String path, - Predicate<ResourceAccessGate> gatePredicate, - Function<ResourceAccessGate, GateResult> gateResultFilter) { - final Iterator<ResourceAccessGateHandler> handlers = - getMatchingResourceAccessGateHandlerIterator(path, operation); + private boolean canDoOperation(ResourceAccessGate.Operation operation, String path, Predicate<ResourceAccessGate> gatePredicate, Function<ResourceAccessGate, GateResult> gateResultFilter) { + final Iterator<ResourceAccessGateHandler> handlers = getMatchingResourceAccessGateHandlerIterator( + path, operation); boolean result = false; - if (handlers != null) { + if ( handlers != null ) { GateResult finalGateResult = null; boolean noGateMatched = true; - while (handlers.hasNext()) { + while ( handlers.hasNext() ) { noGateMatched = false; - final ResourceAccessGateHandler resourceAccessGateHandler = handlers.next(); + final ResourceAccessGateHandler resourceAccessGateHandler = handlers.next(); - final GateResult gateResult = !gatePredicate.test(resourceAccessGateHandler.getResourceAccessGate()) - ? GateResult.GRANTED + final GateResult gateResult = !gatePredicate.test(resourceAccessGateHandler.getResourceAccessGate()) ? GateResult.GRANTED : gateResultFilter.apply(resourceAccessGateHandler.getResourceAccessGate()); if (finalGateResult == null || finalGateResult == GateResult.DENIED) { finalGateResult = gateResult; } - if (finalGateResult == GateResult.GRANTED - || gateResult != GateResult.CANT_DECIDE - && resourceAccessGateHandler.isFinalOperation(operation)) { + if (finalGateResult == GateResult.GRANTED || gateResult != GateResult.CANT_DECIDE && + resourceAccessGateHandler.isFinalOperation(operation)) { break; } } - if (finalGateResult == GateResult.GRANTED || (noGateMatched && this.defaultAllowIfNoGateMatches)) { + if ( finalGateResult == GateResult.GRANTED || (noGateMatched && this.defaultAllowIfNoGateMatches)) { result = true; } } return result; } - + + @Override public boolean canOrderChildren(Resource resource) { - return canDoOperation( - ResourceAccessGate.Operation.ORDER_CHILDREN, - resource.getPath(), - gate -> gate.hasOrderChildrenRestrictions(resource.getResourceResolver()), - gate -> gate.canOrderChildren(resource)); + return canDoOperation(ResourceAccessGate.Operation.ORDER_CHILDREN, resource.getPath(), gate -> gate.hasOrderChildrenRestrictions(resource.getResourceResolver()), gate -> gate.canOrderChildren(resource)); } @Override public boolean canCreate(final String path, final ResourceResolver resolver) { - return canDoOperation( - ResourceAccessGate.Operation.CREATE, - path, - gate -> gate.hasCreateRestrictions(resolver), - gate -> gate.canCreate(path, resolver)); + return canDoOperation(ResourceAccessGate.Operation.CREATE, path, gate -> gate.hasCreateRestrictions(resolver), gate -> gate.canCreate(path, resolver)); } @Override public boolean canUpdate(final Resource resource) { - return canDoOperation( - ResourceAccessGate.Operation.UPDATE, - resource.getPath(), - gate -> gate.hasUpdateRestrictions(resource.getResourceResolver()), - gate -> gate.canUpdate(resource)); + return canDoOperation(ResourceAccessGate.Operation.UPDATE, resource.getPath(), gate -> gate.hasUpdateRestrictions(resource.getResourceResolver()), gate -> gate.canUpdate(resource)); } @Override public boolean canDelete(final Resource resource) { - return canDoOperation( - ResourceAccessGate.Operation.DELETE, - resource.getPath(), - gate -> gate.hasDeleteRestrictions(resource.getResourceResolver()), - gate -> gate.canDelete(resource)); + return canDoOperation(ResourceAccessGate.Operation.DELETE, resource.getPath(), gate -> gate.hasDeleteRestrictions(resource.getResourceResolver()), gate -> gate.canDelete(resource)); } @Override public boolean canExecute(final Resource resource) { - return canDoOperation( - ResourceAccessGate.Operation.EXECUTE, - resource.getPath(), - gate -> gate.hasExecuteRestrictions(resource.getResourceResolver()), - gate -> gate.canExecute(resource)); + return canDoOperation(ResourceAccessGate.Operation.EXECUTE, resource.getPath(), gate -> gate.hasExecuteRestrictions(resource.getResourceResolver()), gate -> gate.canExecute(resource)); } @Override @@ -280,15 +252,20 @@ public abstract class ResourceAccessSecurityImpl implements ResourceAccessSecuri } @Override - public String transformQuery(final String query, final String language, final ResourceResolver resourceResolver) - throws AccessSecurityException { + public String transformQuery(final String query, + final String language, + final ResourceResolver resourceResolver) + throws AccessSecurityException { String returnValue = query; for (ResourceAccessGateHandler handler : allHandlers) { - returnValue = handler.getResourceAccessGate().transformQuery(returnValue, language, resourceResolver); + returnValue = handler.getResourceAccessGate().transformQuery( + returnValue, language, resourceResolver); if (returnValue == null) { - throw new AccessSecurityException("Method transformQuery in ResourceAccessGate " - + handler.getResourceAccessGate().getClass().getName() + " returned null."); + throw new AccessSecurityException( + "Method transformQuery in ResourceAccessGate " + + handler.getResourceAccessGate().getClass() + .getName() + " returned null."); } } diff --git a/src/main/java/org/apache/sling/resourceaccesssecurity/package-info.java b/src/main/java/org/apache/sling/resourceaccesssecurity/package-info.java index be98b82..26ed69e 100644 --- a/src/main/java/org/apache/sling/resourceaccesssecurity/package-info.java +++ b/src/main/java/org/apache/sling/resourceaccesssecurity/package-info.java @@ -21,3 +21,4 @@ package org.apache.sling.resourceaccesssecurity; import org.osgi.annotation.versioning.Version; + diff --git a/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandlerTest.java b/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandlerTest.java index 149eee6..1f5e8e6 100644 --- a/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandlerTest.java +++ b/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessGateHandlerTest.java @@ -47,14 +47,12 @@ public class ResourceAccessGateHandlerTest { Assert.assertTrue(gateHandler.isFinalOperation(Operation.READ)); Assert.assertFalse(gateHandler.isFinalOperation(Operation.EXECUTE)); } - + @Test public void testMultiValueProperties() { Mockito.when(gateRef.getProperty(ResourceAccessGate.PATH)).thenReturn("/content"); - Mockito.when(gateRef.getProperty(ResourceAccessGate.OPERATIONS)) - .thenReturn(new String[] {"read", "update", "invalid"}); - Mockito.when(gateRef.getProperty(ResourceAccessGate.FINALOPERATIONS)) - .thenReturn(new String[] {"read", "update", "invalid"}); + Mockito.when(gateRef.getProperty(ResourceAccessGate.OPERATIONS)).thenReturn(new String[]{"read", "update", "invalid"}); + Mockito.when(gateRef.getProperty(ResourceAccessGate.FINALOPERATIONS)).thenReturn(new String[]{"read", "update", "invalid"}); ResourceAccessGateHandler gateHandler = new ResourceAccessGateHandler(gateRef, gate); Assert.assertTrue(gateHandler.matches("/content", Operation.READ)); Assert.assertFalse(gateHandler.matches("/othercontent", Operation.READ)); diff --git a/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImplTests.java b/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImplTests.java index 1e74ba8..8b3cab1 100644 --- a/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImplTests.java +++ b/src/test/java/org/apache/sling/resourceaccesssecurity/impl/ResourceAccessSecurityImplTests.java @@ -18,8 +18,17 @@ */ package org.apache.sling.resourceaccesssecurity.impl; -import java.util.Arrays; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.util.Collections; +import java.util.Arrays; import org.apache.sling.api.resource.ModifiableValueMap; import org.apache.sling.api.resource.Resource; @@ -30,15 +39,7 @@ import org.junit.Test; import org.mockito.Mockito; import org.osgi.framework.ServiceReference; import org.osgi.service.component.ComponentContext; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class ResourceAccessSecurityImplTests { @@ -46,6 +47,7 @@ public class ResourceAccessSecurityImplTests { ResourceAccessSecurity resourceAccessSecurity; ResourceAccessGate resourceAccessGate; + @Test public void testInitWithMultipleGates() { @@ -56,22 +58,21 @@ public class ResourceAccessSecurityImplTests { ResourceAccessGate resourceAccessGate2 = mock(ResourceAccessGate.class); ComponentContext context = mock(ComponentContext.class); - when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference))) - .thenReturn(resourceAccessGate); - when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference2))) - .thenReturn(resourceAccessGate2); + when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference))).thenReturn(resourceAccessGate); + when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference2))).thenReturn(resourceAccessGate2); try { - resourceAccessSecurity = - new ProviderResourceAccessSecurityImpl(Arrays.asList(serviceReference, serviceReference2), context); + resourceAccessSecurity = new ProviderResourceAccessSecurityImpl( + Arrays.asList(serviceReference, serviceReference2), + context); } catch (Exception e) { fail("Should not throw exception: " + e.getMessage()); } } @Test - public void testCanUpdate() { - initMocks("/content", new String[] {"update"}); + public void testCanUpdate(){ + initMocks("/content", new String[] { "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -80,8 +81,8 @@ public class ResourceAccessSecurityImplTests { } @Test - public void testCannotUpdate() { - initMocks("/content", new String[] {"update"}); + public void testCannotUpdate(){ + initMocks("/content", new String[] { "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -90,8 +91,8 @@ public class ResourceAccessSecurityImplTests { } @Test - public void testCannotUpdateWrongPath() { - initMocks("/content", new String[] {"update"}); + public void testCannotUpdateWrongPath(){ + initMocks("/content", new String[] { "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/wrongcontent"); @@ -100,10 +101,10 @@ public class ResourceAccessSecurityImplTests { } @Test - public void testCanUpdateUsingReadableResource() { + public void testCanUpdateUsingReadableResource(){ // one needs to have also read rights to obtain the resource - initMocks("/content", new String[] {"read", "update"}); + initMocks("/content", new String[] { "read", "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -117,14 +118,16 @@ public class ResourceAccessSecurityImplTests { ModifiableValueMap resultValueMap = readableResource.adaptTo(ModifiableValueMap.class); + resultValueMap.put("modified", "value"); verify(valueMap, times(1)).put("modified", "value"); } + @Test - public void testCannotUpdateUsingReadableResourceIfCannotRead() { - initMocks("/content", new String[] {"read", "update"}); + public void testCannotUpdateUsingReadableResourceIfCannotRead(){ + initMocks("/content", new String[] { "read", "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -136,12 +139,14 @@ public class ResourceAccessSecurityImplTests { when(resourceAccessGate.canUpdate(resource)).thenReturn(ResourceAccessGate.GateResult.GRANTED); Resource readableResource = resourceAccessSecurity.getReadableResource(resource); + assertNull(readableResource); } + @Test - public void testCannotUpdateUsingReadableResourceIfCannotUpdate() { - initMocks("/content", new String[] {"read", "update"}); + public void testCannotUpdateUsingReadableResourceIfCannotUpdate(){ + initMocks("/content", new String[] { "read", "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -158,9 +163,10 @@ public class ResourceAccessSecurityImplTests { assertNull(resultValueMap); } + @Test - public void testCannotUpdateAccidentallyUsingReadableResourceIfCannotUpdate() { - initMocks("/content", new String[] {"read", "update"}); + public void testCannotUpdateAccidentallyUsingReadableResourceIfCannotUpdate(){ + initMocks("/content", new String[] { "read", "update"} ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -181,7 +187,7 @@ public class ResourceAccessSecurityImplTests { @Test public void testCanOrderChildren() { - initMocks("/content", new String[] {"order-children"}); + initMocks("/content", new String[] { "order-children" } ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -191,7 +197,7 @@ public class ResourceAccessSecurityImplTests { @Test public void testCannotOrderChildren() { - initMocks("/content", new String[] {"order-children"}); + initMocks("/content", new String[] { "order-children" } ); Resource resource = mock(Resource.class); when(resource.getPath()).thenReturn("/content"); @@ -199,7 +205,7 @@ public class ResourceAccessSecurityImplTests { assertFalse(resourceAccessSecurity.canOrderChildren(resource)); } - private void initMocks(String path, String[] operations) { + private void initMocks(String path, String[] operations){ serviceReference = mock(ServiceReference.class); resourceAccessGate = mock(ResourceAccessGate.class); @@ -212,9 +218,8 @@ public class ResourceAccessSecurityImplTests { when(serviceReference.getProperty(ResourceAccessGate.OPERATIONS)).thenReturn(operations); ComponentContext context = mock(ComponentContext.class); - when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference))) - .thenReturn(resourceAccessGate); - resourceAccessSecurity = - new ProviderResourceAccessSecurityImpl(Collections.singletonList(serviceReference), context); + when(context.locateService(Mockito.anyString(), Mockito.eq(serviceReference))).thenReturn(resourceAccessGate); + resourceAccessSecurity = new ProviderResourceAccessSecurityImpl(Collections.singletonList(serviceReference), context); } + }
