You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/11/29 15:35:04 UTC

svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Author: markt
Date: Thu Nov 29 14:35:02 2012
New Revision: 1415184

URL: http://svn.apache.org/viewvc?rev=1415184&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54190
Improve unit tests.
Patch by Brian Burch.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1415177-1415179

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java?rev=1415184&r1=1415183&r2=1415184&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java Thu Nov 29 14:35:02 2012
@@ -21,6 +21,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.servlet.http.HttpServletResponse;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -48,9 +50,13 @@ import org.apache.tomcat.util.buf.ByteCh
  */
 public class TestNonLoginAndBasicAuthenticator extends TomcatBaseTest {
 
+    protected static final boolean USE_COOKIES = true;
+    protected static final boolean NO_COOKIES = !USE_COOKIES;
+
     private static final String USER = "user";
     private static final String PWD = "pwd";
     private static final String ROLE = "role";
+    private static final String NICE_METHOD = "Basic";
 
     private static final String HTTP_PREFIX = "http://localhost:";
     private static final String CONTEXT_PATH_NOLOGIN = "/nologin";
@@ -58,12 +64,39 @@ public class TestNonLoginAndBasicAuthent
     private static final String URI_PROTECTED = "/protected";
     private static final String URI_PUBLIC = "/anyoneCanAccess";
 
-    private static final int SHORT_TIMEOUT_SECS = 4;
-    private static final int LONG_TIMEOUT_SECS = 10;
-    private static final long LONG_TIMEOUT_DELAY_MSECS =
-                                    ((LONG_TIMEOUT_SECS + 2) * 1000);
-
-    private static String CLIENT_AUTH_HEADER = "authorization";
+    private static final int SHORT_TIMEOUT_MINS = 1;
+    private static final int LONG_TIMEOUT_MINS = 2;
+    private static final int MANAGER_SCAN_DELAY_SECS = 60;
+    private static final int EXTRA_DELAY_SECS = 5;
+    private static final long TIMEOUT_DELAY_MSECS =
+            (((SHORT_TIMEOUT_MINS * 60)
+                    + MANAGER_SCAN_DELAY_SECS + EXTRA_DELAY_SECS) * 1000);
+
+    private static final String CLIENT_AUTH_HEADER = "authorization";
+    private static final String SERVER_AUTH_HEADER = "WWW-Authenticate";
+    private static final String SERVER_COOKIE_HEADER = "Set-Cookie";
+    private static final String CLIENT_COOKIE_HEADER = "Cookie";
+
+    private static final BasicCredentials NO_CREDENTIALS = null;
+    private static final BasicCredentials GOOD_CREDENTIALS =
+                new BasicCredentials(NICE_METHOD, USER, PWD);
+    private static final BasicCredentials STRANGE_CREDENTIALS =
+                new BasicCredentials("bAsIc", USER, PWD);
+    private static final BasicCredentials BAD_CREDENTIALS =
+                new BasicCredentials(NICE_METHOD, USER, "wrong");
+    private static final BasicCredentials BAD_METHOD =
+                new BasicCredentials("BadMethod", USER, PWD);
+    private static final BasicCredentials SPACED_BASE64 =
+                new BasicCredentials(NICE_METHOD + " ", USER, PWD);
+    private static final BasicCredentials SPACED_USERNAME =
+                new BasicCredentials(NICE_METHOD, " " + USER + " ", PWD);
+    private static final BasicCredentials SPACED_PASSWORD =
+                new BasicCredentials(NICE_METHOD, USER, " " + PWD + " ");
+
+    private Tomcat tomcat;
+    private AuthenticatorBase basicAuthenticator;
+    private AuthenticatorBase nonloginAuthenticator;
+    private List<String> cookies;
 
     /*
      * Try to access an unprotected resource in a webapp that
@@ -72,7 +105,8 @@ public class TestNonLoginAndBasicAuthent
      */
     @Test
     public void testAcceptPublicNonLogin() throws Exception {
-        doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PUBLIC, false, 200);
+        doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PUBLIC, NO_COOKIES,
+                HttpServletResponse.SC_OK);
     }
 
     /*
@@ -82,7 +116,8 @@ public class TestNonLoginAndBasicAuthent
      */
     @Test
     public void testRejectProtectedNonLogin() throws Exception {
-        doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PROTECTED, true, 403);
+        doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PROTECTED, NO_COOKIES,
+                HttpServletResponse.SC_FORBIDDEN);
     }
 
     /*
@@ -92,67 +127,276 @@ public class TestNonLoginAndBasicAuthent
      */
     @Test
     public void testAcceptPublicBasic() throws Exception {
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PUBLIC,
-                false, false, 200, false, 200);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PUBLIC, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
     }
 
     /*
      * Try to access a protected resource in a webapp that
      * has a BASIC login method defined. The access will be
-     * challenged, authenticated and then permitted.
+     * challenged with 401 SC_UNAUTHORIZED, and then be permitted
+     * once authenticated.
      */
     @Test
     public void testAcceptProtectedBasic() throws Exception {
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PROTECTED,
-                false, true, 401, false, 200);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
     }
 
     /*
-     * Try to access a protected resource in a webapp that
-     * has a BASIC login method defined. Verify the server is
-     * prepared to accept non-standard case for the auth scheme.
-     * The access should be challenged, authenticated and then permitted.
+     * This is the same as testAcceptProtectedBasic (above), except
+     * using an invalid password.
+     */
+    @Test
+    public void testAuthMethodBadCredentials() throws Exception {
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, BAD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+    }
+
+    /*
+     * This is the same as testAcceptProtectedBasic (above), except
+     * to verify the server follows RFC2617 by treating the auth-scheme
+     * token as case-insensitive.
      */
     @Test
     public void testAuthMethodCaseBasic() throws Exception {
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PROTECTED,
-                true, true, 401, false, 200);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, STRANGE_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
     }
 
     /*
-     * Logon to access a protected resource in a webapp that uses
-     * BASIC authentication. Wait until that session times-out,
-     * then re-access the resource.
-     * This should be rejected with SC_FORBIDDEN 401 status, which
-     * can be followed by successful re-authentication.
+     * This is the same as testAcceptProtectedBasic (above), except
+     * using an invalid authentication method.
+     *
+     * Note: the container ensures the Basic login method is called.
+     *       BasicAuthenticator does not find the expected authentication
+     *       header method, and so does not extract any credentials.
+     *
+     * The request is rejected with 401 SC_UNAUTHORIZED status. RFC2616
+     * says the response body should identify the auth-schemes that are
+     * acceptable for the container.
+     */
+    @Test
+    public void testAuthMethodBadMethod() throws Exception {
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, BAD_METHOD,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+    }
+
+    /*
+     * This is the same as testAcceptProtectedBasic (above), except
+     * using excess white space after the authentication method.
+     *
+     * The request is rejected with 401 SC_UNAUTHORIZED status.
+     *
+     * TODO: RFC2617 does not define the separation syntax between the
+     *       auth-scheme and basic-credentials tokens. Tomcat should tolerate
+     *       any reasonable amount of white space and return SC_OK.
+     */
+    @Test
+    public void testAuthMethodExtraSpace() throws Exception {
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, SPACED_BASE64,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+    }
+
+    /*
+     * This is the same as testAcceptProtectedBasic (above), except
+     * using white space around the username credential.
+     *
+     * The request is rejected with 401 SC_UNAUTHORIZED status.
+     *
+     * TODO: RFC2617 does not define the separation syntax between the
+     *       auth-scheme and basic-credentials tokens. Tomcat should tolerate
+     *       any reasonable amount of white space and return SC_OK.
+     */
+    @Test
+    public void testUserExtraSpace() throws Exception {
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, SPACED_USERNAME,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+    }
+
+    /*
+     * This is the same as testAcceptProtectedBasic (above), except
+     * using white space around the password credential.
+     *
+     * The request is rejected with 401 SC_UNAUTHORIZED status.
+     *
+     * TODO: RFC2617 does not define the separation syntax between the
+     *       auth-scheme and basic-credentials tokens. Tomcat should tolerate
+     *       any reasonable amount of white space and return SC_OK.
      */
     @Test
