From c77475cda6afed8018258f38569dcced6d3f7ba4 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 9 Jan 2008 18:06:41 +0000 Subject: [PATCH] SEC-272: Added groups support to JdbcDaoImpl. --- .../userdetails/jdbc/JdbcDaoImpl.java | 122 +++++++++++++++--- .../security/PopulatedDatabase.java | 5 + .../userdetails/jdbc/JdbcDaoImplTests.java | 30 +++-- 3 files changed, 133 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcDaoImpl.java b/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcDaoImpl.java index 089075f663..2224e2db01 100644 --- a/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcDaoImpl.java +++ b/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcDaoImpl.java @@ -32,54 +32,85 @@ import org.springframework.dao.DataAccessException; import org.springframework.jdbc.core.SqlParameter; import org.springframework.jdbc.core.support.JdbcDaoSupport; import org.springframework.jdbc.object.MappingSqlQuery; +import org.springframework.util.Assert; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; import java.util.List; +import java.util.ArrayList; +import java.util.Set; +import java.util.HashSet; import javax.sql.DataSource; /** - *

Retrieves user details (username, password, enabled flag, and authorities) from a JDBC location.

- *

A default database structure is assumed, (see {@link #DEF_USERS_BY_USERNAME_QUERY} and {@link + * Retrieves user details (username, password, enabled flag, and authorities) from a JDBC location. + *

+ * A default database structure is assumed, (see {@link #DEF_USERS_BY_USERNAME_QUERY} and {@link * #DEF_AUTHORITIES_BY_USERNAME_QUERY}, which most users of this class will need to override, if using an existing * scheme. This may be done by setting the default query strings used. If this does not provide enough flexibility, * another strategy would be to subclass this class and override the {@link MappingSqlQuery} instances used, via the - * {@link #initMappingSqlQueries()} extension point.

- *

In order to minimise backward compatibility issues, this DAO does not recognise the expiration of user + * {@link #initMappingSqlQueries()} extension point. + *

+ * In order to minimise backward compatibility issues, this DAO does not recognise the expiration of user * accounts or the expiration of user credentials. However, it does recognise and honour the user enabled/disabled - * column.

+ * column. + *

+ * Support for group-based authorities can be enabled by setting the enableGroups property to true + * (you may also then wish to set enableAuthorities to false to disable loading of authorities + * directly). With this approach, authorities are allocated to groups and a user's authorities are determined based + * on the groups they are a member of. The net result is the same (a UserDetails containing a set of + * GrantedAuthoritys is loaded), but the different persistence strategy may be more suitable for the + * administration of some applications. + * * * @author Ben Alex * @author colin sampaleanu + * @author Luke Taylor * @version $Id$ */ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { //~ Static fields/initializers ===================================================================================== public static final String DEF_USERS_BY_USERNAME_QUERY = - "SELECT username,password,enabled FROM users WHERE username = ?"; + "SELECT username,password,enabled " + + "FROM users " + + "WHERE username = ?"; public static final String DEF_AUTHORITIES_BY_USERNAME_QUERY = - "SELECT username,authority FROM authorities WHERE username = ?"; + "SELECT username,authority " + + "FROM authorities " + + "WHERE username = ?"; + public static final String DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY = + "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"; //~ Instance fields ================================================================================================ protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); protected MappingSqlQuery authoritiesByUsernameMapping; + protected MappingSqlQuery groupAuthoritiesByUsernameMapping; protected MappingSqlQuery usersByUsernameMapping; + private String authoritiesByUsernameQuery; - private String rolePrefix = ""; + private String groupAuthoritiesByUsernameQuery; private String usersByUsernameQuery; + private String rolePrefix = ""; private boolean usernameBasedPrimaryKey = true; + private boolean enableAuthorities = true; + private boolean enableGroups; //~ Constructors =================================================================================================== public JdbcDaoImpl() { usersByUsernameQuery = DEF_USERS_BY_USERNAME_QUERY; authoritiesByUsernameQuery = DEF_AUTHORITIES_BY_USERNAME_QUERY; + groupAuthoritiesByUsernameQuery = DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY; } //~ Methods ======================================================================================================== @@ -107,6 +138,7 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { } protected void initDao() throws ApplicationContextException { + Assert.isTrue(enableAuthorities || enableGroups, "Use of either authorities or groups must be enabled"); initMappingSqlQueries(); } @@ -116,14 +148,14 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { protected void initMappingSqlQueries() { this.usersByUsernameMapping = new UsersByUsernameMapping(getDataSource()); this.authoritiesByUsernameMapping = new AuthoritiesByUsernameMapping(getDataSource()); + this.groupAuthoritiesByUsernameMapping = new GroupAuthoritiesByUsernameMapping(getDataSource()); } public boolean isUsernameBasedPrimaryKey() { return usernameBasedPrimaryKey; } - public UserDetails loadUserByUsername(String username) - throws UsernameNotFoundException, DataAccessException { + public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException { List users = usersByUsernameMapping.execute(username); if (users.size() == 0) { @@ -133,7 +165,17 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { UserDetails user = (UserDetails) users.get(0); // contains no GrantedAuthority[] - List dbAuths = authoritiesByUsernameMapping.execute(user.getUsername()); + Set dbAuthsSet = new HashSet(); + + if (enableAuthorities) { + dbAuthsSet.addAll(authoritiesByUsernameMapping.execute(user.getUsername())); + } + + if (enableGroups) { + dbAuthsSet.addAll(groupAuthoritiesByUsernameMapping.execute(user.getUsername())); + } + + List dbAuths = new ArrayList(dbAuthsSet); addCustomAuthorities(user.getUsername(), dbAuths); @@ -166,10 +208,22 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { authoritiesByUsernameQuery = queryString; } + /** + * Allows the default query string used to retrieve group authorities based on username to be overriden, if + * default table or column names need to be changed. The default query is {@link + * #DEF_GROUP_AUTHORITIES_BY_USERNAME_QUERY}; when modifying this query, ensure that all returned columns are mapped + * back to the same column names as in the default query. + * + * @param queryString The query string to set + */ + public void setGroupAuthoritiesByUsernameQuery(String queryString) { + groupAuthoritiesByUsernameQuery = queryString; + } + /** * Allows a default role prefix to be specified. If this is set to a non-empty value, then it is * automatically prepended to any roles read in from the db. This may for example be used to add the - * ROLE_ prefix expected to exist in role names (by default) by some other Spring Security + * ROLE_ prefix expected to exist in role names (by default) by some other Spring Security * classes, in the case that the prefix is not already present in the db. * * @param rolePrefix the new prefix @@ -206,6 +260,29 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { this.usersByUsernameQuery = usersByUsernameQueryString; } + protected boolean getEnableAuthorities() { + return enableAuthorities; + } + + /** + * Enables loading of authorities (roles) from the authorities table. Defaults to true + */ + public void setEnableAuthorities(boolean enableAuthorities) { + this.enableAuthorities = enableAuthorities; + } + + protected boolean getEnableGroups() { + return enableGroups; + } + + /** + * Enables support for group authorities. Defaults to false + * @param enableGroups + */ + public void setEnableGroups(boolean enableGroups) { + this.enableGroups = enableGroups; + } + //~ Inner Classes ================================================================================================== /** @@ -218,8 +295,7 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { compile(); } - protected Object mapRow(ResultSet rs, int rownum) - throws SQLException { + protected Object mapRow(ResultSet rs, int rownum) throws SQLException { String roleName = rolePrefix + rs.getString(2); GrantedAuthorityImpl authority = new GrantedAuthorityImpl(roleName); @@ -227,6 +303,21 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { } } + protected class GroupAuthoritiesByUsernameMapping extends MappingSqlQuery { + protected GroupAuthoritiesByUsernameMapping(DataSource ds) { + super(ds, groupAuthoritiesByUsernameQuery); + declareParameter(new SqlParameter(Types.VARCHAR)); + compile(); + } + + protected Object mapRow(ResultSet rs, int rownum) throws SQLException { + String roleName = rolePrefix + rs.getString(3); + GrantedAuthorityImpl authority = new GrantedAuthorityImpl(roleName); + + return authority; + } + } + /** * Query object to look up a user. */ @@ -237,8 +328,7 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { compile(); } - protected Object mapRow(ResultSet rs, int rownum) - throws SQLException { + protected Object mapRow(ResultSet rs, int rownum) throws SQLException { String username = rs.getString(1); String password = rs.getString(2); boolean enabled = rs.getBoolean(3); diff --git a/core/src/test/java/org/springframework/security/PopulatedDatabase.java b/core/src/test/java/org/springframework/security/PopulatedDatabase.java index 5bcf84b4c8..db5dccc3ce 100644 --- a/core/src/test/java/org/springframework/security/PopulatedDatabase.java +++ b/core/src/test/java/org/springframework/security/PopulatedDatabase.java @@ -113,6 +113,8 @@ public class PopulatedDatabase { template.execute("INSERT INTO GROUPS VALUES (0, 'GROUP_ZERO')"); template.execute("INSERT INTO GROUPS VALUES (1, 'GROUP_ONE')"); template.execute("INSERT INTO GROUPS VALUES (2, 'GROUP_TWO')"); + // Group 3 isn't used + template.execute("INSERT INTO GROUPS VALUES (3, 'GROUP_THREE')"); template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (0, 'ROLE_A')"); template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (1, 'ROLE_B')"); @@ -120,6 +122,9 @@ public class PopulatedDatabase { template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (2, 'ROLE_A')"); template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (2, 'ROLE_B')"); template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (2, 'ROLE_C')"); + template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (3, 'ROLE_D')"); + template.execute("INSERT INTO GROUP_AUTHORITIES VALUES (3, 'ROLE_E')"); + template.execute("INSERT INTO GROUP_MEMBERS VALUES (0, 'jerry', 0)"); template.execute("INSERT INTO GROUP_MEMBERS VALUES (1, 'jerry', 1)"); diff --git a/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcDaoImplTests.java b/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcDaoImplTests.java index b3e3db26e1..50e4eeb73a 100644 --- a/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcDaoImplTests.java +++ b/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcDaoImplTests.java @@ -79,16 +79,14 @@ public class JdbcDaoImplTests extends TestCase { assertTrue(authorities.contains("ROLE_SUPERVISOR")); } - public void testCheckDaoOnlyReturnsGrantedAuthoritiesGrantedToUser() - throws Exception { + public void testCheckDaoOnlyReturnsGrantedAuthoritiesGrantedToUser() throws Exception { JdbcDaoImpl dao = makePopulatedJdbcDao(); UserDetails user = dao.loadUserByUsername("scott"); assertEquals("ROLE_TELLER", user.getAuthorities()[0].getAuthority()); assertEquals(1, user.getAuthorities().length); } - public void testCheckDaoReturnsCorrectDisabledProperty() - throws Exception { + public void testCheckDaoReturnsCorrectDisabledProperty() throws Exception { JdbcDaoImpl dao = makePopulatedJdbcDao(); UserDetails user = dao.loadUserByUsername("peter"); assertTrue(!user.isEnabled()); @@ -103,8 +101,7 @@ public class JdbcDaoImplTests extends TestCase { assertEquals("SELECT USERS FROM FOO", dao.getUsersByUsernameQuery()); } - public void testLookupFailsIfUserHasNoGrantedAuthorities() - throws Exception { + public void testLookupFailsIfUserHasNoGrantedAuthorities() throws Exception { JdbcDaoImpl dao = makePopulatedJdbcDao(); try { @@ -147,6 +144,24 @@ public class JdbcDaoImplTests extends TestCase { assertTrue(authorities.contains("ARBITRARY_PREFIX_ROLE_SUPERVISOR")); } + public void testGroupAuthoritiesAreLoadedCorrectly() throws Exception { + JdbcDaoImpl dao = makePopulatedJdbcDao(); + dao.setEnableAuthorities(false); + dao.setEnableGroups(true); + + UserDetails jerry = dao.loadUserByUsername("jerry"); + assertEquals(3, jerry.getAuthorities().length); + } + + public void testDuplicateGroupAuthoritiesAreRemoved() throws Exception { + JdbcDaoImpl dao = makePopulatedJdbcDao(); + dao.setEnableAuthorities(false); + dao.setEnableGroups(true); + // Tom has roles A, B, C and B, C duplicates + UserDetails tom = dao.loadUserByUsername("tom"); + assertEquals(3, tom.getAuthorities().length); + } + public void testStartupFailsIfDataSourceNotSet() throws Exception { JdbcDaoImpl dao = new JdbcDaoImpl(); @@ -173,8 +188,7 @@ public class JdbcDaoImplTests extends TestCase { //~ Inner Classes ================================================================================================== private class MockMappingSqlQuery extends MappingSqlQuery { - protected Object mapRow(ResultSet arg0, int arg1) - throws SQLException { + protected Object mapRow(ResultSet arg0, int arg1) throws SQLException { return null; } }