Skip to content

Generic error message in Log In page and debug messages #16575

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 14, 2025
Merged

Generic error message in Log In page and debug messages #16575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
590d933
Return Invalid Credentials message on login error
Tejas-Teju Feb 11, 2025
82078a1
Add debug messages for auth exceptions in ProviderManager
Tejas-Teju Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions ...framework/security/config/annotation/web/configurers/DefaultLoginPageConfigurerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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("""
<!DOCTYPE html>
Expand Down Expand Up @@ -184,7 +179,7 @@ public void loginPageWhenErrorThenDefaultLoginPageWithError() throws Exception {

</div>
</body>
</html>""".formatted(badCredentialsLocalizedMessage, token.getToken()));
</html>""".formatted(defaultErrorMessage, token.getToken()));
});
// @formatter:on
}
Expand Down
21 changes: 20 additions & 1 deletion core/src/main/java/org/springframework/security/authentication/ProviderManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}

Expand Down
21 changes: 1 addition & 20 deletions .../org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
25 changes: 3 additions & 22 deletions ...rg/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -244,7 +225,7 @@ void generatesThenRenders() throws ServletException, IOException {
<div class="content">
<form class="login-form" method="post" action="null">
<h2>Please sign in</h2>
<div class="alert alert-danger" role="alert">Bad credentials</div>
<div class="alert alert-danger" role="alert">Invalid credentials</div>
<p>
<label for="username" class="screenreader">Username</label>
<input type="text" id="username" name="username" placeholder="Username" required autofocus>
Expand All @@ -259,12 +240,12 @@ void generatesThenRenders() throws ServletException, IOException {
</form>

<h2>Login with OAuth 2.0</h2>
<div class="alert alert-danger" role="alert">Bad credentials</div>
<div class="alert alert-danger" role="alert">Invalid credentials</div>
<table class="table table-striped">
<tr><td><a href="/oauth2/authorization/google">Google &lt; &gt; &quot; &#39; &amp;</a></td></tr>
</table>
<h2>Login with SAML 2.0</h2>
<div class="alert alert-danger" role="alert">Bad credentials</div>
<div class="alert alert-danger" role="alert">Invalid credentials</div>
<table class="table table-striped">
<tr><td><a href="/saml/sso/google">Google &lt; &gt; &quot; &#39; &amp;</a></td></tr>
</table>
Expand Down