+    public void testPasswordExtraSpace() throws Exception {
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, SPACED_PASSWORD,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+    }
+
+    /*
+     * The default behaviour of BASIC authentication does NOT create
+     * a session on the server. Verify that the client is required to
+     * send a valid authenticate header with every request to access
+     * protected resources.
+     */
+    @Test
+    public void testBasicLoginWithoutSession() throws Exception {
+
+        // this section is identical to testAuthMethodCaseBasic
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
+        // next, try to access the protected resource while not providing
+        // credentials. This confirms the server has not retained any state
+        // data which might allow it to authenticate the client. Expect
+        // to be challenged with 401 SC_UNAUTHORIZED.
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+
+        // finally, provide credentials to confirm the resource
+        // can still be accessed with an authentication header.
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+    }
+
+    /*
+     * Test the optional behaviour of BASIC authentication to create
+     * a session on the server. The server will return a session cookie.
+     *
+     * 1. try to access a protected resource without credentials, so
+     *    get Unauthorized status.
+     * 2. try to access a protected resource when providing credentials,
+     *    so get OK status and a server session cookie.
+     * 3. access the protected resource once more using a session cookie.
+     * 4. repeat using the session cookie.
+     *
+     * Note: The FormAuthenticator is a two-step process and is protected
+     *       from session fixation attacks by the default AuthenticatorBase
+     *       changeSessionIdOnAuthentication setting of true. However,
+     *       BasicAuthenticator is a one-step process and so the
+     *       AuthenticatorBase does not reissue the sessionId.
+     */
+   @Test
+    public void testBasicLoginSessionPersistence() throws Exception {
+
+        setAlwaysUseSession();
+
+        // this section is identical to testAuthMethodCaseBasic
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
+        // confirm the session is not recognised by the server alone
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+
+        // now provide the harvested session cookie for authentication
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                USE_COOKIES, HttpServletResponse.SC_OK);
+
+        // finally, do it again with the cookie to be sure
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                USE_COOKIES, HttpServletResponse.SC_OK);
+    }
+
+    /*
+     * Verify the timeout mechanism works for BASIC sessions. This test
+     * follows the flow of testBasicLoginSessionPersistence (above).
+     */
+   @Test
     public void testBasicLoginSessionTimeout() throws Exception {
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PROTECTED,
-                false, true, 401, false, 200);
-        // wait long enough for the session above to expire
-        Thread.sleep(LONG_TIMEOUT_DELAY_MSECS);
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PROTECTED,
-                false, true, 401, false, 200);
+
+       setAlwaysUseSession();
+
+       // this section is identical to testAuthMethodCaseBasic
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
+        // now provide the harvested session cookie for authentication
+        List<String> originalCookies = cookies;
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                USE_COOKIES, HttpServletResponse.SC_OK);
+
+        // allow the session to time out and lose authentication
+        Thread.sleep(TIMEOUT_DELAY_MSECS);
+
+        // provide the harvested session cookie for authentication
+        // to confirm it has expired
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                USE_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+
+        // finally, do BASIC reauthentication and get another session
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
+        // slightly paranoid verification
+        boolean sameCookies = originalCookies.equals(cookies);
+        assertTrue(!sameCookies);
     }
 
     /*
      * Logon to access a protected resource in a webapp that uses
      * BASIC authentication. Then try to access a protected resource
-     * in a different webapp that does not have a login method.
+     * in a different webapp which does not have a login method.
      * This should be rejected with SC_FORBIDDEN 403 status, confirming
      * there has been no cross-authentication between the webapps.
      */
     @Test
     public void testBasicLoginRejectProtected() throws Exception {
-        doTestBasic(USER, PWD, CONTEXT_PATH_LOGIN + URI_PROTECTED,
-                false, true, 401, false, 200);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
         doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PROTECTED,
-                true, 403);
+                NO_COOKIES, HttpServletResponse.SC_FORBIDDEN);
     }
 
