From 107437da3a48e34adb71f9a7aafb3b291549c9bd Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sat, 29 Nov 2014 15:41:00 -0800 Subject: [PATCH 1/5] Add GitHub::Ldap::MemberOf search strategies --- lib/github/ldap.rb | 1 + lib/github/ldap/member_of.rb | 22 +++++++++ lib/github/ldap/member_of/classic.rb | 46 ++++++++++++++++++ lib/github/ldap/member_of/recursive.rb | 64 ++++++++++++++++++++++++++ test/member_of/classic_test.rb | 26 +++++++++++ test/member_of/recursive_test.rb | 25 ++++++++++ 6 files changed, 184 insertions(+) create mode 100644 lib/github/ldap/member_of.rb create mode 100644 lib/github/ldap/member_of/classic.rb create mode 100644 lib/github/ldap/member_of/recursive.rb create mode 100644 test/member_of/classic_test.rb create mode 100644 test/member_of/recursive_test.rb diff --git a/lib/github/ldap.rb b/lib/github/ldap.rb index 43c3f3b..e6ab060 100644 --- a/lib/github/ldap.rb +++ b/lib/github/ldap.rb @@ -9,6 +9,7 @@ class Ldap require 'github/ldap/virtual_group' require 'github/ldap/virtual_attributes' require 'github/ldap/instrumentation' + require 'github/ldap/member_of' require 'github/ldap/members' require 'github/ldap/membership_validators' diff --git a/lib/github/ldap/member_of.rb b/lib/github/ldap/member_of.rb new file mode 100644 index 0000000..05fb1d8 --- /dev/null +++ b/lib/github/ldap/member_of.rb @@ -0,0 +1,22 @@ +require 'github/ldap/member_of/classic' +require 'github/ldap/member_of/recursive' + +module GitHub + class Ldap + # Provides various strategies to search for groups a user is a member of. + # + # For example: + # + # group = domain.groups(%w(Engineering)).first + # strategy = GitHub::Ldap::MemberOf::Recursive.new(ldap) + # strategy.perform(user) #=> [#] + # + module MemberOf + # Internal: Mapping of strategy name to class. + STRATEGIES = { + :classic => GitHub::Ldap::MemberOf::Classic, + :recursive => GitHub::Ldap::MemberOf::Recursive + } + end + end +end diff --git a/lib/github/ldap/member_of/classic.rb b/lib/github/ldap/member_of/classic.rb new file mode 100644 index 0000000..60dc88c --- /dev/null +++ b/lib/github/ldap/member_of/classic.rb @@ -0,0 +1,46 @@ +module GitHub + class Ldap + module MemberOf + # Look up the groups an entry is a member of. + class Classic + include Filter + + # Internal: The GitHub::Ldap object to search domains with. + attr_reader :ldap + + # Public: Instantiate new search strategy. + # + # - ldap: GitHub::Ldap object + # - options: Hash of options (unused) + def initialize(ldap, options = {}) + @ldap = ldap + @options = options + end + + # Public: Performs search for groups an entry is a member of, including + # subgroups. + # + # Returns Array of Net::LDAP::Entry objects. + def perform(entry) + filter = member_filter(entry) + + if ldap.posix_support_enabled? && !entry[ldap.uid].empty? + filter |= posix_member_filter(entry, ldap.uid) + end + + domains.each_with_object([]) do |domain, entries| + entries.concat domain.search(filter: filter) + end + end + + # Internal: Domains to search through. + # + # Returns an Array of GitHub::Ldap::Domain objects. + def domains + @domains ||= ldap.search_domains.map { |base| ldap.domain(base) } + end + private :domains + end + end + end +end diff --git a/lib/github/ldap/member_of/recursive.rb b/lib/github/ldap/member_of/recursive.rb new file mode 100644 index 0000000..7505a60 --- /dev/null +++ b/lib/github/ldap/member_of/recursive.rb @@ -0,0 +1,64 @@ +module GitHub + class Ldap + module MemberOf + # Look up the groups an entry is a member of, including nested subgroups. + # + # NOTE: this strategy is network and performance intensive. + class Recursive + include Filter + + # Internal: The GitHub::Ldap object to search domains with. + attr_reader :ldap + + # Public: Instantiate new search strategy. + # + # - ldap: GitHub::Ldap object + # - options: Hash of options (unused) + def initialize(ldap, options = {}) + @ldap = ldap + @options = options + end + + # Public: Performs search for groups an entry is a member of, including + # subgroups. + # + # Returns Array of Net::LDAP::Entry objects. + def perform(entry) + filter = member_filter(entry) + + if ldap.posix_support_enabled? && !entry[ldap.uid].empty? + filter |= posix_member_filter(entry, ldap.uid) + end + + entries = domains.each_with_object([]) do |domain, entries| + entries.concat domain.search(filter: filter) + end + + entries.each_with_object(entries.dup) do |entry, entries| + entries.concat search_strategy.perform(entry) + end.select { |entry| group?(entry) } + end + + # Internal: Domains to search through. + # + # Returns an Array of GitHub::Ldap::Domain objects. + def domains + @domains ||= ldap.search_domains.map { |base| ldap.domain(base) } + end + private :domains + + # Internal: The search strategy to recursively search for nested + # subgroups with. + def search_strategy + @search_strategy ||= + GitHub::Ldap::Members::Recursive.new(ldap, attrs: %w(objectClass)) + end + + # Internal: Returns true if the entry is a group. + def group?(entry) + GitHub::Ldap::Group.group?(entry[:objectclass]) + end + end + end + end +end diff --git a/test/member_of/classic_test.rb b/test/member_of/classic_test.rb new file mode 100644 index 0000000..896f939 --- /dev/null +++ b/test/member_of/classic_test.rb @@ -0,0 +1,26 @@ +require_relative '../test_helper' + +class GitHubLdapClassicMemberOfTest < GitHub::Ldap::Test + def setup + @ldap = GitHub::Ldap.new(options.merge(search_domains: %w(dc=github,dc=com))) + @domain = @ldap.domain("dc=github,dc=com") + @entry = @domain.user?('user1') + @strategy = GitHub::Ldap::MemberOf::Classic.new(@ldap) + end + + def find_group(cn) + @domain.groups([cn]).first + end + + def test_finds_groups_entry_is_a_member_of + member_of = @strategy.perform(@entry) + assert_includes member_of.map(&:dn), find_group("nested-group1").dn + end + + def test_finds_subgroups_entry_is_a_member_of + skip "Classic strategy does not support nested subgroups" + member_of = @strategy.perform(@entry) + assert_includes member_of.map(&:dn), find_group("head-group").dn + assert_includes member_of.map(&:dn), find_group("tail-group").dn + end +end diff --git a/test/member_of/recursive_test.rb b/test/member_of/recursive_test.rb new file mode 100644 index 0000000..4c910dd --- /dev/null +++ b/test/member_of/recursive_test.rb @@ -0,0 +1,25 @@ +require_relative '../test_helper' + +class GitHubLdapRecursiveMemberOfTest < GitHub::Ldap::Test + def setup + @ldap = GitHub::Ldap.new(options.merge(search_domains: %w(dc=github,dc=com))) + @domain = @ldap.domain("dc=github,dc=com") + @entry = @domain.user?('user1') + @strategy = GitHub::Ldap::MemberOf::Recursive.new(@ldap) + end + + def find_group(cn) + @domain.groups([cn]).first + end + + def test_finds_groups_entry_is_a_member_of + member_of = @strategy.perform(@entry) + assert_includes member_of.map(&:dn), find_group("nested-group1").dn + end + + def test_finds_subgroups_entry_is_a_member_of + member_of = @strategy.perform(@entry) + assert_includes member_of.map(&:dn), find_group("head-group").dn + assert_includes member_of.map(&:dn), find_group("tail-group").dn + end +end From 765c8b591e7874a73f3a83d4856ede3b84cd8490 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sat, 29 Nov 2014 15:41:22 -0800 Subject: [PATCH 2/5] Expose Group.group? for testing if entry is group --- lib/github/ldap/group.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/github/ldap/group.rb b/lib/github/ldap/group.rb index 25066a0..633e034 100644 --- a/lib/github/ldap/group.rb +++ b/lib/github/ldap/group.rb @@ -69,6 +69,11 @@ def member_names end end + # Internal: Returns true if the object class(es) provided match a group's. + def group?(object_class) + self.class.group?(object_class) + end + # Internal - Check if an object class includes the member names # Use `&` rathen than `include?` because both are arrays. # @@ -76,7 +81,7 @@ def member_names # will fail to match correctly unless we also downcase our group classes. # # Returns true if the object class includes one of the group class names. - def group?(object_class) + def self.group?(object_class) !(GROUP_CLASS_NAMES.map(&:downcase) & object_class.map(&:downcase)).empty? end From 6942a499ef66a614618fde207f74c19197e2a217 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Sat, 29 Nov 2014 15:42:17 -0800 Subject: [PATCH 3/5] Take attrs option for Member search strategy --- lib/github/ldap/members/recursive.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/github/ldap/members/recursive.rb b/lib/github/ldap/members/recursive.rb index 867dda0..9b0becc 100644 --- a/lib/github/ldap/members/recursive.rb +++ b/lib/github/ldap/members/recursive.rb @@ -9,7 +9,7 @@ class Recursive include Filter DEFAULT_MAX_DEPTH = 9 - ATTRS = %w(member uniqueMember memberUid) + DEFAULT_ATTRS = %w(member uniqueMember memberUid) # Internal: The GitHub::Ldap object to search domains with. attr_reader :ldap @@ -17,6 +17,9 @@ class Recursive # Internal: The maximum depth to search for members. attr_reader :depth + # Internal: The attributes to search for. + attr_reader :attrs + # Public: Instantiate new search strategy. # # - ldap: GitHub::Ldap object @@ -25,6 +28,7 @@ def initialize(ldap, options = {}) @ldap = ldap @options = options @depth = options[:depth] || DEFAULT_MAX_DEPTH + @attrs = Array(options[:attrs]).concat DEFAULT_ATTRS end # Public: Performs search for group members, including groups and @@ -95,7 +99,7 @@ def member_entries(entry) # Returns an Array of Net::LDAP::Entry objects. def entries_by_dn(members) members.map do |dn| - ldap.domain(dn).bind(attributes: ATTRS) + ldap.domain(dn).bind(attributes: attrs) end.compact end private :entries_by_dn @@ -106,7 +110,7 @@ def entries_by_dn(members) def entries_by_uid(members) filter = members.map { |uid| Net::LDAP::Filter.eq(ldap.uid, uid) }.reduce(:|) domains.each_with_object([]) do |domain, entries| - entries.concat domain.search(filter: filter, attributes: ATTRS) + entries.concat domain.search(filter: filter, attributes: attrs) end.compact end private :entries_by_uid From db248e10730b8947d343da50bf8c33b855abc1d8 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Mon, 1 Dec 2014 13:36:01 -0800 Subject: [PATCH 4/5] Accept optional depth setting --- lib/github/ldap/member_of/recursive.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/github/ldap/member_of/recursive.rb b/lib/github/ldap/member_of/recursive.rb index 7505a60..0bb6a80 100644 --- a/lib/github/ldap/member_of/recursive.rb +++ b/lib/github/ldap/member_of/recursive.rb @@ -10,13 +10,17 @@ class Recursive # Internal: The GitHub::Ldap object to search domains with. attr_reader :ldap + # Internal: The maximum depth to search for subgroups. + attr_reader :depth + # Public: Instantiate new search strategy. # # - ldap: GitHub::Ldap object - # - options: Hash of options (unused) + # - options: Hash of options def initialize(ldap, options = {}) @ldap = ldap @options = options + @depth = options[:depth] end # Public: Performs search for groups an entry is a member of, including @@ -51,7 +55,9 @@ def domains # subgroups with. def search_strategy @search_strategy ||= - GitHub::Ldap::Members::Recursive.new(ldap, attrs: %w(objectClass)) + GitHub::Ldap::Members::Recursive.new ldap, + depth: depth, + attrs: %w(objectClass) end # Internal: Returns true if the entry is a group. From d33fc087d599971b7efc7fda8a004f1a59ce48df Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Mon, 1 Dec 2014 13:39:20 -0800 Subject: [PATCH 5/5] Test excluded groups, clarify test name --- test/member_of/classic_test.rb | 7 ++++++- test/member_of/recursive_test.rb | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/member_of/classic_test.rb b/test/member_of/classic_test.rb index 896f939..c42becb 100644 --- a/test/member_of/classic_test.rb +++ b/test/member_of/classic_test.rb @@ -12,7 +12,7 @@ def find_group(cn) @domain.groups([cn]).first end - def test_finds_groups_entry_is_a_member_of + def test_finds_groups_entry_is_a_direct_member_of member_of = @strategy.perform(@entry) assert_includes member_of.map(&:dn), find_group("nested-group1").dn end @@ -23,4 +23,9 @@ def test_finds_subgroups_entry_is_a_member_of assert_includes member_of.map(&:dn), find_group("head-group").dn assert_includes member_of.map(&:dn), find_group("tail-group").dn end + + def test_excludes_groups_entry_is_not_a_member_of + member_of = @strategy.perform(@entry) + refute_includes member_of.map(&:dn), find_group("ghe-admins").dn + end end diff --git a/test/member_of/recursive_test.rb b/test/member_of/recursive_test.rb index 4c910dd..3d37ee1 100644 --- a/test/member_of/recursive_test.rb +++ b/test/member_of/recursive_test.rb @@ -12,7 +12,7 @@ def find_group(cn) @domain.groups([cn]).first end - def test_finds_groups_entry_is_a_member_of + def test_finds_groups_entry_is_a_direct_member_of member_of = @strategy.perform(@entry) assert_includes member_of.map(&:dn), find_group("nested-group1").dn end @@ -22,4 +22,9 @@ def test_finds_subgroups_entry_is_a_member_of assert_includes member_of.map(&:dn), find_group("head-group").dn assert_includes member_of.map(&:dn), find_group("tail-group").dn end + + def test_excludes_groups_entry_is_not_a_member_of + member_of = @strategy.perform(@entry) + refute_includes member_of.map(&:dn), find_group("ghe-admins").dn + end end