Skip to content

Commit

Permalink
Merge pull request #37192 from michalvavrik/feature/path-matching-wil…
Browse files Browse the repository at this point in the history
…dcard-latest

Support repeated wildcards in the HTTP permissions path matching patterns
  • Loading branch information
sberyozkin authored Nov 22, 2023
2 parents 0bc38e2 + bae0f03 commit 1323552
Show file tree
Hide file tree
Showing 8 changed files with 1,142 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ It is an exact path match because it does not end with `*`.
<3> This permission set references the previously defined policy.
`roles1` is an example name; you can call the permission sets whatever you want.

WARNING: The `/forbidden` exact path in the example above will not secure the `/forbidden/` path. Don't forget to add new exact path for the `/forbidden/` path.

=== Custom HttpSecurityPolicy

Sometimes it might be useful to register your own named policy. You can get it done by creating application scoped CDI
Expand Down Expand Up @@ -123,10 +125,12 @@ Otherwise, it queries for an exact match and only matches that specific path:

[source,properties]
----
quarkus.http.auth.permission.permit1.paths=/public/*,/css/*,/js/*,/robots.txt
quarkus.http.auth.permission.permit1.paths=/public*,/css/*,/js/*,/robots.txt <1>
quarkus.http.auth.permission.permit1.policy=permit
quarkus.http.auth.permission.permit1.methods=GET,HEAD
----
<1> The `$$*$$` wildcard at the end of the path matches zero or more path segments, but never any word starting from the `/public` path.
For that reason, a path like `/public-info` is not matched by this pattern.

=== Matching a path but not a method

Expand Down Expand Up @@ -170,6 +174,59 @@ quarkus.http.auth.permission.public.policy=permit
----
====

=== Matching multiple sub-paths: longest path to the `*` wildcard wins

Previous examples shown how you can match all sub-paths when a path ends with the `$$*$$` wildcard.
The `$$*$$` wildcard can also be used in the middle of the path, in which case it represents exactly one path segment.
You can't combine this wildcard with any other path segment character, therefore the `$$*$$` wildcard will always be
enclosed with path separators as in the `/public/$$*$$/about-us` path.

What happens if multiple path patterns matches same request path?
Matching is always done on the "longest sub-path to the `$$*$$` wildcard wins" basis.
Every path segment character is considered more specific than the `$$*$$` wildcard.

Here is a simple example:

[source,properties]
----
quarkus.http.auth.permission.secured.paths=/api/*/detail <1>
quarkus.http.auth.permission.secured.policy=authenticated
quarkus.http.auth.permission.public.paths=/api/public-product/detail <2>
quarkus.http.auth.permission.public.policy=permit
----
<1> Request paths like `/api/product/detail` can only be accessed by authenticated users.
<2> The path `/api/public-product/detail` is more specific, therefore accessible by anyone.

[IMPORTANT]
====
All paths secured with the authorization using configuration should be tested.
Writing path patterns with multiple wildcards can be cumbersome.
Please make sure paths are authorized as you intended.
====

In the following example, paths are ordered from the most specific to the least specific one:

.Request path `/one/two/three/four/five` matches ordered from the most specific to the least specific path

[source, text]
----
/one/two/three/four/five
/one/two/three/four/*
/one/two/three/*/five
/one/two/three/*/*
/one/two/*/four/five
/one/*/three/four/five
/*/two/three/four/five
/*/two/three/*/five
/*
----

[IMPORTANT]
====
The `$$*$$` wildcard at the end of the path matches zero or more path segments.
The `$$*$$` wildcard placed anywhere else matches exactly one path segment.
====

=== Matching multiple paths: most specific method wins

When a path is registered with multiple permission sets, the permission sets explicitly specifying an HTTP method that matches the request take precedence.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public class PathMatchingHttpSecurityPolicyTest {
"quarkus.http.auth.permission.public.policy=permit\n" +
"quarkus.http.auth.permission.foo.paths=/api/foo/bar\n" +
"quarkus.http.auth.permission.foo.policy=authenticated\n" +
"quarkus.http.auth.permission.inner-wildcard.paths=/api/*/bar\n" +
"quarkus.http.auth.permission.inner-wildcard.policy=authenticated\n" +
"quarkus.http.auth.permission.inner-wildcard2.paths=/api/next/*/prev\n" +
"quarkus.http.auth.permission.inner-wildcard2.policy=authenticated\n" +
"quarkus.http.auth.permission.inner-wildcard3.paths=/api/one/*/three/*\n" +
"quarkus.http.auth.permission.inner-wildcard3.policy=authenticated\n" +
"quarkus.http.auth.permission.inner-wildcard4.paths=/api/one/*/*/five\n" +
"quarkus.http.auth.permission.inner-wildcard4.policy=authenticated\n" +
"quarkus.http.auth.permission.inner-wildcard5.paths=/api/one/*/jamaica/*\n" +
"quarkus.http.auth.permission.inner-wildcard5.policy=permit\n" +
"quarkus.http.auth.permission.inner-wildcard6.paths=/api/*/sadly/*/dont-know\n" +
"quarkus.http.auth.permission.inner-wildcard6.policy=deny\n" +
"quarkus.http.auth.permission.baz.paths=/api/baz\n" +
"quarkus.http.auth.permission.baz.policy=authenticated\n" +
"quarkus.http.auth.permission.static-resource.paths=/static-file.html\n" +
Expand Down Expand Up @@ -85,6 +97,25 @@ private WebClient getClient() {
return client;
}

