Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lookup groups using group->gr_mem even if getgrouplist() failed #5613

Closed
wants to merge 1 commit into from

Conversation

manu0401
Copy link

getgrouplist() iterate on each group, searching if the user
is included, while group->gr_mem iterated on user list for
the target group. In some case the former method can miss
the result while the later will succeed. This is the case
for OpenLDAP dyngroup, where the group members are automatically
generated on group search. Searching by group member gives no
result, while searching the member of a given groups is fine.

getgrouplist() iterate on each group, searching if the user
is included, while group->gr_mem iterated on user list for
the target group. In some case the former method can miss
the result while the later will succeed. This is the case
for OpenLDAP dyngroup, where the group members are automatically
generated on group search. Searching by group member gives no
result, while searching the member of a given groups is fine.
@michaelrsweet
Copy link
Collaborator

Will consider this, although I am definitely not super happy about the inconsistency...

@michaelrsweet
Copy link
Collaborator

I reworked this change to move the membership list check up front, followed by the getgrouplist check. We already have the list of usernames that are members, so this made more sense than checking after the (semi-expensive) getgrouplist check.

[master 3c27d2a] Always check the group membership list (Issue #5613)

[branch-2.2 666d076] Always check the group membership list (Issue #5613)

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