From ff38f9886de489d55b1e7fd731c336a8f73cd53e Mon Sep 17 00:00:00 2001 From: dae won Date: Mon, 10 Feb 2025 15:39:10 +0900 Subject: [PATCH 1/5] Make mapToUser and mapToGrantedAuthority protected in JdbcUserDetailsManager - Closes gh-16540 Signed-off-by: dae won --- .../security/provisioning/JdbcUserDetailsManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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..b43764094cb 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -181,7 +181,7 @@ protected List loadUsersByUsername(String username) { return getJdbcTemplate().query(getUsersByUsernameQuery(), this::mapToUser, username); } - private UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { + protected UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { String userName = rs.getString(1); String password = rs.getString(2); boolean enabled = rs.getBoolean(3); @@ -390,7 +390,7 @@ public List findGroupAuthorities(String groupName) { this::mapToGrantedAuthority); } - private GrantedAuthority mapToGrantedAuthority(ResultSet rs, int rowNum) throws SQLException { + protected GrantedAuthority mapToGrantedAuthority(ResultSet rs, int rowNum) throws SQLException { String roleName = getRolePrefix() + rs.getString(3); return new SimpleGrantedAuthority(roleName); } From 3a0cb6283f861889ec3f2314001802ca9a16f83f Mon Sep 17 00:00:00 2001 From: dae won Date: Fri, 14 Feb 2025 09:58:19 +0900 Subject: [PATCH 2/5] Add userDetailsMapper as a class member Signed-off-by: dae won --- .../security/provisioning/JdbcUserDetailsManager.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 b43764094cb..a4f97ca0f3c 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -30,6 +30,7 @@ import org.springframework.core.log.LogMessage; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.jdbc.core.PreparedStatementSetter; +import org.springframework.jdbc.core.RowMapper; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -156,6 +157,8 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private UserCache userCache = new NullUserCache(); + private RowMapper userDetailsMapper = this::mapToUser; + public JdbcUserDetailsManager() { } @@ -163,6 +166,11 @@ public JdbcUserDetailsManager(DataSource dataSource) { setDataSource(dataSource); } + public void setUserDetailsMapper(RowMapper mapper) { + Assert.notNull(mapper, "userDetailsMapper cannot be null"); + this.userDetailsMapper = mapper; + } + @Override protected void initDao() throws ApplicationContextException { if (this.authenticationManager == null) { @@ -178,7 +186,7 @@ protected void initDao() throws ApplicationContextException { */ @Override protected List loadUsersByUsername(String username) { - return getJdbcTemplate().query(getUsersByUsernameQuery(), this::mapToUser, username); + return getJdbcTemplate().query(getUsersByUsernameQuery(), userDetailsMapper, username); } protected UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { From ab9524ac1f0eb21e541205a938f0cb091b5f1dae Mon Sep 17 00:00:00 2001 From: dae won Date: Fri, 14 Feb 2025 09:58:56 +0900 Subject: [PATCH 3/5] Refactor mapToUser method visibility to private Signed-off-by: dae won --- .../security/provisioning/JdbcUserDetailsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a4f97ca0f3c..b22ffa01fe0 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -189,7 +189,7 @@ protected List loadUsersByUsername(String username) { return getJdbcTemplate().query(getUsersByUsernameQuery(), userDetailsMapper, username); } - protected UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { + private UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { String userName = rs.getString(1); String password = rs.getString(2); boolean enabled = rs.getBoolean(3); From a427643a7c872e3010d680db7a472974d463ead0 Mon Sep 17 00:00:00 2001 From: dae won Date: Sat, 15 Feb 2025 21:09:29 +0900 Subject: [PATCH 4/5] Add unit tests for setUserDetailsMapper method Signed-off-by: dae won --- .../provisioning/JdbcUserDetailsManager.java | 11 +++++++++ .../JdbcUserDetailsManagerTests.java | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+) 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 b22ffa01fe0..b081bfef6c4 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -166,6 +166,17 @@ public JdbcUserDetailsManager(DataSource dataSource) { setDataSource(dataSource); } + /** + * Sets the {@code RowMapper} to convert each user result row into a + * {@link UserDetails} object. + * + * The default mapper expects columns with names like 'username', 'password', + * 'enabled', etc., and maps them directly to the corresponding UserDetails + * properties. + * @param mapper the {@code RowMapper} to use for mapping rows in the database, must + * not be null + * @since 6.5 + */ public void setUserDetailsMapper(RowMapper mapper) { Assert.notNull(mapper, "userDetailsMapper cannot be null"); this.userDetailsMapper = mapper; 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..a13c8e388cc 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java @@ -16,6 +16,7 @@ package org.springframework.security.provisioning; +import java.sql.SQLException; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -28,6 +29,7 @@ import org.junit.jupiter.api.Test; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowMapper; import org.springframework.security.PopulatedDatabase; import org.springframework.security.TestDataSource; import org.springframework.security.access.AccessDeniedException; @@ -48,14 +50,17 @@ 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.ArgumentMatchers.anyInt; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * Tests for {@link JdbcUserDetailsManager} * * @author Luke Taylor + * @author dae won */ public class JdbcUserDetailsManagerTests { @@ -365,6 +370,24 @@ public void createNewAuthenticationUsesNullPasswordToKeepPassordsSave() { assertThat(updatedAuth.getCredentials()).isNull(); } + @Test + public void setUserDetailsMapperWithNullMapperThrowsException() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> this.manager.setUserDetailsMapper(null)) + .withMessage("userDetailsMapper cannot be null"); + } + + @Test + public void setUserDetailsMapperWithMockMapper() throws SQLException { + RowMapper mockMapper = mock(RowMapper.class); + when(mockMapper.mapRow(any(), anyInt())).thenReturn(joe); + this.manager.setUserDetailsMapper(mockMapper); + insertJoe(); + UserDetails newJoe = this.manager.loadUserByUsername("joe"); + assertThat(joe).isEqualTo(newJoe); + verify(mockMapper).mapRow(any(), anyInt()); + } + private Authentication authenticateJoe() { UsernamePasswordAuthenticationToken auth = UsernamePasswordAuthenticationToken.authenticated("joe", "password", joe.getAuthorities()); From f06c9797cdd2ca11850a736565387833535e00da Mon Sep 17 00:00:00 2001 From: dae won Date: Sat, 15 Feb 2025 23:59:36 +0900 Subject: [PATCH 5/5] Add grantedAuthorityMapper as a class member - Add unit tests for setGrantedAuthorityMapper method Signed-off-by: dae won --- .../provisioning/JdbcUserDetailsManager.java | 23 +++++++++++++-- .../JdbcUserDetailsManagerTests.java | 29 +++++++++++++++---- 2 files changed, 43 insertions(+), 9 deletions(-) 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 b081bfef6c4..291f15f3223 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -159,6 +159,8 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa private RowMapper userDetailsMapper = this::mapToUser; + private RowMapper grantedAuthorityMapper = this::mapToGrantedAuthority; + public JdbcUserDetailsManager() { } @@ -182,6 +184,21 @@ public void setUserDetailsMapper(RowMapper mapper) { this.userDetailsMapper = mapper; } + /** + * Sets the {@code RowMapper} to convert each authority result row into a + * {@link GrantedAuthority} object. + * + * The default mapper expects columns with names like 'authority' or 'role', and maps + * them directly to SimpleGrantedAuthority objects. + * @param mapper the {@code RowMapper} to use for mapping rows in the database to + * GrantedAuthority objects, must not be null + * @since 6.5 + */ + public void setGrantedAuthorityMapper(RowMapper mapper) { + Assert.notNull(mapper, "grantedAuthorityMapper cannot be null"); + this.grantedAuthorityMapper = mapper; + } + @Override protected void initDao() throws ApplicationContextException { if (this.authenticationManager == null) { @@ -197,7 +214,7 @@ protected void initDao() throws ApplicationContextException { */ @Override protected List loadUsersByUsername(String username) { - return getJdbcTemplate().query(getUsersByUsernameQuery(), userDetailsMapper, username); + return getJdbcTemplate().query(getUsersByUsernameQuery(), this.userDetailsMapper, username); } private UserDetails mapToUser(ResultSet rs, int rowNum) throws SQLException { @@ -406,10 +423,10 @@ 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); + this.grantedAuthorityMapper); } - protected GrantedAuthority mapToGrantedAuthority(ResultSet rs, int rowNum) throws SQLException { + private GrantedAuthority mapToGrantedAuthority(ResultSet rs, int rowNum) throws SQLException { String roleName = getRolePrefix() + rs.getString(3); return new SimpleGrantedAuthority(roleName); } 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 a13c8e388cc..65b1d4b1c45 100644 --- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java @@ -52,9 +52,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.BDDMockito.mock; +import static org.mockito.BDDMockito.verify; /** * Tests for {@link JdbcUserDetailsManager} @@ -373,14 +372,14 @@ public void createNewAuthenticationUsesNullPasswordToKeepPassordsSave() { @Test public void setUserDetailsMapperWithNullMapperThrowsException() { assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> this.manager.setUserDetailsMapper(null)) - .withMessage("userDetailsMapper cannot be null"); + .isThrownBy(() -> this.manager.setUserDetailsMapper(null)) + .withMessage("userDetailsMapper cannot be null"); } @Test public void setUserDetailsMapperWithMockMapper() throws SQLException { RowMapper mockMapper = mock(RowMapper.class); - when(mockMapper.mapRow(any(), anyInt())).thenReturn(joe); + given(mockMapper.mapRow(any(), anyInt())).willReturn(joe); this.manager.setUserDetailsMapper(mockMapper); insertJoe(); UserDetails newJoe = this.manager.loadUserByUsername("joe"); @@ -388,6 +387,24 @@ public void setUserDetailsMapperWithMockMapper() throws SQLException { verify(mockMapper).mapRow(any(), anyInt()); } + @Test + public void setGrantedAuthorityMapperWithNullMapperThrowsException() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> this.manager.setGrantedAuthorityMapper(null)) + .withMessage("grantedAuthorityMapper cannot be null"); + } + + @Test + public void setGrantedAuthorityMapperWithMockMapper() throws SQLException { + RowMapper mockMapper = mock(RowMapper.class); + GrantedAuthority mockAuthority = new SimpleGrantedAuthority("ROLE_MOCK"); + given(mockMapper.mapRow(any(), anyInt())).willReturn(mockAuthority); + this.manager.setGrantedAuthorityMapper(mockMapper); + List authGroup = this.manager.findGroupAuthorities("GROUP_0"); + assertThat(authGroup.get(0)).isEqualTo(mockAuthority); + verify(mockMapper).mapRow(any(), anyInt()); + } + private Authentication authenticateJoe() { UsernamePasswordAuthenticationToken auth = UsernamePasswordAuthenticationToken.authenticated("joe", "password", joe.getAuthorities());