SEC-1457: LoginUrlAuthenticationEntryPoint should allow DI of RedirectStrategy #1697

Closed
spring-issuemaster opened this Issue Apr 8, 2010 · 5 comments

1 participant

@spring-issuemaster

Søren Boisen (Migrated from SEC-1457) said:

The RedirectStrategy interface presumably exists to enable customization of the way Spring Security implements redirects.

In classes like ConcurrentSessionFilter and SimpleUrlAuthenticationFailureHandler, it is possible to override the default strategy as embodied in DefaultRedirectStrategy, because they contain a public setRedirectStrategy method.

However LoginUrlAuthenticationEntryPoint does not contain such a method. This means that the use of DefaultRedirectStrategy is hardcoded into the class and not possible to override without subclassing.

I do not see a reason why LoginUrlAuthenticationEntryPoint should not allow customization of the RedirectStrategy. It looks like it was simply overlooked.

Therefore I would like to see a public setRedirectStrategy method added to this class. This should be trivial and not affect existing clients.

@spring-issuemaster

Luke Taylor said:

No, it wasn't overlooked. In fact there was an almost identical issue. Please see SEC-1302.

@spring-issuemaster

Søren Boisen said:

Gah, must have misspelled LoginUrlAuthenticationEntryPoint when I searched for it, sorry about that.

I understand your point about LUAEP already being a strategy. But I would still say that lack of ability to perform DI of the RedirectStrategy reduces the value of having it as a strategy interface at all.

@spring-issuemaster

Luke Taylor said:

I still disagree, because of the same limitations that are quoted in the other issue. Users may come unstuck because of the additional http/https redirect-related code in the commence method. If you need that code (which makes up the bulk of the class), then injecting the same strategy you use elsewhere may produce inconsistent results. If you don't need it, then implement AuthenticationEntryPoint directly (even in the same class as your RedirectStrategy implementation) since most of the existing implementation is unnecessary.

@spring-issuemaster

Ludovic Praud said:

We can DI a redirectStrategy into org.springframework.security.web.session.SessionManagementFilter.redirectStrategy
I have made a redirect strategy which use an URL param and so the request to retrieve it.

LoginUrlAuthenticationEntryPoint should also provide DI of redirectStrategy in order to allow URL param redirection.

It would be great if spring security authentication URL redirection would not be based on session but on request instead to be more RESTful and stateless.
I don't known yet if I will create a feature request issue about that.

@spring-issuemaster

Ludovic Praud said:

My bad, overriding org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint.determineUrlToUseForThisRequest(HttpServletRequest, HttpServletResponse, AuthenticationException) will do it.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment