You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2013/10/27 13:57:28 UTC

svn commit: r1536126 - in /qpid/trunk/qpid/java/broker-core/src: main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java

Author: kwall
Date: Sun Oct 27 12:57:28 2013
New Revision: 1536126

URL: http://svn.apache.org/r1536126
Log:
QPID-5240: Change ExternalSaslServer to avoid NPE possibility.

Contract for SaslServer#getAuthorizationID does not disallow the calling of getAuthorizationID following a failed authentication
so returning null in this case seems reasonable (com.sun.security.sasl.CramMD5Server behaves in this way).

Also refactored EAMT to have small tightly targetted tests.

Modified:
    qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java
    qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java

Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java?rev=1536126&r1=1536125&r2=1536126&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/external/ExternalSaslServer.java Sun Oct 27 12:57:28 2013
@@ -36,7 +36,7 @@ public class ExternalSaslServer implemen
 
     private boolean _complete = false;
     private final Principal _externalPrincipal;
-    private boolean _useFullDN = false;
+    private final boolean _useFullDN;
 
     public ExternalSaslServer(Principal externalPrincipal, boolean useFullDN)
     {
@@ -62,7 +62,7 @@ public class ExternalSaslServer implemen
 
     public String getAuthorizationID()
     {
-        return getAuthenticatedPrincipal().getName();
+        return getAuthenticatedPrincipal() == null ? null : getAuthenticatedPrincipal().getName();
     }
 
     public byte[] unwrap(byte[] incoming, int offset, int len) throws SaslException

Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java?rev=1536126&r1=1536125&r2=1536126&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationManagerTest.java Sun Oct 27 12:57:28 2013
@@ -43,141 +43,151 @@ public class ExternalAuthenticationManag
         createSaslServerTestImpl(_manager);
     }
 
-    public void testCreateSaslServerUsingFullDN() throws Exception
+    public void testAuthenticatePrincipalNull_CausesAuthError() throws Exception
     {
-        createSaslServerTestImpl(_managerUsingFullDN);
-    }
-
-    public void createSaslServerTestImpl(AuthenticationManager manager) throws Exception
-    {
-        SaslServer server = manager.createSaslServer("EXTERNAL", "example.example.com", null);
-
-        assertEquals("Sasl Server mechanism name is not as expected", "EXTERNAL", server.getMechanismName());
-
-        try
-        {
-            server = manager.createSaslServer("PLAIN", "example.example.com", null);
-            fail("Expected creating SaslServer with incorrect mechanism to throw an exception");
-        }
-        catch (SaslException e)
-        {
-            // pass
-        }
-    }
-
-    /**
-     * Test behaviour of the authentication when the useFullDN attribute is set true
-     * and the username is taken directly as the externally supplied Principal
-     */
-    public void testAuthenticateWithFullDN() throws Exception
-    {
-        X500Principal principal = new X500Principal("CN=person, DC=example, DC=com");
-        SaslServer saslServer = _managerUsingFullDN.createSaslServer("EXTERNAL", "example.example.com", principal);
-
-        AuthenticationResult result = _managerUsingFullDN.authenticate(saslServer, new byte[0]);
-        assertNotNull(result);
-        assertEquals("Expected authentication to be successful",
-                     AuthenticationResult.AuthenticationStatus.SUCCESS,
-                     result.getStatus());
-
-        assertOnlyContainsWrapped(principal, result.getPrincipals());
-
-        saslServer = _managerUsingFullDN.createSaslServer("EXTERNAL", "example.example.com", null);
-        result = _managerUsingFullDN.authenticate(saslServer, new byte[0]);
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", null);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
 
         assertNotNull(result);
         assertEquals("Expected authentication to be unsuccessful",
-                     AuthenticationResult.AuthenticationStatus.ERROR,
-                     result.getStatus());
+                AuthenticationResult.AuthenticationStatus.ERROR,
+                result.getStatus());
+        assertNull(saslServer.getAuthorizationID());
     }
 
-    /**
-     * Test behaviour of the authentication when parsing the username from
-     * the Principals DN as <CN>@<DC1>.<DC2>.<DC3>....<DCN>
-     */
-    public void testAuthenticateWithUsernameBasedOnCNAndDC() throws Exception
-    {
-        X500Principal principal;
-        SaslServer saslServer;
-        AuthenticationResult result;
-        UsernamePrincipal expectedPrincipal;
-
-        // DN contains only CN
-        principal = new X500Principal("CN=person");
-        expectedPrincipal = new UsernamePrincipal("person");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
-
-        result = _manager.authenticate(saslServer, new byte[0]);
-        assertNotNull(result);
-        assertEquals("Expected authentication to be successful",
-                     AuthenticationResult.AuthenticationStatus.SUCCESS,
-                     result.getStatus());
-        assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals());
-
-        // Null principal
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", null);
-        result = _manager.authenticate(saslServer, new byte[0]);
+    public void testAuthenticatePrincipalNoCn_CausesAuthError() throws Exception
+    {
+        X500Principal principal = new X500Principal("DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
 
         assertNotNull(result);
         assertEquals("Expected authentication to be unsuccessful",
                 AuthenticationResult.AuthenticationStatus.ERROR,
                 result.getStatus());
+        assertNull(saslServer.getAuthorizationID());
+    }
 
-        // DN doesn't contain CN
-        principal = new X500Principal("DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
-        result = _manager.authenticate(saslServer, new byte[0]);
+    public void testAuthenticatePrincipalEmptyCn_CausesAuthError() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=, DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
 
         assertNotNull(result);
         assertEquals("Expected authentication to be unsuccessful",
                 AuthenticationResult.AuthenticationStatus.ERROR,
                 result.getStatus());
+        assertNull(saslServer.getAuthorizationID());
+    }
 
-        // DN contains empty CN
-        principal = new X500Principal("CN=, DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
-        result = _manager.authenticate(saslServer, new byte[0]);
+    public void testAuthenticatePrincipalCnOnly() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=person");
+        UsernamePrincipal expectedPrincipal = new UsernamePrincipal("person");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
 
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
         assertNotNull(result);
-        assertEquals("Expected authentication to be unsuccessful",
-                AuthenticationResult.AuthenticationStatus.ERROR,
-                result.getStatus());
+        assertEquals("Expected authentication to be successful",
+                     AuthenticationResult.AuthenticationStatus.SUCCESS,
+                     result.getStatus());
+        assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals());
+        assertEquals("person", saslServer.getAuthorizationID());
+    }
 
