Permalink
Browse files

A far more complicated set of LDAP config options that should hopeful…

…ly cover more use cases.
  • Loading branch information...
1 parent 29ebb9d commit 4d6ed6e5bec1c863ad13af3d81c0b8416a97dbb0 @clowder clowder committed Mar 14, 2012
Showing with 58 additions and 33 deletions.
  1. +37 −24 app/concerns/ldap_authentication.rb
  2. +5 −6 config/general.yml.example
  3. +16 −3 test/unit/concerns/ldap_authentication_test.rb
@@ -1,14 +1,14 @@
module LdapAuthentication
def self.extended(base_class)
base_class.extend(LdapAuthentication::ClassMethods)
- base_class.send(:include, LdapAuthentication::InstanceMethods)
-
base_class.class_eval do
class << self
alias_method_chain :authenticate, :ldap
end
field :from_ldap, :type => Boolean, :default => false
+
+ attr_accessible :from_ldap
end
end
@@ -22,8 +22,12 @@ def authenticate_with_ldap(login, password)
:name => authenticated_user.displayname.first,
:password => password,
:password_confirmation => password,
- :role => User::STANDARD_ROLE })
- user.update_attribute(:from_ldap, true)
+ :role => User::STANDARD_ROLE,
+ :from_ldap => true })
+
+ # Use the most recent information from LDAP
+ user.update_attributes!(:email => authenticated_user.mail.first,
+ :name => authenticated_user.displayname.first)
user
else
authenticate_without_ldap(login, password)
@@ -32,33 +36,42 @@ def authenticate_with_ldap(login, password)
private
# Returns a single Net::LDAP::Entry or false
- def ldap_login(login, password)
- ldap_session = new_ldap_session(login, password)
- authenticated_user = ldap_session.bind_as({ :base => ::Configuration.ldap_base,
- :filter => "(#{::Configuration.ldap_username_attr}=#{ login })",
- :password => password })
+ def ldap_login(username, password)
+ ldap_session = new_ldap_session
+ bind_args = args_for(username, password)
+ authenticated_user = ldap_session.bind_as(bind_args)
authenticated_user ? authenticated_user.first : false
end
- def new_ldap_session(login, password)
+ # This is where LDAP jumps up and punches you in the face, all the while
+ # screaming "You never gunna get this, your wasting your time!".
+ def args_for(username, password)
+ user_filter = "#{ ::Configuration.ldap_username_attribute }=#{ username }"
+ args = { :base => ::Configuration.ldap_base,
+ :filter => "(#{ user_filter })",
+ :password => password }
+
+ unless ::Configuration.ldap_can_search_anonymously?
+ # If you can't search your LDAP directory anonymously we'll try and
+ # authenticate you with your user dn before we try and search for your
+ # account (dn example. `uid=clowder,ou=People,dc=mycompany,dc=com`).
+ user_dn = [user_filter, ::Configuration.ldap_base].join(',')
+ args.merge({ :auth => { :username => user_dn, :password => password, :method => :simple } })
+ end
+
+ args
+ end
+
+ def new_ldap_session
Net::LDAP.new(:host => ::Configuration.ldap_host,
:port => ::Configuration.ldap_port,
:encryption => ::Configuration.ldap_encryption.try(:to_sym),
- :base => ::Configuration.ldap_base,
- :auth => {
- :username => "#{::Configuration.ldap_bind_domain_name}\\#{login}",
- :password => password,
- :method => :simple
- }
- )
@lennartkoopmann

lennartkoopmann Mar 15, 2012

I guess this broke it for us. The :auth stuff is required already here, not only for the bind_as.

@clowder

clowder Mar 15, 2012

I think that it should be equivalent to the auth being added on line 55. The only difference is that (as per the other committers comment) I changed it to use a full DN for the user as opposed to the above (which was an Active Directory only feature).

EDIT: so, if you move the auth to the session it works for you?

@lennartkoopmann

lennartkoopmann Mar 15, 2012

Yes, as soon as I add the :auth hash it works.

@clowder

clowder Mar 15, 2012

Ahh ok, I’ll shuffle it then...

Does it work with the DN or the Active Directory style "Down-Level Logon Name”.