@Test
public void testInnerWildcardPath() {
assurePath("/api/any-value/bar", 401);
assurePath("/api/any-value/bar", 401);
assurePath("/api/next/any-value/prev", 401);
assurePath("/api/one/two/three/four", 401);
assurePath("/api////any-value//////bar", 401);
assurePath("/api/next///////any-value////prev", 401);
assurePath("////api//one/two//three////four?door=wood", 401);
assurePath("/api/one/three/four/five", 401);
assurePath("/api/one/3/4/five", 401);
assurePath("////api/one///3/4/five", 401);
assurePath("/api/now/sadly/i/dont-know", 401);
assurePath("/api/now/sadly///i/dont-know", 401);
assurePath("/api/one/three/jamaica/five", 200);
assurePath("/api/one/three/jamaica/football", 200);
assurePath("/api/now/sally/i/dont-know", 200);
}

@ParameterizedTest
@ValueSource(strings = {
// path policy without wildcard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.quarkus.vertx.http.runtime.PolicyMappingConfig;
import io.quarkus.vertx.http.runtime.security.HttpSecurityPolicy.AuthorizationRequestContext;
import io.quarkus.vertx.http.runtime.security.HttpSecurityPolicy.CheckResult;
import io.quarkus.vertx.http.runtime.security.ImmutablePathMatcher.PathMatch;
import io.smallrye.mutiny.Uni;
import io.vertx.ext.web.RoutingContext;

Expand All @@ -33,15 +34,15 @@
*/
public class AbstractPathMatchingHttpSecurityPolicy {

private final PathMatcher<List<HttpMatcher>> pathMatcher = new PathMatcher<>();
private final ImmutablePathMatcher<List<HttpMatcher>> pathMatcher;

AbstractPathMatchingHttpSecurityPolicy(Map<String, PolicyMappingConfig> permissions,
Map<String, PolicyConfig> rolePolicy, String rootPath, Instance<HttpSecurityPolicy> installedPolicies) {
init(permissions, toNamedHttpSecPolicies(rolePolicy, installedPolicies), rootPath);
pathMatcher = init(permissions, toNamedHttpSecPolicies(rolePolicy, installedPolicies), rootPath);
}

public String getAuthMechanismName(RoutingContext routingContext) {
PathMatcher.PathMatch<List<HttpMatcher>> toCheck = pathMatcher.match(routingContext.normalizedPath());
PathMatch<List<HttpMatcher>> toCheck = pathMatcher.match(routingContext.normalizedPath());
if (toCheck.getValue() == null || toCheck.getValue().isEmpty()) {
return null;
}
Expand Down Expand Up @@ -93,9 +94,9 @@ public Uni<? extends CheckResult> apply(CheckResult checkResult) {
});
}

private void init(Map<String, PolicyMappingConfig> permissions,
private static ImmutablePathMatcher<List<HttpMatcher>> init(Map<String, PolicyMappingConfig> permissions,
Map<String, HttpSecurityPolicy> permissionCheckers, String rootPath) {
Map<String, List<HttpMatcher>> tempMap = new HashMap<>();
final var builder = ImmutablePathMatcher.<List<HttpMatcher>> builder().handlerAccumulator(List::addAll);
for (Map.Entry<String, PolicyMappingConfig> entry : permissions.entrySet()) {
HttpSecurityPolicy checker = permissionCheckers.get(entry.getValue().policy);
if (checker == null) {
Expand All @@ -108,34 +109,19 @@ private void init(Map<String, PolicyMappingConfig> permissions,
if (!path.startsWith("/")) {
path = rootPath + path;
}
if (tempMap.containsKey(path)) {
HttpMatcher m = new HttpMatcher(entry.getValue().authMechanism.orElse(null),
new HashSet<>(entry.getValue().methods.orElse(Collections.emptyList())),
checker);
tempMap.get(path).add(m);
} else {
HttpMatcher m = new HttpMatcher(entry.getValue().authMechanism.orElse(null),
new HashSet<>(entry.getValue().methods.orElse(Collections.emptyList())),
checker);
List<HttpMatcher> perms = new ArrayList<>();
tempMap.put(path, perms);
perms.add(m);
if (path.endsWith("/*")) {
String stripped = path.substring(0, path.length() - 2);
pathMatcher.addPrefixPath(stripped.isEmpty() ? "/" : stripped, perms);
} else if (path.endsWith("*")) {
pathMatcher.addPrefixPath(path.substring(0, path.length() - 1), perms);
} else {
pathMatcher.addExactPath(path, perms);
}
}
HttpMatcher m = new HttpMatcher(entry.getValue().authMechanism.orElse(null),
new HashSet<>(entry.getValue().methods.orElse(Collections.emptyList())), checker);
List<HttpMatcher> perms = new ArrayList<>();
perms.add(m);
builder.addPath(path, perms);
}
}
}
return builder.build();
}

public List<HttpSecurityPolicy> findPermissionCheckers(RoutingContext context) {
PathMatcher.PathMatch<List<HttpMatcher>> toCheck = pathMatcher.match(context.normalizedPath());
PathMatch<List<HttpMatcher>> toCheck = pathMatcher.match(context.normalizedPath());
if (toCheck.getValue() == null || toCheck.getValue().isEmpty()) {
return Collections.emptyList();
}
Expand Down
Loading

0 comments on commit 1323552

Please sign in to comment.