From 6f8b0ba41e87ec20e75336e4c6bc37dbd433f87e Mon Sep 17 00:00:00 2001
From: Michael Jumper <mike.jumper@guac-dev.org>
Date: Sun, 1 Mar 2015 12:05:58 -0800
Subject: [PATCH] GUAC-1101: Implement connection and group duplicate checks.
 Fix username duplicate check.

---
 .../jdbc/base/DirectoryObjectService.java     | 48 ++++++++++---------
 .../jdbc/connection/ConnectionMapper.java     | 17 +++++++
 .../jdbc/connection/ConnectionService.java    | 34 ++++++++-----
 .../ConnectionGroupMapper.java                | 17 +++++++
 .../ConnectionGroupService.java               | 27 +++++++----
 .../guacamole/auth/jdbc/user/UserMapper.java  | 14 +++++-
 .../guacamole/auth/jdbc/user/UserService.java | 25 +++++-----
 .../auth/jdbc/connection/ConnectionMapper.xml | 16 +++++++
 .../connectiongroup/ConnectionGroupMapper.xml | 16 +++++++
 .../guacamole/auth/jdbc/user/UserMapper.xml   | 18 ++++++-
 10 files changed, 173 insertions(+), 59 deletions(-)

diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/base/DirectoryObjectService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/base/DirectoryObjectService.java
index 1e879b904..fc2bdb331 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/base/DirectoryObjectService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/base/DirectoryObjectService.java
@@ -215,49 +215,50 @@ public abstract class DirectoryObjectService<InternalType extends DirectoryObjec
     }
 
     /**
-     * Returns whether the given object is valid and can be created as-is. The
-     * object does not yet exist in the database, but the user desires to
-     * create a new object with the given values. This function will be called
-     * prior to any creation operation, and provides a means for the
-     * implementation to abort prior to completion. The default implementation
-     * does nothing.
+     * Returns whether the contents of the given model are valid and can be
+     * used to create a new object as-is. The object does not yet exist in the
+     * database, but the user desires to create a new object with the given
+     * model. This function will be called prior to any creation operation, and
+     * provides a means for the implementation to abort prior to completion. The
+     * default implementation does nothing.
      *
      * @param user
      *     The user creating the object.
      *
-     * @param object
-     *     The object to validate.
+     * @param model
+     *     The model to validate.
      *
      * @throws GuacamoleException
      *     If the object is invalid, or an error prevents validating the given
      *     object.
      */