+    /*
+     * Try to use the session cookie from the BASIC webapp to request
+     * access to the webapp that does not have a login method. (This
+     * is equivalent to Single Signon, but without the Valve.)
+     *
+     * Verify there is no cross-authentication when using similar logic
+     * to testBasicLoginRejectProtected (above).
+     *
+     * This should be rejected with SC_FORBIDDEN 403 status.
+     */
+    @Test
+    public void testBasicLoginRejectProtectedWithSession() throws Exception {
+
+        setAlwaysUseSession();
+
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, NO_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_UNAUTHORIZED);
+        doTestBasic(CONTEXT_PATH_LOGIN + URI_PROTECTED, GOOD_CREDENTIALS,
+                NO_COOKIES, HttpServletResponse.SC_OK);
+
+        // use the session cookie harvested with the other webapp
+        doTestNonLogin(CONTEXT_PATH_NOLOGIN + URI_PROTECTED,
+                USE_COOKIES, HttpServletResponse.SC_FORBIDDEN);
+    }
 
-    private void doTestNonLogin(String uri, boolean expectedReject,
+
+    private void doTestNonLogin(String uri, boolean useCookie,
             int expectedRC) throws Exception {
 
         Map<String,List<String>> reqHeaders =
@@ -160,102 +404,105 @@ public class TestNonLoginAndBasicAuthent
         Map<String,List<String>> respHeaders =
                 new HashMap<String,List<String>>();
 
+        if (useCookie && (cookies != null)) {
+            reqHeaders.put(CLIENT_COOKIE_HEADER + ":", cookies);
+        }
+
         ByteChunk bc = new ByteChunk();
         int rc = getUrl(HTTP_PREFIX + getPort() + uri, bc, reqHeaders,
                 respHeaders);
 
-        if (expectedReject) {
+        if (expectedRC != HttpServletResponse.SC_OK) {
             assertEquals(expectedRC, rc);
             assertTrue(bc.getLength() > 0);
         }
         else {
-            assertEquals(200, rc);
             assertEquals("OK", bc.toString());
         }
     }
 
-    private void doTestBasic(String user, String pwd, String uri,
-            boolean verifyAuthSchemeCase,
-            boolean expectedReject1, int expectedRC1,
-            boolean expectedReject2, int expectedRC2) throws Exception {
-
-        // the first access attempt should be challenged
-        Map<String,List<String>> reqHeaders1 =
-                new HashMap<String,List<String>>();
-        Map<String,List<String>> respHeaders1 =
-                new HashMap<String,List<String>>();
+    private void doTestBasic(String uri, BasicCredentials credentials,
+            boolean useCookie, int expectedRC) throws Exception {
 
-        ByteChunk bc = new ByteChunk();
-        int rc = getUrl(HTTP_PREFIX + getPort() + uri, bc, reqHeaders1,
-                respHeaders1);
+        Map<String,List<String>> reqHeaders =
+                new HashMap<String, List<String>>();
+        Map<String,List<String>> respHeaders =
+                new HashMap<String, List<String>>();
 
-        if (expectedReject1) {
-            assertEquals(expectedRC1, rc);
-            assertTrue(bc.getLength() > 0);
+        if (useCookie && (cookies != null)) {
+            reqHeaders.put(CLIENT_COOKIE_HEADER + ":", cookies);
         }
         else {
-            assertEquals(200, rc);
-            assertEquals("OK", bc.toString());
-            return;
+            if (credentials != null) {
+                List<String> auth = new ArrayList<String>();
+                auth.add(credentials.getCredentials());
+                reqHeaders.put(CLIENT_AUTH_HEADER, auth);
+            }
         }
 
-        // the second access attempt should be sucessful
-        String credentials = user + ":" + pwd;
-        byte[] credentialsBytes = ByteChunk.convertToBytes(credentials);
-        String base64auth = Base64.encode(credentialsBytes);
-        String authScheme = verifyAuthSchemeCase ? "bAsIc " : "Basic ";
-        String authLine = authScheme + base64auth;
-
-        List<String> auth = new ArrayList<String>();
-        auth.add(authLine);
-        Map<String,List<String>> reqHeaders2 = new HashMap<String,List<String>>();
-        reqHeaders2.put(CLIENT_AUTH_HEADER, auth);
-
-        Map<String,List<String>> respHeaders2 =
-            new HashMap<String,List<String>>();
-
-        bc.recycle();
-        rc = getUrl(HTTP_PREFIX + getPort() + uri, bc, reqHeaders2,
-                respHeaders2);
+        ByteChunk bc = new ByteChunk();
+        int rc = getUrl(HTTP_PREFIX + getPort() + uri, bc, reqHeaders,
+                respHeaders);
 
-        if (expectedReject2) {
-            assertEquals(expectedRC2, rc);
+        if (expectedRC != HttpServletResponse.SC_OK) {
+            assertEquals(expectedRC, rc);
             assertTrue(bc.getLength() > 0);
+            if (expectedRC == HttpServletResponse.SC_UNAUTHORIZED) {
+                // The server should identify the acceptable method(s)
+                boolean methodFound = false;
+                List<String> authHeaders = respHeaders.get(SERVER_AUTH_HEADER);
+                for (String authHeader : authHeaders) {
+                    if (authHeader.indexOf(NICE_METHOD) > -1) {
+                        methodFound = true;
+                    }
+                }
+                assertTrue(methodFound);
+            }
         }
         else {
-            assertEquals(200, rc);
             assertEquals("OK", bc.toString());
+            List<String> newCookies = respHeaders.get(SERVER_COOKIE_HEADER);
+            if (newCookies != null) {
+                // harvest cookies whenever the server sends some new ones
+                cookies = newCookies;
+            }
         }
     }
 
 
+    /*
+     * setup two webapps for every test
+     *
+     * note: the super class tearDown method will stop tomcat
+     */
     @Override
     public void setUp() throws Exception {
 
         super.setUp();
 
         // create a tomcat server using the default in-memory Realm
-        Tomcat tomcat = getTomcatInstance();
+        tomcat = getTomcatInstance();
 
         // add the test user and role to the Realm
         tomcat.addUser(USER, PWD);
         tomcat.addRole(USER, ROLE);
 
         // setup both NonLogin and Login webapps
-        setUpNonLogin(tomcat);
-        setUpLogin(tomcat);
+        setUpNonLogin();
+        setUpLogin();
 
         tomcat.start();
     }
 
-    private void setUpNonLogin(Tomcat tomcat) throws Exception {
+
+    private void setUpNonLogin() throws Exception {
 
         // Must have a real docBase for webapps - just use temp
         Context ctxt = tomcat.addContext(CONTEXT_PATH_NOLOGIN,
                 System.getProperty("java.io.tmpdir"));
-        ctxt.setSessionTimeout(LONG_TIMEOUT_SECS);
+        ctxt.setSessionTimeout(LONG_TIMEOUT_MINS);
 
-        // Add protected servlet
+        // Add protected servlet to the context
         Tomcat.addServlet(ctxt, "TesterServlet1", new TesterServlet());
         ctxt.addServletMapping(URI_PROTECTED, "TesterServlet1");
 
@@ -266,7 +513,7 @@ public class TestNonLoginAndBasicAuthent
         sc1.addCollection(collection1);
         ctxt.addConstraint(sc1);
 
-        // Add unprotected servlet
+        // Add unprotected servlet to the context
         Tomcat.addServlet(ctxt, "TesterServlet2", new TesterServlet());
         ctxt.addServletMapping(URI_PUBLIC, "TesterServlet2");
 
@@ -281,17 +528,18 @@ public class TestNonLoginAndBasicAuthent
         LoginConfig lc = new LoginConfig();
         lc.setAuthMethod("NONE");
         ctxt.setLoginConfig(lc);
-        ctxt.getPipeline().addValve(new NonLoginAuthenticator());
+        nonloginAuthenticator = new NonLoginAuthenticator();
+        ctxt.getPipeline().addValve(nonloginAuthenticator);
     }
 
-    private void setUpLogin(Tomcat tomcat) throws Exception {
+    private void setUpLogin() throws Exception {
 
         // Must have a real docBase for webapps - just use temp
         Context ctxt = tomcat.addContext(CONTEXT_PATH_LOGIN,
                 System.getProperty("java.io.tmpdir"));
-        ctxt.setSessionTimeout(SHORT_TIMEOUT_SECS);
+        ctxt.setSessionTimeout(SHORT_TIMEOUT_MINS);
 
-        // Add protected servlet
+        // Add protected servlet to the context
         Tomcat.addServlet(ctxt, "TesterServlet3", new TesterServlet());
         ctxt.addServletMapping(URI_PROTECTED, "TesterServlet3");
         SecurityCollection collection = new SecurityCollection();
@@ -301,7 +549,7 @@ public class TestNonLoginAndBasicAuthent
         sc.addCollection(collection);
         ctxt.addConstraint(sc);
 
-        // Add unprotected servlet
+        // Add unprotected servlet to the context
         Tomcat.addServlet(ctxt, "TesterServlet4", new TesterServlet());
         ctxt.addServletMapping(URI_PUBLIC, "TesterServlet4");
 
@@ -316,7 +564,44 @@ public class TestNonLoginAndBasicAuthent
         LoginConfig lc = new LoginConfig();
         lc.setAuthMethod("BASIC");
         ctxt.setLoginConfig(lc);
-        ctxt.getPipeline().addValve(new BasicAuthenticator());
+        basicAuthenticator = new BasicAuthenticator();
+        ctxt.getPipeline().addValve(basicAuthenticator);
+    }
+
+    /*
+     * Force non-default behaviour for both Authenticators
+     */
+    private void setAlwaysUseSession() {
+
+        basicAuthenticator.setAlwaysUseSession(true);
+        nonloginAuthenticator.setAlwaysUseSession(true);
     }
 
+    /*
+     * Encapsulate the logic to generate an HTTP header
+     * for BASIC Authentication.
+     * Note: only used internally, so no need to validate arguments.
+     */
+    private static final class BasicCredentials {
+
+        private final String method;
+        private final String username;
+        private final String password;
+        private final String credentials;
+
+        private BasicCredentials(String aMethod,
+                String aUsername, String aPassword) {
+            method = aMethod;
+            username = aUsername;
+            password = aPassword;
+            String userCredentials = username + ":" + password;
+            byte[] credentialsBytes = ByteChunk.convertToBytes(userCredentials);
+            String base64auth = Base64.encode(credentialsBytes);
+            credentials= method + " " + base64auth;
+        }
+
+        private String getCredentials() {
+            return credentials;
+        }
+    }
 }
\ No newline at end of file

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1415184&r1=1415183&r2=1415184&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Nov 29 14:35:02 2012
@@ -68,6 +68,11 @@
         Add new attribute <code>renameOnRotate</code> to the AccessLogValve.
         (rjung)
       </add>
+      <fix>
+        <bug>54190</bug>: Correct unit tests for BASIC authentication so that
+        session timeout is correctly tested. Also refactor unit test to make it
+        easier to add additional tests. Patch by Brian Burch. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/12/3 Brian Burch <br...@pingtoo.com>:
> On 03/12/12 16:22, Brian Burch wrote:
>>
>> On 03/12/12 11:44, Brian Burch wrote:
>>>
>>> On 02/12/12 22:00, Konstantin Kolinko wrote:
>>
>> <snip/>
>>>>
>>>> According to Buildbot logs, testBasicLoginSessionTimeout() runs for 2
>>>> minutes (125 seconds), mainly due to a sleep() call.
>>>>
>>>> I wish there were a way to speed up this test.
>>>>
>>>> My thoughts
>>>> a) Maybe use reflection to reduce StandardSession.lastAccessedTime and
>>>> thisAccessedTime by some fixed amount (60000) instead of waiting for
>>>> that time to pass.
>>>
>>>
>>> That would speed up the test... but it sounds like adding a time machine
>>> to the test class! My feeling is this would add inappropriate logical
>>> complexity to a test that has always created and destroyed a tomcat
>>> instance for each test case (there are now 15).
>>>
>>> However, I intend to replicate most of the improvements from this test
>>> class into the other authenticator tests, so I am already apprehensive
>>> about adding too many 60+ second delays to the entire suite.
>>>
>>> Mark and I briefly discussed adding a new protected method for use by
>>> these timeout tests:
>>>
>>> org.apache.catalina.session.StandardSession.setSessionTimeoutSecs(int
>>> secs)
>>
>>
>> Silly me! Sorry if that left you wondering what I meant. That particular
>> method already exists, is public, and is called setSessionTimeout.
>>
>> What I meant to suggest was this:
>>
>> org.apache.catalina.core.StandardContext.setSessionTimeoutSecs(int
>> timeout)
>>
>> because it would be much simpler to modify the Context in the test
>> setUp, rather than each individual Session.
>>
>>> What do you think?
>
>
> The change seemed almost trivial to me, but I couldn't work out how to
> implement it without changing the public api, i.e.
>
> I have only found one place where it is used:
>
> org.apache.catalina.session.ManagerBase.setContext(Context context)
>
> has this logic:
>
> // Register with the new Context (if any)
> if (this.context != null) {
>     setMaxInactiveInterval(this.context.getSessionTimeout() * 60);
>     this.context.addPropertyChangeListener(this);
> }
>
> I wanted to move the "60" back into StandardContext, but that doesn't work
> because it means changing the org.apache.catalina.Context api to work in
> seconds instead of minutes. ManagerBase and StandardContext are in different
> packages, so a protected accessor for the timeout in seconds wouldn't work
> either.
>
> On the other hand, I wondered about defining an alternative timeout in
> seconds within the ManagerBase, which would only be used by unit tests.
> ManagerBase would use its own timeout if non-zero, otherwise use the Context
> value in minutes.
>
> I would really appreciate some guidance on the best way to proceed. (Perhaps
> it is better to keep things simple by living with an 80 seconds test time?)
>

I see two other ways besides modifying the context class:
a) call ManagerBase.setMaxInactiveInterval(), as it is public.
b) сall HttpSession.setMaxInactiveInterval() on that specific session,
e.g. from a HttpSessionListener

BTW, using an HttpSessionListener you can note whether the session has
actually expired (whether sessionDestroyed() was called for it).

The listener could be used to further speed up the test. As far as the
listener knows that all created sessions have expired, the test can
proceed. (So if the background thread runs sooner that in 15 seconds,
the test can continue sooner).

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Posted by Brian Burch <br...@pingtoo.com>.
On 03/12/12 16:22, Brian Burch wrote:
> On 03/12/12 11:44, Brian Burch wrote:
>> On 02/12/12 22:00, Konstantin Kolinko wrote:
> <snip/>
>>> According to Buildbot logs, testBasicLoginSessionTimeout() runs for 2
>>> minutes (125 seconds), mainly due to a sleep() call.
>>>
>>> I wish there were a way to speed up this test.
>>>
>>> My thoughts
>>> a) Maybe use reflection to reduce StandardSession.lastAccessedTime and
>>> thisAccessedTime by some fixed amount (60000) instead of waiting for
>>> that time to pass.
>>
>> That would speed up the test... but it sounds like adding a time machine
>> to the test class! My feeling is this would add inappropriate logical
>> complexity to a test that has always created and destroyed a tomcat
>> instance for each test case (there are now 15).
>>
>> However, I intend to replicate most of the improvements from this test
>> class into the other authenticator tests, so I am already apprehensive
>> about adding too many 60+ second delays to the entire suite.
>>
>> Mark and I briefly discussed adding a new protected method for use by
>> these timeout tests:
>>
>> org.apache.catalina.session.StandardSession.setSessionTimeoutSecs(int
>> secs)
>
> Silly me! Sorry if that left you wondering what I meant. That particular
> method already exists, is public, and is called setSessionTimeout.
>
> What I meant to suggest was this:
>
> org.apache.catalina.core.StandardContext.setSessionTimeoutSecs(int timeout)
>
> because it would be much simpler to modify the Context in the test
> setUp, rather than each individual Session.
>
>> What do you think?

The change seemed almost trivial to me, but I couldn't work out how to 
implement it without changing the public api, i.e.

I have only found one place where it is used:

org.apache.catalina.session.ManagerBase.setContext(Context context)

has this logic:

// Register with the new Context (if any)
if (this.context != null) {
     setMaxInactiveInterval(this.context.getSessionTimeout() * 60);
     this.context.addPropertyChangeListener(this);
}

I wanted to move the "60" back into StandardContext, but that doesn't 
work because it means changing the org.apache.catalina.Context api to 
work in seconds instead of minutes. ManagerBase and StandardContext are 
in different packages, so a protected accessor for the timeout in 
seconds wouldn't work either.

On the other hand, I wondered about defining an alternative timeout in 
seconds within the ManagerBase, which would only be used by unit tests. 
ManagerBase would use its own timeout if non-zero, otherwise use the 
Context value in minutes.

I would really appreciate some guidance on the best way to proceed. 
(Perhaps it is better to keep things simple by living with an 80 seconds 
test time?)

> <snip/>
>>
>>> Best regards,
>>> Konstantin Kolinko
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Posted by Brian Burch <br...@pingtoo.com>.
On 03/12/12 11:44, Brian Burch wrote:
> On 02/12/12 22:00, Konstantin Kolinko wrote:
<snip/>
>> According to Buildbot logs, testBasicLoginSessionTimeout() runs for 2
>> minutes (125 seconds), mainly due to a sleep() call.
>>
>> I wish there were a way to speed up this test.
>>
>> My thoughts
>> a) Maybe use reflection to reduce StandardSession.lastAccessedTime and
>> thisAccessedTime by some fixed amount (60000) instead of waiting for
>> that time to pass.
>
> That would speed up the test... but it sounds like adding a time machine
> to the test class! My feeling is this would add inappropriate logical
> complexity to a test that has always created and destroyed a tomcat
> instance for each test case (there are now 15).
>
> However, I intend to replicate most of the improvements from this test
> class into the other authenticator tests, so I am already apprehensive
> about adding too many 60+ second delays to the entire suite.
>
> Mark and I briefly discussed adding a new protected method for use by
> these timeout tests:
>
> org.apache.catalina.session.StandardSession.setSessionTimeoutSecs(int secs)

Silly me! Sorry if that left you wondering what I meant. That particular 
method already exists, is public, and is called setSessionTimeout.

What I meant to suggest was this:

org.apache.catalina.core.StandardContext.setSessionTimeoutSecs(int timeout)

because it would be much simpler to modify the Context in the test 
setUp, rather than each individual Session.

> What do you think?
>
<snip/>
>
>> Best regards,
>> Konstantin Kolinko
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Posted by Brian Burch <br...@pingtoo.com>.
On 02/12/12 22:00, Konstantin Kolinko wrote:
> 2012/11/29  <ma...@apache.org>:
>> Author: markt
>> Date: Thu Nov 29 14:35:02 2012
>> New Revision: 1415184
>>
>> URL: http://svn.apache.org/viewvc?rev=1415184&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54190
>> Improve unit tests.
>> Patch by Brian Burch.
>>
>> Modified:
>>      tomcat/tc7.0.x/trunk/   (props changed)
>>      tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java
>>      tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>>
>> Propchange: tomcat/tc7.0.x/trunk/
>> ------------------------------------------------------------------------------
>>    Merged /tomcat/trunk:r1415177-1415179
>>
>
>> +    private static final int SHORT_TIMEOUT_MINS = 1;
>> +    private static final int LONG_TIMEOUT_MINS = 2;
>> +    private static final int MANAGER_SCAN_DELAY_SECS = 60;
>> +    private static final int EXTRA_DELAY_SECS = 5;
>> +    private static final long TIMEOUT_DELAY_MSECS =
>> +            (((SHORT_TIMEOUT_MINS * 60)
>> +                    + MANAGER_SCAN_DELAY_SECS + EXTRA_DELAY_SECS) * 1000);
>
> ...
>
>> +        // allow the session to time out and lose authentication
>> +        Thread.sleep(TIMEOUT_DELAY_MSECS);
>
>
> According to Buildbot logs, testBasicLoginSessionTimeout() runs for 2
> minutes (125 seconds), mainly due to a sleep() call.
>
> I wish there were a way to speed up this test.
>
> My thoughts
> a) Maybe use reflection to reduce StandardSession.lastAccessedTime and
> thisAccessedTime by some fixed amount (60000) instead of waiting for
> that time to pass.

