8 Replies Latest reply: Feb 7, 2011 11:10 AM by Lars Huber RSS

Resteasy - destroy session after request - serious bug?

Lars Huber Newbie

Resteasy can be configured to destroy the websession right after the request (default behaviour). In few circumstances the session can't be destroyed anymore. Example is if using basic authentication you can access the previous authenticated session even if giving wrong credentials in request. This can end up in serious security issues.


Current code - 2.2.1.CR3 - org.jboss.seam.resteasy.ResteasyResourceAdapter.java


public void getResource(...) {
    ...
    dispatcher.invoke(in, theResponse);

    // Prevent anemic sessions clog up the server
    if (request.getSession().isNew()
        && application.isDestroySessionAfterRequest()
        && !Session.instance().isInvalid())
    {
        log.debug("Destroying HttpSession after REST request");
        Session.instance().invalidate();
    }
    
    ...
}



Take in account that destroying the session is not placed in finally block.


Scenario:
1. You miss giving the credentials on first service call. AuthenticationFilter throws an exception. From this point, the session.isNew() will always return false but the code in ResteasyResourceAdapter to destroy the session was never reached. Looking at the condition if destroying the session shows, the session will never be destroyed due to the NOT NEW session.


2. Authentication succeeded or even no authentication required. The service throws an exception and the code to invalidate the session is skipped. From now on the old story: the session is not new anymore, never entering the responsible code block.


Personal intepretation:
If resteasy is configured to destroy the session, the session should be invalidated in any case, coming to following code and workaround:




  1. override the Adapter by your own implementation. Unfortunately you have to mention: @Install(precedence = Install.DEPLOYMENT) due the default implementation is already Install.APPLICATION and not Install.FRAMEWORK

  2. override getResource(...)

  3. put responsible code in try finally block as following




   public void getResource(final HttpServletRequest request, final HttpServletResponse response) throws ServletException, IOException {

      try {
         log.debug("processing REST request");

         // TODO: As far as I can tell from tracing RE code: All this thread-local stuff has no effect because
         // the "default" provider factory is always used. But we do it anyway, just to mimic the servlet handler
         // in RE...

         // Wrap in RESTEasy thread-local factory handling
         ThreadLocalResteasyProviderFactory.push(dispatcher.getProviderFactory());

         // Wrap in RESTEasy contexts (this also puts stuff in a thread-local)
         SeamResteasyProviderFactory.pushContext(HttpServletRequest.class, request);
         SeamResteasyProviderFactory.pushContext(HttpServletResponse.class, response);
         SeamResteasyProviderFactory.pushContext(SecurityContext.class, new ServletSecurityContext(request));

         // Wrap in Seam contexts
         new ContextualHttpServletRequest(request) {
            @Override
            public void process() throws ServletException, IOException {
               try {
                  HttpHeaders headers = ServletUtil.extractHttpHeaders(request);
                  UriInfoImpl uriInfo = extractUriInfo(request, application.getResourcePathPrefix());

                  HttpResponse theResponse = new HttpServletResponseWrapper(response, dispatcher.getProviderFactory());

                  // TODO: This requires a SynchronousDispatcher
                  HttpRequest in = new HttpServletInputMessage(request, theResponse, headers, uriInfo, request.getMethod().toUpperCase(),
                        (SynchronousDispatcher) dispatcher);

                 dispatcher.invoke(in, theResponse);
               } finally {
                  // Just take in account if configured to destroy on each request
                  if (application.isDestroySessionAfterRequest()) {
                     log.debug("Destroying HttpSession after REST request");
                     Session.instance().invalidate();
                  }
               }
            }
         }.run();

      } finally {
         // Clean up the thread-locals
         SeamResteasyProviderFactory.clearContextData();
         ThreadLocalResteasyProviderFactory.pop();
         log.debug("completed processing of REST request");
      }
   }



Is it a bug or what is the reason for this kind of implementation.


