SecurityContext and Authentication behaviour in OAuthProviderProcessingFilter is incompatible with DelegatingSecurityContext* classes

[autayeu]

DelegatingSecurityContextCallable and DelegatingSecurityContextRunnable are useful to provide SecurityContext for a thread in a convenient manner. However, current behaviour of the OAuthProviderProcessingFilter prevents them being used as intended.

Namely, the Delegating* classes keep the context, while the provider replaces authentication in that specific context. That leads to access denied exceptions due to Authentication being replaced for the wrong one.

My understanding that for the purposes of keeping-restoring-clearing the security context the whole of SecurityContext should be used, not just authentication part of it.

Example:

1. Working thread schedules a separate thread to get some auxiliary work done.
2. Auxiliary thread inherits the security context by using DelegatingSecurityContext* classes
3. Working thread finishes the processing and in OAuthProviderProcessingFilter.resetPreviousAuthentication the authentication is restored to the previous one (often anonymous) and that changes both working and auxiliary thread.
4. Auxiliary thread has access control problems.

OAuthProviderProcessingFilter restores authentication directly, instead of restoring the context, and that leads to authentication being changed in the auxiliary thread as well, thus defeating the purpose of DelegatingSecurityContext*.

I would change the filter to save-restore-clear security context instead of just authentication. Would such a patch be considered for acceptance or current behaviour has other reasons behind it and the DelegatingSecurityContext* classes should have "siblings" doing DelegatingAuthentication* stuff in addition to context?

[dsyer]

OAuthProviderProcessingFilter is really old and due for a major overhaul if you ask me (but it's never been a high priority) - I would prefer if there wasn't even a filter (the approach in OAuth2 is different). But if you have an idea to improve it, please send a pull request.

[autayeu]

Can you sketch the idea of that "major overhaul" you have in mind?

[dsyer]

The sketch would be the OAuth2 half of this project: instead of relying on Filters for everything, there you will find a @FrameworkEndpoint annotation and a couple (otherwise) completely normal Spring MVC @RequestMappings for the OAuth2 features. That's the way I would go, if I had some time. But don't feel obligated to take that route if all you need is a way to make those Delegating* things behave as you need them.

[autayeu]

Thank you for pointers! Perhaps 2 patches then.

[autayeu]

I've submitted a pull request, please take a look. CLA is filed earlier, for the previous request.