Uploaded image for project: 'atlassian-seraph'
  1. atlassian-seraph
  2. SER-144

Illegal cyclic dependency between LoginFilter and SecurityFilter

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: Medium
    • Resolution: Unresolved
    • Affects Version/s: 2.0.1
    • Fix Version/s: None
    • Labels:
      None
    • Last commented by user?:
      true

      Description

      There is an illegal cyclic dependency between LoginFilter and SecurityFilter which prevents seraph from filtering requests in containers that load filters lazily (e.g. GlassFish v3).

      The filter chain in a typical seraph application looks like this:

      ...
      LoginFilter
      ...
      SecurityFilter
      ...
      

      The issue with this ordering is that, the LoginFilter's doFilter method depends on availability of the SecurityConfig in the ServletContext and this object is put into the context by SecurityFilter's init method. This means that unless SecurityFilter.init method is called, LoginFilter.doFilter will throw NPE.

      If one tried to reorder the filter mapping so that the filter chain would look like this:

      ...
      SecurityFilter
      LoginFilter
      ...
      ...
      

      a new exception is thrown because SecurityFilter.doFilter depends on LoginFilter.doFilter.

      Now one might argue that this is not a problem with containers that eagerly load and initialize filters (e.g. Tomcat or GlassFish v2), but looking at the Servlet 2.5 spec, seraphs relies on an implementation detail of these containers rather than on the spec. The spec doesn't require containers to initialize all filters before any requests are served:

      SRV.6.2.1 Filter Lifecycle

      After deployment of the Web application, and before a request causes the container
      to access a Web resource, the container must locate the list of filters that must be
      applied to the Web resource as described below. The container must ensure that it
      has instantiated a filter of the appropriate class for each filter in the list, and called its
      init(FilterConfig config) method.

      Since the lifecycle of filters and servlets is very similar in nature, we can also look at the specification for servlet lifecycle which explicitely mentions the option to load servlets lazily:

      SRV.2.3.1 Loading and Instantiation
      The servlet container is responsible for loading and instantiating servlets. The load-
      ing and instantiation can occur when the container is started, or delayed until the
      container determines the servlet is needed to service a request.

      Therefore it is by belief that seraph's implementation is incompatible with the servlet spec and should be corrected.

      Ideally the SecurityConfig should be initialized and placed into the servlet context via a context listener. That would guarantee the proper initialization of seraph.

      An alternative hack to get things going until the issue is properly fixed is to patch the LoginFilter as follows:

      diff -r 9b55785d0f45 -r 47d893cb5d51 atlassian-seraph/src/main/java/com/atlassian/seraph/filter/BaseLoginFilter.java
      --- a/atlassian-seraph/src/main/java/com/atlassian/seraph/filter/BaseLoginFilter.java   Sat Dec 19 00:51:19 2009 -0800
      +++ b/atlassian-seraph/src/main/java/com/atlassian/seraph/filter/BaseLoginFilter.java   Sat Dec 19 19:38:03 2009 -0800
      @@ -60,6 +60,9 @@
           {
               // log.debug("LoginFilter.init");
               this.filterConfig = config;
      +
      +        //hack
      +        config.getServletContext().setAttribute(SecurityConfig.STORAGE_KEY, SecurityConfigFactory.getInstance(null));
           }
      
           public void destroy()
      

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              igorminar Igor Minar
              Participants:
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Last commented:
                8 years, 30 weeks, 2 days ago