[GitHub] [guacamole-client] necouchman commented on a change in pull request #389: GUACAMOLE-708: Enable auto-creation of users in JDBC modules

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #389:
URL: https://github.com/apache/guacamole-client/pull/389#discussion_r443090004



##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java
##########
@@ -460,6 +460,47 @@ public ModeledUser retrieveSkeletonUser(AuthenticationProvider authenticationPro
         return user;
         
     }
+    
+    /**
+     * Create a user in the database that does not already exist, setting up an
+     * empty model and inserting both the entity and the user object, and
+     * generating a random password for the account.
+     * 
+     * @param authenticationProvider
+     *     The authentication provider that authenticated the user.
+     * 
+     * @param authenticatedUser
+     *     The authenticated user that is being added to the database.
+     * 
+     * @return
+     *     The ModeledUser associated with the newly-created database object
+     *     for the user.
+     * 
+     * @throws GuacamoleException
+     *     If a ModeledUser cannot be created, or if the user cannot be
+     *     inserted into the database.
+     */
+    public ModeledUser createMissingUser(AuthenticationProvider authenticationProvider,
+            AuthenticatedUser authenticatedUser) throws GuacamoleException {
+        
+        ModeledUser user = getObjectInstance(null,
+                new UserModel(authenticatedUser.getIdentifier()));
+        
+        // Insert the database object
+        entityMapper.insert(user.getModel());
+            
+        // Auto-generate a password
+        user.setPassword(null);
+            
+        // Set up cyclic reference
+        user.setCurrentUser(new ModeledAuthenticatedUser(authenticatedUser,
+            authenticationProvider, user));
+            
+        // Insert the user object
+        userMapper.insert(user.getModel());

Review comment:
       Okay, @mike-jumper, jumping back into this.  The changes you made have vastly simplified the code....
   
   ....but, I've run aground of another issue.  Your first request, long, long ago, was to try to re-use the existing `userService.createObject()` method, both to avoid duplicating code, and to make sure that implicit permissions are granted.  I'm now hitting an issue with this, because the `PrivilegedModeledAuthenticatedUser` doesn't have an `entity_id`, which means when it goes to run `getImplicitPermissions()`, it sets the `entity_id` to `null`, which then causes a database constraint violation trying to insert into `guacamole_user_permission` with a `null` `entity_id` value.
   
   Suggestions?  Refactor `createObject()` to handle these cases?  Or scrap that and just manually do the `entityMapper.insert()` and `userMapper.insert()` calls, and then update permissions after those?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org