Skip to content

Commit 237f7be

Browse files
gsmetgsmet
authored
Merge pull request from GHSA-pv6m-j4fg-qx7h
Apply HTTP Security Policy on normalized path (main)
2 parents c2e05c0 + aff0cf9 commit 237f7be

File tree

16 files changed

+993
-219
lines changed

16 files changed

+993
-219
lines changed

‎extensions/csrf-reactive/runtime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ private String getCookieToken(RoutingContext routing, CsrfReactiveConfig config)
243243
}
244244

245245
private boolean isCsrfTokenRequired(RoutingContext routing, CsrfReactiveConfig config) {
246-
return config.createTokenPath.isPresent() ? config.createTokenPath.get().contains(routing.request().path()) : true;
246+
return config.createTokenPath
247+
.map(value -> value.contains(routing.normalizedPath())).orElse(true);
247248
}
248249

249250
private void createCookie(String csrfToken, RoutingContext routing, CsrfReactiveConfig config) {

‎extensions/keycloak-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/KeycloakPolicyEnforcerAuthorizer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public Uni<CheckResult> checkPermission(RoutingContext request, Uni<SecurityIden
4444
public CheckResult apply(RoutingContext routingContext, SecurityIdentity identity) {
4545

4646
if (identity.isAnonymous()) {
47-
PathConfig pathConfig = resolver.getPolicyEnforcer(null).getPathMatcher().matches(routingContext.request().path());
47+
PathConfig pathConfig = resolver.getPolicyEnforcer(null).getPathMatcher().matches(
48+
routingContext.normalizedPath());
4849
if (pathConfig != null && pathConfig.getEnforcementMode() == EnforcementMode.ENFORCING) {
4950
return CheckResult.DENY;
5051
}

‎extensions/keycloak-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/VertxHttpFacade.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.io.ByteArrayInputStream;
44
import java.io.InputStream;
5-
import java.net.URI;
65
import java.util.List;
76

87
import org.keycloak.adapters.authorization.TokenPrincipal;
@@ -122,7 +121,7 @@ public String getURI() {
122121

123122
@Override
124123
public String getRelativePath() {
125-
return URI.create(request.uri()).getPath();
124+
return routingContext.normalizedPath();
126125
}
127126

128127
@Override

‎extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestResourceHttpPermissionTest.java‎

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
package io.quarkus.resteasy.test.security;
2+
3+
import static org.apache.commons.codec.binary.Base64.encodeBase64URLSafeString;
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
6+
import java.net.URL;
7+
import java.time.Duration;
8+
9+
import jakarta.inject.Inject;
10+
import jakarta.ws.rs.GET;
11+
import jakarta.ws.rs.Path;
12+
13+
import org.jboss.shrinkwrap.api.asset.StringAsset;
14+
import org.junit.jupiter.api.BeforeAll;
15+
import org.junit.jupiter.api.extension.RegisterExtension;
16+
import org.junit.jupiter.params.ParameterizedTest;
17+
import org.junit.jupiter.params.provider.ValueSource;
18+
19+
import io.quarkus.security.test.utils.TestIdentityController;
20+
import io.quarkus.security.test.utils.TestIdentityProvider;
21+
import io.quarkus.test.QuarkusUnitTest;
22+
import io.quarkus.test.common.http.TestHTTPResource;
23+
import io.smallrye.mutiny.Uni;
24+
import io.vertx.core.http.HttpMethod;
25+
import io.vertx.mutiny.core.Vertx;
26+
import io.vertx.mutiny.core.http.HttpClientRequest;
27+
28+
public class JakartaRestResourceHttpPermissionTest {
29+
private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(20);
30+
private static final String APP_PROPS = "quarkus.http.auth.permission.foo.paths=/api/foo,/api/foo/\n" +
31+
"quarkus.http.auth.permission.foo.policy=authenticated\n" +
32+
"quarkus.http.auth.permission.bar.paths=api/bar*\n" +
33+
"quarkus.http.auth.permission.bar.policy=authenticated\n" +
34+
"quarkus.http.auth.permission.baz-fum-pub.paths=/api/baz/fum\n" +
35+
"quarkus.http.auth.permission.baz-fum-pub.policy=permit\n" +
36+
"quarkus.http.auth.permission.baz-fum.paths=/api/baz/fum*\n" +
37+
"quarkus.http.auth.permission.baz-fum.policy=authenticated\n" +
38+
"quarkus.http.auth.permission.root.paths=/\n" +
39+
"quarkus.http.auth.permission.root.policy=authenticated\n" +
40+
"quarkus.http.auth.permission.dot.paths=dot,dot/\n" +
41+
"quarkus.http.auth.permission.dot.policy=authenticated\n";
42+
43+
@RegisterExtension
44+
static QuarkusUnitTest runner = new QuarkusUnitTest()
45+
.withApplicationRoot((jar) -> jar
46+
.addClasses(TestIdentityProvider.class, TestIdentityController.class, ApiResource.class,
47+
RootResource.class, PublicResource.class)
48+
.addAsResource(new StringAsset(APP_PROPS), "application.properties"));
49+
50+
@BeforeAll
51+
public static void setup() {
52+
TestIdentityController.resetRoles().add("test", "test", "test");
53+
}
54+
55+
@TestHTTPResource
56+
URL url;
57+
58+
@Inject
59+
Vertx vertx;
60+
61+
@ParameterizedTest
62+
@ValueSource(strings = {
63+
// path without wildcard, with leading slashes in both policy and @Path
64+
"/api/foo/", "/api/foo",
65+
// path with wildcard, without leading slashes in both policy and @Path
66+
"/api/bar", "/api/bar/", "/api/bar/irish",
67+
// combination of permit and authenticated policies, paths are resolved to /api/baz/fum/ and auth required
68+
"/api/baz/fum/"
69+
})
70+
public void testEmptyPathSegments(String path) {
71+
assurePath(path, 401);
72+
assurePathAuthenticated(path, getLastNonEmptySegmentContent(path));
73+
}
74+
75+
@ParameterizedTest
76+
@ValueSource(strings = { "/", "///", "/?stuff", "" })
77+
public void testRootPath(String path) {
78+
assurePath(path, 401);
79+
assurePathAuthenticated(path);
80+
}
81+
82+
@ParameterizedTest
83+
@ValueSource(strings = { "/one/", "/three?stuff" })
84+
public void testNotSecuredPaths(String path) {
85+
// negative testing - all paths are public unless auth policy is applied
86+
assurePath(path, 200);
87+
}
88+
89+
@ParameterizedTest
90+
@ValueSource(strings = { "/api/foo///", "////api/foo", "/api//foo", "/api/bar///irish", "/api/bar///irish/",
91+
"/api//baz/fum//",
92+
"/api///foo", "////api/bar", "/api///bar", "/api//bar" })
93+
public void testSecuredNotFound(String path) {
94+
assurePath(path, 401);
95+
assurePathAuthenticated(path, 404);
96+
}
97+
98+
private static String getLastNonEmptySegmentContent(String path) {
99+
while (path.endsWith("/") || path.endsWith(".")) {
100+
path = path.substring(0, path.length() - 1);
101+
}
102+
return path.substring(path.lastIndexOf('/') + 1);
103+
}
104+
105+
@Path("/api")
106+
public static class ApiResource {
107+
108+
@GET
109+
@Path("/foo")
110+
public String foo() {
111+
return "foo";
112+
}
113+
114+
@GET
115+
@Path("/bar")
116+
public String bar() {
117+
return "bar";
118+
}
119+
120+
@GET
121+
@Path("/bar/irish")
122+
public String irishBar() {
123+
return "irish";
124+
}
125+
126+
@GET
127+
@Path("/baz/fum")
128+
public String bazFum() {
129+
return "fum";
130+
}
131+
132+
}
133+
134+
@Path("/")
135+
public static class RootResource {
136+
137+
@GET
138+
public String get() {
139+
return "root";
140+
}
141+
142+
}
143+
144+
@Path("/")
145+
public static class PublicResource {
146+
147+
@Path("one")
148+
@GET
149+
public String one() {
150+
return "one";
151+
}
152+
153+
@Path("/two")
154+
@GET
155+
public String two() {
156+
return "two";
157+
}
158+
159+
@Path("/three")
160+
@GET
161+
public String three() {
162+
return "three";
163+
}
164+
165+
@Path("four")
166+
@GET
167+
public String four() {
168+
return "four";
169+
}
170+
171+
@Path("/four#stuff")
172+
@GET
173+
public String fourWitFragment() {
174+
return "four#stuff";
175+
}
176+
177+
@Path("five")
178+
@GET
179+
public String five() {
180+
return "five";
181+
}
182+
183+
}
184+
185+
private void assurePath(String path, int expectedStatusCode) {
186+
assurePath(path, expectedStatusCode, null, false);
187+
}
188+
189+
private void assurePathAuthenticated(String path) {
190+
assurePath(path, 200, null, true);
191+
}
192+
193+
private void assurePathAuthenticated(String path, int statusCode) {
194+
assurePath(path, statusCode, null, true);
195+
}
196+
197+
private void assurePathAuthenticated(String path, String body) {
198+
assurePath(path, 200, body, true);
199+
}
200+
201+
private void assurePath(String path, int expectedStatusCode, String body, boolean auth) {
202+
var httpClient = vertx.createHttpClient();
203+
try {
204+
httpClient
205+
.request(HttpMethod.GET, url.getPort(), url.getHost(), path)
206+
.map(r -> {
207+
if (auth) {
208+
r.putHeader("Authorization", "Basic " + encodeBase64URLSafeString("test:test".getBytes()));
209+
}
210+
return r;
211+
})
212+
.flatMap(HttpClientRequest::send)
213+
.invoke(r -> assertEquals(expectedStatusCode, r.statusCode(), path))
214+
.flatMap(r -> {
215+
if (body != null) {
216+
return r.body().invoke(b -> assertEquals(b.toString(), body, path));
217+
} else {
218+
return Uni.createFrom().nullItem();
219+
}
220+
})
221+
.await()
222+
.atMost(REQUEST_TIMEOUT);
223+
} finally {
224+
httpClient
225+
.close()
226+
.await()
227+
.atMost(REQUEST_TIMEOUT);
228+
}
229+
}
230+
}

0 commit comments

Comments
 (0)