Adding support for SecurityContext Propagation [INT-2166]

[spring-operator]

Artem Bilan opened INT-2166 and commented

There are a lot of cases when it is necessary to use the SecurityContext in async mesage flow inside Web application. The principal is appeared in HttpRequest from springSecurityFilterChain and stored in the ThreadLocal holder. So, at first, I propose to add supoprt of some global ChannelInterceptor which can propagetes that securityContext to async mesasge flow. This is my solution. Sorry for Groovy code.

class SecurityContextPropagationChannelInterceptor extends ChannelInterceptorAdapter {
    @Override
    Message<?> preSend(Message<?> message, MessageChannel channel) {
        if (!(channel in AbstractPollableChannel) || message.headers.securityContext || !SecurityContextHolder.context.authentication) return message
        return MessageBuilder.fromMessage(message).with {
            setHeader 'securityContext', SecurityContextHolder.context
            build()
        }
    }

    @Override
    Message<?> postReceive(Message<?> message, MessageChannel channel) {
        if (channel in AbstractPollableChannel && message.headers.securityContext) {
            SecurityContextHolder.context = message.headers.securityContext
        }
        return message
    }

}

In this case the end-programmer can use

SecurityContextHolder.getContext()

As well as always in any place of his application.

---

Issue Links:

• 7009 SecurityContext is not propagated by asynchronous endpoints

  ("is duplicated by")

• SPR-11967 Add completion callbacks (regardless of failure) to ChannelInterceptor

Referenced from: pull request https://github.com/spring-projects/spring-integration/pull/1493

2 votes, 10 watchers

[spring-operator]

Artem Bilan commented

It was the first strategy. The second strategy is: to produce some custom header element inside header-enricher like correlation-id or reply-channel with some attribute fromSecurityContextHolder=true/false. Plus other abilities for header-enricher.

[spring-operator]

Gary Russell commented

Further to the conversation on #7009, there is clearly interest in propagating the security context in SI apps that use async handoffs. But, as Artem pointed out on the other JIRA, there could be undesirable side-effects of doing it unconditionally in the framework.

Therefore it would need to use an "opt-in" policy - such as an attribute on the \ and/or \ element of the channel.

There are other things to consider too...

1. The propagated context should probably be a clone of the original context rather than a reference - otherwise if it's mutated the results on the "other" thread would be indeterminate.
2. We'd need a mechanism to reliably clear the context on the 'new' thread, if it was set by propagation - perhaps using the ErrorHandlingTaskExecutor that wraps SI executors.
3. ...

I added Rob to the watch list in case he has any pearls of wisdom to add.

[spring-operator]

Mark Fisher commented

I added Luke as a watcher also since he and I discussed this way back in 2008, and at that time he raised enough issues to scare me from going down this path at all.

[spring-operator]

Rob Winch commented

I spoke with Gary a bit offline and it appears this is already being looked into, but one option might be to try to leverage the Async support that will be available in Spring Security 3.2. For those not familiar, please refer to the blog post about the support introduced in Spring Security 3.2 M1.

[spring-operator]

Damien Hollis commented

Has there been any progress on this issue? We are using spring integration in our project and we are keen to have security propagated across async calls - in our particular case we are using jms channels, so specific support for that use case would be appreciated. I'm happy to contribute (depending on how much work is involved) if somebody wants to point me in the right direction.

[spring-operator]

Artem Bilan commented

Hi, Damien!

Thank you for attention to this.

We still decide to keep out of this implementation in the framework.

What is a problem to use in your case suggested workaround in face of <channel-interceptor>?

[spring-operator]

Damien Hollis commented

Hi Artem,

The problem with \ is that I can setup the SecurityContext in postReceive but there is no opportunity to clean it up when the Message has been processed. This isn't the end of the world but I don't like the idea of leaving a SecurityContext bound to a ThreadLocal in case the thread gets used for some other processing that shouldn't have access to the SecurityContext.

Is there a solution to this problem that I'm not aware of?

Regards, Damien

[spring-operator]

Artem Bilan commented

Well, what you need is here:

