[GitHub] [activemq-artemis] ryeats commented on a change in pull request #3281: ARTEMIS-1884 add plugin API for message level authorization policies

[GitBox <gi...@apache.org>]

ryeats commented on a change in pull request #3281:
URL: https://github.com/apache/activemq-artemis/pull/3281#discussion_r498439577



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2444,6 +2446,25 @@ public void callBrokerMessagePlugins(final ActiveMQPluginRunnable<ActiveMQServer
       callBrokerPlugins(getBrokerMessagePlugins(), pluginRun);
    }
 
+   @Override
+   public boolean callBrokerMessagePluginsCanAccept(ServerConsumer serverConsumer, MessageReference messageReference) throws ActiveMQException {
+      for (ActiveMQServerMessagePlugin plugin : getBrokerMessagePlugins()) {

Review comment:
       I am probably not following what you are getting at. If no MessagePlugins are present or if all the plugins are using default canAccept implementation this returns true and no messages are filtered out. I am making an assumption that the results of all MessagePlugin#canAccept calls should be combined as an AND though so if any single plugin returns false the message will not be delivered to that consumer.
   

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
##########
@@ -339,6 +339,22 @@ public static String getUserFromSubject(Subject subject) {
       return validatedUser;
    }
 
+   /**
+    * Get the cached Subject. If the Subject is not in the cache then authenticate again to retrieve
+    * it.
+    *
+    * @param session contains the authentication data
+    * @return the authenticated Subject with all associated role principals or null if not
+    * authenticated or JAAS is not supported by the SecurityManager.
+    */
+   @Override
+   public Subject getSessionSubject(SecurityAuth session) {
+      if (securityManager instanceof ActiveMQSecurityManager5) {

Review comment:
       ActiveMQSecurityManager5 is the only ActiveMQSecurityManager API that exposes returning a users subject the other APIs abstracts away the details of JAAS to a greater extent. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org