mike-jumper commented on a change in pull request #389: GUACAMOLE-708: Enable
auto-creation of users in JDBC modules
URL: https://github.com/apache/guacamole-client/pull/389#discussion_r386142720
##########
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:
I'll have to reread and reacquaint myself with these changes.
I don't think it will make sense to expose permission sets from
`AuthenticatedUser`, nor that there should need to be any fundamental changes
to guacamole-ext. It should be possible for the internals of the JDBC auth to
have their own internal representation of a system-level user without requiring
that permissions be re-anchored to `AuthenticatedUser`.
There are semantic issues with doing so:
1. The `AuthenticatedUser` interface is a pure means of representing
_identity_. Mixing that with identity (authentication) with permissions
(authorization) collapses an established separation of concerns.
2. There is no definition of cross-extension identity except for
`AuthenticatedUser`. As such, the identifiers used by `ObjectPermissionSet`
will not have a well-defined meaning. Given a particular `AuthenticatedUser`, I
can retrieve the corresponding `User` for each other extension and discover
what extension-specific permissions have been granted. If those permissions are
moved to `AuthenticatedUser`, this becomes impossible, and the meaning of the
identifiers becomes ambiguous.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
