From b83ba1c0bc269d2b9dddd73dfe8a90d4d3723e58 Mon Sep 17 00:00:00 2001 From: Robert Gemmell Date: Thu, 9 Jun 2011 10:38:20 +0000 Subject: QPID-3296: update RMIPasswordAuthenticator to authenticate users via AuthenticationManager instead of a PrincipalDatabase Applied patch from Keith Wall git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1133781 13f79535-47bb-0310-9956-ffa450edef68 --- .../configuration/ServerConfigurationTest.java | 56 +++-- ...PrincipalDatabaseAuthenticationManagerTest.java | 47 +++- .../auth/rmi/RMIPasswordAuthenticatorTest.java | 264 ++++++++++----------- .../security/auth/sasl/UsernamePrincipalTest.java | 124 ++++++++++ 4 files changed, 316 insertions(+), 175 deletions(-) create mode 100644 qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/UsernamePrincipalTest.java (limited to 'qpid/java/broker/src/test') diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java index 43540c88a1..494003c8a0 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java @@ -23,24 +23,18 @@ package org.apache.qpid.server.configuration; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.io.RandomAccessFile; import java.util.List; import java.util.Locale; -import junit.framework.TestCase; - import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.XMLConfiguration; import org.apache.qpid.framing.AMQShortString; import org.apache.qpid.server.exchange.Exchange; -import org.apache.qpid.server.protocol.AMQProtocolEngine; -import org.apache.qpid.server.protocol.AMQProtocolSession; import org.apache.qpid.server.registry.ApplicationRegistry; import org.apache.qpid.server.registry.ConfigurationFileApplicationRegistry; import org.apache.qpid.server.util.InternalBrokerBaseCase; import org.apache.qpid.server.virtualhost.VirtualHost; import org.apache.qpid.server.virtualhost.VirtualHostRegistry; -import org.apache.qpid.transport.TestNetworkDriver; public class ServerConfigurationTest extends InternalBrokerBaseCase { @@ -320,20 +314,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase assertEquals(true, serverConfig.getMsgAuth()); } - public void testGetJMXPrincipalDatabase() throws ConfigurationException - { - // Check default - ServerConfiguration serverConfig = new ServerConfiguration(_config); - serverConfig.initialise(); - assertEquals(null, serverConfig.getJMXPrincipalDatabase()); - - // Check value we set - _config.setProperty("security.jmx.principal-database", "a"); - serverConfig = new ServerConfiguration(_config); - serverConfig.initialise(); - assertEquals("a", serverConfig.getJMXPrincipalDatabase()); - } - public void testGetManagementKeyStorePath() throws ConfigurationException { // Check default @@ -831,9 +811,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t\t\n"); out.write("\t\t\t\n"); out.write("\t\t\n"); - out.write("\t\t\n"); - out.write("\t\t\tpasswordfile\n"); - out.write("\t\t\n"); out.write("\t\t\n"); out.write("\t\t\t"); out.write("\t\t\n"); @@ -881,9 +858,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t\t\n"); out.write("\t\t\t\n"); out.write("\t\t\n"); - out.write("\t\t\n"); - out.write("\t\t\tpasswordfile\n"); - out.write("\t\t\n"); out.write("\t\t\n"); out.write("\t\t\t"); out.write("\t\t\n"); @@ -986,9 +960,6 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase out.write("\t\t\t\t\n"); out.write("\t\t\t\n"); out.write("\t\t\n"); - out.write("\t\t\n"); - out.write("\t\t\tpasswordfile\n"); - out.write("\t\t\n"); out.write("\t\t\n"); out.write("\t\t\t"); out.write("\t\t\n"); @@ -1489,4 +1460,31 @@ public class ServerConfigurationTest extends InternalBrokerBaseCase ce.getMessage()); } } + + /* + * Tests that the old element security.jmx.principal-databases (that used to define the + * principal database used for JMX authentication) is rejected. + */ + public void testManagementPrincipalDatabaseRejected() throws ConfigurationException + { + // Check default + ServerConfiguration serverConfig = new ServerConfiguration(_config); + serverConfig.initialise(); + + // Check value we set + _config.setProperty("security.jmx.principal-database(0)", "mydb"); + serverConfig = new ServerConfiguration(_config); + + try + { + serverConfig.initialise(); + fail("Exception not thrown"); + } + catch (ConfigurationException ce) + { + assertEquals("Incorrect error message", + "Validation error : security/jmx/principal-database is no longer a supported element within the configuration xml.", + ce.getMessage()); + } + } } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java index f51ce0b6c6..5a9026cb64 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManagerTest.java @@ -23,11 +23,13 @@ package org.apache.qpid.server.security.auth.manager; import java.security.Provider; import java.security.Security; +import javax.security.auth.Subject; import javax.security.sasl.SaslException; import javax.security.sasl.SaslServer; import org.apache.qpid.server.security.auth.AuthenticationResult; import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus; +import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal; import org.apache.qpid.server.util.InternalBrokerBaseCase; /** @@ -89,17 +91,18 @@ public class PrincipalDatabaseAuthenticationManagerTest extends InternalBrokerBa } /** - * * Tests that the authenticate method correctly interprets an * authentication success. * */ - public void testAuthenticationSuccess() throws Exception + public void testSaslAuthenticationSuccess() throws Exception { SaslServer testServer = createTestSaslServer(true, false); AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); - assertEquals(AuthenticationStatus.SUCCESS, result.status); + final Subject subject = result.getSubject(); + assertTrue(subject.getPrincipals().contains(new UsernamePrincipal("guest"))); + assertEquals(AuthenticationStatus.SUCCESS, result.getStatus()); } /** @@ -108,12 +111,13 @@ public class PrincipalDatabaseAuthenticationManagerTest extends InternalBrokerBa * authentication not complete. * */ - public void testAuthenticationNotCompleted() throws Exception + public void testSaslAuthenticationNotCompleted() throws Exception { SaslServer testServer = createTestSaslServer(false, false); AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); - assertEquals(AuthenticationStatus.CONTINUE, result.status); + assertNull(result.getSubject()); + assertEquals(AuthenticationStatus.CONTINUE, result.getStatus()); } /** @@ -122,12 +126,39 @@ public class PrincipalDatabaseAuthenticationManagerTest extends InternalBrokerBa * authentication error. * */ - public void testAuthenticationError() throws Exception + public void testSaslAuthenticationError() throws Exception { SaslServer testServer = createTestSaslServer(false, true); AuthenticationResult result = _manager.authenticate(testServer, "12345".getBytes()); - assertEquals(AuthenticationStatus.ERROR, result.status); + assertNull(result.getSubject()); + assertEquals(AuthenticationStatus.ERROR, result.getStatus()); + } + + /** + * Tests that the authenticate method correctly interprets an + * authentication success. + * + */ + public void testNonSaslAuthenticationSuccess() throws Exception + { + AuthenticationResult result = _manager.authenticate("guest", "guest"); + final Subject subject = result.getSubject(); + assertFalse("Subject should not be set read-only", subject.isReadOnly()); + assertTrue(subject.getPrincipals().contains(new UsernamePrincipal("guest"))); + assertEquals(AuthenticationStatus.SUCCESS, result.getStatus()); + } + + /** + * Tests that the authenticate method correctly interprets an + * authentication success. + * + */ + public void testNonSaslAuthenticationNotCompleted() throws Exception + { + AuthenticationResult result = _manager.authenticate("guest", "wrongpassword"); + assertNull(result.getSubject()); + assertEquals(AuthenticationStatus.CONTINUE, result.getStatus()); } /** @@ -179,7 +210,7 @@ public class PrincipalDatabaseAuthenticationManagerTest extends InternalBrokerBa @Override public String getAuthorizationID() { - return null; + return complete ? "guest" : null; } @Override diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java index e8c24da68d..eb713e3712 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticatorTest.java @@ -20,188 +20,124 @@ */ package org.apache.qpid.server.security.auth.rmi; -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; import java.util.Collections; import javax.management.remote.JMXPrincipal; import javax.security.auth.Subject; - -import org.apache.qpid.server.security.auth.database.Base64MD5PasswordFilePrincipalDatabase; -import org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase; +import javax.security.sasl.SaslException; +import javax.security.sasl.SaslServer; import junit.framework.TestCase; +import org.apache.qpid.server.security.auth.AuthenticationResult; +import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus; +import org.apache.qpid.server.security.auth.manager.AuthenticationManager; + +/** + * Tests the RMIPasswordAuthenticator and its collaboration with the AuthenticationManager. + * + */ public class RMIPasswordAuthenticatorTest extends TestCase { private final String USERNAME = "guest"; private final String PASSWORD = "guest"; - private final String B64_MD5HASHED_PASSWORD = "CE4DQ6BIb/BVMN9scFyLtA=="; private RMIPasswordAuthenticator _rmipa; - - private Base64MD5PasswordFilePrincipalDatabase _md5Pd; - private File _md5PwdFile; - - private PlainPasswordFilePrincipalDatabase _plainPd; - private File _plainPwdFile; - - private Subject testSubject; + private String[] _credentials; protected void setUp() throws Exception { _rmipa = new RMIPasswordAuthenticator(); - _md5Pd = new Base64MD5PasswordFilePrincipalDatabase(); - _md5PwdFile = createTempPasswordFile(this.getClass().getName()+"md5pwd", USERNAME, B64_MD5HASHED_PASSWORD); - _md5Pd.setPasswordFile(_md5PwdFile.getAbsolutePath()); - - _plainPd = new PlainPasswordFilePrincipalDatabase(); - _plainPwdFile = createTempPasswordFile(this.getClass().getName()+"plainpwd", USERNAME, PASSWORD); - _plainPd.setPasswordFile(_plainPwdFile.getAbsolutePath()); - - testSubject = new Subject(true, + _credentials = new String[] {USERNAME, PASSWORD}; + } + + /** + * Tests a successful authentication. Ensures that a populated read-only subject it returned. + */ + public void testAuthenticationSuccess() + { + final Subject expectedSubject = new Subject(true, Collections.singleton(new JMXPrincipal(USERNAME)), Collections.EMPTY_SET, Collections.EMPTY_SET); - } - - private File createTempPasswordFile(String filenamePrefix, String user, String password) - { - try - { - File testFile = File.createTempFile(filenamePrefix,"tmp"); - testFile.deleteOnExit(); - BufferedWriter writer = new BufferedWriter(new FileWriter(testFile)); + _rmipa.setAuthenticationManager(createTestAuthenticationManager(true, null)); - writer.write(user + ":" + password); - writer.newLine(); - writer.flush(); - writer.close(); - - return testFile; - } - catch (IOException e) - { - fail("Unable to create temporary test password file." + e.getMessage()); - } + Subject newSubject = _rmipa.authenticate(_credentials); + assertTrue("Subject must be readonly", newSubject.isReadOnly()); + assertTrue("Returned subject does not equal expected value", + newSubject.equals(expectedSubject)); - return null; } - - - //********** Test Methods *********// - - public void testAuthenticate() + /** + * Tests a unsuccessful authentication. + */ + public void testUsernameOrPasswordInvalid() { - String[] credentials; - Subject newSubject; - - // Test when no PD has been set - try - { - credentials = new String[]{USERNAME, PASSWORD}; - newSubject = _rmipa.authenticate(credentials); - fail("SecurityException expected due to lack of principal database"); - } - catch (SecurityException se) - { - assertEquals("Unexpected exception message", - RMIPasswordAuthenticator.UNABLE_TO_LOOKUP, se.getMessage()); - } - - //The PrincipalDatabase's are tested primarily by their own tests, but - //minimal tests are done here to exercise their usage in this area. + _rmipa.setAuthenticationManager(createTestAuthenticationManager(false, null)); - // Test correct passwords are verified with an MD5 PD try { - _rmipa.setPrincipalDatabase(_md5Pd); - credentials = new String[]{USERNAME, PASSWORD}; - newSubject = _rmipa.authenticate(credentials); - assertTrue("Returned subject does not equal expected value", - newSubject.equals(testSubject)); - } - catch (Exception e) - { - fail("Unexpected Exception:" + e.getMessage()); - } - - // Test incorrect passwords are not verified with an MD5 PD - try - { - credentials = new String[]{USERNAME, PASSWORD+"incorrect"}; - newSubject = _rmipa.authenticate(credentials); - fail("SecurityException expected due to incorrect password"); + _rmipa.authenticate(_credentials); + fail("Exception not thrown"); } catch (SecurityException se) { assertEquals("Unexpected exception message", RMIPasswordAuthenticator.INVALID_CREDENTIALS, se.getMessage()); - } - - // Test non-existent accounts are not verified with an MD5 PD - try - { - credentials = new String[]{USERNAME+"invalid", PASSWORD}; - newSubject = _rmipa.authenticate(credentials); - fail("SecurityException expected due to non-existant account"); - } - catch (SecurityException se) - { - assertEquals("Unexpected exception message", - RMIPasswordAuthenticator.INVALID_CREDENTIALS, se.getMessage()); - } - // Test correct passwords are verified with a Plain PD - try - { - _rmipa.setPrincipalDatabase(_plainPd); - credentials = new String[]{USERNAME, PASSWORD}; - newSubject = _rmipa.authenticate(credentials); - assertTrue("Returned subject does not equal expected value", - newSubject.equals(testSubject)); - } - catch (Exception e) - { - fail("Unexpected Exception"); } + } + + /** + * Tests case where authentication system itself fails. + */ + public void testAuthenticationFailure() + { + final Exception mockAuthException = new Exception("Mock Auth system failure"); + _rmipa.setAuthenticationManager(createTestAuthenticationManager(false, mockAuthException)); - // Test incorrect passwords are not verified with a Plain PD try { - credentials = new String[]{USERNAME, PASSWORD+"incorrect"}; - newSubject = _rmipa.authenticate(credentials); - fail("SecurityException expected due to incorrect password"); + _rmipa.authenticate(_credentials); + fail("Exception not thrown"); } catch (SecurityException se) { - assertEquals("Unexpected exception message", - RMIPasswordAuthenticator.INVALID_CREDENTIALS, se.getMessage()); + assertEquals("Initial cause not found", mockAuthException, se.getCause()); } - - // Test non-existent accounts are not verified with an Plain PD + } + + + /** + * Tests case where authentication manager is not set. + */ + public void testNullAuthenticationManager() + { try { - credentials = new String[]{USERNAME+"invalid", PASSWORD}; - newSubject = _rmipa.authenticate(credentials); - fail("SecurityException expected due to non existant account"); + _rmipa.authenticate(_credentials); + fail("SecurityException expected due to lack of authentication manager"); } catch (SecurityException se) { assertEquals("Unexpected exception message", - RMIPasswordAuthenticator.INVALID_CREDENTIALS, se.getMessage()); + RMIPasswordAuthenticator.UNABLE_TO_LOOKUP, se.getMessage()); } + } + /** + * Tests case where arguments are non-Strings.. + */ + public void testWithNonStringArrayArgument() + { // Test handling of non-string credential's + final Object[] objCredentials = new Object[]{USERNAME, PASSWORD}; try { - Object[] objCredentials = new Object[]{USERNAME, PASSWORD}; - newSubject = _rmipa.authenticate(objCredentials); + _rmipa.authenticate(objCredentials); fail("SecurityException expected due to non string[] credentials"); } catch (SecurityException se) @@ -209,12 +145,18 @@ public class RMIPasswordAuthenticatorTest extends TestCase assertEquals("Unexpected exception message", RMIPasswordAuthenticator.SHOULD_BE_STRING_ARRAY, se.getMessage()); } - - // Test handling of incorrect number of credential's + } + + /** + * Tests case where there are too many, too few or null arguments. + */ + public void testWithIllegalNumberOfArguments() + { + // Test handling of incorrect number of credentials try { - credentials = new String[]{USERNAME, PASSWORD, PASSWORD}; - newSubject = _rmipa.authenticate(credentials); + _credentials = new String[]{USERNAME, PASSWORD, PASSWORD}; + _rmipa.authenticate(_credentials); fail("SecurityException expected due to supplying wrong number of credentials"); } catch (SecurityException se) @@ -223,12 +165,12 @@ public class RMIPasswordAuthenticatorTest extends TestCase RMIPasswordAuthenticator.SHOULD_HAVE_2_ELEMENTS, se.getMessage()); } - // Test handling of null credential's + // Test handling of null credentials try { //send a null array - credentials = null; - newSubject = _rmipa.authenticate(credentials); + _credentials = null; + _rmipa.authenticate(_credentials); fail("SecurityException expected due to not supplying an array of credentials"); } catch (SecurityException se) @@ -240,8 +182,8 @@ public class RMIPasswordAuthenticatorTest extends TestCase try { //send a null password - credentials = new String[]{USERNAME, null}; - newSubject = _rmipa.authenticate(credentials); + _credentials = new String[]{USERNAME, null}; + _rmipa.authenticate(_credentials); fail("SecurityException expected due to sending a null password"); } catch (SecurityException se) @@ -253,8 +195,8 @@ public class RMIPasswordAuthenticatorTest extends TestCase try { //send a null username - credentials = new String[]{null, PASSWORD}; - newSubject = _rmipa.authenticate(credentials); + _credentials = new String[]{null, PASSWORD}; + _rmipa.authenticate(_credentials); fail("SecurityException expected due to sending a null username"); } catch (SecurityException se) @@ -264,4 +206,50 @@ public class RMIPasswordAuthenticatorTest extends TestCase } } + private AuthenticationManager createTestAuthenticationManager(final boolean successfulAuth, final Exception exception) + { + return new AuthenticationManager() + { + @Override + public void close() + { + throw new UnsupportedOperationException(); + } + + @Override + public String getMechanisms() + { + throw new UnsupportedOperationException(); + } + + @Override + public SaslServer createSaslServer(String mechanism, String localFQDN) throws SaslException + { + throw new UnsupportedOperationException(); + } + + @Override + public AuthenticationResult authenticate(SaslServer server, byte[] response) + { + throw new UnsupportedOperationException(); + } + + @Override + public AuthenticationResult authenticate(String username, String password) + { + if (exception != null) { + return new AuthenticationResult(AuthenticationStatus.ERROR, exception); + } + else if (successfulAuth) + { + return new AuthenticationResult(new Subject()); + } + else + { + return new AuthenticationResult(AuthenticationStatus.CONTINUE); + } + } + }; + } + } diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/UsernamePrincipalTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/UsernamePrincipalTest.java new file mode 100644 index 0000000000..8bff22115e --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/sasl/UsernamePrincipalTest.java @@ -0,0 +1,124 @@ +/* + * + * 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.qpid.server.security.auth.sasl; + +import java.security.Principal; +import javax.security.auth.Subject; +import junit.framework.TestCase; + +/** + * Tests the UsernamePrincipal. + * + */ +public class UsernamePrincipalTest extends TestCase +{ + public void testEqualitySameObject() + { + final UsernamePrincipal principal = new UsernamePrincipal("string"); + assertTrue(principal.equals(principal)); + } + + public void testEqualitySameName() + { + final String string = "string"; + final UsernamePrincipal principal1 = new UsernamePrincipal(string); + final UsernamePrincipal principal2 = new UsernamePrincipal(string); + assertTrue(principal1.equals(principal2)); + } + + public void testEqualityEqualName() + { + final UsernamePrincipal principal1 = new UsernamePrincipal(new String("string")); + final UsernamePrincipal principal2 = new UsernamePrincipal(new String("string")); + assertTrue(principal1.equals(principal2)); + } + + public void testInequalityDifferentUserPrincipals() + { + UsernamePrincipal principal1 = new UsernamePrincipal("string1"); + UsernamePrincipal principal2 = new UsernamePrincipal("string2"); + assertFalse(principal1.equals(principal2)); + } + + public void testInequalityNonUserPrincipal() + { + UsernamePrincipal principal = new UsernamePrincipal("string"); + assertFalse(principal.equals(new String("string"))); + } + + public void testInequalityNull() + { + UsernamePrincipal principal = new UsernamePrincipal("string"); + assertFalse(principal.equals(null)); + } + + public void testGetUsernamePrincipalFromSubject() + { + final UsernamePrincipal expected = new UsernamePrincipal("name"); + final Principal other = new Principal() + { + + @Override + public String getName() + { + return "otherprincipal"; + } + }; + + final Subject subject = new Subject(); + subject.getPrincipals().add(expected); + subject.getPrincipals().add(other); + + final UsernamePrincipal actual = UsernamePrincipal.getUsernamePrincipalFromSubject(subject); + assertSame(expected, actual); + } + + public void testUsernamePrincipalNotInSubject() + { + try + { + UsernamePrincipal.getUsernamePrincipalFromSubject(new Subject()); + fail("Exception not thrown"); + } + catch (IllegalArgumentException iae) + { + // PASS + } + } + + public void testTooManyUsernamePrincipalInSubject() + { + final Subject subject = new Subject(); + subject.getPrincipals().add(new UsernamePrincipal("name1")); + subject.getPrincipals().add(new UsernamePrincipal("name2")); + try + { + + UsernamePrincipal.getUsernamePrincipalFromSubject(subject); + fail("Exception not thrown"); + } + catch (IllegalArgumentException iae) + { + // PASS + } + } + +} -- cgit v1.2.1