Skip to content

Fixed bug with empty groups #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
osan0926 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed bug with empty groups #36

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions Frends.LDAP.AddUserToGroups/Frends.LDAP.AddUserToGroups/AddUserToGroups.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static Result AddUserToGroups([PropertyTab] Input input, [PropertyTab] Co

if (UserExistsInGroup(conn, input.UserDistinguishedName, input.GroupDistinguishedName, cancellationToken) && input.UserExistsAction.Equals(UserExistsAction.Skip))
return new Result(false, "AddUserToGroups LDAP error: User already exists in the group.", input.UserDistinguishedName, input.GroupDistinguishedName);

conn.Modify(input.GroupDistinguishedName, mods);

return new Result(true, null, input.UserDistinguishedName, input.GroupDistinguishedName);
Expand All @@ -62,7 +62,7 @@ public static Result AddUserToGroups([PropertyTab] Input input, [PropertyTab] Co
conn.Disconnect();
}
}

private static bool UserExistsInGroup(LdapConnection connection, string userDn, string groupDn, CancellationToken cancellationToken)
{
// Search for the user's groups
Expand Down Expand Up @@ -90,12 +90,17 @@ private static bool UserExistsInGroup(LdapConnection connection, string userDn,
if (entry != null)
{
LdapAttribute memberAttr = entry.GetAttribute("member");
var currentMembers = memberAttr.StringValueArray;
if (currentMembers.Where(e => e == userDn).Any())
//Check that the attribute exists, otherwise an error will be thrown if the group is empty.
if(memberAttr != null)
{
var currentMembers = memberAttr.StringValueArray;
if (currentMembers.Where(e => e == userDn).Any())
return true;
}
return false;
}
Comment on lines +93 to 101
Copy link

@coderabbitai coderabbitai bot Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the logical flow and code formatting issues.

While the null check addresses the core issue, there are several problems with the current implementation:

  1. Logic error: The return false; on line 100 exits the method immediately when an entry lacks a member attribute, preventing the method from checking other entries that might contain the user.

  2. Code formatting: The formatting doesn't follow Microsoft C# conventions - missing space after if and inconsistent brace placement.

  3. Comment style: The comment should follow standard conventions.

Apply this diff to fix the logical flow and formatting:

-                //Check that the attribute exists, otherwise an error will be thrown if the group is empty.
-                if(memberAttr != null)
-                {
-                    var currentMembers = memberAttr.StringValueArray;
-                    if (currentMembers.Where(e => e == userDn).Any())
-                return true;
-                }
-                return false;
+                // Check that the attribute exists, otherwise an error will be thrown if the group is empty.
+                if (memberAttr != null)
+                {
+                    var currentMembers = memberAttr.StringValueArray;
+                    if (currentMembers.Where(e => e == userDn).Any())
+                        return true;
+                }

The method should continue checking other entries if the current entry doesn't have a member attribute, rather than returning false immediately. The final return false; on line 104 will handle the case where the user is not found in any entry.

🤖 Prompt for AI Agents 
In Frends.LDAP.AddUserToGroups/AddUserToGroups.cs around lines 93 to 101, fix
the logical flow by removing the immediate return false inside the loop when
memberAttr is null, allowing the method to continue checking other entries.
Adjust the code formatting to include a space after if and use consistent brace
placement according to Microsoft C# conventions. Update the comment to follow
standard style guidelines. Ensure the final return false is only after all
entries have been checked.

}

return false;
}
}