1. <channel-interceptor> to propagate SecurityContext for non-direct channels
2. Wrap TaskExecutor for Executor Channels with DelegatingSecurityContextExecutor from Spring Security: https://spring.io/blog/2012/12/17/spring-security-3-2-m1-highlights-servlet-3-api-support
3. For Queue Channels you should implement some SecurityContextCleanInterceptor, because <poller> has the advice-chain attrbiute:

public class SecurityContextCleanInterceptor implements MethodInterceptor {
        public Object invoke(MethodInvocation invocation) throws Throwable {
        try {
            return invocation.proceed();
        }
        finally {
                        SecurityContextHolder.clearContext();
        }
    }
}

Is it reasonable for you?

[spring-operator]

Damien Hollis commented

Hi Artem,

I think your suggests would work for Executor and Queue Channels. However, I couldn't see how to make either work for a Jms Channel. So instead I implemented a MessageHandler and configured it at the beginning of a chain of handlers:

public class ContextPropagatingMessageHandler extends AbstractMessageHandler implements MessageProducer {

    private final MessagingTemplate messagingTemplate = new MessagingTemplate();

    private MessageChannel outputChannel;

    @Override
    public void setOutputChannel(MessageChannel outputChannel) {
        this.outputChannel = outputChannel;
    }

    @Override
    protected void handleMessageInternal(Message<?> message) throws Exception {
        SecurityContext securityContext = (SecurityContext) message.getHeaders().get(ContextPropagatingChannelInterceptor.SECURITY_CONTEXT_KEY);

        if (securityContext == null) {
            sendMessage(message);
        }
        else {
            try {
                SecurityContextHolder.setContext(securityContext);
                sendMessage(message);
            }
            finally {
                SecurityContextHolder.clearContext();
            }
        }
    }

    private void sendMessage(Message<?> message) {
        messagingTemplate.send(outputChannel, message);
    }
}

Can you recommend a better solution?

[spring-operator]

Artem Bilan commented

Hi, Damien!

Sorry for delay. However this issue is very specific and has a lot of places for the imagination ;) Instead of coding a MessageHandler you can build some gereric Advice and use it withing <request-handler-advice-chain>: http://docs.spring.io/spring-integration/docs/2.2.6.RELEASE/reference/html/messaging-endpoints-chapter.html#message-handler-advice-chain

And, as you noticed, provided solution isn't robust and it doesn't cover all use-cases. There is still a window for involving an end-user to achieve the real solution. That's why I said that it shouldn't be provided by framework by default. SecurityContext is very simple and even Serializable object and Srping Security provides enough tools to work with it in the application with Spring infrastructure.

Thanks

[spring-operator]

Christopher Smith commented

I would like to voice support for built-in (or at least packaged-in-an-extension) support for propagating the SecurityContext, as is found in Spring Remoting. While Spring Security does make the tools available to implement propagation, it's a fairly cookie-cutter requirement, and both DRY and security principles suggest it should be handled in a general-purpose library.

Even if there is more than one protocol for how to propagate the context (and I can only think of the one where the context follows the message), it seems that they should be limited enough to be enumerated and supported in-library. What other propagation protocol(s) do you think users would want?

[spring-operator]

Christopher Smith commented

For whatever it's worth, I've written an example project demonstrating the behavior of thread-local data under various combinations of channel types and synchronous/asynchronous invocation. I will note that as a client of a gateway interface, the differing behavior based on a runtime configuration option is surprising in a bad way, and a policy of "all information needs to be written into the API" without some sort of magic summoning-from-the-context like Spring MVC does with controller invocations means that every gateway interface needing security information will have to have its own wrapper that does nothing but propagate the context.

[spring-operator]

Dave Syer commented

I think the fact that it is boilerplate tells me we should have something in the framework. The main concern for me is the clean up. You want to be double sure that background threads do not have ThreadLocals with SecurityContext in them after a message has been processed.

[spring-operator]

Artem Bilan commented

Anyone who is interested, please, take a look to the PR on the matter: https://github.com/spring-projects/spring-integration/pull/1493