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

Proposed by Cees de Groot
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 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

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
=== modified file 'webapp/graphite/account/ldapBackend.py'
--- webapp/graphite/account/ldapBackend.py 2011-09-16 08:10:42 +0000
+++ webapp/graphite/account/ldapBackend.py 2013-06-10 17:12:26 +0000
@@ -22,7 +22,10 @@
22 try:22 try:
23 conn = ldap.initialize(settings.LDAP_URI)23 conn = ldap.initialize(settings.LDAP_URI)
24 conn.protocol_version = ldap.VERSION324 conn.protocol_version = ldap.VERSION3
25 conn.simple_bind_s( settings.LDAP_BASE_USER, settings.LDAP_BASE_PASS )25 conn.start_tls_s()
26 bind_user = settings.LDAP_BASE_USER % username if "%s" in settings.LDAP_BASE_USER else settings.LDAP_BASE_USER
27 bind_pass = settings.LDAP_BASE_PASS % password if "%s" in settings.LDAP_BASE_PASS else settings.LDAP_BASE_PASS
28 conn.simple_bind_s( bind_user, bind_pass )
26 except ldap.LDAPError:29 except ldap.LDAPError:
27 traceback.print_exc()30 traceback.print_exc()
28 return None31 return None
2932
=== modified file 'webapp/graphite/local_settings.py.example'
--- webapp/graphite/local_settings.py.example 2013-03-21 11:17:49 +0000
+++ webapp/graphite/local_settings.py.example 2013-06-10 17:12:26 +0000
@@ -98,8 +98,19 @@
98# OR98# OR
99#LDAP_URI = "ldaps://ldap.mycompany.com:636"99#LDAP_URI = "ldaps://ldap.mycompany.com:636"
100#LDAP_SEARCH_BASE = "OU=users,DC=mycompany,DC=com"100#LDAP_SEARCH_BASE = "OU=users,DC=mycompany,DC=com"
101#
102# A hardcoded base user/pass looks like this:
103#
101#LDAP_BASE_USER = "CN=some_readonly_account,DC=mycompany,DC=com"104#LDAP_BASE_USER = "CN=some_readonly_account,DC=mycompany,DC=com"
102#LDAP_BASE_PASS = "readonly_account_password"105#LDAP_BASE_PASS = "readonly_account_password"
106#
107# ...however, it's often not a good idea. If you include "%s" in the
108# base user/pass, then they'll get expanded with what the user typed
109# in. This lets you effectively bind with the user's own account.
110#
111#LDAP_BASE_USER = "CN=%s,DC=mycompany,DC=com"
112#LDAP_BASE_PASS = "%s"
113#
103#LDAP_USER_QUERY = "(username=%s)" #For Active Directory use "(sAMAccountName=%s)"114#LDAP_USER_QUERY = "(username=%s)" #For Active Directory use "(sAMAccountName=%s)"
104#115#
105# If you want to further customize the ldap connection options you should116# If you want to further customize the ldap connection options you should

Subscribers

People subscribed via source and target branches

to all changes: