Skip to content
Permalink
Browse files
Fixing a permission checking bug.
  • Loading branch information
mifosio-04-04-2018 committed Apr 28, 2017
1 parent 844021e commit 57dca2e8f1ebf66f9854cc93b7eba7f52f41f1c4
Showing 3 changed files with 33 additions and 14 deletions.
@@ -25,8 +25,10 @@
import javax.servlet.http.HttpServletRequest;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.BiPredicate;
import java.util.stream.IntStream;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* @author Myrle Krantz
@@ -71,21 +73,24 @@ private boolean matchesHelper(final String servletPath, final String method,
final boolean opMatches = allowedOperation.containsHttpMethod(method);
final String[] requestPathSegments = servletPath.split("/");

if (servletPathSegmentMatchers.size() == requestPathSegments.length + 1)
return lastSegmentIsStarSegment(servletPathSegmentMatchers);

if (servletPathSegmentMatchers.size() > requestPathSegments.length)
if (servletPathSegmentMatchers.size() > requestPathSegments.length + 1)
return false;

if (servletPathSegmentMatchers.size() == requestPathSegments.length + 1)
if (!lastSegmentIsStarSegment(servletPathSegmentMatchers))
return false;

if (servletPathSegmentMatchers.size() < requestPathSegments.length)
return lastSegmentIsStarSegment(servletPathSegmentMatchers);
if (!lastSegmentIsStarSegment(servletPathSegmentMatchers))
return false;

final boolean aNonMappableSegmentExistsInServletPath =
IntStream.range(0, servletPathSegmentMatchers.size())
.filter(i -> !segmentMatcher.test(servletPathSegmentMatchers.get(i), requestPathSegments[i]))
.findFirst().isPresent();
final Optional<Integer> indexOfFirstNonMappableSegment =
Stream.iterate(0, n -> n + 1)
.limit(Math.min(servletPathSegmentMatchers.size(), requestPathSegments.length))
.filter(i -> !segmentMatcher.test(servletPathSegmentMatchers.get(i), requestPathSegments[i]))
.findFirst();

return opMatches && !aNonMappableSegmentExistsInServletPath;
return opMatches && !indexOfFirstNonMappableSegment.isPresent();
}

private static boolean lastSegmentIsStarSegment(
@@ -106,4 +111,12 @@ private static boolean lastSegmentIsStarSegment(
@Override public int hashCode() {
return Objects.hash(servletPathSegmentMatchers, allowedOperation);
}

@Override
public String toString() {
return "ApplicationPermission{" +
"servletPathSegmentMatchers='" + servletPathSegmentMatchers.stream().map(PermissionSegmentMatcher::getPermissionSegment).collect(Collectors.joining("/")) +
"', allowedOperation=" + allowedOperation +
'}';
}
}
@@ -42,7 +42,7 @@ boolean isParameterSegment() {
return permissionSegment.startsWith("{") && permissionSegment.endsWith("}");
}

String getPermissionSegment() { return permissionSegment; }
public String getPermissionSegment() { return permissionSegment; }

public boolean matches(final String requestSegment, final String principal, boolean isSu) {
if (isStarSegment())
@@ -149,8 +149,14 @@ public static Collection testCases() {
.permittedPath("/{parameter}/").requestedPath("/value")
.expectedResult(true));
ret.add(new TestCase("{parameter} without su")
.permittedPath("/{parameter}/").requestedPath("/value")
.expectedResult(false));
.permittedPath("/{parameter}/").requestedPath("/value")
.expectedResult(false));
ret.add(new TestCase("* at end with request containing more segments")
.permittedPath("/roles/*").requestedPath("/users/antony/password")
.expectedResult(false));
ret.add(new TestCase("* at end with request containing same # segments")
.permittedPath("/x/y/z/*").requestedPath("/m/n/o/")
.expectedResult(false));

return ret;
}

0 comments on commit 57dca2e

Please sign in to comment.