jenkinsci / jenkinsci/active-directory-plugin
[JENKINS-36157] recurisve group lookup is missing groups.
- Dominant language
- Java
- Stars
- 53
- Forks
- 115
- PR merge metrics
- No merged PRs in 30d
Description
AD 1.47
The implementation for [recursive group lookup | https://github.com/jenkinsci/active-directory-plugin/blob/c04fc160d7cbf4f616109190ac3df7dcc1a02970/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java#L630-L632
] is buggy and does not return all groups that a user is a member of.
The code will only add the group to the queue to do recursive lookup if it is not already in the granted authorities.
However, the groups set is pre-populated with all group membership of "Security Groups" via the tokenGroups lookup.
Thus if user "A" is a member of Security Group "SG1" and "SG1" is a member of Distribution Group "DG1" then A is granted "SG1" role but not "DG1"
Fixing this is highly likely to have a performance impact - and many users may not even care about distribution group membership - so when fixing this I would also suggest a "SecurityGroups Only" recursive lookup method that does no recursion whatsoever and just uses whatever is returned by tokenGroups
---
Originally reported by
teilo, imported from: recurisve group lookup is missing groups.
fbelzunc
Raw content of original issue
AD 1.47
The implementation for [recursive group lookup | https://github.com/jenkinsci/active-directory-plugin/blob/c04fc160d7cbf4f616109190ac3df7dcc1a02970/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java#L630-L632
] is buggy and does not return all groups that a user is a member of.The code will only add the group to the queue to do recursive lookup if it is not already in the granted authorities.
However, the groups set is pre-populated with all group membership of "Security Groups" via the tokenGroups lookup.
Thus if user "A" is a member of Security Group "SG1" and "SG1" is a member of Distribution Group "DG1" then A is granted "SG1" role but not "DG1"
Fixing this is highly likely to have a performance impact - and many users may not even care about distribution group membership - so when fixing this I would also suggest a "SecurityGroups Only" recursive lookup method that does no recursion whatsoever and just uses whatever is returned by tokenGroups
Contributor guide
Research direction
Start in src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java, reviewing the recursive group lookup around lines 630-632 and the tokenGroups lookup around lines 485-512. Reproduce the SG1-to-DG1 membership case and assess how recursive lookup should include distribution groups without unnecessarily affecting security-group-only behavior. Done means the missing group is returned and the proposed security-groups-only behavior is addressed or clarified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- authentication
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100