Expected Behavior I can extend and replace default Authentication Filter

Current Behavior https://stackoverflow.com/questions/23817524/how-to-replace-usernamepasswordauthenticationfilter-in-spring-security

Context https://github.com/spring-projects/spring-security/pull/10114

Comment From: eleftherias

Hi @quaff, as mentioned in the associated pull request, you can add a custom filter using

http
    .addFilter(new RestfulUsernamePasswordAuthenticationFilter(...));

Since RestfulUsernamePasswordAuthenticationFilter extends UsernamePasswordAuthenticationFilter it will be added to the filter chain in the same position as UsernamePasswordAuthenticationFilter.

If I understand correctly, the issue is that you are seeing both RestfulUsernamePasswordAuthenticationFilter and UsernamePasswordAuthenticationFilter in the filter chain.

As mentioned in this answer to the linked Stack Overflow question, this behaviour is likely because you are configuring form login.

Configuring http.formLogin() will add a UsernamePasswordAuthenticationFilter.

Are you configuring formLogin() in your application? Does it solve your issue if you remove formLogin() and add the custom filter using addFilter?

Comment From: quaff

Yes I need formLogin, and I want extend UsernamePasswordAuthenticationFilter to accept username and password from request body if content-type is json rather than request parameters.

Comment From: quaff

10114 is closed, should I resubmit PR?

Comment From: eleftherias

@quaff What additional functionality is .formLogin() providing in your application?

Comment From: quaff

@quaff What additional functionality is .formLogin() providing in your application?

http.formLogin()
.authenticationFilter(new RestfulUsernamePasswordAuthenticationFilter(authenticationManager(), objectMapper))) // currently not supported
.loginPage(loginPage)
.loginProcessingUrl(loginProcessingUrl)
.successHandler(authenticationSuccessHandler(http.getSharedObject(RequestCache.class)))
.failureHandler(authenticationFailureHandler())

Comment From: eleftherias

Thanks for the details @quaff. The additional attributes can be configured in the custom filter or in HttpSecurity. It is not necessary to configure .formLogin().

Your configuration may look something like this

http
    .addFilter(getCustomFilter())
    .exceptionHandling((exceptionHandling) -> exceptionHandling
        .authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/login")) // sets the loginPage
    )
    ...

RestfulUsernamePasswordAuthenticationFilter getCustomFilter() {
    RestfulUsernamePasswordAuthenticationFilter filter = new RestfulUsernamePasswordAuthenticationFilter(...);
    filter.setRequiresAuthenticationRequestMatcher(new AntPathRequestMatcher("/login", "POST")); // sets the loginProcessingUrl
    filter.setAuthenticationSuccessHandler(customAuthenticationSuccessHandler());
    filter.setAuthenticationFailureHandler(customAuthenticationFailureHandler());
    return filter;
}

Does that give you the functionality you are looking for?

Comment From: quaff

Thanks for the details @quaff. The additional attributes can be configured in the custom filter or in HttpSecurity. It is not necessary to configure .formLogin().

Your configuration may look something like this

```java http .addFilter(getCustomFilter()) .exceptionHandling((exceptionHandling) -> exceptionHandling .authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/login")) // sets the loginPage ) ...

RestfulUsernamePasswordAuthenticationFilter getCustomFilter() { RestfulUsernamePasswordAuthenticationFilter filter = new RestfulUsernamePasswordAuthenticationFilter(...); filter.setRequiresAuthenticationRequestMatcher(new AntPathRequestMatcher("/login", "POST")); // sets the loginProcessingUrl filter.setAuthenticationSuccessHandler(customAuthenticationSuccessHandler()); filter.setAuthenticationFailureHandler(customAuthenticationFailureHandler()); return filter; } ```

Does that give you the functionality you are looking for?

I think it should works, but it's not graceful, people cannot reuse the regular code style which already ubiquitous on web. This PR is very simple and safe, it provide convenience and keep the same experience as before.

Comment From: quaff

@eleftherias I tried your workaround, It break test caused by missing function provide by formLogin. https://github.com/spring-projects/spring-security/blob/006b9b960797d279b31cf8c8d16f1549c5632b2c/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java#L242-L249

here is my code snippet.

http.exceptionHandling()
.defaultAuthenticationEntryPointFor(new Http403ForbiddenEntryPoint(), RequestUtils::isRequestedFromApi)
.authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint(loginPage));

EDITED: I fix it by

http.exceptionHandling()
.defaultAuthenticationEntryPointFor(new Http403ForbiddenEntryPoint(), RequestUtils::isRequestedFromApi)
.authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint(loginPage), AnyRequestMatcher.INSTANCE);

It's tricky and not robust.

Comment From: eleftherias

This PR is very simple and safe, it provide convenience and keep the same experience as before.

While it may seem like a simple change, it would create numerous complications. One example is what is the expected behaviour if you configure both filter.setAuthenticationSuccessHandler(…) and http.formLogin().successHandler(…)?

