From 50ebd467c3a699142cb2bb27483893b56b00c86f Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 19 Sep 2025 15:48:58 -0600 Subject: [PATCH] Polish Default Login Page Issue gh-17901 --- .../configurers/FormLoginConfigurerTests.java | 14 +++++---- .../LoginUrlAuthenticationEntryPoint.java | 30 +++++++++++++++---- .../ui/DefaultLoginPageGeneratingFilter.java | 16 +++++----- ...DefaultLoginPageGeneratingFilterTests.java | 7 ++--- 4 files changed, 44 insertions(+), 23 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurerTests.java index 421912a2cc..e2bd96ea93 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurerTests.java @@ -402,7 +402,7 @@ public class FormLoginConfigurerTests { UserDetails user = PasswordEncodedUser.user(); this.mockMvc.perform(get("/profile").with(user(user))) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD")); + .andExpect(redirectedUrl("http://localhost/login?factor=password")); this.mockMvc .perform(post("/ott/generate").param("username", "rod") .with(user(user)) @@ -418,11 +418,11 @@ public class FormLoginConfigurerTests { user = PasswordEncodedUser.withUserDetails(user).authorities("profile:read", "FACTOR_OTT").build(); this.mockMvc.perform(get("/profile").with(user(user))) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD")); + .andExpect(redirectedUrl("http://localhost/login?factor=password")); user = PasswordEncodedUser.withUserDetails(user).authorities("profile:read", "FACTOR_PASSWORD").build(); this.mockMvc.perform(get("/profile").with(user(user))) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_OTT")); + .andExpect(redirectedUrl("http://localhost/login?factor=ott")); user = PasswordEncodedUser.withUserDetails(user) .authorities("profile:read", "FACTOR_PASSWORD", "FACTOR_OTT") .build(); @@ -438,7 +438,7 @@ public class FormLoginConfigurerTests { this.mockMvc.perform(get("/login")).andExpect(status().isOk()); this.mockMvc.perform(get("/profile").with(SecurityMockMvcRequestPostProcessors.x509("rod.cer"))) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD")); + .andExpect(redirectedUrl("http://localhost/login?factor=password")); this.mockMvc .perform(post("/login").param("username", "rod") .param("password", "password") @@ -793,7 +793,8 @@ public class FormLoginConfigurerTests { static class MfaDslConfig { @Bean - SecurityFilterChain filterChain(HttpSecurity http, AuthorizationManagerFactory authz) throws Exception { + SecurityFilterChain filterChain(HttpSecurity http, + AuthorizationManagerFactory authz) throws Exception { // @formatter:off http .formLogin(Customizer.withDefaults()) @@ -824,7 +825,8 @@ public class FormLoginConfigurerTests { static class MfaDslX509Config { @Bean - SecurityFilterChain filterChain(HttpSecurity http, AuthorizationManagerFactory authz) throws Exception { + SecurityFilterChain filterChain(HttpSecurity http, + AuthorizationManagerFactory authz) throws Exception { // @formatter:off http .x509(Customizer.withDefaults()) diff --git a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java index 50e0adbc05..f5f58e7607 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java +++ b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java @@ -18,6 +18,7 @@ package org.springframework.security.web.authentication; import java.io.IOException; import java.util.Collection; +import java.util.Locale; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletException; @@ -41,6 +42,7 @@ import org.springframework.security.web.access.ExceptionTranslationFilter; import org.springframework.security.web.util.RedirectUrlBuilder; import org.springframework.security.web.util.UrlUtils; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; @@ -71,6 +73,8 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin private static final Log logger = LogFactory.getLog(LoginUrlAuthenticationEntryPoint.class); + private static final String FACTOR_PREFIX = "FACTOR_"; + private PortMapper portMapper = new PortMapperImpl(); private String loginFormUrl; @@ -110,15 +114,29 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin * @param exception the exception * @return the URL (cannot be null or empty; defaults to {@link #getLoginFormUrl()}) */ + @SuppressWarnings("unchecked") protected String determineUrlToUseForThisRequest(HttpServletRequest request, HttpServletResponse response, AuthenticationException exception) { - Object value = request.getAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE); - if (value instanceof Collection authorities) { - return UriComponentsBuilder.fromUriString(getLoginFormUrl()) - .queryParam("authority", authorities) - .toUriString(); + Collection authorities = getAttribute(request, GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE, + Collection.class); + if (CollectionUtils.isEmpty(authorities)) { + return getLoginFormUrl(); } - return getLoginFormUrl(); + Collection factors = authorities.stream() + .filter((a) -> a.getAuthority().startsWith(FACTOR_PREFIX)) + .map((a) -> a.getAuthority().substring(FACTOR_PREFIX.length()).toLowerCase(Locale.ROOT)) + .toList(); + return UriComponentsBuilder.fromUriString(getLoginFormUrl()).queryParam("factor", factors).toUriString(); + } + + private static @Nullable T getAttribute(HttpServletRequest request, String name, Class clazz) { + Object value = request.getAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE); + if (value == null) { + return null; + } + String message = String.format("Found %s in %s, but expecting a %s", value.getClass(), name, clazz); + Assert.isInstanceOf(clazz, value, message); + return (T) value; } /** 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 e782deddbc..1346185330 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 @@ -88,7 +88,9 @@ public class DefaultLoginPageGeneratingFilter extends GenericFilterBean { private @Nullable String rememberMeParameter; - private final Collection allowedParameters = List.of("authority"); + private final String factorParameter = "factor"; + + private final Collection allowedParameters = List.of(this.factorParameter); @SuppressWarnings("NullAway.Init") private Map oauth2AuthenticationUrlToClientName; @@ -257,29 +259,29 @@ public class DefaultLoginPageGeneratingFilter extends GenericFilterBean { .withRawHtml("passkeyLogin", ""); Predicate wantsAuthority = wantsAuthority(request); - if (wantsAuthority.test("FACTOR_WEBAUTHN")) { + if (wantsAuthority.test("webauthn")) { builder.withRawHtml("javaScript", renderJavaScript(request, contextPath)) .withRawHtml("passkeyLogin", renderPasskeyLogin()); } - if (wantsAuthority.test("FACTOR_PASSWORD")) { + if (wantsAuthority.test("password")) { builder.withRawHtml("formLogin", renderFormLogin(request, loginError, logoutSuccess, contextPath, errorMsg)); } - if (wantsAuthority.test("FACTOR_OTT")) { + if (wantsAuthority.test("ott")) { builder.withRawHtml("oneTimeTokenLogin", renderOneTimeTokenLogin(request, loginError, logoutSuccess, contextPath, errorMsg)); } - if (wantsAuthority.test("FACTOR_AUTHORIZATION_CODE")) { + if (wantsAuthority.test("authorization_code")) { builder.withRawHtml("oauth2Login", renderOAuth2Login(loginError, logoutSuccess, errorMsg, contextPath)); } - if (wantsAuthority.test("FACTOR_SAML_RESPONSE")) { + if (wantsAuthority.test("saml_response")) { builder.withRawHtml("saml2Login", renderSaml2Login(loginError, logoutSuccess, errorMsg, contextPath)); } return builder.render(); } private Predicate wantsAuthority(HttpServletRequest request) { - String[] authorities = request.getParameterValues("authority"); + String[] authorities = request.getParameterValues(this.factorParameter); if (authorities == null) { return (authority) -> true; } 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 ee38d177b0..9717e676a0 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 @@ -204,7 +204,7 @@ public class DefaultLoginPageGeneratingFilterTests { filter.setOneTimeTokenEnabled(true); filter.setOneTimeTokenGenerationUrl("/ott/authenticate"); MockHttpServletResponse response = new MockHttpServletResponse(); - filter.doFilter(TestMockHttpServletRequests.get("/login?authority=FACTOR_OTT").build(), response, this.chain); + filter.doFilter(TestMockHttpServletRequests.get("/login?factor=ott").build(), response, this.chain); assertThat(response.getContentAsString()).contains("Request a One-Time Token"); assertThat(response.getContentAsString()).contains("""