[prev in list] [next in list] [prev in thread] [next in thread] 

List:       activemq-commits
Subject:    [activemq-artemis] branch main updated: ARTEMIS-4399 fix disabled authn/z cache
From:       robbie () apache ! org
Date:       2023-08-24 10:25:36
Message-ID: 169287273623.54599.3943738805006050671 () gitbox2-he-fi ! apache ! org
[Download RAW message or body]

This is an automated email from the ASF dual-hosted git repository.

robbie pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git


The following commit(s) were added to refs/heads/main by this push:
     new 29fafb5fed ARTEMIS-4399 fix disabled authn/z cache
29fafb5fed is described below

commit 29fafb5fed78788851273491f6f49638cbff828b
Author: Justin Bertram <jbertram@apache.org>
AuthorDate: Wed Aug 23 14:43:14 2023 -0500

    ARTEMIS-4399 fix disabled authn/z cache
---
 .../core/security/impl/SecurityStoreImpl.java      | 101 +++++++++++++++++----
 .../core/security/impl/SecurityStoreImplTest.java  |  94 +++++++++++++++++++
 2 files changed, 176 insertions(+), 19 deletions(-)

diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java \
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
 index 14c393d032..35c4c87417 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
                
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
 @@ -98,14 +98,22 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  this.managementClusterUser = managementClusterUser;
       this.managementClusterPassword = managementClusterPassword;
       this.notificationService = notificationService;
-      authenticationCache = CacheBuilder.newBuilder()
-                                        .maximumSize(authenticationCacheSize)
-                                        .expireAfterWrite(invalidationInterval, \
                TimeUnit.MILLISECONDS)
-                                        .build();
-      authorizationCache = CacheBuilder.newBuilder()
-                                       .maximumSize(authorizationCacheSize)
-                                       .expireAfterWrite(invalidationInterval, \
                TimeUnit.MILLISECONDS)
-                                       .build();
+      if (authenticationCacheSize == 0) {
+         authenticationCache = null;
+      } else {
+         authenticationCache = CacheBuilder.newBuilder()
+                                           .maximumSize(authenticationCacheSize)
+                                           .expireAfterWrite(invalidationInterval, \
TimeUnit.MILLISECONDS) +                                           .build();
+      }
+      if (authorizationCacheSize == 0) {
+         authorizationCache = null;
+      } else {
+         authorizationCache = CacheBuilder.newBuilder()
+                                          .maximumSize(authorizationCacheSize)
+                                          .expireAfterWrite(invalidationInterval, \
TimeUnit.MILLISECONDS) +                                          .build();
+      }
       this.securityRepository.registerListener(this);
    }
 
@@ -159,7 +167,7 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  boolean check = true;
 
          Subject subject = null;
