[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

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1076586 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index 877d269..8f33ce2 100644 (file)
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -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 extends ValveBase
             }
         }
 
+        // 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

diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java
index 4291d39..b0da03f 100644 (file)
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -298,7 +298,8 @@ public class StandardContext extends ContainerBase
     /**
      * 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();
 

diff --git a/java/org/apache/catalina/core/StandardWrapper.java b/java/org/apache/catalina/core/StandardWrapper.java
index 657d84f..5e57643 100644 (file)
--- a/java/org/apache/catalina/core/StandardWrapper.java
+++ b/java/org/apache/catalina/core/StandardWrapper.java
@@ -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 ContainerBase
                 }
             }
 
+            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);
             }
 

diff --git a/java/org/apache/catalina/startup/Tomcat.java b/java/org/apache/catalina/startup/Tomcat.java
index 9ad2081..84f6490 100644 (file)
--- a/java/org/apache/catalina/startup/Tomcat.java
+++ b/java/org/apache/catalina/startup/Tomcat.java
@@ -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.StandardHost;
 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;
             }

diff --git a/test/org/apache/catalina/core/TestStandardWrapper.java b/test/org/apache/catalina/core/TestStandardWrapper.java
new file mode 100644 (file)
index 0000000..45537cd
--- /dev/null
+++ b/test/org/apache/catalina/core/TestStandardWrapper.java
@@ -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");
+        }
+    }
+}