From 17257a23fd811ce94643229c6072e4240eece284 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Thu, 3 Jan 2013 10:52:02 -0600 Subject: [PATCH 1/4] Added basic LDAP signin without tests. Manually tested against OpenLDAP and Active Directory with plain authentication. --- Gemfile | 3 + Gemfile.lock | 2 + barkeep_server.rb | 124 +++++++++++++++++++++++++++---- doc/ldap.markdown | 1 + environment.rb | 3 + public/coffee/ldap_signin.coffee | 26 +++++++ public/css/admin.scss | 8 -- public/css/styles.scss | 29 ++++++++ views/admin/repos.erb | 2 +- views/layout.erb | 2 +- views/ldap_signin.erb | 14 ++++ views/select_openid_provider.erb | 37 ++++++--- 12 files changed, 217 insertions(+), 34 deletions(-) create mode 100644 doc/ldap.markdown create mode 100644 public/coffee/ldap_signin.coffee create mode 100644 views/ldap_signin.erb diff --git a/Gemfile b/Gemfile index f5e1258c..c5f2098d 100644 --- a/Gemfile +++ b/Gemfile @@ -71,6 +71,9 @@ gem "therubyracer" # For scripting system setup. gem "terraform" +# For LDAP signins +gem "net-ldap" + group :test do # NOTE(caleb): require rr >= 1.0.3 and scope >= 0.2.3 for mutual compatibility gem "rr", ">= 1.0.3" diff --git a/Gemfile.lock b/Gemfile.lock index 6b67ba16..6f3919e5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -57,6 +57,7 @@ GEM multi_json (1.1.0) mustache (0.99.4) mysql2 (0.3.11) + net-ldap (0.3.1) net-scp (1.0.4) net-ssh (>= 1.99.1) net-ssh (2.2.2) @@ -169,6 +170,7 @@ DEPENDENCIES methodchain mustache mysql2 + net-ldap nokogiri pathological pinion diff --git a/barkeep_server.rb b/barkeep_server.rb index d2176e7e..4461e143 100644 --- a/barkeep_server.rb +++ b/barkeep_server.rb @@ -3,6 +3,7 @@ require "bourbon" require "coffee-script" +require "net/ldap" require "methodchain" require "nokogiri" require "open-uri" @@ -92,6 +93,7 @@ class BarkeepServer < Sinatra::Base "/vendor/flot/jquery.flot.pie.min.js" ] pinion.create_bundle :user_settings_app_js, :concatenate_and_uglify_js, ["/coffee/user_settings.js"] + pinion.create_bundle :ldap_signin_app_js, :concatenate_and_uglify_js, ["/coffee/ldap_signin.js"] end helpers Pinion::SinatraHelpers @@ -101,7 +103,7 @@ class BarkeepServer < Sinatra::Base set :views, "views" enable :sessions - raise "You must have an OpenID provider defined in OPENID_PROVIDERS." if OPENID_PROVIDERS.empty? + raise "You must have an OpenID or LDAP provider defined in OPENID_PROVIDERS or LDAP_PROVIDERS, respectively." if OPENID_PROVIDERS.empty? and LDAP_PROVIDERS.empty? configure :development do STDOUT.sync = true # Flush any output right away when running via Foreman. @@ -167,6 +169,15 @@ def ensure_required_params(*required_params) end end end + + def start_session(name, email) + session[:email] = email + unless User.find(:email => email) + # If there are no admin users yet, make the first user to log in the first admin. + permission = User.find(:permission => "admin").nil? ? "admin" : "normal" + User.new(:email => email, :name => name, :permission => permission).save + end + end end before do @@ -193,9 +204,7 @@ def ensure_required_params(*required_params) # Save url to return to it after login completes. session[:login_started_url] = request.url - redirect(OPENID_PROVIDERS_ARRAY.size == 1 ? - get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) : - "/signin/select_openid_provider") + redirect get_signin_redirect() end end @@ -206,13 +215,11 @@ def ensure_required_params(*required_params) get "/signin" do session.clear session[:login_started_url] = request.referrer - redirect(OPENID_PROVIDERS_ARRAY.size == 1 ? - get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) : - "/signin/select_openid_provider") + redirect get_signin_redirect() end get "/signin/select_openid_provider" do - erb :select_openid_provider, :locals => { :openid_providers => OPENID_PROVIDERS_ARRAY } + erb :select_openid_provider, :locals => { :openid_providers => OPENID_PROVIDERS_ARRAY, :ldap_providers => LDAP_PROVIDERS } end # Users navigate to here from select_openid_provider. @@ -223,6 +230,89 @@ def ensure_required_params(*required_params) redirect get_openid_login_redirect(provider) end + get "/signin/signin_using_ldap_provider" do + provider = LDAP_PROVIDERS[params[:provider_id].to_i] + halt 400, "LDAP provider not found." unless provider + erb :ldap_signin, :locals => { :provider_id => params[:provider_id], :name => provider[:name] || provider[:host] } + end + + post "/signin/ldap_authenticate" do + provider = LDAP_PROVIDERS[params[:provider_id].to_i] + halt 400, "LDAP provider not found." unless provider + + begin + Timeout::timeout(5) do + conn_params = { :host => provider[:host], :port => provider[:port] } + auth_params = { :method => provider[:method] } + + if provider.has_key?(:username) + # Here we use separate credentials, i.e. those in the config file, to query + # the LDAP server in order to locate the correct user entry. + conn = Net::LDAP.new(conn_params.merge({ + :auth => auth_params.merge({ + :username => provider[:username], + :password => provider[:password] })})) + + if conn.bind + # Using the field name provided in the config file, search for the entry + # whose value in that field is equal to the username provided on the web + # form. + filter = Net::LDAP::Filter.eq(provider[:uid], params[:username]) + entries = conn.search(:base => provider[:base], :filter => filter) + if entries.nil? or entries.empty? + halt 401, "Invalid username or password" + elsif entries.size > 1 + # TODO log error message for duplicate user state + halt 401, "LDAP misconfigured" + end + + conn2 = Net::LDAP.new(conn_params.merge({ + :auth => auth_params.merge({ + :username => entries.first.dn, + :password => params[:password] })})) + + if conn2.bind + start_session(entries.first.cn.first, entries.first.mail.first) + session[:login_started_url] + else + halt 401, "Invalid username or password" + end + else + halt 401, "Barkeep could not bind as query user" + end + else + # This is the simpler form of LDAP authentication, wherein we simply + # attempt to bind using the credentials provided on the web form in order + # to validate the user. + user_dn = "#{provider[:uid]}=#{params[:username]},#{provider[:base]}\n" + conn = Net::LDAP.new(conn_params.merge({ + :auth => auth_params.merge({ + :username => user_dn, + :password => params[:password] })})) + + if conn.bind + filter = Net::LDAP::Filter.contains(provider[:uid], params[:username]) + entries = conn.search(:base => provider[:base], :filter => filter) + if entries.nil? or entries.empty? + halt 401, "Invalid username or password" + elsif entries.size > 1 + # TODO log error message for duplicate user state + halt 401, "LDAP misconfigured" + end + + start_session(entries.first.cn.first, entries.first.mail.first) + session[:login_started_url] + else + halt 401, "Invalid username or password" + end + end + end + rescue Timeout::Error => e + session.clear + halt 401, "LDAP server took too long to respond" + end + end + # Handle login complete from openid provider. get "/signin/complete" do @openid_consumer ||= OpenID::Consumer.new(session, @@ -241,12 +331,7 @@ def ensure_required_params(*required_params) halt 401, "Your email #{email} is not authorized to login to Barkeep." end end - session[:email] = email - unless User.find(:email => email) - # If there are no admin users yet, make the first user to log in the first admin. - permission = User.find(:permission => "admin").nil? ? "admin" : "normal" - User.new(:email => email, :name => email, :permission => permission).save - end + start_session(email, email) redirect session[:login_started_url] || "/" end end @@ -539,6 +624,17 @@ def ensure_required_params(*required_params) def logged_in?() current_user && !current_user.demo? end + def get_signin_redirect() + if LDAP_PROVIDERS.empty? + OPENID_PROVIDERS_ARRAY.size == 1 ? + get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) : + "/signin/select_openid_provider" + else + # actually should redirect to LDAP signin page + "/signin/select_openid_provider" + end + end + # Construct redirect url to google openid. def get_openid_login_redirect(openid_provider_url) @openid_consumer ||= OpenID::Consumer.new(session, diff --git a/doc/ldap.markdown b/doc/ldap.markdown new file mode 100644 index 00000000..cc39e75e --- /dev/null +++ b/doc/ldap.markdown @@ -0,0 +1 @@ +Barkeep supports user authentication via LDAP. diff --git a/environment.rb b/environment.rb index 131a69c9..30d74f35 100644 --- a/environment.rb +++ b/environment.rb @@ -23,6 +23,9 @@ # Besides Google, another popular OpenID endpoint is https://me.yahoo.com OPENID_PROVIDERS = "https://www.google.com/accounts/o8/ud" +# A list of hashes of LDAP parameters for signing in your users. For documentation, see doc/ldap.markdown. +LDAP_PROVIDERS = [] + # This is the read-only demo mode which is used in the Barkeep demo linked from getbarkeep.com. # Most production deployments will not want to enable the demo mode, but we want it while developing. ENABLE_READONLY_DEMO_MODE = true diff --git a/public/coffee/ldap_signin.coffee b/public/coffee/ldap_signin.coffee new file mode 100644 index 00000000..450435a6 --- /dev/null +++ b/public/coffee/ldap_signin.coffee @@ -0,0 +1,26 @@ +window.LdapSignin = + init: -> + KeyboardShortcuts.createShortcutContext($("#ldapSignin input")) + + $("#ldapSigninForm").submit (e) => + e.preventDefault() + signinParams = { "provider_id": $("#ldapSignin #providerId").val(), "username": $("#ldapSignin #userName").val(), "password": $("#ldapSignin #password").val() } + $.ajax({ + "type": "POST", + url: "/signin/ldap_authenticate", + data: signinParams, + success: (e) -> + window.location.replace(e) + error: (jqXHR, textStatus, errorThrown) => + @showErrorMessage(if jqXHR.responseText? and $.trim(jqXHR.responseText).length > 0 then jqXHR.responseText else "Unknown error") + console.log("error:") + console.log(jqXHR) + console.log(textStatus) + console.log(errorThrown) + }) + + showErrorMessage: (message) -> + $("#errorMessage").show() + $("#errorMessage").html(message) + +$(document).ready(-> LdapSignin.init()) diff --git a/public/css/admin.scss b/public/css/admin.scss index d33159e6..c278a9b5 100644 --- a/public/css/admin.scss +++ b/public/css/admin.scss @@ -25,14 +25,6 @@ } #repos { - #confirmationMessage { - margin: 10px 0; - padding: 10px; - background-color: #FFF7B1; - border: 1px dashed #333; - display: none; - } - table { width: 100%; border-collapse: collapse; diff --git a/public/css/styles.scss b/public/css/styles.scss index 6d261b78..b064f2b6 100644 --- a/public/css/styles.scss +++ b/public/css/styles.scss @@ -22,6 +22,14 @@ body { strong { font-weight: bold; } +.confirmationMessage { + margin: 10px 0; + padding: 10px; + background-color: #FFF7B1; + border: 1px dashed #333; + display: none; +} + // There are cases where we want focus for tooltips but we don't want to show an outline .noFocusOutline { // FF @@ -612,3 +620,24 @@ table.commitsList, table.authorList { } } } + + +#ldapSignin { + width: 500px; + margin: 0 auto; + form { + margin-top: 12px; + } + label { + font-weight: bold; + font-size: 14px; + } + input.text { + width: 200px; + font-size: 18px; + margin: 0 0 8px 4px; + } + input.standard { + margin-top: 8px; + } +} diff --git a/views/admin/repos.erb b/views/admin/repos.erb index 172478be..a6fd5f46 100644 --- a/views/admin/repos.erb +++ b/views/admin/repos.erb @@ -10,7 +10,7 @@ <%= admin_page_breadcrumb "Manage repositories" %> -
+