Furthermore, the purpose of the FormLoginConfigurer is to add form based authentication by creating a UsernamePasswordAuthenticationFilter. The http.addFilter(…) was created for scenarios like these, that require a custom filter.

If you do not want to use http.addFilter(…), you could instead create a custom DSL modelled after FormLoginConfigurer or post-process the filter created using formLogin().

Given that the desired functionality is already possible with the current DSL, I'm going to close this issue.

Comment From: quaff

post-process

Thanks for telling me the wonderful custom DSL, it works excepts one thing, class FormLoginConfigurer is final cannot be extended

public class MyCustomDsl extends FormLoginConfigurer<HttpSecurity> {
    public FormLoginConfigurer<HttpSecurity> authenticationFilter(
            UsernamePasswordAuthenticationFilter authenticationFilter) {
        super.setAuthenticationFilter(authenticationFilter);
        return this;
    }
}

otherwise it will works like a charm

http.apply(new MyCustomDsl()) // instead if formLogin()
.authenticationFilter(new RestfulUsernamePasswordAuthenticationFilter(authenticationManager(), objectMapper))) // currently not supported
.loginPage(loginPage)
.loginProcessingUrl(loginProcessingUrl)
.successHandler(authenticationSuccessHandler(http.getSharedObject(RequestCache.class)))
.failureHandler(authenticationFailureHandler())

what is the concern make those classes final, and would you consider remove it? @eleftherias

Comment From: eleftherias

@quaff It looks like most of the functionality you are using comes from AbstractAuthenticationFilterConfigurer. I would encourage you to extend that class instead.

Comment From: quaff

@quaff It looks like most of the functionality you are using comes from AbstractAuthenticationFilterConfigurer. I would encourage you to extend that class instead.

That's true for now, what if I need some functionality from FormLoginConfigurer later?

 public class CustomFormLoginConfigurer<H extends HttpSecurityBuilder<H>> extends
        AbstractAuthenticationFilterConfigurer<H, CustomFormLoginConfigurer<H>, UsernamePasswordAuthenticationFilter> {

    public CustomFormLoginConfigurer<H> authenticationFilter(
            UsernamePasswordAuthenticationFilter authenticationFilter) {
        super.setAuthenticationFilter(authenticationFilter);
        return this;
    }
    // createLoginProcessingUrlMatcher MUST implemented here
    // other methods should be copied from FormLoginConfigurer
}

It's not graceful either.

Comment From: PavelTurk

Furthermore, the purpose of the FormLoginConfigurer is to add form based authentication by creating a UsernamePasswordAuthenticationFilter. The http.addFilter(…) was created for scenarios like these, that require a custom filter.

.... If you do not want to use http.addFilter(…), you could instead create a custom DSL modelled after FormLoginConfigurer or post-process the filter created using formLogin().

I don't think that these assumptions are correct. If we add not UsernamePasswordAuthenticationFilter, but SomeFilter that extendsUsernamePasswordAuthenticationFilter' then we don't need to use jc-custom-dsls or post-processing-configured-objects.

Why? Because it is a simple inheritance and polymorphism. If I want to change some behavior using polymorphism do I need to do many other things when it is enough to replace one class/instance with another??? That doesn't have sense at all.

This issue must be reopened as @eleftherias didn't get the idea of it.

Comment From: PavelTurk

@jzheaux could you evaluate the following code, suggested by @quaff? I think it is a good solution.

http.formLogin()
   .authenticationFilter(new RestfulUsernamePasswordAuthenticationFilter(authenticationManager(), objectMapper))) // currently not supported
....

Comment From: apryamostanov

I call it undoubtfully bad architecture, and agree with Pavel Turk. I've a simple task of adding OTP code to my login. How should I do it without writing custom code or using workarounds? It's compromising security.

Comment From: quaff

Why? Because it is a simple inheritance and polymorphism. If I want to change some behavior using polymorphism do I need to do many other things when it is enough to replace one class/instance with another??? That doesn't have sense at all.

Totally agreed, @eleftherias @jzheaux Please rethink.

Comment From: Jankbyte

Hi! Will you improve this features (written in up) for replacing filters? I want create my custom starter that replace UsernamePasswordAuthenticationFilter (i want do OTP authentication) that creates authenticated session every time. Or can add event for UsernamePasswordAuthenticationFilter that allows to implement custom logic BEFORE save authentication in SecurityContext?

Comment From: quaff

@jzheaux Please reconsider, it's a trivial enhancement but very useful and typesafe. I've rebase the PR to latest commit: https://github.com/spring-projects/spring-security/compare/main...quaff:spring-security:cust-auth-filter

Comment From: vatisteve

Hi there,

I'm following this issue as I'm encountering a similar problem in my project. I was wondering if there have been any recent updates or progress on resolving this issue?