Skip to content

Invalid OAuth2 login attempts don't emit a corresponding ApplicationEvent #7793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sdavids13 opened this issue Jan 7, 2020 · 8 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@sdavids13
Copy link

Summary

I would like to log requests for invalid attempts for authenticating via OAuth 2.0 bearer tokens (via JWT) for which I have ApplicationListeners for both AbstractAuthenticationEvent and AbstractAuthorizationEvent events which log failures as they are seen. Currently when presented with invalid JWT bearer tokens a 401 HTTP response is provided but no Application Listener events are fired that corresponds to the error response.

Actual Behavior

When submitting a request with a bad JWT bearer token the application responds with a 401 response code but no ApplicationListener event is fired that correlates to the bad response. Currently I see an error that says:

  "classname": "org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter",
  "method": "doFilterInternal",
  "file": "BearerTokenAuthenticationFilter.java",
  "line": 135,
  "logger_name": "org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter",
  "level": "DEBUG",
  "message": "Authentication request for failed!",
  "stack_trace": "org.springframework.security.oauth2.core.OAuth2AuthenticationException: An error occurred while attempting to decode the Jwt: Signed JWT rejected: Invalid signature
.......

Upon inspection of the BearerTokenAuthenticationFilter it seems to merely manipulate the HTTPServletResponse in the default implementation instead of producing any application events.

Expected Behavior

On malformed JWT bearer token error responses a corresponding application event should be sent that implements an AbstractAuthenticationFailureEvent.

Configuration

Dependencies:

implementation("org.springframework.boot:spring-boot-starter-actuator")
implementation("org.springframework.boot:spring-boot-starter-oauth2-resource-server")
...

WebSecurityConfigurerAdapter:

http
    .cors()
    .and()
    .authorizeRequests()
    .antMatchers(AUTH_WHITELIST)
    .permitAll()
    .anyRequest()
    .hasAnyAuthority("ROLE_REDACTED", .....)
    .and()
    .oauth2ResourceServer()
    .jwt()
    .jwtAuthenticationConverter(authenticationConverter);

Version

Spring Boot 2.2.2.RELEASE -> (Spring Security 5.2.1.RELEASE)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 7, 2020
@sdavids13
Copy link
Author

I attempted to get the event publisher to recognize the OAuth2AuthenticationException by adding the following bean to the Application Context:

  @Bean
  public DefaultAuthenticationEventPublisher authenticationEventPublisher(ApplicationEventPublisher publisher) {
    DefaultAuthenticationEventPublisher authPublisher = new DefaultAuthenticationEventPublisher(publisher);
    Properties p = new Properties();
    p.put(OAuth2AuthenticationException.class.getName(), AuthenticationFailureBadCredentialsEvent.class.getName());
    authPublisher.setAdditionalExceptionMappings(p);

    return authPublisher;
  }

I continued to get log messages saying:

{
  "classname": "org.springframework.security.authentication.DefaultAuthenticationEventPublisher",
  "method": "publishAuthenticationFailure",
  "file": "DefaultAuthenticationEventPublisher.java",
  "line": 125,
  "logger_name": "org.springframework.security.authentication.DefaultAuthenticationEventPublisher",
  "level": "DEBUG",
  "message": "No event was found for the exception org.springframework.security.oauth2.core.OAuth2AuthenticationException"
}

I then wired in the event publisher into the WebSecurityConfigurerAdapter via:

  private final AuthenticationEventPublisher authEventPublisher;
  public SecurityConfig(DefaultAuthenticationEventPublisher authEventPublisher) {
    this.authEventPublisher = authEventPublisher;
  }
  protected void configure(AuthenticationManagerBuilder auth) throws Exception {
    auth.authenticationEventPublisher(authEventPublisher);
  }

Still that didn't work... I then tried to set the properties again in the constructor which still doesn't work either:

  public SecurityConfig(DefaultAuthenticationEventPublisher authEventPublisher) {
    this.authEventPublisher = authEventPublisher;

    Properties p = new Properties();
    p.put(OAuth2AuthenticationException.class.getName(),
        AuthenticationFailureBadCredentialsEvent.class.getName());
    authEventPublisher.setAdditionalExceptionMappings(p);
  }

All produced the same log message saying No event was found for the exception org.springframework.security.oauth2.core.OAuth2AuthenticationException. Finally, I decided to create my own AuthenticationEventPublisher and configure it using configure(AuthenticationManagerBuilder auth) but when looking at the logs the Spring DefaultAuthenticationEventPublisher is still used instead of my custom implementation.

Long story short.. there seems to be 2 problems:

  1. The DefaultAuthenticationEventPublisher should include OAuth2AuthenticationException as one of it's handled exceptions
  2. The WebSecurityConfigurerAdapter doesn't seem to honor passing in a custom authenticationEventPublisher

@jzheaux jzheaux self-assigned this Jan 8, 2020
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 8, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Jan 9, 2020

