You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@geronimo.apache.org by Ivan <xh...@gmail.com> on 2010/09/25 10:30:20 UTC

Re: svn commit: r1001159 - in /geronimo/server/trunk/plugins: j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/

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.
    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.
    Thanks.

2010/9/25 <dj...@apache.org>

> 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

Re: svn commit: r1001159 - in /geronimo/server/trunk/plugins: j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/

Posted by David Jencks <da...@yahoo.com>.

On Sep 26, 2010, at 12:19 AM, Ivan wrote:

> 
> 
> 2010/9/26 David Jencks <da...@yahoo.com>
> 
> On Sep 25, 2010, at 10:04 AM, David Jencks wrote:
> 
>> 
>> 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.  
> 
> I fixed jetty to  notify us when servlets are added dynamically and have a patch
> 
> https://issues.apache.org/jira/secure/attachment/12455591/GERONIMO-5624-2.patch
> 
> that I'll commit when a jetty snapshot is pushed.
>  
>     It is great that Jetty would provide a callback for it ;-)
>     From the patch file, seems that ServletHolder could not provide the servletName for dynamical added servlet, so a new class RegistrationKey is added ? If that is the case, guess that we may just use one map for holding the dynamic added security elements. Another thing is that, in Tomat assembly, the classes in the webXmlAppInfo might be ingored, might need to update the integration codes.


The RegistrationKey is so we can keep track of ServletRegistration.Dynamic as hashmap keys.  Then we don't need to go asking the ServletContext for the registration again.

There are 2 implementations in the WebSecurityConstraintStore at the moment.  I'm planning to move the tomcat one over to the new methods I added and try to reduce the wrapping of tomcat components.  I think there are errors in the native tomcat implementation as well.

thanks
david jencks

> 
>> 
>>>     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'm still thinking about this.  I think that jetty can keep using this even if it sometimes throws UnsupportedOperationException.  The conditions for this are just the same as for all the addServlet methods which are already enforcing this.
> 
>    If the TCK does not complain this, it may be fine. 
>  
> 
> thanks
> david jencks
> 
>> 
>> I will work on both of these.... thanks for bringing them up.
>> 
>> david jencks
>> 
>>>     Thanks.
>>> 
>>> 2010/9/25 <dj...@apache.org>
>>> 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
>> 
> 
> 
> 
> 
> -- 
> Ivan

Re: svn commit: r1001159 - in /geronimo/server/trunk/plugins: j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/

Posted by Ivan <xh...@gmail.com>.

2010/9/26 David Jencks <da...@yahoo.com>

>
> On Sep 25, 2010, at 10:04 AM, David Jencks wrote:
>
>
> 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.
>
>
> I fixed jetty to  notify us when servlets are added dynamically and have a
> patch
>
>
> https://issues.apache.org/jira/secure/attachment/12455591/GERONIMO-5624-2.patch
>
> that I'll commit when a jetty snapshot is pushed.
>

    It is great that Jetty would provide a callback for it ;-)
    From the patch file, seems that ServletHolder could not provide the
servletName for dynamical added servlet, so a new class RegistrationKey is
added ? If that is the case, guess that we may just use one map for holding
the dynamic added security elements. Another thing is that, in Tomat
assembly, the classes in the webXmlAppInfo might be ingored, might need to
update the integration codes.

>
>
>     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'm still thinking about this.  I think that jetty can keep using this even
> if it sometimes throws UnsupportedOperationException.  The conditions for
> this are just the same as for all the addServlet methods which are already
> enforcing this.
>

   If the TCK does not complain this, it may be fine.


>
> thanks
> david jencks
>
>
> I will work on both of these.... thanks for bringing them up.
>
> david jencks
>
>     Thanks.
>
> 2010/9/25 <dj...@apache.org>
>
>> 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
>
>
>
>


-- 
Ivan

Re: svn commit: r1001159 - in /geronimo/server/trunk/plugins: j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/

Posted by David Jencks <da...@yahoo.com>.

On Sep 25, 2010, at 10:04 AM, David Jencks wrote:

> 
> 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.  

I fixed jetty to  notify us when servlets are added dynamically and have a patch

https://issues.apache.org/jira/secure/attachment/12455591/GERONIMO-5624-2.patch

that I'll commit when a jetty snapshot is pushed.

> 
>>     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'm still thinking about this.  I think that jetty can keep using this even if it sometimes throws UnsupportedOperationException.  The conditions for this are just the same as for all the addServlet methods which are already enforcing this.

thanks
david jencks

> 
> I will work on both of these.... thanks for bringing them up.
> 
> david jencks
> 
>>     Thanks.
>> 
>> 2010/9/25 <dj...@apache.org>
>> 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
>

Re: svn commit: r1001159 - in /geronimo/server/trunk/plugins: j2ee/geronimo-web/src/main/java/org/apache/geronimo/web/security/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/jetty8/ jetty8/geronimo-jetty8/src/main/java/org/apache/geronimo/

Posted by David Jencks <da...@yahoo.com>.

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 <dj...@apache.org>
> 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