Merge pull request #58 from github/default-membership-validator

Detect appropriate membership validator strategy by default

Author: jch

lib/github/ldap.rb

@@ -34,6 +34,7 @@ class Ldap
     def_delegator :@connection, :open
 
     attr_reader :uid, :search_domains, :virtual_attributes,
+                :membership_validator,
                 :instrumentation_service
 
     # Build a new GitHub::Ldap instance
@@ -87,6 +88,9 @@ def initialize(options = {})
       # when a base is not explicitly provided.
       @search_domains = Array(options[:search_domains])
 
+      # configure which strategy should be used to validate user membership
+      configure_membership_validation_strategy(options[:membership_validator])
+
       # enables instrumenting queries
       @instrumentation_service = options[:instrumentation_service]
     end
@@ -182,6 +186,23 @@ def search(options, &block)
       end
     end
 
+    # Internal: Searches the host LDAP server's Root DSE for capabilities and
+    # extensions.
+    #
+    # Returns a Net::LDAP::Entry object.
+    def capabilities
+      @capabilities ||=
+        instrument "capabilities.github_ldap" do |payload|
+          begin
+            @connection.search_root_dse
+          rescue Net::LDAP::LdapError => error
+            payload[:error] = error
+            # stubbed result
+            Net::LDAP::Entry.new
+          end
+        end
+    end
+
     # Internal - Determine whether to use encryption or not.
     #
     # encryption: is the encryption method, either 'ssl', 'tls', 'simple_tls' or 'start_tls'.
@@ -214,5 +235,24 @@ def configure_virtual_attributes(attributes)
         VirtualAttributes.new(false)
       end
     end
+
+    # Internal: Configure the membership validation strategy.
+    #
+    # Used by GitHub::Ldap::MembershipValidators::Detect to force a specific
+    # strategy (instead of detecting host capabilities and deciding at runtime).
+    #
+    # If `strategy` is not provided, or doesn't match a known strategy,
+    # defaults to `:detect`. Otherwise the configured strategy is selected.
+    #
+    # Returns the selected membership validator strategy Symbol.
+    def configure_membership_validation_strategy(strategy = nil)
+      @membership_validator =
+        case strategy.to_s
+        when "classic", "recursive", "active_directory"
+          strategy.to_sym
+        else
+          :detect
+        end
+    end
   end
 end

lib/github/ldap/membership_validators.rb

@@ -1,4 +1,5 @@
 require 'github/ldap/membership_validators/base'
+require 'github/ldap/membership_validators/detect'
 require 'github/ldap/membership_validators/classic'
 require 'github/ldap/membership_validators/recursive'
 require 'github/ldap/membership_validators/active_directory'
@@ -13,6 +14,13 @@ class Ldap
     #   validator = GitHub::Ldap::MembershipValidators::Classic.new(ldap, groups)
     #   validator.perform(entry) #=> true
     #
-    module MembershipValidators; end
+    module MembershipValidators
+      # Internal: Mapping of strategy name to class.
+      STRATEGIES = {
+        :classic          => GitHub::Ldap::MembershipValidators::Classic,
+        :recursive        => GitHub::Ldap::MembershipValidators::Recursive,
+        :active_directory => GitHub::Ldap::MembershipValidators::ActiveDirectory
+      }
+    end
   end
 end

lib/github/ldap/membership_validators/detect.rb

@@ -0,0 +1,69 @@
+module GitHub
+  class Ldap
+    module MembershipValidators
+      # Detects the LDAP host's capabilities and determines the appropriate
+      # membership validation strategy at runtime. Currently detects for
+      # ActiveDirectory in-chain membership validation. An explicit strategy can
+      # also be defined via `GitHub::Ldap#membership_validator=`. See also
+      # `GitHub::Ldap#configure_membership_validation_strategy`.
+      class Detect < Base
+        # Internal: The capability required to use the ActiveDirectory strategy.
+        # See: http://msdn.microsoft.com/en-us/library/cc223359.aspx.
+        ACTIVE_DIRECTORY_V61_R2_OID = "1.2.840.113556.1.4.2080".freeze
+
+        def perform(entry)
+          # short circuit validation if there are no groups to check against
+          return true if groups.empty?
+
+          strategy.perform(entry)
+        end
+
+        # Internal: Returns the membership validation strategy object.
+        def strategy
+          @strategy ||= begin
+            strategy = detect_strategy
+            strategy.new(ldap, groups)
+          end
+        end
+
+        # Internal: Detects LDAP host's capabilities and chooses the best
+        # strategy for the host.
+        #
+        # If the strategy has been set explicitly, skips detection and uses the
+        # configured strategy instead.
+        #
+        # Returns the strategy class.
+        def detect_strategy
+          case
+          when GitHub::Ldap::MembershipValidators::STRATEGIES.key?(strategy_config)
+            GitHub::Ldap::MembershipValidators::STRATEGIES[strategy_config]
+          when active_directory_capability?
+            GitHub::Ldap::MembershipValidators::STRATEGIES[:active_directory]
+          else
+            GitHub::Ldap::MembershipValidators::STRATEGIES[:recursive]
+          end
+        end
+
+        # Internal: Returns the configured membership validator strategy Symbol.
+        def strategy_config
+          ldap.membership_validator
+        end
+
+        # Internal: Detect whether the LDAP host is an ActiveDirectory server.
+        #
+        # See: http://msdn.microsoft.com/en-us/library/cc223359.aspx.
+        #
+        # Returns true if the host is an ActiveDirectory server, false otherwise.
+        def active_directory_capability?
+          capabilities[:supportedcapabilities].include?(ACTIVE_DIRECTORY_V61_R2_OID)
+        end
+
+        # Internal: Returns the Net::LDAP::Entry object describing the LDAP
+        # host's capabilities (via the Root DSE).
+        def capabilities
+          ldap.capabilities
+        end
+      end
+    end
+  end
+end

test/ldap_test.rb

@@ -72,6 +72,14 @@ def test_instruments_search
     assert_equal "(uid=user1)",      payload[:filter].to_s
     assert_equal "dc=github,dc=com", payload[:base]
   end
+
+  def test_membership_validator_default
+    assert_equal :detect, @ldap.membership_validator
+  end
+
+  def test_capabilities
+    assert_kind_of Net::LDAP::Entry, @ldap.capabilities
+  end
 end
 
 class GitHubLdapTest < GitHub::Ldap::Test

test/membership_validators/detect_test.rb

@@ -0,0 +1,49 @@
+require_relative '../test_helper'
+
+# NOTE: Since this strategy is targeted at detecting ActiveDirectory
+# capabilities, and we don't have AD setup in CI, we stub out actual queries
+# and test against what AD *would* respond with.
+
+class GitHubLdapDetectMembershipValidatorsTest < 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')
+    @validator = GitHub::Ldap::MembershipValidators::Detect
+  end
+
+  def make_validator(groups)
+    groups = @domain.groups(groups)
+    @validator.new(@ldap, groups)
+  end
+
+  def test_defers_to_configured_strategy
+    @ldap.configure_membership_validation_strategy(:classic)
+    validator = make_validator(%w(group))
+
+    assert_kind_of GitHub::Ldap::MembershipValidators::Classic, validator.strategy
+  end
+
+  def test_detects_active_directory
+    caps = Net::LDAP::Entry.new
+    caps[:supportedcapabilities] =
+      [GitHub::Ldap::MembershipValidators::Detect::ACTIVE_DIRECTORY_V61_R2_OID]
+
+    validator = make_validator(%w(group))
+    @ldap.stub :capabilities, caps do
+      assert_kind_of GitHub::Ldap::MembershipValidators::ActiveDirectory,
+        validator.strategy
+    end
+  end
+
+  def test_falls_back_to_recursive
+    caps = Net::LDAP::Entry.new
+    caps[:supportedcapabilities] = []
+
+    validator = make_validator(%w(group))
+    @ldap.stub :capabilities, caps do
+      assert_kind_of GitHub::Ldap::MembershipValidators::Recursive,
+        validator.strategy
+    end
+  end
+end