You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "necouchman (via GitHub)" <gi...@apache.org> on 2023/03/14 01:20:19 UTC
[GitHub] [guacamole-client] necouchman opened a new pull request, #808: GUACAMOLE-1219: Add support for disabling TOTP for specific users and groups.
necouchman opened a new pull request, #808:
URL: https://github.com/apache/guacamole-client/pull/808
Continuing the work started in #577, this adds an attribute to the TOTP module at both a user and group level that allows for disabling the TOTP requirement for a certain user or users who are members of one or more groups that have the requirement disabled.
This closes #577.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner merged pull request #808: GUACAMOLE-1219: Add support for disabling TOTP for specific users and groups.
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner merged PR #808:
URL: https://github.com/apache/guacamole-client/pull/808
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman commented on a diff in pull request #808: GUACAMOLE-1219: Add support for disabling TOTP for specific users and groups.
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #808:
URL: https://github.com/apache/guacamole-client/pull/808#discussion_r1140527183
##########
extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java:
##########
@@ -203,6 +207,65 @@ private boolean setKey(UserContext context, UserTOTPKey key)
return true;
}
+
+ /**
+ * Checks the user in question, via both UserContext and AuthenticatedUser,
+ * to see if TOTP has been disabled for this user, either directly or via
+ * membership in a group that has had TOTP marked as disabled.
+ *
+ * @param context
+ * The UserContext for the user being verified.
+ *
+ * @param authenticatedUser
+ * The AuthenticatedUser for the user being verified.
+ *
+ * @return
+ * True if TOTP access has been disabled for the user, otherwise
+ * false.
+ *
+ * @throws GuacamoleException
+ * If the extension handling storage fails internally while attempting
+ * to update the user.
+ */
+ private boolean totpDisabled(UserContext context,
+ AuthenticatedUser authenticatedUser)
+ throws GuacamoleException {
+
+ // If TOTP is disabled for this user, return, allowing login to continue
+ if (TOTPUser.TRUTH_VALUE.equals(
+ context.self().getAttributes().get(
+ TOTPUser.TOTP_KEY_DISABLED_ATTRIBUTE_NAME
+ ))) {
+ logger.warn("TOTP validation has been disabled for user \"{}\"",
+ context.self().getIdentifier());
+ return true;
+ }
+
+ // Check if any effective user groups have TOTP marked as disabled
+ Set<String> userGroups = authenticatedUser.getEffectiveUserGroups();
+ Directory<UserGroup> directoryGroups = context.getPrivileged().getUserGroupDirectory();
+ for (String userGroup : userGroups) {
+ UserGroup thisGroup = directoryGroups.get(userGroup);
+ if (thisGroup == null)
+ continue;
+
+ Map<String, String> attributes = thisGroup.getAttributes();
+ if (attributes != null
+ && !attributes.isEmpty()
Review Comment:
Yeah, that makes sense - I was having some issues with `NullPointerException`s in/around this area, which is why I put all of the extra checks in place.
Fixed via rebase.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #808: GUACAMOLE-1219: Add support for disabling TOTP for specific users and groups.
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #808:
URL: https://github.com/apache/guacamole-client/pull/808#discussion_r1140510806
##########
extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java:
##########
@@ -203,6 +207,65 @@ private boolean setKey(UserContext context, UserTOTPKey key)
return true;
}
+
+ /**
+ * Checks the user in question, via both UserContext and AuthenticatedUser,
+ * to see if TOTP has been disabled for this user, either directly or via
+ * membership in a group that has had TOTP marked as disabled.
+ *
+ * @param context
+ * The UserContext for the user being verified.
+ *
+ * @param authenticatedUser
+ * The AuthenticatedUser for the user being verified.
+ *
+ * @return
+ * True if TOTP access has been disabled for the user, otherwise
+ * false.
+ *
+ * @throws GuacamoleException
+ * If the extension handling storage fails internally while attempting
+ * to update the user.
+ */
+ private boolean totpDisabled(UserContext context,
+ AuthenticatedUser authenticatedUser)
+ throws GuacamoleException {
+
+ // If TOTP is disabled for this user, return, allowing login to continue
+ if (TOTPUser.TRUTH_VALUE.equals(
+ context.self().getAttributes().get(
+ TOTPUser.TOTP_KEY_DISABLED_ATTRIBUTE_NAME
+ ))) {
+ logger.warn("TOTP validation has been disabled for user \"{}\"",
+ context.self().getIdentifier());
+ return true;
+ }
+
+ // Check if any effective user groups have TOTP marked as disabled
+ Set<String> userGroups = authenticatedUser.getEffectiveUserGroups();
+ Directory<UserGroup> directoryGroups = context.getPrivileged().getUserGroupDirectory();
+ for (String userGroup : userGroups) {
+ UserGroup thisGroup = directoryGroups.get(userGroup);
+ if (thisGroup == null)
+ continue;
+
+ Map<String, String> attributes = thisGroup.getAttributes();
+ if (attributes != null
+ && !attributes.isEmpty()
Review Comment:
This check doesn't seem necessary - if a map is empty it will never contain the key. For that matter, line 255 shouldn't be needed either - if the key doesn't exist in the map, it'll return `null`, and that will fail the check on line 256.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org