Username extraction bug fixes and additions

README.md

@@ -320,6 +320,10 @@ Use `oauth.username.claim` to map the claim (attribute) where the value you want
 
 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 a prefix that is automatically prepended to the user id. This allows for the consistent mapping of user ids into the same name space and may be needed to prevent name collisions.
+One use case is to assign a different prefix for each server when using different authorization servers for different listeners:
+- `oauth.username.prefix` (e.g.: "internal_" - there is no prefix set by default)
+
 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", for nested attributes use `[topAttrKey].[subAttrKey]`. Claim names can also be single quoted: `['topAttrKey'].['subAttrKey']`)
 - `oauth.fallback.username.prefix` (e.g.: "client-account-")

oauth-client/src/main/java/io/strimzi/kafka/oauth/client/JaasClientOauthLoginCallbackHandler.java

@@ -149,13 +149,14 @@ public void configure(Map<String, ?> configs, String saslMechanism, List<AppConf
 
         principalExtractor = new PrincipalExtractor(
                 config.getValue(Config.OAUTH_USERNAME_CLAIM),
+                config.getValue(Config.OAUTH_USERNAME_PREFIX),
                 config.getValue(Config.OAUTH_FALLBACK_USERNAME_CLAIM),
                 config.getValue(Config.OAUTH_FALLBACK_USERNAME_PREFIX));
 
         isJwt = isAccessTokenJwt(config, LOG, null);
         if (!isJwt && principalExtractor.isConfigured()) {
-            LOG.warn("Token is not JWT ('{}' is 'false') - custom username claim configuration will be ignored ('{}', '{}', '{}')",
-                    Config.OAUTH_ACCESS_TOKEN_IS_JWT, ClientConfig.OAUTH_USERNAME_CLAIM, ClientConfig.OAUTH_FALLBACK_USERNAME_CLAIM, ClientConfig.OAUTH_FALLBACK_USERNAME_PREFIX);
+            LOG.warn("Token is not JWT ('{}' is 'false') - custom username claim configuration will be ignored ('{}', '{}', '{}', '{}')",
+                    Config.OAUTH_ACCESS_TOKEN_IS_JWT, ClientConfig.OAUTH_USERNAME_CLAIM, ClientConfig.OAUTH_USERNAME_PREFIX, ClientConfig.OAUTH_FALLBACK_USERNAME_CLAIM, ClientConfig.OAUTH_FALLBACK_USERNAME_PREFIX);
         }
 
         maxTokenExpirySeconds = config.getValueAsInt(ClientConfig.OAUTH_MAX_TOKEN_EXPIRY_SECONDS, -1);

oauth-common/src/main/java/io/strimzi/kafka/oauth/common/Config.java

@@ -32,6 +32,9 @@ public class Config {
     /** The name of 'oauth.username.claim' config option  */
     public static final String OAUTH_USERNAME_CLAIM = "oauth.username.claim";
 
+    /** The name of 'oauth.username.prefix' config option  */
+    public static final String OAUTH_USERNAME_PREFIX = "oauth.username.prefix";
+
     /** The name of 'oauth.fallback.username.claim' config option  */
     public static final String OAUTH_FALLBACK_USERNAME_CLAIM = "oauth.fallback.username.claim";
 
@@ -149,7 +152,7 @@ public void validate() {}
      *   key.toUpperCase().replace('-', '_').replace('.', '_');
      * </pre>
      * If not, it checks if env variable with name equal to key exists.
-     *
+     * <p>
      * Ultimately, it checks the defaults passed at Config object construction time.
      * <p>
      * If no configuration is found for key, it returns the fallback value.

oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java

@@ -13,7 +13,8 @@
  * 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
+ * If found, and the <code>usernamePrefix</code> is configured, it is prepended to the value of the claim.
+ * 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:
@@ -44,6 +45,7 @@
 public class PrincipalExtractor {
 
     private final Extractor usernameExtractor;
+    private final String usernamePrefix;
     private final Extractor fallbackUsernameExtractor;
     private final String fallbackUsernamePrefix;
 
@@ -52,6 +54,7 @@ public class PrincipalExtractor {
      */
     public PrincipalExtractor() {
         usernameExtractor = null;
+        usernamePrefix = null;
         fallbackUsernameExtractor = null;
         fallbackUsernamePrefix = null;
     }
@@ -60,11 +63,25 @@ public PrincipalExtractor() {
      * Create a new instance
      *
      * @param usernameClaim Attribute name for an attribute containing the user id to lookup first.
+     */
+    public PrincipalExtractor(String usernameClaim) {
+        this.usernameExtractor = parseClaimSpec(usernameClaim);
+        usernamePrefix = null;
+        fallbackUsernameExtractor = null;
+        fallbackUsernamePrefix = null;
+    }
+
+    /**
+     * Create a new instance
+     *
+     * @param usernameClaim Attribute name for an attribute containing the user id to lookup first.
+     * @param usernamePrefix A prefix to prepend to the user id
      * @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) {
+    public PrincipalExtractor(String usernameClaim, String usernamePrefix, String fallbackUsernameClaim, String fallbackUsernamePrefix) {
         this.usernameExtractor = parseClaimSpec(usernameClaim);
+        this.usernamePrefix = usernamePrefix;
         this.fallbackUsernameExtractor = parseClaimSpec(fallbackUsernameClaim);
         this.fallbackUsernamePrefix = fallbackUsernamePrefix;
     }
@@ -81,11 +98,11 @@ public String getPrincipal(JsonNode json) {
         if (usernameExtractor != null) {
             result = extractUsername(usernameExtractor, json);
             if (result != null) {
-                return result;
+                return usernamePrefix != null ? usernamePrefix + result : result;
             }
             if (fallbackUsernameExtractor != null) {
                 result = extractUsername(fallbackUsernameExtractor, json);
-                return result;
+                return result != null && fallbackUsernamePrefix != null ? fallbackUsernamePrefix + result : result;
             }
         }
 
@@ -120,7 +137,7 @@ public String getSub(JsonNode json) {
 
     @Override
     public String toString() {
-        return "PrincipalExtractor {usernameClaim: " + usernameExtractor + ", fallbackUsernameClaim: " + fallbackUsernameExtractor + ", fallbackUsernamePrefix: " + fallbackUsernamePrefix + "}";
+        return "PrincipalExtractor {usernameClaim: " + usernameExtractor + ", usernamePrefix: " + usernamePrefix + ", fallbackUsernameClaim: " + fallbackUsernameExtractor + ", fallbackUsernamePrefix: " + fallbackUsernamePrefix + "}";
     }
 
     /**
@@ -129,7 +146,7 @@ public String toString() {
      * @return True if any of the constructor parameters is set
      */
     public boolean isConfigured() {
-        return usernameExtractor != null || fallbackUsernameExtractor != null || fallbackUsernamePrefix != null;
+        return usernameExtractor != null || usernamePrefix != null || fallbackUsernameExtractor != null || fallbackUsernamePrefix != null;
     }
 
     /**

oauth-common/src/test/java/io/strimzi/kafka/oauth/common/PrincipalExtractorTest.java

@@ -12,16 +12,16 @@ public class PrincipalExtractorTest {
     @Test
     public void testToStringMethod() {
 
-        PrincipalExtractor extractor = new PrincipalExtractor("username.claim", null, null);
-        Assert.assertEquals("PrincipalExtractor {usernameClaim: username.claim, fallbackUsernameClaim: null, fallbackUsernamePrefix: null}", extractor.toString());
+        PrincipalExtractor extractor = new PrincipalExtractor("username.claim", null, null, null);
+        Assert.assertEquals("PrincipalExtractor {usernameClaim: username.claim, usernamePrefix: null, fallbackUsernameClaim: null, fallbackUsernamePrefix: null}", extractor.toString());
 
-        extractor = new PrincipalExtractor("['username'].['claim']", null, null);
-        Assert.assertEquals("PrincipalExtractor {usernameClaim: ['username'].['claim'], fallbackUsernameClaim: null, fallbackUsernamePrefix: null}", extractor.toString());
+        extractor = new PrincipalExtractor("['username'].['claim']", "admins_", null, "client_");
+        Assert.assertEquals("PrincipalExtractor {usernameClaim: ['username'].['claim'], usernamePrefix: admins_, fallbackUsernameClaim: null, fallbackUsernamePrefix: client_}", extractor.toString());
 
-        extractor = new PrincipalExtractor("username", "user.id", null);
-        Assert.assertEquals("PrincipalExtractor {usernameClaim: username, fallbackUsernameClaim: user.id, fallbackUsernamePrefix: null}", extractor.toString());
+        extractor = new PrincipalExtractor("username", null, "user.id", null);
+        Assert.assertEquals("PrincipalExtractor {usernameClaim: username, usernamePrefix: null, fallbackUsernameClaim: user.id, fallbackUsernamePrefix: null}", extractor.toString());
 
-        extractor = new PrincipalExtractor("username", "['user.id']", null);
-        Assert.assertEquals("PrincipalExtractor {usernameClaim: username, fallbackUsernameClaim: ['user.id'], fallbackUsernamePrefix: null}", extractor.toString());
+        extractor = new PrincipalExtractor("username", "intra_", "['user.id']", "intra_service-account-");
+        Assert.assertEquals("PrincipalExtractor {usernameClaim: username, usernamePrefix: intra_, fallbackUsernameClaim: ['user.id'], fallbackUsernamePrefix: intra_service-account-}", extractor.toString());
     }
 }

oauth-common/src/test/java/io/strimzi/kafka/oauth/validator/PrincipalExtractorTest.java

@@ -115,25 +115,31 @@ public void testUsernameClaim() throws IOException {
             "[ 'userInfo' ] [ 'id' ]"
         };
 
+        String usernamePrefix = "prefix-";
+        String fallbackUsernamePrefix = "fallback_prefix-";
+
         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));
+                PrincipalExtractor extractor = new PrincipalExtractor(query, usernamePrefix, null, null);
+                Assert.assertEquals(query + " top level works as primary", applyPrefix(usernamePrefix, expected), extractor.getPrincipal(json));
+            } catch (Exception e) {
+                throw new RuntimeException("Unexpected error while testing: " + query + " expecting it to return: " + applyPrefix(usernamePrefix, expected), e);
+            }
 
-                extractor = new PrincipalExtractor("nonexisting", query, null);
-                Assert.assertEquals(query + " top level works as fallback", expected, extractor.getPrincipal(json));
+            try {
+                PrincipalExtractor extractor = new PrincipalExtractor("nonexisting", usernamePrefix, query, fallbackUsernamePrefix);
+                Assert.assertEquals(query + " top level works as fallback", applyPrefix(fallbackUsernamePrefix, expected), extractor.getPrincipal(json));
             } catch (Exception e) {
-                throw new RuntimeException("Unexpected error while testing: " + query + " expecting it to return: " + expected, e);
+                throw new RuntimeException("Unexpected error while testing: " + query + " expecting it to return: " + applyPrefix(fallbackUsernamePrefix, expected), e);
             }
         }
 
-        for (int i = 0; i < claimSpecError.length; i++) {
-            String query = claimSpecError[i];
+        for (String query : claimSpecError) {
             try {
-                PrincipalExtractor extractor = new PrincipalExtractor(query, "nonexisting", null);
+                PrincipalExtractor extractor = new PrincipalExtractor(query, usernamePrefix, "nonexisting", fallbackUsernamePrefix);
                 extractor.getPrincipal(json);
                 Assert.fail("Should have failed");
             } catch (JsonPathQueryException e) {
@@ -142,4 +148,8 @@ public void testUsernameClaim() throws IOException {
             }
         }
     }
+
+    private String applyPrefix(String prefix, String value) {
+        return value != null ? prefix + value : null;
+    }
 }

oauth-server/src/main/java/io/strimzi/kafka/oauth/server/JaasServerOauthValidatorCallbackHandler.java

@@ -144,10 +144,13 @@
  * <li><em>oauth.username.claim</em> The attribute key that should be used to extract the user id. If not set `sub` attribute is used.<br>
  * The attribute key refers to the JWT token claim when fast local validation is used, or to attribute in the response by introspection endpoint when introspection based validation is used.
  * For nested attributes use '[topAttrKey].[subAttrKey]'. Claim names can also be single quoted: ['topAttrKey'].['subAttrKey']. It has no default value.</li>
+ * <li><em>oauth.username.prefix</em> The prefix to automatically prepend to the user id. <br>
+ * This allows for consistent mapping of user ids into the same name space and may be needed to prevent name collisions. <br>
+ * One use case is to assign a different prefix for each server when using different authorization servers for different listeners. By default there is no prefix.</li>
  * <li><em>oauth.fallback.username.claim</em> The fallback username claim to be used for the user id if the attribute key specified by `oauth.username.claim` <br>
  * is not present. This is useful when `client_credentials` authentication only results in the client id being provided in another claim. For nested attributes same rules apply as for `oauth.username.claim`.
  * <li><em>oauth.fallback.username.prefix</em> The prefix to use with the value of <em>oauth.fallback.username.claim</em> to construct the user id.  <br>
- * This only takes effect if <em>oauth.fallback.username.claim</em> is <em>true</em>, and the value is present for the claim.
+ * This only takes effect if <em>oauth.fallback.username.claim</em> is configured, and the value is present for the claim.
  * Mapping usernames and client ids into the same user id space is useful in preventing name collisions.</li>
  * <li><em>oauth.check.issuer</em> Enable or disable issuer checking. <br>
  * By default issuer is checked using the value configured by <em>oauth.valid.issuer.uri</em>. Default value is <em>true</em></li>
@@ -266,12 +269,13 @@ public void delegatedConfigure(Map<String, ?> configs, String saslMechanism, Lis
         includeAcceptHeader = config.getValueAsBoolean(Config.OAUTH_INCLUDE_ACCEPT_HEADER, true);
 
         String usernameClaim = config.getValue(Config.OAUTH_USERNAME_CLAIM);
+        String usernamePrefix = config.getValue(Config.OAUTH_USERNAME_PREFIX);
         String fallbackUsernameClaim = config.getValue(Config.OAUTH_FALLBACK_USERNAME_CLAIM);
         String fallbackUsernamePrefix = config.getValue(Config.OAUTH_FALLBACK_USERNAME_PREFIX);
 
         validateFallbackUsernameParameters(usernameClaim, fallbackUsernameClaim, fallbackUsernamePrefix);
 
-        principalExtractor = new PrincipalExtractor(usernameClaim, fallbackUsernameClaim, fallbackUsernamePrefix);
+        principalExtractor = new PrincipalExtractor(usernameClaim, usernamePrefix, fallbackUsernameClaim, fallbackUsernamePrefix);
 
         String clientId = config.getValue(Config.OAUTH_CLIENT_ID);
         String clientSecret = config.getValue(Config.OAUTH_CLIENT_SECRET);

testsuite/keycloak-auth-tests/src/test/java/io/strimzi/testsuite/oauth/auth/BasicTests.java

@@ -436,7 +436,7 @@ void passwordGrantWithIntrospection() throws Exception {
         String accessToken = loginWithUsernamePassword(URI.create(tokenEndpointUri), username, password, clientId);
 
         TokenInfo tokenInfo = introspectAccessToken(accessToken,
-                new PrincipalExtractor("preferred_username", null, null));
+                new PrincipalExtractor("preferred_username"));
 
         Assert.assertEquals("Token contains 'preferred_username' claim with value equal to username", username, tokenInfo.principal());
 
@@ -465,7 +465,7 @@ void passwordGrantWithIntrospection() throws Exception {
         accessToken = loginWithUsernamePassword(URI.create(tokenEndpointUri), username, password, confidentialClientId, confidentialClientSecret);
 
         tokenInfo = introspectAccessToken(accessToken,
-                new PrincipalExtractor("preferred_username", null, null));
+                new PrincipalExtractor("preferred_username"));
 
         Assert.assertEquals("Token contains 'preferred_username' claim with value equal to username", username, tokenInfo.principal());
 

testsuite/mockoauth-tests/src/test/java/io/strimzi/testsuite/oauth/MockOAuthTests.java

@@ -8,17 +8,17 @@
 import io.strimzi.testsuite.oauth.common.TestContainersLogCollector;
 import io.strimzi.testsuite.oauth.common.TestContainersWatcher;
 import io.strimzi.testsuite.oauth.mockoauth.AuthorizationEndpointsTest;
-import io.strimzi.testsuite.oauth.mockoauth.JaasServerConfigTest;
-import io.strimzi.testsuite.oauth.mockoauth.metrics.MetricsTest;
 import io.strimzi.testsuite.oauth.mockoauth.ClientAssertionAuthTest;
 import io.strimzi.testsuite.oauth.mockoauth.ConnectTimeoutTests;
 import io.strimzi.testsuite.oauth.mockoauth.JWKSKeyUseTest;
 import io.strimzi.testsuite.oauth.mockoauth.JaasClientConfigTest;
+import io.strimzi.testsuite.oauth.mockoauth.JaasServerConfigTest;
 import io.strimzi.testsuite.oauth.mockoauth.KeycloakAuthorizerTest;
-import io.strimzi.testsuite.oauth.mockoauth.PasswordAuthTest;
+import io.strimzi.testsuite.oauth.mockoauth.PasswordAuthAndPrincipalExtractionTest;
 import io.strimzi.testsuite.oauth.mockoauth.RetriesTests;
 import io.strimzi.testsuite.oauth.mockoauth.KerberosListenerTest;
 
+import io.strimzi.testsuite.oauth.mockoauth.metrics.MetricsTest;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -96,8 +96,8 @@ public void runTests() throws Exception {
             logStart("JaasServerConfigTest :: Server Configuration Tests");
             new JaasServerConfigTest().doTest();
 
-            logStart("PasswordAuthTest :: Password Grant Tests");
-            new PasswordAuthTest().doTest();
+            logStart("PasswordAuthAndPrincipalExtractionTest :: Password Grant  +  Fallback Username / Prefix Tests");
+            new PasswordAuthAndPrincipalExtractionTest().doTest();
 
             logStart("ConnectTimeoutTests :: HTTP Timeout Tests");
             new ConnectTimeoutTests(kafkaContainer).doTest();

testsuite/mockoauth-tests/src/test/java/io/strimzi/testsuite/oauth/mockoauth/JWKSKeyUseTest.java

@@ -25,7 +25,7 @@
 import static io.strimzi.testsuite.oauth.mockoauth.Common.getProjectRoot;
 
 public class JWKSKeyUseTest {
-    private static final Logger log = LoggerFactory.getLogger(PasswordAuthTest.class);
+    private static final Logger log = LoggerFactory.getLogger(JWKSKeyUseTest.class);
 
     public void doTest() throws Exception {
         Services.configure(Collections.emptyMap());

testsuite/mockoauth-tests/src/test/java/io/strimzi/testsuite/oauth/mockoauth/JaasServerConfigTest.java

@@ -85,7 +85,7 @@ private void testJwksValidatorOptions() throws IOException {
                 "keysEndpointUri", "https://sso/jwks",
                 "usernameClaim", "username-claim",
                 "fallbackUsernameClaim", "fallback-username-claim",
-                "fallbackUsernamePrefix", "username-prefix",
+                "fallbackUsernamePrefix", "fallback-username-prefix",
                 "groupsClaimQuery", "\\$\\.groups",
                 "groupsClaimDelimiter", ",",
                 "validIssuerUri", "https://sso",
@@ -239,7 +239,7 @@ private void testIntrospectValidatorOptions() throws IOException {
                 "audience", "client-id",
                 "usernameClaim", "username-claim",
                 "fallbackUsernameClaim", "fallback-username-claim",
-                "fallbackUsernamePrefix", "username-prefix",
+                "fallbackUsernamePrefix", "fallback-username-prefix",
                 "customClaimCheck", "@\\.aud anyof \\['kafka', 'something'\\]",
                 "connectTimeoutSeconds", "10",
                 "readTimeoutSeconds", "10",