Skip to content

Commit

Permalink
Adding docs/tests around new ldap_check_group_membership_without_admin
Browse files Browse the repository at this point in the history 
Removed a couple extraneous internal functions to slim down the logic
from the initial PR. Also, removed the user_in_ldap_group? Devise model
method that was implied but not fully added in the PR. If we have a
static option, it seems redundant. The only thing the extra model method
would accomplish would be to allow model users to specify whether to use
the admin LDAP binding to check group membership in the explicit lookup
function (the static config is used for the auto group checking during
the required groups login flow). This seems like a pretty extraordinary
case of an app; namely, it seems like the only case you'd ever want to
check group membership with non-admin credentials would be if you can't
get or don't want to use admin credentials. I can't think of a plausible
use case where you'd have the credentials and would benefit from an
admin check later on where you wouldn't just want to use those admin
credentials globally (and thus leave the new option at its default
false).
  • Loading branch information
@stevenyxu
stevenyxu committed Apr 28, 2014
1 parent 33887a7 commit 22ef35e
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 66 deletions.
14 changes: 4 additions & 10 deletions README.md
View file
Expand Up @@ -71,33 +71,27 @@ In initializer `config/initializers/devise.rb` :

* `ldap_logger` _(default: true)_
* If set to true, will log LDAP queries to the Rails logger.

* `ldap_create_user` _(default: false)_
* If set to true, all valid LDAP users will be allowed to login and an appropriate user record will be created.
If set to false, you will have to create the user record before they will be allowed to login.