e.g. uid=login,ou=People,dc=mycompany,dc=com vs. domain\\login?

@lennartkoopmann

lennartkoopmann Mar 15, 2012

With the domain\\login shit.

@renegmed

renegmed via email Mar 15, 2012

@renegmed

renegmed via email Mar 15, 2012

+ :base => ::Configuration.ldap_base)
end
end
- module InstanceMethods
- end
-
module Configuration
def ldap_config(key, default = nil)
nested_general_config :ldap, key, default
@@ -84,12 +97,12 @@ def ldap_encryption
ldap_config :encryption
end
- def ldap_username_attr
- ldap_config(:username_attr) || "uid"
+ def ldap_username_attribute
+ ldap_config :username_attribute, 'uid'
end
- def ldap_bind_domain_name
- ldap_config :bind_domain_name
+ def ldap_can_search_anonymously?
+ ldap_config :can_search_anonymously, true
end
end
end
View
@@ -2,18 +2,17 @@ general:
external_hostname: "your-graylog2.example.org" # Used for example to generate permalinks. Don't add 'http://' or trailing slashes.
date_format: "%d.%m.%Y - %H:%M:%S" # http://ruby-doc.org/core/classes/Time.html#M000298 (strftime syntax)
allow_deleting: false # Allowing deleting of messages negatively impacts performance
- allow_version_check: true # Enables manual (/versioncheck/index) and automatic (every 30min from overview page) version checking against graylog2.org via HTTP.
+ allow_version_check: true # Enables manual (/versioncheck/index) and automatic (every 30min from overview page) version checking against graylog2.org via HTTP.
# custom_cookie_name: graylog2_staging1 # Set an own cookie name - Useful for multiple deployments on same host like example.org/staging1/graylog2, example.org/staging2/graylog2
-# Settings for ldap base user authentication
ldap:
enabled: false
host: ldap.mycompany.com
- encryption: start_tls
- port: 389
base: ou=People,dc=mycompany,dc=com
- username_attr: sAMAccountName
- bind_domain_name: yourcompany
+# encryption: start_tls
+# port: 389
+# username_attribute: email
+# can_search_anonymously: true # Defaults to true. If you set this to false you MUST ensure that you set your 'base' properly
# Settings for stream subscription emails.
subscriptions:
@@ -12,7 +12,10 @@ def self.hereditary?
end
setup do
- @existing_user = User.make(:login => 'existing_user')
+ @existing_user = User.make(:login => 'existing_user', :email => 'existing@test.com', :name => 'Existing User')
+ @existing_user_ldap_entry = mock()
+ @existing_user_ldap_entry.stubs(:mail => ['existing@test.com'], :displayname => ['Existing User'])
+
@mock_ldap_entry = mock()
@mock_ldap_entry.stubs(:mail => ['clowder@gmail.com'], :displayname => ['Chris Lowder'])
end
@@ -23,7 +26,6 @@ def test_should_create_local_user_for_new_ldap_users
assert_difference 'User.count' do
user = UserWithLdap.authenticate("clowder", "foobar")
-
assert_equal "clowder", user.login
assert_equal "clowder@gmail.com", user.email
assert_equal "Chris Lowder", user.name
@@ -32,7 +34,7 @@ def test_should_create_local_user_for_new_ldap_users
end
def test_should_not_create_local_user_for_returning_ldap_users
- UserWithLdap.stubs(:ldap_login).returns(@mock_ldap_entry)
+ UserWithLdap.stubs(:ldap_login).returns(@existing_user_ldap_entry)
assert_no_difference 'User.count' do
user = UserWithLdap.authenticate("existing_user", "anything")
@@ -49,5 +51,16 @@ def test_returns_nil_if_authentication_fails
assert_equal nil, user
end
end
+
+ def test_users_are_updated_if_their_remote_details_change
+ UserWithLdap.stubs(:ldap_login).returns(@mock_ldap_entry)
+
+ assert_no_difference 'User.count' do
+ user = UserWithLdap.authenticate(@existing_user.login, "anything")
+
+ assert_equal 'Chris Lowder', user.name
+ assert_equal 'clowder@gmail.com', user.email
+ end
+ end
end

0 comments on commit 4d6ed6e

Please sign in to comment.