SEC-2244: Defaults based on loginPage are now updated when loginPage changes

config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java

@@ -22,6 +22,7 @@ import org.springframework.security.authentication.AuthenticationDetailsSource;
 import org.springframework.security.config.annotation.web.HttpSecurityBuilder;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
 import org.springframework.security.config.annotation.web.configurers.openid.OpenIDLoginConfigurer;
+import org.springframework.security.web.AuthenticationEntryPoint;
 import org.springframework.security.web.PortMapper;
 import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
 import org.springframework.security.web.authentication.AuthenticationFailureHandler;
@@ -30,10 +31,8 @@ import org.springframework.security.web.authentication.LoginUrlAuthenticationEnt
 import org.springframework.security.web.authentication.RememberMeServices;
 import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
 import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler;
-import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
 import org.springframework.security.web.authentication.WebAuthenticationDetailsSource;
 import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
-import org.springframework.security.web.util.AntPathRequestMatcher;
 import org.springframework.security.web.util.MediaTypeRequestMatcher;
 import org.springframework.security.web.util.RequestMatcher;
 import org.springframework.web.accept.ContentNegotiationStrategy;
@@ -78,10 +77,10 @@ public abstract class AbstractAuthenticationFilterConfigurer<B  extends HttpSecu
      */
     protected AbstractAuthenticationFilterConfigurer(F authenticationFilter, String defaultLoginProcessingUrl) {
         this.authFilter = authenticationFilter;
-        loginUrl("/login");
+        setLoginPage("/login");
-        failureUrl("/login?error");
+        if(defaultLoginProcessingUrl != null) {
-        loginProcessingUrl(defaultLoginProcessingUrl);
+            loginProcessingUrl(defaultLoginProcessingUrl);
-        this.customLoginPage = false;
+        }
     }
 
     /**
@@ -118,19 +117,6 @@ public abstract class AbstractAuthenticationFilterConfigurer<B  extends HttpSecu
         return successHandler(handler);
     }
 
-    /**
-     * Specifies the URL used to log in. If the request matches the URL and is an HTTP POST, the
-     * {@link UsernamePasswordAuthenticationFilter} will attempt to authenticate
-     * the request. Otherwise, if the request matches the URL the user will be sent to the login form.
-     *
-     * @param loginUrl the URL used to perform authentication
-     * @return the {@link FormLoginConfigurer} for additional customization
-     */
-    public final T loginUrl(String loginUrl) {
-        loginProcessingUrl(loginUrl);
-        return loginPage(loginUrl);
-    }
-
     /**
      * Specifies the URL to validate the credentials.
      *
@@ -186,7 +172,7 @@ public abstract class AbstractAuthenticationFilterConfigurer<B  extends HttpSecu
 
     /**
      * Ensures the urls for {@link #failureUrl(String)} and
-     * {@link #loginUrl(String)} are granted access to any user.
+     * {@link #authenticationUrls(String)} are granted access to any user.
      *
      * @param permitAll true to grant access to the URLs false to skip this step
      * @return the {@link FormLoginConfigurer} for additional customization
@@ -230,9 +216,11 @@ public abstract class AbstractAuthenticationFilterConfigurer<B  extends HttpSecu
 
     @Override
     public void init(B http) throws Exception {
+        updateAuthenticationDefaults();
         if(permitAll) {
             PermitAllSupport.permitAll(http, loginPage, loginProcessingUrl, failureUrl);
         }
+
         registerDefaultAuthenticationEntryPoint(http);
     }
 
@@ -289,54 +277,87 @@ public abstract class AbstractAuthenticationFilterConfigurer<B  extends HttpSecu
      * </p>
      */
     protected T loginPage(String loginPage) {
-        this.loginPage = loginPage;
+        setLoginPage(loginPage);
-        this.authenticationEntryPoint = new LoginUrlAuthenticationEntryPoint(loginPage);
+        updateAuthenticationDefaults();
         this.customLoginPage = true;
         return getSelf();
     }
 
     /**
-    *
+     *
-    * @return true if a custom login page has been specified, else false
+     * @return true if a custom login page has been specified, else false
-    */
+     */
-   public final boolean isCustomLoginPage() {
+    public final boolean isCustomLoginPage() {
-       return customLoginPage;
+        return customLoginPage;
-   }
+    }
 
-   /**
+    /**
-    * Gets the Authentication Filter
+     * Gets the Authentication Filter
-    * @return
+     *
-    */
+     * @return
-   protected final F getAuthenticationFilter() {
+     */
-       return authFilter;
+    protected final F getAuthenticationFilter() {
-   }
+        return authFilter;
+    }
 
-   /**
+    /**
-    * Gets the login page
+     * Gets the login page
-    * @return the login page
+     *
-    */
+     * @return the login page
-   protected final String getLoginPage() {
+     */
-       return loginPage;
+    protected final String getLoginPage() {
-   }
+        return loginPage;
+    }
 
