mike-jumper commented on a change in pull request #383: GUACAMOLE-715: Change
how null JDBC users are handled
URL: https://github.com/apache/guacamole-client/pull/383#discussion_r273293147
##########
File path:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java
##########
@@ -392,8 +399,12 @@ public ModeledAuthenticatedUser
retrieveAuthenticatedUser(AuthenticationProvider
* The AuthenticatedUser to retrieve the corresponding ModeledUser of.
*
* @return
- * The ModeledUser which corresponds to the given AuthenticatedUser, or
- * null if no such user exists.
+ * The ModeledUser which corresponds to the given AuthenticatedUser, if
+ * the user exists in the database. If the user does not exist, and
+ * the module has been configured to require JDBC authentication via
+ * a "[db]-user-required" parameter, null will be returned. Otherwise
Review comment:
This now looks OK, but this feels like a separation of concerns issue to me.
The `UserService` has up until now not dealt with configuration details from
the `JDBCEnvironment` at all, and this function is aimed at actual retrieval of
data.
What do you think about a separate function that embodies strictly the
semantics of creating a skeleton, in-memory `AuthenticatedUser`? That way
`retrieveAuthenticatedUser()` can truly be about retrieval and can remain
independent of configuration concerns?
----------------------------------------------------------------
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
