From d8ba636293b4ddfa49a85422f192421affef4d34 Mon Sep 17 00:00:00 2001 From: Marko Strukelj Date: Mon, 19 Jun 2023 15:18:26 +0200 Subject: [PATCH 1/6] Principal extraction from nested username claim was broken Signed-off-by: Marko Strukelj --- README.md | 10 +++--- .../oauth/common/PrincipalExtractor.java | 8 ++--- .../validator/PrincipalExtractorTest.java | 34 +++++++++++++++++++ ...asServerOauthValidatorCallbackHandler.java | 5 +-- 4 files changed, 46 insertions(+), 11 deletions(-) create mode 100644 oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java diff --git a/README.md b/README.md index fcb4e93b..99b6faff 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 '.') 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 '.') - `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 '.') 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 '.') - `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 '.') 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/PrincipalExtractor.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java index 6b3cecca..42bc9736 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 @@ -10,7 +10,7 @@ /** * 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. @@ -29,7 +29,7 @@ public PrincipalExtractor() {} /** * 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. Use '.' to refer to nested child objects. * @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 */ @@ -49,13 +49,13 @@ public String getPrincipal(JsonNode json) { String result; if (usernameClaim != null) { - result = getClaimFromJWT(json, usernameClaim); + result = getClaimFromJWT(usernameClaim, json); if (result != null) { return result; } if (fallbackUsernameClaim != null) { - result = getClaimFromJWT(json, fallbackUsernameClaim); + result = getClaimFromJWT(fallbackUsernameClaim, json); if (result != null) { return fallbackUsernamePrefix == null ? result : fallbackUsernamePrefix + result; } 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..bf6d8475 --- /dev/null +++ b/oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java @@ -0,0 +1,34 @@ +/* + * 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 io.strimzi.kafka.oauth.common.JSONUtil; +import io.strimzi.kafka.oauth.common.PrincipalExtractor; +import org.junit.Assert; +import org.junit.Test; + +public class PrincipalExtractorTest { + + @Test + public void testNestedClaim() { + + ObjectNode json = JSONUtil.newObjectNode(); + ObjectNode current = json.putObject("oauth"); + current = current.putObject("username"); + current.put("claim", "user1"); + current.put("fallbackClaim", "fallbackUser1"); + + PrincipalExtractor extractor = new PrincipalExtractor("oauth.username.claim", null, null); + String principal = extractor.getPrincipal(json); + + Assert.assertEquals("principal == user1", "user1", principal); + + extractor = new PrincipalExtractor("oauth.username.id", "oauth.username.fallbackClaim", null); + principal = extractor.getPrincipal(json); + + Assert.assertEquals("fallback principal == fallbackUser1", "fallbackUser1", principal); + } +} 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..bbe8cb77 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: *