This is an automated email from the ASF dual-hosted git repository.
cshannon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/main by this push:
new b404e1e335 Optimize SecurityContext#isInOneOf (#1874)
b404e1e335 is described below
commit b404e1e335011af93ff3c40318bcfc2fc2800660
Author: Christopher L. Shannon <[email protected]>
AuthorDate: Fri Apr 3 18:20:25 2026 -0400
Optimize SecurityContext#isInOneOf (#1874)
This method was iterating over a set to find matching entries instead of
just using contains(). This updates switches to using contains() which
will be faster and also cleans up the code with a for each loop.
This also removes the copying of the principal set set copy that is
unnecessary and actually not thread safe without external sync when using
Jaas.
A small test was added but there are several other authorization tests
that already exist that also exercise this method.
---
.../apache/activemq/security/SecurityContext.java | 16 ++--
.../activemq/security/SecurityContextTest.java | 95 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 8 deletions(-)
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/security/SecurityContext.java
b/activemq-broker/src/main/java/org/apache/activemq/security/SecurityContext.java
index fd677ce590..f24b6ffc3f 100644
---
a/activemq-broker/src/main/java/org/apache/activemq/security/SecurityContext.java
+++
b/activemq-broker/src/main/java/org/apache/activemq/security/SecurityContext.java
@@ -54,14 +54,9 @@ public abstract class SecurityContext {
}
public boolean isInOneOf(Set<?> allowedPrincipals) {
- Iterator<?> allowedIter = allowedPrincipals.iterator();
- HashSet<?> userPrincipals = new HashSet<Object>(getPrincipals());
- while (allowedIter.hasNext()) {
- Iterator<?> userIter = userPrincipals.iterator();
- Object allowedPrincipal = allowedIter.next();
- while (userIter.hasNext()) {
- if (allowedPrincipal.equals(userIter.next()))
- return true;
+ for (Object allowedPrincipal : allowedPrincipals) {
+ if (contains(allowedPrincipal)) {
+ return true;
}
}
return false;
@@ -69,6 +64,11 @@ public abstract class SecurityContext {
public abstract Set<Principal> getPrincipals();
+ public boolean contains(Object principal) {
+ Set<Principal> principals = getPrincipals();
+ return principals != null && principals.contains(principal);
+ }
+
public String getUserName() {
return userName;
}
diff --git
a/activemq-broker/src/test/java/org/apache/activemq/security/SecurityContextTest.java
b/activemq-broker/src/test/java/org/apache/activemq/security/SecurityContextTest.java
new file mode 100644
index 0000000000..51ffb95d81
--- /dev/null
+++
b/activemq-broker/src/test/java/org/apache/activemq/security/SecurityContextTest.java
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.security;
+
+import static
org.apache.activemq.security.SecurityContextTest.TestPrincipal.testPrincipal;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.security.Principal;
+import java.util.Objects;
+import java.util.Set;
+import org.junit.Test;
+
+public class SecurityContextTest {
+
+ @Test
+ public void testIsOneOf() {
+ SecurityContext context = newContext(Set.of(testPrincipal("one"),
+ testPrincipal("two"), testPrincipal("three")));
+
+ // test valid combos
+ assertTrue(context.isInOneOf(Set.of(testPrincipal("one"))));
+ assertTrue(context.isInOneOf(Set.of(testPrincipal("two"))));
+ assertTrue(context.isInOneOf(Set.of(testPrincipal("three"))));
+ assertTrue(context.isInOneOf(Set.of(testPrincipal("three"),
+ testPrincipal("four"), testPrincipal("five"))));
+
+ // test no matching
+ assertFalse(context.isInOneOf(Set.of(testPrincipal("four"),
+ testPrincipal("five"))));
+ assertFalse(context.isInOneOf(Set.of()));
+ // different impl types, should not find
+ assertFalse(context.isInOneOf(Set.of((Principal) () -> "one")));
+
+ // empty set
+ context = newContext(Set.of());
+ assertFalse(context.isInOneOf(Set.of(testPrincipal("one"))));
+ assertFalse(context.isInOneOf(Set.of()));
+ }
+
+ private SecurityContext newContext(Set<Principal> principals) {
+ return new SecurityContext("user") {
+ @Override
+ public Set<Principal> getPrincipals() {
+ return principals;
+ }
+ };
+ }
+
+ static class TestPrincipal implements Principal {
+ private final String name;
+
+ private TestPrincipal(String name) {
+ this.name = name;
+ }
+
+ @Override
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ TestPrincipal that = (TestPrincipal) o;
+ return Objects.equals(name, that.name);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name);
+ }
+
+ static TestPrincipal testPrincipal(String name) {
+ return new TestPrincipal(name);
+ }
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
