From 9b820ee975ef57aaec8efd0551c7b64639af9745 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Tue, 9 Dec 2014 17:25:26 -0800 Subject: [PATCH 1/5] Accept depth as option for Recursive strategy constructor This makes the perform method signature identical for all strategies. --- lib/github/ldap/membership_validators/base.rb | 8 +++++--- .../ldap/membership_validators/recursive.rb | 17 ++++++++++++++++- test/membership_validators/recursive_test.rb | 8 ++++---- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/github/ldap/membership_validators/base.rb b/lib/github/ldap/membership_validators/base.rb index 3c47853..be378d1 100644 --- a/lib/github/ldap/membership_validators/base.rb +++ b/lib/github/ldap/membership_validators/base.rb @@ -13,9 +13,11 @@ class Base # # - ldap: GitHub::Ldap object # - groups: Array of Net::LDAP::Entry group objects - def initialize(ldap, groups) - @ldap = ldap - @groups = groups + # - options: Hash of options + def initialize(ldap, groups, options = {}) + @ldap = ldap + @groups = groups + @options = options end # Abstract: Performs the membership validation check. diff --git a/lib/github/ldap/membership_validators/recursive.rb b/lib/github/ldap/membership_validators/recursive.rb index 8c40aeb..8544f30 100644 --- a/lib/github/ldap/membership_validators/recursive.rb +++ b/lib/github/ldap/membership_validators/recursive.rb @@ -21,7 +21,22 @@ class Recursive < Base DEFAULT_MAX_DEPTH = 9 ATTRS = %w(dn cn) - def perform(entry, depth = DEFAULT_MAX_DEPTH) + # Internal: The maximum depth to search for membership. + attr_reader :depth + + # Public: Instantiate new search strategy. + # + # - ldap: GitHub::Ldap object + # - options: Hash of options + # - groups: Array of Net::LDAP::Entry group objects + # + # NOTE: This overrides default behavior to configure `depth`. + def initialize(ldap, groups, options = {}) + super + @depth = options[:depth] || DEFAULT_MAX_DEPTH + end + + def perform(entry) # short circuit validation if there are no groups to check against return true if groups.empty? diff --git a/test/membership_validators/recursive_test.rb b/test/membership_validators/recursive_test.rb index e351532..ea6462d 100644 --- a/test/membership_validators/recursive_test.rb +++ b/test/membership_validators/recursive_test.rb @@ -8,9 +8,9 @@ def setup @validator = GitHub::Ldap::MembershipValidators::Recursive end - def make_validator(groups) + def make_validator(groups, depth = nil) groups = @domain.groups(groups) - @validator.new(@ldap, groups) + @validator.new(@ldap, groups, depth: depth) end def test_validates_user_in_group @@ -34,8 +34,8 @@ def test_validates_user_in_great_grandchild_group end def test_does_not_validate_user_in_great_granchild_group_with_depth - validator = make_validator(%w(n-depth-nested-group3)) - refute validator.perform(@entry, 2) + validator = make_validator(%w(n-depth-nested-group3), 2) + refute validator.perform(@entry) end def test_does_not_validate_user_not_in_group From 4c2dc84e87fd3e1a1849414fe0eb69280fb4781d Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Tue, 9 Dec 2014 17:27:26 -0800 Subject: [PATCH 2/5] Minor tweak to test helper --- test/membership_validators/recursive_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/membership_validators/recursive_test.rb b/test/membership_validators/recursive_test.rb index ea6462d..072ffca 100644 --- a/test/membership_validators/recursive_test.rb +++ b/test/membership_validators/recursive_test.rb @@ -8,9 +8,9 @@ def setup @validator = GitHub::Ldap::MembershipValidators::Recursive end - def make_validator(groups, depth = nil) + def make_validator(groups, options = {}) groups = @domain.groups(groups) - @validator.new(@ldap, groups, depth: depth) + @validator.new(@ldap, groups, options) end def test_validates_user_in_group @@ -34,7 +34,7 @@ def test_validates_user_in_great_grandchild_group end def test_does_not_validate_user_in_great_granchild_group_with_depth - validator = make_validator(%w(n-depth-nested-group3), 2) + validator = make_validator(%w(n-depth-nested-group3), depth: 2) refute validator.perform(@entry) end From ec8ee6cfbc5a588c5defd66f611313d6f4a6e7b4 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 10 Dec 2014 23:34:36 -0800 Subject: [PATCH 3/5] Document depth option --- lib/github/ldap/membership_validators/recursive.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/github/ldap/membership_validators/recursive.rb b/lib/github/ldap/membership_validators/recursive.rb index 8544f30..4567261 100644 --- a/lib/github/ldap/membership_validators/recursive.rb +++ b/lib/github/ldap/membership_validators/recursive.rb @@ -27,8 +27,9 @@ class Recursive < Base # Public: Instantiate new search strategy. # # - ldap: GitHub::Ldap object + # - groups: Array of Net::LDAP::Entry group objects # - options: Hash of options - # - groups: Array of Net::LDAP::Entry group objects + # depth: Integer limit of recursion # # NOTE: This overrides default behavior to configure `depth`. def initialize(ldap, groups, options = {}) From d72403679e2429be3b537f57a884cc9c2c401939 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 10 Dec 2014 23:35:03 -0800 Subject: [PATCH 4/5] Keep depth param as override --- lib/github/ldap/membership_validators/recursive.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/github/ldap/membership_validators/recursive.rb b/lib/github/ldap/membership_validators/recursive.rb index 4567261..ff78f5a 100644 --- a/lib/github/ldap/membership_validators/recursive.rb +++ b/lib/github/ldap/membership_validators/recursive.rb @@ -37,7 +37,7 @@ def initialize(ldap, groups, options = {}) @depth = options[:depth] || DEFAULT_MAX_DEPTH end - def perform(entry) + def perform(entry, depth_override = nil) # short circuit validation if there are no groups to check against return true if groups.empty? @@ -52,7 +52,7 @@ def perform(entry) next if membership.empty? # recurse to at most `depth` - depth.times do |n| + (depth_override || depth).times do |n| # find groups whose members include membership groups membership = domain.search(filter: membership_filter(membership), attributes: ATTRS) From 51e2732112a3b61305b08ea4486d9ba403e6aec5 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Thu, 11 Dec 2014 00:01:46 -0800 Subject: [PATCH 5/5] Warn that the depth_override param is deprecated cc @jch --- lib/github/ldap/membership_validators/recursive.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/github/ldap/membership_validators/recursive.rb b/lib/github/ldap/membership_validators/recursive.rb index ff78f5a..3b78545 100644 --- a/lib/github/ldap/membership_validators/recursive.rb +++ b/lib/github/ldap/membership_validators/recursive.rb @@ -38,6 +38,14 @@ def initialize(ldap, groups, options = {}) end def perform(entry, depth_override = nil) + if depth_override + warn "DEPRECATION WARNING: Calling Recursive#perform with a second argument is deprecated." + warn "Usage:" + warn " strategy = GitHub::Ldap::MembershipValidators::Recursive.new \\" + warn " ldap, depth: 5" + warn " strategy#perform(entry)" + end + # short circuit validation if there are no groups to check against return true if groups.empty?