diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java index f4646fe6f53..4a1c9d05d7d 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java @@ -22,14 +22,12 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.support.MessageSourceAccessor; import org.springframework.mock.web.MockHttpSession; import org.springframework.security.config.ObjectPostProcessor; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; -import org.springframework.security.core.SpringSecurityMessageSource; import org.springframework.security.core.userdetails.PasswordEncodedUser; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.provisioning.InMemoryUserDetailsManager; @@ -77,8 +75,6 @@ public class DefaultLoginPageConfigurerTests { @Autowired MockMvc mvc; - MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); - @Test public void getWhenFormLoginEnabledThenRedirectsToLoginPage() throws Exception { this.spring.register(DefaultLoginPageConfig.class).autowire(); @@ -148,8 +144,7 @@ public void loginPageWhenErrorThenDefaultLoginPageWithError() throws Exception { this.mvc.perform(get("/login?error").session((MockHttpSession) mvcResult.getRequest().getSession()) .sessionAttr(csrfAttributeName, csrfToken)) .andExpect((result) -> { - String badCredentialsLocalizedMessage = this.messages - .getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"); + String defaultErrorMessage = "Invalid credentials"; CsrfToken token = (CsrfToken) result.getRequest().getAttribute(CsrfToken.class.getName()); assertThat(result.getResponse().getContentAsString()).isEqualTo(""" @@ -184,7 +179,7 @@ public void loginPageWhenErrorThenDefaultLoginPageWithError() throws Exception { - """.formatted(badCredentialsLocalizedMessage, token.getToken())); + """.formatted(defaultErrorMessage, token.getToken())); }); // @formatter:on } 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 a09283b08c0..303444cc6b1 100644 --- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java +++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java @@ -185,13 +185,25 @@ public Authentication authenticate(Authentication authentication) throws Authent break; } } - catch (AccountStatusException | InternalAuthenticationServiceException ex) { + catch (AccountStatusException ex) { prepareException(ex, authentication); + logger.debug(LogMessage.format("Authentication failed for user '%s' since account status is %s", + authentication.getName(), ex.getMessage())); + // SEC-546: Avoid polling additional providers if auth failure is due to + // invalid account status + throw ex; + } + catch (InternalAuthenticationServiceException ex) { + prepareException(ex, authentication); + logger.debug(LogMessage.format( + "Authentication failed due to an internal authentication service error: %s", ex.getMessage())); // SEC-546: Avoid polling additional providers if auth failure is due to // invalid account status throw ex; } catch (AuthenticationException ex) { + logger.debug(LogMessage.format("Authentication failed with provider %s since %s", + provider.getClass().getSimpleName(), ex.getMessage())); lastException = ex; } } @@ -241,6 +253,13 @@ public Authentication authenticate(Authentication authentication) throws Authent if (parentException == null) { prepareException(lastException, authentication); } + + // Ensure this message is not logged when authentication is attempted by + // the parent provider + if (this.parent != null) { + logger.debug("Denying authentication since all attempted providers failed"); + } + throw lastException; } diff --git a/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java index 4085eed3458..1acc36e4efa 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java @@ -29,14 +29,10 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpSession; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.web.WebAttributes; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.rememberme.AbstractRememberMeServices; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; import org.springframework.web.filter.GenericFilterBean; /** @@ -221,7 +217,7 @@ private void doFilter(HttpServletRequest request, HttpServletResponse response, } private String generateLoginPageHtml(HttpServletRequest request, boolean loginError, boolean logoutSuccess) { - String errorMsg = loginError ? getLoginErrorMessage(request) : "Invalid credentials"; + String errorMsg = "Invalid credentials"; String contextPath = request.getContextPath(); return HtmlTemplates.fromTemplate(LOGIN_PAGE_TEMPLATE) @@ -358,21 +354,6 @@ private static String renderSaml2Row(String contextPath, String url, String clie .render(); } - private String getLoginErrorMessage(HttpServletRequest request) { - HttpSession session = request.getSession(false); - if (session == null) { - return "Invalid credentials"; - } - if (!(session - .getAttribute(WebAttributes.AUTHENTICATION_EXCEPTION) instanceof AuthenticationException exception)) { - return "Invalid credentials"; - } - if (!StringUtils.hasText(exception.getMessage())) { - return "Invalid credentials"; - } - return exception.getMessage(); - } - private String renderHiddenInput(String name, String value) { return HtmlTemplates.fromTemplate(HIDDEN_HTML_INPUT_TEMPLATE) .withValue("name", name) diff --git a/web/src/test/java/org/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java index a3b62830d6b..39bdc513e04 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java @@ -18,17 +18,14 @@ import java.io.IOException; import java.util.Collections; -import java.util.Locale; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import org.junit.jupiter.api.Test; -import org.springframework.context.support.MessageSourceAccessor; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.core.SpringSecurityMessageSource; import org.springframework.security.web.WebAttributes; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; @@ -128,22 +125,6 @@ public void generatesForWithQueryNoMatch() throws Exception { assertThat(response.getContentAsString()).isEmpty(); } - /* SEC-1111 */ - @Test - public void handlesNonIso8859CharsInErrorMessage() throws Exception { - DefaultLoginPageGeneratingFilter filter = new DefaultLoginPageGeneratingFilter( - new UsernamePasswordAuthenticationFilter()); - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/login"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.setQueryString("error"); - MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); - String message = messages.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", - "Bad credentials", Locale.KOREA); - request.getSession().setAttribute(WebAttributes.AUTHENTICATION_EXCEPTION, new BadCredentialsException(message)); - filter.doFilter(request, response, this.chain); - assertThat(response.getContentAsString()).contains(message); - } - // gh-5394 @Test public void generatesForOAuth2LoginAndEscapesClientName() throws Exception { @@ -244,7 +225,7 @@ void generatesThenRenders() throws ServletException, IOException {

Please sign in

- +

@@ -259,12 +240,12 @@ void generatesThenRenders() throws ServletException, IOException {

Login with OAuth 2.0

- +
Google < > " ' &

Login with SAML 2.0

- +
Google < > " ' &