diff options
Diffstat (limited to 'qpid/java/broker-plugins/access-control')
8 files changed, 212 insertions, 44 deletions
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java index a684e52ce4..f04dd38aca 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/AbstractConfiguration.java @@ -27,10 +27,10 @@ import org.apache.log4j.Logger; public abstract class AbstractConfiguration implements ConfigurationFile { - protected static final Logger _logger = Logger.getLogger(ConfigurationFile.class); + private static final Logger _logger = Logger.getLogger(ConfigurationFile.class); - protected File _file; - protected RuleSet _config; + private File _file; + private RuleSet _config; public AbstractConfiguration(File file) { diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java index fdbd96e63e..b887d1e079 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/Action.java @@ -21,8 +21,6 @@ package org.apache.qpid.server.security.access.config; import java.util.Comparator; -import java.util.HashMap; -import java.util.Map; import org.apache.commons.lang.builder.EqualsBuilder; import org.apache.commons.lang.builder.HashCodeBuilder; @@ -35,7 +33,7 @@ import org.apache.qpid.server.security.access.Operation; /** * An access control v2 rule action. * - * An action consists of an {@link Operation} on an {@link ObjectType} with certain properties, stored in a {@link Map}. + * An action consists of an {@link Operation} on an {@link ObjectType} with certain properties, stored in a {@link java.util.Map}. * The operation and object should be an allowable combination, based on the {@link ObjectType#isAllowed(Operation)} * method of the object, which is exposed as the {@link #isAllowed()} method here. The internal {@link #propertiesMatch(Map)} * and {@link #valueMatches(String, String)} methods are used to determine wildcarded matching of properties, with @@ -111,10 +109,9 @@ public class Action /** @see Comparable#compareTo(Object) */ public boolean matches(Action a) { - return (Operation.ALL == a.getOperation() - || (getOperation() == a.getOperation() - && getObjectType() == a.getObjectType() - && _properties.matches(a.getProperties()))); + return ((Operation.ALL == a.getOperation() || getOperation() == a.getOperation()) + && (ObjectType.ALL == a.getObjectType() || getObjectType() == a.getObjectType()) + && _properties.matches(a.getProperties())); } /** @@ -136,12 +133,6 @@ public class Action } else if (a.getOperation() == b.getOperation()) { - // Same operator, compare rest of action - -// || (getOperation() == a.getOperation() -// && getObjectType() == a.getObjectType() -// && _properties.matches(a.getProperties()))); - return 1; // b is more specific } else // Different operations diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java index 9f2168a31c..b299b62538 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/PlainConfiguration.java @@ -78,7 +78,7 @@ public class PlainConfiguration extends AbstractConfiguration try { - _st = new StreamTokenizer(new BufferedReader(new FileReader(_file))); + _st = new StreamTokenizer(new BufferedReader(new FileReader(getFile()))); _st.resetSyntax(); // setup the tokenizer _st.commentChar(COMMENT); // single line comments diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java index 402b991419..815df99f80 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/config/RuleSet.java @@ -312,9 +312,9 @@ public class RuleSet return true; } - // C++ broker authorise function prototype + // CPP broker authorise function prototype // virtual bool authorise(const std::string& id, const Action& action, const ObjectType& objType, - // const std::string& name, std::map<Property, std::string>* params=0); + // const std::string& name, std::map<Property, std::string>* params=0) // Possibly add a String name paramater? diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java index a97b66a287..d8a5bd4085 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java @@ -109,7 +109,7 @@ public class AccessControl extends AbstractPlugin { super.configure(config); - AccessControlConfiguration accessConfig = (AccessControlConfiguration) _config; + AccessControlConfiguration accessConfig = (AccessControlConfiguration) getConfig(); _ruleSet = accessConfig.getRuleSet(); } diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java index 72eac7dbe6..7c83446cf1 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java @@ -23,10 +23,9 @@ package org.apache.qpid.server.security.access.plugins; import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory; import org.apache.qpid.server.security.SecurityPluginActivator; import org.apache.qpid.server.security.SecurityPluginFactory; -import org.osgi.framework.BundleActivator; /** - * The OSGi {@link BundleActivator} for {@link AccessControl}. + * The OSGi {@link org.osgi.framework.BundleActivator} for {@link AccessControl}. */ public class AccessControlActivator extends SecurityPluginActivator { diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java index b5c89910a6..c4db6db820 100644 --- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java +++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java @@ -58,7 +58,7 @@ public class AccessControlConfiguration extends ConfigurationPlugin public String getFileName() { - return _configuration.getString(""); + return getConfig().getString(""); } public void validateConfiguration() throws ConfigurationException diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java index 09d26e5451..61e867f459 100644 --- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java +++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java @@ -24,6 +24,7 @@ import java.util.Arrays; import junit.framework.TestCase; +import org.apache.commons.configuration.ConfigurationException; import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin; import org.apache.qpid.server.logging.UnitTestMessageLogger; import org.apache.qpid.server.logging.actors.CurrentActor; @@ -52,10 +53,20 @@ public class AccessControlTest extends TestCase private AccessControl _plugin = null; // Class under test private final UnitTestMessageLogger messageLogger = new UnitTestMessageLogger(); - protected void setUp() throws Exception + private void setUpGroupAccessControl() throws ConfigurationException { - super.setUp(); + configureAccessControl(createGroupRuleSet()); + } + + private void configureAccessControl(final RuleSet rs) throws ConfigurationException + { + _plugin = (AccessControl) AccessControl.FACTORY.newInstance(createConfiguration(rs)); + SecurityManager.setThreadSubject(null); + CurrentActor.set(new TestLogActor(messageLogger)); + } + private RuleSet createGroupRuleSet() + { final RuleSet rs = new RuleSet(); rs.addGroup("aclGroup1", Arrays.asList(new String[] {"member1", "member2"})); @@ -68,11 +79,7 @@ public class AccessControlTest extends TestCase // Catch all rule rs.grant(3, Rule.ALL, Permission.DENY_LOG, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); - _plugin = (AccessControl) AccessControl.FACTORY.newInstance(createConfiguration(rs)); - - SecurityManager.setThreadSubject(null); - - CurrentActor.set(new TestLogActor(messageLogger)); + return rs; } protected void tearDown() throws Exception @@ -81,68 +88,238 @@ public class AccessControlTest extends TestCase SecurityManager.setThreadSubject(null); } - /** + /** * ACL plugin must always abstain if there is no subject attached to the thread. */ - public void testNoSubjectAlwaysAbstains() + public void testNoSubjectAlwaysAbstains() throws ConfigurationException { + setUpGroupAccessControl(); SecurityManager.setThreadSubject(null); final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.ABSTAIN, result); } - /** + /** * Tests that an allow rule expressed with a username allows an operation performed by a thread running * with the same username. */ - public void testUsernameAllowsOperation() + public void testUsernameAllowsOperation() throws ConfigurationException { + setUpGroupAccessControl(); SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user1")); final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.ALLOWED, result); } - /** + /** * Tests that an allow rule expressed with an <b>ACL groupname</b> allows an operation performed by a thread running * by a user who belongs to the same group.. */ - public void testAclGroupMembershipAllowsOperation() + public void testAclGroupMembershipAllowsOperation() throws ConfigurationException { + setUpGroupAccessControl(); SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("member1")); final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.ALLOWED, result); } - /** + /** * Tests that a deny rule expressed with an <b>External groupname</b> denies an operation performed by a thread running * by a user who belongs to the same group. */ - public void testExternalGroupMembershipDeniesOperation() + public void testExternalGroupMembershipDeniesOperation() throws ConfigurationException { + setUpGroupAccessControl(); SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user3", "extGroup1")); - + final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.DENIED, result); } - /** + /** * Tests that the catch all deny denies the operation and logs with the logging actor. */ - public void testCatchAllRuleDeniesUnrecognisedUsername() + public void testCatchAllRuleDeniesUnrecognisedUsername() throws ConfigurationException { + setUpGroupAccessControl(); SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("unknown", "unkgroup1", "unkgroup2")); - + assertEquals("Expecting zero messages before test", 0, messageLogger.getLogMessages().size()); final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY); assertEquals(Result.DENIED, result); - + assertEquals("Expecting one message before test", 1, messageLogger.getLogMessages().size()); assertTrue("Logged message does not contain expected string", messageLogger.messageContains(0, "ACL-1002")); } - + + /** + * Tests that a grant access method rule allows any access operation to be performed on any component + */ + public void testAuthoriseAccessMethodWhenAllAccessOperationsAllowedOnAllComponents() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user4 access right on any method in any component + rs.grant(1, "user4", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, new ObjectProperties(ObjectProperties.STAR)); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user4")); + + ObjectProperties actionProperties = new ObjectProperties("getName"); + actionProperties.put(ObjectProperties.Property.COMPONENT, "Test"); + + final Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, actionProperties); + assertEquals(Result.ALLOWED, result); + } + + /** + * Tests that a grant access method rule allows any access operation to be performed on a specified component + */ + public void testAuthoriseAccessMethodWhenAllAccessOperationsAllowedOnSpecifiedComponent() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user5 access right on any methods in "Test" component + ObjectProperties ruleProperties = new ObjectProperties(ObjectProperties.STAR); + ruleProperties.put(ObjectProperties.Property.COMPONENT, "Test"); + rs.grant(1, "user5", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, ruleProperties); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user5")); + + ObjectProperties actionProperties = new ObjectProperties("getName"); + actionProperties.put(ObjectProperties.Property.COMPONENT, "Test"); + Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, actionProperties); + assertEquals(Result.ALLOWED, result); + + actionProperties.put(ObjectProperties.Property.COMPONENT, "Test2"); + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, actionProperties); + assertEquals(Result.DEFER, result); + } + + /** + * Tests that a grant access method rule allows any access operation to be performed on a specified component + */ + public void testAuthoriseAccessMethodWhenSpecifiedAccessOperationsAllowedOnSpecifiedComponent() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user6 access right on "getAttribute" method in "Test" component + ObjectProperties ruleProperties = new ObjectProperties("getAttribute"); + ruleProperties.put(ObjectProperties.Property.COMPONENT, "Test"); + rs.grant(1, "user6", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, ruleProperties); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user6")); + + ObjectProperties properties = new ObjectProperties("getAttribute"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + properties.put(ObjectProperties.Property.COMPONENT, "Test2"); + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.DEFER, result); + + properties = new ObjectProperties("getAttribute2"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.DEFER, result); + } + + /** + * Tests that granting of all method rights on a method allows a specified operation to be performed on any component + */ + public void testAuthoriseAccessUpdateMethodWhenAllRightsGrantedOnSpecifiedMethodForAllComponents() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user8 all rights on method queryNames in all component + rs.grant(1, "user8", Permission.ALLOW, Operation.ALL, ObjectType.METHOD, new ObjectProperties("queryNames")); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user8")); + + ObjectProperties properties = new ObjectProperties(); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + properties.put(ObjectProperties.Property.NAME, "queryNames"); + + Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + result = _plugin.authorise(Operation.UPDATE, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + properties = new ObjectProperties("getAttribute"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + result = _plugin.authorise(Operation.UPDATE, ObjectType.METHOD, properties); + assertEquals(Result.DEFER, result); + + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.DEFER, result); + } + + /** + * Tests that granting of all method rights allows any operation to be performed on any component + */ + public void testAuthoriseAccessUpdateMethodWhenAllRightsGrantedOnAllMethodsInAllComponents() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user9 all rights on any method in all component + rs.grant(1, "user9", Permission.ALLOW, Operation.ALL, ObjectType.METHOD, new ObjectProperties()); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user9")); + + ObjectProperties properties = new ObjectProperties("queryNames"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + + Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + result = _plugin.authorise(Operation.UPDATE, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + properties = new ObjectProperties("getAttribute"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + result = _plugin.authorise(Operation.UPDATE, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + } + + /** + * Tests that granting of access method rights with mask allows matching operations to be performed on the specified component + */ + public void testAuthoriseAccessMethodWhenMatchingAcessOperationsAllowedOnSpecifiedComponent() throws ConfigurationException + { + final RuleSet rs = new RuleSet(); + + // grant user9 all rights on "getAttribute*" methods in Test component + ObjectProperties ruleProperties = new ObjectProperties(); + ruleProperties.put(ObjectProperties.Property.COMPONENT, "Test"); + ruleProperties.put(ObjectProperties.Property.NAME, "getAttribute*"); + + rs.grant(1, "user9", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, ruleProperties); + configureAccessControl(rs); + SecurityManager.setThreadSubject(TestPrincipalUtils.createTestSubject("user9")); + + ObjectProperties properties = new ObjectProperties("getAttributes"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + Result result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + properties = new ObjectProperties("getAttribute"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.ALLOWED, result); + + properties = new ObjectProperties("getAttribut"); + properties.put(ObjectProperties.Property.COMPONENT, "Test"); + result = _plugin.authorise(Operation.ACCESS, ObjectType.METHOD, properties); + assertEquals(Result.DEFER, result); + } + /** * Creates a configuration plugin for the {@link AccessControl} plugin. */ @@ -150,6 +327,7 @@ public class AccessControlTest extends TestCase { final ConfigurationPlugin cp = new ConfigurationPlugin() { + @SuppressWarnings("unchecked") public AccessControlConfiguration getConfiguration(final String plugin) { return new AccessControlConfiguration() |