-   /**
+    /**
-    * Gets the URL to submit an authentication request to (i.e. where
+     * Gets the URL to submit an authentication request to (i.e. where
-    * username/password must be submitted)
+     * username/password must be submitted)
-    *
+     *
-    * @return the URL to submit an authentication request to
+     * @return the URL to submit an authentication request to
-    */
+     */
-   protected final String getLoginProcessingUrl() {
+    protected final String getLoginProcessingUrl() {
-       return loginProcessingUrl;
+        return loginProcessingUrl;
-   }
+    }
 
-   /**
+    /**
-    * Gets the URL to send users to if authentication fails
+     * Gets the URL to send users to if authentication fails
-    * @return
+     *
-    */
+     * @return
-   protected final String getFailureUrl() {
+     */
-       return failureUrl;
+    protected final String getFailureUrl() {
-   }
+        return failureUrl;
+    }
 
+    /**
+     * Updates the default values for authentication.
+     *
+     * @throws Exception
+     */
+    private void updateAuthenticationDefaults() {
+        if (loginProcessingUrl == null) {
+            loginProcessingUrl(loginPage);
+        }
+        if (failureHandler == null) {
+            failureUrl(loginPage + "?error");
+        }
+
+        final LogoutConfigurer<B> logoutConfigurer = getBuilder()
+                .getConfigurer(LogoutConfigurer.class);
+        if (logoutConfigurer != null
+                && !logoutConfigurer.isCustomLogoutSuccess()) {
+            logoutConfigurer.logoutSuccessUrl(loginPage + "?logout");
+        }
+    }
+
+    /**
+     * Sets the loginPage and updates the {@link AuthenticationEntryPoint}.
+     * @param loginPage
+     */
+    private void setLoginPage(String loginPage) {
+        this.loginPage = loginPage;
+        this.authenticationEntryPoint = new LoginUrlAuthenticationEntryPoint(
+                loginPage);
+    }
 
     @SuppressWarnings("unchecked")
     private T getSelf() {

config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java

@@ -70,7 +70,7 @@ public final class FormLoginConfigurer<H extends HttpSecurityBuilder<H>> extends
      * @see HttpSecurity#formLogin()
      */
     public FormLoginConfigurer() {
-        super(new UsernamePasswordAuthenticationFilter(),"/login");
+        super(new UsernamePasswordAuthenticationFilter(),null);
         usernameParameter("username");
         passwordParameter("password");
     }
@@ -147,6 +147,31 @@ public final class FormLoginConfigurer<H extends HttpSecurityBuilder<H>> extends
      * &lt;/form&gt;
      * </pre>
      *
+     * <h2>Impact on other defaults</h2>
+     *
+     * Updating this value, also impacts a number of other default values. For example,
+     * the following are the default values when only formLogin() was specified.
+     *
+     * <ul>
+     * <li>/login GET - the login form</li>
+     * <li>/login POST - process the credentials and if valid authenticate the
+     * user</li>
+     * <li>/login?error GET - redirect here for failed authentication attempts</li>
+     * <li>/login?logout GET - redirect here after successfully logging out</li>
+     * </ul>
+     *
+     * If "/authenticate" was passed to this method it update the defaults as shown
+     * below:
+     *
+     * <ul>
+     * <li>/authenticate GET - the login form</li>
+     * <li>/authenticate POST - process the credentials and if valid authenticate the
+     * user</li>
+     * <li>/authenticate?error GET - redirect here for failed authentication attempts</li>
+     * <li>/authenticate?logout GET - redirect here after successfully logging out</li>
+     * </ul>
+     *
+     *
      * @param loginPage
      *            the login page to redirect to if authentication is required
      *            (i.e. "/login")

config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java

@@ -223,7 +223,7 @@ public final class LogoutConfigurer<H extends HttpSecurityBuilder<H>> extends Ab
      *
      * @return true if logout success handling has been customized, else false
      */
-    private boolean isCustomLogoutSuccess() {
+    boolean isCustomLogoutSuccess() {
         return customLogoutSuccess;
     }
 

config/src/main/java/org/springframework/security/config/annotation/web/configurers/openid/OpenIDLoginConfigurer.java

@@ -225,6 +225,31 @@ public final class OpenIDLoginConfigurer<H extends HttpSecurityBuilder<H>> exten
      * {@link OpenIDAuthenticationFilter#DEFAULT_CLAIMED_IDENTITY_FIELD}</li>
      * </ul>
      *
+     *
+     * <h2>Impact on other defaults</h2>
+     *
+     * Updating this value, also impacts a number of other default values. For example,
+     * the following are the default values when only formLogin() was specified.
+     *
+     * <ul>
+     * <li>/login GET - the login form</li>
+     * <li>/login POST - process the credentials and if valid authenticate the
+     * user</li>
+     * <li>/login?error GET - redirect here for failed authentication attempts</li>
+     * <li>/login?logout GET - redirect here after successfully logging out</li>
+     * </ul>
+     *
+     * If "/authenticate" was passed to this method it update the defaults as shown
+     * below:
+     *
+     * <ul>
+     * <li>/authenticate GET - the login form</li>
+     * <li>/authenticate POST - process the credentials and if valid authenticate the
+     * user</li>
+     * <li>/authenticate?error GET - redirect here for failed authentication attempts</li>
+     * <li>/authenticate?logout GET - redirect here after successfully logging out</li>
+     * </ul>
+     *
      * @param loginPage the login page to redirect to if authentication is required (i.e. "/login")
      * @return the {@link FormLoginConfigurer} for additional customization
      */

config/src/test/groovy/org/springframework/security/config/annotation/web/SampleWebSecurityConfigurerAdapterTests.groovy

@@ -173,7 +173,7 @@ public class SampleWebSecurityConfigurerAdapterTests extends BaseSpringSpec {
                     .anyRequest().hasRole("USER")
                     .and()
                 .formLogin()
-                    .loginUrl("/login")
+                    .loginPage("/login")
                     // set permitAll for all URLs associated with Form Login
                     .permitAll();
         }
@@ -314,7 +314,7 @@ public class SampleWebSecurityConfigurerAdapterTests extends BaseSpringSpec {
                         .anyRequest().hasRole("USER")
                         .and()
                     .formLogin()
-                        .loginUrl("/login")
+                        .loginPage("/login")
                         .permitAll();
             }
         }

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

@@ -41,7 +41,7 @@ import org.springframework.security.web.authentication.logout.LogoutFilter
 import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy
 import org.springframework.security.web.context.SecurityContextPersistenceFilter
 import org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter
-import org.springframework.security.web.csrf.CsrfFilter;
+import org.springframework.security.web.csrf.CsrfFilter
 import org.springframework.security.web.header.HeaderWriterFilter
 import org.springframework.security.web.savedrequest.RequestCacheAwareFilter
 import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter
@@ -49,6 +49,8 @@ import org.springframework.security.web.session.SessionManagementFilter
 import org.springframework.security.web.util.AnyRequestMatcher
 import org.springframework.test.util.ReflectionTestUtils
 
+import spock.lang.Unroll
+
 /**
  *
  * @author Rob Winch
@@ -106,7 +108,7 @@ class FormLoginConfigurerTests extends BaseSpringSpec {
                     .anyRequest().hasRole("USER")
                     .and()
                 .formLogin()
-                    .loginUrl("/login")
+                    .loginPage("/login")
         }
     }
 
@@ -144,6 +146,46 @@ class FormLoginConfigurerTests extends BaseSpringSpec {
         }
     }
 
+    @Unroll
+    def "FormLogin loginConventions changes defaults"() {
+        when: "load formLogin() with permitAll"
+            loadConfig(FormLoginDefaultsConfig)
+            MockHttpServletResponse response = new MockHttpServletResponse()
+            request = new MockHttpServletRequest(servletPath : servletPath, requestURI: servletPath, queryString: query, method: method)
+            setupCsrf()
+
+        then: "the other default login/logout URLs are updated and granted access"
+            springSecurityFilterChain.doFilter(request, response, new MockFilterChain())
+            response.redirectedUrl == redirectUrl
+
+        where:
+            servletPath     | method | query | redirectUrl
+            "/authenticate" | "GET"  | null    | null
+            "/authenticate" | "POST" | null    | "/authenticate?error"
+            "/authenticate" | "GET"  | "error" | null
+            "/logout"       | "POST" | null    | "/authenticate?logout"
+            "/authenticate" | "GET"  | "logout"| null
+    }
+
+    @Configuration
+    @EnableWebSecurity
+    static class FormLoginDefaultsConfig extends BaseWebConfig {
+
+        @Override
+        protected void configure(HttpSecurity http) {
+            http
+                .authorizeRequests()
+                    .anyRequest().hasRole("USER")
+                    .and()
+                .formLogin()
+                    .loginPage("/authenticate")
+                    .permitAll()
+                    .and()
+                .logout()
+                    .permitAll()
+        }
+    }
+
     def "FormLogin uses PortMapper"() {
         when: "load formLogin() with permitAll"
             FormLoginUsesPortMapperConfig.PORT_MAPPER = Mock(PortMapper)

samples/openid-jc/src/main/java/org/springframework/security/samples/config/SecurityConfig.java

@@ -18,7 +18,6 @@ public class SecurityConfig extends WebSecurityConfigurerAdapter {
                 .anyRequest().authenticated()
                 .and()
             .openidLogin()
-                .loginPage("/login")
                 .permitAll()
                 .authenticationUserDetailsService(new CustomUserDetailsService())
                 .attributeExchange("https://www.google.com/.*")