Author: markt
Date: Thu Mar  3 11:16:51 2011
New Revision: 1076586

URL: http://svn.apache.org/viewvc?rev=1076586&view=rev
Log:
[SECURITY]
Start of fix for issue reported on users list that @ServletSecurity annotations 
were ignored.
This fix is not yet complete. This first part:
- Triggers the loading of the Wrapper before the constraints are processed to 
ensure that any @ServletSecurity annotations are taken account of
- Makes sure the constraints collection is thread-safe given new usage
- Adds scanning for @ServletSecurity when a Servlet is loaded
- Ensure there is always an authenticator when using the embedded Tomcat class 
so that @ServletSecurity will have an effect
- Adds a simple unit test to check @ServletSecurity annotations are processed
Further commits will add additional test cases and any changes required for 
those test cases to pass

Added:
    tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java   (with 
props)
Modified:
    tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java
    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
    tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java
    tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java

Modified: 
tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java?rev=1076586&r1=1076585&r2=1076586&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java 
Thu Mar  3 11:16:51 2011
@@ -37,6 +37,7 @@ import org.apache.catalina.Manager;
 import org.apache.catalina.Realm;
 import org.apache.catalina.Session;
 import org.apache.catalina.Valve;
+import org.apache.catalina.Wrapper;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
 import org.apache.catalina.deploy.LoginConfig;
@@ -478,6 +479,13 @@ public abstract class AuthenticatorBase 
             }
         }
 
+        // The Servlet may specify security constraints through annotations.
+        // Ensure that they have been processed before constraints are checked
+        Wrapper wrapper = (Wrapper) request.getMappingData().wrapper; 
+        if (wrapper.getServlet() != null) {
+            wrapper.load();
+        }
+
         Realm realm = this.context.getRealm();
         // Is this request URI subject to a security constraint?
         SecurityConstraint [] constraints

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1076586&r1=1076585&r2=1076586&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Thu Mar  3 
11:16:51 2011
@@ -298,7 +298,8 @@ public class StandardContext extends Con
     /**
      * The security constraints for this web application.
      */
-    private SecurityConstraint constraints[] = new SecurityConstraint[0];
+    private volatile SecurityConstraint constraints[] =
+            new SecurityConstraint[0];
     
     private final Object constraintsLock = new Object();
 

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java?rev=1076586&r1=1076585&r2=1076586&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java Thu Mar  3 
11:16:51 2011
@@ -42,9 +42,11 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.ServletSecurityElement;
 import javax.servlet.SingleThreadModel;
 import javax.servlet.UnavailableException;
 import javax.servlet.annotation.MultipartConfig;
+import javax.servlet.annotation.ServletSecurity;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.ContainerServlet;
@@ -1075,10 +1077,20 @@ public class StandardWrapper extends Con
                 }
             }
 
+            ServletSecurity secAnnotation =
+                servlet.getClass().getAnnotation(ServletSecurity.class);
+            Context ctxt = (Context) getParent();
+            if (secAnnotation != null) {
+                ctxt.addServletSecurity(
+                        new ApplicationServletRegistration(this, ctxt),
+                        new ServletSecurityElement(secAnnotation));
+            }
+            
+
             // Special handling for ContainerServlet instances
             if ((servlet instanceof ContainerServlet) &&
                   (isContainerProvidedServlet(servletClass) ||
-                    ((Context)getParent()).getPrivileged() )) {
+                    ctxt.getPrivileged() )) {
                 ((ContainerServlet) servlet).setWrapper(this);
             }
 

Modified: tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java?rev=1076586&r1=1076585&r2=1076586&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java Thu Mar  3 
11:16:51 2011
@@ -42,6 +42,7 @@ import org.apache.catalina.Realm;
 import org.apache.catalina.Server;
 import org.apache.catalina.Service;
 import org.apache.catalina.Wrapper;
+import org.apache.catalina.authenticator.NonLoginAuthenticator;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.core.NamingContextListener;
 import org.apache.catalina.core.StandardContext;
@@ -50,6 +51,7 @@ import org.apache.catalina.core.Standard
 import org.apache.catalina.core.StandardServer;
 import org.apache.catalina.core.StandardService;
 import org.apache.catalina.core.StandardWrapper;
+import org.apache.catalina.deploy.LoginConfig;
 import org.apache.catalina.realm.GenericPrincipal;
 import org.apache.catalina.realm.RealmBase;
 import org.apache.catalina.session.StandardManager;
@@ -698,6 +700,13 @@ public class Tomcat {
                 if (event.getType().equals(Lifecycle.CONFIGURE_START_EVENT)) {
                     context.setConfigured(true);
                 }
+                // LoginConfig is required to process @ServletSecurity
+                // annotations
+                if (context.getLoginConfig() == null) {
+                    context.setLoginConfig(
+                            new LoginConfig("NONE", null, null, null));
+                    context.getPipeline().addValve(new 
NonLoginAuthenticator());
+                }
             } catch (ClassCastException e) {
                 return;
             }

Added: tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java?rev=1076586&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java (added)
+++ tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java Thu Mar 
 3 11:16:51 2011
@@ -0,0 +1,73 @@
+/*
+ *  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.catalina.core;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.annotation.HttpConstraint;
+import javax.servlet.annotation.ServletSecurity;
+import javax.servlet.annotation.ServletSecurity.EmptyRoleSemantic;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TestStandardWrapper extends TomcatBaseTest {
+
+    public void testSecurityAnnotations1() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Must have a real docBase - just use temp
+        Context ctx =
+            tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+        
+        Wrapper wrapper = Tomcat.addServlet(ctx, "servlet",
+                "org.apache.catalina.core.TestStandardWrapper$DenyServlet");
+        wrapper.setAsyncSupported(true);
+        ctx.addServletMapping("/", "servlet");
+        
+        tomcat.start();
+        
+        // Call the servlet once
+        ByteChunk bc = new ByteChunk();
+        int rc = getUrl("http://localhost:"; + getPort() + "/", bc, null);
+        
+        assertNull(bc.toString());
+        assertEquals(403, rc);
+    }
+
+    @ServletSecurity(@HttpConstraint(EmptyRoleSemantic.DENY))
+    public static class DenyServlet extends HttpServlet {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            
+            resp.setContentType("text/plain");
+            resp.getWriter().print("FAIL");
+        }
+    }
+}

Propchange: tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
------------------------------------------------------------------------------
    svn:eol-style = native



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

Reply via email to