Martino Piccinato (Migrated from SEC-745) said:
I propose to introduce a mechanism similar to the one introduced with SEC-516 to give the possibility to resolve dinamically:
Different implementations of TargetUrlResolver interface could be used for default cases leaving open the possiblity to use more complex strategies.
My case with this (and with previous SEC-516) is providing different themes/behaviours to different organizations/departments using the same cas single sign on system. Now I have to use a custom implementation of AuthenticationEntryPoint and a customer Filter that just copy 90% of the current spring security class code but add my desired behaviour.
If you accept this I could provide a patch against latest trunk.
Luke Taylor said:
I think it would make sense to refactor TargetUrlResolver and remove the SavedRequest argument (which is available from the session anyway). Then, as you say, this interface could be used as a strategy across multiple parts of the framework, not just for authentication success URLs. This is quite a major change though and would have to be done in a future version. It would also be nice to simplify AbstractProcessingFilter to move most of the logic for working out the URL into the strategy. There is a lot of historical code in there – properties, protected methods and now a strategy. This could be made a lot simpler.
This strategy could also encapsulate the redirect/forward logic rather than just supplying a URL.
I’ve introduced new strategies:
void onSuccessfulAuthentication(HttpServletRequest, HttpServletResponse, Authentication authentication);
for successful login, and
void onUnsuccessfulAuthentication(HttpServletRequest, HttpServletResponse, AuthenticationException exception);
for error handling.
We already have AccessDeniedHandler and LogoutHandler, so this makes for a more standard approach. The existing LogoutHandler interface could also be reused for determining the logout destination and is also used in ConcurrentSessionFilter.
I don’t really see much purpose in introducing an additional strategy for entry points since the entry point is itself a strategy already.
LogoutHandler isn’t really suitable for this purpose as it shouldn’t really throw any exceptions, whereas a redirect/forward strategy needs to have IOException,ServletException. I’ve added a LogoutSuccessHandler strategy. Since the is essentially the same as for an AuthenticationSuccessHandler, I’ve refactored the existing implementations into a common base class AbstractAuthenticationTargetUrlRequestHandler for consistency.
This issue supersedes #1358