* If set to true, all valid LDAP users will be allowed to login and an appropriate user record will be created. If set to false, you will have to create the user record before they will be allowed to login.
* `ldap_config` _(default: #{Rails.root}/config/ldap.yml)_
* Where to find the LDAP config file. Commented out to use the default, change if needed.

* Where to find the LDAP config file. Commented out to use the default, change if needed.
* `ldap_update_password` _(default: true)_
* When doing password resets, if true will update the LDAP server. Requires admin password in the ldap.yml

* `ldap_check_group_membership` _(default: false)_
* When set to true, the user trying to login will be checked to make sure they are in all of groups specified in the ldap.yml file.

* `ldap_check_attributes` _(default: false)_
* When set to true, the user trying to login will be checked to make sure they have all of the attributes in the ldap.yml file.

* `ldap_use_admin_to_bind` _(default: false)_
* When set to true, the admin user will be used to bind to the LDAP server during authentication.
* `ldap_check_group_membership_without_admin` _(default: false)_
* When set to true, the group membership check is done with the user's own credentials rather than with admin credentials. Since these credentials are only available to the Device user model during the login flow, the group check function will not work if a group check is performed when this option is true outside of the login flow (e.g., before particular actions).

Advanced Configuration
----------------------
These parameters will be added to `config/initializers/devise.rb` when you pass the `--advanced` switch to the generator:

* `ldap_auth_username_builder` _(default: `Proc.new() {|attribute, login, ldap| "#{attribute}=#{login},#{ldap.base}" }`)_
* You can pass a proc to the username option to explicitly specify the format that you search for a users' DN on your LDAP server.

* `ldap_auth_password_build` _(default: `Proc.new() {|new_password| Net::LDAP::Password.generate(:sha, new_password) }`)_
* Optionally you can define a proc to create custom password encrption when user reset password

Expand Down
8 changes: 0 additions & 8 deletions lib/devise_ldap_authenticatable/ldap/adapter.rb
View file
Expand Up @@ -45,18 +45,10 @@ def self.get_groups(login)
self.ldap_connect(login).user_groups
end

def self.get_groups_as_user(login)
self.ldap_connect(login).auth_user_groups
end

def self.in_ldap_group?(login, group_name, group_attribute = nil)
self.ldap_connect(login).in_group?(group_name, group_attribute)
end

def self.user_in_ldap_group?(login, group_name, group_attribute = nil)
self.ldap_connect(login).user_in_group?(group_name, group_attribute)
end

def self.get_dn(login)
self.ldap_connect(login).dn
end
Expand Down
43 changes: 11 additions & 32 deletions lib/devise_ldap_authenticatable/ldap/connection.rb
View file
Expand Up @@ -103,18 +103,10 @@ def in_required_groups?
return false if @required_groups.nil?

for group in @required_groups
if @check_group_membership_without_admin
if group.is_a?(Array)
return false unless user_in_group?(group[1], group[0])
else
return false unless user_in_group?(group)
end
if group.is_a?(Array)
return false unless in_group?(group[1], group[0])
else
if group.is_a?(Array)
return false unless in_group?(group[1], group[0])
else
return false unless in_group?(group)
end
return false unless in_group?(group)
end
end
return true
Expand All @@ -123,40 +115,28 @@ def in_required_groups?
def in_group?(group_name, group_attribute = LDAP::DEFAULT_GROUP_UNIQUE_MEMBER_LIST_KEY)
in_group = false

admin_ldap = Connection.admin
if @check_group_membership_without_admin
group_checking_ldap = @ldap
else
group_checking_ldap = Connection.admin
end

unless ::Devise.ldap_ad_group_check
admin_ldap.search(:base => group_name, :scope => Net::LDAP::SearchScope_BaseObject) do |entry|
group_checking_ldap.search(:base => group_name, :scope => Net::LDAP::SearchScope_BaseObject) do |entry|
if entry[group_attribute].include? dn
in_group = true
DeviseLdapAuthenticatable::Logger.send("User #{dn} IS included in group: #{group_name}")
end
end
else
# AD optimization - extension will recursively check sub-groups with one query
# "(memberof:1.2.840.113556.1.4.1941:=group_name)"
search_result = admin_ldap.search(:base => dn,
search_result = group_checking_ldap.search(:base => dn,
:filter => Net::LDAP::Filter.ex("memberof:1.2.840.113556.1.4.1941", group_name),
:scope => Net::LDAP::SearchScope_BaseObject)
# Will return the user entry if belongs to group otherwise nothing
if search_result.length == 1 && search_result[0].dn.eql?(dn)
in_group = true
end
end

unless in_group
DeviseLdapAuthenticatable::Logger.send("User #{dn} is not in group: #{group_name}")
end

return in_group
end

def user_in_group?(group_name, group_attribute = LDAP::DEFAULT_GROUP_UNIQUE_MEMBER_LIST_KEY)
in_group = false

@ldap.search(:base => group_name, :scope => Net::LDAP::SearchScope_BaseObject) do |entry|
if entry.uniqueMember.include? dn
in_group = true
## Logging because it's a nice thing to do.
DeviseLdapAuthenticatable::Logger.send("User #{dn} IS included in group: #{group_name}")
end
end
Expand All @@ -168,7 +148,6 @@ def user_in_group?(group_name, group_attribute = LDAP::DEFAULT_GROUP_UNIQUE_MEMB
return in_group
end


def has_required_attribute?
return true unless ::Devise.ldap_check_attributes

Expand Down
26 changes: 13 additions & 13 deletions lib/generators/devise_ldap_authenticatable/install_generator.rb
View file
@@ -1,31 +1,31 @@
module DeviseLdapAuthenticatable
class InstallGenerator < Rails::Generators::Base
source_root File.expand_path("../templates", __FILE__)

class_option :user_model, :type => :string, :default => "user", :desc => "Model to update"
class_option :update_model, :type => :boolean, :default => true, :desc => "Update model to change from database_authenticatable to ldap_authenticatable"
class_option :add_rescue, :type => :boolean, :default => true, :desc => "Update Application Controller with resuce_from for DeviseLdapAuthenticatable::LdapException"
class_option :advanced, :type => :boolean, :desc => "Add advanced config options to the devise initializer"


def create_ldap_config
copy_file "ldap.yml", "config/ldap.yml"
end

def create_default_devise_settings
inject_into_file "config/initializers/devise.rb", default_devise_settings, :after => "Devise.setup do |config|\n"
end

def update_user_model
gsub_file "app/models/#{options.user_model}.rb", /:database_authenticatable/, ":ldap_authenticatable" if options.update_model?
end

def update_application_controller
inject_into_class "app/controllers/application_controller.rb", ApplicationController, rescue_from_exception if options.add_rescue?
end

private

def default_devise_settings
settings = <<-eof
# ==> LDAP Configuration
Expand All @@ -38,26 +38,26 @@ def default_devise_settings
# config.ldap_check_attributes = false
# config.ldap_use_admin_to_bind = false
# config.ldap_ad_group_check = false
eof
if options.advanced?
settings << <<-eof
# ==> Advanced LDAP Configuration
# config.ldap_auth_username_builder = Proc.new() {|attribute, login, ldap| "\#{attribute}=\#{login},\#{ldap.base}" }
eof
end

settings
end

def rescue_from_exception
<<-eof
rescue_from DeviseLdapAuthenticatable::LdapException do |exception|
render :text => exception, :status => 500
end
eof
end

end
end
19 changes: 16 additions & 3 deletions spec/unit/user_spec.rb
View file
Expand Up @@ -171,19 +171,32 @@ def should_not_be_validated(user, password, message = "Password is not properly
describe "check group membership w/out admin bind" do
before do
@user = Factory.create(:user)
::Devise.ldap_check_group_membership_without_admin = true
end

after do
::Devise.ldap_check_group_membership_without_admin = false
end

it "should return true for user being in the users group" do
assert_equal true, @user.user_in_ldap_group?('cn=users,ou=groups,dc=test,dc=com')
assert_equal true, @user.in_ldap_group?('cn=users,ou=groups,dc=test,dc=com')
end

it "should return false for user being in the admins group" do
assert_equal false, @user.user_in_ldap_group?('cn=admins,ou=groups,dc=test,dc=com')
assert_equal false, @user.in_ldap_group?('cn=admins,ou=groups,dc=test,dc=com')
end

it "should return false for a user being in a nonexistent group" do
assert_equal false, @user.user_in_ldap_group?('cn=thisgroupdoesnotexist,ou=groups,dc=test,dc=com')
assert_equal false, @user.in_ldap_group?('cn=thisgroupdoesnotexist,ou=groups,dc=test,dc=com')
end

# TODO: add a test that confirms the user's own binding is used rather
# than the admin binding by creating an LDAP user who can't do group
# lookups perhaps?

# TODO: add a test to demonstrate this function won't work on a user
# after the initial login request if the password isn't available. This
# might have to be more of a full stack test.
end

describe "use role attribute for authorization" do
Expand Down

0 comments on commit 22ef35e

Please sign in to comment.