-         Pair<Boolean, Subject> cacheEntry = \
authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password, \
connection)); +         Pair<Boolean, Subject> cacheEntry = \
getAuthenticationCacheEntry(user, password, connection);  if (cacheEntry != null) {
             if (!cacheEntry.getA()) {
                // cached authentication failed previously so don't check again
@@ -186,7 +194,7 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  if (securityManager instanceof ActiveMQSecurityManager5) {
                try {
                   subject = ((ActiveMQSecurityManager5) \
                securityManager).authenticate(user, password, connection, \
                securityDomain);
-                  authenticationCache.put(createAuthenticationCacheKey(user, \
password, connection), new Pair<>(subject != null, subject)); +                  \
putAuthenticationCacheEntry(user, password, connection, subject);  validatedUser = \
getUserFromSubject(subject);  } catch (NoCacheLoginException e) {
                   handleNoCacheLoginException(e);
@@ -313,12 +321,12 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  // if we get here we're granted, add to the cache
          ConcurrentHashSet<SimpleString> set;
          String key = createAuthorizationCacheKey(user, checkType);
-         ConcurrentHashSet<SimpleString> act = authorizationCache.getIfPresent(key);
+         ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(key);
          if (act != null) {
             set = act;
          } else {
             set = new ConcurrentHashSet<>();
-            authorizationCache.put(key, set);
+            putAuthorizationCacheEntry(set, key);
          }
          set.add(fqqn != null ? fqqn : bareAddress);
       }
@@ -394,7 +402,7 @@ public class SecurityStoreImpl implements SecurityStore, \
                HierarchicalRepositoryC
     * @return the authenticated Subject with all associated role principals
     */
    private Subject getSubjectForAuthorization(SecurityAuth auth, \
                ActiveMQSecurityManager5 securityManager) {
-      Pair<Boolean, Subject> cached = \
authenticationCache.getIfPresent(createAuthenticationCacheKey(auth.getUsername(), \
auth.getPassword(), auth.getRemotingConnection())); +      Pair<Boolean, Subject> \
cached = getAuthenticationCacheEntry(auth.getUsername(), auth.getPassword(), \
auth.getRemotingConnection());  
       if (cached == null && auth.getUsername() == null && auth.getPassword() == null \
                && auth.getRemotingConnection() instanceof \
                ManagementRemotingConnection) {
          AccessControlContext accessControlContext = AccessController.getContext();
@@ -410,7 +418,7 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  if (cached == null) {
          try {
             Subject subject = securityManager.authenticate(auth.getUsername(), \
                auth.getPassword(), auth.getRemotingConnection(), \
                auth.getSecurityDomain());
-            authenticationCache.put(createAuthenticationCacheKey(auth.getUsername(), \
auth.getPassword(), auth.getRemotingConnection()), new Pair<>(subject != null, \
subject)); +            putAuthenticationCacheEntry(auth.getUsername(), \
auth.getPassword(), auth.getRemotingConnection(), subject);  return subject;
          } catch (NoCacheLoginException e) {
             handleNoCacheLoginException(e);
@@ -424,26 +432,71 @@ public class SecurityStoreImpl implements SecurityStore, \
                HierarchicalRepositoryC
       logger.debug("Skipping authentication cache due to exception: {}", \
e.getMessage());  }
 
+   private void putAuthenticationCacheEntry(String user,
+                                            String password,
+                                            RemotingConnection connection,
+                                            Subject subject) {
+      if (authenticationCache != null) {
+         authenticationCache.put(createAuthenticationCacheKey(user, password, \
connection), new Pair<>(subject != null, subject)); +      }
+   }
+
+   private Pair<Boolean, Subject> getAuthenticationCacheEntry(String user,
+                                                              String password,
+                                                              RemotingConnection \
connection) { +      if (authenticationCache == null) {
+         return null;
+      } else {
+         return authenticationCache.getIfPresent(createAuthenticationCacheKey(user, \
password, connection)); +      }
+   }
+
+   private void putAuthorizationCacheEntry(ConcurrentHashSet<SimpleString> set, \
String key) { +      if (authorizationCache != null) {
+         authorizationCache.put(key, set);
+      }
+   }
+
+   private ConcurrentHashSet<SimpleString> getAuthorizationCacheEntry(String key) {
+      if (authorizationCache == null) {
+         return null;
+      } else {
+         return authorizationCache.getIfPresent(key);
+      }
+   }
+
    public void invalidateAuthorizationCache() {
-      authorizationCache.invalidateAll();
+      if (authorizationCache != null) {
+         authorizationCache.invalidateAll();
+      }
    }
 
    public void invalidateAuthenticationCache() {
-      authenticationCache.invalidateAll();
+      if (authenticationCache != null) {
+         authenticationCache.invalidateAll();
+      }
    }
 
    public long getAuthenticationCacheSize() {
-      return authenticationCache.size();
+      if (authenticationCache == null) {
+         return 0;
+      } else {
+         return authenticationCache.size();
+      }
    }
 
    public long getAuthorizationCacheSize() {
-      return authorizationCache.size();
+      if (authorizationCache == null) {
+         return 0;
+      } else {
+         return authorizationCache.size();
+      }
    }
 
    private boolean checkAuthorizationCache(final SimpleString dest, final String \
user, final CheckType checkType) {  boolean granted = false;
 
-      ConcurrentHashSet<SimpleString> act = \
authorizationCache.getIfPresent(createAuthorizationCacheKey(user, checkType)); +      \
ConcurrentHashSet<SimpleString> act = \
getAuthorizationCacheEntry(createAuthorizationCacheKey(user, checkType));  if (act != \
null) {  granted = act.contains(dest);
       }
@@ -458,4 +511,14 @@ public class SecurityStoreImpl implements SecurityStore, \
HierarchicalRepositoryC  private String createAuthorizationCacheKey(String user, \
CheckType checkType) {  return user + "." + checkType.name();
    }
+
+   // for testing
+   protected Cache<String, Pair<Boolean, Subject>> getAuthenticationCache() {
+      return authenticationCache;
+   }
+
+   // for testing
+   protected Cache<String, ConcurrentHashSet<SimpleString>> getAuthorizationCache() \
{ +      return authorizationCache;
+   }
 }
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java \
b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
 new file mode 100644
index 0000000000..ae22d1b812
--- /dev/null
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java
 @@ -0,0 +1,94 @@
+/*
+ * 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.artemis.core.security.impl;
+
+import javax.security.auth.Subject;
+import java.util.Set;
+
+import org.apache.activemq.artemis.core.security.CheckType;
+import org.apache.activemq.artemis.core.security.Role;
+import org.apache.activemq.artemis.core.security.SecurityAuth;
+import org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5;
+import org.apache.activemq.artemis.spi.core.security.jaas.NoCacheLoginException;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class SecurityStoreImplTest {
+
+   @Test
+   public void zeroCacheSizeTest() throws Exception {
+      ActiveMQSecurityManager5 securityManager = new ActiveMQSecurityManager5() {
+         @Override
+         public Subject authenticate(String user,
+                                     String password,
+                                     RemotingConnection remotingConnection,
+                                     String securityDomain) throws \
NoCacheLoginException { +            return new Subject();
+         }
+
+         @Override
+         public boolean authorize(Subject subject, Set<Role> roles, CheckType \
checkType, String address) { +            return true;
+         }
+
+         @Override
+         public boolean validateUser(String user, String password) {
+            return false;
+         }
+
+         @Override
+         public boolean validateUserAndRole(String user, String password, Set<Role> \
roles, CheckType checkType) { +            return false;
+         }
+      };
+      SecurityStoreImpl securityStore = new SecurityStoreImpl(new \
HierarchicalObjectRepository<>(), securityManager, 999, true, "", null, null, 0, 0); \
+      assertNull(securityStore.getAuthenticationCache()); +      \
securityStore.authenticate(RandomUtil.randomString(), RandomUtil.randomString(), \
null); +      assertEquals(0, securityStore.getAuthenticationCacheSize());
+      securityStore.invalidateAuthenticationCache(); // ensure this doesn't throw an \
NPE +
+      assertNull(securityStore.getAuthorizationCache());
+      securityStore.check(RandomUtil.randomSimpleString(), CheckType.SEND, new \
SecurityAuth() { +         @Override
+         public String getUsername() {
+            return RandomUtil.randomString();
+         }
+
+         @Override
+         public String getPassword() {
+            return RandomUtil.randomString();
+         }
+
+         @Override
+         public RemotingConnection getRemotingConnection() {
+            return null;
+         }
+
+         @Override
+         public String getSecurityDomain() {
+            return null;
+         }
+      });
+      assertEquals(0, securityStore.getAuthorizationCacheSize());
+      securityStore.invalidateAuthorizationCache(); // ensure this doesn't throw an \
NPE +   }
+}
\ No newline at end of file


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic