Skip to content

Commit 3a9fa21

Browse files
committed
[fixes #9057] - Scope permission checks may result in a false positive
1 parent 9e2a05e commit 3a9fa21

File tree

5 files changed

+65
-2
lines changed

5 files changed

+65
-2
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Uni<Boolean> apply(Permission permission) {
7171
if (context != null) {
7272
String scopes = permission.getActions();
7373

74-
if (scopes == null) {
74+
if (scopes == null || "".equals(scopes)) {
7575
return Uni.createFrom().item(context.hasResourcePermission(permission.getName()));
7676
}
7777

integration-tests/keycloak-authorization/src/main/java/io/quarkus/it/keycloak/ProtectedResource.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.quarkus.it.keycloak;
22

3+
import java.security.BasicPermission;
34
import java.util.Collections;
45
import java.util.List;
56
import java.util.Map;
@@ -13,6 +14,7 @@
1314
import javax.ws.rs.POST;
1415
import javax.ws.rs.Path;
1516
import javax.ws.rs.Produces;
17+
import javax.ws.rs.QueryParam;
1618
import javax.ws.rs.core.Context;
1719
import javax.ws.rs.core.MediaType;
1820

@@ -43,6 +45,27 @@ public List<Permission> apply(Boolean granted) {
4345
});
4446
}
4547

48+
@GET
49+
@Path("/scope")
50+
@Produces(MediaType.APPLICATION_JSON)
51+
public Uni<List<Permission>> hasScopePermission(@QueryParam("scope") String scope) {
52+
return identity.checkPermission(new BasicPermission("Scope Permission Resource") {
53+
@Override
54+
public String getActions() {
55+
return scope;
56+
}
57+
}).onItem()
58+
.apply(new Function<Boolean, List<Permission>>() {
59+
@Override
60+
public List<Permission> apply(Boolean granted) {
61+
if (granted) {
62+
return identity.getAttribute("permissions");
63+
}
64+
throw new ForbiddenException();
65+
}
66+
});
67+
}
68+
4669
@Path("/claim-protected")
4770
@GET
4871
@Produces(MediaType.APPLICATION_JSON)

integration-tests/keycloak-authorization/src/main/resources/application.properties

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,7 @@ quarkus.keycloak.policy-enforcer.paths.8.name=Public
4949
quarkus.keycloak.policy-enforcer.paths.8.path=/hello
5050
quarkus.keycloak.policy-enforcer.paths.8.enforcement-mode=DISABLED
5151

52+
quarkus.keycloak.policy-enforcer.paths.9.name=Scope Permission Resource
53+
quarkus.keycloak.policy-enforcer.paths.9.path=/api/permission/scope
54+
5255
admin-url=${keycloak.url}

integration-tests/keycloak-authorization/src/test/java/io/quarkus/it/keycloak/KeycloakTestResource.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ private static ClientRepresentation createClient(String clientId) {
9393
configureHttpResponseClaimBasedPermission(authorizationSettings);
9494
configureBodyClaimBasedPermission(authorizationSettings);
9595
configurePaths(authorizationSettings);
96+
configureScopePermission(authorizationSettings);
9697

9798
client.setAuthorizationSettings(authorizationSettings);
9899

@@ -108,6 +109,13 @@ private static void configurePermissionResourcePermission(ResourceServerRepresen
108109
createPermission(settings, createResource(settings, "Permission Resource", "/api/permission"), policy);
109110
}
110111

112+
private static void configureScopePermission(ResourceServerRepresentation settings) {
113+
PolicyRepresentation policy = createJSPolicy("Grant Policy", "$evaluation.grant();", settings);
114+
createScopePermission(settings,
115+
createResource(settings, "Scope Permission Resource", "/api/permission/scope", "read", "write"), policy,
116+
"read");
117+
}
118+
111119
private static void configureClaimBasedPermission(ResourceServerRepresentation settings) {
112120
PolicyRepresentation policy = createJSPolicy("Claim-Based Policy", "var context = $evaluation.getContext();\n"
113121
+ "var attributes = context.getAttributes();\n"
@@ -166,10 +174,30 @@ private static void createPermission(ResourceServerRepresentation settings, Reso
166174
settings.getPolicies().add(permission);
167175
}
168176

177+
private static void createScopePermission(ResourceServerRepresentation settings, ResourceRepresentation resource,
178+
PolicyRepresentation policy, String scope) {
179+
PolicyRepresentation permission = new PolicyRepresentation();
180+
181+
permission.setName(resource.getName() + " Permission");
182+
permission.setType("scope");
183+
permission.setResources(new HashSet<>());
184+
permission.getResources().add(resource.getName());
185+
permission.setScopes(new HashSet<>());
186+
permission.getScopes().add(scope);
187+
permission.setPolicies(new HashSet<>());
188+
permission.getPolicies().add(policy.getName());
189+
190+
settings.getPolicies().add(permission);
191+
}
192+
169193
private static ResourceRepresentation createResource(ResourceServerRepresentation authorizationSettings, String name,
170-
String uri) {
194+
String uri, String... scopes) {
171195
ResourceRepresentation resource = new ResourceRepresentation(name);
172196

197+
for (String scope : scopes) {
198+
resource.addScope(scope);
199+
}
200+
173201
if (uri != null) {
174202
resource.setUris(Collections.singleton(uri));
175203
}

integration-tests/keycloak-authorization/src/test/java/io/quarkus/it/keycloak/PolicyEnforcerTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ public void testUserHasRoleConfidential() {
4040
.then()
4141
.statusCode(200)
4242
.and().body(Matchers.containsString("Permission Resource"));
43+
RestAssured.given().auth().oauth2(getAccessToken("jdoe"))
44+
.when().get("/api/permission/scope?scope=write")
45+
.then()
46+
.statusCode(403);
47+
RestAssured.given().auth().oauth2(getAccessToken("jdoe"))
48+
.when().get("/api/permission/scope?scope=read")
49+
.then()
50+
.statusCode(200)
51+
.and().body(Matchers.containsString("read"));
4352
;
4453
RestAssured.given().auth().oauth2(getAccessToken("admin"))
4554
.when().get("/api/permission")

0 commit comments

Comments
 (0)