diff --git a/oauth-keycloak-authorizer/pom.xml b/oauth-keycloak-authorizer/pom.xml
index 22b582fa..fb7a6e65 100644
--- a/oauth-keycloak-authorizer/pom.xml
+++ b/oauth-keycloak-authorizer/pom.xml
@@ -44,6 +44,17 @@
scala-library
provided
+
+ com.github.spotbugs
+ spotbugs-annotations
+ provided
+
+
+ junit
+ junit
+ ${junit.version}
+ test
+
@@ -56,6 +67,10 @@
${maven.compiler.target}
+
+ maven-surefire-plugin
+ ${maven.surefire.version}
+
diff --git a/oauth-keycloak-authorizer/src/main/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipal.java b/oauth-keycloak-authorizer/src/main/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipal.java
index 0001d0f8..27a68675 100644
--- a/oauth-keycloak-authorizer/src/main/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipal.java
+++ b/oauth-keycloak-authorizer/src/main/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipal.java
@@ -7,9 +7,19 @@
import io.strimzi.kafka.oauth.common.BearerTokenWithPayload;
import org.apache.kafka.common.security.auth.KafkaPrincipal;
-import java.util.Objects;
-
-public class JwtKafkaPrincipal extends KafkaPrincipal {
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+/**
+ * This class uses the KafkaPrincipal object to store additional info obtained at session authentication time,
+ * and required later by a custom authorizer.
+ *
+ * This class is the only notion of client session that we can get. Kafka code holds on to it for as long as the session is alive,
+ * and then the object can be garbage collected.
+ *
+ * Any additional fields should not be included in equals / hashcode check. If they are, that will break re-authentication.
+ */
+@SuppressFBWarnings("EQ_DOESNT_OVERRIDE_EQUALS")
+public final class JwtKafkaPrincipal extends KafkaPrincipal {
private final BearerTokenWithPayload jwt;
@@ -25,20 +35,4 @@ public JwtKafkaPrincipal(String principalType, String name, BearerTokenWithPaylo
public BearerTokenWithPayload getJwt() {
return jwt;
}
-
- @Override
- public boolean equals(Object o) {
- if (this == o) return true;
- if (o == null || getClass() != o.getClass()) return false;
-
- if (!super.equals(o)) return false;
-
- JwtKafkaPrincipal that = (JwtKafkaPrincipal) o;
- return Objects.equals(jwt, that.jwt);
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(super.hashCode(), jwt);
- }
}
diff --git a/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipalTest.java b/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipalTest.java
new file mode 100644
index 00000000..aba4b383
--- /dev/null
+++ b/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/JwtKafkaPrincipalTest.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2017-2019, 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.server.authorizer;
+
+import io.strimzi.kafka.oauth.common.BearerTokenWithPayload;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class JwtKafkaPrincipalTest {
+
+ @Test
+ public void testEquals() {
+
+ BearerTokenWithPayload token = new MockBearerTokenWithPayload("service-account-my-client",
+ System.currentTimeMillis(), System.currentTimeMillis() + 60000, null, "BEARER-TOKEN-9823eh982u", "Whatever");
+ JwtKafkaPrincipal principal = new JwtKafkaPrincipal("User", "service-account-my-client", token);
+
+
+ BearerTokenWithPayload token2 = new MockBearerTokenWithPayload("bob",
+ System.currentTimeMillis(), System.currentTimeMillis() + 60000, null, "BEARER-TOKEN-0000dd0000", null);
+ JwtKafkaPrincipal principal2 = new JwtKafkaPrincipal("User", "service-account-my-client", token2);
+
+
+ JwtKafkaPrincipal principal3 = new JwtKafkaPrincipal("User", "service-account-my-client");
+
+ JwtKafkaPrincipal principal4 = new JwtKafkaPrincipal("User", "bob");
+
+
+ Assert.assertTrue("principal should be equal to principal2", principal.equals(principal2));
+ Assert.assertTrue("principal2 should be equal to principal", principal2.equals(principal));
+
+ Assert.assertTrue("principal should be equal to principal3", principal.equals(principal3));
+ Assert.assertTrue("principal3 should be equal to principal", principal3.equals(principal));
+
+ Assert.assertTrue("principal2 should be equal to principal3", principal2.equals(principal3));
+ Assert.assertTrue("principal3 should be equal to principal2", principal3.equals(principal2));
+
+ Assert.assertTrue("principal should be equal to itself", principal.equals(principal));
+ Assert.assertTrue("principal2 should be equal to itself", principal2.equals(principal2));
+ Assert.assertTrue("principal3 should be equal to itself", principal3.equals(principal3));
+ Assert.assertTrue("principal4 should be equal to itself", principal4.equals(principal4));
+
+ Assert.assertFalse("principal should not be equal to principal4", principal.equals(principal4));
+ Assert.assertFalse("principal4 should not be equal to principal", principal4.equals(principal));
+ Assert.assertFalse("principal3 should not be equal to principal4", principal3.equals(principal4));
+ Assert.assertFalse("principal4 should not be equal to principal3", principal4.equals(principal3));
+
+ long hash1 = principal.hashCode();
+ long hash2 = principal2.hashCode();
+ long hash3 = principal3.hashCode();
+ long hash4 = principal4.hashCode();
+
+ Assert.assertTrue("Hashcode1 should be equal to hashcode2", hash1 == hash2);
+ Assert.assertTrue("Hashcode1 should be equal to hashcode3", hash1 == hash3);
+ Assert.assertFalse("Hashcode1 should not be equal to hashcode4", hash1 == hash4);
+ }
+}
diff --git a/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/MockBearerTokenWithPayload.java b/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/MockBearerTokenWithPayload.java
new file mode 100644
index 00000000..ca31e6a4
--- /dev/null
+++ b/oauth-keycloak-authorizer/src/test/java/io/strimzi/kafka/oauth/server/authorizer/MockBearerTokenWithPayload.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2017-2019, 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.server.authorizer;
+
+import io.strimzi.kafka.oauth.common.BearerTokenWithPayload;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+public class MockBearerTokenWithPayload implements BearerTokenWithPayload {
+
+
+ private final String principalName;
+ private final long createTime;
+ private final long lifetime;
+ private final Set scopes;
+ private final String token;
+ private Object payload;
+
+ MockBearerTokenWithPayload(String principalName, long createTime, long lifetime, String scope, String token, Object payload) {
+ this.principalName = principalName;
+ this.createTime = createTime;
+ this.lifetime = lifetime;
+
+ Set scopesSet = new HashSet<>();
+ String[] parsedScopes = scope != null ? scope.split(" ") : new String[0];
+ for (String s: parsedScopes) {
+ scopesSet.add(s);
+ }
+ scopes = Collections.unmodifiableSet(scopesSet);
+
+ this.token = token;
+ this.payload = payload;
+ }
+
+ @Override
+ public Object getPayload() {
+ return payload;
+ }
+
+ @Override
+ public void setPayload(Object payload) {
+ this.payload = payload;
+ }
+
+ @Override
+ public String value() {
+ return token;
+ }
+
+ @Override
+ public Set scope() {
+ return scopes;
+ }
+
+ @Override
+ public long lifetimeMs() {
+ return lifetime;
+ }
+
+ @Override
+ public String principalName() {
+ return principalName;
+ }
+
+ @Override
+ public Long startTimeMs() {
+ return createTime;
+ }
+}
diff --git a/pom.xml b/pom.xml
index 814dd87d..1368cc8e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -59,6 +59,7 @@
1.8
3.8.1
3.1.1
+ 2.22.1
3.1.0
3.0.1
1.6
@@ -210,6 +211,10 @@
+
+ maven-surefire-plugin
+ ${maven.surefire.version}
+