Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add optional Group membership when authenticating users #37

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add optional Group membership when authenticating users #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
801a640
Added option for group
techno1ology Sep 25, 2013
93db001
Updated documentation.
techno1ology Sep 25, 2013
f35fda8
Updated documentation.
techno1ology Sep 25, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ Use the LDAP strategy as a middleware in your application:
:uid => 'sAMAccountName',
:name_proc => Proc.new {|name| name.gsub(/@.*$/,'')}
:bind_dn => 'default_bind_dn'
:password => 'password'
:password => 'password',
:group => 'group'

All of the listed options are required, with the exception of :title, :name_proc, :bind_dn, and :password.
All of the listed options are required, with the exception of :title, :name_proc, :bind_dn, :password and :group.
Allowed values of :method are: :plain, :ssl, :tls.

:bind_dn and :password is the default credentials to perform user lookup.
Expand All @@ -38,6 +39,10 @@ Allowed values of :method are: :plain, :ssl, :tls.
Use them to initialize a SASL connection to server. If you are not familiar with these authentication methods,
please just avoid them.

:group will additionally allow users to check whether the user belongs to a specific group. After a user has been
authenticated, group will be checked. If the user does not belong to the specified group, the user will be redirected
to /auth/failure with the message, :invalid_group

Direct users to '/auth/ldap' to have them authenticated via your company's LDAP server.


Expand Down
15 changes: 15 additions & 0 deletions lib/omniauth/strategies/ldap.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class LDAP
option :method, :plain
option :uid, 'sAMAccountName'
option :name_proc, lambda {|n| n}
option :group

def request_phase
OmniAuth::LDAP::Adaptor.validate @options
Expand All @@ -42,6 +43,11 @@ def callback_phase
@ldap_user_info = @adaptor.bind_as(:filter => Net::LDAP::Filter.eq(@adaptor.uid, @options[:name_proc].call(request['username'])),:size => 1, :password => request['password'])
return fail!(:invalid_credentials) if !@ldap_user_info

# If group is specified in options, validate membership
if @options[:group]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend moving this check into the is_member function. That is, is_member should return true if @options[:group] has not been set. This prevents anyone else that might use is_member from needing to understand that it has a precondition of @options[:group] having been set, this precondition is unneeded.

return fail!(:invalid_group) unless is_member?(@ldap_user_info)
end

@user_info = self.class.map_user(@@config, @ldap_user_info)
super
rescue Exception => e
Expand Down Expand Up @@ -86,6 +92,15 @@ def self.map_user(mapper, object)
def missing_credentials?
request['username'].nil? or request['username'].empty? or request['password'].nil? or request['password'].empty?
end # missing_credentials?

def is_member?(ldap_user_info)
ldap_user_info.memberof.each do |value|
group_parts = value.split(',')
expected_group = group_parts[0].match(/CN=(.+)/)
return true if expected_group[1] == options[:group]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recommend allowing options[:group] be an array of groups. Also recommend checking the type of the options[group], encapsulate in a function to do this.

end
return false
end # is_member?(Net::LDAP::Entry)
end
end
end
Expand Down