Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markt-asf committed Mar 3, 2011
1 parent a3847ed commit dbac5e2
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 2 deletions.
8 changes: 8 additions & 0 deletions java/org/apache/catalina/authenticator/AuthenticatorBase.java
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -478,6 +479,13 @@ public void invoke(Request request, Response response)
}
}

// 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
Expand Down
3 changes: 2 additions & 1 deletion java/org/apache/catalina/core/StandardContext.java
Expand Up @@ -298,7 +298,8 @@ public StandardContext() {
/**
* 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();

Expand Down
14 changes: 13 additions & 1 deletion java/org/apache/catalina/core/StandardWrapper.java
Expand Up @@ -42,9 +42,11 @@
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;
Expand Down Expand Up @@ -1075,10 +1077,20 @@ public synchronized Servlet loadServlet() throws ServletException {
}
}

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);
}

Expand Down
9 changes: 9 additions & 0 deletions java/org/apache/catalina/startup/Tomcat.java
Expand Up @@ -42,6 +42,7 @@
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;
Expand All @@ -50,6 +51,7 @@
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;
Expand Down Expand Up @@ -698,6 +700,13 @@ public void lifecycleEvent(LifecycleEvent event) {
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;
}
Expand Down
73 changes: 73 additions & 0 deletions 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");
}
}
}

0 comments on commit dbac5e2

Please sign in to comment.