Skip to content

Add recursive LDAP parent group search (AD-style hierarchy across all LDAPs)#4113

Merged
nabokihms merged 1 commit into
dexidp:masterfrom
Dex-Team-Search:NestedGSPrototype
Jun 21, 2025
Merged

Add recursive LDAP parent group search (AD-style hierarchy across all LDAPs)#4113
nabokihms merged 1 commit into
dexidp:masterfrom
Dex-Team-Search:NestedGSPrototype

Conversation

@EthanDieterich
Copy link
Copy Markdown
Contributor

@EthanDieterich EthanDieterich commented Apr 30, 2025

Overview

This pull request introduces improvements to Dex's LDAP connector to enable universal nested group search support across a variety of LDAP server implementations. It updates the code to allow recursive group membership discovery during user authentication and provides a more robust testing framework to validate the functionality.

This PR builds is largely based upon @paroque’s original #1058,

The groups() function in ldap.go was significantly refactored to add support for recursive nested group searching. Previously, the function directly built and executed LDAP search requests inline for each user matcher, then collected group names from the immediate search results. In the new version, the LDAP querying has been moved into a separate helper function called queryGroups(), which makes the main groups() function cleaner and easier to follow. A recursion flag and attribute have been added to the configuration. When recursion is enabled, after the initial group search, the function continues to search for parent groups using the specified recursion attribute. This process repeats until no new groups are found, allowing Dex to build a complete list of all direct and inherited group memberships. To prevent infinite loops and redundant results, the function checks for duplicate names and circular group references. If recursion is not enabled, the function simply returns the user’s direct group memberships, maintaining the original behavior.

To support per-matcher control over recursive searching, two new fields were added to the UserMatcher struct: Recursive and RecursionGroupAttr. The Recursive flag allows admins to specify whether recursion should be applied for a particular user matcher. The RecursionGroupAttr field lets them define which LDAP attribute should be used during the recursive lookup phase, allowing flexibility across different schemas. These assure that recursion is easily configurable and has broad compatibility.

To support the new recursive nested group functionality, a new unit test called TestNestedGroups was added. Using a new LDAP config and LDIF addition, this tests basic nested structures including a multi-level nesting and circular group references. It checks that users inherit all the correct group memberships through the recursive lookup process.

What this PR does / why we need it

