diff --git a/.travis/build.sh b/.travis/build.sh index 0dd801df..cf6297d7 100755 --- a/.travis/build.sh +++ b/.travis/build.sh @@ -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=$? diff --git a/README.md b/README.md index fcb4e93b..fe6fb435 100644 --- a/README.md +++ b/README.md @@ -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. @@ -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. @@ -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. diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/JSONUtil.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/JSONUtil.java index a60803fd..9a7ad575 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/JSONUtil.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/JSONUtil.java @@ -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) { diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java index 6b3cecca..ad180630 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java @@ -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. - * + *
* First a claim configured as usernameClaim
is looked up.
* If not found the claim configured as fallbackUsernameClaim
is looked up. If that one is found and if
* the fallbackUsernamePrefix
is configured prefix the found value with the prefix, otherwise not.
+ *
+ * The claim specification uses the following rules: + *
+ * 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). + *
+ * For more on JsonPath syntax see https://github.com/json-path/JsonPath. + *
+ * Examples of claim specification: + *
+ * 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) + *+ * + * 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; } @@ -48,16 +78,15 @@ 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; } } } @@ -65,6 +94,22 @@ public String getPrincipal(JsonNode json) { 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
sub
claim
*
@@ -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 + "}";
}
/**
@@ -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:
+ * + * Examples of claim specification: + *
+ * 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) + *+ * + * @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; + } } } diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/jsonpath/JsonPathQuery.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/jsonpath/JsonPathQuery.java index 6d82d186..24066c8a 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/jsonpath/JsonPathQuery.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/jsonpath/JsonPathQuery.java @@ -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); } } diff --git a/oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java b/oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java new file mode 100644 index 00000000..1cf5032b --- /dev/null +++ b/oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java @@ -0,0 +1,145 @@ +/* + * Copyright 2017-2023, Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +package io.strimzi.kafka.oauth.validator; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.jayway.jsonpath.InvalidPathException; +import io.strimzi.kafka.oauth.common.PrincipalExtractor; +import io.strimzi.kafka.oauth.jsonpath.JsonPathQueryException; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +import static io.strimzi.kafka.oauth.common.JSONUtil.readJSON; + +public class PrincipalExtractorTest { + + @Test + public void testUsernameClaim() throws IOException { + + ObjectNode json = readJSON("{ " + + "\"sub\": \"u123456\", " + + "\"userId\": \"alice\", " + + "\"user.id\": \"alice-123456\", " + + "\"'user.id'\": \"quoted-alice-123456\", " + + "\"$user.id\": \"$alice\", " + + "\"$$user.id\": \"$$alice\", " + + "\"$.user.id\": \"$.alice\", " + + "\"userInfo\": {\"id\": \"alice@example.com\" }, " + + "\"user.info\": {\"sub.id\": \"alice-123456@example.com\"}, " + + "\"foo'bar\": \"alice-in-wonderland\", " + + "\"uname[1\": \"white-rabbit\" " + + "}", ObjectNode.class); + + String[] claimSpec = { + // top-level userId attribute + "userId", "alice", + // top-level userInfo attribute (a misconfiguration as you hit the nested object in test json) + // Should we log a warning and display a matched object rather than just return null (the result is failed authentication anyway)? + "userInfo", null, + // top-level userInfo.id attribute (no match) + "userInfo.id", null, + // nested id attribute under userInfo top-level attribute + "['userInfo']['id']", "alice@example.com", + // nested id attribute under userInfo top-level attribute + "['userInfo']id", "alice@example.com", + // nested id attribute under userInfo top-level attribute + "['userInfo'].id", "alice@example.com", + // nested id attribute under userInfo top-level attribute + "[ 'userInfo' ][ 'id' ]", "alice@example.com", + // nested id attribute under userInfo top-level attribute + "[ 'userInfo' ].[ 'id' ]", "alice@example.com", + // nested sub.id attribute under user.info top-level attribute + "['user.info']['sub.id']", "alice-123456@example.com", + // user.id top-level attribute + "user.id", "alice-123456", + // user.id top level attribute + "['user.id']", "alice-123456", + // $user.id top-level attribute + "$user.id", "$alice", + // $user.id top-level attribute + "['$user.id']", "$alice", + // $$user.id top-level attribute + "$$user.id", "$$alice", + // 'user.id' top-level attribute (quotes are part of the attribute name) + "'user.id'", "quoted-alice-123456", + // $['user.id'] top-level attribute (dollar, square brackets and quotes are part of the attribute name, no match) + "$['user.id']", null, + // uname[1 top-level attribute + "['uname[1']", "white-rabbit", + // uname]1 top-level attribute (no match) + "['uname]1']", null, + // user.id] top-level attribute (closing square bracket is part of the attribute name, no match) + "user.id]", null, + // foo'bar top-level attribute + "foo'bar", "alice-in-wonderland", + // foo'bar top-level attribute + "['foo\\'bar']", "alice-in-wonderland", + // baz attribute nested under bar, nested under top-level attribute foo (no match) + "['foo']bar['baz']", null, + // nested id attribute under user.info top level attribute (no match) + "['user.info'].id", null, + // nested sub.id attribute under user.info top-level attribute + "['user.info'].['sub.id']", "alice-123456@example.com", + // 'user.id' top-level attribute (quotes are part of the attribute name) + "['\\'user.id\\'']", "quoted-alice-123456", + // top-level $.user.id attribute + "$.user.id", "$.alice", + // top-level $.user.id attribute + "['$.user.id']", "$.alice", + }; + + String[] claimSpecError = { + // square brackets but no single quotes + "[user.id]", + // square brackets but no single quotes + "[user.info][sub.id]", + // opening square bracket but no single quote + "[user.id", + // opening square bracket but no single quote + "[user.info].id", + // opening square bracket but no single quote + "[foo].bar[baz]", + // third square bracket segment has no single quotes + "['foo']bar[baz]", + // ending single quote can only be followed by a closing square bracket + "['foo'bar]", + // ending single quote can only be followed by a closing square bracket (inner quote not escaped) + "['foo'bar']", + // ending single quote can only be followed by a closing square bracket (inner quotes not escaped) + "[''user.id'']", + // there should be no space between ] and [, only optional dot (.) + "[ 'userInfo' ] [ 'id' ]" + }; + + for (int i = 0; i < claimSpec.length; i++) { + String query = claimSpec[i]; + String expected = claimSpec[++i]; + + try { + PrincipalExtractor extractor = new PrincipalExtractor(query, null, null); + Assert.assertEquals(query + " top level works as primary", expected, extractor.getPrincipal(json)); + + extractor = new PrincipalExtractor("nonexisting", query, null); + Assert.assertEquals(query + " top level works as fallback", expected, extractor.getPrincipal(json)); + } catch (Exception e) { + throw new RuntimeException("Unexpected error while testing: " + query + " expecting it to return: " + expected, e); + } + } + + for (int i = 0; i < claimSpecError.length; i++) { + String query = claimSpecError[i]; + try { + PrincipalExtractor extractor = new PrincipalExtractor(query, "nonexisting", null); + extractor.getPrincipal(json); + Assert.fail("Should have failed"); + } catch (JsonPathQueryException e) { + Assert.assertTrue("Cause instanceof InvalidPathException", e.getCause() instanceof InvalidPathException); + // ignored + } + } + } +} diff --git a/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/JaasServerOauthValidatorCallbackHandler.java b/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/JaasServerOauthValidatorCallbackHandler.java index 978caba9..fa5e9a68 100644 --- a/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/JaasServerOauthValidatorCallbackHandler.java +++ b/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/JaasServerOauthValidatorCallbackHandler.java @@ -137,9 +137,10 @@ * Common optional sasl.jaas.config configuration: *