You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/06/20 05:03:43 UTC
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #389: GUACAMOLE-708: Enable auto-creation of users in JDBC modules
mike-jumper commented on a change in pull request #389:
URL: https://github.com/apache/guacamole-client/pull/389#discussion_r443101547
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java
##########
@@ -429,14 +429,19 @@ public InternalType retrieveObject(ModeledAuthenticatedUser user,
// Build list of implicit permissions
Collection<ObjectPermissionModel> implicitPermissions =
- new ArrayList<ObjectPermissionModel>(IMPLICIT_OBJECT_PERMISSIONS.length);
+ new ArrayList<>();
Review comment:
Why is the initial length is going away?
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerGuacamoleProperties.java
##########
@@ -193,5 +193,13 @@ private SQLServerGuacamoleProperties() {}
public String getName() { return "sqlserver-driver"; }
};
+
+ public static final BooleanGuacamoleProperty SQLSERVER_AUTO_CREATE_ACCOUNTS =
Review comment:
Here, too - missing JavaDoc.
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/ModeledDirectoryObjectService.java
##########
@@ -429,14 +429,19 @@ public InternalType retrieveObject(ModeledAuthenticatedUser user,
// Build list of implicit permissions
Collection<ObjectPermissionModel> implicitPermissions =
- new ArrayList<ObjectPermissionModel>(IMPLICIT_OBJECT_PERMISSIONS.length);
+ new ArrayList<>();
UserModel userModel = user.getUser().getModel();
for (ObjectPermission.Type permission : IMPLICIT_OBJECT_PERMISSIONS) {
+ Integer entityId = userModel.getEntityID();
+
+ if (entityId == null)
+ continue;
Review comment:
If I'm reading this correctly, if user model does not yet have an associated entity ID, the entirety of this loop will be skipped (one iteration at a time). Is this correct and intended?
Can you clarify under what circumstances the entity ID would not be present?
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/java/org/apache/guacamole/auth/mysql/conf/MySQLGuacamoleProperties.java
##########
@@ -240,5 +240,13 @@ private MySQLGuacamoleProperties() {}
public String getName() { return "mysql-ssl-client-password"; }
};
+
+ public static final BooleanGuacamoleProperty MYSQL_AUTO_CREATE_ACCOUNTS =
Review comment:
Missing JavaDoc.
##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java
##########
@@ -126,9 +127,15 @@ public ModeledUserContext getUserContext(AuthenticationProvider authenticationPr
}
// If no user account is found, and database-specific account
- // restrictions do not apply, get an empty user.
+ // restrictions do not apply, get a skeleton user.
else if (!databaseRestrictionsApplicable) {
user = userService.retrieveSkeletonUser(authenticationProvider, authenticatedUser);
+
+ // If auto account creation is enabled, add user to DB.
+ if(environment.autoCreateAbsentAccounts()) {
Review comment:
Your `if` nemesis returns. ;)
----------------------------------------------------------------
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