diff --git a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java index 69d7bf34cb..efab8e7b31 100644 --- a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java @@ -331,10 +331,6 @@ final class AuthenticationConfigBuilder { void createLoginPageFilterIfNeeded() { boolean needLoginPage = formFilter != null || openIDFilter != null; 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); // 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 - // has been set + // has been set. + String formLoginPage = getLoginFormUrl(formEntryPoint); 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) { return formEntryPoint; } // Otherwise use OpenID if enabled - if (openIDFilter != null && formFilter == null) { + if (openIDFilter != null) { return openIDEntryPoint; } @@ -533,6 +535,11 @@ final class AuthenticationConfigBuilder { 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(); } diff --git a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java index 24f8220e4c..463f0a8cb0 100644 --- a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java @@ -993,6 +993,52 @@ public class HttpSecurityBeanDefinitionParserTests { "", appContext); } + @Test + public void openIDAndFormLoginWorkTogether() throws Exception { + setContext( + "" + + " " + + " " + + "" + + 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( + "" + + " " + + " " + + "" + + 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( + "" + + " " + + " " + + "" + + AUTH_PROVIDER_XML); + ExceptionTranslationFilter etf = (ExceptionTranslationFilter) getFilter(ExceptionTranslationFilter.class); + LoginUrlAuthenticationEntryPoint ap = (LoginUrlAuthenticationEntryPoint) etf.getAuthenticationEntryPoint(); + assertEquals("/openid_login", ap.getLoginFormUrl()); + } + @SuppressWarnings("unchecked") @Test public void openIDWithAttributeExchangeConfigurationIsParsedCorrectly() throws Exception { @@ -1018,6 +1064,15 @@ public class HttpSecurityBeanDefinitionParserTests { assertEquals(2, attributes.get(1).getCount()); } + @Test(expected=BeanDefinitionParsingException.class) + public void multipleLoginPagesCausesError() throws Exception { + setContext( + "" + + " " + + " " + + "" + + AUTH_PROVIDER_XML); + } private void setContext(String context) { appContext = new InMemoryXmlApplicationContext(context); 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 1f7ca09cc9..430c1b127f 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 @@ -67,7 +67,7 @@ public class DefaultLoginPageGeneratingFilter extends GenericFilterBean { if (openIDFilter != null) { openIdEnabled = true; openIDauthenticationUrl = openIDFilter.getFilterProcessesUrl(); - openIDusernameParameter = (String) (new BeanWrapperImpl(openIDFilter)).getPropertyValue("claimedIdentityFieldName"); + openIDusernameParameter = "j_username"; if (openIDFilter.getRememberMeServices() instanceof AbstractRememberMeServices) { openIDrememberMeParameter = ((AbstractRememberMeServices)openIDFilter.getRememberMeServices()).getParameter();