From 09c105fe3730f1fffaa91e329c57a032154943d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Tue, 7 Nov 2023 15:28:41 +0100 Subject: [PATCH] Fix OIDC permission mapping NPE when scope is empty --- .../io/quarkus/oidc/runtime/OidcUtils.java | 7 ++++++ .../io/quarkus/it/keycloak/OidcResource.java | 18 +++++++++++++++ .../BearerTokenAuthorizationTest.java | 23 ++++++++++++++++--- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java index 7b3d41938b0b5..8fcc88f5c15ce 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java @@ -208,6 +208,9 @@ private static List findClaimWithRoles(OidcTenantConfig.Roles rolesConfi return convertJsonArrayToList((JsonArray) claimValue); } else if (claimValue != null) { String sep = rolesConfig.getRoleClaimSeparator().isPresent() ? rolesConfig.getRoleClaimSeparator().get() : " "; + if (claimValue.toString().isBlank()) { + return Collections.emptyList(); + } return Arrays.asList(claimValue.toString().split(sep)); } else { return Collections.emptyList(); @@ -237,6 +240,10 @@ private static Object findClaimValue(String claimPath, JsonObject json, String[] private static List convertJsonArrayToList(JsonArray claimValue) { List list = new ArrayList<>(claimValue.size()); for (int i = 0; i < claimValue.size(); i++) { + String claimValueStr = claimValue.getString(i); + if (claimValueStr == null || claimValueStr.isBlank()) { + continue; + } list.add(claimValue.getString(i)); } return list; diff --git a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java index 7082b5ca3c6b0..76f67a46ae814 100644 --- a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java +++ b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java @@ -237,6 +237,16 @@ public String testAccessToken(@QueryParam("kid") String kid, @QueryParam("sub") " \"expires_in\": 300 }"; } + @POST + @Path("accesstoken-empty-scope") + @Produces("application/json") + public String testAccessTokenWithEmptyScope(@QueryParam("kid") String kid, @QueryParam("sub") String subject) { + return "{\"access_token\": \"" + jwt(null, subject, kid, true) + "\"," + + " \"token_type\": \"Bearer\"," + + " \"refresh_token\": \"123456789\"," + + " \"expires_in\": 300 }"; + } + @POST @Path("opaque-token") @Produces("application/json") @@ -290,6 +300,10 @@ public boolean disableRotate() { } private String jwt(String audience, String subject, String kid) { + return jwt(audience, subject, kid, false); + } + + private String jwt(String audience, String subject, String kid, boolean withEmptyScope) { JwtClaimsBuilder builder = Jwt.claim("typ", "Bearer") .upn("alice") .preferredUserName("alice") @@ -302,6 +316,10 @@ private String jwt(String audience, String subject, String kid) { builder.subject(subject); } + if (withEmptyScope) { + builder.claim("scope", ""); + } + return builder.jws().keyId(kid) .sign(key.getPrivateKey()); } diff --git a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java index 73d7f7b646591..12fe6f454dd4e 100644 --- a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java +++ b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java @@ -517,15 +517,24 @@ public void testJwtTokenIntrospectionOnlyAndUserInfo() { + "introspection_client_id:none,introspection_client_secret:none,active:true,userinfo:alice,cache-size:0")); } + // verifies empty scope claim makes no difference (e.g. doesn't cause NPE) + RestAssured.given().auth().oauth2(getAccessTokenWithEmptyScopeFromSimpleOidc("2")) + .when().get("/tenant/tenant-oidc-introspection-only/api/user") + .then() + .statusCode(200) + .body(equalTo( + "tenant-oidc-introspection-only:alice,client_id:client-introspection-only," + + "introspection_client_id:none,introspection_client_secret:none,active:true,userinfo:alice,cache-size:0")); + RestAssured.given().auth().oauth2(getAccessTokenFromSimpleOidc("987654321", "2")) .when().get("/tenant/tenant-oidc-introspection-only/api/user") .then() .statusCode(401); RestAssured.when().get("/oidc/jwk-endpoint-call-count").then().body(equalTo("0")); - RestAssured.when().get("/oidc/introspection-endpoint-call-count").then().body(equalTo("4")); + RestAssured.when().get("/oidc/introspection-endpoint-call-count").then().body(equalTo("5")); RestAssured.when().post("/oidc/disable-introspection").then().body(equalTo("false")); - RestAssured.when().get("/oidc/userinfo-endpoint-call-count").then().body(equalTo("4")); + RestAssured.when().get("/oidc/userinfo-endpoint-call-count").then().body(equalTo("5")); RestAssured.when().get("/cache/size").then().body(equalTo("0")); } @@ -694,13 +703,21 @@ private String getAccessTokenFromSimpleOidc(String kid) { } private String getAccessTokenFromSimpleOidc(String subject, String kid) { + return getAccessTokenFromSimpleOidc(subject, kid, "/oidc/accesstoken"); + } + + private String getAccessTokenWithEmptyScopeFromSimpleOidc(String kid) { + return getAccessTokenFromSimpleOidc("123456789", kid, "/oidc/accesstoken-empty-scope"); + } + + private static String getAccessTokenFromSimpleOidc(String subject, String kid, String tokenEndpoint) { String json = RestAssured .given() .queryParam("sub", subject) .queryParam("kid", kid) .formParam("grant_type", "authorization_code") .when() - .post("/oidc/accesstoken") + .post(tokenEndpoint) .body().asString(); JsonObject object = new JsonObject(json); return object.getString("access_token");