On Sep 25, 2010, at 1:30 AM, Ivan wrote:

> Hi, David
>     Current way seems to miss some changes in GERONIMO-5577, we need to 
> record all the really added dynamic  servlets for the security annotation 
> scanning later.

I agree.  

>     Also, from my side, I think that the use of ServletContext.createServlet 
> is not correct in Jetty, seems Jetty use this method to create all the 
> servlet instances, while from the JavaDoc, this method is only intended for 
> dynamic use, and should throw some exceptions.

I'm not so sure about this. I need to study the specs some more.

I will work on both of these.... thanks for bringing them up.

david jencks

>     Thanks.
> 
> 2010/9/25 <[email protected]>
> Author: djencks
> Date: Sat Sep 25 07:59:39 2010
> New Revision: 1001159
> 
> URL: http://svn.apache.org/viewvc?rev=1001159&view=rev
> Log:
> GERONIMO-5624 For jetty, overide jetty internal methods (that I just added) 
> instead of wrapping the ServletContext.Dynamic
> 
> Removed:
>    
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoApplicationServletRegistrationAdapter.java
>    
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/security/JACCSecurityEventListener.java
> Modified:
>    
> geronimo/server/trunk/plugins/j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/WebSecurityConstraintStore.java
>    
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/WebAppContextWrapper.java
>    
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoWebAppContext.java
>    
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/JaccSecurityHandler.java
> 
> Modified: 
> geronimo/server/trunk/plugins/j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/WebSecurityConstraintStore.java
> URL: 
> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/WebSecurityConstraintStore.java?rev=1001159&r1=1001158&r2=1001159&view=diff
> ==============================================================================
> --- 
> geronimo/server/trunk/plugins/j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/WebSecurityConstraintStore.java
>  (original)
> +++ 
> geronimo/server/trunk/plugins/j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/WebSecurityConstraintStore.java
>  Sat Sep 25 07:59:39 2010
> @@ -29,6 +29,7 @@ import java.util.Set;
> 
>  import javax.servlet.HttpMethodConstraintElement;
>  import javax.servlet.ServletContext;
> +import javax.servlet.ServletRegistration;
>  import javax.servlet.ServletSecurityElement;
>  import javax.servlet.annotation.ServletSecurity;
>  import javax.servlet.annotation.ServletSecurity.TransportGuarantee;
> @@ -152,6 +153,12 @@ public class WebSecurityConstraintStore
>         return containerCreatedDynamicServlets.containsKey(servlet);
>     }
> 
> +    public Set<String> setDynamicServletSecurity(ServletRegistration.Dynamic 
> registration, ServletSecurityElement constraint) {
> +        dynamicServletNameSecurityElementMap.put(registration.getName(), 
> constraint);
> +        Set<String> uneffectedUrlPatterns = new 
> HashSet<String>(registration.getMappings());
> +        uneffectedUrlPatterns.retainAll(webXmlConstraintUrlPatterns);
> +        return uneffectedUrlPatterns;
> +    }
>     public Set<String> setDynamicServletSecurity(String servletName, 
> ServletSecurityElement constraint, Collection<String> urlPatterns) {
>         dynamicServletNameSecurityElementMap.put(servletName, constraint);
>         Set<String> uneffectedUrlPatterns = new HashSet<String>(urlPatterns);
> 
> Modified: 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/WebAppContextWrapper.java
> URL: 
> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/WebAppContextWrapper.java?rev=1001159&r1=1001158&r2=1001159&view=diff
> ==============================================================================
> --- 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/WebAppContextWrapper.java
>  (original)
> +++ 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/WebAppContextWrapper.java
>  Sat Sep 25 07:59:39 2010
> @@ -44,7 +44,6 @@ import org.apache.geronimo.j2ee.jndi.Con
>  import org.apache.geronimo.j2ee.management.impl.InvalidObjectNameException;
>  import org.apache.geronimo.jetty8.handler.GeronimoWebAppContext;
>  import org.apache.geronimo.jetty8.handler.IntegrationContext;
> -import org.apache.geronimo.jetty8.security.JACCSecurityEventListener;
>  import org.apache.geronimo.jetty8.security.SecurityHandlerFactory;
>  import org.apache.geronimo.kernel.Kernel;
>  import org.apache.geronimo.kernel.ObjectNameUtil;
> @@ -55,7 +54,6 @@ import org.apache.geronimo.management.ge
>  import 
> org.apache.geronimo.security.jacc.ApplicationPolicyConfigurationManager;
>  import org.apache.geronimo.security.jacc.RunAsSource;
>  import org.apache.geronimo.transaction.GeronimoUserTransaction;
> -import org.apache.geronimo.web.WebAttributeName;
>  import org.apache.geronimo.web.info.WebAppInfo;
>  import org.eclipse.jetty.http.MimeTypes;
>  import org.eclipse.jetty.security.SecurityHandler;
> @@ -181,7 +179,7 @@ public class WebAppContextWrapper implem
>         Context componentContext = contextSource.getContext();
>         UserTransaction userTransaction = new 
> GeronimoUserTransaction(transactionManager);
>         IntegrationContext integrationContext = new 
> IntegrationContext(componentContext, unshareableResources, 
> applicationManagedSecurityResources, trackedConnectionAssociator, 
> userTransaction, bundle, holder);
> -        webAppContext = new GeronimoWebAppContext(securityHandler, 
> sessionHandler, servletHandler, null, integrationContext, classLoader, 
> modulePath, webAppInfo);
> +        webAppContext = new GeronimoWebAppContext(securityHandler, 
> sessionHandler, servletHandler, null, integrationContext, classLoader, 
> modulePath, webAppInfo, policyContextID, 
> applicationPolicyConfigurationManager);
>         webAppContext.setContextPath(contextPath);
>         //See Jetty-386.  Setting this to true can expose secured content.
>         webAppContext.setCompactPath(compactPath);
> @@ -234,7 +232,6 @@ public class WebAppContextWrapper implem
>         if (contextParamMap != null) {
>             webAppContext.getInitParams().putAll(contextParamMap);
>         }
> -//        setListenerClassNames(listenerClassNames);
>         webAppContext.setDistributable(distributable);
>         webAppContext.setWelcomeFiles(welcomeFiles);
>         setLocaleEncodingMapping(localeEncodingMapping);
> @@ -246,13 +243,6 @@ public class WebAppContextWrapper implem
>         }
>         //supply web.xml to jasper
>         webAppContext.setAttribute(JASPER_WEB_XML_NAME, originalSpecDD);
> -
> -        if (securityHandlerFactory != null) {
> -            float schemaVersion = (Float) 
> deploymentAttributes.get(WebAttributeName.SCHEMA_VERSION.name());
> -            boolean metaComplete = (Boolean) 
> deploymentAttributes.get(WebAttributeName.META_COMPLETE.name());
> -            webAppContext.addLifeCycleListener(new 
> JACCSecurityEventListener(bundle, webAppInfo, schemaVersion >= 2.5f && 
> !metaComplete, applicationPolicyConfigurationManager, policyContextID,
> -                    (GeronimoWebAppContext.SecurityContext) 
> webAppContext.getServletContext()));
> -        }
>     }
> 
> 
> 
> Modified: 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoWebAppContext.java
> URL: 
> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoWebAppContext.java?rev=1001159&r1=1001158&r2=1001159&view=diff
> ==============================================================================
> --- 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoWebAppContext.java
>  (original)
> +++ 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/GeronimoWebAppContext.java
>  Sat Sep 25 07:59:39 2010
> @@ -26,14 +26,20 @@ import java.net.URL;
>  import java.util.Collections;
>  import java.util.Enumeration;
>  import java.util.EventListener;
> +import java.util.HashMap;
>  import java.util.HashSet;
> +import java.util.Map;
>  import java.util.Set;
> 
>  import javax.naming.NamingException;
> +import javax.security.auth.login.LoginException;
> +import javax.security.jacc.PolicyContextException;
>  import javax.servlet.Filter;
>  import javax.servlet.Servlet;
>  import javax.servlet.ServletException;
> +import javax.servlet.ServletRegistration;
>  import javax.servlet.ServletRegistration.Dynamic;
> +import javax.servlet.ServletSecurityElement;
>  import javax.servlet.http.HttpServletRequest;
>  import javax.servlet.http.HttpServletResponse;
> 
> @@ -41,8 +47,11 @@ import org.apache.geronimo.connector.out
>  import 
> org.apache.geronimo.connector.outbound.connectiontracking.SharedConnectorInstanceContext;
>  import org.apache.geronimo.osgi.web.WebApplicationConstants;
>  import org.apache.geronimo.osgi.web.WebApplicationUtils;
> +import 
> org.apache.geronimo.security.jacc.ApplicationPolicyConfigurationManager;
> +import org.apache.geronimo.security.jacc.ComponentPermissions;
>  import org.apache.geronimo.web.assembler.Assembler;
>  import org.apache.geronimo.web.info.WebAppInfo;
> +import org.apache.geronimo.web.security.SpecSecurityBuilder;
>  import org.apache.geronimo.web.security.WebSecurityConstraintStore;
>  import org.apache.xbean.osgi.bundle.util.BundleUtils;
>  import org.eclipse.jetty.security.SecurityHandler;
> @@ -67,12 +76,22 @@ public class GeronimoWebAppContext exten
>     private final String modulePath;
>     private final ClassLoader classLoader;
>     private final WebAppInfo webAppInfo;
> +    private final WebSecurityConstraintStore webSecurityConstraintStore;
> +    private final String policyContextId;
> +    private final ApplicationPolicyConfigurationManager 
> applicationPolicyConfigurationManager;
>     private ServiceRegistration serviceRegistration;
>     boolean fullyStarted = false;
> 
> -    public GeronimoWebAppContext(SecurityHandler securityHandler, 
> SessionHandler sessionHandler, ServletHandler servletHandler, ErrorHandler 
> errorHandler, IntegrationContext integrationContext, ClassLoader classLoader, 
> String modulePath, WebAppInfo webAppInfo) {
> +    public GeronimoWebAppContext(SecurityHandler securityHandler,
> +                                 SessionHandler sessionHandler,
> +                                 ServletHandler servletHandler,
> +                                 ErrorHandler errorHandler,
> +                                 IntegrationContext integrationContext,
> +                                 ClassLoader classLoader,
> +                                 String modulePath,
> +                                 WebAppInfo webAppInfo, String 
> policyContextId, ApplicationPolicyConfigurationManager 
> applicationPolicyConfigurationManager) {
>         super(sessionHandler, securityHandler, servletHandler, errorHandler);
> -        _scontext = securityHandler == null ? new Context() : new 
> SecurityContext();
> +        _scontext = new Context();
>         this.integrationContext = integrationContext;
>         setClassLoader(classLoader);
>         this.classLoader = classLoader;
> @@ -87,6 +106,12 @@ public class GeronimoWebAppContext exten
>         }
>         this.modulePath = modulePath;
>         this.webAppInfo = webAppInfo;
> +        this.policyContextId = policyContextId;
> +        this.applicationPolicyConfigurationManager = 
> applicationPolicyConfigurationManager;
> +        //TODO schemaVersion >= 2.5f && !metaComplete but only for a 
> while....
> +        boolean annotationScanRequired = true;
> +        webSecurityConstraintStore = new 
> WebSecurityConstraintStore(webAppInfo, integrationContext.getBundle(), 
> annotationScanRequired, _scontext);
> +
>     }
> 
>     public void registerServletContext() {
> @@ -116,6 +141,24 @@ public class GeronimoWebAppContext exten
>                 assembler.assemble(getServletContext(), webAppInfo);
>                 ((GeronimoWebAppContext.Context) _scontext).webXmlProcessed = 
> true;
>                 super.doStart();
> +                if (applicationPolicyConfigurationManager != null) {
> +                    SpecSecurityBuilder specSecurityBuilder = new 
> SpecSecurityBuilder(webSecurityConstraintStore.exportMergedWebAppInfo());
> +                    Map<String, ComponentPermissions> 
> contextIdPermissionsMap = new HashMap<String, ComponentPermissions>();
> +                    contextIdPermissionsMap.put(policyContextId, 
> specSecurityBuilder.buildSpecSecurityConfig());
> +                    //Update ApplicationPolicyConfigurationManager
> +                    try {
> +                        
> applicationPolicyConfigurationManager.updateApplicationPolicyConfiguration(contextIdPermissionsMap);
> +                    } catch (LoginException e) {
> +                        throw new RuntimeException("Fail to set application 
> policy configurations", e);
> +                    } catch (PolicyContextException e) {
> +                        throw new RuntimeException("Fail to set application 
> policy configurations", e);
> +                    } catch (ClassNotFoundException e) {
> +                        throw new RuntimeException("Fail to set application 
> policy configurations", e);
> +                    } finally {
> +                        //Clear SpecSecurityBuilder
> +                        specSecurityBuilder.clear();
> +                    }
> +                }
>                 fullyStarted = true;
>             } finally {
>                 setRestrictListeners(true);
> @@ -227,6 +270,17 @@ public class GeronimoWebAppContext exten
>         return paths;
>     }
> 
> +
> +    @Override
> +    public Set<String> setServletSecurity(ServletRegistration.Dynamic 
> registration, ServletSecurityElement servletSecurityElement) {
> +        return 
> webSecurityConstraintStore.setDynamicServletSecurity(registration, 
> servletSecurityElement);
> +    }
> +
> +    @Override
> +    protected void addRoles(String... roles) {
> +        webSecurityConstraintStore.declareRoles(roles);
> +    }
> +
>     private Resource lookupResource(String uriInContext) {
>         Bundle bundle = integrationContext.getBundle();
>         URL url = BundleUtils.getEntry(bundle, uriInContext);
> @@ -286,82 +340,8 @@ public class GeronimoWebAppContext exten
>         @Override
>         public <T extends Servlet> T createServlet(Class<T> c) throws 
> ServletException {
>             try {
> -                return (T) 
> integrationContext.getHolder().newInstance(c.getName(), classLoader, 
> integrationContext.getComponentContext());
> -            } catch (IllegalAccessException e) {
> -                throw new ServletException("Could not create servlet " + 
> c.getName(), e);
> -            } catch (InstantiationException e) {
> -                throw new ServletException("Could not create servlet " + 
> c.getName(), e);
> -            }
> -        }
> -    }
> -
> -    public class SecurityContext extends Context {
> -
> -        private WebSecurityConstraintStore webSecurityConstraintStore;
> -
> -        @Override
> -        public Dynamic addServlet(String servletName, Class<? extends 
> Servlet> servletClass) {
> -            Dynamic dynamic = super.addServlet(servletName, servletClass);
> -            if (!webXmlProcessed) {
> -                return dynamic;
> -            }
> -            
> webSecurityConstraintStore.addContainerCreatedDynamicServletEntry(servletName,
>  servletClass.getName());
> -            return 
> createGeronimoApplicationServletRegistrationAdapter(dynamic, servletName);
> -        }
> -
> -        @Override
> -        public Dynamic addServlet(String servletName, Servlet servlet) {
> -            Dynamic dynamic = super.addServlet(servletName, servlet);
> -            if (!webXmlProcessed) {
> -                return dynamic;
> -            }
> -            if 
> (webSecurityConstraintStore.isContainerCreatedDynamicServlet(servlet)) {
> -                
> webSecurityConstraintStore.addContainerCreatedDynamicServletEntry(servletName,
>  servlet.getClass().getName());
> -            }
> -            return 
> createGeronimoApplicationServletRegistrationAdapter(dynamic, servletName);
> -        }
> -
> -        @Override
> -        public Dynamic addServlet(String servletName, String className) {
> -            Dynamic dynamic = super.addServlet(servletName, className);
> -            if (!webXmlProcessed) {
> -                return dynamic;
> -            }
> -            
> webSecurityConstraintStore.addContainerCreatedDynamicServletEntry(servletName,
>  className);
> -            return 
> createGeronimoApplicationServletRegistrationAdapter(dynamic, servletName);
> -        }
> -
> -        @Override
> -        public void declareRoles(String... roles) {
> -            if (!isStarting())
> -                throw new IllegalStateException();
> -            if (!_enabled)
> -                throw new UnsupportedOperationException();
> -            webSecurityConstraintStore.declareRoles(roles);
> -        }
> -
> -        protected Dynamic 
> createGeronimoApplicationServletRegistrationAdapter(Dynamic 
> applicationServletRegistration, String servletName) {
> -            if (applicationServletRegistration == null) {
> -                return null;
> -            }
> -            return new 
> GeronimoApplicationServletRegistrationAdapter(GeronimoWebAppContext.this, 
> applicationServletRegistration);
> -        }
> -
> -        public WebSecurityConstraintStore getWebSecurityConstraintStore() {
> -            return webSecurityConstraintStore;
> -        }
> -
> -        public void setWebSecurityConstraintStore(WebSecurityConstraintStore 
> webSecurityConstraintStore) {
> -            this.webSecurityConstraintStore = webSecurityConstraintStore;
> -        }
> -
> -        @Override
> -        public <T extends Servlet> T createServlet(Class<T> c) throws 
> ServletException {
> -            try {
>                 T servlet = (T) 
> integrationContext.getHolder().newInstance(c.getName(), classLoader, 
> integrationContext.getComponentContext());
> -                if (isStarting()) {
> -                    
> webSecurityConstraintStore.addContainerCreatedDynamicServlet(servlet);
> -                }
> +                
> webSecurityConstraintStore.addContainerCreatedDynamicServlet(servlet);
>                 return servlet;
>             } catch (IllegalAccessException e) {
>                 throw new ServletException("Could not create servlet " + 
> c.getName(), e);
> @@ -369,5 +349,7 @@ public class GeronimoWebAppContext exten
>                 throw new ServletException("Could not create servlet " + 
> c.getName(), e);
>             }
>         }
> +
>     }
> +
>  }
> 
> Modified: 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/JaccSecurityHandler.java
> URL: 
> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/JaccSecurityHandler.java?rev=1001159&r1=1001158&r2=1001159&view=diff
> ==============================================================================
> --- 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/JaccSecurityHandler.java
>  (original)
> +++ 
> geronimo/server/trunk/plugins/jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/handler/JaccSecurityHandler.java
>  Sat Sep 25 07:59:39 2010
> @@ -19,11 +19,15 @@ package org.apache.geronimo.jetty8.handl
>  import java.io.IOException;
>  import java.security.AccessControlContext;
>  import java.security.AccessControlException;
> +import java.util.Collections;
> +import java.util.Set;
> 
>  import javax.security.jacc.PolicyContext;
>  import javax.security.jacc.WebResourcePermission;
>  import javax.security.jacc.WebUserDataPermission;
>  import javax.servlet.ServletException;
> +import javax.servlet.ServletRegistration;
> +import javax.servlet.ServletSecurityElement;
>  import javax.servlet.http.HttpServletRequest;
>  import javax.servlet.http.HttpServletResponse;
> 
> @@ -71,6 +75,7 @@ public class JaccSecurityHandler extends
>      *      javax.servlet.http.HttpServletRequest,
>      *      javax.servlet.http.HttpServletResponse, int)
>      */
> +    @Override
>     public void handle(String target, Request baseRequest, HttpServletRequest 
> request,
>                        HttpServletResponse response) throws IOException,
>             ServletException {
> @@ -89,10 +94,12 @@ public class JaccSecurityHandler extends
>         }
>     }
> 
> +    @Override
>     protected Object prepareConstraintInfo(String pathInContext, Request 
> request) {
>         return null;
>     }
> 
> +    @Override
>     protected boolean checkUserDataPermissions(String pathInContext, Request 
> request, Response response, Object constraintInfo) throws IOException {
>         boolean notIntegral = request.isSecure() || 
> !request.getConnection().isIntegral(request);
> 
> @@ -122,10 +129,12 @@ public class JaccSecurityHandler extends
>         return result;
>     }
> 
> +    @Override
>     protected boolean isAuthMandatory(Request base_request, Response 
> base_response, Object constraintInfo) {
>         return !checkWebResourcePermission(base_request, defaultAcc);
>     }
> 
> +    @Override
>     protected boolean checkWebResourcePermissions(String pathInContext, 
> Request request, Response response, Object constraintInfo, UserIdentity 
> userIdentity) throws IOException {
>         if (!(userIdentity instanceof GeronimoUserIdentity)){
>             //we already checked against default_acc and got false
> 
> 
> 
> 
> 
> -- 
> Ivan

Reply via email to