Ticket #269: Make separation of concerns in MySQLUser more strict.

This commit is contained in:
Michael Jumper
2013-02-26 19:02:51 -08:00
parent ffe49b4347
commit d06dda94d7
4 changed files with 41 additions and 66 deletions

View File

@@ -35,7 +35,6 @@
* ***** END LICENSE BLOCK ***** */ * ***** END LICENSE BLOCK ***** */
package net.sourceforge.guacamole.net.auth.mysql; package net.sourceforge.guacamole.net.auth.mysql;
import com.google.inject.Inject;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
@@ -43,9 +42,6 @@ import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.net.auth.AbstractUser; import net.sourceforge.guacamole.net.auth.AbstractUser;
import net.sourceforge.guacamole.net.auth.User; import net.sourceforge.guacamole.net.auth.User;
import net.sourceforge.guacamole.net.auth.mysql.model.UserWithBLOBs; import net.sourceforge.guacamole.net.auth.mysql.model.UserWithBLOBs;
import net.sourceforge.guacamole.net.auth.mysql.service.PasswordEncryptionService;
import net.sourceforge.guacamole.net.auth.mysql.service.PermissionCheckService;
import net.sourceforge.guacamole.net.auth.mysql.service.SaltService;
import net.sourceforge.guacamole.net.auth.permission.Permission; import net.sourceforge.guacamole.net.auth.permission.Permission;
/** /**
@@ -59,24 +55,6 @@ public class MySQLUser extends AbstractUser {
*/ */
private Integer userID; private Integer userID;
/**
* Service for encrypting passwords.
*/
@Inject
private PasswordEncryptionService passwordService;
/**
* Service for generating random salts.
*/
@Inject
private SaltService saltService;
/**
* Service for checking permissions.
*/
@Inject
private PermissionCheckService permissionCheckService;
/** /**
* The set of current permissions a user has. * The set of current permissions a user has.
*/ */
@@ -104,7 +82,7 @@ public class MySQLUser extends AbstractUser {
* @param name The name to assign to this MySQLUser. * @param name The name to assign to this MySQLUser.
*/ */
public void init(String name) { public void init(String name) {
setUsername(name); init(null, name, null, Collections.EMPTY_SET);
} }
/** /**
@@ -116,23 +94,25 @@ public class MySQLUser extends AbstractUser {
* data in the given object. * data in the given object.
*/ */
public void init(User user) throws GuacamoleException { public void init(User user) throws GuacamoleException {
setUsername(user.getUsername()); init(null, user.getUsername(), user.getPassword(), user.getPermissions());
setPassword(user.getPassword());
permissions.addAll(user.getPermissions());
} }
/** /**
* Initializes a new MySQLUser initialized from the given data from the * Initializes a new MySQLUser initialized from the given data from the
* database. * database.
* *
* @param user The user object, as retrieved from the database. * @param userID The ID of the user in the database, if any.
* @param username The username of this user.
* @param password The password to assign to this user.
* @param permissions The permissions to assign to this user, as
* retrieved from the database.
*/ */
public void init(UserWithBLOBs user) { public void init(Integer userID, String username, String password,
this.userID = user.getUser_id(); Set<Permission> permissions) {
setUsername(user.getUsername()); this.userID = userID;
setUsername(username);
permissions.addAll( setPassword(password);
permissionCheckService.getAllPermissions(user.getUser_id())); permissions.addAll(permissions);
} }
/** /**
@@ -211,32 +191,4 @@ public class MySQLUser extends AbstractUser {
removedPermissions.add(permission); removedPermissions.add(permission);
} }
/**
* Converts this MySQLUser into an object that can be inserted/updated
* into the database. Beware that this object does not have associated
* permissions. The permissions of this MySQLUser must be dealt with
* separately.
*
* @return A new UserWithBLOBs containing all associated data of this
* MySQLUser.
*/
public UserWithBLOBs toUserWithBLOBs() {
// Create new user
UserWithBLOBs user = new UserWithBLOBs();
user.setUser_id(userID);
user.setUsername(getUsername());
// Set password if specified
if (getPassword() != null) {
byte[] salt = saltService.generateSalt();
user.setPassword_salt(salt);
user.setPassword_hash(
passwordService.createPasswordHash(getPassword(), salt));
}
return user;
}
} }

View File

@@ -694,9 +694,22 @@ public class UserDirectory implements Directory<String, net.sourceforge.guacamol
permissionCheckService.verifyUserUpdateAccess(this.user_id, permissionCheckService.verifyUserUpdateAccess(this.user_id,
object.getUsername()); object.getUsername());
// Update the user in the database // Build database user from non-database structure
MySQLUser mySQLUser = (MySQLUser) object; MySQLUser mySQLUser = (MySQLUser) object;
userDAO.updateByPrimaryKeySelective(mySQLUser.toUserWithBLOBs()); UserWithBLOBs user = new UserWithBLOBs();
user.setUser_id(mySQLUser.getUserID());
user.setUsername(mySQLUser.getUsername());
// Set password if specified
if (mySQLUser.getPassword() != null) {
byte[] salt = saltService.generateSalt();
user.setPassword_salt(salt);
user.setPassword_hash(
passwordService.createPasswordHash(mySQLUser.getPassword(), salt));
}
// Update the user in the database
userDAO.updateByPrimaryKeySelective(user);
// Update permissions in database // Update permissions in database
createPermissions(mySQLUser.getUserID(), mySQLUser.getNewPermissions()); createPermissions(mySQLUser.getUserID(), mySQLUser.getNewPermissions());

View File

@@ -394,7 +394,7 @@ public class PermissionCheckService {
Set<MySQLUser> affectedUsers = new HashSet<MySQLUser>(); Set<MySQLUser> affectedUsers = new HashSet<MySQLUser>();
for(UserWithBLOBs affectedUser : userDBOjects) { for(UserWithBLOBs affectedUser : userDBOjects) {
MySQLUser mySQLUser = mySQLUserProvider.get(); MySQLUser mySQLUser = mySQLUserProvider.get();
mySQLUser.init(affectedUser); mySQLUser.init(affectedUser.getUsername());
affectedUsers.add(mySQLUser); affectedUsers.add(mySQLUser);
} }

View File

@@ -38,7 +38,6 @@ package net.sourceforge.guacamole.net.auth.mysql.service;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.List; import java.util.List;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.net.auth.Connection; import net.sourceforge.guacamole.net.auth.Connection;
@@ -83,6 +82,12 @@ public class ProviderService {
@Inject @Inject
Provider<MySQLGuacamoleSocket> mySQLGuacamoleSocketProvider; Provider<MySQLGuacamoleSocket> mySQLGuacamoleSocketProvider;
/**
* Service for checking permissions.
*/
@Inject
private PermissionCheckService permissionCheckService;
/** /**
* Create a new user based on the provided object. * Create a new user based on the provided object.
* @param user * @param user
@@ -134,7 +139,12 @@ public class ProviderService {
*/ */
public MySQLUser getExistingMySQLUser(UserWithBLOBs user) { public MySQLUser getExistingMySQLUser(UserWithBLOBs user) {
MySQLUser mySQLUser = mySQLUserProvider.get(); MySQLUser mySQLUser = mySQLUserProvider.get();
mySQLUser.init(user); mySQLUser.init(
user.getUser_id(),
user.getUsername(),
permissionCheckService.getAllPermissions(user.getUser_id())
);
return mySQLUser; return mySQLUser;
} }