@sdavids13, thanks for the report, and sorry to hear about all the debugging woes!

I did a bit of digging and found that the OAuth2ResourceServerConfigurer is relying on this AuthenticationEventPublisher in WebSecurityConfigurerAdapter#getHttp:

DefaultAuthenticationEventPublisher eventPublisher = objectPostProcessor
				.postProcess(new DefaultAuthenticationEventPublisher());

It would be better if this first looked for an AuthenticationEventPublisher bean, and then defaulted if it couldn't find one.

In the meantime, the following workaround should do the trick:

private ProviderManager providerManager() {
    JwtDecoder jwtDecoder = JwtDecoders.fromIssuerLocation(...);
    JwtAuthenticationProvider authenticationProvider = 
        new JwtAuthenticationProvider(jwtDecoder);
    authenticationProvider.setJwtAuthenticationConverter(jwtAuthenticationConverter);
    ProviderManager providerManager = new ProviderManager
        (Arrays.asList(authenticationProvider));
    providerManager.setAuthenticationEventPublisher(fooPublisher);
    return providerManager;
}

And then:

protected void configure(HttpSecurity http) throws Exception {
    http
        .cors(withDefaults())
        .authorizeRequests(authz -> authz
            .antMatchers(AUTH_WHITELIST).permitAll()
            .anyRequest().hasAnyAuthority("ROLE_REDACTED", .....)
        )
        .oauth2ResourceServer(oauth2 -> oauth2
            .jwt(j -> j
                .authenticationManager(providerManager())
            )
        );
}

@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Jan 9, 2020
@sdavids13
Copy link
Author

Looks like the work-around you provided works - thanks! Might I also suggest you add the following code as well:

diff --git a/core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java b/core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java
index 2d0015fc5..54f72495f 100644
--- a/core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java
+++ b/core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java
@@ -92,6 +92,9 @@ public class DefaultAuthenticationEventPublisher implements AuthenticationEventP
                addMapping(
                                "org.springframework.security.authentication.cas.ProxyUntrustedException",
                                AuthenticationFailureProxyUntrustedEvent.class);
+               addMapping(
+                               "org.springframework.security.oauth2.core.OAuth2AuthenticationException",
+                               AuthenticationFailureBadCredentialsEvent.class);
        }
 
        public void publishAuthenticationSuccess(Authentication authentication) {

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 10, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Jan 13, 2020

@sdavids13 I'm glad the workaround worked!

As for adding the exception by default, my concern is that not all OAuth2AuthenticationExceptions are due to bad credentials, so the event could be misleading.

One way might be to simplify how custom mappings are added. It seems reasonable to overload setAdditionalExceptionMappings to take a more powerful type:

public void setAdditionalExceptionMappings(Map<
        Class<? extends AuthenticationException>,
        Class<? extends AbstractAuthenticationFailureEvent>> additionalExceptionMappings)

And then you could instead do:

@Bean 
AuthenticationEventPublisher eventPublisher(ApplicationEventPublisher application) {
    AuthenticationEventPublisher authentication = 
        new DefaultAuthenticationEventPublisher(application);
    authentication.setAdditionalExceptionMappings(
        Collections.singletonMap(OAuth2AuthenticationException.class, FooEvent.class));
    return authentication;
}

Would you be interested in submitting a PR for that feature?

@jzheaux
Copy link
Contributor

jzheaux commented Jan 13, 2020

I should have mentioned that I do also agree that we should probably have some kind of OAuth mapping there - we probably just need a more specific exception type.

@jzheaux
Copy link
Contributor

jzheaux commented Jan 14, 2020

@sdavids13 Feel free to take the PR for a spin to see if it suits your needs. It introduces InvalidBearerTokenException which gets thrown whenever the resource server fails to authenticate the bearer token.

jzheaux added a commit to jzheaux/spring-security that referenced this issue Jan 14, 2020
jzheaux added a commit that referenced this issue Jan 14, 2020
@lsko
Copy link

lsko commented Jan 23, 2020

Will this issue will be a solution for Spring Security Oauth2's deprecated OAuth2AuthenticationFailureEvent class? Right now the deprecated class refers to the Migration Guide but there doesn't seem to have a replacement solution. Thank you.

jzheaux added a commit to jzheaux/spring-security that referenced this issue Feb 4, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Feb 5, 2020

@lsko, this ticket will address bearer token errors only, mapping them to AuthenticationFailureBadCredentialsEvent.

There is no plan at this time to port over OAuth2AuthenticationFailureEvent; however, there are some tickets in progress right now for making it simpler to wire DefaultAuthenticationEventPublisher to publish events on a broader basis.

@jzheaux jzheaux closed this as completed in fbdecda Feb 5, 2020
@jzheaux jzheaux added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Feb 5, 2020
@jzheaux jzheaux added this to the 5.3.0.RC1 milestone Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants