From 9da4028d931f47f564d1ac27e6f9aa1ce94fa78a Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Wed, 15 Nov 2023 11:21:11 +0100 Subject: [PATCH 01/13] Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries --- .../security/config/Elements.java | 5 +- .../config/SecurityNamespaceHandler.java | 8 +- ...nticationProviderBeanDefinitionParser.java | 5 +- ...ordDetailsManagerBeanDefinitionParser.java | 70 +++++++ .../JdbcUserServiceBeanDefinitionParser.java | 2 + .../security/config/spring-security-6.3.xsd | 197 +++++++++++++++--- ...tailsManagerBeanDefinitionParserTests.java | 167 +++++++++++++++ .../provisioning/JdbcUserDetailsManager.java | 10 +- .../JdbcUserPasswordDetailsManager.java | 50 +++++ .../JdbcUserDetailsManagerTests.java | 18 +- .../JdbcUserPasswordDetailsManagerTests.java | 44 ++++ 11 files changed, 531 insertions(+), 45 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java create mode 100644 config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java create mode 100644 core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java create mode 100644 core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java 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..58245e89b15 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 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..7f505a2be98 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. @@ -21,6 +21,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.security.config.authentication.*; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -30,10 +31,6 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.NamespaceHandler; 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.JdbcUserServiceBeanDefinitionParser; -import org.springframework.security.config.authentication.UserServiceBeanDefinitionParser; import org.springframework.security.config.http.FilterChainBeanDefinitionParser; import org.springframework.security.config.http.FilterChainMapBeanDefinitionDecorator; import org.springframework.security.config.http.FilterInvocationSecurityMetadataSourceParser; @@ -173,6 +170,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/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..92e419be15e --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -0,0 +1,70 @@ +/* + * 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.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.security.config.Elements; +import org.springframework.util.StringUtils; +import org.w3c.dom.Element; + +/** + * @author Luke Taylor + * @author Geir Hedemark + */ +public 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_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_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 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); + } + } + +} 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..59a62f34329 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,6 +25,8 @@ /** * @author Luke Taylor + * @deprecated Use {@link JdbcUserPasswordDetailsManagerBeanDefinitionParser} instead, as this will update password encodings + * to more safe variants as users log in */ public class JdbcUserServiceBeanDefinitionParser extends AbstractUserDetailsServiceBeanDefinitionParser { 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..f7686966133 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 @@ -124,7 +124,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -408,7 +412,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -488,7 +496,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -554,7 +566,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -892,13 +908,23 @@ - - - - - - - +<<<<<<< HEAD + + + + + + + +======= + + + + + + + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -1402,7 +1428,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -1427,7 +1457,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -1484,7 +1518,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -1531,7 +1569,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2064,7 +2106,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2121,7 +2167,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2574,7 +2624,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2607,7 +2661,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2670,7 +2728,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2717,7 +2779,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2815,7 +2881,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2848,8 +2918,13 @@ - - +<<<<<<< HEAD + + +======= + + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -2866,7 +2941,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -3009,7 +3088,11 @@ - +<<<<<<< HEAD + +======= + +>>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) @@ -3061,7 +3144,6 @@ - @@ -3159,6 +3241,65 @@ + + + Causes creation of a JDBC-based UserDetailsService that also keeps password encodings updated to the current standard. + + + + + + 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" + + + + + + 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 @@ -3809,4 +3950,4 @@ - \ No newline at end of file + 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..fdab9785763 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -0,0 +1,167 @@ +/* + * 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.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 org.w3c.dom.Element; + +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 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) { + this.appContext = new InMemoryXmlApplicationContext(context); + } + +} 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..e80f31fc05b 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.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. @@ -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 public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager { public static final String DEF_CREATE_USER_SQL = "insert into users (username, password, enabled) values (?,?,?)"; @@ -124,7 +128,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private String userExistsSql = DEF_USER_EXISTS_SQL; - private String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; + protected String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; private String findAllGroupsSql = DEF_FIND_GROUPS_SQL; @@ -154,7 +158,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private AuthenticationManager authenticationManager; - private UserCache userCache = new NullUserCache(); + protected UserCache userCache = new NullUserCache(); public JdbcUserDetailsManager() { } 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..610aca452fe --- /dev/null +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -0,0 +1,50 @@ +/* + * 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 org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.UserDetailsPasswordService; + +/** + * 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. + * + * @author Geir Hedemark + * @since TBD + */ +public class JdbcUserPasswordDetailsManager extends JdbcUserDetailsManager implements UserDetailsPasswordService { + @Override + public UserDetails updatePassword(UserDetails user, String newPassword) { + this.logger.debug("Updating password for user '" + user.getUsername() + "'"); + getJdbcTemplate().update(this.changePasswordSql, 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..a1cfdd656a5 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.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. @@ -68,9 +68,9 @@ public class JdbcUserDetailsManagerTests { private static TestDataSource dataSource; - private JdbcUserDetailsManager manager; + JdbcUserDetailsManager manager; - private MockUserCache cache; + MockUserCache cache; private JdbcTemplate template; @@ -85,9 +85,13 @@ public static void clearDataSource() throws Exception { dataSource = null; } + public JdbcUserDetailsManager makeInstance() { + return new JdbcUserDetailsManager(); + } + @BeforeEach public void initializeManagerAndCreateTables() { - this.manager = new JdbcUserDetailsManager(); + this.manager = makeInstance(); this.cache = new MockUserCache(); this.manager.setUserCache(this.cache); this.manager.setDataSource(dataSource); @@ -372,7 +376,7 @@ private Authentication authenticateJoe() { return auth; } - private void insertJoe() { + public 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')"); @@ -380,7 +384,7 @@ private void insertJoe() { this.cache.putUserInCache(joe); } - private class MockUserCache implements UserCache { + public class MockUserCache implements UserCache { private Map cache = new HashMap<>(); @@ -399,7 +403,7 @@ public void removeUserFromCache(String username) { this.cache.remove(username); } - Map getUserMap() { + public Map getUserMap() { return this.cache; } 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..93d6138502f --- /dev/null +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -0,0 +1,44 @@ +/* + * 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 org.junit.jupiter.api.Test; +import org.springframework.security.core.userdetails.UserDetails; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link JdbcUserPasswordDetailsManager} + * + * @author Geir Hedemark + */ +public class JdbcUserPasswordDetailsManagerTests extends JdbcUserDetailsManagerTests { + @Override + public JdbcUserDetailsManager makeInstance() { + return new JdbcUserPasswordDetailsManager(); + } + + @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(); + } +} From 46bcbdc28a4d16cd1b66aff2c83a85f62b08be87 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Wed, 15 Nov 2023 11:50:52 +0100 Subject: [PATCH 02/13] Added support for configuring the service using code, updated tests to allow overriding SQL query --- .../AuthenticationManagerBuilder.java | 3 + .../AuthenticationConfiguration.java | 13 +- ...cUserPasswordDetailsManagerConfigurer.java | 205 ++++++++++++++++++ .../HttpSecurityConfiguration.java | 11 + ...NamespaceJdbcUserPasswordServiceTests.java | 159 ++++++++++++++ .../CustomJdbcUserServiceSampleConfig.sql | 4 +- 6 files changed, 392 insertions(+), 3 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserPasswordDetailsManagerConfigurer.java create mode 100644 config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java 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..9b7a0c25f70 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 @@ -34,6 +34,7 @@ import org.springframework.security.config.annotation.authentication.configurers.ldap.LdapAuthenticationProviderConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; +import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.UserDetailsAwareConfigurer; import org.springframework.security.core.Authentication; @@ -155,6 +156,8 @@ public InMemoryUserDetailsManagerConfigurer inMemo * @return a {@link JdbcUserDetailsManagerConfigurer} to allow customization of the * JDBC authentication * @throws Exception if an error occurs when adding the JDBC authentication + * @deprecated Use jdbcAuthenticationWithPasswordManagement instead, as this keeps the password encoding up to + * date over time. Please consult the migrationn documentation as database changes might be necessary. */ 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..5e6146ea665 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. @@ -44,6 +44,7 @@ import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; +import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration; import org.springframework.security.core.Authentication; @@ -288,11 +289,21 @@ 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 public JdbcUserDetailsManagerConfigurer jdbcAuthentication() throws Exception { return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } + @Override + public JdbcUserPasswordDetailsManagerConfigurer jdbcAuthenticationWithPasswordManagement() throws Exception { + return super.jdbcAuthenticationWithPasswordManagement().passwordEncoder(this.defaultPasswordEncoder); + } + @Override public DaoAuthenticationConfigurer userDetailsService( T userDetailsService) throws Exception { 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..f98782b7314 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserPasswordDetailsManagerConfigurer.java @@ -0,0 +1,205 @@ +/* + * 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 TBD + */ +public 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().setChangePasswordSql(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 groupAuthoritiesByUsername(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..d42b4074c5c 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 @@ -35,6 +35,7 @@ import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; +import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; @@ -184,11 +185,21 @@ 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 public JdbcUserDetailsManagerConfigurer jdbcAuthentication() throws Exception { return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } + @Override + public JdbcUserPasswordDetailsManagerConfigurer jdbcAuthenticationWithPasswordManagement() throws Exception { + return super.jdbcAuthenticationWithPasswordManagement().passwordEncoder(this.defaultPasswordEncoder); + } + @Override public DaoAuthenticationConfigurer userDetailsService( T userDetailsService) throws Exception { diff --git a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java new file mode 100644 index 00000000000..599256a73de --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java @@ -0,0 +1,159 @@ +/* + * 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; + +import javax.sql.DataSource; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; +import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.config.test.SpringTestContext; +import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.core.userdetails.PasswordEncodedUser; +import org.springframework.security.core.userdetails.UserCache; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.core.userdetails.jdbc.JdbcDaoImpl; +import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers; +import org.springframework.test.web.servlet.MockMvc; + +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders.formLogin; +import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; + +/** + * @author Rob Winch + * @author Geir Hedemark + */ +@ExtendWith(SpringTestContextExtension.class) +public class NamespaceJdbcUserPasswordServiceTests { + + public final SpringTestContext spring = new SpringTestContext(this); + + @Autowired + private MockMvc mockMvc; + + @Test + public void jdbcUserService() throws Exception { + this.spring.register(DataSourceConfig.class, JdbcUserServiceConfig.class).autowire(); + SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated().withUsername("user"); + this.mockMvc.perform(formLogin()).andExpect(user); + } + + @Test + public void jdbcUserServiceCustom() throws Exception { + this.spring.register(CustomDataSourceConfig.class, CustomJdbcUserServiceSampleConfig.class).autowire(); + // @formatter:off + SecurityMockMvcResultMatchers.AuthenticatedMatcher dba = authenticated() + .withUsername("user") + .withRoles("DBA", "USER"); + // @formatter:on + this.mockMvc.perform(formLogin()).andExpect(dba); + } + + @Configuration + @EnableWebSecurity + static class JdbcUserServiceConfig { + + @Autowired + void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws Exception { + // @formatter:off + auth + .jdbcAuthenticationWithPasswordManagement() + .withDefaultSchema() + .withUser(PasswordEncodedUser.user()) + .dataSource(dataSource); // jdbc-user-service@data-source-ref + // @formatter:on + } + + } + + @Configuration + static class DataSourceConfig { + + @Bean + DataSource dataSource() { + EmbeddedDatabaseBuilder builder = new EmbeddedDatabaseBuilder(); + return builder.setType(EmbeddedDatabaseType.HSQL).build(); + } + + } + + @Configuration + @EnableWebSecurity + static class CustomJdbcUserServiceSampleConfig { + + @Autowired + void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws Exception { + // @formatter:off + auth + .jdbcAuthenticationWithPasswordManagement() + // jdbc-user-service@dataSource + .dataSource(dataSource) + // jdbc-user-service@cache-ref + .userCache(new CustomUserCache()) + // jdbc-user-service@users-byusername-query + .usersByUsernameQuery("select principal,credentials,true from users where principal = ?") + // jdbc-user-service@change-password-query + .changePasswordQuery("update users set credentials = ? where principal = ?") + // jdbc-user-service@authorities-by-username-query + .authoritiesByUsernameQuery("select principal,role from roles where principal = ?") + // jdbc-user-service@group-authorities-by-username-query + .groupAuthoritiesByUsername(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) + // jdbc-user-service@role-prefix + .rolePrefix("ROLE_"); + // @formatter:on + } + + static class CustomUserCache implements UserCache { + + @Override + public UserDetails getUserFromCache(String username) { + return null; + } + + @Override + public void putUserInCache(UserDetails user) { + } + + @Override + public void removeUserFromCache(String username) { + } + + } + + } + + @Configuration + static class CustomDataSourceConfig { + + @Bean + DataSource dataSource() { + EmbeddedDatabaseBuilder builder = new EmbeddedDatabaseBuilder() + // simulate that the DB already has the schema loaded and users in it + .addScript("CustomJdbcUserServiceSampleConfig.sql"); + return builder.setType(EmbeddedDatabaseType.HSQL).build(); + } + + } + +} 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); From b083e6545815cde2ad592a42dc1796a6a0ffd5df Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Wed, 15 Nov 2023 20:18:28 +0100 Subject: [PATCH 03/13] Made the schema tests run, ensured it is possible to change the change password query for the new manager --- ...ordDetailsManagerBeanDefinitionParser.java | 5 ++ .../security/config/spring-security-6.3.xsd | 7 ++ ...tailsManagerBeanDefinitionParserTests.java | 8 ++ .../config/doc/XsdDocumentedTests.java | 2 +- .../provisioning/JdbcUserDetailsManager.java | 21 +++-- .../JdbcUserPasswordDetailsManager.java | 1 + .../namespace/authentication-manager.adoc | 89 ++++++++++++++++++- 7 files changed, 125 insertions(+), 8 deletions(-) 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 index 92e419be15e..8e4398c5b9b 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -30,6 +30,7 @@ public class JdbcUserPasswordDetailsManagerBeanDefinitionParser extends Abstract 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 @@ -51,6 +52,7 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit 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); @@ -65,6 +67,9 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit builder.addPropertyValue("enableGroups", Boolean.TRUE); builder.addPropertyValue("groupAuthoritiesByUsernameQuery", groupAuthoritiesQuery); } + if (StringUtils.hasText(changePasswordQuery)) { + builder.addPropertyValue("changePasswordQuery", changePasswordQuery); + } } } 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 f7686966133..7b93921411d 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 @@ -3291,6 +3291,13 @@ + + + An SQL statement to change a user's password. The default is + "update users set password = ? where username = ?" + + + A non-empty string prefix that will be added to role strings loaded from persistent 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 index fdab9785763..9b358d9b96c 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -107,6 +107,14 @@ public void groupQueryIsParsedCorrectly() throws Exception { 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, "changePasswordSql")).isEqualTo("blah blah"); + } + @Test public void cacheRefIsparsedCorrectly() { setContext("" 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..9f225da5509 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 @@ -151,7 +151,7 @@ public void sizeWhenReadingFilesystemThenIsCorrectNumberOfSchemaFiles() throws I .list((dir, name) -> name.endsWith(".xsd")); // @formatter:on assertThat(schemas.length) - .withFailMessage("the count is equal to 25, if not then schemaDocument needs updating") + .withFailMessage(String.format("the count should be equal to 24, not the current %d, if not then schemaDocument needs updating", schemas.length)) .isEqualTo(25); } 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 e80f31fc05b..60aa942190e 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -128,7 +128,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private String userExistsSql = DEF_USER_EXISTS_SQL; - protected String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; + public String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; private String findAllGroupsSql = DEF_FIND_GROUPS_SQL; @@ -472,11 +472,6 @@ public void setUserExistsSql(String userExistsSql) { this.userExistsSql = userExistsSql; } - public void setChangePasswordSql(String changePasswordSql) { - Assert.hasText(changePasswordSql, "changePasswordSql should have text"); - this.changePasswordSql = changePasswordSql; - } - public void setFindAllGroupsSql(String findAllGroupsSql) { Assert.hasText(findAllGroupsSql, "findAllGroupsSql should have text"); this.findAllGroupsSql = findAllGroupsSql; @@ -566,4 +561,18 @@ private void validateAuthorities(Collection authorit } } + public void setChangePasswordSql(String changePasswordSql) { + this.changePasswordSql = changePasswordSql; + } + + public String getChangePasswordSql() { + return changePasswordSql; + } + + public void setChangePasswordQuery(String changePasswordSql) { + this.changePasswordSql = changePasswordSql; + } + public String setChangePasswordQuery() { + return changePasswordSql; + } } diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java index 610aca452fe..48c4f52833d 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -40,6 +40,7 @@ * @since TBD */ public class JdbcUserPasswordDetailsManager extends JdbcUserDetailsManager implements UserDetailsPasswordService { + @Override public UserDetails updatePassword(UserDetails user, String newPassword) { this.logger.debug("Updating password for user '" + user.getUsername() + "'"); 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]] From 9e6f12a60338eb8d9cb0d8b6ee5ef431296aa231 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Wed, 27 Dec 2023 11:41:17 +0100 Subject: [PATCH 04/13] gh-7750: Added some rudimentary migration documentation --- ...NamespaceJdbcUserPasswordServiceTests.java | 159 ------------------ .../ROOT/pages/migration-7/jdbcusers.adoc | 59 +++++++ 2 files changed, 59 insertions(+), 159 deletions(-) delete mode 100644 config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java create mode 100644 docs/modules/ROOT/pages/migration-7/jdbcusers.adoc diff --git a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java deleted file mode 100644 index 599256a73de..00000000000 --- a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserPasswordServiceTests.java +++ /dev/null @@ -1,159 +0,0 @@ -/* - * 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; - -import javax.sql.DataSource; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; -import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; -import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; -import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.test.SpringTestContext; -import org.springframework.security.config.test.SpringTestContextExtension; -import org.springframework.security.core.userdetails.PasswordEncodedUser; -import org.springframework.security.core.userdetails.UserCache; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.jdbc.JdbcDaoImpl; -import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers; -import org.springframework.test.web.servlet.MockMvc; - -import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders.formLogin; -import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; - -/** - * @author Rob Winch - * @author Geir Hedemark - */ -@ExtendWith(SpringTestContextExtension.class) -public class NamespaceJdbcUserPasswordServiceTests { - - public final SpringTestContext spring = new SpringTestContext(this); - - @Autowired - private MockMvc mockMvc; - - @Test - public void jdbcUserService() throws Exception { - this.spring.register(DataSourceConfig.class, JdbcUserServiceConfig.class).autowire(); - SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated().withUsername("user"); - this.mockMvc.perform(formLogin()).andExpect(user); - } - - @Test - public void jdbcUserServiceCustom() throws Exception { - this.spring.register(CustomDataSourceConfig.class, CustomJdbcUserServiceSampleConfig.class).autowire(); - // @formatter:off - SecurityMockMvcResultMatchers.AuthenticatedMatcher dba = authenticated() - .withUsername("user") - .withRoles("DBA", "USER"); - // @formatter:on - this.mockMvc.perform(formLogin()).andExpect(dba); - } - - @Configuration - @EnableWebSecurity - static class JdbcUserServiceConfig { - - @Autowired - void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws Exception { - // @formatter:off - auth - .jdbcAuthenticationWithPasswordManagement() - .withDefaultSchema() - .withUser(PasswordEncodedUser.user()) - .dataSource(dataSource); // jdbc-user-service@data-source-ref - // @formatter:on - } - - } - - @Configuration - static class DataSourceConfig { - - @Bean - DataSource dataSource() { - EmbeddedDatabaseBuilder builder = new EmbeddedDatabaseBuilder(); - return builder.setType(EmbeddedDatabaseType.HSQL).build(); - } - - } - - @Configuration - @EnableWebSecurity - static class CustomJdbcUserServiceSampleConfig { - - @Autowired - void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws Exception { - // @formatter:off - auth - .jdbcAuthenticationWithPasswordManagement() - // jdbc-user-service@dataSource - .dataSource(dataSource) - // jdbc-user-service@cache-ref - .userCache(new CustomUserCache()) - // jdbc-user-service@users-byusername-query - .usersByUsernameQuery("select principal,credentials,true from users where principal = ?") - // jdbc-user-service@change-password-query - .changePasswordQuery("update users set credentials = ? where principal = ?") - // jdbc-user-service@authorities-by-username-query - .authoritiesByUsernameQuery("select principal,role from roles where principal = ?") - // jdbc-user-service@group-authorities-by-username-query - .groupAuthoritiesByUsername(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) - // jdbc-user-service@role-prefix - .rolePrefix("ROLE_"); - // @formatter:on - } - - static class CustomUserCache implements UserCache { - - @Override - public UserDetails getUserFromCache(String username) { - return null; - } - - @Override - public void putUserInCache(UserDetails user) { - } - - @Override - public void removeUserFromCache(String username) { - } - - } - - } - - @Configuration - static class CustomDataSourceConfig { - - @Bean - DataSource dataSource() { - EmbeddedDatabaseBuilder builder = new EmbeddedDatabaseBuilder() - // simulate that the DB already has the schema loaded and users in it - .addScript("CustomJdbcUserServiceSampleConfig.sql"); - return builder.setType(EmbeddedDatabaseType.HSQL).build(); - } - - } - -} 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..aea3b115571 --- /dev/null +++ b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc @@ -0,0 +1,59 @@ += 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 a drop-in replacement for the old implementation: + +.... +@Bean +UserDetailsManager users(DataSource dataSource) { + return new JdbcUserPasswordDetailsManager(dataSource); +} +.... + +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. From 77ef6e527d1fe65b790d8af95028eb766e7d45a5 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Wed, 27 Dec 2023 12:25:01 +0100 Subject: [PATCH 05/13] gh-7750: Add back schema support for user password manager --- .../security/config/spring-security-6.3.rnc | 25 +- .../security/config/spring-security-6.3.xsd | 253 ++++++------------ 2 files changed, 110 insertions(+), 168 deletions(-) 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..f37879345b9 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 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 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 7b93921411d..4696c77d1c9 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 @@ -124,11 +124,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -412,11 +408,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -496,11 +488,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -566,11 +554,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -908,23 +892,13 @@ -<<<<<<< HEAD - - - - - - - -======= - - - - - - - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + + + + + + + @@ -1428,11 +1402,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -1457,11 +1427,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -1518,11 +1484,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -1569,11 +1531,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2106,11 +2064,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2167,11 +2121,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2624,11 +2574,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2661,11 +2607,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2728,11 +2670,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2779,11 +2717,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2881,11 +2815,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -2918,13 +2848,8 @@ -<<<<<<< HEAD - - -======= - - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + + @@ -2941,11 +2866,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -3088,11 +3009,7 @@ -<<<<<<< HEAD - -======= - ->>>>>>> 8e41f25ca9 (Added support for xml configuring a JdbcUserPasswordDetailsManager, holding off on supporting specification of password update queries) + @@ -3144,6 +3061,7 @@ + @@ -3241,72 +3159,73 @@ - - - Causes creation of a JDBC-based UserDetailsService that also keeps password encodings updated to the current standard. - - - - - - A bean identifier, used for referring to the bean elsewhere in the context. - - - - - - - - - - The bean ID of the DataSource which provides the required tables. + + + Causes creation of a JDBC-based UserPasswordDetailsService. - - - + + + - Defines a reference to a cache for use with a UserDetailsService. + A bean identifier, used for referring to the bean elsewhere in the context. - - - - An SQL statement to query a username, password, and enabled status given a username. - Default is "select username,password,enabled from users where username = ?" + + + + + + + + The bean ID of the DataSource which provides the required tables. - - - - - An SQL statement to query for a user's granted authorities given a username. The default - is "select username, authority from authorities where username = ?" + + + + + Defines a reference to a cache for use with a UserDetailsService. - - - - - 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 query a username, password, and enabled status given a username. + Default is "select username,password,enabled from users where username = ?" - - - - - An SQL statement to change a user's password. The default is - "update users set password = ? 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 = ?" - - - - - 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. + + + + + 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 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" + + + + + + 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 @@ -3957,4 +3876,4 @@ - + \ No newline at end of file From 43ff8555cda1e09cbabe44f5b43fd975af500722 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Thu, 28 Dec 2023 14:11:48 +0100 Subject: [PATCH 06/13] gh-7750: Changes according to pull request comments --- .../config/SecurityNamespaceHandler.java | 6 +- .../AuthenticationManagerBuilder.java | 5 +- .../AuthenticationConfiguration.java | 5 - .../JdbcUserDetailsManagerConfigurer.java | 4 +- .../HttpSecurityConfiguration.java | 5 - ...ordDetailsManagerBeanDefinitionParser.java | 4 +- .../JdbcUserServiceBeanDefinitionParser.java | 1 + .../NamespaceJdbcUserServiceTests.java | 2 +- ...tailsManagerBeanDefinitionParserTests.java | 4 +- .../provisioning/JdbcUserDetailsManager.java | 25 +- .../JdbcUserPasswordDetailsManager.java | 523 +++++++++++++++++- .../JdbcUserDetailsManagerTests.java | 18 +- .../JdbcUserPasswordDetailsManagerTests.java | 382 ++++++++++++- 13 files changed, 930 insertions(+), 54 deletions(-) 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 7f505a2be98..a76676c6b34 100644 --- a/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java +++ b/config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java @@ -21,7 +21,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.security.config.authentication.*; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -31,6 +30,11 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.NamespaceHandler; 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; import org.springframework.security.config.http.FilterChainMapBeanDefinitionDecorator; import org.springframework.security.config.http.FilterInvocationSecurityMetadataSourceParser; 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 9b7a0c25f70..bdb6b287860 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 @@ -34,7 +34,6 @@ import org.springframework.security.config.annotation.authentication.configurers.ldap.LdapAuthenticationProviderConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; -import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.UserDetailsAwareConfigurer; import org.springframework.security.core.Authentication; @@ -150,14 +149,14 @@ 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 Use jdbcAuthenticationWithPasswordManagement instead, as this keeps the password encoding up to - * date over time. Please consult the migrationn documentation as database changes might be necessary. + * date over time. Please consult the migration documentation for Spring 7 as database changes might be necessary. */ 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 5e6146ea665..028bbd3c3d4 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 @@ -299,11 +299,6 @@ public JdbcUserDetailsManagerConfigurer jdbcAuthen return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } - @Override - public JdbcUserPasswordDetailsManagerConfigurer jdbcAuthenticationWithPasswordManagement() throws Exception { - return super.jdbcAuthenticationWithPasswordManagement().passwordEncoder(this.defaultPasswordEncoder); - } - @Override public DaoAuthenticationConfigurer userDetailsService( T userDetailsService) throws Exception { diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java index 1020cd62072..2bbac6f0e18 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java @@ -44,7 +44,7 @@ * @author Rob Winch * @since 3.2 */ -public class JdbcUserDetailsManagerConfigurer> +public final class JdbcUserDetailsManagerConfigurer> extends UserDetailsManagerConfigurer> { private DataSource dataSource; @@ -120,7 +120,7 @@ public JdbcUserDetailsManagerConfigurer authoritiesByUsernameQuery(String que * @return The {@link JdbcUserDetailsManagerConfigurer} used for additional * customizations */ - public JdbcUserDetailsManagerConfigurer groupAuthoritiesByUsername(String query) { + public JdbcUserDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { JdbcUserDetailsManager userDetailsService = getUserDetailsService(); userDetailsService.setEnableGroups(true); userDetailsService.setGroupAuthoritiesByUsernameQuery(query); 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 d42b4074c5c..f674b4f5567 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 @@ -195,11 +195,6 @@ public JdbcUserDetailsManagerConfigurer jdbcAuthen return super.jdbcAuthentication().passwordEncoder(this.defaultPasswordEncoder); } - @Override - public JdbcUserPasswordDetailsManagerConfigurer jdbcAuthenticationWithPasswordManagement() throws Exception { - return super.jdbcAuthenticationWithPasswordManagement().passwordEncoder(this.defaultPasswordEncoder); - } - @Override public DaoAuthenticationConfigurer userDetailsService( T userDetailsService) throws Exception { 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 index 8e4398c5b9b..1620559718d 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -46,7 +46,7 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit } else { parserContext.getReaderContext() - .error(ATT_DATA_SOURCE + " is required for " + Elements.JDBC_USER_SERVICE, + .error(ATT_DATA_SOURCE + " is required for " + Elements.JDBC_USER_PASSWORD_SERVICE, parserContext.extractSource(element)); } String usersQuery = element.getAttribute(ATT_USERS_BY_USERNAME_QUERY); @@ -68,7 +68,7 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit builder.addPropertyValue("groupAuthoritiesByUsernameQuery", groupAuthoritiesQuery); } if (StringUtils.hasText(changePasswordQuery)) { - builder.addPropertyValue("changePasswordQuery", changePasswordQuery); + builder.addPropertyValue("changePasswordSql", 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 59a62f34329..378176ae33f 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 @@ -28,6 +28,7 @@ * @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/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java index 751a4a97b36..5d865118a24 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java @@ -115,7 +115,7 @@ void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws // jdbc-user-service@authorities-by-username-query .authoritiesByUsernameQuery("select principal,role from roles where principal = ?") // jdbc-user-service@group-authorities-by-username-query - .groupAuthoritiesByUsername(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) + .groupAuthoritiesByUsernameQuery(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) // jdbc-user-service@role-prefix .rolePrefix("ROLE_"); // @formatter:on 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 index 9b358d9b96c..4be536e3fee 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -18,6 +18,8 @@ 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; @@ -29,7 +31,6 @@ import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.provisioning.JdbcUserPasswordDetailsManager; import org.springframework.security.util.FieldUtils; -import org.w3c.dom.Element; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -169,6 +170,7 @@ public void rolePrefixIsUsedWhenSet() { } private void setContext(String context) { + System.out.println("Context: "+ context); this.appContext = new InMemoryXmlApplicationContext(context); } 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 60aa942190e..4ec004d7b14 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2022 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. @@ -128,7 +128,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private String userExistsSql = DEF_USER_EXISTS_SQL; - public String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; + private String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; private String findAllGroupsSql = DEF_FIND_GROUPS_SQL; @@ -158,7 +158,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private AuthenticationManager authenticationManager; - protected UserCache userCache = new NullUserCache(); + private UserCache userCache = new NullUserCache(); public JdbcUserDetailsManager() { } @@ -472,6 +472,11 @@ public void setUserExistsSql(String userExistsSql) { this.userExistsSql = userExistsSql; } + public void setChangePasswordSql(String changePasswordSql) { + Assert.hasText(changePasswordSql, "changePasswordSql should have text"); + this.changePasswordSql = changePasswordSql; + } + public void setFindAllGroupsSql(String findAllGroupsSql) { Assert.hasText(findAllGroupsSql, "findAllGroupsSql should have text"); this.findAllGroupsSql = findAllGroupsSql; @@ -561,18 +566,4 @@ private void validateAuthorities(Collection authorit } } - public void setChangePasswordSql(String changePasswordSql) { - this.changePasswordSql = changePasswordSql; - } - - public String getChangePasswordSql() { - return changePasswordSql; - } - - public void setChangePasswordQuery(String changePasswordSql) { - this.changePasswordSql = changePasswordSql; - } - public String setChangePasswordQuery() { - return changePasswordSql; - } } diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java index 48c4f52833d..b78ef2be28f 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -16,9 +16,38 @@ 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, @@ -39,7 +68,499 @@ * @author Geir Hedemark * @since TBD */ -public class JdbcUserPasswordDetailsManager extends JdbcUserDetailsManager implements UserDetailsPasswordService { +public class JdbcUserPasswordDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager, UserDetailsPasswordService { + + public static final String DEF_CREATE_USER_SQL = "insert into users (username, password, enabled) values (?,?,?)"; + + public static final String DEF_DELETE_USER_SQL = "delete from users where username = ?"; + + public static final String DEF_UPDATE_USER_SQL = "update users set password = ?, enabled = ? where username = ?"; + + public static final String DEF_INSERT_AUTHORITY_SQL = "insert into authorities (username, authority) values (?,?)"; + + public static final String DEF_DELETE_USER_AUTHORITIES_SQL = "delete from authorities where username = ?"; + + public static final String DEF_USER_EXISTS_SQL = "select username from users where username = ?"; + + public static final String DEF_CHANGE_PASSWORD_SQL = "update users set password = ? where username = ?"; + + public static final String DEF_FIND_GROUPS_SQL = "select group_name from groups"; + + public static final String DEF_FIND_USERS_IN_GROUP_SQL = "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_SQL = "insert into groups (group_name) values (?)"; + + public static final String DEF_FIND_GROUP_ID_SQL = "select id from groups where group_name = ?"; + + public static final String DEF_INSERT_GROUP_AUTHORITY_SQL = "insert into group_authorities (group_id, authority) values (?,?)"; + + public static final String DEF_DELETE_GROUP_SQL = "delete from groups where id = ?"; + + public static final String DEF_DELETE_GROUP_AUTHORITIES_SQL = "delete from group_authorities where group_id = ?"; + + public static final String DEF_DELETE_GROUP_MEMBERS_SQL = "delete from group_members where group_id = ?"; + + public static final String DEF_RENAME_GROUP_SQL = "update groups set group_name = ? where group_name = ?"; + + public static final String DEF_INSERT_GROUP_MEMBER_SQL = "insert into group_members (group_id, username) values (?,?)"; + + public static final String DEF_DELETE_GROUP_MEMBER_SQL = "delete from group_members where group_id = ? and username = ?"; + + public static final String DEF_GROUP_AUTHORITIES_QUERY_SQL = "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_SQL = "delete from group_authorities where group_id = ? and authority = ?"; + + protected final Log logger = LogFactory.getLog(getClass()); + + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + + private String createUserSql = DEF_CREATE_USER_SQL; + + private String deleteUserSql = DEF_DELETE_USER_SQL; + + private String updateUserSql = DEF_UPDATE_USER_SQL; + + private String createAuthoritySql = DEF_INSERT_AUTHORITY_SQL; + + private String deleteUserAuthoritiesSql = DEF_DELETE_USER_AUTHORITIES_SQL; + + private String userExistsSql = DEF_USER_EXISTS_SQL; + + private String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; + + private String findAllGroupsSql = DEF_FIND_GROUPS_SQL; + + private String findUsersInGroupSql = DEF_FIND_USERS_IN_GROUP_SQL; + + private String insertGroupSql = DEF_INSERT_GROUP_SQL; + + private String findGroupIdSql = DEF_FIND_GROUP_ID_SQL; + + private String insertGroupAuthoritySql = DEF_INSERT_GROUP_AUTHORITY_SQL; + + private String deleteGroupSql = DEF_DELETE_GROUP_SQL; + + private String deleteGroupAuthoritiesSql = DEF_DELETE_GROUP_AUTHORITIES_SQL; + + private String deleteGroupMembersSql = DEF_DELETE_GROUP_MEMBERS_SQL; + + private String renameGroupSql = DEF_RENAME_GROUP_SQL; + + private String insertGroupMemberSql = DEF_INSERT_GROUP_MEMBER_SQL; + + private String deleteGroupMemberSql = DEF_DELETE_GROUP_MEMBER_SQL; + + private String groupAuthoritiesSql = DEF_GROUP_AUTHORITIES_QUERY_SQL; + + private String deleteGroupAuthoritySql = DEF_DELETE_GROUP_AUTHORITY_SQL; + + 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.createUserSql, (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.updateUserSql, (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.createAuthoritySql, user.getUsername(), auth.getAuthority()); + } + } + + public void deleteUser(String username) { + if (getEnableAuthorities()) { + deleteUserAuthorities(username); + } + getJdbcTemplate().update(this.deleteUserSql, username); + this.userCache.removeUserFromCache(username); + } + + private void deleteUserAuthorities(String username) { + getJdbcTemplate().update(this.deleteUserAuthoritiesSql, 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.changePasswordSql, 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.userExistsSql, 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.findAllGroupsSql, String.class); + } + + @Override + public List findUsersInGroup(String groupName) { + Assert.hasText(groupName, "groupName should have text"); + return getJdbcTemplate().queryForList(this.findUsersInGroupSql, 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.insertGroupSql, groupName); + int groupId = findGroupId(groupName); + for (GrantedAuthority a : authorities) { + String authority = a.getAuthority(); + getJdbcTemplate().update(this.insertGroupAuthoritySql, (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.deleteGroupMembersSql, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupAuthoritiesSql, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupSql, 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.renameGroupSql, 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.insertGroupMemberSql, (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.deleteGroupMemberSql, (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.groupAuthoritiesSql, 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.deleteGroupAuthoritySql, (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.insertGroupAuthoritySql, (ps) -> { + ps.setInt(1, id); + ps.setString(2, authority.getAuthority()); + }); + } + + private int findGroupId(String group) { + return getJdbcTemplate().queryForObject(this.findGroupIdSql, 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 setCreateUserSql(String createUserSql) { + Assert.hasText(createUserSql, "createUserSql should have text"); + this.createUserSql = createUserSql; + } + + public void setDeleteUserSql(String deleteUserSql) { + Assert.hasText(deleteUserSql, "deleteUserSql should have text"); + this.deleteUserSql = deleteUserSql; + } + + public void setUpdateUserSql(String updateUserSql) { + Assert.hasText(updateUserSql, "updateUserSql should have text"); + this.updateUserSql = updateUserSql; + } + + public void setCreateAuthoritySql(String createAuthoritySql) { + Assert.hasText(createAuthoritySql, "createAuthoritySql should have text"); + this.createAuthoritySql = createAuthoritySql; + } + + public void setDeleteUserAuthoritiesSql(String deleteUserAuthoritiesSql) { + Assert.hasText(deleteUserAuthoritiesSql, "deleteUserAuthoritiesSql should have text"); + this.deleteUserAuthoritiesSql = deleteUserAuthoritiesSql; + } + + public void setUserExistsSql(String userExistsSql) { + Assert.hasText(userExistsSql, "userExistsSql should have text"); + this.userExistsSql = userExistsSql; + } + + public void setChangePasswordSql(String changePasswordSql) { + Assert.hasText(changePasswordSql, "changePasswordSql should have text"); + this.changePasswordSql = changePasswordSql; + } + + public void setFindAllGroupsSql(String findAllGroupsSql) { + Assert.hasText(findAllGroupsSql, "findAllGroupsSql should have text"); + this.findAllGroupsSql = findAllGroupsSql; + } + + public void setFindUsersInGroupSql(String findUsersInGroupSql) { + Assert.hasText(findUsersInGroupSql, "findUsersInGroupSql should have text"); + this.findUsersInGroupSql = findUsersInGroupSql; + } + + public void setInsertGroupSql(String insertGroupSql) { + Assert.hasText(insertGroupSql, "insertGroupSql should have text"); + this.insertGroupSql = insertGroupSql; + } + + public void setFindGroupIdSql(String findGroupIdSql) { + Assert.hasText(findGroupIdSql, "findGroupIdSql should have text"); + this.findGroupIdSql = findGroupIdSql; + } + + public void setInsertGroupAuthoritySql(String insertGroupAuthoritySql) { + Assert.hasText(insertGroupAuthoritySql, "insertGroupAuthoritySql should have text"); + this.insertGroupAuthoritySql = insertGroupAuthoritySql; + } + + public void setDeleteGroupSql(String deleteGroupSql) { + Assert.hasText(deleteGroupSql, "deleteGroupSql should have text"); + this.deleteGroupSql = deleteGroupSql; + } + + public void setDeleteGroupAuthoritiesSql(String deleteGroupAuthoritiesSql) { + Assert.hasText(deleteGroupAuthoritiesSql, "deleteGroupAuthoritiesSql should have text"); + this.deleteGroupAuthoritiesSql = deleteGroupAuthoritiesSql; + } + + public void setDeleteGroupMembersSql(String deleteGroupMembersSql) { + Assert.hasText(deleteGroupMembersSql, "deleteGroupMembersSql should have text"); + this.deleteGroupMembersSql = deleteGroupMembersSql; + } + + public void setRenameGroupSql(String renameGroupSql) { + Assert.hasText(renameGroupSql, "renameGroupSql should have text"); + this.renameGroupSql = renameGroupSql; + } + + public void setInsertGroupMemberSql(String insertGroupMemberSql) { + Assert.hasText(insertGroupMemberSql, "insertGroupMemberSql should have text"); + this.insertGroupMemberSql = insertGroupMemberSql; + } + + public void setDeleteGroupMemberSql(String deleteGroupMemberSql) { + Assert.hasText(deleteGroupMemberSql, "deleteGroupMemberSql should have text"); + this.deleteGroupMemberSql = deleteGroupMemberSql; + } + + public void setGroupAuthoritiesSql(String groupAuthoritiesSql) { + Assert.hasText(groupAuthoritiesSql, "groupAuthoritiesSql should have text"); + this.groupAuthoritiesSql = groupAuthoritiesSql; + } + + public void setDeleteGroupAuthoritySql(String deleteGroupAuthoritySql) { + Assert.hasText(deleteGroupAuthoritySql, "deleteGroupAuthoritySql should have text"); + this.deleteGroupAuthoritySql = deleteGroupAuthoritySql; + } + + /** + * 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) { 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 a1cfdd656a5..67d0c83fa59 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2022 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. @@ -68,9 +68,9 @@ public class JdbcUserDetailsManagerTests { private static TestDataSource dataSource; - JdbcUserDetailsManager manager; + private JdbcUserDetailsManager manager; - MockUserCache cache; + private MockUserCache cache; private JdbcTemplate template; @@ -85,13 +85,9 @@ public static void clearDataSource() throws Exception { dataSource = null; } - public JdbcUserDetailsManager makeInstance() { - return new JdbcUserDetailsManager(); - } - @BeforeEach public void initializeManagerAndCreateTables() { - this.manager = makeInstance(); + this.manager = new JdbcUserDetailsManager(); this.cache = new MockUserCache(); this.manager.setUserCache(this.cache); this.manager.setDataSource(dataSource); @@ -376,7 +372,7 @@ private Authentication authenticateJoe() { return auth; } - public void insertJoe() { + 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')"); @@ -384,7 +380,7 @@ public void insertJoe() { this.cache.putUserInCache(joe); } - public class MockUserCache implements UserCache { + private class MockUserCache implements UserCache { private Map cache = new HashMap<>(); @@ -403,7 +399,7 @@ public void removeUserFromCache(String username) { this.cache.remove(username); } - public Map getUserMap() { + Map getUserMap() { return this.cache; } diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java index 93d6138502f..3835bb08d99 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -15,20 +15,351 @@ */ 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.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.BDDMockito.*; +import static org.mockito.Mockito.*; /** * Tests for {@link JdbcUserPasswordDetailsManager} * * @author Geir Hedemark */ -public class JdbcUserPasswordDetailsManagerTests extends JdbcUserDetailsManagerTests { - @Override - public JdbcUserDetailsManager makeInstance() { - return new JdbcUserPasswordDetailsManager(); +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.setCreateUserSql(JdbcUserPasswordDetailsManager.DEF_CREATE_USER_SQL); + this.manager.setUpdateUserSql(JdbcUserPasswordDetailsManager.DEF_UPDATE_USER_SQL); + this.manager.setUserExistsSql(JdbcUserPasswordDetailsManager.DEF_USER_EXISTS_SQL); + this.manager.setCreateAuthoritySql(JdbcUserPasswordDetailsManager.DEF_INSERT_AUTHORITY_SQL); + this.manager.setDeleteUserAuthoritiesSql(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_AUTHORITIES_SQL); + this.manager.setDeleteUserSql(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_SQL); + this.manager.setChangePasswordSql(JdbcUserPasswordDetailsManager.DEF_CHANGE_PASSWORD_SQL); + 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.setCreateUserSql( + "insert into users (username, password, enabled, acc_locked, acc_expired, creds_expired) values (?,?,?,?,?,?)"); + this.manager.setUpdateUserSql( + "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 @@ -41,4 +372,45 @@ public void updatePasswordSucceeds() { 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; + } + + } + } From 5973f395ccb9fa5fa81b0f07e8118c9751ab877c Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Thu, 28 Dec 2023 14:49:24 +0100 Subject: [PATCH 07/13] gh-7750: Changes after my own walkthrough of the PR --- .../java/org/springframework/security/config/Elements.java | 2 +- .../builders/AuthenticationManagerBuilder.java | 5 +++-- .../configuration/AuthenticationConfiguration.java | 4 ++-- .../provisioning/JdbcUserDetailsManagerConfigurer.java | 4 ++-- .../JdbcUserPasswordDetailsManagerConfigurer.java | 6 +++--- .../web/configuration/HttpSecurityConfiguration.java | 2 +- .../JdbcUserPasswordDetailsManagerBeanDefinitionParser.java | 2 +- .../springframework/security/config/spring-security-6.3.rnc | 2 +- .../springframework/security/config/spring-security-6.3.xsd | 5 ++--- .../authentication/NamespaceJdbcUserServiceTests.java | 2 +- .../security/config/doc/XsdDocumentedTests.java | 5 +++-- .../security/provisioning/JdbcUserDetailsManager.java | 2 +- .../provisioning/JdbcUserPasswordDetailsManager.java | 5 ++++- .../security/provisioning/JdbcUserDetailsManagerTests.java | 4 ++++ docs/modules/ROOT/pages/migration-7/jdbcusers.adoc | 6 +++++- 15 files changed, 34 insertions(+), 22 deletions(-) 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 58245e89b15..74b638fb2b6 100644 --- a/config/src/main/java/org/springframework/security/config/Elements.java +++ b/config/src/main/java/org/springframework/security/config/Elements.java @@ -32,7 +32,7 @@ public abstract class Elements { public static final String USER_SERVICE = "user-service"; - @Deprecated + @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"; 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 bdb6b287860..c9c5d394f66 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 @@ -155,9 +155,10 @@ public InMemoryUserDetailsManagerConfigurer inMemo * @return a {@link JdbcUserDetailsManagerConfigurer} to allow customization of the * JDBC authentication * @throws Exception if an error occurs when adding the JDBC authentication - * @deprecated Use jdbcAuthenticationWithPasswordManagement instead, as this keeps the password encoding up to - * date over time. Please consult the migration documentation for Spring 7 as database changes might be necessary. + * @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 028bbd3c3d4..4032d5f992d 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 @@ -290,11 +290,11 @@ public InMemoryUserDetailsManagerConfigurer inMemo } /** - * @deprecated Use jdbcAuthenticationWithPasswordManagement instead, as this keeps the password up to date. + * @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 + @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/JdbcUserDetailsManagerConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java index 2bbac6f0e18..1020cd62072 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/provisioning/JdbcUserDetailsManagerConfigurer.java @@ -44,7 +44,7 @@ * @author Rob Winch * @since 3.2 */ -public final class JdbcUserDetailsManagerConfigurer> +public class JdbcUserDetailsManagerConfigurer> extends UserDetailsManagerConfigurer> { private DataSource dataSource; @@ -120,7 +120,7 @@ public JdbcUserDetailsManagerConfigurer authoritiesByUsernameQuery(String que * @return The {@link JdbcUserDetailsManagerConfigurer} used for additional * customizations */ - public JdbcUserDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { + public JdbcUserDetailsManagerConfigurer groupAuthoritiesByUsername(String query) { JdbcUserDetailsManager userDetailsService = getUserDetailsService(); userDetailsService.setEnableGroups(true); userDetailsService.setGroupAuthoritiesByUsernameQuery(query); 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 index f98782b7314..0d3ecd11f82 100644 --- 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 @@ -44,9 +44,9 @@ * @param the type of the {@link ProviderManagerBuilder} that is being configured * @author Rob Winch * @author Geir Hedemark - * @since TBD + * @since 6.3 */ -public class JdbcUserPasswordDetailsManagerConfigurer> +public final class JdbcUserPasswordDetailsManagerConfigurer> extends UserDetailsManagerConfigurer> { private DataSource dataSource; @@ -137,7 +137,7 @@ public JdbcUserPasswordDetailsManagerConfigurer authoritiesByUsernameQuery(St * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional * customizations */ - public JdbcUserPasswordDetailsManagerConfigurer groupAuthoritiesByUsername(String query) { +public JdbcUserPasswordDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { JdbcUserPasswordDetailsManager userDetailsService = getUserDetailsService(); userDetailsService.setEnableGroups(true); userDetailsService.setGroupAuthoritiesByUsernameQuery(query); 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 f674b4f5567..acba62b46f2 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 @@ -190,7 +190,7 @@ public InMemoryUserDetailsManagerConfigurer inMemo * Please consult the migration documentation as database changes might be necessary. */ @Override - @Deprecated + @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/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java index 1620559718d..43294dd8d0b 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -25,7 +25,7 @@ * @author Luke Taylor * @author Geir Hedemark */ -public class JdbcUserPasswordDetailsManagerBeanDefinitionParser extends AbstractUserDetailsServiceBeanDefinitionParser { +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"; 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 f37879345b9..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 @@ -1141,7 +1141,7 @@ 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 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 = ?" attribute change-password-query {xsd:token}? jdbc-user-password-service.attlist &= role-prefix? 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 4696c77d1c9..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 @@ -3211,9 +3211,8 @@ - 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 = ?" diff --git a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java index 5d865118a24..751a4a97b36 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/authentication/NamespaceJdbcUserServiceTests.java @@ -115,7 +115,7 @@ void configure(AuthenticationManagerBuilder auth, DataSource dataSource) throws // jdbc-user-service@authorities-by-username-query .authoritiesByUsernameQuery("select principal,role from roles where principal = ?") // jdbc-user-service@group-authorities-by-username-query - .groupAuthoritiesByUsernameQuery(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) + .groupAuthoritiesByUsername(JdbcDaoImpl.DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY) // jdbc-user-service@role-prefix .rolePrefix("ROLE_"); // @formatter:on 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 9f225da5509..5cee01228fd 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,10 @@ public void sizeWhenReadingFilesystemThenIsCorrectNumberOfSchemaFiles() throws I .getParentFile() .list((dir, name) -> name.endsWith(".xsd")); // @formatter:on + int expectedCount = 25; assertThat(schemas.length) - .withFailMessage(String.format("the count should be equal to 24, not the current %d, if not then schemaDocument needs updating", schemas.length)) - .isEqualTo(25); + .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/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java index 4ec004d7b14..c2a07fcd76e 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -66,7 +66,7 @@ * ensure password encoding is kept updated. Please note that this migration might require changes to your code * and database. */ -@Deprecated +@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 index b78ef2be28f..5217784e793 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -65,8 +65,11 @@ * 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 TBD + * @since 6.3 */ public class JdbcUserPasswordDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager, UserDetailsPasswordService { 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..b14e381601d 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,11 @@ * 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/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc index aea3b115571..b526cf9222a 100644 --- a/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc +++ b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc @@ -43,6 +43,9 @@ UserDetailsManager users(DataSource dataSource) { } .... +There is an additional configuration parameter 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. @@ -56,4 +59,5 @@ creation of the new bean: .... -The configuration settings are the same as for the old 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. From 3bc6072c458ea2b2c6d9dec2e38b2671a142058c Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Thu, 28 Dec 2023 15:06:54 +0100 Subject: [PATCH 08/13] gh-7750: Rename all JdbcUserPasswordDetailsManager properties named *Sql to *Query, update documentation --- ...cUserPasswordDetailsManagerConfigurer.java | 2 +- ...ordDetailsManagerBeanDefinitionParser.java | 2 +- ...tailsManagerBeanDefinitionParserTests.java | 2 +- .../JdbcUserPasswordDetailsManager.java | 244 +++++++++--------- .../JdbcUserPasswordDetailsManagerTests.java | 19 +- .../ROOT/pages/migration-7/jdbcusers.adoc | 7 +- 6 files changed, 139 insertions(+), 137 deletions(-) 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 index 0d3ecd11f82..eda181b5dc5 100644 --- 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 @@ -100,7 +100,7 @@ public JdbcUserPasswordDetailsManagerConfigurer usersByUsernameQuery(String q * customizations */ public JdbcUserPasswordDetailsManagerConfigurer changePasswordQuery(String query) { - getUserDetailsService().setChangePasswordSql(query); + getUserDetailsService().setChangePasswordQuery(query); return this; } 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 index 43294dd8d0b..c18a4bd629f 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -68,7 +68,7 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit builder.addPropertyValue("groupAuthoritiesByUsernameQuery", groupAuthoritiesQuery); } if (StringUtils.hasText(changePasswordQuery)) { - builder.addPropertyValue("changePasswordSql", changePasswordQuery); + builder.addPropertyValue("changePasswordQuery", changePasswordQuery); } } 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 index 4be536e3fee..a765551baf9 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -113,7 +113,7 @@ public void changePasswordQueryIsParsedCorrectly() throws Exception { setContext("" + DATA_SOURCE); JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean("myUserService"); - assertThat(FieldUtils.getFieldValue(mgr, "changePasswordSql")).isEqualTo("blah blah"); + assertThat(FieldUtils.getFieldValue(mgr, "changePasswordQuery")).isEqualTo("blah blah"); } @Test diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java index 5217784e793..30d3b83e556 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -73,92 +73,92 @@ */ public class JdbcUserPasswordDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager, UserDetailsPasswordService { - public static final String DEF_CREATE_USER_SQL = "insert into users (username, password, enabled) values (?,?,?)"; + public static final String DEF_CREATE_USER_QUERY = "insert into users (username, password, enabled) values (?,?,?)"; - public static final String DEF_DELETE_USER_SQL = "delete from users where username = ?"; + public static final String DEF_DELETE_USER_QUERY = "delete from users where username = ?"; - public static final String DEF_UPDATE_USER_SQL = "update users set password = ?, enabled = ? where username = ?"; + public static final String DEF_UPDATE_USER_QUERY = "update users set password = ?, enabled = ? where username = ?"; - public static final String DEF_INSERT_AUTHORITY_SQL = "insert into authorities (username, authority) values (?,?)"; + public static final String DEF_INSERT_AUTHORITY_QUERY = "insert into authorities (username, authority) values (?,?)"; - public static final String DEF_DELETE_USER_AUTHORITIES_SQL = "delete from authorities where username = ?"; + public static final String DEF_DELETE_USER_AUTHORITIES_QUERY = "delete from authorities where username = ?"; - public static final String DEF_USER_EXISTS_SQL = "select username from users where username = ?"; + public static final String DEF_USER_EXISTS_QUERY = "select username from users where username = ?"; - public static final String DEF_CHANGE_PASSWORD_SQL = "update users set password = ? where username = ?"; + public static final String DEF_CHANGE_PASSWORD_QUERY = "update users set password = ? where username = ?"; - public static final String DEF_FIND_GROUPS_SQL = "select group_name from groups"; + public static final String DEF_FIND_GROUPS_QUERY = "select group_name from groups"; - public static final String DEF_FIND_USERS_IN_GROUP_SQL = "select username from group_members gm, groups g " + 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_SQL = "insert into groups (group_name) values (?)"; + public static final String DEF_INSERT_GROUP_QUERY = "insert into groups (group_name) values (?)"; - public static final String DEF_FIND_GROUP_ID_SQL = "select id from groups where group_name = ?"; + public static final String DEF_FIND_GROUP_ID_QUERY = "select id from groups where group_name = ?"; - public static final String DEF_INSERT_GROUP_AUTHORITY_SQL = "insert into group_authorities (group_id, authority) values (?,?)"; + public static final String DEF_INSERT_GROUP_AUTHORITY_QUERY = "insert into group_authorities (group_id, authority) values (?,?)"; - public static final String DEF_DELETE_GROUP_SQL = "delete from groups where id = ?"; + public static final String DEF_DELETE_GROUP_QUERY = "delete from groups where id = ?"; - public static final String DEF_DELETE_GROUP_AUTHORITIES_SQL = "delete from group_authorities where group_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_SQL = "delete from group_members 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_SQL = "update groups set group_name = ? where group_name = ?"; + public static final String DEF_RENAME_GROUP_QUERY = "update groups set group_name = ? where group_name = ?"; - public static final String DEF_INSERT_GROUP_MEMBER_SQL = "insert into group_members (group_id, username) values (?,?)"; + 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_SQL = "delete from group_members where group_id = ? and username = ?"; + 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_SQL = "select g.id, g.group_name, ga.authority " + 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_SQL = "delete from group_authorities where group_id = ? and authority = ?"; + 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 createUserSql = DEF_CREATE_USER_SQL; + private String createUserQuery = DEF_CREATE_USER_QUERY; - private String deleteUserSql = DEF_DELETE_USER_SQL; + private String deleteUserQuery = DEF_DELETE_USER_QUERY; - private String updateUserSql = DEF_UPDATE_USER_SQL; + private String updateUserQuery = DEF_UPDATE_USER_QUERY; - private String createAuthoritySql = DEF_INSERT_AUTHORITY_SQL; + private String createAuthorityQuery = DEF_INSERT_AUTHORITY_QUERY; - private String deleteUserAuthoritiesSql = DEF_DELETE_USER_AUTHORITIES_SQL; + private String deleteUserAuthoritiesQuery = DEF_DELETE_USER_AUTHORITIES_QUERY; - private String userExistsSql = DEF_USER_EXISTS_SQL; + private String userExistsQuery = DEF_USER_EXISTS_QUERY; - private String changePasswordSql = DEF_CHANGE_PASSWORD_SQL; + private String changePasswordQuery = DEF_CHANGE_PASSWORD_QUERY; - private String findAllGroupsSql = DEF_FIND_GROUPS_SQL; + private String findAllGroupsQuery = DEF_FIND_GROUPS_QUERY; - private String findUsersInGroupSql = DEF_FIND_USERS_IN_GROUP_SQL; + private String findUsersInGroupQuery = DEF_FIND_USERS_IN_GROUP_QUERY; - private String insertGroupSql = DEF_INSERT_GROUP_SQL; + private String insertGroupQuery = DEF_INSERT_GROUP_QUERY; - private String findGroupIdSql = DEF_FIND_GROUP_ID_SQL; + private String findGroupIdQuery = DEF_FIND_GROUP_ID_QUERY; - private String insertGroupAuthoritySql = DEF_INSERT_GROUP_AUTHORITY_SQL; + private String insertGroupAuthorityQuery = DEF_INSERT_GROUP_AUTHORITY_QUERY; - private String deleteGroupSql = DEF_DELETE_GROUP_SQL; + private String deleteGroupQuery = DEF_DELETE_GROUP_QUERY; - private String deleteGroupAuthoritiesSql = DEF_DELETE_GROUP_AUTHORITIES_SQL; + private String deleteGroupAuthoritiesQuery = DEF_DELETE_GROUP_AUTHORITIES_QUERY; - private String deleteGroupMembersSql = DEF_DELETE_GROUP_MEMBERS_SQL; + private String deleteGroupMembersQuery = DEF_DELETE_GROUP_MEMBERS_QUERY; - private String renameGroupSql = DEF_RENAME_GROUP_SQL; + private String renameGroupQuery = DEF_RENAME_GROUP_QUERY; - private String insertGroupMemberSql = DEF_INSERT_GROUP_MEMBER_SQL; + private String insertGroupMemberQuery = DEF_INSERT_GROUP_MEMBER_QUERY; - private String deleteGroupMemberSql = DEF_DELETE_GROUP_MEMBER_SQL; + private String deleteGroupMemberQuery = DEF_DELETE_GROUP_MEMBER_QUERY; - private String groupAuthoritiesSql = DEF_GROUP_AUTHORITIES_QUERY_SQL; + private String groupAuthoritiesQuery = DEF_GROUP_AUTHORITIES_QUERY_QUERY; - private String deleteGroupAuthoritySql = DEF_DELETE_GROUP_AUTHORITY_SQL; + private String deleteGroupAuthorityQuery = DEF_DELETE_GROUP_AUTHORITY_QUERY; private AuthenticationManager authenticationManager; @@ -207,7 +207,7 @@ private UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { public void createUser(final UserDetails user) { validateUserDetails(user); - getJdbcTemplate().update(this.createUserSql, (ps) -> { + getJdbcTemplate().update(this.createUserQuery, (ps) -> { ps.setString(1, user.getUsername()); ps.setString(2, user.getPassword()); ps.setBoolean(3, user.isEnabled()); @@ -226,7 +226,7 @@ public void createUser(final UserDetails user) { public void updateUser(final UserDetails user) { validateUserDetails(user); - getJdbcTemplate().update(this.updateUserSql, (ps) -> { + getJdbcTemplate().update(this.updateUserQuery, (ps) -> { ps.setString(1, user.getPassword()); ps.setBoolean(2, user.isEnabled()); int paramCount = ps.getParameterMetaData().getParameterCount(); @@ -250,7 +250,7 @@ public void updateUser(final UserDetails user) { private void insertUserAuthorities(UserDetails user) { for (GrantedAuthority auth : user.getAuthorities()) { - getJdbcTemplate().update(this.createAuthoritySql, user.getUsername(), auth.getAuthority()); + getJdbcTemplate().update(this.createAuthorityQuery, user.getUsername(), auth.getAuthority()); } } @@ -258,12 +258,12 @@ public void deleteUser(String username) { if (getEnableAuthorities()) { deleteUserAuthorities(username); } - getJdbcTemplate().update(this.deleteUserSql, username); + getJdbcTemplate().update(this.deleteUserQuery, username); this.userCache.removeUserFromCache(username); } private void deleteUserAuthorities(String username) { - getJdbcTemplate().update(this.deleteUserAuthoritiesSql, username); + getJdbcTemplate().update(this.deleteUserAuthoritiesQuery, username); } public void changePassword(String oldPassword, String newPassword) throws AuthenticationException { @@ -285,7 +285,7 @@ public void changePassword(String oldPassword, String newPassword) throws Authen this.logger.debug("No authentication manager set. Password won't be re-checked."); } this.logger.debug("Changing password for user '" + username + "'"); - getJdbcTemplate().update(this.changePasswordSql, newPassword, username); + getJdbcTemplate().update(this.changePasswordQuery, newPassword, username); Authentication authentication = createNewAuthentication(currentUser, newPassword); SecurityContext context = this.securityContextHolderStrategy.createEmptyContext(); context.setAuthentication(authentication); @@ -303,7 +303,7 @@ protected Authentication createNewAuthentication(Authentication currentAuth, Str @Override public boolean userExists(String username) { - List users = getJdbcTemplate().queryForList(this.userExistsSql, new 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 + "'", @@ -314,13 +314,13 @@ public boolean userExists(String username) { @Override public List findAllGroups() { - return getJdbcTemplate().queryForList(this.findAllGroupsSql, String.class); + return getJdbcTemplate().queryForList(this.findAllGroupsQuery, String.class); } @Override public List findUsersInGroup(String groupName) { Assert.hasText(groupName, "groupName should have text"); - return getJdbcTemplate().queryForList(this.findUsersInGroupSql, new String[] { groupName }, String.class); + return getJdbcTemplate().queryForList(this.findUsersInGroupQuery, new String[] { groupName }, String.class); } @Override @@ -329,11 +329,11 @@ public void createGroup(final String groupName, final List aut Assert.notNull(authorities, "authorities cannot be null"); this.logger.debug("Creating new group '" + groupName + "' with authorities " + AuthorityUtils.authorityListToSet(authorities)); - getJdbcTemplate().update(this.insertGroupSql, groupName); + getJdbcTemplate().update(this.insertGroupQuery, groupName); int groupId = findGroupId(groupName); for (GrantedAuthority a : authorities) { String authority = a.getAuthority(); - getJdbcTemplate().update(this.insertGroupAuthoritySql, (ps) -> { + getJdbcTemplate().update(this.insertGroupAuthorityQuery, (ps) -> { ps.setInt(1, groupId); ps.setString(2, authority); }); @@ -346,9 +346,9 @@ public void deleteGroup(String groupName) { Assert.hasText(groupName, "groupName should have text"); int id = findGroupId(groupName); PreparedStatementSetter groupIdPSS = (ps) -> ps.setInt(1, id); - getJdbcTemplate().update(this.deleteGroupMembersSql, groupIdPSS); - getJdbcTemplate().update(this.deleteGroupAuthoritiesSql, groupIdPSS); - getJdbcTemplate().update(this.deleteGroupSql, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupMembersQuery, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupAuthoritiesQuery, groupIdPSS); + getJdbcTemplate().update(this.deleteGroupQuery, groupIdPSS); } @Override @@ -356,7 +356,7 @@ 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.renameGroupSql, newName, oldName); + getJdbcTemplate().update(this.renameGroupQuery, newName, oldName); } @Override @@ -365,7 +365,7 @@ public void addUserToGroup(final String username, final String groupName) { Assert.hasText(username, "username should have text"); Assert.hasText(groupName, "groupName should have text"); int id = findGroupId(groupName); - getJdbcTemplate().update(this.insertGroupMemberSql, (ps) -> { + getJdbcTemplate().update(this.insertGroupMemberQuery, (ps) -> { ps.setInt(1, id); ps.setString(2, username); }); @@ -378,7 +378,7 @@ public void removeUserFromGroup(final String username, final String groupName) { Assert.hasText(username, "username should have text"); Assert.hasText(groupName, "groupName should have text"); int id = findGroupId(groupName); - getJdbcTemplate().update(this.deleteGroupMemberSql, (ps) -> { + getJdbcTemplate().update(this.deleteGroupMemberQuery, (ps) -> { ps.setInt(1, id); ps.setString(2, username); }); @@ -389,7 +389,7 @@ public void removeUserFromGroup(final String username, final String groupName) { public List findGroupAuthorities(String groupName) { this.logger.debug("Loading authorities for group '" + groupName + "'"); Assert.hasText(groupName, "groupName should have text"); - return getJdbcTemplate().query(this.groupAuthoritiesSql, new String[] { groupName }, + return getJdbcTemplate().query(this.groupAuthoritiesQuery, new String[] { groupName }, this::mapToGrantedAuthority); } @@ -404,7 +404,7 @@ public void removeGroupAuthority(String groupName, final GrantedAuthority author Assert.hasText(groupName, "groupName should have text"); Assert.notNull(authority, "authority cannot be null"); int id = findGroupId(groupName); - getJdbcTemplate().update(this.deleteGroupAuthoritySql, (ps) -> { + getJdbcTemplate().update(this.deleteGroupAuthorityQuery, (ps) -> { ps.setInt(1, id); ps.setString(2, authority.getAuthority()); }); @@ -416,14 +416,14 @@ public void addGroupAuthority(final String groupName, final GrantedAuthority aut Assert.hasText(groupName, "groupName should have text"); Assert.notNull(authority, "authority cannot be null"); int id = findGroupId(groupName); - getJdbcTemplate().update(this.insertGroupAuthoritySql, (ps) -> { + getJdbcTemplate().update(this.insertGroupAuthorityQuery, (ps) -> { ps.setInt(1, id); ps.setString(2, authority.getAuthority()); }); } private int findGroupId(String group) { - return getJdbcTemplate().queryForObject(this.findGroupIdSql, Integer.class, group); + return getJdbcTemplate().queryForObject(this.findGroupIdQuery, Integer.class, group); } /** @@ -441,104 +441,104 @@ public void setAuthenticationManager(AuthenticationManager authenticationManager this.authenticationManager = authenticationManager; } - public void setCreateUserSql(String createUserSql) { - Assert.hasText(createUserSql, "createUserSql should have text"); - this.createUserSql = createUserSql; + public void setCreateUserQuery(String createUserQuery) { + Assert.hasText(createUserQuery, "createUserQuery should have text"); + this.createUserQuery = createUserQuery; } - public void setDeleteUserSql(String deleteUserSql) { - Assert.hasText(deleteUserSql, "deleteUserSql should have text"); - this.deleteUserSql = deleteUserSql; + public void setDeleteUserQuery(String deleteUserQuery) { + Assert.hasText(deleteUserQuery, "deleteUserQuery should have text"); + this.deleteUserQuery = deleteUserQuery; } - public void setUpdateUserSql(String updateUserSql) { - Assert.hasText(updateUserSql, "updateUserSql should have text"); - this.updateUserSql = updateUserSql; + public void setUpdateUserQuery(String updateUserQuery) { + Assert.hasText(updateUserQuery, "updateUserQuery should have text"); + this.updateUserQuery = updateUserQuery; } - public void setCreateAuthoritySql(String createAuthoritySql) { - Assert.hasText(createAuthoritySql, "createAuthoritySql should have text"); - this.createAuthoritySql = createAuthoritySql; + public void setCreateAuthorityQuery(String createAuthorityQuery) { + Assert.hasText(createAuthorityQuery, "createAuthorityQuery should have text"); + this.createAuthorityQuery = createAuthorityQuery; } - public void setDeleteUserAuthoritiesSql(String deleteUserAuthoritiesSql) { - Assert.hasText(deleteUserAuthoritiesSql, "deleteUserAuthoritiesSql should have text"); - this.deleteUserAuthoritiesSql = deleteUserAuthoritiesSql; + public void setDeleteUserAuthoritiesQuery(String deleteUserAuthoritiesQuery) { + Assert.hasText(deleteUserAuthoritiesQuery, "deleteUserAuthoritiesQuery should have text"); + this.deleteUserAuthoritiesQuery = deleteUserAuthoritiesQuery; } - public void setUserExistsSql(String userExistsSql) { - Assert.hasText(userExistsSql, "userExistsSql should have text"); - this.userExistsSql = userExistsSql; + public void setUserExistsQuery(String userExistsQuery) { + Assert.hasText(userExistsQuery, "userExistsQuery should have text"); + this.userExistsQuery = userExistsQuery; } - public void setChangePasswordSql(String changePasswordSql) { - Assert.hasText(changePasswordSql, "changePasswordSql should have text"); - this.changePasswordSql = changePasswordSql; + public void setChangePasswordQuery(String changePasswordQuery) { + Assert.hasText(changePasswordQuery, "changePasswordQuery should have text"); + this.changePasswordQuery = changePasswordQuery; } - public void setFindAllGroupsSql(String findAllGroupsSql) { - Assert.hasText(findAllGroupsSql, "findAllGroupsSql should have text"); - this.findAllGroupsSql = findAllGroupsSql; + public void setFindAllGroupsQuery(String findAllGroupsQuery) { + Assert.hasText(findAllGroupsQuery, "findAllGroupsQuery should have text"); + this.findAllGroupsQuery = findAllGroupsQuery; } - public void setFindUsersInGroupSql(String findUsersInGroupSql) { - Assert.hasText(findUsersInGroupSql, "findUsersInGroupSql should have text"); - this.findUsersInGroupSql = findUsersInGroupSql; + public void setFindUsersInGroupQuery(String findUsersInGroupQuery) { + Assert.hasText(findUsersInGroupQuery, "findUsersInGroupQuery should have text"); + this.findUsersInGroupQuery = findUsersInGroupQuery; } - public void setInsertGroupSql(String insertGroupSql) { - Assert.hasText(insertGroupSql, "insertGroupSql should have text"); - this.insertGroupSql = insertGroupSql; + public void setInsertGroupQuery(String insertGroupQuery) { + Assert.hasText(insertGroupQuery, "insertGroupQuery should have text"); + this.insertGroupQuery = insertGroupQuery; } - public void setFindGroupIdSql(String findGroupIdSql) { - Assert.hasText(findGroupIdSql, "findGroupIdSql should have text"); - this.findGroupIdSql = findGroupIdSql; + public void setFindGroupIdQuery(String findGroupIdQuery) { + Assert.hasText(findGroupIdQuery, "findGroupIdQuery should have text"); + this.findGroupIdQuery = findGroupIdQuery; } - public void setInsertGroupAuthoritySql(String insertGroupAuthoritySql) { - Assert.hasText(insertGroupAuthoritySql, "insertGroupAuthoritySql should have text"); - this.insertGroupAuthoritySql = insertGroupAuthoritySql; + public void setInsertGroupAuthorityQuery(String insertGroupAuthorityQuery) { + Assert.hasText(insertGroupAuthorityQuery, "insertGroupAuthorityQuery should have text"); + this.insertGroupAuthorityQuery = insertGroupAuthorityQuery; } - public void setDeleteGroupSql(String deleteGroupSql) { - Assert.hasText(deleteGroupSql, "deleteGroupSql should have text"); - this.deleteGroupSql = deleteGroupSql; + public void setDeleteGroupQuery(String deleteGroupQuery) { + Assert.hasText(deleteGroupQuery, "deleteGroupQuery should have text"); + this.deleteGroupQuery = deleteGroupQuery; } - public void setDeleteGroupAuthoritiesSql(String deleteGroupAuthoritiesSql) { - Assert.hasText(deleteGroupAuthoritiesSql, "deleteGroupAuthoritiesSql should have text"); - this.deleteGroupAuthoritiesSql = deleteGroupAuthoritiesSql; + public void setDeleteGroupAuthoritiesQuery(String deleteGroupAuthoritiesQuery) { + Assert.hasText(deleteGroupAuthoritiesQuery, "deleteGroupAuthoritiesQuery should have text"); + this.deleteGroupAuthoritiesQuery = deleteGroupAuthoritiesQuery; } - public void setDeleteGroupMembersSql(String deleteGroupMembersSql) { - Assert.hasText(deleteGroupMembersSql, "deleteGroupMembersSql should have text"); - this.deleteGroupMembersSql = deleteGroupMembersSql; + public void setDeleteGroupMembersQuery(String deleteGroupMembersQuery) { + Assert.hasText(deleteGroupMembersQuery, "deleteGroupMembersQuery should have text"); + this.deleteGroupMembersQuery = deleteGroupMembersQuery; } - public void setRenameGroupSql(String renameGroupSql) { - Assert.hasText(renameGroupSql, "renameGroupSql should have text"); - this.renameGroupSql = renameGroupSql; + public void setRenameGroupQuery(String renameGroupQuery) { + Assert.hasText(renameGroupQuery, "renameGroupQuery should have text"); + this.renameGroupQuery = renameGroupQuery; } - public void setInsertGroupMemberSql(String insertGroupMemberSql) { - Assert.hasText(insertGroupMemberSql, "insertGroupMemberSql should have text"); - this.insertGroupMemberSql = insertGroupMemberSql; + public void setInsertGroupMemberQuery(String insertGroupMemberQuery) { + Assert.hasText(insertGroupMemberQuery, "insertGroupMemberQuery should have text"); + this.insertGroupMemberQuery = insertGroupMemberQuery; } - public void setDeleteGroupMemberSql(String deleteGroupMemberSql) { - Assert.hasText(deleteGroupMemberSql, "deleteGroupMemberSql should have text"); - this.deleteGroupMemberSql = deleteGroupMemberSql; + public void setDeleteGroupMemberQuery(String deleteGroupMemberQuery) { + Assert.hasText(deleteGroupMemberQuery, "deleteGroupMemberQuery should have text"); + this.deleteGroupMemberQuery = deleteGroupMemberQuery; } - public void setGroupAuthoritiesSql(String groupAuthoritiesSql) { - Assert.hasText(groupAuthoritiesSql, "groupAuthoritiesSql should have text"); - this.groupAuthoritiesSql = groupAuthoritiesSql; + public void setGroupAuthoritiesQuery(String groupAuthoritiesQuery) { + Assert.hasText(groupAuthoritiesQuery, "groupAuthoritiesQuery should have text"); + this.groupAuthoritiesQuery = groupAuthoritiesQuery; } - public void setDeleteGroupAuthoritySql(String deleteGroupAuthoritySql) { - Assert.hasText(deleteGroupAuthoritySql, "deleteGroupAuthoritySql should have text"); - this.deleteGroupAuthoritySql = deleteGroupAuthoritySql; + public void setDeleteGroupAuthorityQuery(String deleteGroupAuthorityQuery) { + Assert.hasText(deleteGroupAuthorityQuery, "deleteGroupAuthorityQuery should have text"); + this.deleteGroupAuthorityQuery = deleteGroupAuthorityQuery; } /** @@ -568,7 +568,7 @@ private void validateAuthorities(Collection authorit @Override public UserDetails updatePassword(UserDetails user, String newPassword) { this.logger.debug("Updating password for user '" + user.getUsername() + "'"); - getJdbcTemplate().update(this.changePasswordSql, newPassword, 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/JdbcUserPasswordDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java index 3835bb08d99..d89dcffae93 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -47,7 +47,6 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; import static org.mockito.BDDMockito.*; -import static org.mockito.Mockito.*; /** * Tests for {@link JdbcUserPasswordDetailsManager} @@ -88,13 +87,13 @@ public void initializeManagerAndCreateTables() { this.cache = new JdbcUserPasswordDetailsManagerTests.MockUserCache(); this.manager.setUserCache(this.cache); this.manager.setDataSource(dataSource); - this.manager.setCreateUserSql(JdbcUserPasswordDetailsManager.DEF_CREATE_USER_SQL); - this.manager.setUpdateUserSql(JdbcUserPasswordDetailsManager.DEF_UPDATE_USER_SQL); - this.manager.setUserExistsSql(JdbcUserPasswordDetailsManager.DEF_USER_EXISTS_SQL); - this.manager.setCreateAuthoritySql(JdbcUserPasswordDetailsManager.DEF_INSERT_AUTHORITY_SQL); - this.manager.setDeleteUserAuthoritiesSql(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_AUTHORITIES_SQL); - this.manager.setDeleteUserSql(JdbcUserPasswordDetailsManager.DEF_DELETE_USER_SQL); - this.manager.setChangePasswordSql(JdbcUserPasswordDetailsManager.DEF_CHANGE_PASSWORD_SQL); + 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," @@ -122,9 +121,9 @@ private void setUpAccLockingColumns() { 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.setCreateUserSql( + this.manager.setCreateUserQuery( "insert into users (username, password, enabled, acc_locked, acc_expired, creds_expired) values (?,?,?,?,?,?)"); - this.manager.setUpdateUserSql( + this.manager.setUpdateUserQuery( "update users set password = ?, enabled = ?, acc_locked=?, acc_expired=?, creds_expired=? where username = ?"); } diff --git a/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc index b526cf9222a..5969c2d63b3 100644 --- a/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc +++ b/docs/modules/ROOT/pages/migration-7/jdbcusers.adoc @@ -34,7 +34,7 @@ is no DSL-based feature that allows configuration of the new `+JdbcUserPasswordD === Bean-based configuration -The new user details implementation is a drop-in replacement for the old implementation: +The new user details implementation is mostly a drop-in replacement for the old implementation: .... @Bean @@ -43,7 +43,10 @@ UserDetailsManager users(DataSource dataSource) { } .... -There is an additional configuration parameter available for changing the password of a user. This query +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 From 45ef25c47b1da9f780844f4a3c8eeb7bd9f3fed4 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Sat, 20 Jan 2024 10:15:12 +0100 Subject: [PATCH 09/13] gh-7750: make manager class final --- .../security/provisioning/JdbcUserPasswordDetailsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java index 30d3b83e556..f6dcc3c85d3 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -71,7 +71,7 @@ * @author Geir Hedemark * @since 6.3 */ -public class JdbcUserPasswordDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager, UserDetailsPasswordService { +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 (?,?,?)"; From 113d6ace7faf60380c846ccf9102b3e21803f1d1 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Sat, 20 Jan 2024 10:26:53 +0100 Subject: [PATCH 10/13] gh-7750: eradicate mentions of JdbcUserDetailsManager from documentation --- .../servlet/authentication/passwords/jdbc.adoc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 From 67e8ee661be75d2670bbdc1a36e3448466971843 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Sat, 20 Jan 2024 11:22:02 +0100 Subject: [PATCH 11/13] gh-7750: make checkstyle happy --- .../configuration/AuthenticationConfiguration.java | 1 - .../JdbcUserPasswordDetailsManagerConfigurer.java | 2 +- .../web/configuration/HttpSecurityConfiguration.java | 1 - .../JdbcUserPasswordDetailsManagerBeanDefinitionParser.java | 4 +++- .../client/oidc/authentication/OidcIdTokenDecoderFactory.java | 2 +- .../authentication/ReactiveOidcIdTokenDecoderFactory.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) 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 4032d5f992d..0e2b7450d53 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 @@ -44,7 +44,6 @@ import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; -import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration; import org.springframework.security.core.Authentication; 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 index eda181b5dc5..83f5a137bdf 100644 --- 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 @@ -137,7 +137,7 @@ public JdbcUserPasswordDetailsManagerConfigurer authoritiesByUsernameQuery(St * @return The {@link JdbcUserPasswordDetailsManagerConfigurer} used for additional * customizations */ -public JdbcUserPasswordDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { + public JdbcUserPasswordDetailsManagerConfigurer groupAuthoritiesByUsernameQuery(String query) { JdbcUserPasswordDetailsManager userDetailsService = getUserDetailsService(); userDetailsService.setEnableGroups(true); userDetailsService.setGroupAuthoritiesByUsernameQuery(query); 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 acba62b46f2..f804d129bfe 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 @@ -35,7 +35,6 @@ import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.authentication.configurers.provisioning.InMemoryUserDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserDetailsManagerConfigurer; -import org.springframework.security.config.annotation.authentication.configurers.provisioning.JdbcUserPasswordDetailsManagerConfigurer; import org.springframework.security.config.annotation.authentication.configurers.userdetails.DaoAuthenticationConfigurer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; 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 index c18a4bd629f..0c957a99291 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -13,13 +13,15 @@ * 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; -import org.w3c.dom.Element; /** * @author Luke Taylor 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())) { From 6bef316980b5851c479a9acb67282ddfe7ce7595 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Sat, 20 Jan 2024 16:26:14 +0100 Subject: [PATCH 12/13] gh-7750: make checkstyle happy --- ...asswordDetailsManagerBeanDefinitionParserTests.java | 2 +- .../JdbcUserPasswordDetailsManagerTests.java | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) 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 index a765551baf9..5cdef62e618 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -170,7 +170,7 @@ public void rolePrefixIsUsedWhenSet() { } private void setContext(String context) { - System.out.println("Context: "+ context); + System.out.println("Context: " + context); this.appContext = new InMemoryXmlApplicationContext(context); } diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java index d89dcffae93..747b5ba1226 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.security.provisioning; import java.util.Collections; @@ -44,9 +45,12 @@ import org.springframework.security.core.userdetails.UserCache; import org.springframework.security.core.userdetails.UserDetails; -import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.BDDMockito.*; +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} From 2c917f67034455f86cf8731eaf21006e48482e19 Mon Sep 17 00:00:00 2001 From: Geir Hedemark Date: Sun, 21 Jan 2024 09:51:14 +0100 Subject: [PATCH 13/13] gh-7550: gradle format --- .../builders/AuthenticationManagerBuilder.java | 5 +++-- .../AuthenticationConfiguration.java | 5 +++-- ...JdbcUserPasswordDetailsManagerConfigurer.java | 12 +++++++----- .../configuration/HttpSecurityConfiguration.java | 5 +++-- ...sswordDetailsManagerBeanDefinitionParser.java | 8 +++++--- .../JdbcUserServiceBeanDefinitionParser.java | 7 ++++--- ...dDetailsManagerBeanDefinitionParserTests.java | 3 ++- .../security/config/doc/XsdDocumentedTests.java | 5 +++-- .../provisioning/JdbcUserDetailsManager.java | 6 +++--- .../JdbcUserPasswordDetailsManager.java | 16 +++++++++------- .../JdbcUserDetailsManagerTests.java | 5 +++-- .../JdbcUserPasswordDetailsManagerTests.java | 14 +++++++------- .../OidcIdTokenDecoderFactoryTests.java | 9 ++++----- .../ReactiveOidcIdTokenDecoderFactoryTests.java | 10 ++++------ 14 files changed, 60 insertions(+), 50 deletions(-) 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 c9c5d394f66..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 @@ -155,8 +155,9 @@ public InMemoryUserDetailsManagerConfigurer inMemo * @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 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 { 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 0e2b7450d53..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 @@ -289,8 +289,9 @@ public InMemoryUserDetailsManagerConfigurer inMemo } /** - * @deprecated Use JdbcUserPasswordDetailsManager instead, as this keeps the password up to date. - * Please consult the migration documentation as database changes might be necessary. + * @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.") 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 index 83f5a137bdf..abfbf3e5460 100644 --- 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 @@ -33,9 +33,9 @@ /** * 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. + * 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 @@ -95,7 +95,8 @@ public JdbcUserPasswordDetailsManagerConfigurer usersByUsernameQuery(String q * * 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. + * @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 */ @@ -159,7 +160,8 @@ public JdbcUserPasswordDetailsManagerConfigurer rolePrefix(String rolePrefix) /** * Defines the {@link UserCache} to use * @param userCache the {@link UserCache} to use - * @return the {@link JdbcUserPasswordDetailsManagerConfigurer} for further customizations + * @return the {@link JdbcUserPasswordDetailsManagerConfigurer} for further + * customizations */ public JdbcUserPasswordDetailsManagerConfigurer userCache(UserCache userCache) { getUserDetailsService().setUserCache(userCache); 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 f804d129bfe..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 @@ -185,8 +185,9 @@ public InMemoryUserDetailsManagerConfigurer inMemo } /** - * @deprecated Use jdbcAuthenticationWithPasswordManagement instead, as this keeps the password up to date. - * Please consult the migration documentation as database changes might be necessary. + * @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.") 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 index 0c957a99291..f7ebe2c372a 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParser.java @@ -27,7 +27,9 @@ * @author Luke Taylor * @author Geir Hedemark */ -public final class JdbcUserPasswordDetailsManagerBeanDefinitionParser extends AbstractUserDetailsServiceBeanDefinitionParser { +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"; @@ -48,8 +50,8 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit } else { parserContext.getReaderContext() - .error(ATT_DATA_SOURCE + " is required for " + Elements.JDBC_USER_PASSWORD_SERVICE, - parserContext.extractSource(element)); + .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); 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 378176ae33f..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,10 +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 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") +@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/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java index 5cdef62e618..058674ce466 100644 --- a/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/authentication/JdbcUserPasswordDetailsManagerBeanDefinitionParserTests.java @@ -72,7 +72,8 @@ public void beanNameIsCorrect() { @Test public void validUsernameIsFound() { setContext("" + DATA_SOURCE); - JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext.getBean(BeanIds.USER_DETAILS_SERVICE); + JdbcUserPasswordDetailsManager mgr = (JdbcUserPasswordDetailsManager) this.appContext + .getBean(BeanIds.USER_DETAILS_SERVICE); assertThat(mgr.loadUserByUsername("rod")).isNotNull(); } 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 5cee01228fd..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 @@ -151,8 +151,9 @@ public void sizeWhenReadingFilesystemThenIsCorrectNumberOfSchemaFiles() throws I .list((dir, name) -> name.endsWith(".xsd")); // @formatter:on 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)) + 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/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java index c2a07fcd76e..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,9 +62,9 @@ * * @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 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 { diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java index f6dcc3c85d3..3f1e4c0432d 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManager.java @@ -53,9 +53,8 @@ * 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. + * 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, @@ -65,13 +64,15 @@ * 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 + * 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 final class JdbcUserPasswordDetailsManager extends JdbcDaoImpl + implements UserDetailsManager, GroupManager, UserDetailsPasswordService { public static final String DEF_CREATE_USER_QUERY = "insert into users (username, password, enabled) values (?,?,?)"; @@ -118,7 +119,7 @@ public final class JdbcUserPasswordDetailsManager extends JdbcDaoImpl implements protected final Log logger = LogFactory.getLog(getClass()); private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder - .getContextHolderStrategy(); + .getContextHolderStrategy(); private String createUserQuery = DEF_CREATE_USER_QUERY; @@ -279,7 +280,7 @@ public void changePassword(String oldPassword, String newPassword) throws Authen if (this.authenticationManager != null) { this.logger.debug(LogMessage.format("Reauthenticating user '%s' for password change request.", username)); this.authenticationManager - .authenticate(UsernamePasswordAuthenticationToken.unauthenticated(username, oldPassword)); + .authenticate(UsernamePasswordAuthenticationToken.unauthenticated(username, oldPassword)); } else { this.logger.debug("No authentication manager set. Password won't be re-checked."); @@ -572,4 +573,5 @@ public UserDetails updatePassword(UserDetails user, String newPassword) { 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 b14e381601d..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,8 +56,9 @@ * 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 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.") diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java index 747b5ba1226..5fd4a2ba096 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserPasswordDetailsManagerTests.java @@ -103,8 +103,8 @@ public void initializeManagerAndCreateTables() { 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))"); + .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); } @@ -195,7 +195,7 @@ public void userExistsReturnsTrueForExistingUsername() { @Test public void changePasswordFailsForUnauthenticatedUser() { assertThatExceptionOfType(AccessDeniedException.class) - .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); + .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); } @Test @@ -246,7 +246,7 @@ public void changePasswordFailsIfReAuthenticationFails() { given(am.authenticate(any(Authentication.class))).willThrow(new BadCredentialsException("")); this.manager.setAuthenticationManager(am); assertThatExceptionOfType(BadCredentialsException.class) - .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); + .isThrownBy(() -> this.manager.changePassword("password", "newPassword")); // Check password hasn't changed. UserDetails newJoe = this.manager.loadUserByUsername("joe"); assertThat(newJoe.getPassword()).isEqualTo("password"); @@ -298,7 +298,7 @@ public void deleteGroupRemovesData() { 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(); + .isZero(); } @Test @@ -311,13 +311,13 @@ public void addingGroupUserSetsCorrectData() { public void removeUserFromGroupDeletesGroupMemberRow() { this.manager.removeUserFromGroup("jerry", "GROUP_1"); assertThat(this.template.queryForList("select group_id from group_members where username = 'jerry'")) - .hasSize(1); + .hasSize(1); } @Test public void findGroupAuthoritiesReturnsCorrectAuthorities() { assertThat(AuthorityUtils.createAuthorityList("ROLE_A")) - .isEqualTo(this.manager.findGroupAuthorities("GROUP_0")); + .isEqualTo(this.manager.findGroupAuthorities("GROUP_0")); } @Test 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)); } + }