Spring Security
  1. Spring Security
  2. SEC-1092

SpringContext is null then ConcurrentSessionFilter worked

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 M1
    • Component/s: Core
    • Labels:
      None

      Description

      ConcurrentSessionFilter don't fill SpringContext then invalidate the session. I think, it would be nice to place HttpSessionContextIntegrationFilter, to fill SpringContext, before ConcurrentSessionFilter.

        Activity

        Hide
        Nikita Koksharov added a comment -

        Sorry for mistake SecurityContext, not SpringContext.

        Show
        Nikita Koksharov added a comment - Sorry for mistake SecurityContext, not SpringContext.
        Hide
        Luke Taylor added a comment -

        It's not very clear from your title/description what the problem is.

        Could you please explain in detail why you think there is a bug, where it is in the code and if possible supply a test case?

        Show
        Luke Taylor added a comment - It's not very clear from your title/description what the problem is. Could you please explain in detail why you think there is a bug, where it is in the code and if possible supply a test case?
        Hide
        Nikita Koksharov added a comment -

        It's a bug because of i should have access to SecurityContext when HttpSession destroyed:

        public void onApplicationEvent(ApplicationEvent event) {
        if (!(event instanceof HttpSessionDestroyedEvent))

        { return; }

        SecurityContext context = SecurityContextHolder.getContext();
        ....
        }

        but with current version of Spring security i wrote:

        private void setupSecurityContext(HttpSession session) {
        if (SecurityContextHolder.getContext() != null
        && SecurityContextHolder.getContext().getAuthentication() != null) { return; }
        SecurityContext context =
        (SecurityContext) session.getAttribute(
        HttpSessionContextIntegrationFilter.SPRING_SECURITY_CONTEXT_KEY);
        if (context != null) { SecurityContextHolder.setContext(context); }
        }

        public void onApplicationEvent(ApplicationEvent event) {
        if (!(event instanceof HttpSessionDestroyedEvent)) { return; }

        HttpSessionDestroyedEvent httpEvent = (HttpSessionDestroyedEvent)event;
        setupSecurityContext(httpEvent.getSession());

        SecurityContext context = SecurityContextHolder.getContext();
        ....
        }

        Show
        Nikita Koksharov added a comment - It's a bug because of i should have access to SecurityContext when HttpSession destroyed: public void onApplicationEvent(ApplicationEvent event) { if (!(event instanceof HttpSessionDestroyedEvent)) { return; } SecurityContext context = SecurityContextHolder.getContext(); .... } but with current version of Spring security i wrote: private void setupSecurityContext(HttpSession session) { if (SecurityContextHolder.getContext() != null && SecurityContextHolder.getContext().getAuthentication() != null) { return; } SecurityContext context = (SecurityContext) session.getAttribute( HttpSessionContextIntegrationFilter.SPRING_SECURITY_CONTEXT_KEY); if (context != null) { SecurityContextHolder.setContext(context); } } public void onApplicationEvent(ApplicationEvent event) { if (!(event instanceof HttpSessionDestroyedEvent)) { return; } HttpSessionDestroyedEvent httpEvent = (HttpSessionDestroyedEvent)event; setupSecurityContext(httpEvent.getSession()); SecurityContext context = SecurityContextHolder.getContext(); .... }
        Hide
        Nikita Koksharov added a comment -

        ConcurrentSessionFilter drop concurrent session --> HttpSessionDestroyedEvent appear and current SecurityContext is null.

        Show
        Nikita Koksharov added a comment - ConcurrentSessionFilter drop concurrent session --> HttpSessionDestroyedEvent appear and current SecurityContext is null.
        Hide
        Luke Taylor added a comment -

        So you are essentially saying that you want to access the security context via the thread local rather than having to read it from the session?

        If so, that doesn't seem like a bug to me. The information you need is already available through the session and there isn't really any reason why an event listener should make assumptions about the context in which it's called (such as thread local information) - its job is to handle the information passed to it via the event object. The current filter ordering has been established for years now and I don't think the risk of side effects from changing it is justified here.

        The subject of session management within Spring Security should probably be revisited in the future as there are now may areas where it crops up (e.g. session fixation protection) and there may be future changes to concurrent session handling which may affect the filters, but I don't believe switching the order just to save a few lines of code is worthwhile.

        Show
        Luke Taylor added a comment - So you are essentially saying that you want to access the security context via the thread local rather than having to read it from the session? If so, that doesn't seem like a bug to me. The information you need is already available through the session and there isn't really any reason why an event listener should make assumptions about the context in which it's called (such as thread local information) - its job is to handle the information passed to it via the event object. The current filter ordering has been established for years now and I don't think the risk of side effects from changing it is justified here. The subject of session management within Spring Security should probably be revisited in the future as there are now may areas where it crops up (e.g. session fixation protection) and there may be future changes to concurrent session handling which may affect the filters, but I don't believe switching the order just to save a few lines of code is worthwhile.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Nikita Koksharov
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: