QPID-5567 : Further changes to SecurityMangager

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1570239 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 36 insertions, 45 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
index 75006ae697..f579ea0ec5 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
@@ -113,49 +113,31 @@ public class DefaultAccessControl implements AccessControl
     }
 
     /**
-     * Object instance access authorisation.
-     *
-	 * Delegate to the {@link #authorise(Operation, ObjectType, ObjectProperties)} method, with
-     * the operation set to ACCESS and no object properties.
-	 */
-    public Result access(ObjectType objectType)
-    {
-        InetAddress addressOfClient = null;
-        final Subject subject = Subject.getSubject(AccessController.getContext());
-        if(subject != null)
-        {
-            Set<ConnectionPrincipal> principals = subject.getPrincipals(ConnectionPrincipal.class);
-            if(!principals.isEmpty())
-            {
-                SocketAddress address = principals.iterator().next().getConnection().getRemoteAddress();
-                if(address instanceof InetSocketAddress)
-                {
-                    addressOfClient = ((InetSocketAddress) address).getAddress();
-                }
-            }
-        }
-        return authoriseFromAddress(Operation.ACCESS, objectType, ObjectProperties.EMPTY, addressOfClient);
-    }
-
-    /**
      * Check if an operation is authorised by asking the  configuration object about the access
      * control rules granted to the current thread's {@link Subject}. If there is no current
      * user the plugin will abstain.
      */
     public Result authorise(Operation operation, ObjectType objectType, ObjectProperties properties)
     {
-        return authoriseFromAddress(operation, objectType, properties, null);
-    }
-
-    public Result authoriseFromAddress(Operation operation, ObjectType objectType, ObjectProperties properties, InetAddress addressOfClient)
-    {
+        InetAddress addressOfClient = null;
         final Subject subject = Subject.getSubject(AccessController.getContext());
+
         // Abstain if there is no subject/principal associated with this thread
         if (subject == null  || subject.getPrincipals().size() == 0)
         {
             return Result.ABSTAIN;
         }
 
+        Set<ConnectionPrincipal> principals = subject.getPrincipals(ConnectionPrincipal.class);
+        if(!principals.isEmpty())
+        {
+            SocketAddress address = principals.iterator().next().getConnection().getRemoteAddress();
+            if(address instanceof InetSocketAddress)
+            {
+                addressOfClient = ((InetSocketAddress) address).getAddress();
+            }
+        }
+
         if(_logger.isDebugEnabled())
         {
             _logger.debug("Checking " + operation + " " + objectType + " " + ObjectUtils.defaultIfNull(addressOfClient, ""));
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
index 8ac4e0c424..e907a88001 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
@@ -246,7 +246,7 @@ public class DefaultAccessControlTest extends TestCase
 
                 DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
 
-                accessControl.access(ObjectType.VIRTUALHOST);
+                accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
 
                 verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY, inetAddress);
                 return null;
@@ -282,7 +282,7 @@ public class DefaultAccessControlTest extends TestCase
                         inetAddress)).thenThrow(new RuntimeException());
 
                 DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
-                Result result = accessControl.access(ObjectType.VIRTUALHOST);
+                Result result = accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
 
                 assertEquals(Result.DENIED, result);
                 return null;
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
index e78e59b7fc..f959977d1f 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
@@ -46,6 +46,9 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
 import java.util.Arrays;
 
 /**
@@ -201,7 +204,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         }
     }
 
-    private Object authoriseAndInvoke(Method method, Object[] args) throws IllegalAccessException, InvocationTargetException
+    private Object authoriseAndInvoke(final Method method, final Object[] args) throws Throwable
     {
         String methodName;
         // Get the component, type and impact, which may be null
@@ -229,23 +232,29 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         Operation operation = (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO) ? Operation.ACCESS : Operation.UPDATE;
         security.authoriseMethod(operation, type, methodName);
 
-        boolean oldAccessChecksDisabled = false;
-        if(_managementRightsInferAllAccess)
+        if (_managementRightsInferAllAccess)
         {
-            oldAccessChecksDisabled = SecurityManager.setAccessChecksDisabled(true);
+            try
+            {
+                return Subject.doAs(SecurityManager.SYSTEM, new PrivilegedExceptionAction<Object>()
+                {
+                    @Override
+                    public Object run() throws IllegalAccessException, InvocationTargetException
+                    {
+                        return method.invoke(_mbs, args);
+                    }
+                });
+            }
+            catch (PrivilegedActionException e)
+            {
+                throw e.getCause();
+            }
         }
-
-        try
+        else
         {
             return method.invoke(_mbs, args);
         }
-        finally
-        {
-            if(_managementRightsInferAllAccess)
-            {
-                SecurityManager.setAccessChecksDisabled(oldAccessChecksDisabled);
-            }
-        }
+
     }
 
     private String getType(Method method, Object[] args)