Security Context has been nullified while using @Transactional attribute on Service method call about spring-data-couchbase HOT 5 OPEN

The default strategy for SecurityContextHolder is MODE_THREADLOCAL. The spring-data-implemenation of @transactional does not execute the method in the same thread as it was called from. Using MODE_GLOBAL will allow it to be retrieved from a different thread, but is not suitable for multi-user, multi-threaded applications.

I would suggested retrieving the authorization in the caller and passing it as an argument to the service.

@Service
class ResolverService{

SpaceService spaceService;

public boolean resolverA(Object someData){
     spaceService.spaceA(someData, SecurityContextHolder.getContext()); // <-- pass arg
     ....
    return true;
} 

}

@Service
@Transactional
class SpaceService {

public boolean spaceA(Object someData, SecurityContext ctx){ // <-- add as arg
     SecurityContextHolder.setContext(ctx);  // <--- set
     //Do stuff
    someInsideMethodCalls(someData);
     ....
    return true;
} 

}

from spring-data-couchbase.

hi @mikereiche
thanks for the response.
In any case - we cannot use GLOBAL in our case and we're not using LOCAL - we're using INHERITABLE (specifically for such cases). We did have this solution already, but it's seems to be not perfect - if tomorrow we'll need another context data(say, not security) passed by any other context to be passed - we'll need to add another parameter. In Additional, it means that each invocation for SpaceService service will need to pass such security context parameter - which making code much more complex and will be harder to support.

The spring-data-implemenation of @transactional does not execute the method in the same thread as it was called from.

This is clear and this is one of the reasons why we're using MODE_INHERITABLETHREADLOCAL. Is there any reason not to pass security context if we're using @Transactional? Or, at least to check strategy on the context - and decide if it should be removed and passed over (I don't have issue to remove it - if it's LOCAL, but MODE_INHERITABLETHREADLOCAL seems to be done for such cases)?

from spring-data-couchbase.

We did have this solution already, but it's seems to be not perfect - if tomorrow we'll need another context data(say, not security) passed by any other context to be passed - we'll need to add another parameter.

Applications typically have a single uber Context object that holds multiple objects and is passed as an argument to methods that need the context. As additional objects are needed, they are just added to the Context object, there is no need to add arguments. Passing as an argument also avoids any future problems where SomeFutureContextHolder will not work.

Is there any reason not to pass security context if we're using @transactional?

There is no reason not to pass it. It's just that thread-local mechanisms of SecureContextHolder don't work. SecureContextHolder does have means for you to provide a custom strategy.

Because @transactional uses reactor, it executes the method in a thread from .environment().transactionsSchedulers().schedulerBlocking(), it will not be in the same thread or a child-thread of the caller. There is no means for having transactionsSchedulers() to provide a custom implementation (that could perhaps provide a Scheduler with a thread which is a child of the current thread). The risk here would be that (a) the creator of the Cluster Environment would forget to provide a transactionSchedulers() to override the default; or (b) the transactionSchedulers() that they did provide did not give a schedulerBlocking() that was a child of the thread that set the SecureContextHandler(); or (c) the schedulerBlocking() would leak schedulers.

The mechanism for passing contextual data through a reactive chain is by writing it to the reactive context using contextWrite(). However - the call to a @transactional method is simply a method call - nothing from implementation is exposed, so there is no place to do that contextWrite() from the caller. Furthermore, the Couchbase Transaction call ReactiveTransaction.runBlocking(...) does not expose the context either. Even if a ReactiveTransaction.runBlockingWriteContext(...., ctx) was added, the caller would need to know what to what 'ctx' was. It could possibly be something analogous to SecurityContextHolder - such as TransactionContextHolder that uses ThreadLocal. ThreadLocal would work for this since it would be stored and retrieved in the same thread - before the execution of the reactor chain.

from spring-data-couchbase.

There is no reason not to pass it. It's just that thread-local mechanisms of SecureContextHolder don't work. SecureContextHolder does have means for you to provide a custom strategy.

we're using org.springframework.security.core.context.SecurityContextHolder#MODE_INHERITABLETHREADLOCAL . In such case - even if thread will not be the same thread as before - context should be passed to newly created thread from current one, isn't it?

see https://docs.spring.io/spring-security/site/docs/3.0.x/reference/technical-overview.html.

So, the question - why it's not working for MODE_INHERITABLETHREADLOCAL and will it be fixed?

from spring-data-couchbase.

In such case ... context should be passed to newly created thread from current one, isn't it?

That is correct, but it's not the case here. Please refer to my previous post where I provided all the details and all the options.

So, the question - why it's not working for MODE_INHERITABLETHREADLOCAL and will it be fixed?

You can file a ticket with spring-boot security for SecurityContextHolder, and they will tell you the same thing.

from spring-data-couchbase.