Merge pull request #36920 from michalvavrik/feature/fix-oidc-perm-npe

Fix OIDC scope to permission mapping when scope is empty
quarkusio · Nov 7, 2023 · 878bca8 · 878bca8
2 parents ff281c8 + 09c105f
commit 878bca8
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 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
@@ -208,6 +208,9 @@ private static List<String> 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<String> convertJsonArrayToList(JsonArray claimValue) {
         List<String> 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
@@ -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/...tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java b/...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");