Add new repo

diff --git a/views/layout.erb b/views/layout.erb index 13f43694..9f1ee397 100644 --- a/views/layout.erb +++ b/views/layout.erb @@ -61,7 +61,7 @@
<% if current_user %> - <%= current_user.email %> | + <%= current_user.name %> | <% if current_user.admin? %> ">Admin | <% end %> diff --git a/views/ldap_signin.erb b/views/ldap_signin.erb new file mode 100644 index 00000000..25386cf5 --- /dev/null +++ b/views/ldap_signin.erb @@ -0,0 +1,14 @@ +<%= js_bundle(:ldap_signin_app_js) %> + +
+

Sign in with LDAP on <%= name %>

+
+ +
+
+
+
+
+ +
+
diff --git a/views/select_openid_provider.erb b/views/select_openid_provider.erb index c199a4e8..3797afed 100644 --- a/views/select_openid_provider.erb +++ b/views/select_openid_provider.erb @@ -4,13 +4,30 @@ %>

Select one of these services to sign in with:

-
    - <% openid_providers.each_with_index do |provider_url, index| %> -
  1. - - <%= URI.parse(provider_url).host %> - -
    2. - <% end %> -
-
\ No newline at end of file + + <% if not openid_providers.empty? %> +

OpenID

+
    + <% openid_providers.each_with_index do |provider_url, index| %> +
  1. + + <%= URI.parse(provider_url).host %> + +
    2. + <% end %> +
