diff --git a/README.md b/README.md index 2bc10e3b..85811514 100644 --- a/README.md +++ b/README.md @@ -458,7 +458,7 @@ See [JsonPathFilterQuery JavaDoc](oauth-common/src/main/java/io/strimzi/kafka/oa When using custom authorization (by installing a custom authorizer) you may want to take user's group membership into account when making the authorization decisions. One way is to obtain and inspect a parsed JWT token from `io.strimzi.kafka.oauth.server.OAuthKafkaPrincipal` object available through `AuthorizableRequestContext` passed to your `authorize()` method. -Another way is to configure group extraction at authentication time, and get groups as a list of principals from `OAuthKafkaPrincipal` object. +Another way is to configure group extraction at authentication time, and get groups as a set of principals from `OAuthKafkaPrincipal` object. There are two configuration parameters for configuring group extraction: - `oauth.groups.claim` (e.g.: `$.roles.client-roles.kafka`) diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/BearerTokenWithPayload.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/BearerTokenWithPayload.java index 3f03f9e9..56820f14 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/BearerTokenWithPayload.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/BearerTokenWithPayload.java @@ -6,7 +6,7 @@ import org.apache.kafka.common.security.oauthbearer.OAuthBearerToken; import com.fasterxml.jackson.databind.node.ObjectNode; -import java.util.List; +import java.util.Set; /** * This extension of OAuthBearerToken provides a way to associate any additional information with the token @@ -38,13 +38,11 @@ public interface BearerTokenWithPayload extends OAuthBearerToken { void setPayload(Object payload); /** - * Get groups associated with this token (principal). Logically, groups should be considered a Set. - * However, depending on the infrastructure (e.g. authorization server used), the order may be predictable and configurable, - * and could be used during authorization in a custom authorizer. + * Get groups associated with this token (principal). * * @return The groups for the user */ - List getGroups(); + Set getGroups(); /** * The token claims as a JSON object. For JWT tokens it contains the content of the JWT Payload part of the token. diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/TokenInfo.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/TokenInfo.java index 706f6642..e6a2768b 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/TokenInfo.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/TokenInfo.java @@ -9,7 +9,6 @@ import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; public class TokenInfo { @@ -26,7 +25,7 @@ public class TokenInfo { private Set scopes = new HashSet<>(); private long expiresAt; private String principal; - private List groups; + private Set groups; private long issuedAt; private ObjectNode payload; @@ -34,7 +33,7 @@ public TokenInfo(JsonNode payload, String token, String principal) { this(payload, token, principal, null); } - public TokenInfo(JsonNode payload, String token, String principal, List groups) { + public TokenInfo(JsonNode payload, String token, String principal, Set groups) { this(token, payload.has(SCOPE) ? payload.get(SCOPE).asText() : null, principal, @@ -48,10 +47,10 @@ public TokenInfo(JsonNode payload, String token, String principal, List this.payload = (ObjectNode) payload; } - public TokenInfo(String token, String scope, String principal, List groups, long issuedAtMs, long expiresAtMs) { + public TokenInfo(String token, String scope, String principal, Set groups, long issuedAtMs, long expiresAtMs) { this.token = token; this.principal = principal; - this.groups = groups != null ? Collections.unmodifiableList(groups) : null; + this.groups = groups != null ? Collections.unmodifiableSet(groups) : null; this.issuedAt = issuedAtMs; this.expiresAt = expiresAtMs; @@ -78,7 +77,7 @@ public String principal() { return principal; } - public List groups() { + public Set groups() { return groups; } diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/JWTSignatureValidator.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/JWTSignatureValidator.java index ac286d72..e2e5555e 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/JWTSignatureValidator.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/JWTSignatureValidator.java @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -364,7 +365,7 @@ public TokenInfo validate(String token) { } String principal = extractPrincipal(t); - List groups = extractGroups(t); + Set groups = extractGroups(t); return new TokenInfo(t, token, principal, groups); } @@ -383,7 +384,7 @@ private String extractPrincipal(JsonNode tokenJson) { return principal; } - private List extractGroups(JsonNode tokenJson) { + private Set extractGroups(JsonNode tokenJson) { if (groupsQuery == null) { return null; } @@ -393,9 +394,9 @@ private List extractGroups(JsonNode tokenJson) { } List groups = JSONUtil.asListOfString(result, groupsDelimiter != null ? groupsDelimiter : ","); // sanitize the result - groups = groups.stream().map(String::trim).filter(v -> !v.isEmpty()).collect(Collectors.toList()); + Set groupSet = groups.stream().map(String::trim).filter(v -> !v.isEmpty()).collect(Collectors.toSet()); - return groups.isEmpty() ? null : groups; + return groupSet.isEmpty() ? null : groupSet; } @SuppressWarnings({"deprecation", "unchecked"}) diff --git a/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/OAuthIntrospectionValidator.java b/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/OAuthIntrospectionValidator.java index 7cc9f3f1..3410a109 100644 --- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/OAuthIntrospectionValidator.java +++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/validator/OAuthIntrospectionValidator.java @@ -22,6 +22,7 @@ import java.net.URISyntaxException; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static io.strimzi.kafka.oauth.common.HttpUtil.post; @@ -256,7 +257,7 @@ public TokenInfo validate(String token) { } } performOptionalChecks(response); - List groups = null; + Set groups = null; if (groupsMatcher != null) { groups = extractGroupsFromResponse(response); @@ -271,7 +272,7 @@ public TokenInfo validate(String token) { return new TokenInfo(token, scopes, principal, groups, iat, expiresMillis); } - private List extractGroupsFromResponse(JsonNode userInfoJson) { + private Set extractGroupsFromResponse(JsonNode userInfoJson) { JsonNode result = groupsMatcher.apply(userInfoJson); if (result == null) { return null; @@ -279,7 +280,7 @@ private List extractGroupsFromResponse(JsonNode userInfoJson) { List groups = JSONUtil.asListOfString(result, groupsDelimiter != null ? groupsDelimiter : ","); // sanitize the result - return groups.stream().map(String::trim).filter(v -> !v.isEmpty()).collect(Collectors.toList()); + return groups.stream().map(String::trim).filter(v -> !v.isEmpty()).collect(Collectors.toSet()); } private JsonNode getUserInfoEndpointResponse(String token) { 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 a1c6879a..d22748c3 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 @@ -581,7 +581,7 @@ public void setPayload(Object value) { } @Override - public List getGroups() { + public Set getGroups() { return ti.groups(); } diff --git a/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipal.java b/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipal.java index 476a4fb1..3549b960 100644 --- a/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipal.java +++ b/oauth-server/src/main/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipal.java @@ -10,7 +10,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; -import java.util.List; +import java.util.Set; import static io.strimzi.kafka.oauth.common.LogUtil.mask; @@ -24,32 +24,32 @@ public final class OAuthKafkaPrincipal extends KafkaPrincipal { private final BearerTokenWithPayload jwt; - private final List groups; + private final Set groups; public OAuthKafkaPrincipal(String principalType, String name) { - this(principalType, name, (List) null); + this(principalType, name, (Set) null); } - public OAuthKafkaPrincipal(String principalType, String name, List groups) { + public OAuthKafkaPrincipal(String principalType, String name, Set groups) { super(principalType, name); this.jwt = null; - this.groups = groups == null ? null : Collections.unmodifiableList(groups); + this.groups = groups == null ? null : Collections.unmodifiableSet(groups); } public OAuthKafkaPrincipal(String principalType, String name, BearerTokenWithPayload jwt) { super(principalType, name); this.jwt = jwt; - List parsedGroups = jwt.getGroups(); + Set parsedGroups = jwt.getGroups(); - this.groups = parsedGroups == null ? null : Collections.unmodifiableList(parsedGroups); + this.groups = parsedGroups == null ? null : Collections.unmodifiableSet(parsedGroups); } public BearerTokenWithPayload getJwt() { return jwt; } - public List getGroups() { + public Set getGroups() { return groups; } diff --git a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/MockBearerTokenWithPayload.java b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/MockBearerTokenWithPayload.java index 604735bd..4ab9fd41 100644 --- a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/MockBearerTokenWithPayload.java +++ b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/MockBearerTokenWithPayload.java @@ -9,21 +9,20 @@ import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; public class MockBearerTokenWithPayload implements BearerTokenWithPayload { private final String principalName; - private final List groups; + private final Set groups; private final long createTime; private final long lifetime; private final Set scopes; private final String token; private Object payload; - MockBearerTokenWithPayload(String principalName, List groups, long createTime, long lifetime, String scope, String token, Object payload) { + MockBearerTokenWithPayload(String principalName, Set groups, long createTime, long lifetime, String scope, String token, Object payload) { this.principalName = principalName; this.groups = groups; this.createTime = createTime; @@ -51,7 +50,7 @@ public void setPayload(Object payload) { } @Override - public List getGroups() { + public Set getGroups() { return groups; } diff --git a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipalTest.java b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipalTest.java index f6545aba..11204092 100644 --- a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipalTest.java +++ b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthKafkaPrincipalTest.java @@ -14,18 +14,19 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; public class OAuthKafkaPrincipalTest { @Test public void testEquals() { - BearerTokenWithPayload token = new MockBearerTokenWithPayload("service-account-my-client", Arrays.asList("group1", "group2"), + BearerTokenWithPayload token = new MockBearerTokenWithPayload("service-account-my-client", new HashSet(Arrays.asList("group1", "group2")), System.currentTimeMillis(), System.currentTimeMillis() + 60000, null, "BEARER-TOKEN-9823eh982u", "Whatever"); OAuthKafkaPrincipal principal = new OAuthKafkaPrincipal("User", "service-account-my-client", token); - BearerTokenWithPayload token2 = new MockBearerTokenWithPayload("bob", Collections.emptyList(), + BearerTokenWithPayload token2 = new MockBearerTokenWithPayload("bob", Collections.emptySet(), System.currentTimeMillis(), System.currentTimeMillis() + 60000, null, "BEARER-TOKEN-0000dd0000", null); OAuthKafkaPrincipal principal2 = new OAuthKafkaPrincipal("User", "service-account-my-client", token2); diff --git a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthSessionAuthorizerTest.java b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthSessionAuthorizerTest.java index d9e5fddd..e1f99be0 100644 --- a/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthSessionAuthorizerTest.java +++ b/oauth-server/src/test/java/io/strimzi/kafka/oauth/server/OAuthSessionAuthorizerTest.java @@ -29,6 +29,7 @@ import java.net.UnknownHostException; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -103,7 +104,7 @@ private void testNonOAuthUserWithDelegate(Authorizer authorizer, MockAuthorizer private void testOAuthUserWithDelegate(Authorizer authorizer, MockAuthorizer delegateAuthorizer) throws Exception { // Prepare condition after mock OAuth athentication with valid token - TokenInfo tokenInfo = new TokenInfo("accesstoken123", null, "User:bob", Arrays.asList("group1", "group2"), + TokenInfo tokenInfo = new TokenInfo("accesstoken123", null, "User:bob", new HashSet(Arrays.asList("group1", "group2")), System.currentTimeMillis() - 100000, System.currentTimeMillis() + 100000); BearerTokenWithPayload token = new JaasServerOauthValidatorCallbackHandler.BearerTokenWithPayloadImpl(tokenInfo);