That would speed up the test... but it sounds like adding a time machine 
to the test class! My feeling is this would add inappropriate logical 
complexity to a test that has always created and destroyed a tomcat 
instance for each test case (there are now 15).

However, I intend to replicate most of the improvements from this test 
class into the other authenticator tests, so I am already apprehensive 
about adding too many 60+ second delays to the entire suite.

Mark and I briefly discussed adding a new protected method for use by 
these timeout tests:

org.apache.catalina.session.StandardSession.setSessionTimeoutSecs(int secs)

What do you think?

> b) Maybe call ManagerBase.setProcessExpiresFrequency(1) instead of the
> default value (6)  and reduce MANAGER_SCAN_DELAY_SECS from 60 to 10.

Mark and I discussed this idea earlier, but I was reluctant to increase 
complexity in an already radical change to the original.

Your suggestion showed me how to achieve faster detection in a fairly 
simple manner, so I have reopened the bug and provided a new patch that 
shaves 50 seconds off the run time of the timeout test. Thanks!

> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1415184 - in /tomcat/tc7.0.x/trunk: ./ test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java webapps/docs/changelog.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/11/29  <ma...@apache.org>:
> Author: markt
> Date: Thu Nov 29 14:35:02 2012
> New Revision: 1415184
>
> URL: http://svn.apache.org/viewvc?rev=1415184&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54190
> Improve unit tests.
> Patch by Brian Burch.
>
> Modified:
>     tomcat/tc7.0.x/trunk/   (props changed)
>     tomcat/tc7.0.x/trunk/test/org/apache/catalina/authenticator/TestNonLoginAndBasicAuthenticator.java
>     tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
> Propchange: tomcat/tc7.0.x/trunk/
> ------------------------------------------------------------------------------
>   Merged /tomcat/trunk:r1415177-1415179
>

> +    private static final int SHORT_TIMEOUT_MINS = 1;
> +    private static final int LONG_TIMEOUT_MINS = 2;
> +    private static final int MANAGER_SCAN_DELAY_SECS = 60;
> +    private static final int EXTRA_DELAY_SECS = 5;
> +    private static final long TIMEOUT_DELAY_MSECS =
> +            (((SHORT_TIMEOUT_MINS * 60)
> +                    + MANAGER_SCAN_DELAY_SECS + EXTRA_DELAY_SECS) * 1000);

...

> +        // allow the session to time out and lose authentication
> +        Thread.sleep(TIMEOUT_DELAY_MSECS);


According to Buildbot logs, testBasicLoginSessionTimeout() runs for 2
minutes (125 seconds), mainly due to a sleep() call.

I wish there were a way to speed up this test.

My thoughts
a) Maybe use reflection to reduce StandardSession.lastAccessedTime and
thisAccessedTime by some fixed amount (60000) instead of waiting for
that time to pass.

b) Maybe call ManagerBase.setProcessExpiresFrequency(1) instead of the
default value (6)  and reduce MANAGER_SCAN_DELAY_SECS from 60 to 10.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org