LdapAdapter: Keep track of already processed groups to account for circular references #38817
Conversation
This file contains 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
…rcular references
ghost
added
area-auth
RoadTrain
commented
Dec 3, 2021
| @@ -109,18 +111,26 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr | |||
| logger.LogWarning($"More than one response received for query: {filter} with distinguished name: {distinguishedName}"); | |||
| } | |||
|
|
|||
| var group = searchResponse.Entries[0]; //Get the object that was found on ldap | |||
| string name = group.DistinguishedName; | |||
| retrievedClaims.Add(name); | |||
There was a problem hiding this comment.
In general, we use a group Common Name (CN, like SomeGroup) as a claim name. In this particular instance, a Distinguished Name (DN, like CN=SomeGroup,DC=domain,DC=com) was used instead, which I think is incorrect.
Possibly a typo, was introduced in #25075
1 task
Tratcher
approved these changes
Dec 9, 2021
|
@RoadTrain @macsux, this should show up in our 7.0 rolling builds by next week. Can you download and verify the fix from there? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 fixes a particular corner case which is possible in real-world AD deployments, namely circular group references.
That is, e.g.
GroupA -> GroupB -> GroupC -> GroupA. This is a problematic scenario for any software working with AD.The fix is basically to keep track of groups that we already processed by recursively passing a
HashSetcontaining already processed entries.Fixes #37225
Fixes #38769