From d6da4d20b4991183da9748dcbe3f8bd3da071932 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Tue, 23 Oct 2018 14:08:23 -0400 Subject: [PATCH] Set AuthenticationEventPublisher on each AuthenticationManagerBuilder Fixes gh-6009 --- .../WebSecurityConfigurerAdapter.java | 1 + .../client/OAuth2LoginConfigurerTests.java | 35 ++++++++++++++++++- .../authentication/ProviderManager.java | 12 ++++--- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java index c75b94ccb5..5055dcebf9 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java @@ -200,6 +200,7 @@ public abstract class WebSecurityConfigurerAdapter implements AuthenticationManager authenticationManager = authenticationManager(); authenticationBuilder.parentAuthenticationManager(authenticationManager); + authenticationBuilder.authenticationEventPublisher(eventPublisher); Map, Object> sharedObjects = createSharedObjects(); http = new HttpSecurity(objectPostProcessor, authenticationBuilder, diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java index cdf7be5add..1ac2e561a6 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java @@ -20,11 +20,13 @@ import org.junit.Before; import org.junit.Test; import org.springframework.beans.PropertyAccessorFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationListener; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.event.AuthenticationSuccessEvent; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; @@ -145,6 +147,30 @@ public class OAuth2LoginConfigurerTests { .isInstanceOf(OAuth2UserAuthority.class).hasToString("ROLE_USER"); } + // gh-6009 + @Test + public void oauth2LoginWhenSuccessThenAuthenticationSuccessEventPublished() throws Exception { + // setup application context + loadConfig(OAuth2LoginConfig.class); + + // setup authorization request + OAuth2AuthorizationRequest authorizationRequest = createOAuth2AuthorizationRequest(); + this.authorizationRequestRepository.saveAuthorizationRequest( + authorizationRequest, this.request, this.response); + + // setup authentication parameters + this.request.setParameter("code", "code123"); + this.request.setParameter("state", authorizationRequest.getState()); + + // perform test + this.springSecurityFilterChain.doFilter(this.request, this.response, this.filterChain); + + // assertions + assertThat(OAuth2LoginConfig.EVENTS).isNotEmpty(); + assertThat(OAuth2LoginConfig.EVENTS).hasSize(1); + assertThat(OAuth2LoginConfig.EVENTS.get(0)).isInstanceOf(AuthenticationSuccessEvent.class); + } + @Test public void oauth2LoginCustomWithConfigurer() throws Exception { // setup application context @@ -348,7 +374,9 @@ public class OAuth2LoginConfigurerTests { } @EnableWebSecurity - static class OAuth2LoginConfig extends CommonWebSecurityConfigurerAdapter { + static class OAuth2LoginConfig extends CommonWebSecurityConfigurerAdapter implements ApplicationListener { + static List EVENTS = new ArrayList<>(); + @Override protected void configure(HttpSecurity http) throws Exception { http @@ -357,6 +385,11 @@ public class OAuth2LoginConfigurerTests { new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION)); super.configure(http); } + + @Override + public void onApplicationEvent(AuthenticationSuccessEvent event) { + EVENTS.add(event); + } } @EnableWebSecurity diff --git a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java index 411c51acb2..090f8b2605 100644 --- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java +++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.security.authentication; import java.util.Collections; @@ -158,6 +157,7 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar Class toTest = authentication.getClass(); AuthenticationException lastException = null; Authentication result = null; + Authentication parentResult = null; boolean debug = logger.isDebugEnabled(); for (AuthenticationProvider provider : getProviders()) { @@ -196,7 +196,7 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar if (result == null && parent != null) { // Allow the parent to try. try { - result = parent.authenticate(authentication); + result = parentResult = parent.authenticate(authentication); } catch (ProviderNotFoundException e) { // ignore as we will throw below if no other exception occurred prior to @@ -217,7 +217,11 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar ((CredentialsContainer) result).eraseCredentials(); } - eventPublisher.publishAuthenticationSuccess(result); + // If the parent AuthenticationManager was attempted and successful than it will publish an AuthenticationSuccessEvent + // This check prevents a duplicate AuthenticationSuccessEvent if the parent AuthenticationManager already published it + if (parentResult == null) { + eventPublisher.publishAuthenticationSuccess(result); + } return result; }