Skip to content

Commit

Permalink
Fix OIDC permission mapping NPE when scope is empty
Browse files Browse the repository at this point in the history
(cherry picked from commit 09c105f)
  • Loading branch information
michalvavrik authored and gsmet committed Nov 7, 2023
1 parent c5b1867 commit 5305d5b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,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();
Expand Down Expand Up @@ -234,6 +237,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 5305d5b

Please sign in to comment.