Skip to content

Commit 50ebd46

Browse files
jzheauxjzheaux
committed
Polish Default Login Page
Issue spring-projectsgh-17901
1 parent 42376e2 commit 50ebd46

File tree

4 files changed

+43
-22
lines changed

4 files changed

+43
-22
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurerTests.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ void requestWhenUnauthenticatedThenRequiresTwoSteps() throws Exception {
402402
UserDetails user = PasswordEncodedUser.user();
403403
this.mockMvc.perform(get("/profile").with(user(user)))
404404
.andExpect(status().is3xxRedirection())
405-
.andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD"));
405+
.andExpect(redirectedUrl("http://localhost/login?factor=password"));
406406
this.mockMvc
407407
.perform(post("/ott/generate").param("username", "rod")
408408
.with(user(user))
@@ -418,11 +418,11 @@ void requestWhenUnauthenticatedThenRequiresTwoSteps() throws Exception {
418418
user = PasswordEncodedUser.withUserDetails(user).authorities("profile:read", "FACTOR_OTT").build();
419419
this.mockMvc.perform(get("/profile").with(user(user)))
420420
.andExpect(status().is3xxRedirection())
421-
.andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD"));
421+
.andExpect(redirectedUrl("http://localhost/login?factor=password"));
422422
user = PasswordEncodedUser.withUserDetails(user).authorities("profile:read", "FACTOR_PASSWORD").build();
423423
this.mockMvc.perform(get("/profile").with(user(user)))
424424
.andExpect(status().is3xxRedirection())
425-
.andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_OTT"));
425+
.andExpect(redirectedUrl("http://localhost/login?factor=ott"));
426426
user = PasswordEncodedUser.withUserDetails(user)
427427
.authorities("profile:read", "FACTOR_PASSWORD", "FACTOR_OTT")
428428
.build();
@@ -438,7 +438,7 @@ void requestWhenUnauthenticatedX509ThenRequiresTwoSteps() throws Exception {
438438
this.mockMvc.perform(get("/login")).andExpect(status().isOk());
439439
this.mockMvc.perform(get("/profile").with(SecurityMockMvcRequestPostProcessors.x509("rod.cer")))
440440
.andExpect(status().is3xxRedirection())
441-
.andExpect(redirectedUrl("http://localhost/login?authority=FACTOR_PASSWORD"));
441+
.andExpect(redirectedUrl("http://localhost/login?factor=password"));
442442
this.mockMvc
443443
.perform(post("/login").param("username", "rod")
444444
.param("password", "password")
@@ -793,7 +793,8 @@ public <O> O postProcess(O object) {
793793
static class MfaDslConfig {
794794

795795
@Bean
796-
SecurityFilterChain filterChain(HttpSecurity http, AuthorizationManagerFactory<RequestAuthorizationContext> authz) throws Exception {
796+
SecurityFilterChain filterChain(HttpSecurity http,
797+
AuthorizationManagerFactory<RequestAuthorizationContext> authz) throws Exception {
797798
// @formatter:off
798799
http
799800
.formLogin(Customizer.withDefaults())
@@ -824,7 +825,8 @@ AuthorizationManagerFactory<?> authz() {
824825
static class MfaDslX509Config {
825826

826827
@Bean
827-
SecurityFilterChain filterChain(HttpSecurity http, AuthorizationManagerFactory<RequestAuthorizationContext> authz) throws Exception {
828+
SecurityFilterChain filterChain(HttpSecurity http,
829+
AuthorizationManagerFactory<RequestAuthorizationContext> authz) throws Exception {
828830
// @formatter:off
829831
http
830832
.x509(Customizer.withDefaults())

‎web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.io.IOException;
2020
import java.util.Collection;
21+
import java.util.Locale;
2122

2223
import jakarta.servlet.RequestDispatcher;
2324
import jakarta.servlet.ServletException;
@@ -41,6 +42,7 @@
4142
import org.springframework.security.web.util.RedirectUrlBuilder;
4243
import org.springframework.security.web.util.UrlUtils;
4344
import org.springframework.util.Assert;
45+
import org.springframework.util.CollectionUtils;
4446
import org.springframework.util.StringUtils;
4547
import org.springframework.web.util.UriComponentsBuilder;
4648

@@ -71,6 +73,8 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin
7173

7274
private static final Log logger = LogFactory.getLog(LoginUrlAuthenticationEntryPoint.class);
7375

76+
private static final String FACTOR_PREFIX = "FACTOR_";
77+
7478
private PortMapper portMapper = new PortMapperImpl();
7579

7680
private String loginFormUrl;
@@ -110,15 +114,29 @@ public void afterPropertiesSet() {
110114
* @param exception the exception
111115
* @return the URL (cannot be null or empty; defaults to {@link #getLoginFormUrl()})
112116
*/
117+
@SuppressWarnings("unchecked")
113118
protected String determineUrlToUseForThisRequest(HttpServletRequest request, HttpServletResponse response,
114119
AuthenticationException exception) {
120+
Collection<GrantedAuthority> authorities = getAttribute(request, GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE,
121+
Collection.class);
122+
if (CollectionUtils.isEmpty(authorities)) {
123+
return getLoginFormUrl();
124+
}
125+
Collection<String> factors = authorities.stream()
126+
.filter((a) -> a.getAuthority().startsWith(FACTOR_PREFIX))
127+
.map((a) -> a.getAuthority().substring(FACTOR_PREFIX.length()).toLowerCase(Locale.ROOT))
128+
.toList();
129+
return UriComponentsBuilder.fromUriString(getLoginFormUrl()).queryParam("factor", factors).toUriString();
130+
}
131+
132+
private static <T> @Nullable T getAttribute(HttpServletRequest request, String name, Class<T> clazz) {
115133
Object value = request.getAttribute(GrantedAuthority.MISSING_AUTHORITIES_ATTRIBUTE);
116-
if (value instanceof Collection<?> authorities) {
117-
return UriComponentsBuilder.fromUriString(getLoginFormUrl())
118-
.queryParam("authority", authorities)
119-
.toUriString();
134+
if (value == null) {
135+
return null;
120136
}
121-
return getLoginFormUrl();
137+
String message = String.format("Found %s in %s, but expecting a %s", value.getClass(), name, clazz);
138+
Assert.isInstanceOf(clazz, value, message);
139+
return (T) value;
122140
}
123141

124142
/**

‎web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ public class DefaultLoginPageGeneratingFilter extends GenericFilterBean {
8888

8989
private @Nullable String rememberMeParameter;
9090

91-
private final Collection<String> allowedParameters = List.of("authority");
91+
private final String factorParameter = "factor";
92+
93+
private final Collection<String> allowedParameters = List.of(this.factorParameter);
9294

9395
@SuppressWarnings("NullAway.Init")
9496
private Map<String, String> oauth2AuthenticationUrlToClientName;
@@ -257,29 +259,29 @@ private String generateLoginPageHtml(HttpServletRequest request, boolean loginEr
257259
.withRawHtml("passkeyLogin", "");
258260

259261
Predicate<String> wantsAuthority = wantsAuthority(request);
260-
if (wantsAuthority.test("FACTOR_WEBAUTHN")) {
262+
if (wantsAuthority.test("webauthn")) {
261263
builder.withRawHtml("javaScript", renderJavaScript(request, contextPath))
262264
.withRawHtml("passkeyLogin", renderPasskeyLogin());
263265
}
264-
if (wantsAuthority.test("FACTOR_PASSWORD")) {
266+
if (wantsAuthority.test("password")) {
265267
builder.withRawHtml("formLogin",
266268
renderFormLogin(request, loginError, logoutSuccess, contextPath, errorMsg));
267269
}
268-
if (wantsAuthority.test("FACTOR_OTT")) {
270+
if (wantsAuthority.test("ott")) {
269271
builder.withRawHtml("oneTimeTokenLogin",
270272
renderOneTimeTokenLogin(request, loginError, logoutSuccess, contextPath, errorMsg));
271273
}
272-
if (wantsAuthority.test("FACTOR_AUTHORIZATION_CODE")) {
274+
if (wantsAuthority.test("authorization_code")) {
273275
builder.withRawHtml("oauth2Login", renderOAuth2Login(loginError, logoutSuccess, errorMsg, contextPath));
274276
}
275-
if (wantsAuthority.test("FACTOR_SAML_RESPONSE")) {
277+
if (wantsAuthority.test("saml_response")) {
276278
builder.withRawHtml("saml2Login", renderSaml2Login(loginError, logoutSuccess, errorMsg, contextPath));
277279
}
278280
return builder.render();
279281
}
280282

281283
private Predicate<String> wantsAuthority(HttpServletRequest request) {
282-
String[] authorities = request.getParameterValues("authority");
284+
String[] authorities = request.getParameterValues(this.factorParameter);
283285
if (authorities == null) {
284286
return (authority) -> true;
285287
}

‎web/src/test/java/org/springframework/security/web/authentication/DefaultLoginPageGeneratingFilterTests.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public void generateWhenOneTimeTokenRequestedThenOttForm() throws Exception {
204204
filter.setOneTimeTokenEnabled(true);
205205
filter.setOneTimeTokenGenerationUrl("/ott/authenticate");
206206
MockHttpServletResponse response = new MockHttpServletResponse();
207-
filter.doFilter(TestMockHttpServletRequests.get("/login?authority=FACTOR_OTT").build(), response, this.chain);
207+
filter.doFilter(TestMockHttpServletRequests.get("/login?factor=ott").build(), response, this.chain);
208208
assertThat(response.getContentAsString()).contains("Request a One-Time Token");
209209
assertThat(response.getContentAsString()).contains("""
210210
<form id="ott-form" class="login-form" method="post" action="/ott/authenticate">
@@ -231,9 +231,8 @@ public void generateWhenTwoAuthoritiesRequestedThenBothForms() throws Exception
231231
filter.setOneTimeTokenEnabled(true);
232232
filter.setOneTimeTokenGenerationUrl("/ott/authenticate");
233233
MockHttpServletResponse response = new MockHttpServletResponse();
234-
filter.doFilter(
235-
TestMockHttpServletRequests.get("/login?authority=FACTOR_OTT&authority=FACTOR_PASSWORD").build(),
236-
response, this.chain);
234+
filter.doFilter(TestMockHttpServletRequests.get("/login?factor=ott&factor=password").build(), response,
235+
this.chain);
237236
assertThat(response.getContentAsString()).contains("Request a One-Time Token");
238237
assertThat(response.getContentAsString()).contains("""
239238
<form id="ott-form" class="login-form" method="post" action="/ott/authenticate">

0 commit comments

Comments
 (0)