diff --git a/core/src/main/java/org/springframework/security/userdetails/GroupManager.java b/core/src/main/java/org/springframework/security/userdetails/GroupManager.java new file mode 100644 index 0000000000..f7664ad292 --- /dev/null +++ b/core/src/main/java/org/springframework/security/userdetails/GroupManager.java @@ -0,0 +1,83 @@ +package org.springframework.security.userdetails; + +import org.springframework.security.GrantedAuthority; + +/** + * Allows management of groups of authorities and their members. + *

+ * Typically this will be used to supplement the functionality of a {@link UserDetailsManager} + * in situations where the organization of application granted authorities into groups is preferred over + * a straight mapping of users to roles. + *

+ * With this scenario, users are allocated to groups and take on the list of authorities which + * are assigned to the group, providing more flexible administration options. + * + * @author Luke Taylor + * @version $Id$ + */ +public interface GroupManager { + + /** + * Returns the names of all groups that this group manager controls. + */ + String[] findAllGroups(); + + /** + * Locates the users who are members of a group + * + * @param groupName the group whose members are required + * @return the usernames of the group members + */ + String[] findUsersInGroup(String groupName); + + /** + * Creates a new group with the specified list of authorities. + * + * @param groupName the name for the new group + * @param authorities the authorities which are to be allocated to this group. + */ + void createGroup(String groupName, GrantedAuthority[] authorities); + + /** + * Removes a group, including all members and authorities. + * + * @param groupName the group to remove. + */ + void deleteGroup(String groupName); + + /** + * Changes the name of a group without altering the assigned authorities or members. + */ + void renameGroup(String oldName, String newName); + + /** + * Makes a user a member of a particular group. + * + * @param username the user to be given membership. + * @param group the name of the group to which the user will be added. + */ + void addUserToGroup(String username, String group); + + /** + * Deletes a user's membership of a group. + * + * @param username the user + * @param groupName the group to remove them from + */ + void removeUserFromGroup(String username, String groupName); + + /** + * Obtains the list of authorities which are assigned to a group. + */ + GrantedAuthority[] findGroupAuthorities(String groupName); + + /** + * Assigns a new authority to a group. + */ + void addGroupAuthority(String groupName, GrantedAuthority authority); + + /** + * Deletes an authority from those assigned to a group + */ + void removeGroupAuthority(String groupName, GrantedAuthority authority); +} 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 2224e2db01..6f21f834a5 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 @@ -125,14 +125,6 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { */ protected void addCustomAuthorities(String username, List authorities) {} - public String getAuthoritiesByUsernameQuery() { - return authoritiesByUsernameQuery; - } - - public String getRolePrefix() { - return rolePrefix; - } - public String getUsersByUsernameQuery() { return usersByUsernameQuery; } @@ -151,10 +143,6 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { this.groupAuthoritiesByUsernameMapping = new GroupAuthoritiesByUsernameMapping(getDataSource()); } - public boolean isUsernameBasedPrimaryKey() { - return usernameBasedPrimaryKey; - } - public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException { List users = usersByUsernameMapping.execute(username); @@ -208,6 +196,10 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { authoritiesByUsernameQuery = queryString; } + protected String getAuthoritiesByUsernameQuery() { + return authoritiesByUsernameQuery; + } + /** * 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 @@ -232,6 +224,10 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { this.rolePrefix = rolePrefix; } + protected String getRolePrefix() { + return rolePrefix; + } + /** * If true (the default), indicates the {@link #getUsersByUsernameQuery()} returns a username * in response to a query. If false, indicates that a primary key is used instead. If set to @@ -246,6 +242,10 @@ public class JdbcDaoImpl extends JdbcDaoSupport implements UserDetailsService { this.usernameBasedPrimaryKey = usernameBasedPrimaryKey; } + protected boolean isUsernameBasedPrimaryKey() { + return usernameBasedPrimaryKey; + } + /** * Allows the default query string used to retrieve users based on username to be overriden, if default * table or column names need to be changed. The default query is {@link #DEF_USERS_BY_USERNAME_QUERY}; when diff --git a/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManager.java index eaf04a4a86..101ade8f1f 100644 --- a/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManager.java @@ -5,13 +5,15 @@ import org.springframework.security.Authentication; import org.springframework.security.AuthenticationException; import org.springframework.security.AuthenticationManager; import org.springframework.security.GrantedAuthority; +import org.springframework.security.GrantedAuthorityImpl; +import org.springframework.security.util.AuthorityUtils; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; import org.springframework.security.providers.dao.UserCache; import org.springframework.security.providers.dao.cache.NullUserCache; import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.UserDetailsManager; -import org.springframework.security.userdetails.GroupsManager; +import org.springframework.security.userdetails.GroupManager; import org.springframework.context.ApplicationContextException; import org.springframework.jdbc.core.SqlParameter; import org.springframework.jdbc.object.MappingSqlQuery; @@ -35,7 +37,7 @@ import java.util.List; * @version $Id$ * @since 2.0 */ -public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupsManager { +public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsManager, GroupManager { //~ Static fields/initializers ===================================================================================== // UserDetailsManager SQL @@ -54,7 +56,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa public static final String DEF_CHANGE_PASSWORD_SQL = "update users set password = ? where username = ?"; - // GroupsManager SQL + // GroupManager SQL public static final String DEF_FIND_GROUPS_SQL = "select group_name from groups"; public static final String DEF_FIND_USERS_IN_GROUP_SQL = @@ -77,6 +79,15 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa "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 = ?"; //~ Instance fields ================================================================================================ @@ -101,6 +112,9 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa 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; protected SqlUpdate insertUser; protected SqlUpdate deleteUser; @@ -120,6 +134,9 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa protected SqlUpdate deleteGroupAuthorities; protected SqlUpdate renameGroup; protected SqlUpdate insertGroupMember; + protected SqlUpdate deleteGroupMember; + protected SqlQuery groupAuthoritiesQuery; + protected SqlUpdate deleteGroupAuthority; private AuthenticationManager authenticationManager; @@ -129,7 +146,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa protected void initDao() throws ApplicationContextException { if (authenticationManager == null) { - logger.info("No authentication manager set. Reauthentication of users when changing passwords will" + + logger.info("No authentication manager set. Reauthentication of users when changing passwords will " + "not be performed."); } @@ -151,6 +168,9 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa deleteGroupMembers = new DeleteGroupMembers(getDataSource()); renameGroup = new RenameGroup(getDataSource()); insertGroupMember = new InsertGroupMember(getDataSource()); + deleteGroupMember = new DeleteGroupMember (getDataSource()); + groupAuthoritiesQuery = new GroupAuthoritiesByGroupNameMapping(getDataSource()); + deleteGroupAuthority = new DeleteGroupAuthority(getDataSource()); super.initDao(); } @@ -158,22 +178,24 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa //~ UserDetailsManager implementation ============================================================================== public void createUser(UserDetails user) { + validateUserDetails(user); insertUser.update(new Object[] {user.getUsername(), user.getPassword(), Boolean.valueOf(user.isEnabled())}); - - for (int i=0; i < user.getAuthorities().length; i++) { - insertAuthority.update(user.getUsername(), user.getAuthorities()[i].getAuthority()); - } + insertUserAuthorities(user); } public void updateUser(UserDetails user) { + validateUserDetails(user); updateUser.update(new Object[] {user.getPassword(), Boolean.valueOf(user.isEnabled()), user.getUsername()}); deleteUserAuthorities.update(user.getUsername()); + insertUserAuthorities(user); + userCache.removeUserFromCache(user.getUsername()); + } + + private void insertUserAuthorities(UserDetails user) { for (int i=0; i < user.getAuthorities().length; i++) { insertAuthority.update(user.getUsername(), user.getAuthorities()[i].getAuthority()); } - - userCache.removeUserFromCache(user.getUsername()); } public void deleteUser(String username) { @@ -233,28 +255,32 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa //~ GroupManager implementation ==================================================================================== - public List findAllGroups() { - return findAllGroupsQuery.execute(); + public String[] findAllGroups() { + return (String[]) findAllGroupsQuery.execute().toArray(new String[0]); } - public List findUsersInGroup(String groupName) { + public String[] findUsersInGroup(String groupName) { Assert.hasText(groupName); - return findUsersInGroupQuery.execute(groupName); + return (String[]) findUsersInGroupQuery.execute(groupName).toArray(new String[0]); } public void createGroup(String groupName, GrantedAuthority[] authorities) { Assert.hasText(groupName); Assert.notNull(authorities); + logger.debug("Creating new group '" + groupName + "' with authorities " + + AuthorityUtils.authorityArrayToSet(authorities)); + insertGroup.update(groupName); - Integer key = (Integer) findGroupIdQuery.findObject(groupName); + Integer id = (Integer) findGroupIdQuery.findObject(groupName); for (int i=0; i < authorities.length; i++) { - insertGroupAuthority.update( new Object[] {key, authorities[i].getAuthority()}); + insertGroupAuthority.update( new Object[] {id, authorities[i].getAuthority()}); } } public void deleteGroup(String groupName) { + logger.debug("Deleting group '" + groupName + "'"); Assert.hasText(groupName); int id = ((Integer) findGroupIdQuery.findObject(groupName)).intValue(); @@ -264,19 +290,58 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa } public void renameGroup(String oldName, String newName) { + logger.debug("Changing group name from '" + oldName + "' to '" + newName + "'"); Assert.hasText(oldName); Assert.hasText(newName); - - renameGroup.update(newName, oldName); + + renameGroup.update(newName, oldName); } public void addUserToGroup(String username, String groupName) { + logger.debug("Adding user '" + username + "' to group '" + groupName + "'"); Assert.hasText(username); Assert.hasText(groupName); - Integer key = (Integer) findGroupIdQuery.findObject(groupName); + Integer id = (Integer) findGroupIdQuery.findObject(groupName); - insertGroupMember.update(new Object[] {key, username}); + insertGroupMember.update(new Object[] {id, username}); + userCache.removeUserFromCache(username); + } + + public void removeUserFromGroup(String username, String groupName) { + logger.debug("Removing user '" + username + "' to group '" + groupName + "'"); + Assert.hasText(username); + Assert.hasText(groupName); + + Integer id = (Integer) findGroupIdQuery.findObject(groupName); + + deleteGroupMember.update(new Object[] {id, username}); + userCache.removeUserFromCache(username); + } + + public GrantedAuthority[] findGroupAuthorities(String groupName) { + logger.debug("Loading authorities for group '" + groupName + "'"); + Assert.hasText(groupName); + + return (GrantedAuthority[]) groupAuthoritiesQuery.execute(groupName).toArray(new GrantedAuthority[0]); + } + + public void removeGroupAuthority(String groupName, GrantedAuthority authority) { + logger.debug("Removing authority '" + authority + "' from group '" + groupName + "'"); + Assert.hasText(groupName); + Assert.notNull(authority); + + Integer id = (Integer) findGroupIdQuery.findObject(groupName); + deleteGroupAuthority.update(new Object[] {id, authority}); + } + + public void addGroupAuthority(String groupName, GrantedAuthority authority) { + logger.debug("Adding authority '" + authority + "' to group '" + groupName + "'"); + Assert.hasText(groupName); + Assert.notNull(authority); + + Integer id = (Integer) findGroupIdQuery.findObject(groupName); + insertGroupAuthority.update(new Object[] {id, authority.getAuthority()}); } public void setAuthenticationManager(AuthenticationManager authenticationManager) { @@ -333,6 +398,20 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa 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(GrantedAuthority[] authorities) { + Assert.notNull(authorities, "Authorities list must not be null"); + + for (int i=0; i < authorities.length; i++) { + Assert.notNull(authorities[i], "Authorities list contains a null entry"); + Assert.hasText(authorities[i].getAuthority(), "getAuthority() method must return a non-empty string"); + } + } + //~ Inner Classes ================================================================================================== protected class InsertUser extends SqlUpdate { @@ -484,7 +563,7 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa super(ds, renameGroupSql); declareParameter(new SqlParameter(Types.VARCHAR)); declareParameter(new SqlParameter(Types.VARCHAR)); - compile(); + compile(); } } @@ -497,4 +576,36 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa } } + private class DeleteGroupMember extends SqlUpdate { + public DeleteGroupMember(DataSource ds) { + super(ds, deleteGroupMemberSql); + declareParameter(new SqlParameter(Types.INTEGER)); + declareParameter(new SqlParameter(Types.VARCHAR)); + compile(); + } + } + + protected class GroupAuthoritiesByGroupNameMapping extends MappingSqlQuery { + protected GroupAuthoritiesByGroupNameMapping(DataSource ds) { + super(ds, groupAuthoritiesSql); + declareParameter(new SqlParameter(Types.VARCHAR)); + compile(); + } + + protected Object mapRow(ResultSet rs, int rownum) throws SQLException { + String roleName = getRolePrefix() + rs.getString(3); + GrantedAuthorityImpl authority = new GrantedAuthorityImpl(roleName); + + return authority; + } + } + + private class DeleteGroupAuthority extends SqlUpdate { + public DeleteGroupAuthority(DataSource ds) { + super(ds, deleteGroupAuthoritySql); + declareParameter(new SqlParameter(Types.INTEGER)); + declareParameter(new SqlParameter(Types.VARCHAR)); + compile(); + } + } } diff --git a/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManagerTests.java index 22a11dd59a..c5f5025be1 100644 --- a/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/userdetails/jdbc/JdbcUserDetailsManagerTests.java @@ -5,6 +5,8 @@ import org.springframework.security.Authentication; import org.springframework.security.BadCredentialsException; import org.springframework.security.MockAuthenticationManager; import org.springframework.security.PopulatedDatabase; +import org.springframework.security.GrantedAuthority; +import org.springframework.security.GrantedAuthorityImpl; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; import org.springframework.security.providers.dao.UserCache; @@ -25,6 +27,8 @@ import java.util.Map; import java.util.HashMap; import java.util.List; import java.util.Collections; +import java.util.ArrayList; +import java.util.Arrays; /** * Tests for {@link JdbcUserDetailsManager} @@ -187,7 +191,7 @@ public class JdbcUserDetailsManagerTests { @Test public void findAllGroupsReturnsExpectedGroupNames() { - List groups = manager.findAllGroups(); + List groups = new ArrayList(Arrays.asList(manager.findAllGroups())); assertEquals(4, groups.size()); Collections.sort(groups); @@ -199,11 +203,11 @@ public class JdbcUserDetailsManagerTests { @Test public void findGroupMembersReturnsCorrectData() { - List groupMembers = manager.findUsersInGroup("GROUP_0"); - assertEquals(1, groupMembers.size()); - assertEquals("jerry", groupMembers.get(0)); + String[] groupMembers = manager.findUsersInGroup("GROUP_0"); + assertEquals(1, groupMembers.length); + assertEquals("jerry", groupMembers[0]); groupMembers = manager.findUsersInGroup("GROUP_1"); - assertEquals(2, groupMembers.size()); + assertEquals(2, groupMembers.length); } @Test @@ -227,7 +231,7 @@ public class JdbcUserDetailsManagerTests { assertEquals(0, template.queryForList("select * from group_authorities").size()); assertEquals(0, template.queryForList("select * from group_members").size()); - assertEquals(0, template.queryForList("select id from groups").size()); + assertEquals(0, template.queryForList("select id from groups").size()); } @Test @@ -244,6 +248,38 @@ public class JdbcUserDetailsManagerTests { assertEquals(2, template.queryForList("select username from group_members where group_id = 0").size()); } + @Test + public void removeUserFromGroupDeletesGroupMemberRow() throws Exception { + manager.removeUserFromGroup("jerry", "GROUP_1"); + + assertEquals(1, template.queryForList("select group_id from group_members where username = 'jerry'").size()); + } + + @Test + public void findGroupAuthoritiesReturnsCorrectAuthorities() throws Exception { + GrantedAuthority[] authorities = manager.findGroupAuthorities("GROUP_0"); + + assertEquals("ROLE_A", authorities[0].getAuthority()); + } + + @Test + public void addGroupAuthorityInsertsCorrectGroupAuthorityRow() throws Exception { + GrantedAuthority auth = new GrantedAuthorityImpl("ROLE_X"); + manager.addGroupAuthority("GROUP_0", auth); + + template.queryForObject("select authority from group_authorities where authority = 'ROLE_X' and group_id = 0", String.class); + } + + @Test + public void deleteGroupAuthorityRemovesCorrectRows() throws Exception { + GrantedAuthority auth = new GrantedAuthorityImpl("ROLE_A"); + manager.removeGroupAuthority("GROUP_0", auth); + assertEquals(0, template.queryForList("select authority from group_authorities where group_id = 0").size()); + + manager.removeGroupAuthority("GROUP_2", auth); + assertEquals(2, template.queryForList("select authority from group_authorities where group_id = 2").size()); + } + private Authentication authenticateJoe() { UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken("joe","password", joe.getAuthorities()); @@ -261,7 +297,7 @@ public class JdbcUserDetailsManagerTests { } private class MockUserCache implements UserCache { - private Map cache = new HashMap(); + private Map cache = new HashMap(); public UserDetails getUserFromCache(String username) { return (User) cache.get(username);