Skip to content

Also find groups added through groupOfUniqueNames #78

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.

Sign up for GitHub

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

Closed
adamruzicka wants to merge 2 commits into from
Closed

Also find groups added through groupOfUniqueNames #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0667edb
Also find groups added through groupOfUniqueNames
adamruzicka Apr 8, 2024
27f6f9d
Look up user before looking for groups
adamruzicka Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/ldap_fluff/posix_member_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ def find_user(uid, base_dn = @base)
# return an ldap user with groups attached
# note : this method is not particularly fast for large ldap systems
def find_user_groups(uid)
Copy link
Member

@ekohl ekohl Apr 9, 2024

Choose a reason for hiding this comment

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

I was digging deeper on what should be done for POSIX. If we assume this maps to the nss-pam-ldap implementation then according to https://arthurdejong.org/nss-pam-ldapd/README this is the spec:

Currently, two ways of specifying group membership are supported. The first, by using the memberUid attribute, is the simplest and by far the fastest (takes the least number of lookups). The attribute values are user names (same as the uid attribute for posixAccount entries) and are returned without further processing.

The second method is to use DN values in the member attribute (attribute names can be changed by using the attribute mapping options as described in the manual page). This is potentially a lot slower because in the worst case every DN has to be looked up in the LDAP server to find the proper value for the uid attribute.

If the LDAP server supports the deref control (provided by the deref overlay in OpenLDAP) the DN to uid expansing is performed by the LDAP server.

If the DN value already contains a uid value (e.g. uid=arthur, dc=example, dc=com) a further lookup is skipped and the uid value from the DN is used.

For other DN values an extra lookup is performed to expand it to a uid. These lookups are cached and are configurable with the cache dn2uid configuration option.

The member attribute may also contain the DN of another group entry. These nested groups are parsed recursively depending on the nss_nested_groups option.

Currently, the memberOf attribute in posixAccount entries is unsupported.

If we look at the method, then we see there is already a constructor parameter that determines @attr_login, which is not respected right now.

Reading the documentation it's not clear to me if memberUid or member is an XOR, but it looks like groupOfUniqueNames is not part of the posix spec at all.

In #72 (comment) I already noted that I think it's a different flavor we should support. Most likely an expansion of the FreeIPA flavor.

Copy link
Contributor Author

@adamruzicka adamruzicka May 9, 2024

Choose a reason for hiding this comment

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

How about instead of exposing this as a brand new flavor, we allow enabling the use of groupOfNames/groupOfUniqueNames in a similar way how we handle nis netgroups? Something along the lines of adamruzicka/ldap_fluff@unique-group-search...adamruzicka:ldap_fluff:unique-group-search-ext

Copy link
Member

@ekohl ekohl Aug 29, 2024

Choose a reason for hiding this comment

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

Looking at the code I think that could work. For LdapFluff::Posix::MemberService I was debating a subclass but it looks like it's not strictly needed.

user = find_user(uid).first
Copy link
Member

@ekohl ekohl Apr 9, 2024

Choose a reason for hiding this comment

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

Should this somehow yell if there's more than 1 user?

Copy link
Contributor Author

@adamruzicka adamruzicka Apr 9, 2024

Choose a reason for hiding this comment

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

Can it happen? I was living under the impression that the uid we get should uniquely identify a single object within the subtree set by @base

Copy link
Member

@ekohl ekohl Apr 9, 2024

Choose a reason for hiding this comment

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

It wouldn't be a good practice, but if you organize your LDAP tree with different organization units it can happen.

Concrete example: let's say your base is dc=example,dc=com you can have cn=myuid,ou=People,dc=example,dc=com and cn=myuid,ou=Systems,dc=example,dc=com.

Copy link

@lhellebr lhellebr Apr 9, 2024

Choose a reason for hiding this comment

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

There can be multiple entries with the same uid (as opposed to dn)

groups = []
@ldap.search(
:filter => Net::LDAP::Filter.eq('memberuid', uid),
:filter => user_group_filter(uid, user[:dn].first),
:base => @group_base, :attributes => ["cn"]
).each do |entry|
groups << entry[:cn][0]
end
groups
rescue UIDNotFoundException
return []
end

def times_in_groups(uid, gids, all)
Expand Down Expand Up @@ -52,4 +55,12 @@ class UIDNotFoundException < LdapFluff::Error

class GIDNotFoundException < LdapFluff::Error
end

private

def user_group_filter(uid, user_dn)
unique_filter = Net::LDAP::Filter.eq('uniquemember', user_dn) &
Net::LDAP::Filter.eq('objectClass', 'groupOfUniqueNames')
Copy link
Member

@ekohl ekohl Apr 9, 2024

Choose a reason for hiding this comment

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

There's also groupOfNames. Should that also be supported? We search for it here:

ldap_fluff/lib/ldap_fluff/posix.rb

Line 24 in 77a387a

Net::LDAP::Filter.eq('objectClass', 'groupOfUniqueNames') |

Copy link

@lhellebr lhellebr Apr 9, 2024

Choose a reason for hiding this comment

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

There are multiple group-like object types in LDAP. I think it's a high level decision what we support (and document!)

Net::LDAP::Filter.eq('memberuid', uid) | unique_filter
end
end
2 changes: 1 addition & 1 deletion test/lib/ldap_test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def netiq_group_payload
end

def posix_user_payload
[{ :cn => ["john"] }]
[{ :cn => ["john"], :dn => ["cn=john,ou=people,dc=internet,dc=com"] }]
end

def posix_group_payload
Expand Down
21 changes: 16 additions & 5 deletions test/posix_member_services_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,32 @@ def test_find_user
end

def test_find_user_groups
user = posix_group_payload
@ldap.expect(:search, user, [:filter => @ms.name_filter('john'),
group = posix_group_payload
user = posix_user_payload
username = 'john'

@ldap.expect(:search, user, [:filter => @ms.name_filter(username),
:base => config.base_dn])
filter = @ms.send(:user_group_filter, username, user.first[:dn].first)
@ldap.expect(:search, group, [:filter => filter,
:base => config.group_base,
:attributes => ["cn"]])
@ms.ldap = @ldap
assert_equal ['broze'], @ms.find_user_groups('john')
assert_equal ['broze'], @ms.find_user_groups(username)
@ldap.verify
end

def test_find_no_groups
@ldap.expect(:search, [], [:filter => @ms.name_filter("john"),
user = posix_user_payload
username = 'john'
@ldap.expect(:search, user, [:filter => @ms.name_filter(username),
:base => config.base_dn])
filter = @ms.send(:user_group_filter, username, user.first[:dn].first)
@ldap.expect(:search, [], [:filter => filter,
:base => config.group_base,
:attributes => ["cn"]])
@ms.ldap = @ldap
assert_equal [], @ms.find_user_groups('john')
assert_equal [], @ms.find_user_groups(username)
@ldap.verify
end

Expand Down