Through extensive testing using both OpenLDAP and Apache Directory servers, we confirmed that Dex’s LDAP connector does not support nested group structures by default. While Dex could identify a user’s direct group memberships, it failed to recognize inherited group memberships through multi-level nesting, which are common in real-world LDAP environments. Although some systems like Microsoft Active Directory offer built-in nested group search (as cited in #1058 (comment)), many LDAP implementations do not, and we wanted to create a universal solution that would work across all implementations.

This PR introduces recursive group lookup functionality to Dex’s LDAP connector, allowing it to correctly discover and include all nested group memberships during authentication. Without this improvement for many LDAP implementations, users belonging to nested groups would not be authorized correctly, leading to access control issues for organizations relying on LDAP group hierarchies.

DexCITestingDiagram

Our CI testing demonstrates the functionality with a series of example groups on the left:

Without the PR:

  • John’s Groups: intermediateGroup
  • Jane’s Groups: childGroup

With the PR and enabling recursion:

  • John’s Groups: intermediateGroup, parentGroup
  • Jane’s Groups: childGroup, intermediateGroup, parentGroup

The PR makes it so that after Dex finds a user’s direct groups, it can keep searching for any parent groups they’re part of until all group memberships, direct or indirect, are found.

On the right, we’re testing a circular reference scenario to ensure that the search correctly identifies both users as members of both circular groups without introducing duplicates or causing an infinite loop.

Finally, it is worth noting two more things:

  1. We found that the AD-specific member:1.2.840.113556.1.4.1941: filter timed out after 2 minutes when searching for a
    Member’s groups in a real-world large corporate AD environment. Searching that same corporate AD for the same Member’s groups with Dex (including this PR’s functionality) takes 7 seconds, and correctly returns 300+ groups.

  2. In addition to running the CI tests with OpenLDAP, we manually tested this new Dex functionality with Apache DS, where it also performs correctly.

Overall, these changes make Dex’s LDAP connector more flexible, more reliable across different environments, and better suited for real-world deployments that depend on complex group hierarchies.

@EthanDieterich EthanDieterich marked this pull request as ready for review April 30, 2025 15:04
@EthanDieterich EthanDieterich marked this pull request as draft April 30, 2025 15:07
@jsquyres jsquyres force-pushed the NestedGSPrototype branch from b948b34 to 852397b Compare April 30, 2025 18:44
@EthanDieterich EthanDieterich marked this pull request as ready for review April 30, 2025 18:50
@jsquyres
Copy link
Copy Markdown

Maintainers: we had a last minute-shuffle on this PR. It's now ready to review. Thanks!

@EthanDieterich
Copy link
Copy Markdown
Contributor Author

Hello maintainers,
To accompany this feature PR, I've now also opened a documentation PR here: dexidp/website#196.

@jsquyres
Copy link
Copy Markdown

Ping; just checking in. Can someone review this PR? Thanks!

@jsquyres
Copy link
Copy Markdown

Periodic ping: we made this PR back on April 30. Can someone authorize the CI and give this a code review?

Thank you!

@nabokihms
Copy link
Copy Markdown
Member

@EthanDieterich The PR requires a rebase now, otherwise CI checks will not run

@nabokihms nabokihms self-requested a review June 19, 2025 18:30
Copy link
Copy Markdown
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

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

Some minor configuration suggestions, otherwise looks good enough. I appreciate your patience.

Comment thread connector/ldap/ldap.go Outdated
Comment thread connector/ldap/ldap.go Outdated
This commit enables universal nested group search support across a
variety of LDAP server implementations.  It updates the code to allow
recursive group membership discovery during user authentication and
provides CI tests to validate the functionality.

Based on @paroque’s original dexidp#1058
PR.

- Removed `Recursive` boolean flag from config and logic
- Made recursion behavior dependant on presence of `RecursionGroupAttr`
- Updated log messages to reflect changes and follow `slog` structured format

Signed-off-by: Ethan Dieterich <ethandieterich@gmail.com>
@jsquyres
Copy link
Copy Markdown

Looks like the CI failure is indicating that this PR needs some kind of label. Is release-note/enhancement appropriate? (especially when paired with the corresponding docs update PR dexidp/website#196)

Copy link
Copy Markdown
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

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

LGTM. Could you also update the doc PR as well? Thanks!

@nabokihms nabokihms merged commit 7208747 into dexidp:master Jun 21, 2025
9 of 10 checks passed
@jsquyres jsquyres deleted the NestedGSPrototype branch June 21, 2025 12:36
EthanDieterich added a commit to Dex-Team-Search/Dex-RecursiveSearch-Docs that referenced this pull request Jun 23, 2025
- Modified to align with final changes in dexidp/dex#4113
- Clarifies new behavior of `recursionGroupAttr` as well as the removal of `recursive` boolean

Signed-off-by: Ethan Dieterich <ethandieterich@gmail.com>
@as428y
Copy link
Copy Markdown

as428y commented Aug 26, 2025

Hi
Tried this feature,
Having a lot of groups, the performance is not as good as expected.
For me, it took ~60 seconds to iterate over all groups I'm in

Maybe the following points will help

Replaced O(n²) duplicate detection with O(1) map lookup
Added circular reference protection
Multiple string comparisons
query batching

Possibly having multi-threading will improve it

xtremerui pushed a commit to concourse/dex that referenced this pull request Sep 1, 2025
<!-- Release notes generated using configuration in .github/release.yml at v2.44.0 -->

## What's Changed
### Enhancements 🚀
* Allow server startup with partial connector failures by @manojVivek in dexidp#4159
* Add recursive LDAP parent group search (AD-style hierarchy across all LDAPs) by @EthanDieterich in dexidp#4113
* feat: Add ModifyGroupNames claimMutation to oidc connector by @peschmae in dexidp#4144
* authproxy connector: add support for specifying group header separator by @a-buck in dexidp#3745
* fix: join issuer URL with discovery path without extra slash after issuer URL by @vizv in dexidp#4263
* feat: grpc api list clients by @daemonfire300 in dexidp#4202
### Bug Fixes 🐛
* 🐛 remove extra method="get" from device-code template by @tuminoid in dexidp#4145
* [oidc] pass httpClient to the TokenIdentity context by @marriva in dexidp#4223
* Resolve CVE by updating gomplate to 4.3.3 by @philBrown in dexidp#4224
* fix: device code should not require scope by @cardoe in dexidp#4203
* fix: device code pending HTTP response by @cardoe in dexidp#4204
* Allow compilation without CGO by @nabokihms in dexidp#4266
### Dependency Updates ⬆️
* Resolve CVE by updating gomplate to 4.3.2 by @nathanlaceyraft in dexidp#4146
* build(deps): bump actions/attest-build-provenance from 2.3.0 to 2.4.0 by @dependabot[bot] in dexidp#4180
* build(deps): bump golang.org/x/net from 0.40.0 to 0.41.0 by @dependabot[bot] in dexidp#4171
* build(deps): bump google.golang.org/grpc from 1.72.1 to 1.73.0 in /examples by @dependabot[bot] in dexidp#4174
* build(deps): bump github/codeql-action from 3.28.18 to 3.29.0 by @dependabot[bot] in dexidp#4179
* build(deps): bump aquasecurity/trivy-action from 0.30.0 to 0.31.0 by @dependabot[bot] in dexidp#4167
* build(deps): bump ossf/scorecard-action from 2.4.1 to 2.4.2 by @dependabot[bot] in dexidp#4162
* build(deps): bump docker/build-push-action from 6.17.0 to 6.18.0 by @dependabot[bot] in dexidp#4155
* build(deps): bump distroless/static-debian12 from `188ddfb` to `627d6c5` by @dependabot[bot] in dexidp#4181
* build(deps): bump sigstore/cosign-installer from 3.8.2 to 3.9.0 by @dependabot[bot] in dexidp#4187
* build(deps): bump google.golang.org/api from 0.233.0 to 0.238.0 by @dependabot[bot] in dexidp#4186
* build(deps): bump docker/setup-buildx-action from 3.10.0 to 3.11.0 by @dependabot[bot] in dexidp#4185
* build(deps): bump anchore/sbom-action from 0.20.0 to 0.20.1 by @dependabot[bot] in dexidp#4184
* build(deps): bump github.com/go-sql-driver/mysql from 1.9.2 to 1.9.3 by @dependabot[bot] in dexidp#4183
* build(deps): bump the etcd group with 2 updates by @dependabot[bot] in dexidp#4175
* build(deps): bump alpine from 3.21.3 to 3.22.0 by @dependabot[bot] in dexidp#4163
* build(deps): bump google.golang.org/grpc from 1.72.1 to 1.73.0 in /api/v2 by @dependabot[bot] in dexidp#4170
* build(deps): bump docker/setup-buildx-action from 3.11.0 to 3.11.1 by @dependabot[bot] in dexidp#4189
* build(deps): bump mheap/github-action-required-labels from 5.5.0 to 5.5.1 by @dependabot[bot] in dexidp#4190
* build(deps): bump google.golang.org/grpc from 1.73.0 to 1.74.0 in /examples by @dependabot[bot] in dexidp#4219
* build(deps): bump anchore/sbom-action from 0.20.1 to 0.20.2 by @dependabot[bot] in dexidp#4205
* build(deps): bump sigstore/cosign-installer from 3.9.0 to 3.9.2 by @dependabot[bot] in dexidp#4225
* build(deps): bump aquasecurity/trivy-action from 0.31.0 to 0.32.0 by @dependabot[bot] in dexidp#4210
* build(deps): bump github.com/go-jose/go-jose/v4 from 4.1.0 to 4.1.1 by @dependabot[bot] in dexidp#4196
* build(deps): bump golang.org/x/crypto from 0.39.0 to 0.40.0 by @dependabot[bot] in dexidp#4214
* build(deps): bump alpine from 3.22.0 to 3.22.1 by @dependabot[bot] in dexidp#4217
* build(deps): bump the etcd group with 2 updates by @dependabot[bot] in dexidp#4213
* build(deps): bump github.com/oklog/run from 1.1.0 to 1.2.0 by @dependabot[bot] in dexidp#4199
* build(deps): bump github.com/mattn/go-sqlite3 from 1.14.28 to 1.14.29 by @dependabot[bot] in dexidp#4239
* build(deps): bump github/codeql-action from 3.29.0 to 3.29.4 by @dependabot[bot] in dexidp#4238
* build(deps): bump google.golang.org/grpc from 1.73.0 to 1.74.2 by @dependabot[bot] in dexidp#4235
* build(deps): bump the etcd group with 2 updates by @dependabot[bot] in dexidp#4233
* build(deps): bump google.golang.org/grpc from 1.74.0 to 1.74.2 in /examples by @dependabot[bot] in dexidp#4232
* build(deps): bump anchore/sbom-action from 0.20.2 to 0.20.4 by @dependabot[bot] in dexidp#4229
* build(deps): bump google.golang.org/api from 0.238.0 to 0.243.0 by @dependabot[bot] in dexidp#4234
* build(deps): bump golang.org/x/net from 0.41.0 to 0.42.0 by @dependabot[bot] in dexidp#4227
* build(deps): bump github.com/coreos/go-oidc/v3 from 3.14.1 to 3.15.0 in /examples by @dependabot[bot] in dexidp#4246
* build(deps): bump the etcd group with 2 updates by @dependabot[bot] in dexidp#4243
* build(deps): bump distroless/static-debian12 from `627d6c5` to `cdf4daa` by @dependabot[bot] in dexidp#4253
* build(deps): bump docker/metadata-action from 5.7.0 to 5.8.0 by @dependabot[bot] in dexidp#4252
* build(deps): bump github.com/mattn/go-sqlite3 from 1.14.29 to 1.14.30 by @dependabot[bot] in dexidp#4249
* build(deps): bump github/codeql-action from 3.29.4 to 3.29.5 by @dependabot[bot] in dexidp#4244
* build(deps): bump google.golang.org/api from 0.243.0 to 0.244.0 by @dependabot[bot] in dexidp#4247
* build(deps): bump docker/login-action from 3.4.0 to 3.5.0 by @dependabot[bot] in dexidp#4254
* build(deps): bump github.com/go-jose/go-jose/v4 from 4.1.1 to 4.1.2 by @dependabot[bot] in dexidp#4255
* build(deps): bump github.com/prometheus/client_golang from 1.22.0 to 1.23.0 by @dependabot[bot] in dexidp#4257
* build(deps): bump google.golang.org/api from 0.244.0 to 0.246.0 by @dependabot[bot] in dexidp#4258
* build(deps): bump anchore/sbom-action from 0.20.4 to 0.20.5 by @dependabot[bot] in dexidp#4272
* build(deps): bump github.com/mattn/go-sqlite3 from 1.14.30 to 1.14.32 by @dependabot[bot] in dexidp#4271
* build(deps): bump github/codeql-action from 3.29.7 to 3.29.9 by @dependabot[bot] in dexidp#4269
* build(deps): bump actions/checkout from 4.2.2 to 5.0.0 by @dependabot[bot] in dexidp#4267
* build(deps): bump golang.org/x/net from 0.42.0 to 0.43.0 by @dependabot[bot] in dexidp#4262
* build(deps): bump actions/cache from 4.2.3 to 4.2.4 by @dependabot[bot] in dexidp#4261
* build(deps): bump cloud.google.com/go/compute/metadata from 0.7.0 to 0.8.0 by @dependabot[bot] in dexidp#4259
* build(deps): bump google.golang.org/protobuf from 1.36.6 to 1.36.7 by @dependabot[bot] in dexidp#4275
* build(deps): bump google.golang.org/api from 0.246.0 to 0.247.0 by @dependabot[bot] in dexidp#4274
* chore: update ent generated code by @sagikazarmark in dexidp#4276
* feat: update Go to 1.25 by @sagikazarmark in dexidp#4277
* build(deps): bump google.golang.org/grpc from 1.73.0 to 1.74.2 in /api/v2 by @dependabot[bot] in dexidp#4236
* build(deps): bump google.golang.org/protobuf from 1.36.6 to 1.36.7 in /api/v2 by @dependabot[bot] in dexidp#4260
* build(deps): bump actions/dependency-review-action from 4.7.1 to 4.7.2 by @dependabot[bot] in dexidp#4278
* build(deps): bump distroless/static-debian12 from `cdf4daa` to `a9f88e0` by @dependabot[bot] in dexidp#4291
* build(deps): bump github.com/beevik/etree from 1.5.1 to 1.6.0 by @dependabot[bot] in dexidp#4288
* build(deps): bump github/codeql-action from 3.29.9 to 3.29.11 by @dependabot[bot] in dexidp#4287
* build(deps): bump google.golang.org/grpc from 1.74.2 to 1.75.0 in /examples by @dependabot[bot] in dexidp#4282
* build(deps): bump google.golang.org/grpc from 1.74.2 to 1.75.0 by @dependabot[bot] in dexidp#4281
* build(deps): bump google.golang.org/grpc from 1.74.2 to 1.75.0 in /api/v2 by @dependabot[bot] in dexidp#4280
* build(deps): bump google.golang.org/api from 0.247.0 to 0.248.0 by @dependabot[bot] in dexidp#4283
* build(deps): bump google.golang.org/protobuf from 1.36.7 to 1.36.8 by @dependabot[bot] in dexidp#4285
* build(deps): bump actions/dependency-review-action from 4.7.2 to 4.7.3 by @dependabot[bot] in dexidp#4290
* build(deps): bump google.golang.org/protobuf from 1.36.7 to 1.36.8 in /api/v2 by @dependabot[bot] in dexidp#4286
* build(deps): bump actions/attest-build-provenance from 2.4.0 to 3.0.0 by @dependabot[bot] in dexidp#4296
* build(deps): bump aquasecurity/trivy-action from 0.32.0 to 0.33.0 by @dependabot[bot] in dexidp#4293
* build(deps): bump github.com/stretchr/testify from 1.10.0 to 1.11.1 by @dependabot[bot] in dexidp#4292

## New Contributors
* @nathanlaceyraft made their first contribution in dexidp#4146
* @manojVivek made their first contribution in dexidp#4159
* @marriva made their first contribution in dexidp#4223
* @peschmae made their first contribution in dexidp#4144
* @philBrown made their first contribution in dexidp#4224
* @a-buck made their first contribution in dexidp#3745
* @vizv made their first contribution in dexidp#4263
* @daemonfire300 made their first contribution in dexidp#4202

**Full Changelog**: dexidp/dex@v2.43.0...v2.44.0
yardenshoham pushed a commit to yardenshoham/dex that referenced this pull request Jan 29, 2026
This commit enables universal nested group search support across a
variety of LDAP server implementations.  It updates the code to allow
recursive group membership discovery during user authentication and
provides CI tests to validate the functionality.

Based on @paroque’s original dexidp#1058
PR.

- Removed `Recursive` boolean flag from config and logic
- Made recursion behavior dependant on presence of `RecursionGroupAttr`
- Updated log messages to reflect changes and follow `slog` structured format

Signed-off-by: Ethan Dieterich <ethandieterich@gmail.com>
slrz pushed a commit to hdacloud/dex that referenced this pull request Feb 6, 2026
This commit enables universal nested group search support across a
variety of LDAP server implementations.  It updates the code to allow
recursive group membership discovery during user authentication and
provides CI tests to validate the functionality.

Based on @paroque’s original dexidp#1058
PR.

- Removed `Recursive` boolean flag from config and logic
- Made recursion behavior dependant on presence of `RecursionGroupAttr`
- Updated log messages to reflect changes and follow `slog` structured format

Signed-off-by: Ethan Dieterich <ethandieterich@gmail.com>
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.

4 participants