-    protected void validateNewObject(AuthenticatedUser user,
-            ExternalType object) throws GuacamoleException {
+    protected void validateNewModel(AuthenticatedUser user,
+            ModelType model) throws GuacamoleException {
 
         // By default, do nothing.
 
     }
 
     /**
-     * Returns whether the given object is valid and can updated as-is. The
-     * object already exists in the database, but the user desires to update
-     * the object to the given values. This function will be called prior to
-     * update operation, and provides a means for the implementation to abort
-     * prior to completion. The default implementation does nothing.
+     * Returns whether the given model is valid and can be used to update an
+     * existing object as-is. The object already exists in the database, but the
+     * user desires to update the object to the given model. This function will
+     * be called prior to update operation, and provides a means for the
+     * implementation to abort prior to completion. The default implementation
+     * does nothing.
      *
      * @param user
      *     The user updating the existing object.
      *
-     * @param object 
-     *     The object to validate.
+     * @param model
+     *     The model to validate.
      *
      * @throws GuacamoleException
      *     If the object is invalid, or an error prevents validating the given
      *     object.
      */
-    protected void validateExistingObject(AuthenticatedUser user,
-            InternalType object) throws GuacamoleException {
+    protected void validateExistingModel(AuthenticatedUser user,
+            ModelType model) throws GuacamoleException {
 
         // By default, do nothing.
 
@@ -362,10 +363,10 @@ public abstract class DirectoryObjectService<InternalType extends DirectoryObjec
         if (user.getUser().isAdministrator() || hasCreatePermission(user)) {
 
             // Validate object prior to creation
-            validateNewObject(user, object);
+            ModelType model = getModelInstance(user, object);
+            validateNewModel(user, model);
 
             // Create object
-            ModelType model = getModelInstance(user, object);
             getObjectMapper().insert(model);
 
             // Build list of implicit permissions
@@ -447,10 +448,11 @@ public abstract class DirectoryObjectService<InternalType extends DirectoryObjec
         if (hasObjectPermission(user, object.getIdentifier(), ObjectPermission.Type.UPDATE)) {
 
             // Validate object prior to creation
-            validateExistingObject(user, object);
+            ModelType model = object.getModel();
+            validateExistingModel(user, model);
 
             // Update object
-            getObjectMapper().update(object.getModel());
+            getObjectMapper().update(model);
             return;
         }
 
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.java
index 8c8cf0a2b..faa7f21d1 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.java
@@ -71,5 +71,22 @@ public interface ConnectionMapper extends DirectoryObjectMapper<ConnectionModel>
      */
     Set<String> selectReadableIdentifiersWithin(@Param("user") UserModel user,
             @Param("parentIdentifier") String parentIdentifier);
+
+    /**
+     * Selects the connection within the given parent group and having the
+     * given name. If no such connection exists, null is returned.
+     *
+     * @param parentIdentifier
+     *     The identifier of the parent group to search within.
+     *
+     * @param name
+     *     The name of the connection to find.
+     *
+     * @return
+     *     The connection having the given name within the given parent group,
+     *     or null if no such connection exists.
+     */
+    ConnectionModel selectOneByName(@Param("parentIdentifier") String parentIdentifier,
+            @Param("name") String name);
     
 }
\ No newline at end of file
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionService.java
index fcc43208b..f7d0b5ac5 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionService.java
@@ -147,26 +147,37 @@ public class ConnectionService extends DirectoryObjectService<ModeledConnection,
     }
 
     @Override
-    protected void validateNewObject(AuthenticatedUser user, Connection object)
-            throws GuacamoleException {
+    protected void validateNewModel(AuthenticatedUser user,
+            ConnectionModel model) throws GuacamoleException {
 
         // Name must not be blank
-        if (object.getName().trim().isEmpty())
+        if (model.getName().trim().isEmpty())
             throw new GuacamoleClientException("Connection names must not be blank.");
-        
-        // FIXME: Do not attempt to create duplicate connections
+
+        // Do not attempt to create duplicate connections
+        ConnectionModel existing = connectionMapper.selectOneByName(model.getParentIdentifier(), model.getName());
+        if (existing != null)
+            throw new GuacamoleClientException("The connection \"" + model.getName() + "\" already exists.");
 
     }
 
     @Override
-    protected void validateExistingObject(AuthenticatedUser user,
-            ModeledConnection object) throws GuacamoleException {
+    protected void validateExistingModel(AuthenticatedUser user,
+            ConnectionModel model) throws GuacamoleException {
 
         // Name must not be blank
-        if (object.getName().trim().isEmpty())
+        if (model.getName().trim().isEmpty())
             throw new GuacamoleClientException("Connection names must not be blank.");
         
-        // FIXME: Check whether such a connection is already present
+        // Check whether such a connection is already present
+        ConnectionModel existing = connectionMapper.selectOneByName(model.getParentIdentifier(), model.getName());
+        if (existing != null) {
+
+            // If the specified name matches a DIFFERENT existing connection, the update cannot continue
+            if (!existing.getObjectID().equals(model.getObjectID()))
+                throw new GuacamoleClientException("The connection \"" + model.getName() + "\" already exists.");
+
+        }
         
     }
 
@@ -238,10 +249,11 @@ public class ConnectionService extends DirectoryObjectService<ModeledConnection,
         // Update connection
         super.updateObject(user, object);
 
-        // Replace existing parameters with new parameters
+        // Replace existing parameters with new parameters, if any
         Collection<ParameterModel> parameterModels = getParameterModels(object);
         parameterMapper.delete(object.getIdentifier());
-        parameterMapper.insert(parameterModels);
+        if (!parameterModels.isEmpty())
+            parameterMapper.insert(parameterModels);
         
     }
 
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.java
index ad682cd20..a08ef7c20 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.java
@@ -71,5 +71,22 @@ public interface ConnectionGroupMapper extends DirectoryObjectMapper<ConnectionG
      */
     Set<String> selectReadableIdentifiersWithin(@Param("user") UserModel user,
             @Param("parentIdentifier") String parentIdentifier);
+
+    /**
+     * Selects the connection group within the given parent group and having
+     * the given name. If no such connection group exists, null is returned.
+     *
+     * @param parentIdentifier
+     *     The identifier of the parent group to search within.
+     *
+     * @param name
+     *     The name of the connection group to find.
+     *
+     * @return
+     *     The connection group having the given name within the given parent
+     *     group, or null if no such connection group exists.
+     */
+    ConnectionGroupModel selectOneByName(@Param("parentIdentifier") String parentIdentifier,
+            @Param("name") String name);
     
 }
\ No newline at end of file
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupService.java
index f144dcb87..cfed2edd0 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupService.java
@@ -130,26 +130,37 @@ public class ConnectionGroupService extends DirectoryObjectService<ModeledConnec
     }
 
     @Override
-    protected void validateNewObject(AuthenticatedUser user, ConnectionGroup object)
-            throws GuacamoleException {
+    protected void validateNewModel(AuthenticatedUser user,
+            ConnectionGroupModel model) throws GuacamoleException {
 
         // Name must not be blank
-        if (object.getName().trim().isEmpty())
+        if (model.getName().trim().isEmpty())
             throw new GuacamoleClientException("Connection group names must not be blank.");
         
-        // FIXME: Do not attempt to create duplicate connection groups
+        // Do not attempt to create duplicate connection groups
+        ConnectionGroupModel existing = connectionGroupMapper.selectOneByName(model.getParentIdentifier(), model.getName());
+        if (existing != null)
+            throw new GuacamoleClientException("The connection group \"" + model.getName() + "\" already exists.");
 
     }
 
     @Override
-    protected void validateExistingObject(AuthenticatedUser user,
-            ModeledConnectionGroup object) throws GuacamoleException {
+    protected void validateExistingModel(AuthenticatedUser user,
+            ConnectionGroupModel model) throws GuacamoleException {
 
         // Name must not be blank
-        if (object.getName().trim().isEmpty())
+        if (model.getName().trim().isEmpty())
             throw new GuacamoleClientException("Connection group names must not be blank.");
         
-        // FIXME: Check whether such a connection group is already present
+        // Check whether such a connection group is already present
+        ConnectionGroupModel existing = connectionGroupMapper.selectOneByName(model.getParentIdentifier(), model.getName());
+        if (existing != null) {
+
+            // If the specified name matches a DIFFERENT existing connection group, the update cannot continue
+            if (!existing.getObjectID().equals(model.getObjectID()))
+                throw new GuacamoleClientException("The connection group \"" + model.getName() + "\" already exists.");
+
+        }
         
     }
 
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.java
index fbe83ae25..0563196de 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.java
@@ -46,7 +46,19 @@ public interface UserMapper extends DirectoryObjectMapper<UserModel> {
      *     The user having the given username and password, or null if no such
      *     user exists.
      */
-    UserModel selectByCredentials(@Param("username") String username,
+    UserModel selectOneByCredentials(@Param("username") String username,
             @Param("password") String password);
+
+    /**
+     * Returns the user having the given username, if any. If no such user
+     * exists, null is returned.
+     *
+     * @param username
+     *     The username of the user to return.
+     *
+     * @return
+     *     The user having the given username, or null if no such user exists.
+     */
+    UserModel selectOne(@Param("username") String username);
     
 }
\ No newline at end of file
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserService.java
index 0785337ee..1fd24dc54 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/glyptodon/guacamole/auth/jdbc/user/UserService.java
@@ -118,38 +118,35 @@ public class UserService extends DirectoryObjectService<ModeledUser, User, UserM
     }
 
     @Override
-    protected void validateNewObject(AuthenticatedUser user, User object)
+    protected void validateNewModel(AuthenticatedUser user, UserModel model)
             throws GuacamoleException {
 
         // Username must not be blank
-        if (object.getIdentifier().trim().isEmpty())
+        if (model.getIdentifier().trim().isEmpty())
             throw new GuacamoleClientException("The username must not be blank.");
         
         // Do not create duplicate users
-        Collection<UserModel> existing = userMapper.select(Collections.singleton(object.getIdentifier()));
+        Collection<UserModel> existing = userMapper.select(Collections.singleton(model.getIdentifier()));
         if (!existing.isEmpty())
-            throw new GuacamoleClientException("User \"" + object.getIdentifier() + "\" already exists.");
+            throw new GuacamoleClientException("User \"" + model.getIdentifier() + "\" already exists.");
 
     }
 
     @Override
-    protected void validateExistingObject(AuthenticatedUser user,
-            ModeledUser object) throws GuacamoleException {
+    protected void validateExistingModel(AuthenticatedUser user,
+            UserModel model) throws GuacamoleException {
 
         // Username must not be blank
-        if (object.getIdentifier().trim().isEmpty())
+        if (model.getIdentifier().trim().isEmpty())
             throw new GuacamoleClientException("The username must not be blank.");
         
         // Check whether such a user is already present
-        ModeledUser existing = retrieveObject(user, object.getIdentifier());
+        UserModel existing = userMapper.selectOne(model.getIdentifier());
         if (existing != null) {
 
-            UserModel existingModel = existing.getModel();
-            UserModel updatedModel = object.getModel();
-
             // Do not rename to existing user
-            if (!existingModel.getObjectID().equals(updatedModel.getObjectID()))
-                throw new GuacamoleClientException("User \"" + object.getIdentifier() + "\" already exists.");
+            if (!existing.getObjectID().equals(model.getObjectID()))
+                throw new GuacamoleClientException("User \"" + model.getIdentifier() + "\" already exists.");
             
         }
         
@@ -173,7 +170,7 @@ public class UserService extends DirectoryObjectService<ModeledUser, User, UserM
         String password = credentials.getPassword();
 
         // Retrieve user model, if the user exists
-        UserModel userModel = userMapper.selectByCredentials(username, password);
+        UserModel userModel = userMapper.selectOneByCredentials(username, password);
         if (userModel == null)
             return null;
 
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.xml b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.xml
index ac8fb6d6b..2211da069 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.xml
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connection/ConnectionMapper.xml
@@ -107,6 +107,22 @@
 
     </select>
 
+    <!-- Select single connection by name -->
+    <select id="selectOneByName" resultMap="ConnectionResultMap">
+
+        SELECT
+            connection_id,
+            connection_name,
+            parent_id,
+            protocol 
+        FROM guacamole_connection
+        WHERE 
+            <if test="parentIdentifier != null">parent_id = #{parentIdentifier,jdbcType=VARCHAR}</if>
+            <if test="parentIdentifier == null">parent_id IS NULL</if>
+            AND connection_name = #{name,jdbcType=VARCHAR}
+
+    </select>
+
     <!-- Delete single connection by identifier -->
     <delete id="delete">
         DELETE FROM guacamole_connection
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.xml b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.xml
index 3e3f8c156..4eb20da1c 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.xml
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/connectiongroup/ConnectionGroupMapper.xml
@@ -108,6 +108,22 @@
 
     </select>
 
+    <!-- Select single connection group by name -->
+    <select id="selectOneByName" resultMap="ConnectionGroupResultMap">
+
+        SELECT
+            connection_group_id,
+            connection_group_name,
+            parent_id,
+            type
+        FROM guacamole_connection_group
+        WHERE 
+            <if test="parentIdentifier != null">parent_id = #{parentIdentifier,jdbcType=VARCHAR}</if>
+            <if test="parentIdentifier == null">parent_id IS NULL</if>
+            AND connection_group_name = #{name,jdbcType=VARCHAR}
+
+    </select>
+
     <!-- Delete single connection group by identifier -->
     <delete id="delete">
         DELETE FROM guacamole_connection_group
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.xml b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.xml
index b3726eb8f..fb7e6ff55 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.xml
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/resources/org/glyptodon/guacamole/auth/jdbc/user/UserMapper.xml
@@ -87,8 +87,8 @@
 
     </select>
 
-
-    <select id="selectByCredentials" resultMap="UserResultMap">
+    <!-- Select single user by credentials -->
+    <select id="selectOneByCredentials" resultMap="UserResultMap">
         SELECT
             user_id,
             username,
@@ -100,6 +100,20 @@
             AND password_hash = UNHEX(SHA2(CONCAT(#{password,jdbcType=VARCHAR}, HEX(password_salt)), 256))
     </select>
 
+    <!-- Select single user by username -->
+    <select id="selectOne" resultMap="UserResultMap">
+
+        SELECT
+            user_id,
+            username,
+            password_hash,
+            password_salt
+        FROM guacamole_user
+        WHERE
+            username = #{identifier,jdbcType=VARCHAR}
+
+    </select>
+
     <!-- Delete single user by username -->
     <delete id="delete">
         DELETE FROM guacamole_user