Skip to content

AD/LDAP team-sync: supporting method readEntryByDn - #26947

Open
patrickmann wants to merge 6 commits into
masterfrom
ldap-team-sync-membership-flag
Open

AD/LDAP team-sync: supporting method readEntryByDn#26947
patrickmann wants to merge 6 commits into
masterfrom
ldap-team-sync-membership-flag

Conversation

@patrickmann

@patrickmann patrickmann commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

/nocl No user-facing behaviour on its own; changelog is in the linked enterprise PR

Description

Adds UnboundLDAPConnector.readEntryByDn, needed by Graylog2/graylog-plugin-enterprise#15155.

Both existing search methods are SearchScope.SUB from a caller-supplied base, so a caller that follows a DN reference out of its own search base cannot read that entry at all. AD memberOf nesting does exactly that: a chain can pass through a group in another OU.

readEntryByDn reads at the DN itself with SearchScope.BASE. noSuchObject and referral come back as an empty Optional rather than an exception - a stale memberOf back-link pointing at a deleted group, or a DN in a domain this connection cannot follow, is a dead end for the caller, not a reason to fail the login it is running under.

The enterprise side uses it to fix org-wide AD reconciliation dropping team memberships reachable only through a group outside the configured group search base. Details and the live repro are in the linked PR. Nothing in this repo calls it yet, so there is no behaviour change here on its own.

How Tested

  • UnboundLDAPConnectorTest: 9 tests, 0 failures against the in-process ApacheDS, including two new cases

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have requested a documentation update.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

patrickmann and others added 6 commits August 10, 2026 16:55
Declares ldap_team_sync_org_wide_membership off, which selects the pre-7.1.2
per-user membership behaviour: only the logging-in user's synced-team memberships
are updated, instead of reconciling every user on the backend org-wide on every
login. The org-wide read is proportional to the number of Graylog users on the
backend and is what makes logins slow on large AD/LDAP directories.

The value here is the whole per-release-line difference: 7.1 declares it on so a
patch release does not change behaviour under customers, while master takes the
change for 8.0. The flag is read by graylog-plugin-enterprise.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The org-wide behaviour also shipped on the 7.0 line, in 7.0.7 - 7.0.12
(git tag --contains 49faf87881); 7.1.0 and 7.1.1 do not have it. Describing it as
"the 7.1.2 - 7.1.7 behaviour" told 7.0 customers this did not concern them.

Also drops "and the default here". This file ships to customers and the 7.1
backport declares the flag on, where that parenthetical would have said the exact
opposite of the value two lines below it.

Spells out what off actually means for revocation: it waits for the affected
user's own password login, and an account that only authenticates with an API
token is never revoked, because sessions and tokens do not re-provision and there
is no periodical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both search methods here are SearchScope.SUB from a caller-supplied base, so a
caller that follows a DN reference out of its own search base cannot read the
entry at all. AD memberOf nesting does exactly that: a chain can pass through a
group in another OU, and the enterprise team-sync resolver needs that group's
edges to keep walking.

readEntryByDn reads at the DN itself with SearchScope.BASE. noSuchObject and
referral come back as an empty Optional rather than an exception: a stale
memberOf back-link pointing at a deleted group, or a DN in a domain this
connection cannot follow, is a dead end for the caller, not a reason to fail the
login it is running under.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The membership mode is now selected by an enterprise cluster-config setting
(TeamSyncMembershipConfig) rather than a feature flag, so remove the
ldap_team_sync_org_wide_membership entry from feature-flag.config. The
readEntryByDn addition stays: the enterprise base-scoping fix depends on it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@patrickmann patrickmann changed the title Declare the AD/LDAP team-sync membership feature flag Add UnboundLDAPConnector.readEntryByDn for out-of-base AD group nesting Aug 12, 2026
@patrickmann patrickmann changed the title Add UnboundLDAPConnector.readEntryByDn for out-of-base AD group nesting AD/LDAP team-sync: supporting method readEntryByDn Aug 12, 2026
@patrickmann
patrickmann marked this pull request as ready for review August 13, 2026 07:03
@patrickmann
patrickmann requested a review from a team August 13, 2026 07:03
@patrickmann
patrickmann requested review from kodjo-anipah and removed request for a team August 13, 2026 09:39

@kodjo-anipah kodjo-anipah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM good catch

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.

2 participants