Skip to content

New Module add-group#688

Open
termanix wants to merge 23 commits intoPennyw0rth:mainfrom
termanix:add-group
Open

New Module add-group#688
termanix wants to merge 23 commits intoPennyw0rth:mainfrom
termanix:add-group

Conversation

@termanix
Copy link
Contributor

@termanix termanix commented May 14, 2025

Description

This new module provides add/remove users to groups. It works for SMB and LDAP, also supports Bloodhound, AddSelf, AddGroupMember etc.
LDAP part works with LDAP3

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested on local lab and HTB, not yet GOAD.

Screenshots (if appropriate):

image

image

image

Checklist:

  • I have ran Ruff against my changes (via poetry: poetry run python -m ruff check . --preview, use --fix to automatically fix what it can)
  • I have added or updated the tests/e2e_commands.txt file if necessary
  • New and existing e2e tests pass locally with my changes
  • My code follows the style guidelines of this project (should be covered by Ruff above)
  • If reliant on third party dependencies, such as Impacket, dploot, lsassy, etc, I have linked the relevant PRs in those projects
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (PR here: https://github.com/Pennyw0rth/NetExec-Wiki)

@NeffIsBack
Copy link
Member

Nice one! Thanks for the work!

@NeffIsBack NeffIsBack mentioned this pull request May 15, 2025
9 tasks
@Marshall-Hallenbeck Marshall-Hallenbeck added the enhancement New feature or request label May 17, 2025
@NeffIsBack NeffIsBack removed the enhancement New feature or request label May 22, 2025
@NeffIsBack NeffIsBack mentioned this pull request Oct 27, 2025
13 tasks
Copy link
Member

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

@termanix can you switch to impackets ldap connection for modification and remove ldap3? Also maybe we should change the module name to modify_groups because we do a lot more than just adding things. Thoughts everyone?

@termanix
Copy link
Contributor Author

termanix commented Feb 6, 2026

Yea sure, I will remove ldap3 but need time. Probably in 2-3 weeks. Changing name it's okay for me btw.

@NeffIsBack
Copy link
Member

Yea sure, I will remove ldap3 but need time. Probably in 2-3 weeks. Changing name it's okay for me btw.

Great👍no worries, won't have time until march anyway

@termanix
Copy link
Contributor Author

@NeffIsBack It's ready to review

Copy link
Member

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

The logic can probably be stripped down a lot. Please do:

  • Implement the "TODO OU modification". If you don't have time now that's totally fine, but we should not merge something with open "TODOs" in the code/options.
  • Actually use the ldap.add()/ldap.modify() functions that were added
  • See review comments

I will push an update to the impacket code because currently the installation is too old and does not include the added ldap functions.

"""
Required (at least one of):
GROUP Name of the group to add/remove the user to/from
OU TO DO --> Distinguished name of the OU to move the user to
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is not ready for a review 😅

Comment on lines +64 to +68
"""
To do
if not self.group and not self.ou:
context.log.fail("Either GROUP or OU parameter is required!")
sys.exit(1)"""
Copy link
Member

Choose a reason for hiding this comment

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

Please don't leave these artifacts

@NeffIsBack NeffIsBack added this to the v1.6.0 milestone Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants