浏览代码

SEC-1269: Combining <form-login> and <open-id> fails to find entry point. Fixed entry point choice conditions when using openID and/or form-login

Luke Taylor 16 年之前
父节点
当前提交
799b96520b

+ 13 - 6
config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java

@@ -331,10 +331,6 @@ final class AuthenticationConfigBuilder {
     void createLoginPageFilterIfNeeded() {
     void createLoginPageFilterIfNeeded() {
         boolean needLoginPage = formFilter != null || openIDFilter != null;
         boolean needLoginPage = formFilter != null || openIDFilter != null;
         String formLoginPage = getLoginFormUrl(formEntryPoint);
         String formLoginPage = getLoginFormUrl(formEntryPoint);
-        // If the login URL is the default one, then it is assumed not to have been set explicitly
-        if (DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL == formLoginPage) {
-            formLoginPage = null;
-        }
         String openIDLoginPage = getLoginFormUrl(openIDEntryPoint);
         String openIDLoginPage = getLoginFormUrl(openIDEntryPoint);
 
 
         // If no login page has been defined, add in the default page generator.
         // If no login page has been defined, add in the default page generator.
@@ -498,15 +494,21 @@ final class AuthenticationConfigBuilder {
         }
         }
 
 
         // If formLogin has been enabled either through an element or auto-config, then it is used if no openID login page
         // If formLogin has been enabled either through an element or auto-config, then it is used if no openID login page
-        // has been set
+        // has been set.
+        String formLoginPage = getLoginFormUrl(formEntryPoint);
         String openIDLoginPage = getLoginFormUrl(openIDEntryPoint);
         String openIDLoginPage = getLoginFormUrl(openIDEntryPoint);
 
 
+        if (formLoginPage != null && openIDLoginPage != null) {
+            pc.getReaderContext().error("Only one login-page can be defined, either for OpenID or form-login, " +
+                    "but not both.", pc.extractSource(openIDLoginElt));
+        }
+
         if (formFilter != null && openIDLoginPage == null) {
         if (formFilter != null && openIDLoginPage == null) {
             return formEntryPoint;
             return formEntryPoint;
         }
         }
 
 
         // Otherwise use OpenID if enabled
         // Otherwise use OpenID if enabled
-        if (openIDFilter != null && formFilter == null) {
+        if (openIDFilter != null) {
             return openIDEntryPoint;
             return openIDEntryPoint;
         }
         }
 
 
@@ -533,6 +535,11 @@ final class AuthenticationConfigBuilder {
              return null;
              return null;
         }
         }
 
 
+        // If the login URL is the default one, then it is assumed not to have been set explicitly
+        if (DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL.equals(pv.getValue())) {
+            return null;
+        }
+
         return (String) pv.getValue();
         return (String) pv.getValue();
     }
     }
 
 

+ 55 - 0
config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java

@@ -993,6 +993,52 @@ public class HttpSecurityBeanDefinitionParserTests {
                 "</http>", appContext);
                 "</http>", appContext);
     }
     }
 
 
+    @Test
+    public void openIDAndFormLoginWorkTogether() throws Exception {
+        setContext(
+                "<http>" +
+                "   <openid-login />" +
+                "   <form-login />" +
+                "</http>" +
+                AUTH_PROVIDER_XML);
+        ExceptionTranslationFilter etf = (ExceptionTranslationFilter) getFilter(ExceptionTranslationFilter.class);
+        LoginUrlAuthenticationEntryPoint ap = (LoginUrlAuthenticationEntryPoint) etf.getAuthenticationEntryPoint();
+        assertEquals("/spring_security_login", ap.getLoginFormUrl());
+        // Default login filter should be present since we haven't specified any login URLs
+        getFilter(DefaultLoginPageGeneratingFilter.class);
+    }
+
+    @Test
+    public void formLoginEntryPointTakesPrecedenceIfLoginUrlIsSet() throws Exception {
+        setContext(
+                "<http>" +
+                "   <openid-login />" +
+                "   <form-login login-page='/form_login_page' />" +
+                "</http>" +
+                AUTH_PROVIDER_XML);
+        ExceptionTranslationFilter etf = (ExceptionTranslationFilter) getFilter(ExceptionTranslationFilter.class);
+        LoginUrlAuthenticationEntryPoint ap = (LoginUrlAuthenticationEntryPoint) etf.getAuthenticationEntryPoint();
+        assertEquals("/form_login_page", ap.getLoginFormUrl());
+        try {
+            getFilter(DefaultLoginPageGeneratingFilter.class);
+            fail("Login page generating filter shouldn't be present");
+        } catch (Exception expected) {
+        }
+    }
+
+    @Test
+    public void openIDEntryPointTakesPrecedenceIfLoginUrlIsSet() throws Exception {
+        setContext(
+                "<http>" +
+                "   <openid-login login-page='/openid_login' />" +
+                "   <form-login />" +
+                "</http>" +
+                AUTH_PROVIDER_XML);
+        ExceptionTranslationFilter etf = (ExceptionTranslationFilter) getFilter(ExceptionTranslationFilter.class);
+        LoginUrlAuthenticationEntryPoint ap = (LoginUrlAuthenticationEntryPoint) etf.getAuthenticationEntryPoint();
+        assertEquals("/openid_login", ap.getLoginFormUrl());
+    }
+
     @SuppressWarnings("unchecked")
     @SuppressWarnings("unchecked")
     @Test
     @Test
     public void openIDWithAttributeExchangeConfigurationIsParsedCorrectly() throws Exception {
     public void openIDWithAttributeExchangeConfigurationIsParsedCorrectly() throws Exception {
@@ -1018,6 +1064,15 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertEquals(2, attributes.get(1).getCount());
         assertEquals(2, attributes.get(1).getCount());
     }
     }
 
 
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void multipleLoginPagesCausesError() throws Exception {
+        setContext(
+                "<http>" +
+                "   <openid-login login-page='/openid_login_page' />" +
+                "   <form-login login-page='/form_login_page' />" +
+                "</http>" +
+                AUTH_PROVIDER_XML);
+    }
 
 
     private void setContext(String context) {
     private void setContext(String context) {
         appContext = new InMemoryXmlApplicationContext(context);
         appContext = new InMemoryXmlApplicationContext(context);

+ 1 - 1
web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java

@@ -67,7 +67,7 @@ public class DefaultLoginPageGeneratingFilter extends GenericFilterBean {
         if (openIDFilter != null) {
         if (openIDFilter != null) {
             openIdEnabled = true;
             openIdEnabled = true;
             openIDauthenticationUrl = openIDFilter.getFilterProcessesUrl();
             openIDauthenticationUrl = openIDFilter.getFilterProcessesUrl();
-            openIDusernameParameter = (String) (new BeanWrapperImpl(openIDFilter)).getPropertyValue("claimedIdentityFieldName");
+            openIDusernameParameter = "j_username";
 
 
             if (openIDFilter.getRememberMeServices() instanceof AbstractRememberMeServices) {
             if (openIDFilter.getRememberMeServices() instanceof AbstractRememberMeServices) {
                 openIDrememberMeParameter = ((AbstractRememberMeServices)openIDFilter.getRememberMeServices()).getParameter();
                 openIDrememberMeParameter = ((AbstractRememberMeServices)openIDFilter.getRememberMeServices()).getParameter();