Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Principal extraction from nested username claim was broken #194

Merged
merged 6 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .travis/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ elif [[ "$arch" != 'ppc64le' ]]; then
EXIT=$?
exitIfError

clearDockerEnv
mvn -e -V -B clean install -f testsuite -Pkafka-3_3_2
EXIT=$?
exitIfError

# Excluded by default to not exceed Travis job timeout
if [ "$SKIP_DISABLED" == "false" ]; then

clearDockerEnv
mvn -e -V -B clean install -f testsuite -Pkafka-3_3_2
EXIT=$?
exitIfError

clearDockerEnv
mvn -e -V -B clean install -f testsuite -Pkafka-3_2_3 -DfailIfNoTests=false -Dtest=\!KeycloakKRaftAuthorizationTests
EXIT=$?
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,12 +316,12 @@ If the configured `oauth.client.id` is `kafka`, the following are valid examples

JWT tokens contain unique user identification in `sub` claim. However, this is often a long number or a UUID, but we usually prefer to use human-readable usernames, which may also be present in JWT token.
Use `oauth.username.claim` to map the claim (attribute) where the value you want to use as user id is stored:
- `oauth.username.claim` (e.g.: "preferred_username")
- `oauth.username.claim` (e.g.: "preferred_username", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)

If `oauth.username.claim` is specified the value of that claim is used instead, but if not set, the automatic fallback claim is the `sub` claim.

You can specify the secondary claim to fall back to, which allows you to map multiple account types into the same principal namespace:
- `oauth.fallback.username.claim` (e.g.: "client_id")
- `oauth.fallback.username.claim` (e.g.: "client_id", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)
- `oauth.fallback.username.prefix` (e.g.: "client-account-")

If `oauth.username.claim` is specified but value does not exist in the token, then `oauth.fallback.username.claim` is used. If value for that doesn't exist either, the exception is thrown.
Expand Down Expand Up @@ -400,10 +400,10 @@ Introspection Endpoint may or may not return identifying information which we co
If the information is available we attempt to extract the user id from Introspection Endpoint response.

Use `oauth.username.claim` to map the attribute where the user id is stored:
- `oauth.username.claim` (e.g.: "preferred_username")
- `oauth.username.claim` (e.g.: "preferred_username", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)

You can fall back to a secondary attribute, which allows you to map multiple account types into the same user id namespace:
- `oauth.fallback.username.claim` (e.g.: "client_id")
- `oauth.fallback.username.claim` (e.g.: "client_id", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)
- `oauth.fallback.username.prefix` (e.g.: "client-account-")

If `oauth.username.claim` is specified but value does not exist in the Introspection Endpoint response, then `oauth.fallback.username.claim` is used. If value for that doesn't exist either, the exception is thrown.
Expand Down Expand Up @@ -985,7 +985,7 @@ Audience is sent to the Token Endpoint when obtaining the access token.

For debug purposes you may want to properly configure which JWT token attribute contains the user id of the account used to obtain the access token:

- `oauth.username.claim` (e.g.: "preferred_username")
- `oauth.username.claim` (e.g.: "preferred_username", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)

