From 7ddc00521ed88df2086eba60d059742057b0d127 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Fri, 8 Mar 2024 14:00:12 +0100 Subject: [PATCH] Improve logging for Global Authentication Closes gh-14663 --- ...ticationProviderBeanManagerConfigurer.java | 64 ++++++++++++++++- ...alizeUserDetailsBeanManagerConfigurer.java | 69 ++++++++++++++++++- 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeAuthenticationProviderBeanManagerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeAuthenticationProviderBeanManagerConfigurer.java index 62eabe3b4c9..c2365acf2e9 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeAuthenticationProviderBeanManagerConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeAuthenticationProviderBeanManagerConfigurer.java @@ -16,8 +16,15 @@ package org.springframework.security.config.annotation.authentication.configuration; +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.Order; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; @@ -49,16 +56,33 @@ public void init(AuthenticationManagerBuilder auth) throws Exception { class InitializeAuthenticationProviderManagerConfigurer extends GlobalAuthenticationConfigurerAdapter { + private final Log logger = LogFactory.getLog(getClass()); + @Override public void configure(AuthenticationManagerBuilder auth) { if (auth.isConfigured()) { return; } - AuthenticationProvider authenticationProvider = getBeanOrNull(AuthenticationProvider.class); - if (authenticationProvider == null) { + List> authenticationProviders = getBeansWithName( + AuthenticationProvider.class); + if (authenticationProviders.isEmpty()) { return; } + else if (authenticationProviders.size() > 1) { + List beanNames = authenticationProviders.stream().map(BeanWithName::getName).toList(); + this.logger.info(LogMessage.format("Found %s AuthenticationProvider beans, with names %s. " + + "Global Authentication Manager will not be configured with AuthenticationProviders. " + + "Consider publishing a single AuthenticationProvider bean, or wiring your Providers directly " + + "using the DSL.", authenticationProviders.size(), beanNames)); + return; + } + var authenticationProvider = authenticationProviders.get(0).getBean(); + var authenticationProviderBeanName = authenticationProviders.get(0).getName(); + auth.authenticationProvider(authenticationProvider); + this.logger.info(LogMessage.format( + "Global AuthenticationManager configured with AuthenticationProvider bean with name %s", + authenticationProviderBeanName)); } /** @@ -74,6 +98,42 @@ private T getBeanOrNull(Class type) { return InitializeAuthenticationProviderBeanManagerConfigurer.this.context.getBean(beanNames[0], type); } + /** + * @return a list of beans of the requested class, along with their names. If + * there are no registered beans of that type, the list is empty. + */ + private List> getBeansWithName(Class type) { + List> beanWithNames = new ArrayList<>(); + String[] beanNames = InitializeAuthenticationProviderBeanManagerConfigurer.this.context + .getBeanNamesForType(type); + for (String beanName : beanNames) { + T bean = InitializeAuthenticationProviderBeanManagerConfigurer.this.context.getBean(beanNames[0], type); + beanWithNames.add(new BeanWithName(bean, beanName)); + } + return beanWithNames; + } + + static class BeanWithName { + + private final T bean; + + private final String name; + + BeanWithName(T bean, String name) { + this.bean = bean; + this.name = name; + } + + T getBean() { + return this.bean; + } + + String getName() { + return this.name; + } + + } + } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java index 621b53f9eeb..35942589079 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/InitializeUserDetailsBeanManagerConfigurer.java @@ -16,9 +16,16 @@ package org.springframework.security.config.annotation.authentication.configuration; +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.context.ApplicationContext; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.core.log.LogMessage; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; import org.springframework.security.authentication.password.CompromisedPasswordChecker; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; @@ -55,15 +62,35 @@ public void init(AuthenticationManagerBuilder auth) throws Exception { class InitializeUserDetailsManagerConfigurer extends GlobalAuthenticationConfigurerAdapter { + private final Log logger = LogFactory.getLog(getClass()); + @Override public void configure(AuthenticationManagerBuilder auth) throws Exception { + List> userDetailsServices = getBeansWithName(UserDetailsService.class); if (auth.isConfigured()) { + if (!userDetailsServices.isEmpty()) { + this.logger.warn("Global AuthenticationManager configured with an AuthenticationProvider bean. " + + "UserDetailsService beans will not be used for username/password login. " + + "Consider removing the AuthenticationProvider bean. " + + "Alternatively, consider using the UserDetailsService in a manually instantiated " + + "DaoAuthenticationProvider."); + } + return; + } + + if (userDetailsServices.isEmpty()) { return; } - UserDetailsService userDetailsService = getBeanOrNull(UserDetailsService.class); - if (userDetailsService == null) { + else if (userDetailsServices.size() > 1) { + List beanNames = userDetailsServices.stream().map(BeanWithName::getName).toList(); + this.logger.warn(LogMessage.format("Found %s UserDetailsService beans, with names %s. " + + "Global Authentication Manager will not use a UserDetailsService for username/password login. " + + "Consider publishing a single UserDetailsService bean.", userDetailsServices.size(), + beanNames)); return; } + var userDetailsService = userDetailsServices.get(0).getBean(); + var userDetailsServiceBeanName = userDetailsServices.get(0).getName(); PasswordEncoder passwordEncoder = getBeanOrNull(PasswordEncoder.class); UserDetailsPasswordService passwordManager = getBeanOrNull(UserDetailsPasswordService.class); CompromisedPasswordChecker passwordChecker = getBeanOrNull(CompromisedPasswordChecker.class); @@ -83,6 +110,9 @@ public void configure(AuthenticationManagerBuilder auth) throws Exception { } provider.afterPropertiesSet(); auth.authenticationProvider(provider); + this.logger.info(LogMessage.format( + "Global AuthenticationManager configured with UserDetailsService bean with name %s", + userDetailsServiceBeanName)); } /** @@ -97,6 +127,41 @@ private T getBeanOrNull(Class type) { return InitializeUserDetailsBeanManagerConfigurer.this.context.getBean(beanNames[0], type); } + /** + * @return a list of beans of the requested class, along with their names. If + * there are no registered beans of that type, the list is empty. + */ + private List> getBeansWithName(Class type) { + List> beanWithNames = new ArrayList<>(); + String[] beanNames = InitializeUserDetailsBeanManagerConfigurer.this.context.getBeanNamesForType(type); + for (String beanName : beanNames) { + T bean = InitializeUserDetailsBeanManagerConfigurer.this.context.getBean(beanNames[0], type); + beanWithNames.add(new BeanWithName(bean, beanName)); + } + return beanWithNames; + } + + static class BeanWithName { + + private final T bean; + + private final String name; + + BeanWithName(T bean, String name) { + this.bean = bean; + this.name = name; + } + + T getBean() { + return this.bean; + } + + String getName() { + return this.name; + } + + } + } }