+ <% end %> + + <% if not ldap_providers.empty? %> +

LDAP

+
    + <% ldap_providers.each_with_index do |provider, index| %> +
  1. + + <%= provider.has_key?(:name) ? provider[:name] : provider[:host] %> + +
    2. + <% end %> +
+ <% end %> +
From a9c840077a4bce271342b92938434cc89664020b Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Thu, 3 Jan 2013 12:09:06 -0600 Subject: [PATCH 2/4] Show first LDAP signin page if it's the only provider. --- barkeep_server.rb | 13 +++++++------ test/integration/barkeep_server_integration_test.rb | 2 +- ...enid_provider.erb => select_signin_provider.erb} | 0 3 files changed, 8 insertions(+), 7 deletions(-) rename views/{select_openid_provider.erb => select_signin_provider.erb} (100%) diff --git a/barkeep_server.rb b/barkeep_server.rb index 4461e143..35ce73a3 100644 --- a/barkeep_server.rb +++ b/barkeep_server.rb @@ -218,11 +218,11 @@ def start_session(name, email) redirect get_signin_redirect() end - get "/signin/select_openid_provider" do - erb :select_openid_provider, :locals => { :openid_providers => OPENID_PROVIDERS_ARRAY, :ldap_providers => LDAP_PROVIDERS } + get "/signin/select_signin_provider" do + erb :select_signin_provider, :locals => { :openid_providers => OPENID_PROVIDERS_ARRAY, :ldap_providers => LDAP_PROVIDERS } end - # Users navigate to here from select_openid_provider. + # Users navigate to here from select_signin_provider. # - provider_id: an integer indicating which provider from OPENID_PROVIDERS_ARRAY to use for authentication. get "/signin/signin_using_openid_provider" do provider = OPENID_PROVIDERS_ARRAY[params[:provider_id].to_i] @@ -628,10 +628,11 @@ def get_signin_redirect() if LDAP_PROVIDERS.empty? OPENID_PROVIDERS_ARRAY.size == 1 ? get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) : - "/signin/select_openid_provider" + "/signin/select_signin_provider" else - # actually should redirect to LDAP signin page - "/signin/select_openid_provider" + LDAP_PROVIDERS.size == 1 and OPENID_PROVIDERS_ARRAY.empty? ? + "/signin/signin_using_ldap_provider?provider_id=0" : + "/signin/select_signin_provider" end end diff --git a/test/integration/barkeep_server_integration_test.rb b/test/integration/barkeep_server_integration_test.rb index 5a5852c5..55288f0a 100644 --- a/test/integration/barkeep_server_integration_test.rb +++ b/test/integration/barkeep_server_integration_test.rb @@ -19,7 +19,7 @@ def server() SERVER end should "show a selection of OpenID providers" do # This page is reached when the user tries to log in and the server is configured with more than one # openID provider. Our default configuration just has google as the OpenID provider. - get "/signin/select_openid_provider" + get "/signin/select_signin_provider" assert_status 200 assert_equal ["www.google.com"], dom_response.css("#openIdProviders li").map(&:text).map(&:strip) end diff --git a/views/select_openid_provider.erb b/views/select_signin_provider.erb similarity index 100% rename from views/select_openid_provider.erb rename to views/select_signin_provider.erb From fb1f45b628cb595fc562c594b3797a6c1c8bb551 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Sat, 12 Jan 2013 14:41:25 -0600 Subject: [PATCH 3/4] Add /favicon.ico to unauthenticated routes, so it doesn't end up as the referrer when we log in with LDAP. --- barkeep_server.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barkeep_server.rb b/barkeep_server.rb index 35ce73a3..f5938566 100644 --- a/barkeep_server.rb +++ b/barkeep_server.rb @@ -40,7 +40,7 @@ NODE_MODULES_BIN_PATH = "./node_modules/.bin" OPENID_AX_EMAIL_SCHEMA = "http://axschema.org/contact/email" -UNAUTHENTICATED_ROUTES = ["/signin", "/signout", "/inspire", "/statusz", "/api/"] +UNAUTHENTICATED_ROUTES = ["/signin", "/signout", "/inspire", "/statusz", "/api/", "/favicon.ico"] # NOTE(philc): Currently we let you see previews of individual commits and the code review stats without # being logged in, as a friendly UX. When we flesh out our auth model, we should intentionally make this # configurable. From f21c6a4412272544abe9f8db46519fdfe9f14566 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Sat, 12 Jan 2013 18:32:57 -0600 Subject: [PATCH 4/4] Added LDAP documentation, cleaned up get_signin_redirect(). --- barkeep_server.rb | 12 ++++---- doc/ldap.markdown | 72 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/barkeep_server.rb b/barkeep_server.rb index f5938566..ce855853 100644 --- a/barkeep_server.rb +++ b/barkeep_server.rb @@ -625,14 +625,12 @@ def start_session(name, email) def logged_in?() current_user && !current_user.demo? end def get_signin_redirect() - if LDAP_PROVIDERS.empty? - OPENID_PROVIDERS_ARRAY.size == 1 ? - get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) : - "/signin/select_signin_provider" + if OPENID_PROVIDERS_ARRAY.size == 1 and LDAP_PROVIDERS.empty? + get_openid_login_redirect(OPENID_PROVIDERS_ARRAY.first) + elsif OPENID_PROVIDERS_ARRAY.empty? and LDAP_PROVIDERS.size == 1 + "/signin/signin_using_ldap_provider?provider_id=0" else - LDAP_PROVIDERS.size == 1 and OPENID_PROVIDERS_ARRAY.empty? ? - "/signin/signin_using_ldap_provider?provider_id=0" : - "/signin/select_signin_provider" + "/signin/select_signin_provider" end end diff --git a/doc/ldap.markdown b/doc/ldap.markdown index cc39e75e..17af7133 100644 --- a/doc/ldap.markdown +++ b/doc/ldap.markdown @@ -1 +1,71 @@ -Barkeep supports user authentication via LDAP. +Barkeep's LDAP user authentication has been tested against OpenLDAP and Active Directory, but in a limited permutation of setups. If you have issues authenticating against a particular setup, please check your server configuration, talk to your sysadmin, and check the [open issues list](https://github.com/ooyala/barkeep/issues). If you still have trouble, please [open a new issue](https://github.com/ooyala/barkeep/issues/new). + +## Unsupported features +LDAP group authentication is not supported, nor is any form of SSL encryption. Pull requests are welcome! + +## Configuration +Add a hash to the list in `LDAP_PROVIDERS` in `environment.rb`. Barkeep supports multiple LDAP providers, and can be used in conjunction with any number of OpenID providers. However, usernames must be unique across all providers. + +### Parameters +* `:name` : The name of this provider that will be displayed above the signin form. If multiple providers are specified, it will be used on the provider selection page. +* `:host` : The host name or IP address of the LDAP server. +* `:port` : The port to talk to on the LDAP server. +* `:base` : The RDN under which user accounts may be found. +* `:method` : The authentication type to use when binding. According to the net-ldap source code, it appears that supported values are `:simple`, `:anon`, `:anonymous`, `:sasl`, and `:gss_spnego`. **LDAP authentication has only been tested with `:simple`.** +* `:uid` : This can mean two different things, depending on whether `:username` is provided. + + If `:username` is not provided, it is used as the top-level attribute name used to construct the user's DN. It's probably something like `"CN"` or `"uid"`. For example, if `:uid` is `"CN"`, `:base` is `"OU=Developers,dc=company,dc=com"`, and the user provided `Joe Smith` as their username, then Barkeep will attempt to bind as `CN=Joe Smith,OU=Developers,dc=company,dc=com` with the password that the user provided. + + On the other hand, if `:username` is provided, `uid` is interpreted as the LDAP attribute to search for among the entries under `:base`. This is how Active Directory authentication via LDAP can work. Suppose `:uid` is set to `"employee_name"`, the user provided `jsmith` as their username, and `:base` is as before. Then Barkeep will bind as `:username` with `:password`, query the LDAP server for all entries under `OU=Developers,dc=company,dc=com`, and find the first entry whose attribute `employee_name` is `jsmith`. Let's say this entry is again `"CN=Joe Smith,OU=Developers,dc=company,dc=com"`. Barkeep will then attempt to bind as *that* DN, with the password the user provided. When authenticating with Active Directory, use `"sAMAccountName"` for `:uid`. If there is a cleaner way to accomplish these use cases, please do let us know. +* `:username` : (optional) The DN of to bind as when executing a query under `:base` for `:uid`. For an anonymous bind, specify as `""`. Active Directory also seems to let you get away with `"name@company.com", in lieu of a DN. +* `:password` : (optional) The password to go with `:username`. For an anonymous bind, specify as `""`. + +## Examples + +### OpenLDAP + LDAP_PROVIDERS = [{ + :name => "OpenLDAP", + :host => "ldap.example.com", + :port => 389, + :base => "OU=users,DC=example,DC=com", + :method => :simple, + :uid => "CN" + }] + +### Active Directory + +#### Bind as DN + LDAP_PROVIDERS = [{ + :name => "Active Directory", + :host => "ad.example.com", + :port => 389, + :base => "OU=users,DC=example,DC=com", + :method => :simple, + :uid => "sAMAccountName", + :username => "CN=Account Query Bot,OU=users,DC=example,DC=com", + :password => "pass123" + }] + +#### Bind as username + LDAP_PROVIDERS = [{ + :name => "Active Directory", + :host => "ad.example.com", + :port => 389, + :base => "OU=users,DC=example,DC=com", + :method => :simple, + :uid => "sAMAccountName", + :username => "account_query_bot@example.com", + :password => "pass123" + }] + +#### Anonymous bind (untested) + LDAP_PROVIDERS = [{ + :name => "Active Directory", + :host => "ad.example.com", + :port => 389, + :base => "OU=users,DC=example,DC=com", + :method => :simple, + :uid => "sAMAccountName", + :username => "", + :password => "" + }]