GUAC-1101: Fix user creation/deletion. Fix system permission modification.

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

@@ -134,13 +134,21 @@ public class MySQLUser implements User, DirectoryObject<UserModel> {
         // Store plaintext password internally
         this.password = password;
 
-        // Generate new salt and hash given password using newly-generated salt
+        // If no password provided, clear password salt and hash
+        if (password == null) {
+            userModel.setPasswordSalt(null);
+            userModel.setPasswordHash(null);
+        }
+
+        // Otherwise generate new salt and hash given password using newly-generated salt
+        else {
             byte[] salt = saltService.generateSalt();
             byte[] hash = encryptionService.createPasswordHash(password, salt);
 
             // Set stored salt and hash
             userModel.setPasswordSalt(salt);
             userModel.setPasswordHash(hash);
+        }
 
     }
 

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

@@ -92,8 +92,7 @@ public class UserDirectory implements Directory<User> {
     @Override
     @Transactional
     public void add(User object) throws GuacamoleException {
-        MySQLUser user = (MySQLUser) object;
+        userService.createObject(currentUser, object);
-        userService.createObject(currentUser, user);
     }
 
     @Override

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

@@ -40,14 +40,20 @@ import org.glyptodon.guacamole.net.auth.permission.ObjectPermissionSet;
  * permissions of the current user.
  *
  * @author Michael Jumper
- * @param <ObjectType>
+ * @param <InternalType>
- *     The type of object this service provides access to.
+ *     The specific internal implementation of the type of object this service
+ *     provides access to.
+ *
+ * @param <ExternalType>
+ *     The external interface or implementation of the type of object this
+ *     service provides access to, as defined by the guacamole-ext API.
  *
  * @param <ModelType>
- *     The underlying model object used to represent ObjectType in the
+ *     The underlying model object used to represent InternalType in the
  *     database.
  */
-public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<ModelType>, ModelType> {
+public abstract class DirectoryObjectService<InternalType extends DirectoryObject<ModelType>,
+        ExternalType, ModelType> {
 
     /**
      * Returns an instance of a mapper for the type of object used by this
@@ -72,9 +78,25 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      * @return
      *     An object which is backed by the given model object.
      */
-    protected abstract ObjectType getObjectInstance(AuthenticatedUser currentUser,
+    protected abstract InternalType getObjectInstance(AuthenticatedUser currentUser,
             ModelType model);
 
+    /**
+     * Returns an instance of a model object which is based on the given
+     * object.
+     *
+     * @param currentUser
+     *     The user for whom this model object is being created.
+     *
+     * @param object 
+     *     The object to use to produce the returned model object.
+     *
+     * @return
+     *     A model object which is based on the given object.
+     */
+    protected abstract ModelType getModelInstance(AuthenticatedUser currentUser,
+            ExternalType object);
+
     /**
      * Returns whether the given user has permission to create the type of
      * objects that this directory object service manages.
@@ -125,11 +147,11 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      *     A collection of objects which are backed by the models in the given
      *     collection.
      */
-    protected Collection<ObjectType> getObjectInstances(AuthenticatedUser currentUser,
+    protected Collection<InternalType> getObjectInstances(AuthenticatedUser currentUser,
             Collection<ModelType> models) {
 
         // Create new collection of objects by manually converting each model
-        Collection<ObjectType> objects = new ArrayList<ObjectType>(models.size());
+        Collection<InternalType> objects = new ArrayList<InternalType>(models.size());
         for (ModelType model : models)
             objects.add(getObjectInstance(currentUser, model));
 
@@ -154,11 +176,11 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      * @throws GuacamoleException
      *     If an error occurs while retrieving the requested object.
      */
-    public ObjectType retrieveObject(AuthenticatedUser user,
+    public InternalType retrieveObject(AuthenticatedUser user,
             String identifier) throws GuacamoleException {
 
         // Pull objects having given identifier
-        Collection<ObjectType> objects = retrieveObjects(user, Collections.singleton(identifier));
+        Collection<InternalType> objects = retrieveObjects(user, Collections.singleton(identifier));
 
         // If no such object, return null
         if (objects.isEmpty())
@@ -189,7 +211,7 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      * @throws GuacamoleException
      *     If an error occurs while retrieving the requested objects.
      */
-    public Collection<ObjectType> retrieveObjects(AuthenticatedUser user,
+    public Collection<InternalType> retrieveObjects(AuthenticatedUser user,
             Collection<String> identifiers) throws GuacamoleException {
 
         // Do not query if no identifiers given
@@ -226,12 +248,12 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      *     If the user lacks permission to create the object, or an error
      *     occurs while creating the object.
      */
-    public void createObject(AuthenticatedUser user, ObjectType object)
+    public void createObject(AuthenticatedUser user, ExternalType object)
         throws GuacamoleException {
 
         // Only create object if user has permission to do so
         if (user.getUser().isAdministrator() || hasCreatePermission(user)) {
-            getObjectMapper().insert(object.getModel());
+            getObjectMapper().insert(getModelInstance(user, object));
             return;
         }
 
@@ -286,7 +308,7 @@ public abstract class DirectoryObjectService<ObjectType extends DirectoryObject<
      *     If the user lacks permission to update the object, or an error
      *     occurs while updating the object.
      */
-    public void updateObject(AuthenticatedUser user, ObjectType object)
+    public void updateObject(AuthenticatedUser user, InternalType object)
         throws GuacamoleException {
 
         // Get object permissions

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

@@ -70,55 +70,14 @@ public class SystemPermissionService
     protected SystemPermissionModel getModelInstance(final MySQLUser targetUser,
             final SystemPermission permission) {
 
-        // Populate and return model object
+        SystemPermissionModel model = new SystemPermissionModel();
-        return new SystemPermissionModel() {
 
-            /**
+        // Populate model object with data from user and permission
-             * The ID of the user to whom this permission is granted.
+        model.setUserID(targetUser.getModel().getUserID());
-             */
+        model.setUsername(targetUser.getModel().getUsername());
-            private Integer userID = targetUser.getModel().getUserID();
+        model.setType(permission.getType());
 
-            /**
+        return model;
-             * The username of the user to whom this permission is granted.
-             */
-            private String username = targetUser.getModel().getUsername();
-
-            /**
-             * The type of action granted by this permission.
-             */
-            private SystemPermission.Type type = permission.getType();
-            
-            @Override
-            public Integer getUserID() {
-                return userID;
-            }
-
-            @Override
-            public void setUserID(Integer userID) {
-                this.userID = userID;
-            }
-
-            @Override
-            public String getUsername() {
-                return username;
-            }
-
-            @Override
-            public void setUsername(String username) {
-                this.username = username;
-            }
-
-            @Override
-            public SystemPermission.Type getType() {
-                return type;
-            }
-
-            @Override
-            public void setType(SystemPermission.Type type) {
-                this.type = type;
-            }
-
-        };
         
     }
 
@@ -191,8 +150,16 @@ public class SystemPermissionService
 
         // Only an admin can read permissions that aren't his own
         if (user.getUser().getIdentifier().equals(targetUser.getIdentifier())
-                || user.getUser().isAdministrator())
+                || user.getUser().isAdministrator()) {
-            return getPermissionInstance(getPermissionMapper().selectOne(targetUser.getModel(), type));
+
+            // Read permission from database, return null if not found
+            SystemPermissionModel model = getPermissionMapper().selectOne(targetUser.getModel(), type);
+            if (model == null)
+                return null;
+
+            return getPermissionInstance(model);
+
+        }
 
         // User cannot read this user's permissions
         throw new GuacamoleSecurityException("Permission denied.");

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

@@ -31,6 +31,7 @@ import net.sourceforge.guacamole.net.auth.mysql.dao.DirectoryObjectMapper;
 import net.sourceforge.guacamole.net.auth.mysql.dao.UserMapper;
 import net.sourceforge.guacamole.net.auth.mysql.model.UserModel;
 import org.glyptodon.guacamole.GuacamoleException;
+import org.glyptodon.guacamole.net.auth.User;
 import org.glyptodon.guacamole.net.auth.permission.ObjectPermissionSet;
 import org.glyptodon.guacamole.net.auth.permission.SystemPermission;
 import org.glyptodon.guacamole.net.auth.permission.SystemPermissionSet;
@@ -41,7 +42,7 @@ import org.glyptodon.guacamole.net.auth.permission.SystemPermissionSet;
  *
  * @author Michael Jumper, James Muehlner
  */
-public class UserService extends DirectoryObjectService<MySQLUser, UserModel> {
+public class UserService extends DirectoryObjectService<MySQLUser, User, UserModel> {
 
     /**
      * Mapper for accessing users.
@@ -68,6 +69,22 @@ public class UserService extends DirectoryObjectService<MySQLUser, UserModel> {
         return user;
     }
 
+    @Override
+    protected UserModel getModelInstance(AuthenticatedUser currentUser,
+            final User object) {
+
+        // Create new MySQLUser backed by blank model
+        UserModel model = new UserModel();
+        MySQLUser user = getObjectInstance(currentUser, model);
+
+        // Set model contents through MySQLUser, copying the provided user
+        user.setIdentifier(object.getIdentifier());
+        user.setPassword(object.getPassword());
+
+        return model;
+        
+    }
+
     @Override
     protected boolean hasCreatePermission(AuthenticatedUser user)
             throws GuacamoleException {

extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/SystemPermissionMapper.xml

@@ -83,8 +83,7 @@
             permission
         )
         VALUES
-            <foreach collection="permissions" item="permission"
+            <foreach collection="permissions" item="permission" separator=",">
-                     open="(" separator="," close=")">
                 (#{permission.userID,jdbcType=INTEGER},
                  #{permission.type,jdbcType=VARCHAR})
             </foreach>

extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/UserMapper.xml

@@ -115,9 +115,9 @@
             password_salt
         )
         VALUES (
-            #{username,jdbcType=VARCHAR},
+            #{object.username,jdbcType=VARCHAR},
-            #{passwordHash,jdbcType=BINARY},
+            #{object.passwordHash,jdbcType=BINARY},
-            #{passwordSalt,jdbcType=BINARY}
+            #{object.passwordSalt,jdbcType=BINARY}
         )
 
         <selectKey resultType="java.lang.Integer" keyProperty="userID" order="AFTER">
@@ -129,9 +129,9 @@
     <!-- Update single user -->
     <update id="update" parameterType="net.sourceforge.guacamole.net.auth.mysql.model.UserModel">
         UPDATE guacamole_user
-        SET password_hash = #{passwordHash,jdbcType=BINARY},
+        SET password_hash = #{object.passwordHash,jdbcType=BINARY},
-            password_salt = #{passwordSalt,jdbcType=BINARY}
+            password_salt = #{object.passwordSalt,jdbcType=BINARY}
-        WHERE user_id = #{userID,jdbcType=VARCHAR}
+        WHERE user_id = #{object.userID,jdbcType=VARCHAR}
     </update>
 
 </mapper>