diff --git a/config/src/main/java/org/springframework/security/config/Elements.java b/config/src/main/java/org/springframework/security/config/Elements.java index 60e0b7371c4..74b638fb2b6 100644 --- a/config/src/main/java/org/springframework/security/config/Elements.java +++ b/config/src/main/java/org/springframework/security/config/Elements.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,8 +32,11 @@ public abstract class Elements { public static final String USER_SERVICE = "user-service"; + @Deprecated(since = "For removal in 7.0.") public static final String JDBC_USER_SERVICE = "jdbc-user-service"; + public static final String JDBC_USER_PASSWORD_SERVICE = "jdbc-user-password-service"; + public static final String FILTER_CHAIN_MAP = "filter-chain-map"; public static final String INTERCEPT_METHODS = "intercept-methods"; diff --git a/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java b/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java index 260b46a4cd7..a76676c6b34 100644 --- a/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java +++ b/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2022 the original author or authors. + * Copyright 2009-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ import org.springframework.beans.factory.xml.ParserContext; import org.springframework.security.config.authentication.AuthenticationManagerBeanDefinitionParser; import org.springframework.security.config.authentication.AuthenticationProviderBeanDefinitionParser; +import org.springframework.security.config.authentication.JdbcUserPasswordDetailsManagerBeanDefinitionParser; import org.springframework.security.config.authentication.JdbcUserServiceBeanDefinitionParser; import org.springframework.security.config.authentication.UserServiceBeanDefinitionParser; import org.springframework.security.config.http.FilterChainBeanDefinitionParser; @@ -173,6 +174,7 @@ private void loadParsers() { this.parsers.put(Elements.LDAP_USER_SERVICE, new LdapUserServiceBeanDefinitionParser()); this.parsers.put(Elements.USER_SERVICE, new UserServiceBeanDefinitionParser()); this.parsers.put(Elements.JDBC_USER_SERVICE, new JdbcUserServiceBeanDefinitionParser()); + this.parsers.put(Elements.JDBC_USER_PASSWORD_SERVICE, new JdbcUserPasswordDetailsManagerBeanDefinitionParser()); this.parsers.put(Elements.AUTHENTICATION_PROVIDER, new AuthenticationProviderBeanDefinitionParser()); this.parsers.put(Elements.GLOBAL_METHOD_SECURITY, new GlobalMethodSecurityBeanDefinitionParser()); this.parsers.put(Elements.METHOD_SECURITY, new MethodSecurityBeanDefinitionParser()); diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/builders/AuthenticationManagerBuilder.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/builders/AuthenticationManagerBuilder.java index 73e8a31a449..6a43226f225 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/builders/AuthenticationManagerBuilder.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/builders/AuthenticationManagerBuilder.java @@ -149,13 +149,17 @@ public InMemoryUserDetailsManagerConfigurer inMemo * {@link #getDefaultUserDetailsService()} method. Note that additional * {@link UserDetailsService}'s may override this {@link UserDetailsService} as the * default. See the User Schema section of the reference for the default schema. *

* @return a {@link JdbcUserDetailsManagerConfigurer} to allow customization of the * JDBC authentication * @throws Exception if an error occurs when adding the JDBC authentication + * @deprecated JdbcUserDetailsManager has been superseded by + * JdbcUserPasswordDetailsManager, and does not support DSL configuration. Please + * declare a bean instead. */ + @Deprecated(since = "For removal in 7.0.") public JdbcUserDetailsManagerConfigurer jdbcAuthentication() throws Exception { return apply(new JdbcUserDetailsManagerConfigurer<>()); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java index e85fdb0886a..93197f6260b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -288,7 +288,13 @@ public InMemoryUserDetailsManagerConfigurer inMemo return super.inMemoryAuthentication().passwordEncoder(this.defaultPasswordEncoder); } + /** + * @deprecated Use JdbcUserPasswordDetailsManager instead, as this keeps the + * password up to date. Please consult the migration documentation as database + * changes might be necessary. + */ @Override + @Deprecated(since = "For removal in 7.0.") public JdbcUserDetailsManagerConfigurer jdbcAuthentication() throws Exception { return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserPasswordDetailsManagerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserPasswordDetailsManagerConfigurer.java new file mode 100644 index 00000000000..abfbf3e5460 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserPasswordDetailsManagerConfigurer.java @@ -0,0 +1,207 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.config.annotation.authentication.configurers.provisioning; + +import java.util.ArrayList; +import java.util.List; + +import javax.sql.DataSource; + +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; +import org.springframework.jdbc.datasource.init.DataSourceInitializer; +import org.springframework.jdbc.datasource.init.DatabasePopulator; +import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; +import org.springframework.security.config.annotation.authentication.ProviderManagerBuilder; +import org.springframework.security.core.userdetails.UserCache; +import org.springframework.security.provisioning.JdbcUserPasswordDetailsManager; + +/** + * Configures an + * {@link org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder} + * to have JDBC authentication, with user management that will automatically update the + * encoding of a password if necessary. It also allows easily adding users to the database + * used for authentication and setting up the schema. + * + *

+ * The only required method is the {@link #dataSource(javax.sql.DataSource)} all other + * methods have reasonable defaults. + * + * @param the type of the {@link ProviderManagerBuilder} that is being configured + * @author Rob Winch + * @author Geir Hedemark + * @since 6.3 + */ +public final class JdbcUserPasswordDetailsManagerConfigurer> + extends UserDetailsManagerConfigurer> { + + private DataSource dataSource; + + private List initScripts = new ArrayList<>(); + + public JdbcUserPasswordDetailsManagerConfigurer(JdbcUserPasswordDetailsManager manager) { + super(manager); + } + + public JdbcUserPasswordDetailsManagerConfigurer() { + this(new JdbcUserPasswordDetailsManager()); + } + + /** + * Populates the {@link DataSource} to be used. This is the only required attribute. + * @param dataSource the {@link DataSource} to be used. Cannot be null. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer dataSource(DataSource dataSource) { + this.dataSource = dataSource; + getUserDetailsService().setDataSource(dataSource); + return this; + } + + /** + * Sets the query to be used for finding a user by their username. For example: + * + * + * select username,password,enabled from users where username = ? + * + * @param query The query to use for selecting the username, password, and if the user + * is enabled by username. Must contain a single parameter for the username. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer usersByUsernameQuery(String query) { + getUserDetailsService().setUsersByUsernameQuery(query); + return this; + } + + /** + * Sets the query to be used for updating a password for a user. For example: + * + * + * update users set password = ? where username = ? + * + * @param query The query to use for setting the password for a user. Must contain a + * parameter for the password, and one for the username. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer changePasswordQuery(String query) { + getUserDetailsService().setChangePasswordQuery(query); + return this; + } + + /** + * Sets the query to be used for finding a user's authorities by their username. For + * example: + * + * + * select username,authority from authorities where username = ? + * + * @param query The query to use for selecting the username, authority by username. + * Must contain a single parameter for the username. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer authoritiesByUsernameQuery(String query) { + getUserDetailsService().setAuthoritiesByUsernameQuery(query); + return this; + } + + /** + * An SQL statement to query user's group authorities given a username. For example: + * + * + * select + * g.id, g.group_name, ga.authority + * from + * groups g, group_members gm, group_authorities ga + * where + * gm.username = ? and g.id = ga.group_id and g.id = gm.group_id + * + * @param query The query to use for selecting the authorities by group. Must contain + * a single parameter for the username. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { + JdbcUserPasswordDetailsManager userDetailsService = getUserDetailsService(); + userDetailsService.setEnableGroups(true); + userDetailsService.setGroupAuthoritiesByUsernameQuery(query); + return this; + } + + /** + * A non-empty string prefix that will be added to role strings loaded from persistent + * storage (default is ""). + * @param rolePrefix + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer rolePrefix(String rolePrefix) { + getUserDetailsService().setRolePrefix(rolePrefix); + return this; + } + + /** + * Defines the {@link UserCache} to use + * @param userCache the {@link UserCache} to use + * @return the {@link JdbcUserPasswordDetailsManagerConfigurer} for further + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer userCache(UserCache userCache) { + getUserDetailsService().setUserCache(userCache); + return this; + } + + @Override + protected void initUserDetailsService() throws Exception { + if (!this.initScripts.isEmpty()) { + getDataSourceInit().afterPropertiesSet(); + } + super.initUserDetailsService(); + } + + @Override + public JdbcUserPasswordDetailsManager getUserDetailsService() { + return (JdbcUserPasswordDetailsManager) super.getUserDetailsService(); + } + + /** + * Populates the default schema that allows users and authorities to be stored. + * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional + * customizations + */ + public JdbcUserPasswordDetailsManagerConfigurer withDefaultSchema() { + this.initScripts.add(new ClassPathResource("org/springframework/security/core/userdetails/jdbc/users.ddl")); + return this; + } + + protected DatabasePopulator getDatabasePopulator() { + ResourceDatabasePopulator dbp = new ResourceDatabasePopulator(); + dbp.setScripts(this.initScripts.toArray(new Resource[0])); + return dbp; + } + + private DataSourceInitializer getDataSourceInit() { + DataSourceInitializer dsi = new DataSourceInitializer(); + dsi.setDatabasePopulator(getDatabasePopulator()); + dsi.setDataSource(this.dataSource); + return dsi; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/HttpSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/HttpSecurityConfiguration.java index ba962a4fad8..b616a5b230b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/HttpSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/HttpSecurityConfiguration.java @@ -184,7 +184,13 @@ public InMemoryUserDetailsManagerConfigurer inMemo return super.inMemoryAuthentication().passwordEncoder(this.defaultPasswordEncoder); } + /** + * @deprecated Use jdbcAuthenticationWithPasswordManagement instead, as this keeps + * the password up to date. Please consult the migration documentation as database + * changes might be necessary. + */ @Override + @Deprecated(since = "For removal in 7.0.") public JdbcUserDetailsManagerConfigurer jdbcAuthentication() throws Exception { return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } diff --git a/config/src/main/java/org/springframework/security/config/authentication/AuthenticationProviderBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/AuthenticationProviderBeanDefinitionParser.java index 794f30d179a..cd2fb44e10c 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/AuthenticationProviderBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/AuthenticationProviderBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -50,6 +50,9 @@ public BeanDefinition parse(Element element, ParserContext pc) { authProvider.getPropertyValues().addPropertyValue("passwordEncoder", passwordEncoder); } Element userServiceElt = DomUtils.getChildElementByTagName(element, Elements.USER_SERVICE); + if (userServiceElt == null) { + userServiceElt = DomUtils.getChildElementByTagName(element, Elements.JDBC_USER_PASSWORD_SERVICE); + } if (userServiceElt == null) { userServiceElt = DomUtils.getChildElementByTagName(element, Elements.JDBC_USER_SERVICE); } diff --git a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java new file mode 100644 index 00000000000..f7ebe2c372a --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -0,0 +1,79 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.config.authentication; + +import org.w3c.dom.Element; + +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.security.config.Elements; +import org.springframework.util.StringUtils; + +/** + * @author Luke Taylor + * @author Geir Hedemark + */ +public final class JdbcUserPasswordDetailsManagerBeanDefinitionParser + extends AbstractUserDetailsServiceBeanDefinitionParser { + + static final String ATT_DATA_SOURCE = "data-source-ref"; + static final String ATT_USERS_BY_USERNAME_QUERY = "users-by-username-query"; + static final String ATT_AUTHORITIES_BY_USERNAME_QUERY = "authorities-by-username-query"; + static final String ATT_GROUP_AUTHORITIES_QUERY = "group-authorities-by-username-query"; + static final String ATT_CHANGE_PASSWORD_QUERY = "change-password-query"; + static final String ATT_ROLE_PREFIX = "role-prefix"; + + @Override + protected String getBeanClassName(Element element) { + return "org.springframework.security.provisioning.JdbcUserPasswordDetailsManager"; + } + + @Override + protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) { + String dataSource = element.getAttribute(ATT_DATA_SOURCE); + if (dataSource != null) { + builder.addPropertyReference("dataSource", dataSource); + } + else { + parserContext.getReaderContext() + .error(ATT_DATA_SOURCE + " is required for " + Elements.JDBC_USER_PASSWORD_SERVICE, + parserContext.extractSource(element)); + } + String usersQuery = element.getAttribute(ATT_USERS_BY_USERNAME_QUERY); + String authoritiesQuery = element.getAttribute(ATT_AUTHORITIES_BY_USERNAME_QUERY); + String groupAuthoritiesQuery = element.getAttribute(ATT_GROUP_AUTHORITIES_QUERY); + String changePasswordQuery = element.getAttribute(ATT_CHANGE_PASSWORD_QUERY); + String rolePrefix = element.getAttribute(ATT_ROLE_PREFIX); + if (StringUtils.hasText(rolePrefix)) { + builder.addPropertyValue("rolePrefix", rolePrefix); + } + if (StringUtils.hasText(usersQuery)) { + builder.addPropertyValue("usersByUsernameQuery", usersQuery); + } + if (StringUtils.hasText(authoritiesQuery)) { + builder.addPropertyValue("authoritiesByUsernameQuery", authoritiesQuery); + } + if (StringUtils.hasText(groupAuthoritiesQuery)) { + builder.addPropertyValue("enableGroups", Boolean.TRUE); + builder.addPropertyValue("groupAuthoritiesByUsernameQuery", groupAuthoritiesQuery); + } + if (StringUtils.hasText(changePasswordQuery)) { + builder.addPropertyValue("changePasswordQuery", changePasswordQuery); + } + } + +} diff --git a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java index a0d7de83f65..7e62981dbce 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserServiceBeanDefinitionParser.java @@ -25,7 +25,11 @@ /** * @author Luke Taylor + * @deprecated Use {@link JdbcUserPasswordDetailsManagerBeanDefinitionParser} instead, as + * this will update password encodings to more safe variants as users log in */ +@Deprecated( + since = "For removal in 7.0. Use JdbcUserPasswordDetailsManagerBeanDefinitionParser instead, as this will update password encodings to more safe variants as users log in") public class JdbcUserServiceBeanDefinitionParser extends AbstractUserDetailsServiceBeanDefinitionParser { static final String ATT_DATA_SOURCE = "data-source-ref"; diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-6.3.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-6.3.rnc index 7f89ced5afd..9a35a6f2be5 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-6.3.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-6.3.rnc @@ -1123,6 +1123,29 @@ jdbc-user-service.attlist &= jdbc-user-service.attlist &= role-prefix? +jdbc-user-password-service = + ## Causes creation of a JDBC-based UserPasswordDetailsService. + element jdbc-user-password-service {id? & jdbc-user-password-service.attlist} +jdbc-user-password-service.attlist &= + ## The bean ID of the DataSource which provides the required tables. + attribute data-source-ref {xsd:token} +jdbc-user-password-service.attlist &= + cache-ref? +jdbc-user-password-service.attlist &= + ## An SQL statement to query a username, password, and enabled status given a username. Default is "select username,password,enabled from users where username = ?" + attribute users-by-username-query {xsd:token}? +jdbc-user-password-service.attlist &= + ## An SQL statement to query for a user's granted authorities given a username. The default is "select username, authority from authorities where username = ?" + attribute authorities-by-username-query {xsd:token}? +jdbc-user-password-service.attlist &= + ## An SQL statement to query user's group authorities given a username. The default is "select g.id, g.group_name, ga.authority from groups g, group_members gm, group_authorities ga where gm.username = ? and g.id = ga.group_id and g.id = gm.group_id" + attribute group-authorities-by-username-query {xsd:token}? +jdbc-user-password-service.attlist &= + ## An SQL statement to update a users' password given a username. The default is "update users set password = ? where username = ?" + attribute change-password-query {xsd:token}? +jdbc-user-password-service.attlist &= + role-prefix? + csrf = ## Element for configuration of the CsrfFilter for protection against CSRF. It also updates the default RequestCache to only replay "GET" requests. element csrf {csrf-options.attlist} @@ -1321,7 +1344,7 @@ header.attlist &= ## Reference to a custom HeaderWriter implementation. ref? -any-user-service = user-service | jdbc-user-service | ldap-user-service +any-user-service = user-service | jdbc-user-service | jdbc-user-password-service | ldap-user-service custom-filter = ## Used to indicate that a filter bean declaration should be incorporated into the security filter chain. diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-6.3.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-6.3.xsd index f123ad830a7..d63b70006c8 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-6.3.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-6.3.xsd @@ -3159,6 +3159,72 @@ + + + Causes creation of a JDBC-based UserPasswordDetailsService. + + + + + + A bean identifier, used for referring to the bean elsewhere in the context. + + + + + + + + + + The bean ID of the DataSource which provides the required tables. + + + + + + Defines a reference to a cache for use with a UserDetailsService. + + + + + + An SQL statement to query a username, password, and enabled status given a username. + Default is "select username,password,enabled from users where username = ?" + + + + + + An SQL statement to query for a user's granted authorities given a username. The default + is "select username, authority from authorities where username = ?" + + + + + + An SQL statement to query user's group authorities given a username. The default is + "select g.id, g.group_name, ga.authority from groups g, group_members gm, + group_authorities ga where gm.username = ? and g.id = ga.group_id and g.id = gm.group_id" + + + + + + An SQL statement to update a users' password given a username. The default is "update + users set password = ? where username = ?" + + + + + + A non-empty string prefix that will be added to role strings loaded from persistent + storage (e.g. "ROLE_"). Use the value "none" for no prefix in cases where the default is + non-empty. + + + + Element for configuration of the CsrfFilter for protection against CSRF. It also updates diff --git a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java new file mode 100644 index 00000000000..058674ce466 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -0,0 +1,178 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.config.authentication; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.w3c.dom.Element; + +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.CachingUserDetailsService; +import org.springframework.security.authentication.ProviderManager; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.authentication.dao.DaoAuthenticationProvider; +import org.springframework.security.config.BeanIds; +import org.springframework.security.config.util.InMemoryXmlApplicationContext; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.provisioning.JdbcUserPasswordDetailsManager; +import org.springframework.security.util.FieldUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +/** + * @author Ben Alex + * @author Luke Taylor + * @author Eddú Meléndez + * @author Geir Hedemark + */ +public class JdbcUserPasswordDetailsManagerBeanDefinitionParserTests { + + private static String USER_CACHE_XML = ""; + + // @formatter:off + private static String DATA_SOURCE = " " + + " " + + " " + + " " + + " " + + " "; + // @formatter:on + + private InMemoryXmlApplicationContext appContext; + + @AfterEach + public void closeAppContext() { + if (this.appContext != null) { + this.appContext.close(); + } + } + + @Test + public void beanNameIsCorrect() { + assertThat(JdbcUserPasswordDetailsManager.class.getName()) + .isEqualTo(new JdbcUserPasswordDetailsManagerBeanDefinitionParser().getBeanClassName(mock(Element.class))); + } + + @Test + public void validUsernameIsFound() { + setContext("" + DATA_SOURCE); + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext + .getBean(BeanIds.USER_DETAILS_SERVICE); + assertThat(mgr.loadUserByUsername("rod")).isNotNull(); + } + + @Test + public void beanIdIsParsedCorrectly() { + setContext("" + DATA_SOURCE); + assertThat(this.appContext.getBean("myUserService") instanceof JdbcUserPasswordDetailsManager).isTrue(); + } + + @Test + public void usernameAndAuthorityQueriesAreParsedCorrectly() throws Exception { + String userQuery = "select username, password, true from users where username = ?"; + String authoritiesQuery = "select username, authority from authorities where username = ? and 1 = 1"; + // @formatter:off + setContext("" + DATA_SOURCE); + // @formatter:on + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean("myUserService"); + assertThat(FieldUtils.getFieldValue(mgr, "usersByUsernameQuery")).isEqualTo(userQuery); + assertThat(FieldUtils.getFieldValue(mgr, "authoritiesByUsernameQuery")).isEqualTo(authoritiesQuery); + assertThat(mgr.loadUserByUsername("rod") != null).isTrue(); + } + + @Test + public void groupQueryIsParsedCorrectly() throws Exception { + setContext("" + DATA_SOURCE); + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean("myUserService"); + assertThat(FieldUtils.getFieldValue(mgr, "groupAuthoritiesByUsernameQuery")).isEqualTo("blah blah"); + assertThat((Boolean) FieldUtils.getFieldValue(mgr, "enableGroups")).isTrue(); + } + + @Test + public void changePasswordQueryIsParsedCorrectly() throws Exception { + setContext("" + DATA_SOURCE); + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean("myUserService"); + assertThat(FieldUtils.getFieldValue(mgr, "changePasswordQuery")).isEqualTo("blah blah"); + } + + @Test + public void cacheRefIsparsedCorrectly() { + setContext("" + + DATA_SOURCE + USER_CACHE_XML); + CachingUserDetailsService cachingUserService = (CachingUserDetailsService) this.appContext + .getBean("myUserService" + AbstractUserDetailsServiceBeanDefinitionParser.CACHING_SUFFIX); + assertThat(this.appContext.getBean("userCache")).isSameAs(cachingUserService.getUserCache()); + assertThat(cachingUserService.loadUserByUsername("rod")).isNotNull(); + assertThat(cachingUserService.loadUserByUsername("rod")).isNotNull(); + } + + @Test + public void isSupportedByAuthenticationProviderElement() { + // @formatter:off + setContext("" + + " " + + " " + + " " + + "" + + DATA_SOURCE); + // @formatter:on + AuthenticationManager mgr = (AuthenticationManager) this.appContext.getBean(BeanIds.AUTHENTICATION_MANAGER); + mgr.authenticate(UsernamePasswordAuthenticationToken.unauthenticated("rod", "koala")); + } + + @Test + public void cacheIsInjectedIntoAuthenticationProvider() { + // @formatter:off + setContext("" + + " " + + " " + + " " + + "" + + DATA_SOURCE + + USER_CACHE_XML); + // @formatter:on + ProviderManager mgr = (ProviderManager) this.appContext.getBean(BeanIds.AUTHENTICATION_MANAGER); + DaoAuthenticationProvider provider = (DaoAuthenticationProvider) mgr.getProviders().get(0); + assertThat(this.appContext.getBean("userCache")).isSameAs(provider.getUserCache()); + provider.authenticate(UsernamePasswordAuthenticationToken.unauthenticated("rod", "koala")); + assertThat(provider.getUserCache().getUserFromCache("rod")).isNotNull() + .withFailMessage("Cache should contain user after authentication"); + } + + @Test + public void rolePrefixIsUsedWhenSet() { + setContext("" + + DATA_SOURCE); + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean("myUserService"); + UserDetails rod = mgr.loadUserByUsername("rod"); + assertThat(AuthorityUtils.authorityListToSet(rod.getAuthorities())).contains("PREFIX_ROLE_SUPERVISOR"); + } + + private void setContext(String context) { + System.out.println("Context: " + context); + this.appContext = new InMemoryXmlApplicationContext(context); + } + +} diff --git a/config/src/test/java/org/springframework/security/config/doc/XsdDocumentedTests.java b/config/src/test/java/org/springframework/security/config/doc/XsdDocumentedTests.java index 2ac71e58266..8d1790bc505 100644 --- a/config/src/test/java/org/springframework/security/config/doc/XsdDocumentedTests.java +++ b/config/src/test/java/org/springframework/security/config/doc/XsdDocumentedTests.java @@ -150,9 +150,11 @@ public void sizeWhenReadingFilesystemThenIsCorrectNumberOfSchemaFiles() throws I .getParentFile() .list((dir, name) -> name.endsWith(".xsd")); // @formatter:on - assertThat(schemas.length) - .withFailMessage("the count is equal to 25, if not then schemaDocument needs updating") - .isEqualTo(25); + int expectedCount = 25; + assertThat(schemas.length).withFailMessage(String.format( + "the count should be equal to %d, not the current %d, if not then schemaDocument needs updating", + expectedCount, schemas.length)) + .isEqualTo(expectedCount); } /** diff --git a/config/src/test/resources/CustomJdbcUserServiceSampleConfig.sql b/config/src/test/resources/CustomJdbcUserServiceSampleConfig.sql index 7ca4c6e6975..a3ab2ec0fca 100644 --- a/config/src/test/resources/CustomJdbcUserServiceSampleConfig.sql +++ b/config/src/test/resources/CustomJdbcUserServiceSampleConfig.sql @@ -1,4 +1,4 @@ -create table users(principal varchar_ignorecase(50) not null primary key,credentials varchar_ignorecase(50) not null); +create table users(principal varchar_ignorecase(50) not null primary key,credentials varchar_ignorecase(100) not null); create table roles (principal varchar_ignorecase(50) not null,role varchar_ignorecase(50) not null,constraint fk_roles_users foreign key(principal) references users(principal)); create unique index ix_auth_principal on roles (principal,role); create table groups (id bigint generated by default as identity(start with 0) primary key,group_name varchar_ignorecase(50) not null); @@ -10,4 +10,4 @@ insert into roles values('user','USER'); insert into groups values(1,'OPERATIONS'); insert into group_authorities values(1,'DBA'); -insert into group_members values(1,'user',1); \ No newline at end of file +insert into group_members values(1,'user',1); diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java index d6a5b7c8878..1fcf2a60e8b 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -62,7 +62,11 @@ * * @author Luke Taylor * @since 2.0 + * @deprecated Use {@link JdbcUserPasswordDetailsManager} instead, as this ensure password + * encoding is kept updated. Please note that this migration might require changes to your + * code and database. */ +@Deprecated(since = "For removal in 7.0.") public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager { public static final String DEF_CREATE_USER_SQL = "insert into users (username, password, enabled) values (?,?,?)"; diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java new file mode 100644 index 00000000000..3f1e4c0432d --- /dev/null +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -0,0 +1,577 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.provisioning; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.Collection; +import java.util.List; + +import javax.sql.DataSource; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.context.ApplicationContextException; +import org.springframework.core.log.LogMessage; +import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.jdbc.core.PreparedStatementSetter; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserCache; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsPasswordService; +import org.springframework.security.core.userdetails.cache.NullUserCache; +import org.springframework.security.core.userdetails.jdbc.JdbcDaoImpl; +import org.springframework.util.Assert; + +/** + * Jdbc user management manager, based on the same table structure as the base class, + * JdbcDaoImpl. + *

+ * This manager will automatically keep the password of the user encoded with the current + * password encoding, making it easier to manage password security over time. + *

+ * Provides CRUD operations for both users and groups. Note that if the + * {@link #setEnableAuthorities(boolean) enableAuthorities} property is set to false, + * calls to createUser, updateUser and deleteUser will not store the authorities from the + * UserDetails or delete authorities for the user. Since this class cannot + * differentiate between authorities which were loaded for an individual or for a group of + * which the individual is a member, it's important that you take this into account when + * using this implementation for managing your users. + * + * This class is an evolution of the previous JdbcUserDetailsManager, which was part of + * spring security since version 2.0 + * + * @author Luke Taylor + * @author Geir Hedemark + * @since 6.3 + */ +public final class JdbcUserPasswordDetailsManager extends JdbcDaoImpl + implements UserDetailsManager, GroupManager, UserDetailsPasswordService { + + public static final String DEF_CREATE_USER_QUERY = "insert into users (username, password, enabled) values (?,?,?)"; + + public static final String DEF_DELETE_USER_QUERY = "delete from users where username = ?"; + + public static final String DEF_UPDATE_USER_QUERY = "update users set password = ?, enabled = ? where username = ?"; + + public static final String DEF_INSERT_AUTHORITY_QUERY = "insert into authorities (username, authority) values (?,?)"; + + public static final String DEF_DELETE_USER_AUTHORITIES_QUERY = "delete from authorities where username = ?"; + + public static final String DEF_USER_EXISTS_QUERY = "select username from users where username = ?"; + + public static final String DEF_CHANGE_PASSWORD_QUERY = "update users set password = ? where username = ?"; + + public static final String DEF_FIND_GROUPS_QUERY = "select group_name from groups"; + + public static final String DEF_FIND_USERS_IN_GROUP_QUERY = "select username from group_members gm, groups g " + + "where gm.group_id = g.id and g.group_name = ?"; + + public static final String DEF_INSERT_GROUP_QUERY = "insert into groups (group_name) values (?)"; + + public static final String DEF_FIND_GROUP_ID_QUERY = "select id from groups where group_name = ?"; + + public static final String DEF_INSERT_GROUP_AUTHORITY_QUERY = "insert into group_authorities (group_id, authority) values (?,?)"; + + public static final String DEF_DELETE_GROUP_QUERY = "delete from groups where id = ?"; + + public static final String DEF_DELETE_GROUP_AUTHORITIES_QUERY = "delete from group_authorities where group_id = ?"; + + public static final String DEF_DELETE_GROUP_MEMBERS_QUERY = "delete from group_members where group_id = ?"; + + public static final String DEF_RENAME_GROUP_QUERY = "update groups set group_name = ? where group_name = ?"; + + public static final String DEF_INSERT_GROUP_MEMBER_QUERY = "insert into group_members (group_id, username) values (?,?)"; + + public static final String DEF_DELETE_GROUP_MEMBER_QUERY = "delete from group_members where group_id = ? and username = ?"; + + public static final String DEF_GROUP_AUTHORITIES_QUERY_QUERY = "select g.id, g.group_name, ga.authority " + + "from groups g, group_authorities ga " + "where g.group_name = ? " + "and g.id = ga.group_id "; + + public static final String DEF_DELETE_GROUP_AUTHORITY_QUERY = "delete from group_authorities where group_id = ? and authority = ?"; + + protected final Log logger = LogFactory.getLog(getClass()); + + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + + private String createUserQuery = DEF_CREATE_USER_QUERY; + + private String deleteUserQuery = DEF_DELETE_USER_QUERY; + + private String updateUserQuery = DEF_UPDATE_USER_QUERY; + + private String createAuthorityQuery = DEF_INSERT_AUTHORITY_QUERY; + + private String deleteUserAuthoritiesQuery = DEF_DELETE_USER_AUTHORITIES_QUERY; + + private String userExistsQuery = DEF_USER_EXISTS_QUERY; + + private String changePasswordQuery = DEF_CHANGE_PASSWORD_QUERY; + + private String findAllGroupsQuery = DEF_FIND_GROUPS_QUERY; + + private String findUsersInGroupQuery = DEF_FIND_USERS_IN_GROUP_QUERY; + + private String insertGroupQuery = DEF_INSERT_GROUP_QUERY; + + private String findGroupIdQuery = DEF_FIND_GROUP_ID_QUERY; + + private String insertGroupAuthorityQuery = DEF_INSERT_GROUP_AUTHORITY_QUERY; + + private String deleteGroupQuery = DEF_DELETE_GROUP_QUERY; + + private String deleteGroupAuthoritiesQuery = DEF_DELETE_GROUP_AUTHORITIES_QUERY; + + private String deleteGroupMembersQuery = DEF_DELETE_GROUP_MEMBERS_QUERY; + + private String renameGroupQuery = DEF_RENAME_GROUP_QUERY; + + private String insertGroupMemberQuery = DEF_INSERT_GROUP_MEMBER_QUERY; + + private String deleteGroupMemberQuery = DEF_DELETE_GROUP_MEMBER_QUERY; + + private String groupAuthoritiesQuery = DEF_GROUP_AUTHORITIES_QUERY_QUERY; + + private String deleteGroupAuthorityQuery = DEF_DELETE_GROUP_AUTHORITY_QUERY; + + private AuthenticationManager authenticationManager; + + private UserCache userCache = new NullUserCache(); + + public JdbcUserPasswordDetailsManager() { + } + + public JdbcUserPasswordDetailsManager(DataSource dataSource) { + setDataSource(dataSource); + } + + protected void initDao() throws ApplicationContextException { + if (this.authenticationManager == null) { + this.logger.info( + "No authentication manager set. Reauthentication of users when changing passwords will not be performed."); + } + super.initDao(); + } + + /** + * Executes the SQL usersByUsernameQuery and returns a list of UserDetails + * objects. There should normally only be one matching user. + */ + @Override + protected List loadUsersByUsername(String username) { + return getJdbcTemplate().query(getUsersByUsernameQuery(), this::mapToUser, username); + } + + private UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { + String userName = rs.getString(1); + String password = rs.getString(2); + boolean enabled = rs.getBoolean(3); + boolean accLocked = false; + boolean accExpired = false; + boolean credsExpired = false; + if (rs.getMetaData().getColumnCount() > 3) { + // NOTE: acc_locked, acc_expired and creds_expired are also to be loaded + accLocked = rs.getBoolean(4); + accExpired = rs.getBoolean(5); + credsExpired = rs.getBoolean(6); + } + return new User(userName, password, enabled, !accExpired, !credsExpired, !accLocked, + AuthorityUtils.NO_AUTHORITIES); + } + + public void createUser(final UserDetails user) { + validateUserDetails(user); + getJdbcTemplate().update(this.createUserQuery, (ps) -> { + ps.setString(1, user.getUsername()); + ps.setString(2, user.getPassword()); + ps.setBoolean(3, user.isEnabled()); + int paramCount = ps.getParameterMetaData().getParameterCount(); + if (paramCount > 3) { + // NOTE: acc_locked, acc_expired and creds_expired are also to be inserted + ps.setBoolean(4, !user.isAccountNonLocked()); + ps.setBoolean(5, !user.isAccountNonExpired()); + ps.setBoolean(6, !user.isCredentialsNonExpired()); + } + }); + if (getEnableAuthorities()) { + insertUserAuthorities(user); + } + } + + public void updateUser(final UserDetails user) { + validateUserDetails(user); + getJdbcTemplate().update(this.updateUserQuery, (ps) -> { + ps.setString(1, user.getPassword()); + ps.setBoolean(2, user.isEnabled()); + int paramCount = ps.getParameterMetaData().getParameterCount(); + if (paramCount == 3) { + ps.setString(3, user.getUsername()); + } + else { + // NOTE: acc_locked, acc_expired and creds_expired are also updated + ps.setBoolean(3, !user.isAccountNonLocked()); + ps.setBoolean(4, !user.isAccountNonExpired()); + ps.setBoolean(5, !user.isCredentialsNonExpired()); + ps.setString(6, user.getUsername()); + } + }); + if (getEnableAuthorities()) { + deleteUserAuthorities(user.getUsername()); + insertUserAuthorities(user); + } + this.userCache.removeUserFromCache(user.getUsername()); + } + + private void insertUserAuthorities(UserDetails user) { + for (GrantedAuthority auth : user.getAuthorities()) { + getJdbcTemplate().update(this.createAuthorityQuery, user.getUsername(), auth.getAuthority()); + } + } + + public void deleteUser(String username) { + if (getEnableAuthorities()) { + deleteUserAuthorities(username); + } + getJdbcTemplate().update(this.deleteUserQuery, username); + this.userCache.removeUserFromCache(username); + } + + private void deleteUserAuthorities(String username) { + getJdbcTemplate().update(this.deleteUserAuthoritiesQuery, username); + } + + public void changePassword(String oldPassword, String newPassword) throws AuthenticationException { + Authentication currentUser = this.securityContextHolderStrategy.getContext().getAuthentication(); + if (currentUser == null) { + // This would indicate bad coding somewhere + throw new AccessDeniedException( + "Can't change password as no Authentication object found in context " + "for current user."); + } + String username = currentUser.getName(); + // If an authentication manager has been set, re-authenticate the user with the + // supplied password. + if (this.authenticationManager != null) { + this.logger.debug(LogMessage.format("Reauthenticating user '%s' for password change request.", username)); + this.authenticationManager + .authenticate(UsernamePasswordAuthenticationToken.unauthenticated(username, oldPassword)); + } + else { + this.logger.debug("No authentication manager set. Password won't be re-checked."); + } + this.logger.debug("Changing password for user '" + username + "'"); + getJdbcTemplate().update(this.changePasswordQuery, newPassword, username); + Authentication authentication = createNewAuthentication(currentUser, newPassword); + SecurityContext context = this.securityContextHolderStrategy.createEmptyContext(); + context.setAuthentication(authentication); + this.securityContextHolderStrategy.setContext(context); + this.userCache.removeUserFromCache(username); + } + + protected Authentication createNewAuthentication(Authentication currentAuth, String newPassword) { + UserDetails user = loadUserByUsername(currentAuth.getName()); + UsernamePasswordAuthenticationToken newAuthentication = UsernamePasswordAuthenticationToken.authenticated(user, + null, user.getAuthorities()); + newAuthentication.setDetails(currentAuth.getDetails()); + return newAuthentication; + } + + @Override + public boolean userExists(String username) { + List users = getJdbcTemplate().queryForList(this.userExistsQuery, new String[] { username }, + String.class); + if (users.size() > 1) { + throw new IncorrectResultSizeDataAccessException("More than one user found with name '" + username + "'", + 1); + } + return users.size() == 1; + } + + @Override + public List findAllGroups() { + return getJdbcTemplate().queryForList(this.findAllGroupsQuery, String.class); + } + + @Override + public List findUsersInGroup(String groupName) { + Assert.hasText(groupName, "groupName should have text"); + return getJdbcTemplate().queryForList(this.findUsersInGroupQuery, new String[] { groupName }, String.class); + } + + @Override + public void createGroup(final String groupName, final List authorities) { + Assert.hasText(groupName, "groupName should have text"); + Assert.notNull(authorities, "authorities cannot be null"); + this.logger.debug("Creating new group '" + groupName + "' with authorities " + + AuthorityUtils.authorityListToSet(authorities)); + getJdbcTemplate().update(this.insertGroupQuery, groupName); + int groupId = findGroupId(groupName); + for (GrantedAuthority a : authorities) { + String authority = a.getAuthority(); + getJdbcTemplate().update(this.insertGroupAuthorityQuery, (ps) -> { + ps.setInt(1, groupId); + ps.setString(2, authority); + }); + } + } + + @Override + public void deleteGroup(String groupName) { + this.logger.debug("Deleting group '" + groupName + "'"); + Assert.hasText(groupName, "groupName should have text"); + int id = findGroupId(groupName); + PreparedStatementSetter groupIdPSS = (ps) -> ps.setInt(1, id); + getJdbcTemplate().update(this.deleteGroupMembersQuery, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupAuthoritiesQuery, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupQuery, groupIdPSS); + } + + @Override + public void renameGroup(String oldName, String newName) { + this.logger.debug("Changing group name from '" + oldName + "' to '" + newName + "'"); + Assert.hasText(oldName, "oldName should have text"); + Assert.hasText(newName, "newName should have text"); + getJdbcTemplate().update(this.renameGroupQuery, newName, oldName); + } + + @Override + public void addUserToGroup(final String username, final String groupName) { + this.logger.debug("Adding user '" + username + "' to group '" + groupName + "'"); + Assert.hasText(username, "username should have text"); + Assert.hasText(groupName, "groupName should have text"); + int id = findGroupId(groupName); + getJdbcTemplate().update(this.insertGroupMemberQuery, (ps) -> { + ps.setInt(1, id); + ps.setString(2, username); + }); + this.userCache.removeUserFromCache(username); + } + + @Override + public void removeUserFromGroup(final String username, final String groupName) { + this.logger.debug("Removing user '" + username + "' to group '" + groupName + "'"); + Assert.hasText(username, "username should have text"); + Assert.hasText(groupName, "groupName should have text"); + int id = findGroupId(groupName); + getJdbcTemplate().update(this.deleteGroupMemberQuery, (ps) -> { + ps.setInt(1, id); + ps.setString(2, username); + }); + this.userCache.removeUserFromCache(username); + } + + @Override + public List findGroupAuthorities(String groupName) { + this.logger.debug("Loading authorities for group '" + groupName + "'"); + Assert.hasText(groupName, "groupName should have text"); + return getJdbcTemplate().query(this.groupAuthoritiesQuery, new String[] { groupName }, + this::mapToGrantedAuthority); + } + + private GrantedAuthority mapToGrantedAuthority(ResultSet rs, int rowNum) throws SQLException { + String roleName = getRolePrefix() + rs.getString(3); + return new SimpleGrantedAuthority(roleName); + } + + @Override + public void removeGroupAuthority(String groupName, final GrantedAuthority authority) { + this.logger.debug("Removing authority '" + authority + "' from group '" + groupName + "'"); + Assert.hasText(groupName, "groupName should have text"); + Assert.notNull(authority, "authority cannot be null"); + int id = findGroupId(groupName); + getJdbcTemplate().update(this.deleteGroupAuthorityQuery, (ps) -> { + ps.setInt(1, id); + ps.setString(2, authority.getAuthority()); + }); + } + + @Override + public void addGroupAuthority(final String groupName, final GrantedAuthority authority) { + this.logger.debug("Adding authority '" + authority + "' to group '" + groupName + "'"); + Assert.hasText(groupName, "groupName should have text"); + Assert.notNull(authority, "authority cannot be null"); + int id = findGroupId(groupName); + getJdbcTemplate().update(this.insertGroupAuthorityQuery, (ps) -> { + ps.setInt(1, id); + ps.setString(2, authority.getAuthority()); + }); + } + + private int findGroupId(String group) { + return getJdbcTemplate().queryForObject(this.findGroupIdQuery, Integer.class, group); + } + + /** + * Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use + * the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}. + * + * @since 5.8 + */ + public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null"); + this.securityContextHolderStrategy = securityContextHolderStrategy; + } + + public void setAuthenticationManager(AuthenticationManager authenticationManager) { + this.authenticationManager = authenticationManager; + } + + public void setCreateUserQuery(String createUserQuery) { + Assert.hasText(createUserQuery, "createUserQuery should have text"); + this.createUserQuery = createUserQuery; + } + + public void setDeleteUserQuery(String deleteUserQuery) { + Assert.hasText(deleteUserQuery, "deleteUserQuery should have text"); + this.deleteUserQuery = deleteUserQuery; + } + + public void setUpdateUserQuery(String updateUserQuery) { + Assert.hasText(updateUserQuery, "updateUserQuery should have text"); + this.updateUserQuery = updateUserQuery; + } + + public void setCreateAuthorityQuery(String createAuthorityQuery) { + Assert.hasText(createAuthorityQuery, "createAuthorityQuery should have text"); + this.createAuthorityQuery = createAuthorityQuery; + } + + public void setDeleteUserAuthoritiesQuery(String deleteUserAuthoritiesQuery) { + Assert.hasText(deleteUserAuthoritiesQuery, "deleteUserAuthoritiesQuery should have text"); + this.deleteUserAuthoritiesQuery = deleteUserAuthoritiesQuery; + } + + public void setUserExistsQuery(String userExistsQuery) { + Assert.hasText(userExistsQuery, "userExistsQuery should have text"); + this.userExistsQuery = userExistsQuery; + } + + public void setChangePasswordQuery(String changePasswordQuery) { + Assert.hasText(changePasswordQuery, "changePasswordQuery should have text"); + this.changePasswordQuery = changePasswordQuery; + } + + public void setFindAllGroupsQuery(String findAllGroupsQuery) { + Assert.hasText(findAllGroupsQuery, "findAllGroupsQuery should have text"); + this.findAllGroupsQuery = findAllGroupsQuery; + } + + public void setFindUsersInGroupQuery(String findUsersInGroupQuery) { + Assert.hasText(findUsersInGroupQuery, "findUsersInGroupQuery should have text"); + this.findUsersInGroupQuery = findUsersInGroupQuery; + } + + public void setInsertGroupQuery(String insertGroupQuery) { + Assert.hasText(insertGroupQuery, "insertGroupQuery should have text"); + this.insertGroupQuery = insertGroupQuery; + } + + public void setFindGroupIdQuery(String findGroupIdQuery) { + Assert.hasText(findGroupIdQuery, "findGroupIdQuery should have text"); + this.findGroupIdQuery = findGroupIdQuery; + } + + public void setInsertGroupAuthorityQuery(String insertGroupAuthorityQuery) { + Assert.hasText(insertGroupAuthorityQuery, "insertGroupAuthorityQuery should have text"); + this.insertGroupAuthorityQuery = insertGroupAuthorityQuery; + } + + public void setDeleteGroupQuery(String deleteGroupQuery) { + Assert.hasText(deleteGroupQuery, "deleteGroupQuery should have text"); + this.deleteGroupQuery = deleteGroupQuery; + } + + public void setDeleteGroupAuthoritiesQuery(String deleteGroupAuthoritiesQuery) { + Assert.hasText(deleteGroupAuthoritiesQuery, "deleteGroupAuthoritiesQuery should have text"); + this.deleteGroupAuthoritiesQuery = deleteGroupAuthoritiesQuery; + } + + public void setDeleteGroupMembersQuery(String deleteGroupMembersQuery) { + Assert.hasText(deleteGroupMembersQuery, "deleteGroupMembersQuery should have text"); + this.deleteGroupMembersQuery = deleteGroupMembersQuery; + } + + public void setRenameGroupQuery(String renameGroupQuery) { + Assert.hasText(renameGroupQuery, "renameGroupQuery should have text"); + this.renameGroupQuery = renameGroupQuery; + } + + public void setInsertGroupMemberQuery(String insertGroupMemberQuery) { + Assert.hasText(insertGroupMemberQuery, "insertGroupMemberQuery should have text"); + this.insertGroupMemberQuery = insertGroupMemberQuery; + } + + public void setDeleteGroupMemberQuery(String deleteGroupMemberQuery) { + Assert.hasText(deleteGroupMemberQuery, "deleteGroupMemberQuery should have text"); + this.deleteGroupMemberQuery = deleteGroupMemberQuery; + } + + public void setGroupAuthoritiesQuery(String groupAuthoritiesQuery) { + Assert.hasText(groupAuthoritiesQuery, "groupAuthoritiesQuery should have text"); + this.groupAuthoritiesQuery = groupAuthoritiesQuery; + } + + public void setDeleteGroupAuthorityQuery(String deleteGroupAuthorityQuery) { + Assert.hasText(deleteGroupAuthorityQuery, "deleteGroupAuthorityQuery should have text"); + this.deleteGroupAuthorityQuery = deleteGroupAuthorityQuery; + } + + /** + * Optionally sets the UserCache if one is in use in the application. This allows the + * user to be removed from the cache after updates have taken place to avoid stale + * data. + * @param userCache the cache used by the AuthenticationManager. + */ + public void setUserCache(UserCache userCache) { + Assert.notNull(userCache, "userCache cannot be null"); + this.userCache = userCache; + } + + private void validateUserDetails(UserDetails user) { + Assert.hasText(user.getUsername(), "Username may not be empty or null"); + validateAuthorities(user.getAuthorities()); + } + + private void validateAuthorities(Collection authorities) { + Assert.notNull(authorities, "Authorities list must not be null"); + for (GrantedAuthority authority : authorities) { + Assert.notNull(authority, "Authorities list contains a null entry"); + Assert.hasText(authority.getAuthority(), "getAuthority() method must return a non-empty string"); + } + } + + @Override + public UserDetails updatePassword(UserDetails user, String newPassword) { + this.logger.debug("Updating password for user '" + user.getUsername() + "'"); + getJdbcTemplate().update(this.changePasswordQuery, newPassword, user.getUsername()); + this.userCache.removeUserFromCache(user.getUsername()); + return User.withUserDetails(user).password(newPassword).build(); + } + +} diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java index 67d0c83fa59..5abeb45f6c0 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java @@ -56,7 +56,12 @@ * Tests for {@link JdbcUserDetailsManager} * * @author Luke Taylor + * @deprecated JdbcUserDetailsManager has been superseded by + * JdbcUserPasswordDetailsManager. If fixing bugs in JdbcUserDetailsManager, please + * remember to transplant any new tests to JdbcUserPasswordDetailsManagerTests + * */ +@Deprecated(since = "For removal in 7.0.") public class JdbcUserDetailsManagerTests { private static final String SELECT_JOE_SQL = "select * from users where username = 'joe'"; diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java new file mode 100644 index 00000000000..5fd4a2ba096 --- /dev/null +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -0,0 +1,419 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.provisioning; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.PopulatedDatabase; +import org.springframework.security.TestDataSource; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserCache; +import org.springframework.security.core.userdetails.UserDetails; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.mock; +import static org.mockito.BDDMockito.verify; + +/** + * Tests for {@link JdbcUserPasswordDetailsManager} + * + * @author Geir Hedemark + */ +public class JdbcUserPasswordDetailsManagerTests { + + private static final String SELECT_JOE_SQL = "select * from users where username = 'joe'"; + + private static final String SELECT_JOE_AUTHORITIES_SQL = "select * from authorities where username = 'joe'"; + + private static final UserDetails joe = new User("joe", "password", true, true, true, true, + AuthorityUtils.createAuthorityList("A", "C", "B")); + + private static TestDataSource dataSource; + + private JdbcUserPasswordDetailsManager manager; + + private JdbcUserPasswordDetailsManagerTests.MockUserCache cache; + + private JdbcTemplate template; + + @BeforeAll + public static void createDataSource() { + dataSource = new TestDataSource("jdbcusermgrtest"); + } + + @AfterAll + public static void clearDataSource() throws Exception { + dataSource.destroy(); + dataSource = null; + } + + @BeforeEach + public void initializeManagerAndCreateTables() { + this.manager = new JdbcUserPasswordDetailsManager(); + this.cache = new JdbcUserPasswordDetailsManagerTests.MockUserCache(); + this.manager.setUserCache(this.cache); + this.manager.setDataSource(dataSource); + this.manager.setCreateUserQuery(JdbcUserPasswordDetailsManager.DEF_CREATE_USER_QUERY); + this.manager.setUpdateUserQuery(JdbcUserPasswordDetailsManager.DEF_UPDATE_USER_QUERY); + this.manager.setUserExistsQuery(JdbcUserPasswordDetailsManager.DEF_USER_EXISTS_QUERY); + this.manager.setCreateAuthorityQuery(JdbcUserPasswordDetailsManager.DEF_INSERT_AUTHORITY_QUERY); + this.manager.setDeleteUserAuthoritiesQuery(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_AUTHORITIES_QUERY); + this.manager.setDeleteUserQuery(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_QUERY); + this.manager.setChangePasswordQuery(JdbcUserPasswordDetailsManager.DEF_CHANGE_PASSWORD_QUERY); + this.manager.initDao(); + this.template = this.manager.getJdbcTemplate(); + this.template.execute("create table users(username varchar(20) not null primary key," + + "password varchar(20) not null, enabled boolean not null)"); + this.template + .execute("create table authorities (username varchar(20) not null, authority varchar(20) not null, " + + "constraint fk_authorities_users foreign key(username) references users(username))"); + PopulatedDatabase.createGroupTables(this.template); + PopulatedDatabase.insertGroupData(this.template); + } + + @AfterEach + public void dropTablesAndClearContext() { + this.template.execute("drop table authorities"); + this.template.execute("drop table users"); + this.template.execute("drop table group_authorities"); + this.template.execute("drop table group_members"); + this.template.execute("drop table groups"); + SecurityContextHolder.clearContext(); + } + + private void setUpAccLockingColumns() { + this.template.execute("alter table users add column acc_locked boolean default false not null"); + this.template.execute("alter table users add column acc_expired boolean default false not null"); + this.template.execute("alter table users add column creds_expired boolean default false not null"); + this.manager.setUsersByUsernameQuery( + "select username,password,enabled, acc_locked, acc_expired, creds_expired from users where username = ?"); + this.manager.setCreateUserQuery( + "insert into users (username, password, enabled, acc_locked, acc_expired, creds_expired) values (?,?,?,?,?,?)"); + this.manager.setUpdateUserQuery( + "update users set password = ?, enabled = ?, acc_locked=?, acc_expired=?, creds_expired=? where username = ?"); + } + + @Test + public void createUserInsertsCorrectData() { + this.manager.createUser(joe); + UserDetails joe2 = this.manager.loadUserByUsername("joe"); + assertThat(joe2).isEqualTo(joe); + } + + @Test + public void createUserInsertsCorrectDataWithLocking() { + setUpAccLockingColumns(); + UserDetails user = new User("joe", "pass", true, false, true, false, + AuthorityUtils.createAuthorityList("A", "B")); + this.manager.createUser(user); + UserDetails user2 = this.manager.loadUserByUsername(user.getUsername()); + assertThat(user2).usingRecursiveComparison().isEqualTo(user); + } + + @Test + public void deleteUserRemovesUserDataAndAuthoritiesAndClearsCache() { + insertJoe(); + this.manager.deleteUser("joe"); + assertThat(this.template.queryForList(SELECT_JOE_SQL)).isEmpty(); + assertThat(this.template.queryForList(SELECT_JOE_AUTHORITIES_SQL)).isEmpty(); + assertThat(this.cache.getUserMap().containsKey("joe")).isFalse(); + } + + @Test + public void updateUserChangesDataCorrectlyAndClearsCache() { + insertJoe(); + User newJoe = new User("joe", "newpassword", false, true, true, true, + AuthorityUtils.createAuthorityList(new String[] { "D", "F", "E" })); + this.manager.updateUser(newJoe); + UserDetails joe = this.manager.loadUserByUsername("joe"); + assertThat(joe).isEqualTo(newJoe); + assertThat(this.cache.getUserMap().containsKey("joe")).isFalse(); + } + + @Test + public void updateUserChangesDataCorrectlyAndClearsCacheWithLocking() { + setUpAccLockingColumns(); + insertJoe(); + User newJoe = new User("joe", "newpassword", false, false, false, true, + AuthorityUtils.createAuthorityList("D", "F", "E")); + this.manager.updateUser(newJoe); + UserDetails joe = this.manager.loadUserByUsername(newJoe.getUsername()); + assertThat(joe).usingRecursiveComparison().isEqualTo(newJoe); + assertThat(this.cache.getUserMap().containsKey(newJoe.getUsername())).isFalse(); + } + + @Test + public void userExistsReturnsFalseForNonExistentUsername() { + assertThat(this.manager.userExists("joe")).isFalse(); + } + + @Test + public void userExistsReturnsTrueForExistingUsername() { + insertJoe(); + assertThat(this.manager.userExists("joe")).isTrue(); + assertThat(this.cache.getUserMap()).containsKey("joe"); + } + + @Test + public void changePasswordFailsForUnauthenticatedUser() { + assertThatExceptionOfType(AccessDeniedException.class) + .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); + } + + @Test + public void changePasswordSucceedsWithAuthenticatedUserAndNoAuthenticationManagerSet() { + insertJoe(); + authenticateJoe(); + this.manager.changePassword("wrongpassword", "newPassword"); + UserDetails newJoe = this.manager.loadUserByUsername("joe"); + assertThat(newJoe.getPassword()).isEqualTo("newPassword"); + assertThat(this.cache.getUserMap().containsKey("joe")).isFalse(); + } + + @Test + public void changePasswordWhenCustomSecurityContextHolderStrategyThenUses() { + insertJoe(); + Authentication authentication = authenticateJoe(); + SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class); + given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication)); + given(strategy.createEmptyContext()).willReturn(new SecurityContextImpl()); + this.manager.setSecurityContextHolderStrategy(strategy); + this.manager.changePassword("wrongpassword", "newPassword"); + verify(strategy).getContext(); + } + + @Test + public void changePasswordSucceedsWithIfReAuthenticationSucceeds() { + insertJoe(); + Authentication currentAuth = authenticateJoe(); + AuthenticationManager am = mock(AuthenticationManager.class); + given(am.authenticate(currentAuth)).willReturn(currentAuth); + this.manager.setAuthenticationManager(am); + this.manager.changePassword("password", "newPassword"); + UserDetails newJoe = this.manager.loadUserByUsername("joe"); + assertThat(newJoe.getPassword()).isEqualTo("newPassword"); + // The password in the context should also be altered + Authentication newAuth = SecurityContextHolder.getContext().getAuthentication(); + assertThat(newAuth.getName()).isEqualTo("joe"); + assertThat(newAuth.getDetails()).isEqualTo(currentAuth.getDetails()); + assertThat(newAuth.getCredentials()).isNull(); + assertThat(this.cache.getUserMap().containsKey("joe")).isFalse(); + } + + @Test + public void changePasswordFailsIfReAuthenticationFails() { + insertJoe(); + authenticateJoe(); + AuthenticationManager am = mock(AuthenticationManager.class); + given(am.authenticate(any(Authentication.class))).willThrow(new BadCredentialsException("")); + this.manager.setAuthenticationManager(am); + assertThatExceptionOfType(BadCredentialsException.class) + .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); + // Check password hasn't changed. + UserDetails newJoe = this.manager.loadUserByUsername("joe"); + assertThat(newJoe.getPassword()).isEqualTo("password"); + assertThat(SecurityContextHolder.getContext().getAuthentication().getCredentials()).isEqualTo("password"); + assertThat(this.cache.getUserMap()).containsKey("joe"); + } + + @Test + public void findAllGroupsReturnsExpectedGroupNames() { + List groups = this.manager.findAllGroups(); + assertThat(groups).hasSize(4); + Collections.sort(groups); + assertThat(groups.get(0)).isEqualTo("GROUP_0"); + assertThat(groups.get(1)).isEqualTo("GROUP_1"); + assertThat(groups.get(2)).isEqualTo("GROUP_2"); + assertThat(groups.get(3)).isEqualTo("GROUP_3"); + } + + @Test + public void findGroupMembersReturnsCorrectData() { + List groupMembers = this.manager.findUsersInGroup("GROUP_0"); + assertThat(groupMembers).hasSize(1); + assertThat(groupMembers.get(0)).isEqualTo("jerry"); + groupMembers = this.manager.findUsersInGroup("GROUP_1"); + assertThat(groupMembers).hasSize(2); + } + + @Test + @SuppressWarnings("unchecked") + public void createGroupInsertsCorrectData() { + this.manager.createGroup("TEST_GROUP", AuthorityUtils.createAuthorityList("ROLE_X", "ROLE_Y")); + List roles = this.template.queryForList("select ga.authority from groups g, group_authorities ga " + + "where ga.group_id = g.id " + "and g.group_name = 'TEST_GROUP'"); + assertThat(roles).hasSize(2); + } + + @Test + public void deleteGroupRemovesData() { + this.manager.deleteGroup("GROUP_0"); + this.manager.deleteGroup("GROUP_1"); + this.manager.deleteGroup("GROUP_2"); + this.manager.deleteGroup("GROUP_3"); + assertThat(this.template.queryForList("select * from group_authorities")).isEmpty(); + assertThat(this.template.queryForList("select * from group_members")).isEmpty(); + assertThat(this.template.queryForList("select id from groups")).isEmpty(); + } + + @Test + public void renameGroupIsSuccessful() { + this.manager.renameGroup("GROUP_0", "GROUP_X"); + assertThat(this.template.queryForObject("select id from groups where group_name = 'GROUP_X'", Integer.class)) + .isZero(); + } + + @Test + public void addingGroupUserSetsCorrectData() { + this.manager.addUserToGroup("tom", "GROUP_0"); + assertThat(this.template.queryForList("select username from group_members where group_id = 0")).hasSize(2); + } + + @Test + public void removeUserFromGroupDeletesGroupMemberRow() { + this.manager.removeUserFromGroup("jerry", "GROUP_1"); + assertThat(this.template.queryForList("select group_id from group_members where username = 'jerry'")) + .hasSize(1); + } + + @Test + public void findGroupAuthoritiesReturnsCorrectAuthorities() { + assertThat(AuthorityUtils.createAuthorityList("ROLE_A")) + .isEqualTo(this.manager.findGroupAuthorities("GROUP_0")); + } + + @Test + public void addGroupAuthorityInsertsCorrectGroupAuthorityRow() { + GrantedAuthority auth = new SimpleGrantedAuthority("ROLE_X"); + this.manager.addGroupAuthority("GROUP_0", auth); + this.template.queryForObject( + "select authority from group_authorities where authority = 'ROLE_X' and group_id = 0", String.class); + } + + @Test + public void deleteGroupAuthorityRemovesCorrectRows() { + GrantedAuthority auth = new SimpleGrantedAuthority("ROLE_A"); + this.manager.removeGroupAuthority("GROUP_0", auth); + assertThat(this.template.queryForList("select authority from group_authorities where group_id = 0")).isEmpty(); + this.manager.removeGroupAuthority("GROUP_2", auth); + assertThat(this.template.queryForList("select authority from group_authorities where group_id = 2")).hasSize(2); + } + + // SEC-1156 + @Test + public void createUserDoesNotSaveAuthoritiesIfEnableAuthoritiesIsFalse() { + this.manager.setEnableAuthorities(false); + this.manager.createUser(joe); + assertThat(this.template.queryForList(SELECT_JOE_AUTHORITIES_SQL)).isEmpty(); + } + + // SEC-1156 + @Test + public void updateUserDoesNotSaveAuthoritiesIfEnableAuthoritiesIsFalse() { + this.manager.setEnableAuthorities(false); + insertJoe(); + this.template.execute("delete from authorities where username='joe'"); + this.manager.updateUser(joe); + assertThat(this.template.queryForList(SELECT_JOE_AUTHORITIES_SQL)).isEmpty(); + } + + // SEC-2166 + @Test + public void createNewAuthenticationUsesNullPasswordToKeepPassordsSave() { + insertJoe(); + UsernamePasswordAuthenticationToken currentAuth = UsernamePasswordAuthenticationToken.authenticated("joe", null, + AuthorityUtils.createAuthorityList("ROLE_USER")); + Authentication updatedAuth = this.manager.createNewAuthentication(currentAuth, "new"); + assertThat(updatedAuth.getCredentials()).isNull(); + } + + @Test + public void updatePasswordSucceeds() { + insertJoe(); + UserDetails joe = this.manager.loadUserByUsername("joe"); + UserDetails returnedJoe = ((JdbcUserPasswordDetailsManager) this.manager).updatePassword(joe, "newPassword"); + assertThat(returnedJoe.getPassword()).isEqualTo("newPassword"); + UserDetails newJoe = this.manager.loadUserByUsername("joe"); + assertThat(newJoe.getPassword()).isEqualTo("newPassword"); + assertThat(this.cache.getUserMap().containsKey("joe")).isFalse(); + } + + private Authentication authenticateJoe() { + UsernamePasswordAuthenticationToken auth = UsernamePasswordAuthenticationToken.authenticated("joe", "password", + joe.getAuthorities()); + SecurityContextHolder.getContext().setAuthentication(auth); + return auth; + } + + private void insertJoe() { + this.template.execute("insert into users (username, password, enabled) values ('joe','password','true')"); + this.template.execute("insert into authorities (username, authority) values ('joe','A')"); + this.template.execute("insert into authorities (username, authority) values ('joe','B')"); + this.template.execute("insert into authorities (username, authority) values ('joe','C')"); + this.cache.putUserInCache(joe); + } + + private class MockUserCache implements UserCache { + + private Map cache = new HashMap<>(); + + @Override + public UserDetails getUserFromCache(String username) { + return this.cache.get(username); + } + + @Override + public void putUserInCache(UserDetails user) { + this.cache.put(user.getUsername(), user); + } + + @Override + public void removeUserFromCache(String username) { + this.cache.remove(username); + } + + Map getUserMap() { + return this.cache; + } + + } + +} diff --git a/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc new file mode 100644 index 00000000000..5969c2d63b3 --- /dev/null +++ b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc @@ -0,0 +1,66 @@ += JDBC-based user storage + +The existing JdbcUserDetailsManager has been extended to update the encoding of passwords during login +if the existing password encoding can be improved. The new behaviour is located in +JdbcUserPasswordDetailsManager because existing users might get into problems when the password is +re-encoded if the password field is too short. + +There are no configuration changes necessary to the bean itself. + +== Excending the size of the password field + +If the example supplied with spring security is used it is suggested that the size of the password +field is doubled: + +.... +alter table users alter column credentials varchar_ignorecase(100); +.... + +If a database such as postgres is used, the following will accept any future encodings which require +even more space: + +.... +alter table users alter column credentials text; +.... + +== Configuring the new JdbcUserPasswordDetailsManager + +=== Existing DSL configuration + +Spring has historically had a DSL to configure spring security. To configure a `+JdbcUserDetailsManager+` +one invoked the function `+AuthenticationManagerBuilder.jdbcAuthentication+`. If using this kind of +configuration it is suggested to move to one of the following two configuration options instead. There +is no DSL-based feature that allows configuration of the new `+JdbcUserPasswordDetailsManager+`. + +=== Bean-based configuration + +The new user details implementation is mostly a drop-in replacement for the old implementation: + +.... +@Bean +UserDetailsManager users(DataSource dataSource) { + return new JdbcUserPasswordDetailsManager(dataSource); +} +.... + +All properties that were previously named `+*Sql+` has been renamed to be `+*Query+` to align with the +XSD namespace. + +There is an additional configuration parameter `+changePasswordQuery+` available for changing the password of a user. This query +is necessary for the new behaviour of the user details implementation. + +Please refer to +https://docs.spring.io/spring-security/reference/servlet/authentication/passwords/user-details-service.html[the spring security reference documentation] +for more details. + +=== XML-based configuration + +Where the XML configuration format previously referred to a `+jdbc-user-service+` tag, there is now a new tag that will trigger +creation of the new bean: + +.... + +.... + +The configuration settings are the same as for the old bean, but now also allows setting an sql query +for changing the password of the user through the `+change-password-query+` attribute. diff --git a/docs/modules/ROOT/pages/servlet/appendix/namespace/authentication-manager.adoc b/docs/modules/ROOT/pages/servlet/appendix/namespace/authentication-manager.adoc index 3719fe7394c..502276e25ba 100644 --- a/docs/modules/ROOT/pages/servlet/appendix/namespace/authentication-manager.adoc +++ b/docs/modules/ROOT/pages/servlet/appendix/namespace/authentication-manager.adoc @@ -94,13 +94,13 @@ A reference to a bean that implements UserDetailsService that may be created usi === Child Elements of +* <> * <> * xref:servlet/appendix/namespace/ldap.adoc#nsa-ldap-user-service[ldap-user-service] * <> * <> - [[nsa-jdbc-user-service]] == Causes creation of a JDBC-based UserDetailsService. @@ -178,6 +178,93 @@ select username, password, enabled from users where username = ? ---- +[[nsa-jdbc-user-password-service]] +== +Causes creation of a JDBC-based UserDetailsService. This service will automatically update the encoding of +a password that is stored with an outdated/insecure encoding. + + +[[nsa-jdbc-user-password-service-attributes]] +=== Attributes + + +[[nsa-jdbc-user-password-service-authorities-by-username-query]] +* **authorities-by-username-query** +An SQL statement to query for a user's granted authorities given a username. + +The default is + +[source] +---- +select username, authority from authorities where username = ? +---- + + + + +[[nsa-jdbc-user-password-service-cache-ref]] +* **cache-ref** +Defines a reference to a cache for use with a UserDetailsService. + + +[[nsa-jdbc-user-password-service-data-source-ref]] +* **data-source-ref** +The bean ID of the DataSource which provides the required tables. + + +[[nsa-jdbc-user-password-service-group-authorities-by-username-query]] +* **group-authorities-by-username-query** +An SQL statement to query user's group authorities given a username. +The default is + ++ + +[source] +---- +select +g.id, g.group_name, ga.authority +from +groups g, group_members gm, group_authorities ga +where +gm.username = ? and g.id = ga.group_id and g.id = gm.group_id +---- + +[[nsa-jdbc-user-password-service-change-password-query]] +* **change-password-query** +An SQL statement to update the users password. +The default is + ++ + +[source] +---- +update users set password = ? where username = ? +---- + +[[nsa-jdbc-user-password-service-id]] +* **id** +A bean identifier, used for referring to the bean elsewhere in the context. + + +[[nsa-jdbc-user-password-service-role-prefix]] +* **role-prefix** +A non-empty string prefix that will be added to role strings loaded from persistent storage (default is "ROLE_"). +Use the value "none" for no prefix in cases where the default is non-empty. + + +[[nsa-jdbc-user-password-service-users-by-username-query]] +* **users-by-username-query** +An SQL statement to query a username, password, and enabled status given a username. +The default is + ++ + +[source] +---- +select username, password, enabled from users where username = ? +---- + + [[nsa-password-encoder]] diff --git a/docs/modules/ROOT/pages/servlet/authentication/passwords/jdbc.adoc b/docs/modules/ROOT/pages/servlet/authentication/passwords/jdbc.adoc index 2a2d4dfa1ec..e0d08775b54 100644 --- a/docs/modules/ROOT/pages/servlet/authentication/passwords/jdbc.adoc +++ b/docs/modules/ROOT/pages/servlet/authentication/passwords/jdbc.adoc @@ -2,7 +2,7 @@ = JDBC Authentication Spring Security's `JdbcDaoImpl` implements xref:servlet/authentication/passwords/user-details-service.adoc#servlet-authentication-userdetailsservice[`UserDetailsService`] to provide support for username-and-password-based authentication that is retrieved by using JDBC. -`JdbcUserDetailsManager` extends `JdbcDaoImpl` to provide management of `UserDetails` through the `UserDetailsManager` interface. +`JdbcUserPasswordDetailsManager` extends `JdbcDaoImpl` to provide management of `UserDetails` through the `UserDetailsManager` interface. `UserDetails`-based authentication is used by Spring Security when it is configured to xref:servlet/authentication/passwords/index.adoc#servlet-authentication-unpwd-input[accept a username/password] for authentication. In the following sections, we discuss: @@ -95,7 +95,7 @@ create table group_members ( [[servlet-authentication-jdbc-datasource]] == Setting up a DataSource -Before we configure `JdbcUserDetailsManager`, we must create a `DataSource`. +Before we configure `JdbcUserPasswordDetailsManager`, we must create a `DataSource`. In our example, we set up an https://docs.spring.io/spring-framework/docs/current/spring-framework-reference/data-access.html#jdbc-embedded-database-support[embedded DataSource] that is initialized with the <>. .Embedded Data Source @@ -140,12 +140,12 @@ fun dataSource(): DataSource { In a production environment, you want to ensure that you set up a connection to an external database. [[servlet-authentication-jdbc-bean]] -== JdbcUserDetailsManager Bean +== JdbcUserPasswordDetailsManager Bean In this sample, we use xref:features/authentication/password-storage.adoc#authentication-password-storage-boot-cli[Spring Boot CLI] to encode a password value of `password` and get the encoded password of `+{bcrypt}$2a$10$GRLdNijSQMUvl/au9ofL.eDwmoohzzS7.rmNSJZ.0FxO/BTk76klW+`. See the xref:features/authentication/password-storage.adoc#authentication-password-storage[PasswordEncoder] section for more details about how to store passwords. -.JdbcUserDetailsManager +.JdbcUserPasswordDetailsManager [tabs] ====== @@ -165,7 +165,7 @@ UserDetailsManager users(DataSource dataSource) { .password("{bcrypt}$2a$10$GRLdNijSQMUvl/au9ofL.eDwmoohzzS7.rmNSJZ.0FxO/BTk76klW") .roles("USER", "ADMIN") .build(); - JdbcUserDetailsManager users = new JdbcUserDetailsManager(dataSource); + JdbcUserPasswordDetailsManager users = new JdbcUserPasswordDetailsManager(dataSource); users.createUser(user); users.createUser(admin); return users; @@ -176,14 +176,14 @@ XML:: + [source,xml,role="secondary",attrs="-attributes"] ---- - + - + ---- Kotlin:: @@ -202,7 +202,7 @@ fun users(dataSource: DataSource): UserDetailsManager { .password("{bcrypt}$2a$10\$GRLdNijSQMUvl/au9ofL.eDwmoohzzS7.rmNSJZ.0FxO/BTk76klW") .roles("USER", "ADMIN") .build(); - val users = JdbcUserDetailsManager(dataSource) + val users = JdbcUserPasswordDetailsManager(dataSource) users.createUser(user) users.createUser(admin) return users diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java index cf3e5fb206b..fe6996aedf1 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactory.java @@ -171,7 +171,7 @@ private NimbusJwtDecoder buildDecoder(ClientRegistration clientRegistration) { } return NimbusJwtDecoder.withJwkSetUri(jwkSetUri) .jwsAlgorithm((SignatureAlgorithm) jwsAlgorithm) - .restOperations(restOperationsFactory.apply(clientRegistration)) + .restOperations(this.restOperationsFactory.apply(clientRegistration)) .build(); } if (jwsAlgorithm != null && MacAlgorithm.class.isAssignableFrom(jwsAlgorithm.getClass())) { diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java index 0e2b2b2ddd9..1cc8e5a6e44 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactory.java @@ -168,7 +168,7 @@ private NimbusReactiveJwtDecoder buildDecoder(ClientRegistration clientRegistrat } return NimbusReactiveJwtDecoder.withJwkSetUri(jwkSetUri) .jwsAlgorithm((SignatureAlgorithm) jwsAlgorithm) - .webClient(webClientFactory.apply(clientRegistration)) + .webClient(this.webClientFactory.apply(clientRegistration)) .build(); } if (jwsAlgorithm != null && MacAlgorithm.class.isAssignableFrom(jwsAlgorithm.getClass())) { diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java index b417d17b676..52db517f5c9 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java @@ -100,7 +100,7 @@ public void setClaimTypeConverterFactoryWhenNullThenThrowIllegalArgumentExceptio @Test public void setRestOperationsFactoryWhenNullThenThrowIllegalArgumentException() { assertThatIllegalArgumentException() - .isThrownBy(() -> this.idTokenDecoderFactory.setRestOperationsFactory(null)); + .isThrownBy(() -> this.idTokenDecoderFactory.setRestOperationsFactory(null)); } @Test @@ -187,13 +187,12 @@ public void createDecoderWhenCustomClaimTypeConverterFactorySetThenApplied() { @Test public void createDecoderWhenCustomRestOperationsFactorySetThenApplied() { - Function customRestOperationsFactory = mock( - Function.class); + Function customRestOperationsFactory = mock(Function.class); this.idTokenDecoderFactory.setRestOperationsFactory(customRestOperationsFactory); ClientRegistration clientRegistration = this.registration.build(); - given(customRestOperationsFactory.apply(same(clientRegistration))) - .willReturn(new RestTemplate()); + given(customRestOperationsFactory.apply(same(clientRegistration))).willReturn(new RestTemplate()); this.idTokenDecoderFactory.createDecoder(clientRegistration); verify(customRestOperationsFactory).apply(same(clientRegistration)); } + } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java index 87f7dd67f23..8f304e098d2 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java @@ -97,8 +97,7 @@ public void setClaimTypeConverterFactoryWhenNullThenThrowIllegalArgumentExceptio @Test public void setWebClientFactoryWhenNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException() - .isThrownBy(() -> this.idTokenDecoderFactory.setWebClientFactory(null)); + assertThatIllegalArgumentException().isThrownBy(() -> this.idTokenDecoderFactory.setWebClientFactory(null)); } @Test @@ -185,13 +184,12 @@ public void createDecoderWhenCustomClaimTypeConverterFactorySetThenApplied() { @Test public void createDecoderWhenCustomWebClientFactorySetThenApplied() { - Function customWebClientFactory = mock( - Function.class); + Function customWebClientFactory = mock(Function.class); this.idTokenDecoderFactory.setWebClientFactory(customWebClientFactory); ClientRegistration clientRegistration = this.registration.build(); - given(customWebClientFactory.apply(same(clientRegistration))) - .willReturn(WebClient.create()); + given(customWebClientFactory.apply(same(clientRegistration))).willReturn(WebClient.create()); this.idTokenDecoderFactory.createDecoder(clientRegistration); verify(customWebClientFactory).apply(same(clientRegistration)); } + }