necouchman commented on a change in pull request #389:
URL: https://github.com/apache/guacamole-client/pull/389#discussion_r443158336
##########
File path:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java
##########
@@ -427,16 +427,22 @@ public InternalType
retrieveObject(ModeledAuthenticatedUser user,
protected Collection<ObjectPermissionModel>
getImplicitPermissions(ModeledAuthenticatedUser user,
ModelType model) {
+ // Get the user model and check for an entity ID.
+ UserModel userModel = user.getUser().getModel();
+ Integer entityId = userModel.getEntityID();
+ if (entityId == null)
+ return Collections.emptyList();
Review comment:
Okay, I think I see what you're getting at. Basically...
- `createObject()` requires two parameters - the user creating the object,
and the object being created.
- In `JDBCAuthenticationService` I make the call to
`userService.createObject()` to create the user. The second argument for that
call is obviously the user record that is being automatically created, here.
The first argument (as currently implemented) is a new
`PrivilegedModeledAuthenticatedUser` based on the user that was authenticated
and is currently being added. So, essentially, we're giving the user that is
currently being added the privilege to create their own account in the database.
- However, because that user doesn't already exist (presumably _at the time
of_ the `new PrivilegedModeledAuthenticatedUser()` call), there is no entity,
so the `entityId` field is null, resulting in a constraint violation when
trying to insert those implicit privileges.
- Thus, the only implicit privilege that actually ever gets created with
these checks is the one for the user to `READ` their own account. Which I
think is actually okay and desirable - otherwise, the user will end up with
full privileges on their own account, which might be okay but also might be
more than what the sysadmin desires, and I'm all about least privilege.
Let me know if there's something different I should do, here:
- Move the `new PrivilegedModeledAuthenticatedUser()` to a different place.
- Use the `userContext.getPrivileged()` instead of manually creating that
user.
- Use a different based account for the `new
PrivilegedModeledAuthenticatedUser()` call such that an entity exists.
- Something else?
----------------------------------------------------------------
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]
