DelegatingSecurityContextTaskExecutor / DelegatingSecurityContextRunnable / DelegatingSecurityContextCallable should provide extension points

[tkrah]

Hi,

I am using the DelegatingSecurityContextTaskExecutor and the DelegatingSecurityContextRunnable / Callable stack in a custom ForkJoinPool to make sure, my SecurityContext is transported on the various scheduled / submitted Tasks in the selected pool.

That works find so far but I need to hook those wrappers so I can modify the execution environment to do additional tasks before the runnable / callable is called (like the setup of the context does) and to cleanup things after the original runnable / callable comes back.

At the moment as the code is written (package protected, private, final stuff) I need to / I am forced to copy the whole code for my custom ForkJoinPool and TaskExecutor and provide also custom wrapper classes to meet that requirement, because the whole code written now, does not have a strong focus on being extended / customized / enriched.

It would be nice if you could just provide e.g. protected methods / wrappers or some functional interfaces for setup / finished callbacks I could provide when creating the pool so that I can just use your classes, enriched by my provided callbacks which are called on the correct time in the lifecycle, so that I am not forced to mimic the context saving / restoring behavior and have my addon glue executed too.

Thanks for considering.

[jzheaux]

@tkrah, thanks for the suggestion.

DelegatingSecurityContext executors can each take a delegate executor in its constructor, so you can do things like this:

Executor base = ExecutorService...
TaskExecutor executor = new TaskExecutorAdapter(base);
executor.setTaskDecorator((runnable) -> {
    // ... do additional tasks
    runnable.run();
    // ... do clean up 
});
DelegatingSecurityContextTaskExecutor security = new DelegatingSecurityContextTaskExceutor(executor);

Does that meet your needs?

[tkrah]

Hi,

that's a nice suggestion, totally did miss that.
The Docs specify that the decorator runs against the possible wrapped runnable, so it should have access to the security context stuff, need to test it but it should be sufficient for that part.

But I also use the DelegatingSecurityContextCallable and Runnable classes as a wrapper for a custom ForkJoinPool to wrap submitted tasks there too, the TaskDecorator does not help here - any idea how to handle this with the current code or does those need some actual work to reuse them there too?

[jzheaux]

If you already have a custom ForkJoinPool, I wonder if you can do the wrapping yourself:

// ...

@Override 
public void execute(Runnable task) {
    // ... do additional tasks
    task.run();
    // ... do clean up
}

// ...

perhaps you could reuse the TaskDecorator that you create:

// ...

@Override 
public void execute(Runnable task) {
    this.decorator.decorate(task).run();
}

// ...

[tkrah]

Had time to look at the TaskDecorator - while it does not handle Callables well (documented on the setTaskDecorator setter) it does not work for my usecase, or at least I miss how to integrate it.

TaskExecutorAdapter adapter = new TaskExecutorAdapter(taskExecutor);
adapter.setTaskDecorator(new MyTaskDecorator(applicationContext));
this.myTaskExecutor = new DelegatingSecurityContextTaskExecutor(adapter);

Decorator:

public Runnable decorate(Runnable runnable) {
		return () -> {
			setup();
			try {
				runnable.run();
			} finally {
				clearScope();
			}
		};
	};

When I run that, the runnable provided is a DelegatingSecurityContextRunnable but that one did not run yet which of cause my code does need, because it needs access to the delegated context here and SecurityContextHolder.getContext() is not ready yet.
The docs document that with:

Note that such a decorator is not necessarily being applied to the user-supplied Runnable/Callable but rather to the actual execution callback (which may be a wrapper around the user-supplied task).

and here is the problem for me.
I would need a decorator, which transparently decorates the user supplied Runnable / Callable and not the wrapper provided, because I need the Wrapper, which is the DelegatingSecurityContextRunnable and does the setup / cleanup of the context to run before mine.

At the moment I am achieving this by duplicating the DelegatingSecurityContextAsyncTaskExecutor code and in the submit / execute methods there is this:

        protected final Runnable wrap(Runnable delegate) {
		return DelegatingSecurityContextRunnable.create(delegate, this.securityContext,
				this.securityContextHolderStrategy);
	}

	protected final <T> Callable<T> wrap(Callable<T> delegate) {
		return DelegatingSecurityContextCallable.create(delegate, this.securityContext,
				this.securityContextHolderStrategy);
	}

And those are protected final, if they were not final I could provide my own wrappers by overriding them but as they are final I have to copy and modify the whole class hierarchy to provide my wrappers.
And here we are at the tickets proposal / question, how to better integrate a custom wrapper / decorator which does run on the user provided Runnable / Callable but after the SecurityContext code did run already.

I'll hope I did make my problem clear, if not just tell me, thanks.

[jzheaux]

Thanks, @tkrah, for the extra detail. I think at this point it would be ideal for you to share a minimal GitHub sample. I can go through and either provide a pull request to that repo or use the sample to see what kind of change is best to Spring Security. Can you please provide a minimal sample?

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[tkrah]

Ok, I'll try to provide an example, may take a little bit time though.