greets
Lars

  • 2. Re: Resteasy - destroy session after request - serious bug?
    David Beaumont Newbie

    I think this might have broken my application!


    2.2.1 CR2 was fine. In 2.2.1 Final if you browse to a page which makes a request to resteasy after loading (in my case for a chart) the session is destroyed. If the user submits any forms on that page they will then get an exception because the view has expired. If they browse anywhere else they will find they have also been logged out.


    Adding:


    <resteasy:application destroy-session-after-request="false"/>
    



    isn't an option because then all rest requests will cause sessions to be created.


    Is there a problem with the code that detects whether there is an existing session?

  • 3. Re: Resteasy - destroy session after request - serious bug?
    Jozef Hartinger Master

    David,


    looks like I accidentally created a regressions when fixing the previous issue. I've opened JBSEAM-4775 to address the problem.

  • 4. Re: Resteasy - destroy session after request - serious bug?
    Lars Huber Newbie

    David, you are right. Using resteasy with authenticated user will not work. In the ResourceAdapter it is essential to test on session.isNew(). Inside the try finally block it will at least work for exceptions thrown by the resteasy service itself. But there is one problem left: session.isNew() will not work if resteasy service is authentication restricted and there are no or wrong credentials on first call. This means the ResteasyResourceAdapter will never be invoked to be able to destroy the session. So there must be another way to know if session must be destroyed directly on first failing call. This might be a flag in session scope who the session created, probabely if resteasy with destroy-session was the creator.
    There should be a third option for destroy-session-after-request : always, never, ifCreated .

  • 5. Re: Resteasy - destroy session after request - serious bug?
    Lars Huber Newbie

    David, are you sure the option


    <resteasy:application destroy-session-after-request="false"/>
    



    does not work for you? Requests shouldn't always create a new session. Have you tried?


    @Jozef: I'm sorry for not having provided a solution for all use cases.

  • 6. Re: Resteasy - destroy session after request - serious bug?
    David Beaumont Newbie

    Hi Lars,


    I think I misread the documentation then. I have added the snippet above to my components.xml and the user is no longer logged out when they view pages which make rest requests.


    It's obviously very important that not every request to my rest services results in a long lasting session (10 minutes in my app) being created. Is there an easy way to test whether this is happening?


    Thanks,

    David

  • 7. Re: Resteasy - destroy session after request - serious bug?
    David Beaumont Newbie

    I should also add that none of the rest services are authenticated in any way and the application as a whole has forms based login (not HTTP basic). Not sure if this is important to the code above.


    David

  • 8. Re: Resteasy - destroy session after request - serious bug?
    Lars Huber Newbie

    Hi David


    You can use 2.2.1.FINAL . Just override the ResteasyResourceAdapter with old implementation.


    @Name("org.jboss.seam.resteasy.resourceAdapter")
    @BypassInterceptors
    @Install(precedence = Install.DEPLOYMENT) // essential because seam's framework impl does not declare precedence=BUILT_IN
    public class FixedResteasyResourceAdapter extends org.jboss.seam.resteasy.ResteasyResourceAdapter {
    
       @Logger
       Log log;
    
       @Override
       public void getResource(final HttpServletRequest request, final HttpServletResponse response)
             throws ServletException, IOException
       {
    
          try
          {
             log.debug("processing REST request");
    
             // TODO: As far as I can tell from tracing RE code: All this thread-local stuff has no effect because
             // the "default" provider factory is always used. But we do it anyway, just to mimic the servlet handler
             // in RE...
    
             // Wrap in RESTEasy thread-local factory handling
             ThreadLocalResteasyProviderFactory.push(dispatcher.getProviderFactory());
    
             // Wrap in RESTEasy contexts (this also puts stuff in a thread-local)
             SeamResteasyProviderFactory.pushContext(HttpServletRequest.class, request);
             SeamResteasyProviderFactory.pushContext(HttpServletResponse.class, response);
             SeamResteasyProviderFactory.pushContext(SecurityContext.class, new ServletSecurityContext(request));
    
             // Wrap in Seam contexts
             new ContextualHttpServletRequest(request)
             {
                @Override
                public void process() throws ServletException, IOException
                {
    
                   HttpHeaders headers = ServletUtil.extractHttpHeaders(request);
                   UriInfoImpl uriInfo = extractUriInfo(request, application.getResourcePathPrefix());
    
                   HttpResponse theResponse = new HttpServletResponseWrapper(
                         response,
                         dispatcher.getProviderFactory()
                   );
    
                   // TODO: This requires a SynchronousDispatcher
                   HttpRequest in = new HttpServletInputMessage(
                         request,
                         theResponse,
                         headers,
                         uriInfo,
                         request.getMethod().toUpperCase(),
                         (SynchronousDispatcher) dispatcher
                   );
    
                   dispatcher.invoke(in, theResponse);
    
                   // Prevent anemic sessions clog up the server
                   if (request.getSession().isNew()
                         && application.isDestroySessionAfterRequest()
                         && !Session.instance().isInvalid())
                   {
                      log.debug("Destroying HttpSession after REST request");
                      Session.instance().invalidate();
                   }
                }
             }.run();
    
          }
          finally
          {
             // Clean up the thread-locals
             SeamResteasyProviderFactory.clearContextData();
             ThreadLocalResteasyProviderFactory.pop();
             log.debug("completed processing of REST request");
          }
       }
    }