[activemq-artemis] branch master updated: ARTEMIS-3102 throw more accurate exception when re-authentication fails

[jbertram]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 1a149b9  ARTEMIS-3102 throw more accurate exception when 
re-authentication fails
     new a9566e7  This closes #3444
1a149b9 is described below

commit 1a149b9af4faff0c41322efe349de61d2475ba9e
Author: Justin Bertram <jbert...@apache.org>
AuthorDate: Tue Feb 9 11:00:26 2021 -0600

    ARTEMIS-3102 throw more accurate exception when re-authentication fails
---
 .../core/security/impl/SecurityStoreImpl.java      | 60 ++++++++++++++--------
 1 file changed, 38 insertions(+), 22 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 a49745a..93f7fe0 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
@@ -188,28 +188,7 @@ public class SecurityStoreImpl implements SecurityStore, 
HierarchicalRepositoryC
 
          // authentication failed, send a notification & throw an exception
          if (!userIsValid && validatedUser == null) {
-            String certSubjectDN = getCertSubjectDN(connection);
-
-            if (notificationService != null) {
-               TypedProperties props = new TypedProperties();
-               props.putSimpleStringProperty(ManagementHelper.HDR_USER, 
SimpleString.toSimpleString(user));
-               
props.putSimpleStringProperty(ManagementHelper.HDR_CERT_SUBJECT_DN, 
SimpleString.toSimpleString(certSubjectDN));
-               
props.putSimpleStringProperty(ManagementHelper.HDR_REMOTE_ADDRESS, 
SimpleString.toSimpleString(connection == null ? "null" : 
connection.getRemoteAddress()));
-
-               Notification notification = new Notification(null, 
CoreNotificationType.SECURITY_AUTHENTICATION_VIOLATION, props);
-
-               notificationService.sendNotification(notification);
-            }
-
-            Exception e = 
ActiveMQMessageBundle.BUNDLE.unableToValidateUser(connection == null ? "null" : 
connection.getRemoteAddress(), user, certSubjectDN);
-
-            
ActiveMQServerLogger.LOGGER.securityProblemWhileAuthenticating(e.getMessage());
-
-            if (AuditLogger.isResourceLoggingEnabled()) {
-               AuditLogger.userFailedLoggedInAudit(subject, e.getMessage());
-            }
-
-            throw e;
+            authenticationFailed(user, connection);
          }
 
          if (AuditLogger.isAnyLoggingEnabled() && connection != null) {
@@ -280,6 +259,18 @@ public class SecurityStoreImpl implements SecurityStore, 
HierarchicalRepositoryC
          final Boolean validated;
          if (securityManager instanceof ActiveMQSecurityManager5) {
             Subject subject = getSubjectForAuthorization(session, 
((ActiveMQSecurityManager5) securityManager));
+
+            /**
+             * A user may authenticate successfully at first, but then later 
when their Subject is evicted from the
+             * local cache re-authentication may fail. This could happen, for 
example, if the user was removed
+             * from LDAP or the user's token expired.
+             *
+             * If the subject is null then authorization will *always* fail.
+             */
+            if (subject == null) {
+               authenticationFailed(user, session.getRemotingConnection());
+            }
+
             validated = ((ActiveMQSecurityManager5) 
securityManager).authorize(subject, roles, checkType, fqqn != null ? 
fqqn.toString() : bareAddress.toString());
          } else if (securityManager instanceof ActiveMQSecurityManager4) {
             validated = ((ActiveMQSecurityManager4) 
securityManager).validateUserAndRole(user, session.getPassword(), roles, 
checkType, bareAddress.toString(), session.getRemotingConnection(), 
session.getSecurityDomain()) != null;
@@ -365,6 +356,31 @@ public class SecurityStoreImpl implements SecurityStore, 
HierarchicalRepositoryC
       return null;
    }
 
+   private void authenticationFailed(String user, RemotingConnection 
connection) throws Exception {
+      String certSubjectDN = getCertSubjectDN(connection);
+
+      if (notificationService != null) {
+         TypedProperties props = new TypedProperties();
+         props.putSimpleStringProperty(ManagementHelper.HDR_USER, 
SimpleString.toSimpleString(user));
+         props.putSimpleStringProperty(ManagementHelper.HDR_CERT_SUBJECT_DN, 
SimpleString.toSimpleString(certSubjectDN));
+         props.putSimpleStringProperty(ManagementHelper.HDR_REMOTE_ADDRESS, 
SimpleString.toSimpleString(connection == null ? "null" : 
connection.getRemoteAddress()));
+
+         Notification notification = new Notification(null, 
CoreNotificationType.SECURITY_AUTHENTICATION_VIOLATION, props);
+
+         notificationService.sendNotification(notification);
+      }
+
+      Exception e = 
ActiveMQMessageBundle.BUNDLE.unableToValidateUser(connection == null ? "null" : 
connection.getRemoteAddress(), user, certSubjectDN);
+
+      
ActiveMQServerLogger.LOGGER.securityProblemWhileAuthenticating(e.getMessage());
+
+      if (AuditLogger.isResourceLoggingEnabled()) {
+         AuditLogger.userFailedLoggedInAudit(null, e.getMessage());
+      }
+
+      throw e;
+   }
+
    /**
     * Get the cached Subject. If the Subject is not in the cache then 
authenticate again to retrieve it.
     *