Merge lp:~casedeg/graphite/ldap-fix into lp:graphite

Proposed by Cees de Groot on 2013-06-10
Status: Needs review
Proposed branch: lp:~casedeg/graphite/ldap-fix
Merge into: lp:graphite
Diff against target: 39 lines (+15/-1)
2 files modified
webapp/graphite/account/ldapBackend.py (+4/-1)
webapp/graphite/local_settings.py.example (+11/-0)
To merge this branch: bzr merge lp:~casedeg/graphite/ldap-fix
Reviewer Review Type Date Requested Status
graphite-dev 2013-06-10 Pending
Review via email: mp+168499@code.launchpad.net

Description of the change

We had issues hooking up graphite to LDAP because our LDAP directory doesn't allow anonymous browsing and a R/O account with a hardcoded password is frowned up by the admins.

Therefore, I created a change that allows you to configure the LDAP backend so that the user's own credentials are used for the initial bind() call. This is also how I remember (vaguely, it's been a while ago since I toyed with LDAP ;-)) how LDAP auth should be done.

To post a comment you must log in.

Unmerged revisions

951. By Cees de Groot on 2013-06-10

Add the possibility to bind to LDAP with the user's account.

This patch interprets LDAP_BASE_USER and LDAP_BASE_PASS, looking
for a "%s" in the configured values. If encountered, the %s is
expanded with the user's entered username or password, respecitely.

In this way, you can bind using the user's credentials to the
LDAP server, which is arguably more secure than hardcoding a
special LDAP R/O account and password or allowing anonymous
browsing.

The patch is fully backwards compatible.

Also in the patch, a start_tls_s() call. This should work against
any recently up-to-date LDAP server and helps to further prop
up security.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'webapp/graphite/account/ldapBackend.py'
2--- webapp/graphite/account/ldapBackend.py 2011-09-16 08:10:42 +0000
3+++ webapp/graphite/account/ldapBackend.py 2013-06-10 17:12:26 +0000
4@@ -22,7 +22,10 @@
5 try:
6 conn = ldap.initialize(settings.LDAP_URI)
7 conn.protocol_version = ldap.VERSION3
8- conn.simple_bind_s( settings.LDAP_BASE_USER, settings.LDAP_BASE_PASS )
9+ conn.start_tls_s()
10+ bind_user = settings.LDAP_BASE_USER % username if "%s" in settings.LDAP_BASE_USER else settings.LDAP_BASE_USER
11+ bind_pass = settings.LDAP_BASE_PASS % password if "%s" in settings.LDAP_BASE_PASS else settings.LDAP_BASE_PASS
12+ conn.simple_bind_s( bind_user, bind_pass )
13 except ldap.LDAPError:
14 traceback.print_exc()
15 return None
16
17=== modified file 'webapp/graphite/local_settings.py.example'
18--- webapp/graphite/local_settings.py.example 2013-03-21 11:17:49 +0000
19+++ webapp/graphite/local_settings.py.example 2013-06-10 17:12:26 +0000
20@@ -98,8 +98,19 @@
21 # OR
22 #LDAP_URI = "ldaps://ldap.mycompany.com:636"
23 #LDAP_SEARCH_BASE = "OU=users,DC=mycompany,DC=com"
24+#
25+# A hardcoded base user/pass looks like this:
26+#
27 #LDAP_BASE_USER = "CN=some_readonly_account,DC=mycompany,DC=com"
28 #LDAP_BASE_PASS = "readonly_account_password"
29+#
30+# ...however, it's often not a good idea. If you include "%s" in the
31+# base user/pass, then they'll get expanded with what the user typed
32+# in. This lets you effectively bind with the user's own account.
33+#
34+#LDAP_BASE_USER = "CN=%s,DC=mycompany,DC=com"
35+#LDAP_BASE_PASS = "%s"
36+#
37 #LDAP_USER_QUERY = "(username=%s)" #For Active Directory use "(sAMAccountName=%s)"
38 #
39 # If you want to further customize the ldap connection options you should

Subscribers

People subscribed via source and target branches

to all changes: