Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
danielbobbert authored Feb 28, 2024
2 parents 36dba35 + 38bae4b commit 9b8b409
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 25 deletions.
3 changes: 3 additions & 0 deletions docs/src/main/asciidoc/config-reference.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,9 @@ Properties in the profile aware file have priority over profile aware properties

[WARNING]
====
Do not use profile aware files to set `quarkus.profile` or `quarkus.test.profile`. This will not work because the
profile is required in advance to load the profile aware files.
A profile aware file is only loaded if the unprofiled `application.properties` is also available in the same location
and the file extension matches between the files. This is required to keep a consistent loading order and pair all the
resources together.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,41 @@ quarkus.http.auth.permission.roles1.policy=role-policy1
----
<1> This permission references the default built-in `permit` policy to allow `GET` methods to `/public`.
In this case, the demonstrated setting would not affect this example because this request is allowed anyway.
<2> This permission references the built-in `deny` policy for `/forbidden`.
<2> This permission references the built-in `deny` policy for both `/forbidden` and `/forbidden/` paths.
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]
[IMPORTANT]
====
The exact path `/forbidden` in the example will not secure the `/forbidden/` path.
It is necessary to add a new exact path for the `/forbidden/` path to ensure proper security coverage.
The exact path pattern `/forbidden` in the example above also secures the `/forbidden/` path.
This way, the `forbidden` endpoint in the example below is secured by the `deny1` permission.
[source,java]
----
package org.acme.crud;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
@Path("/forbidden")
public class ForbiddenResource {
@GET
public String forbidden() { <1>
return "No!";
}
}
----
<1> Both `/forbidden` and `/forbidden/` paths need to be secured in order to secure the `forbidden` endpoint.
If you need to permit access to the `/forbidden/` path, please add new permission with more specific exact path like in the example below:
[source,properties]
----
quarkus.http.auth.permission.permit1.paths=/forbidden/ <1>
quarkus.http.auth.permission.permit1.policy=permit
----
<1> The `/forbidden/` path is not secured.
====

[[custom-http-security-policy]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class JakartaRestResourceHttpPermissionTest {
"quarkus.http.auth.permission.bar.policy=authenticated\n" +
"quarkus.http.auth.permission.baz-fum-pub.paths=/api/baz/fum\n" +
"quarkus.http.auth.permission.baz-fum-pub.policy=permit\n" +
"quarkus.http.auth.permission.baz-fum-deny.paths=/api/baz/fum/\n" +
"quarkus.http.auth.permission.baz-fum-deny.policy=authenticated\n" +
"quarkus.http.auth.permission.baz-fum.paths=/api/baz/fum*\n" +
"quarkus.http.auth.permission.baz-fum.policy=authenticated\n" +
"quarkus.http.auth.permission.root.paths=/\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class JakartaRestResourceHttpPermissionTest {
"quarkus.http.auth.permission.bar.policy=authenticated\n" +
"quarkus.http.auth.permission.baz-fum-pub.paths=/api/baz/fum\n" +
"quarkus.http.auth.permission.baz-fum-pub.policy=permit\n" +
"quarkus.http.auth.permission.baz-fum-deny.paths=/api/baz/fum/\n" +
"quarkus.http.auth.permission.baz-fum-deny.policy=authenticated\n" +
"quarkus.http.auth.permission.baz-fum.paths=/api/baz/fum*\n" +
"quarkus.http.auth.permission.baz-fum.policy=authenticated\n" +
"quarkus.http.auth.permission.root.paths=/\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ public void testHealthCheckPaths(String path) {
public void testMiscellaneousPaths() {
// /api/baz with segment indicating version shouldn't match /api/baz path policy
assurePath("/api/baz;v=1.1", 200);
// /api/baz/ is different resource than secured /api/baz, therefore request should succeed
assurePath("/api/baz/", 200);
// /api/baz/ is different resource than secured /api/baz, but we secure both when there is not more specific exact path pattern
assurePath("/api/baz/", 401);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ public static class ImmutablePathMatcherBuilder<T> {

private static final String STRING_PATH_SEPARATOR = "/";
private final Map<String, T> exactPathMatches = new HashMap<>();
/**
* Exact paths we proactively secure when more specify permissions are not specified.
* For example path for exact path '/api/hello' we add extra pattern for the '/api/hello/'.
* This helps to secure Jakarta REST endpoints by default as both paths may point to the same endpoint there.
* However, we only do that when user didn't declare any permission for the '/api/hello/'.
* This way, user can still forbid access to the `/api/hello' path and permit access to the '/api/hello/' path.
*/
private final Map<String, T> additionalExactPathMatches = new HashMap<>();
private final Map<String, Path<T>> pathsWithWildcard = new HashMap<>();
private BiConsumer<T, T> handlerAccumulator;

Expand Down Expand Up @@ -188,6 +196,9 @@ public void accept(SubstringMatch<T> match1, SubstringMatch<T> match2) {

paths.put(p.path, handler, subPathMatcher);
}
for (var e : additionalExactPathMatches.entrySet()) {
exactPathMatches.putIfAbsent(e.getKey(), e.getValue());
}
int[] lengths = buildLengths(paths.keys());
return new ImmutablePathMatcher<>(defaultHandler, paths.asImmutableMap(), exactPathMatches, lengths,
hasPathWithInnerWildcard);
Expand Down Expand Up @@ -289,6 +300,20 @@ private void addExactPath(final String path, final T handler) {
} else {
exactPathMatches.put(path, handler);
}
// when 'path.equals("/api/hello")' then the other path is '/api/hello/'
final String otherPath;
if (path.endsWith(STRING_PATH_SEPARATOR)) {
if (path.length() == 1) {
// path '/' is only valid option, '' is not allowed
return;
}
// drop path separator
otherPath = path.substring(0, path.length() - 1);
} else {
otherPath = path + STRING_PATH_SEPARATOR;
}
// if key is already present, then we have the right handler into which new ones have already been merged
additionalExactPathMatches.putIfAbsent(otherPath, handler);
}

private static int[] buildLengths(Iterable<String> keys) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ public void testPrefixPathWithEndingWildcard() {
final Object prefixPathMatcher2 = new Object();
matcher = ImmutablePathMatcher.builder().addPath("/one/two/*", prefixPathMatcher1)
.addPath("/one/two/three", exactPathMatcher1).addPath("/one/two", exactPathMatcher2)
.addPath("/one/two/three*", prefixPathMatcher2).addPath("/one/two/three/four", exactPathMatcher3).build();
.addPath("/one/two/", prefixPathMatcher1).addPath("/one/two/three/", prefixPathMatcher2)
.addPath("/one/two/three*", prefixPathMatcher2).addPath("/one/two/three/four/", prefixPathMatcher2)
.addPath("/one/two/three/four", exactPathMatcher3).build();
assertMatched(matcher, "/one/two/three", exactPathMatcher1);
assertMatched(matcher, "/one/two", exactPathMatcher2);
assertMatched(matcher, "/one/two/three/four", exactPathMatcher3);
Expand Down Expand Up @@ -72,8 +74,10 @@ public void testPrefixPathDefaultHandler() {
final Object prefixPathMatcher1 = new Object();
final Object prefixPathMatcher2 = new Object();
matcher = ImmutablePathMatcher.builder().addPath("/one/two/*", prefixPathMatcher1).addPath("/*", defaultHandler)
.addPath("/one/two/three", exactPathMatcher1).addPath("/one/two", exactPathMatcher2)
.addPath("/one/two/three*", prefixPathMatcher2).addPath("/one/two/three/four", exactPathMatcher3).build();
.addPath("/one/two/three", exactPathMatcher1).addPath("/one/two/three/", prefixPathMatcher2)
.addPath("/one/two", exactPathMatcher2).addPath("/one/two/", prefixPathMatcher1)
.addPath("/one/two/three*", prefixPathMatcher2).addPath("/one/two/three/four", exactPathMatcher3)
.addPath("/one/two/three/four/", prefixPathMatcher2).build();
assertMatched(matcher, "/one/two/three", exactPathMatcher1);
assertMatched(matcher, "/one/two", exactPathMatcher2);
assertMatched(matcher, "/one/two/three/four", exactPathMatcher3);
Expand Down Expand Up @@ -120,7 +124,7 @@ public void testSpecialChars() {
ImmutablePathMatcher<Object> matcher = ImmutablePathMatcher.builder().addPath("/one/two#three", handler2)
.addPath("/one/two?three=four", handler1).addPath("/one/*/three?one\\\\\\=two", handler3)
.addPath("/one/two#three*", handler4).addPath("/*/two#three*", handler5).addPath("/*", HANDLER)
.build();
.addPath("/one/two#three/", handler4).build();
assertMatched(matcher, "/one/two#three", handler2);
assertMatched(matcher, "/one/two?three=four", handler1);
assertMatched(matcher, "/one/any-value/three?one\\\\\\=two", handler3);
Expand All @@ -142,7 +146,7 @@ public void testSpecialChars() {
assertMatched(matcher, "/one1/two#three/christmas!", handler5);
assertMatched(matcher, "/one1/two#thre");
// no default handler
matcher = ImmutablePathMatcher.builder().addPath("/one/two#three", handler2)
matcher = ImmutablePathMatcher.builder().addPath("/one/two#three", handler2).addPath("/one/two#three/", handler4)
.addPath("/one/two?three=four", handler1).addPath("/one/*/three?one\\\\\\=two", handler3)
.addPath("/one/two#three*", handler4).addPath("/*/two#three*", handler5).build();
assertMatched(matcher, "/one/two#three", handler2);
Expand Down Expand Up @@ -181,7 +185,7 @@ public void testInnerWildcardsWithExactMatches() {
.addPath("/one/two/three", handler2).addPath("/one/two/three/four", handler3)
.addPath("/", handler4).addPath("/*", HANDLER).addPath("/one/two/*/four", handler5)
.addPath("/one/*/three/four", handler6).addPath("/*/two/three/four", handler7)
.addPath("/*/two", handler8).build();
.addPath("/*/two", handler8).addPath("/*/two/three/four/", HANDLER).build();
assertMatched(matcher, "/one/two", handler1);
assertMatched(matcher, "/one/two/three", handler2);
assertMatched(matcher, "/one/two/three/four", handler3);
Expand Down Expand Up @@ -216,7 +220,7 @@ public void testInnerWildcardsOnly() {
assertMatched(matcher, "/one/two/three/4/five", handler5);
assertMatched(matcher, "/one/two/three/sergey/five", handler5);
assertMatched(matcher, "/one/two/three/sergey/five-ish");
assertMatched(matcher, "/one/two/three/sergey/five/");
assertMatched(matcher, "/one/two/three/sergey/five/", handler5);
assertMatched(matcher, "/one/two/three/four", handler4);
assertMatched(matcher, "/one/two/3/four", handler4);
assertMatched(matcher, "/one/two/three", handler3);
Expand All @@ -228,11 +232,11 @@ public void testInnerWildcardsOnly() {
assertMatched(matcher, "/ho-hey/two", handler2);
assertMatched(matcher, "/ho-hey/two2");
assertMatched(matcher, "/ho-hey/two2/");
assertMatched(matcher, "/ho-hey/two/");
assertMatched(matcher, "/ho-hey/two/", handler2);
assertMatched(matcher, "/ho-hey/hey-ho/three", handler1);
assertMatched(matcher, "/1/2/three", handler1);
assertMatched(matcher, "/1/two/three", handler1);
assertMatched(matcher, "/1/two/three/");
assertMatched(matcher, "/1/two/three/", handler1);
assertMatched(matcher, "/1/two/three/f");
// no default path handler
matcher = ImmutablePathMatcher.builder().addPath("/*/two", handler2)
Expand All @@ -242,8 +246,8 @@ public void testInnerWildcardsOnly() {
assertMatched(matcher, "/one/two/three/four/five", handler5);
assertMatched(matcher, "/one/two/three/4/five", handler5);
assertMatched(matcher, "/one/two/three/sergey/five", handler5);
assertMatched(matcher, "/one/two/three/sergey/five/", handler5);
assertNotMatched(matcher, "/one/two/three/sergey/five-ish");
assertNotMatched(matcher, "/one/two/three/sergey/five/");
assertMatched(matcher, "/one/two/three/four", handler4);
assertMatched(matcher, "/one/two/3/four", handler4);
assertMatched(matcher, "/one/two/three", handler3);
Expand All @@ -253,13 +257,13 @@ public void testInnerWildcardsOnly() {
assertMatched(matcher, "/two/two", handler2);
assertMatched(matcher, "/2/two", handler2);
assertMatched(matcher, "/ho-hey/two", handler2);
assertMatched(matcher, "/ho-hey/two/", handler2);
assertNotMatched(matcher, "/ho-hey/two2");
assertNotMatched(matcher, "/ho-hey/two2/");
assertNotMatched(matcher, "/ho-hey/two/");
assertMatched(matcher, "/ho-hey/hey-ho/three", handler1);
assertMatched(matcher, "/1/2/three", handler1);
assertMatched(matcher, "/1/two/three", handler1);
assertNotMatched(matcher, "/1/two/three/");
assertMatched(matcher, "/1/two/three/", handler1);
assertNotMatched(matcher, "/1/two/three/f");
}

Expand Down Expand Up @@ -377,7 +381,7 @@ public void testPrefixPathHandlerMerging() {
handler6.add("AgentBrown");
var matcher = ImmutablePathMatcher.<List<String>> builder().handlerAccumulator(List::addAll).addPath("/path*", handler1)
.addPath("/path*", handler2).addPath("/path/*", handler3).addPath("/path/", handler4)
.addPath("/path/*/", handler5).addPath("/*", handler6).build();
.addPath("/path/*/", handler5).addPath("/*", handler6).addPath("/path", handler1).build();
var handler = matcher.match("/path").getValue();
assertNotNull(handler);
assertTrue(handler.contains("Neo"));
Expand Down Expand Up @@ -505,11 +509,11 @@ public void testDefaultHandlerOneInnerWildcard() {
assertMatched(matcher, "/3/one");
assertMatched(matcher, "/4/one");
assertMatched(matcher, "/4/one");
assertMatched(matcher, "/1/one/");
assertNotMatched(matcher, "/");
assertNotMatched(matcher, "/1");
assertNotMatched(matcher, "/1/");
assertNotMatched(matcher, "/1/two");
assertNotMatched(matcher, "/1/one/");
assertNotMatched(matcher, "/1/one1");
assertNotMatched(matcher, "/1/on");
assertNotMatched(matcher, "/1/one/two");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public Response.ResponseBuilder location(URI location) {
try {
String host = req.getRequestHost();
int port = -1;
int index = host.indexOf(":");
int index = host.lastIndexOf(":");
if (index > -1) {
port = Integer.parseInt(host.substring(index + 1));
host = host.substring(0, index);
Expand Down Expand Up @@ -69,7 +69,7 @@ public Response.ResponseBuilder contentLocation(URI location) {
try {
String host = req.getRequestHost();
int port = -1;
int index = host.indexOf(":");
int index = host.lastIndexOf(":");
if (index > -1) {
port = Integer.parseInt(host.substring(index + 1));
host = host.substring(0, index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public RestResponse.ResponseBuilder<T> location(URI location) {
try {
String host = req.getRequestHost();
int port = -1;
int index = host.indexOf(":");
int index = host.lastIndexOf(":");
if (index > -1) {
port = Integer.parseInt(host.substring(index + 1));
host = host.substring(0, index);
Expand Down Expand Up @@ -69,7 +69,7 @@ public RestResponse.ResponseBuilder<T> contentLocation(URI location) {
try {
String host = req.getRequestHost();
int port = -1;
int index = host.indexOf(":");
int index = host.lastIndexOf(":");
if (index > -1) {
port = Integer.parseInt(host.substring(index + 1));
host = host.substring(0, index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ analysis_results.types.reflection=6522
analysis_results.types.reflection.tolerance=3
analysis_results.methods.reflection=4893
analysis_results.methods.reflection.tolerance=3
analysis_results.fields.reflection=163
analysis_results.fields.reflection=168
analysis_results.fields.reflection.tolerance=3
analysis_results.types.jni=61
analysis_results.types.jni.tolerance=1
Expand Down

0 comments on commit 9b8b409

Please sign in to comment.