This does not affect how Kafka client is presented to the Kafka Broker.
The broker performs user id extraction from the token once again or it uses the Introspection Endpoint or the User Info Endpoint to get the user id.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ public static String getClaimFromJWT(String claim, Object token) {
* @return Value of the specific claim as String or null if claim not present
*/
public static String getClaimFromJWT(JsonNode node, String... path) {
if (path.length == 0) {
return null;
}
for (String p: path) {
node = node.get(p);
if (node == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,67 @@
package io.strimzi.kafka.oauth.common;

import com.fasterxml.jackson.databind.JsonNode;
import io.strimzi.kafka.oauth.jsonpath.JsonPathQuery;

import static io.strimzi.kafka.oauth.common.JSONUtil.getClaimFromJWT;

/**
* An object with logic for extracting a principal name (i.e. a user id) from a JWT token.
*
* <p>
* First a claim configured as <code>usernameClaim</code> is looked up.
* If not found the claim configured as <code>fallbackUsernameClaim</code> is looked up. If that one is found and if
* the <code>fallbackUsernamePrefix</code> is configured prefix the found value with the prefix, otherwise not.
* <p>
* The claim specification uses the following rules:
* <ul>
* <li>If the claim specification starts with an opening square bracket '[', it is interpreted as a JsonPath query, and allows
* targeting a nested attribute. </li>
* <li>Otherwise, it is interpreted as a top level attribute name.</li>
* </ul>
* <p>
* A JsonPath query is resolved relative to JSON object containing info to identify user
* (a JWT payload, a response from Introspection Endpoint or a response from User Info Endpoint).
* <p>
* For more on JsonPath syntax see https://github.com/json-path/JsonPath.
* <p>
* Examples of claim specification:
* <pre>
* userId ... use top level attribute named 'userId'
* user.id ... use top level attribute named 'user.id'
* $userid ... use top level attribute named '$userid'
* ['userInfo']['id'] ... use nested attribute 'id' under 'userInfo' top level attribute
* ['userInfo'].id ... use nested attribute 'id' under 'userInfo' top level attribute (second segment not using brackets)
* ['user.info']['user.id'] ... use nested attribute 'user.id' under 'user.info' top level attribute
* ['user.info'].['user.id'] ... use nested attribute 'user.id' under 'user.info' top level attribute (optional dot)
* </pre>
*
* See PrincipalExtractorTest.java for more working and non-working examples of claim specification.
*/
public class PrincipalExtractor {

private String usernameClaim;
private String fallbackUsernameClaim;
private String fallbackUsernamePrefix;
private final Extractor usernameExtractor;
private final Extractor fallbackUsernameExtractor;
private final String fallbackUsernamePrefix;

/**
* Create a new instance
*/
public PrincipalExtractor() {}
public PrincipalExtractor() {
usernameExtractor = null;
fallbackUsernameExtractor = null;
fallbackUsernamePrefix = null;
}

/**
* Create a new instance
*
* @param usernameClaim Attribute name for an attribute containing the user id to lookup first
* @param usernameClaim Attribute name for an attribute containing the user id to lookup first.
* @param fallbackUsernameClaim Attribute name for an attribute containg the user id to lookup as a fallback
* @param fallbackUsernamePrefix A prefix to prepend to the value of the fallback attribute value if set
*/
public PrincipalExtractor(String usernameClaim, String fallbackUsernameClaim, String fallbackUsernamePrefix) {
this.usernameClaim = usernameClaim;
this.fallbackUsernameClaim = fallbackUsernameClaim;
this.usernameExtractor = parseClaimSpec(usernameClaim);
this.fallbackUsernameExtractor = parseClaimSpec(fallbackUsernameClaim);
this.fallbackUsernamePrefix = fallbackUsernamePrefix;
}

Expand All @@ -48,23 +78,38 @@ public PrincipalExtractor(String usernameClaim, String fallbackUsernameClaim, St
public String getPrincipal(JsonNode json) {
String result;

if (usernameClaim != null) {
result = getClaimFromJWT(json, usernameClaim);
if (usernameExtractor != null) {
result = extractUsername(usernameExtractor, json);
if (result != null) {
return result;
}

if (fallbackUsernameClaim != null) {
result = getClaimFromJWT(json, fallbackUsernameClaim);
if (fallbackUsernameExtractor != null) {
result = extractUsername(fallbackUsernameExtractor, json);
if (result != null) {
return fallbackUsernamePrefix == null ? result : fallbackUsernamePrefix + result;
return result;
}
}
}

return null;
}

private String extractUsername(Extractor extractor, JsonNode json) {
if (extractor.getAttributeName() != null) {
String result = getClaimFromJWT(json, extractor.getAttributeName());
if (result != null && !result.isEmpty()) {
return result;
}
} else {
JsonNode queryResult = extractor.getJSONPathQuery().apply(json);
String result = queryResult == null ? null : queryResult.asText().trim();
if (result != null && !result.isEmpty()) {
return result;
}
}
return null;
}

/**
* Get the value of <code>sub</code> claim
*
Expand All @@ -77,7 +122,7 @@ public String getSub(JsonNode json) {

@Override
public String toString() {
return "PrincipalExtractor {usernameClaim: " + usernameClaim + ", fallbackUsernameClaim: " + fallbackUsernameClaim + ", fallbackUsernamePrefix: " + fallbackUsernamePrefix + "}";
return "PrincipalExtractor {usernameClaim: " + usernameExtractor + ", fallbackUsernameClaim: " + fallbackUsernameExtractor + ", fallbackUsernamePrefix: " + fallbackUsernamePrefix + "}";
}

/**
Expand All @@ -86,6 +131,66 @@ public String toString() {
* @return True if any of the constructor parameters is set
*/
public boolean isConfigured() {
return usernameClaim != null || fallbackUsernameClaim != null || fallbackUsernamePrefix != null;
return usernameExtractor != null || fallbackUsernameExtractor != null || fallbackUsernamePrefix != null;
}

/**
* The claim specification uses the following rules:
* <ul>
* <li>If the claim specification starts with an opening square bracket '[', it is interpreted as a JsonPath query, and allows
* targeting a nested attribute. </li>
* <li>Otherwise, it is interpreted as a top level attribute name.</li>
* </ul>
* For more on JsonPath syntax see https://github.com/json-path/JsonPath.
* <p>
* Examples of claim specification:
* <pre>
* userId ... use top level attribute named 'userId'
* user.id ... use top level attribute named 'user.id'
* $userid ... use top level attribute named '$userid'
* ['userInfo']['id'] ... use nested attribute 'id' under 'userInfo' top level attribute
* ['userInfo'].id ... use nested attribute 'id' under 'userInfo' top level attribute (second segment not using brackets)
* ['user.info']['user.id'] ... use nested attribute 'user.id' under 'user.info' top level attribute
* ['user.info'].['user.id'] ... use nested attribute 'user.id' under 'user.info' top level attribute (optional dot)
* </pre>
*
* @param spec Claim specification
* @return Result containing either a claim with top level attribute name or a JsonPathQuery object
*/
private static Extractor parseClaimSpec(String spec) {
spec = spec == null ? null : spec.trim();
if (spec == null || spec.isEmpty()) {
return null;
}

if (!spec.startsWith("[")) {
return new Extractor(spec);
}

return new Extractor(JsonPathQuery.parse(spec));
}

static class Extractor {

private final String attributeName;
private final JsonPathQuery query;

private Extractor(JsonPathQuery query) {
this.query = query;
this.attributeName = null;
}

private Extractor(String attributeName) {
this.attributeName = attributeName;
this.query = null;
}

String getAttributeName() {
return attributeName;
}

JsonPathQuery getJSONPathQuery() {
return query;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private JsonPathQuery(String query) {
try {
this.matcher = new Matcher(ctx, query, false);
} catch (JsonPathException e) {
throw new JsonPathQueryException("Failed to parse filter query: \"" + query + "\"", e);
throw new JsonPathQueryException("Failed to parse JsonPath query: \"" + query + "\"", e);
}
}

Expand Down
Loading