Ticket #269: Clean up UserDirectory, fix JavaDoc.

extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/UserDirectory.java

@@ -1,3 +1,6 @@
+
+package net.sourceforge.guacamole.net.auth.mysql;
+
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
@@ -33,7 +36,6 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-package net.sourceforge.guacamole.net.auth.mysql;
 
 import com.google.common.base.Preconditions;
 import com.google.inject.Inject;
@@ -45,17 +47,18 @@ import java.util.Map;
 import java.util.Set;
 import net.sourceforge.guacamole.GuacamoleException;
 import net.sourceforge.guacamole.net.auth.Directory;
-import net.sourceforge.guacamole.net.auth.User;
 import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionMapper;
 import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionPermissionMapper;
 import net.sourceforge.guacamole.net.auth.mysql.dao.SystemPermissionMapper;
 import net.sourceforge.guacamole.net.auth.mysql.dao.UserMapper;
 import net.sourceforge.guacamole.net.auth.mysql.dao.UserPermissionMapper;
+import net.sourceforge.guacamole.net.auth.mysql.model.Connection;
 import net.sourceforge.guacamole.net.auth.mysql.model.ConnectionExample;
 import net.sourceforge.guacamole.net.auth.mysql.model.ConnectionPermissionExample;
 import net.sourceforge.guacamole.net.auth.mysql.model.ConnectionPermissionKey;
 import net.sourceforge.guacamole.net.auth.mysql.model.SystemPermissionExample;
 import net.sourceforge.guacamole.net.auth.mysql.model.SystemPermissionKey;
+import net.sourceforge.guacamole.net.auth.mysql.model.User;
 import net.sourceforge.guacamole.net.auth.mysql.model.UserExample;
 import net.sourceforge.guacamole.net.auth.mysql.model.UserPermissionExample;
 import net.sourceforge.guacamole.net.auth.mysql.model.UserPermissionKey;
@@ -73,7 +76,7 @@ import org.mybatis.guice.transactional.Transactional;
  * A MySQL based implementation of the User Directory.
  * @author James Muehlner
  */