-        // DN contains CN and DC
-        principal = new X500Principal("CN=person, DC=example, DC=com");
-        expectedPrincipal = new UsernamePrincipal("person@example.com");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
+    public void testAuthenticatePrinicpalCnAndDc() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=person, DC=example, DC=com");
+        UsernamePrincipal expectedPrincipal = new UsernamePrincipal("person@example.com");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
 
-        result = _manager.authenticate(saslServer, new byte[0]);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
         assertNotNull(result);
         assertEquals("Expected authentication to be successful",
                 AuthenticationResult.AuthenticationStatus.SUCCESS,
                 result.getStatus());
         assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals());
+        assertEquals("person@example.com", saslServer.getAuthorizationID());
+    }
 
-        // DN contains CN and DC and other components
-        principal = new X500Principal("CN=person, DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
-        expectedPrincipal = new UsernamePrincipal("person@example.com");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
+    public void testAuthenticatePrinicpalCnDc_OtherComponentsIgnored() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=person, DC=example, DC=com, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
+        UsernamePrincipal expectedPrincipal = new UsernamePrincipal("person@example.com");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
 
-        result = _manager.authenticate(saslServer, new byte[0]);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
         assertNotNull(result);
         assertEquals("Expected authentication to be successful",
                 AuthenticationResult.AuthenticationStatus.SUCCESS,
                 result.getStatus());
         assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals());
+        assertEquals("person@example.com", saslServer.getAuthorizationID());
+    }
 
-        // DN contains CN and DC and other components
-        principal = new X500Principal("CN=person, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
-        expectedPrincipal = new UsernamePrincipal("person");
-        saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
+    public void testAuthenticatePrincipalCn_OtherComponentsIgnored() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=person, O=My Company Ltd, L=Newbury, ST=Berkshire, C=GB");
+        UsernamePrincipal expectedPrincipal = new UsernamePrincipal("person");
+        SaslServer saslServer = _manager.createSaslServer("EXTERNAL", "example.example.com", principal);
 
-        result = _manager.authenticate(saslServer, new byte[0]);
+        AuthenticationResult result = _manager.authenticate(saslServer, new byte[0]);
         assertNotNull(result);
         assertEquals("Expected authentication to be successful",
                 AuthenticationResult.AuthenticationStatus.SUCCESS,
                 result.getStatus());
         assertOnlyContainsWrapped(expectedPrincipal, result.getPrincipals());
+        assertEquals("person", saslServer.getAuthorizationID());
+    }
+
+    public void testFullDNMode_CreateSaslServer() throws Exception
+    {
+        createSaslServerTestImpl(_managerUsingFullDN);
+    }
+
+    public void testFullDNMode_Authenticate() throws Exception
+    {
+        X500Principal principal = new X500Principal("CN=person, DC=example, DC=com");
+        SaslServer saslServer = _managerUsingFullDN.createSaslServer("EXTERNAL", "example.example.com", principal);
+
+        AuthenticationResult result = _managerUsingFullDN.authenticate(saslServer, new byte[0]);
+        assertNotNull(result);
+        assertEquals("Expected authentication to be successful",
+                     AuthenticationResult.AuthenticationStatus.SUCCESS,
+                     result.getStatus());
+
+        assertOnlyContainsWrapped(principal, result.getPrincipals());
+        assertEquals("CN=person,DC=example,DC=com", saslServer.getAuthorizationID());
+    }
+
+    public void testFullDNMode_AuthenticatePrincipalNull_CausesAuthError() throws Exception
+    {
+        SaslServer saslServer = _managerUsingFullDN.createSaslServer("EXTERNAL", "example.example.com", null);
+        AuthenticationResult result = _managerUsingFullDN.authenticate(saslServer, new byte[0]);
+
+        assertNotNull(result);
+        assertEquals("Expected authentication to be unsuccessful",
+                     AuthenticationResult.AuthenticationStatus.ERROR,
+                     result.getStatus());
+        assertNull(saslServer.getAuthorizationID());
+    }
+
+    private void createSaslServerTestImpl(AuthenticationManager manager) throws Exception
+    {
+        SaslServer server = manager.createSaslServer("EXTERNAL", "example.example.com", null);
+
+        assertEquals("Sasl Server mechanism name is not as expected", "EXTERNAL", server.getMechanismName());
+
+        try
+        {
+            server = manager.createSaslServer("PLAIN", "example.example.com", null);
+            fail("Expected creating SaslServer with incorrect mechanism to throw an exception");
+        }
+        catch (SaslException e)
+        {
+            // pass
+        }
     }
 
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org