[GitHub] [guacamole-client] necouchman commented on a diff in pull request #808: GUACAMOLE-1219: Add support for disabling TOTP for specific users and groups.

["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