-public class UserDirectory implements Directory<String, User> {
+public class UserDirectory implements Directory<String, net.sourceforge.guacamole.net.auth.User> {
 
     /**
      * The user who this user directory belongs to.
@@ -81,24 +84,46 @@ public class UserDirectory implements Directory<String, User> {
      */
     private MySQLUser user;
 
+    /**
+     * DAO for accessing users, which will be injected.
+     */
     @Inject
     UserMapper userDAO;
 
+    /**
+     * DAO for accessing connections, which will be injected.
+     */
     @Inject
     ConnectionMapper connectionDAO;
 
+    /**
+     * DAO for accessing user permissions, which will be injected.
+     */
     @Inject
     UserPermissionMapper userPermissionDAO;
 
+    /**
+     * DAO for accessing connection permissions, which will be injected.
+     */
     @Inject
     ConnectionPermissionMapper connectionPermissionDAO;
 
+    /**
+     * DAO for accessing system permissions, which will be injected.
+     */
     @Inject
     SystemPermissionMapper systemPermissionDAO;
 
+    /**
+     * Utility class for checking various permissions, which will be injected.
+     */
     @Inject
     PermissionCheckUtility permissionCheckUtility;
 
+    /**
+     * Utility class that provides convenient access to object creation and
+     * retrieval functions.
+     */
     @Inject
     ProviderUtility providerUtility;
 
@@ -112,7 +137,8 @@ public class UserDirectory implements Directory<String, User> {
 
     @Transactional
     @Override
-    public User get(String identifier) throws GuacamoleException {
+    public net.sourceforge.guacamole.net.auth.User get(String identifier)
+            throws GuacamoleException {
         permissionCheckUtility.verifyUserReadAccess(this.user.getUserID(), identifier);
         return providerUtility.getExistingMySQLUser(identifier);
     }
@@ -120,301 +146,410 @@ public class UserDirectory implements Directory<String, User> {
     @Transactional
     @Override
     public Set<String> getIdentifiers() throws GuacamoleException {
-        Set<String> userNameSet = new HashSet<String>();
+
+        // Get set of all readable users
         Set<MySQLUser> users = permissionCheckUtility.getReadableUsers(user.getUserID());
-        for(MySQLUser mySQLUser : users) {
+        
+        // Build set of usernames of readable users
+        Set<String> userNameSet = new HashSet<String>();
+        for (MySQLUser mySQLUser : users)
             userNameSet.add(mySQLUser.getUsername());
-        }
+
         return userNameSet;
     }
 
     @Override
     @Transactional
-    public void add(User object) throws GuacamoleException {
+    public void add(net.sourceforge.guacamole.net.auth.User object)
+            throws GuacamoleException {
+
+        // Verify current user has permission to create users
         permissionCheckUtility.verifyCreateUserPermission(this.user.getUserID());
         Preconditions.checkNotNull(object);
 
-        //create user in database
+        // Create user in database
         MySQLUser mySQLUser = providerUtility.getNewMySQLUser(object);
         userDAO.insert(mySQLUser.getUser());
 
-        //create permissions in database
+        // Create permissions of new user in database
         updatePermissions(mySQLUser);
 
-        //finally, give the current user full access to the newly created user.
+        // Give the current user full access to the newly created user.
         UserPermissionKey newUserPermission = new UserPermissionKey();
         newUserPermission.setUser_id(this.user.getUserID());
         newUserPermission.setAffected_user_id(mySQLUser.getUserID());
+
+        // READ permission on new user
         newUserPermission.setPermission(MySQLConstants.USER_READ);
         userPermissionDAO.insert(newUserPermission);
+
+        // UPDATE permission on new user
         newUserPermission.setPermission(MySQLConstants.USER_UPDATE);
         userPermissionDAO.insert(newUserPermission);
+
+        // DELETE permission on new user
         newUserPermission.setPermission(MySQLConstants.USER_DELETE);
         userPermissionDAO.insert(newUserPermission);
+
+        // ADMINISTER permission on new user
         newUserPermission.setPermission(MySQLConstants.USER_ADMINISTER);
         userPermissionDAO.insert(newUserPermission);
+
     }
 
     /**
      * Update all the permissions for a given user to be only those specified in the user object.
      * Delete any permissions not in the list, and create any in the list that do not exist
      * in the database.
-     * @param user
+     * 
-     * @throws GuacamoleException
+     * @param user The user whose permissions should be updated.
+     * @throws GuacamoleException If an error occurs while updating the
+     *                            permissions of the given user.
      */
     private void updatePermissions(MySQLUser user) throws GuacamoleException {
+
+        // Partition given permissions by permission type
         List<UserPermission> userPermissions = new ArrayList<UserPermission>();
         List<ConnectionPermission> connectionPermissions = new ArrayList<ConnectionPermission>();
         List<SystemPermission> systemPermissions = new ArrayList<SystemPermission>();
         
-        // Get the list of all the users and connections that the user performing the user save action has.
+        for (Permission permission : user.getPermissions()) {
-        // Need to make sure the user saving this user has permission to administrate all the objects in the permission list.
+
-        Set<Integer> administerableUsers = permissionCheckUtility.getAdministerableUserIDs(this.user.getUserID());
+            if (permission instanceof UserPermission)
-        Set<Integer> administerableConnections = permissionCheckUtility.getAdministerableConnectionIDs(this.user.getUserID());
+                userPermissions.add((UserPermission) permission);
+
+            else if (permission instanceof ConnectionPermission)
+                connectionPermissions.add((ConnectionPermission) permission);
+
+            else if (permission instanceof SystemPermission)
+                systemPermissions.add((SystemPermission) permission);
 
-        for(Permission permission : user.getPermissions()) {
-            if(permission instanceof UserPermission)
-                userPermissions.add((UserPermission)permission);
-            else if(permission instanceof ConnectionPermission)
-                connectionPermissions.add((ConnectionPermission)permission);
-            else if(permission instanceof SystemPermission)
-                systemPermissions.add((SystemPermission)permission);
         }
 
-        updateUserPermissions(userPermissions, user, administerableUsers);
+        // Update each type of permission appropriately
-        updateConnectionPermissions(connectionPermissions, user, administerableConnections);
+        updateUserPermissions(userPermissions, user);
+        updateConnectionPermissions(connectionPermissions, user);
         updateSystemPermissions(systemPermissions, user);
+        
     }
 
     /**
      * Update all the permissions having to do with users for a given user.
-     * @param permissions
+     * 
-     * @param user
+     * @param permissions The permissions the given user should have when
+     *                    this operation completes.
+     * @param user The user to change the permissions of.
+     * @throws GuacamoleException If permission to alter the access permissions
+     *                            of affected objects is denied.
      */
-    private void updateUserPermissions(Iterable<UserPermission> permissions, MySQLUser user, Set<Integer> administerableUsers) throws GuacamoleException {
+    private void updateUserPermissions(Iterable<UserPermission> permissions,
+            MySQLUser user)
+            throws GuacamoleException {
 
+        // Get set of administerable users
+        Set<Integer> administerableUsers =
+                permissionCheckUtility.getAdministerableUserIDs(this.user.getUserID());
+
+        // Get list of usernames for all given user permissions.
         List<String> usernames = new ArrayList<String>();
-        for(UserPermission permission : permissions) {
+        for (UserPermission permission : permissions)
             usernames.add(permission.getObjectIdentifier());
-        }
 
-        // find all the users by username
+        // Find all the users by username
         UserExample userExample = new UserExample();
         userExample.createCriteria().andUsernameIn(usernames);
-        List<net.sourceforge.guacamole.net.auth.mysql.model.User> dbUsers = userDAO.selectByExample(userExample);
+        List<User> dbUsers = userDAO.selectByExample(userExample);
         List<Integer> userIDs = new ArrayList<Integer>();
 
-        Map<String, net.sourceforge.guacamole.net.auth.mysql.model.User> dbUserMap = new HashMap<String, net.sourceforge.guacamole.net.auth.mysql.model.User>();
+        // Build map of found users, indexed by username
-        for(net.sourceforge.guacamole.net.auth.mysql.model.User dbUser : dbUsers) {
+        Map<String, User> dbUserMap = new HashMap<String, User>();
+        for (User dbUser : dbUsers) {
             dbUserMap.put(dbUser.getUsername(), dbUser);
             userIDs.add(dbUser.getUser_id());
         }
 
-        // find any user permissions that may already exist
+        // Find any user permissions that may already exist
         UserPermissionExample userPermissionExample = new UserPermissionExample();
         userPermissionExample.createCriteria().andAffected_user_idIn(userIDs);
-        List<UserPermissionKey> existingPermissions = userPermissionDAO.selectByExample(userPermissionExample);
+        List<UserPermissionKey> existingPermissions =
-        Set<Integer> existingUserIDs = new HashSet<Integer>();
+                userPermissionDAO.selectByExample(userPermissionExample);
-        for(UserPermissionKey userPermission : existingPermissions) {
-            existingUserIDs.add(userPermission.getAffected_user_id());
-        }
 
-        // delete any permissions that are not in the provided list
+        // Build list of currently-present permissions
+        Set<Integer> existingUserIDs = new HashSet<Integer>();
+        for (UserPermissionKey userPermission : existingPermissions)
+            existingUserIDs.add(userPermission.getAffected_user_id());
+
+        // Delete any permissions that are not in the provided list
         userPermissionExample.clear();
         userPermissionExample.createCriteria().andAffected_user_idNotIn(userIDs);
-        List<UserPermissionKey> permissionsToDelete = userPermissionDAO.selectByExample(userPermissionExample);
+        List<UserPermissionKey> permissionsToDelete =
+                userPermissionDAO.selectByExample(userPermissionExample);
 
-        // verify that the user actually has permission to administrate every one of these users
+        // Verify that the user actually has permission to administrate every one of these users
-        for(UserPermissionKey permissionToDelete : permissionsToDelete) {
+        for (UserPermissionKey permissionToDelete : permissionsToDelete) {
-            if(!administerableUsers.contains(permissionToDelete.getAffected_user_id()))
+            if (!administerableUsers.contains(permissionToDelete.getAffected_user_id()))
-                throw new GuacamolePermissionException("User '" + this.user.getUsername() + "' does not have permission to administrate user " + permissionToDelete.getAffected_user_id());
+                throw new GuacamolePermissionException(
+                      "User '" + this.user.getUsername()
+                    + "' does not have permission to administrate user "
+                    + permissionToDelete.getAffected_user_id());
         }
 
         userPermissionDAO.deleteByExample(userPermissionExample);
 
-        // finally, insert the new permissions
+        // Finally, insert the new permissions
-        for(UserPermission permission : permissions) {
+        for (UserPermission permission : permissions) {
-            net.sourceforge.guacamole.net.auth.mysql.model.User dbAffectedUser = dbUserMap.get(permission.getObjectIdentifier());
-            if(dbAffectedUser == null)
-                throw new GuacamoleException("User '" + permission.getObjectIdentifier() + "' not found.");
 
-            // the permission for this user already exists, we don't need to create it again
+            // Get user
-            if(existingUserIDs.contains(dbAffectedUser.getUser_id()))
+            User dbAffectedUser = dbUserMap.get(permission.getObjectIdentifier());
+            if (dbAffectedUser == null)
+                throw new GuacamoleException(
+                          "User '" + permission.getObjectIdentifier()
+                        + "' not found.");
+
+            // If the permission for this user already exists, we don't need to
+            // create it again
+            if (existingUserIDs.contains(dbAffectedUser.getUser_id()))
                 continue;
 
 
-            // verify that the user actually has permission to administrate every one of these users
+            // Verify that the user actually has permission to administrate
-            if(!administerableUsers.contains(dbAffectedUser.getUser_id()))
+            // every one of these users
-                throw new GuacamolePermissionException("User '" + this.user.getUsername() + "' does not have permission to administrate user " + dbAffectedUser.getUser_id());
+            if (!administerableUsers.contains(dbAffectedUser.getUser_id()))
+                throw new GuacamolePermissionException(
+                      "User '" + this.user.getUsername()
+                    + "' does not have permission to administrate user "
+                    + dbAffectedUser.getUser_id());
 
+            // Create new permission
             UserPermissionKey newPermission = new UserPermissionKey();
             newPermission.setAffected_user_id(dbAffectedUser.getUser_id());
             newPermission.setPermission(permission.getType().name());
             newPermission.setUser_id(user.getUserID());
             userPermissionDAO.insert(newPermission);
+
         }
     }
 
     /**
-     * Update all the permissions having to do with connections for a given user.
+     * Update all the permissions having to do with connections for a given
-     * @param permissions
+     * user.
-     * @param user
+     * 
+     * @param permissions The permissions the user should have after this
+     *                    operation completes.
+     * @param user The user to assign or remove permissions from.
+     * @throws GuacamoleException If permission to alter the access permissions
+     *                            of affected objects is deniedD
      */
-    private void updateConnectionPermissions(Iterable<ConnectionPermission> permissions, MySQLUser user, Set<Integer> administerableConnections) throws GuacamoleException {
+    private void updateConnectionPermissions(
+            Iterable<ConnectionPermission> permissions, MySQLUser user)
+            throws GuacamoleException {
 
-        List<String> connectionnames = new ArrayList<String>();
+        // Get adminsterable connection identifiers
-        for(ConnectionPermission permission : permissions) {
+        Set<Integer> administerableConnections =
-            connectionnames.add(permission.getObjectIdentifier());
+                permissionCheckUtility.getAdministerableConnectionIDs(this.user.getUserID());
-        }
 
-        // find all the connections by connectionname
+        // Build list of affected connection names from the permissions given
+        List<String> connectionNames = new ArrayList<String>();
+        for (ConnectionPermission permission : permissions)
+            connectionNames.add(permission.getObjectIdentifier());
+
+        // Find all the connections by connection name
         ConnectionExample connectionExample = new ConnectionExample();
-        connectionExample.createCriteria().andConnection_nameIn(connectionnames);
+        connectionExample.createCriteria().andConnection_nameIn(connectionNames);
-        List<net.sourceforge.guacamole.net.auth.mysql.model.Connection> dbConnections = connectionDAO.selectByExample(connectionExample);
+        List<Connection> dbConnections = connectionDAO.selectByExample(connectionExample);
         List<Integer> connectionIDs = new ArrayList<Integer>();
 
-        Map<String, net.sourceforge.guacamole.net.auth.mysql.model.Connection> dbConnectionMap = new HashMap<String, net.sourceforge.guacamole.net.auth.mysql.model.Connection>();
+        // Build map of found connections, indexed by name
-        for(net.sourceforge.guacamole.net.auth.mysql.model.Connection dbConnection : dbConnections) {
+        Map<String, Connection> dbConnectionMap = new HashMap<String, Connection>();
+        for (Connection dbConnection : dbConnections) {
             dbConnectionMap.put(dbConnection.getConnection_name(), dbConnection);
             connectionIDs.add(dbConnection.getConnection_id());
         }
 
-        // find any connection permissions that may already exist
+        // Find any connection permissions that may already exist
         ConnectionPermissionExample connectionPermissionExample = new ConnectionPermissionExample();
         connectionPermissionExample.createCriteria().andConnection_idIn(connectionIDs);
-        List<ConnectionPermissionKey> existingPermissions = connectionPermissionDAO.selectByExample(connectionPermissionExample);
+        List<ConnectionPermissionKey> existingPermissions =
+                connectionPermissionDAO.selectByExample(connectionPermissionExample);
         Set<Integer> existingConnectionIDs = new HashSet<Integer>();
-        for(ConnectionPermissionKey connectionPermission : existingPermissions) {
+        for (ConnectionPermissionKey connectionPermission : existingPermissions)
             existingConnectionIDs.add(connectionPermission.getConnection_id());
-        }
 
-        // delete any permissions that are not in the provided list
+        // Delete any permissions that are not in the provided list
         connectionPermissionExample.clear();
         connectionPermissionExample.createCriteria().andConnection_idNotIn(connectionIDs);
+        List<ConnectionPermissionKey> connectionPermissionsToDelete =
+                connectionPermissionDAO.selectByExample(connectionPermissionExample);
 
-        //make sure the user has permission to administrate each of these connections
+        // Make sure the user has permission to administrate each of these connections
-        List<ConnectionPermissionKey> connectionPermissionsToDelete = connectionPermissionDAO.selectByExample(connectionPermissionExample);
+        // corresponding to the permissions we are about to delete
-
+        for (ConnectionPermissionKey connectionPermissionToDelete : connectionPermissionsToDelete) {
-        for(ConnectionPermissionKey connectionPermissionToDelete : connectionPermissionsToDelete) {
+            if (!administerableConnections.contains(connectionPermissionToDelete.getConnection_id()))
-            if(!administerableConnections.contains(connectionPermissionToDelete.getConnection_id()))
+                throw new GuacamolePermissionException(
-                throw new GuacamolePermissionException("User '" + this.user.getUsername() + "' does not have permission to administrate connection " + connectionPermissionToDelete.getConnection_id());
+                    "User '" + this.user.getUsername() +
+                    "' does not have permission to administrate connection "
+                    + connectionPermissionToDelete.getConnection_id());
         }
 
         connectionPermissionDAO.deleteByExample(connectionPermissionExample);
 
-        // finally, insert the new permissions
+        // Finally, insert the new permissions
-        for(ConnectionPermission permission : permissions) {
+        for (ConnectionPermission permission : permissions) {
-            net.sourceforge.guacamole.net.auth.mysql.model.Connection dbConnection = dbConnectionMap.get(permission.getObjectIdentifier());
-            if(dbConnection == null)
-                throw new GuacamoleException("Connection '" + permission.getObjectIdentifier() + "' not found.");
 
-            // the permission for this connection already exists, we don't need to create it again
+            // Get permission
-            if(existingConnectionIDs.contains(dbConnection.getConnection_id()))
+            Connection dbConnection = dbConnectionMap.get(permission.getObjectIdentifier());
+            if (dbConnection == null)
+                throw new GuacamoleException(
+                    "Connection '" + permission.getObjectIdentifier()
+                    + "' not found.");
+
+            // If the permission for this connection already exists, we don't need to create it again
+            if (existingConnectionIDs.contains(dbConnection.getConnection_id()))
                 continue;
 
-            if(!administerableConnections.contains(dbConnection.getConnection_id()))
+            // Throw exception if permission to administer this connection
-                throw new GuacamolePermissionException("User '" + this.user.getUsername() + "' does not have permission to administrate connection " + dbConnection.getConnection_id());
+            // is not granted
+            if (!administerableConnections.contains(dbConnection.getConnection_id()))
+                throw new GuacamolePermissionException(
+                      "User '" + this.user.getUsername()
+                    + "' does not have permission to administrate connection "
+                    + dbConnection.getConnection_id());
 
 
+            // Insert previously-non-existent connection permission
             ConnectionPermissionKey newPermission = new ConnectionPermissionKey();
             newPermission.setConnection_id(dbConnection.getConnection_id());
             newPermission.setPermission(permission.getType().name());
             newPermission.setConnection_id(user.getUserID());
             connectionPermissionDAO.insert(newPermission);
+
         }
     }
 
     /**
-     * Update all system permissions for a given user.
+     * Update all system permissions for a given user. All permissions in
-     * @param permissions
+     * the given list not already granted to the user will be inserted, and all
-     * @param user
+     * permissions not in the list but already granted to the user will be
+     * deleted.
+     * 
+     * @param permissions The system permissions that the given user should
+     *                    have.
+     * @param user The user whose permissions should be updated.
      */
-    private void updateSystemPermissions(Iterable<SystemPermission> permissions, MySQLUser user) {
+    private void updateSystemPermissions(Iterable<SystemPermission> permissions,
+            MySQLUser user) {
+
+        // Build list of requested system permissions
         List<String> systemPermissionTypes = new ArrayList<String>();
-        for(SystemPermission permission : permissions) {
+        for (SystemPermission permission : permissions) {
+
+            // Connection directory permission
             String operation = permission.getType().name();
-            if(permission instanceof ConnectionDirectoryPermission)
+            if (permission instanceof ConnectionDirectoryPermission)
                 systemPermissionTypes.add(operation + "_CONNECTION");
-            else if(permission instanceof UserDirectoryPermission)
+
+            // User directory permission
+            else if (permission instanceof UserDirectoryPermission)
                 systemPermissionTypes.add(operation + "_USER");
+
         }
 
-        //delete all system permissions not in the list
+        // Delete all system permissions not in the list
         SystemPermissionExample systemPermissionExample = new SystemPermissionExample();
         systemPermissionExample.createCriteria().andUser_idEqualTo(user.getUserID()).andPermissionNotIn(systemPermissionTypes);
         systemPermissionDAO.deleteByExample(systemPermissionExample);
 
-        // find all existing system permissions
+        // Find all existing system permissions
         systemPermissionExample.clear();
         systemPermissionExample.createCriteria().andUser_idEqualTo(user.getUserID()).andPermissionIn(systemPermissionTypes);
         List<SystemPermissionKey> existingPermissions = systemPermissionDAO.selectByExample(systemPermissionExample);
         Set<String> existingPermissionTypes = new HashSet<String>();
-        for(SystemPermissionKey existingPermission : existingPermissions) {
+        for (SystemPermissionKey existingPermission : existingPermissions) {
             existingPermissionTypes.add(existingPermission.getPermission());
         }
 
-        // finally, insert any new system permissions for this user
+        // Finally, insert any NEW system permissions for this user
-        for(String systemPermissionType : systemPermissionTypes) {
+        for (String systemPermissionType : systemPermissionTypes) {
-            //do not insert the permission if it already exists
+
-            if(existingPermissionTypes.contains(systemPermissionType))
+            // Do not insert the permission if it already exists
+            if (existingPermissionTypes.contains(systemPermissionType))
                 continue;
 
+            // Insert permission
             SystemPermissionKey newSystemPermission = new SystemPermissionKey();
             newSystemPermission.setUser_id(user.getUserID());
             newSystemPermission.setPermission(systemPermissionType);
             systemPermissionDAO.insert(newSystemPermission);
+
         }
+
     }
 
     @Override
     @Transactional
-    public void update(User object) throws GuacamoleException {
+    public void update(net.sourceforge.guacamole.net.auth.User object)
-        permissionCheckUtility.verifyUserUpdateAccess(this.user.getUserID(), object.getUsername());
+            throws GuacamoleException {
-        //update the user in the database
+
+        // Validate permission to update this user is granted
+        permissionCheckUtility.verifyUserUpdateAccess(this.user.getUserID(),
+                object.getUsername());
+
+        // Update the user in the database
         MySQLUser mySQLUser = providerUtility.getExistingMySQLUser(object);
         userDAO.updateByPrimaryKey(mySQLUser.getUser());
 
-        //update permissions in database
+        // Update permissions in database
         updatePermissions(mySQLUser);
+
     }
 
     @Override
     @Transactional
     public void remove(String identifier) throws GuacamoleException {
-        permissionCheckUtility.verifyUserDeleteAccess(this.user.getUserID(), identifier);
 
+        // Validate current user has permission to remove the specified user
+        permissionCheckUtility.verifyUserDeleteAccess(this.user.getUserID(),
+                identifier);
+
+        // Get specified user
         MySQLUser mySQLUser = providerUtility.getExistingMySQLUser(identifier);
 
-        //delete all the user permissions in the database
+        // Delete all the user permissions in the database
         deleteAllPermissions(mySQLUser);
 
-        //delete the user in the database
+        // Delete the user in the database
         userDAO.deleteByPrimaryKey(mySQLUser.getUserID());
+
     }
 
     /**
-     * Delete all permissions associated with the provided user. This is only used when deleting a user.
+     * Delete all permissions associated with the provided user. This is only
-     * @param user
+     * used when deleting a user.
+     * 
+     * @param user The user to delete all permissions of.
      */
-    private void deleteAllPermissions(MySQLUser user) throws GuacamolePermissionException {
+    private void deleteAllPermissions(MySQLUser user) {
-        //delete all user permissions
+
+        // Delete all user permissions
         UserPermissionExample userPermissionExample = new UserPermissionExample();
         userPermissionExample.createCriteria().andUser_idEqualTo(user.getUserID());
         userPermissionDAO.deleteByExample(userPermissionExample);
 
-        //delete all connection permissions
+        // Delete all connection permissions
         ConnectionPermissionExample connectionPermissionExample = new ConnectionPermissionExample();
         connectionPermissionExample.createCriteria().andUser_idEqualTo(user.getUserID());
         connectionPermissionDAO.deleteByExample(connectionPermissionExample);
 
-        //delete all system permissions
+        // Delete all system permissions
         SystemPermissionExample systemPermissionExample = new SystemPermissionExample();
         systemPermissionExample.createCriteria().andUser_idEqualTo(user.getUserID());
         systemPermissionDAO.deleteByExample(systemPermissionExample);
 
-        //delete all permissions that refer to this user
+        // Delete all permissions that refer to this user
         userPermissionExample.createCriteria();
         userPermissionExample.createCriteria().andAffected_user_idEqualTo(user.getUserID());
         userPermissionDAO.deleteByExample(userPermissionExample);
+
     }
+
 }