From 4123d96cd515a703d190a0f5322432ba13bb1151 Mon Sep 17 00:00:00 2001 From: Panayiotis Vlissidis Date: Fri, 7 Dec 2018 19:44:34 +0200 Subject: [PATCH] JdbcUserDetailsManager handles extra UserDetails attributes Check ResutSetMetaData to see if extra columns are present in order to also handle the UserDetails attributes: accountNonExpired, accountNonLocked and credentialsNonExpired. Fixes gh-4399 --- .../core/userdetails/jdbc/JdbcDaoImpl.java | 3 +- .../provisioning/JdbcUserDetailsManager.java | 57 ++++++++++++++++++- .../JdbcUserDetailsManagerTests.java | 46 ++++++++++++++- 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java b/core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java index e07468a955..8e577bce96 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/jdbc/JdbcDaoImpl.java @@ -295,7 +295,8 @@ public class JdbcDaoImpl extends JdbcDaoSupport } return new User(returnUsername, userFromUserQuery.getPassword(), - userFromUserQuery.isEnabled(), true, true, true, combinedAuthorities); + userFromUserQuery.isEnabled(), userFromUserQuery.isAccountNonExpired(), + userFromUserQuery.isCredentialsNonExpired(), userFromUserQuery.isAccountNonLocked(), combinedAuthorities); } /** 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 0a4ab85551..3909880fbd 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-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -24,6 +24,7 @@ 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.userdetails.User; import org.springframework.security.core.userdetails.UserCache; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.cache.NullUserCache; @@ -145,15 +146,51 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa // ~ UserDetailsManager implementation // ============================================================================== + /** + * Executes the SQL usersByUsernameQuery and returns a list of UserDetails + * objects. There should normally only be one matching user. + */ + protected List loadUsersByUsername(String username) { + return getJdbcTemplate().query(getUsersByUsernameQuery(), new String[]{username}, + (rs, rowNum) -> { + + 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(createUserSql, new PreparedStatementSetter() { + @Override public void setValues(PreparedStatement ps) throws SQLException { 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()) { @@ -163,11 +200,25 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa public void updateUser(final UserDetails user) { validateUserDetails(user); + getJdbcTemplate().update(updateUserSql, new PreparedStatementSetter() { + @Override public void setValues(PreparedStatement ps) throws SQLException { ps.setString(1, user.getPassword()); ps.setBoolean(2, user.isEnabled()); - ps.setString(3, user.getUsername()); + + 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()); + } + } }); 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 72db6bbdad..4748d8b853 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-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -106,6 +106,19 @@ public class JdbcUserDetailsManagerTests { SecurityContextHolder.clearContext(); } + private void setUpAccLockingColumns() { + template.execute("alter table users add column acc_locked boolean default false not null"); + template.execute("alter table users add column acc_expired boolean default false not null"); + template.execute("alter table users add column creds_expired boolean default false not null"); + + manager.setUsersByUsernameQuery( + "select username,password,enabled, acc_locked, acc_expired, creds_expired from users where username = ?"); + manager.setCreateUserSql( + "insert into users (username, password, enabled, acc_locked, acc_expired, creds_expired) values (?,?,?,?,?,?)"); + manager.setUpdateUserSql( + "update users set password = ?, enabled = ?, acc_locked=?, acc_expired=?, creds_expired=? where username = ?"); + } + @Test public void createUserInsertsCorrectData() { manager.createUser(joe); @@ -115,6 +128,19 @@ public class JdbcUserDetailsManagerTests { assertThat(joe2).isEqualTo(joe); } + @Test + public void createUserInsertsCorrectDataWithLocking() { + setUpAccLockingColumns(); + + UserDetails user = new User("joe", "pass", true, false, true, false, + AuthorityUtils.createAuthorityList("A", "B")); + manager.createUser(user); + + UserDetails user2 = manager.loadUserByUsername(user.getUsername()); + + assertThat(user2).isEqualToComparingFieldByField(user); + } + @Test public void deleteUserRemovesUserDataAndAuthoritiesAndClearsCache() { insertJoe(); @@ -139,6 +165,24 @@ public class JdbcUserDetailsManagerTests { assertThat(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")); + + manager.updateUser(newJoe); + + UserDetails joe = manager.loadUserByUsername(newJoe.getUsername()); + + assertThat(joe).isEqualToComparingFieldByField(newJoe); + assertThat(cache.getUserMap().containsKey(newJoe.getUsername())).isFalse(); + } + + @Test public void userExistsReturnsFalseForNonExistentUsername() { assertThat(manager.userExists("joe")).isFalse();