feat(ldap): Add Active Directory nested group matching and fix PHP LDAP auth errors#791
Merged
mastacontrola merged 19 commits intoFOGProject:dev-branchfrom Mar 2, 2026
Merged
Conversation
Correct various typos, spacing and punctuation in UI/error strings and translations. Changes include: standardizing "Active Multicast Tasks" title; removing/adding punctuation in exception and status messages (accesscontrol, wolbroadcast, pending MAC report); fixing "LDAP Connection Name" spacing; and a large update to Portuguese translations (messages.po) including Project-Id, translator metadata, language tag and many corrected msgstr entries for consistency and clarity. These edits improve UI text accuracy and translation quality.
Introduce an enableNestedGroup option (lsEnableNestedGroup) throughout the LDAP plugin: mapping, DB schema, and UI checkbox. Add a userAccountControl check after binding to deny logins for disabled AD accounts. Implement nested group membership checks for Active Directory using the matching-rule-in-chain OID (1.2.840.113556.1.4.1941) against memberOf to detect admin/user group membership and return appropriate access levels, with a fallback to existing behavior. Bump plugin version to 1.5.5_2.
This reverts commit d9a4431.
Introduce an enableNestedGroup option (lsEnableNestedGroup) throughout the LDAP plugin: mapping, DB schema, and UI checkbox. Add a userAccountControl check after binding to deny logins for disabled AD accounts. Implement nested group membership checks for Active Directory using the matching-rule-in-chain OID (1.2.840.113556.1.4.1941) against memberOf to detect admin/user group membership and return appropriate access levels, with a fallback to existing behavior. Bump plugin version to 1.5.5_2.
added 9 commits
February 25, 2026 19:40
Add an "Enable Nested Group Matching (Active Directory only)" checkbox to the LDAP management form and persist its value. The patch reads enableNestedGroup from POST in both add and update flows and sets it on the LDAP object before saving. Changes made in packages/web/lib/plugins/ldap/pages/ldapmanagementpage.class.php.
Introduce $enableNestedGroup and $nestedGroupChecked to determine the nested-group checkbox state from POST input or the stored object setting. This ensures the LDAP management form reflects the enableNestedGroup value when rendering.
Rework LDAP nested-group handling and filter construction. Retrieve enableNestedGroup earlier and build distinct LDAP filters for Active Directory nested group matching using the memberOf recursion OID (1.2.840.113556.1.4.1941). Support multiple admin/user groups (explode/trim) and construct appropriate |-clauses via implode. Preserve non-nested legacy filter when nested matching is disabled. Overall cleanup removed the previous per-check early returns and centralized filter setup for admin and user group lookups.
Refactor LDAP plugin string concatenation for Active Directory nested-group filters in packages/web/lib/plugins/ldap/class/ldap.class.php. The change standardizes line breaks around concatenated filter parts, removes a redundant re-declaration of $grpMemAttr_forimplode, and cleans up whitespace for improved readability without altering behavior.
Escape the username when building LDAP search filters using $this->escape(..., LDAP_ESCAPE_FILTER) to prevent special-character injection. Also correct the sprintf templates and concatenation for AD nested group filters so the membership clauses are wrapped and combined properly (removes redundant parentheses and properly embeds the :1.2.840.113556.1.4.1941 matching rule). These changes improve security and correctness of LDAP/AD queries.
Replace dynamic $grpMemAttr usage with the literal 'memberOf' when building nested-group LDAP filters (using the 1.2.840.113556.1.4.1941 matching rule) to ensure correct Active Directory nested group matching. Also simplify a comment about checking userAccountControl after binding as the user.
Build correct LDAP filters for AD nested group membership by using name-based OR clauses and the memberOf matching rule with an escaped user DN; replaces previous string concatenation to avoid malformed filters. Removes a redundant unbind call. Also fix a casing typo in the LDAP management page (SearchDN -> searchDN) to use the correct property.
Replace duplicated if/else blocks for admin and user group filters with a single sprintf-based construction that conditionally appends the AD nested-group OID (:1.2.840.113556.1.4.1941:) when $enableNestedGroup is true. This simplifies the code by unifying filter generation for adminGroups and userGroups while preserving existing filter semantics and reusing $grpMemAttr_forimplode for group member concatenation.
eduardomozart
changed the title
eduardomozart
changed the title
Removed Active Directory userAccountControl checks for account status.
PHP: Add support for nested group matching by appending the MS_MATCHING_RULE_IN_CHAIN OID (:1.2.840.113556.1.4.1941:) to group membership attributes when enableNestedGroup is true. Remove the Active Directory userAccountControl check (ACCOUNTDISABLE) that previously blocked logins. JS: Adjust form handlers — stop using the select-based #useGroupMatch handler and switch to #groupmatch checkbox (passing its checked state). Include #nestedgroupmatch in the readonly/disabled toggling and enable/disable it appropriately. Minor cleanup: comment out e.preventDefault() calls so change events behave normally.
…/fogproject into ldap-nested-groups
Prevent toggling the nested group checkbox when fields are readonly and unify readonly handling across group fields. Added a click handler on #nestedgroupmatch to stop changes if it's readonly, removed usage of .prop('disabled') for that control, and updated ldapUseGroupToggle to set .prop('readonly') for all relevant inputs (including #nestedgroupmatch) while applying visual cues (accent-color and cursor) to indicate the disabled-like state.
Member
|
I'm trusting the testing of this. I don't know exactly what this is requesting (just being honest) but i appreciate all the effort you have put in thus far. Goign to allow it as based on the coding I've seen nothing is too problematic in my eyes. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces support for nested group matching in Active Directory and resolves two PHP errors that could occur during the LDAP authentication process.
Features & Enhancements:
LDAP_MATCHING_RULE_IN_CHAIN(1.2.840.113556.1.4.1941) OID. This allows FOG to properly authenticate users who are members of nested groups, rather than requiring direct membership.Bug Fixes
#useGroupMatchto#groupmatch), removed broken.optionsproperty calls on the checkbox, and removed an erroneouse.preventDefault()that was preventing the checkbox from visually updating. The dependent fields now properly gray out when disabled.ldap_unbind()on an already closed LDAP object results in a Fatal Error (Uncaught Error: LDAP connection has already been closed). Removed a redundant@$this->unbind();call inside_getAccessLevel()to prevent this crash when an authentication check fails.Undefined property: stdClass::$SearchDN) on line 114 ofldapmanagementpage.class.phpby correcting the property name to lowercase$searchDN.Testing Performed
Authentication was tested across multiple scenarios to ensure both backward compatibility and new functionality work as expected:
Related Error Logs Addressed
PHP Warning Fixed:
PHP Fatal Error Fixed: