[Ovirt-devel] [PATCH] Modified LDAPConnection to work with an SRV record, then fallback to the ldap.yml file if necessary.

[Perry N. Myers]

Darryl L. Pierce wrote:
> ---
>  wui/src/app/controllers/hardware_controller.rb |    2 +-
>  wui/src/app/helpers/ldap_connection.rb         |   25 ++++++++++++++++++-----
>  wui/src/app/models/account.rb                  |    2 +-
>  3 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb
> index 296fec3..54d9e91 100644
> --- a/wui/src/app/controllers/hardware_controller.rb
> +++ b/wui/src/app/controllers/hardware_controller.rb
> @@ -144,7 +144,7 @@ class HardwareController < ApplicationController
>      dates = [ Date::ABBR_MONTHNAMES[today.month] + ' ' + today.day.to_s ]
>      1.upto(6){ |x|  # TODO get # of days from wui
>         dte = today - x
> -       dates.push ( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s )
> +       dates.push( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s )
>      }
>      dates.reverse! # want in ascending order
>  
> diff --git a/wui/src/app/helpers/ldap_connection.rb b/wui/src/app/helpers/ldap_connection.rb
> index af8b41b..b19e504 100644
> --- a/wui/src/app/helpers/ldap_connection.rb
> +++ b/wui/src/app/helpers/ldap_connection.rb
> @@ -21,15 +21,28 @@
>  # connections with an LDAP server.
>  #
>  class LDAPConnection
> -
> -  @@config = YAML::load(File.open("#{RAILS_ROOT}/config/ldap.yml"))
> +  @@config = YAML::load(File.open("#{RAILS_ROOT}/config/ldap.yml"))[ENV['RAILS_ENV']]
>  
>    # Connects the specified LDAP server.
>    def self.connect(base = nil, host = nil, port = nil)
> +
> +    # Use the SRV record for the LDAP server, otherwise
> +    # default to what is defined in the ldap.yml file.
> +    unless defined? host
> +      unless defined? @host
> +	Resolv::DNS.new
> +	
> +	dns.getresources("_ldap._tcp.#{`dnsdomainname`}",Resolv::DNS::Resource::IN::SRV).collect do |resource|
> +	  @host = resource.address

You're not getting the port from dns srv as well?  We should be getting 
both host and port from the dns srv record.

> +	end
> +      end
> +
> +      @host = @@config["host"] unless defined? @host
> +    end
>      
> -    base = @@config[ENV['RAILS_ENV']]["base"] if base == nil
> -    host = @@config[ENV['RAILS_ENV']]["host"] if host == nil
> -    port = @@config[ENV['RAILS_ENV']]["port"] if port == nil
> +    base = @@config["base"] if base == nil
> +    host = @@config["host"] if host == nil
> +    port = @@config["port"] if port == nil

Where is base confgured from?  If it's the default obtained from ldap.yaml 
it'll be wrong.  It would seem that you should calculate what base is 
using the value of host.  i.e. if host is foo.bar.com then base should be 
dc=bar,dc=com.

>      ActiveLdap::Base.establish_connection(:host => host,
>  					  :port => port,
> @@ -38,7 +51,7 @@ class LDAPConnection
>  
>    # Returns whether a connection already exists to the LDAP server.
>    def self.connected?
> -    return ActiveLdap::Base.connected?
> +    ActiveLdap::Base.connected?
>    end
>  
>    # Disconnects from the LDAP server.
> diff --git a/wui/src/app/models/account.rb b/wui/src/app/models/account.rb
> index a2ed1d2..384beda 100644
> --- a/wui/src/app/models/account.rb
> +++ b/wui/src/app/models/account.rb
> @@ -20,7 +20,7 @@
>  # +Account+ represents a single user's account from the LDAP server.
>  #
>  class Account < ActiveLdap::Base
> -  ldap_mapping :dn_attribute => 'cn', :scope => :one, :prefix => 'cn=users,cn=accounts'
> +  ldap_mapping :dn_attribute => 'cn', :scope => :one, :prefix => "cn=users,cn=accounts"
>  
>    @@users = nil
>  


-- 
|=-        Red Hat, Engineering, Emerging Technologies, Boston        -=|
|=-                     Email: pmyers at redhat.com                      -=|
|=-         Office: +1 412 474 3552   Mobile: +1 703 362 9622         -=|
|=- GnuPG: E65E4F3D 88F9 F1C9 C2F3 1303 01FE 817C C5D2 8B91 E65E 4F3D -=|