From caed6e29e869b4f2e172da1c4aa32eb2dae7e2cd Mon Sep 17 00:00:00 2001 From: Timothy Allen <Timothy.Allen@optiver.com.au> Date: Mon, 19 Jan 2015 10:53:50 +1100 Subject: [PATCH] Only trust .bind_user() with a non-empty password. There are two reasons one migh call .bind_user(): you might want to connect to an LDAP server and perform operations on that user's behalf, or you might want to check whether a username and password pair are valid. Unfortunately, if you give the password as an empty string, many LDAP servers will grant you access as an anonymous user, regardless of the username you ask for, so just because .bind_user() accepts a username/password pair doesn't mean that's the correct password for that user. Therefore: - I've added a warning to the bind_user() docstring. - I've modified the `basic_auth_required()` decorator to guard against empty passwords. - I've modified the various code examples to guard against empty passwords. --- examples/blueprints/blueprints/core/views.py | 2 +- examples/groups/app.py | 2 +- flask_simpleldap/__init__.py | 15 ++++++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/examples/blueprints/blueprints/core/views.py b/examples/blueprints/blueprints/core/views.py index ad4a806..cbb4dd5 100644 --- a/examples/blueprints/blueprints/core/views.py +++ b/examples/blueprints/blueprints/core/views.py @@ -18,7 +18,7 @@ def login(): user = request.form['user'] passwd = request.form['passwd'] test = ldap.bind_user(user, passwd) - if test is None: + if test is None or passwd == '': return 'Invalid credentials' else: session['user_id'] = request.form['user'] diff --git a/examples/groups/app.py b/examples/groups/app.py index cbda3c6..ceeb1cf 100644 --- a/examples/groups/app.py +++ b/examples/groups/app.py @@ -36,7 +36,7 @@ def login(): user = request.form['user'] passwd = request.form['passwd'] test = ldap.bind_user(user, passwd) - if test is None: + if test is None or passwd = '': return 'Invalid credentials' else: session['user_id'] = request.form['user'] diff --git a/flask_simpleldap/__init__.py b/flask_simpleldap/__init__.py index 74fdad5..2287de3 100644 --- a/flask_simpleldap/__init__.py +++ b/flask_simpleldap/__init__.py @@ -123,6 +123,15 @@ class LDAP(object): """Attempts to bind a user to the LDAP server using the credentials supplied. + .. note:: + + Many LDAP servers will grant anonymous access if ``password`` is + the empty string, causing this method to return :obj:`True` no + matter what username is given. If you want to use this method to + validate a username and password, rather than actually connecting + to the LDAP server as a particular user, make sure ``password`` is + not empty. + :param str username: The username to attempt to bind with. :param str password: The password of the username we're attempting to bind with. @@ -319,7 +328,11 @@ class LDAP(object): req_username = request.authorization.username req_password = request.authorization.password - if req_username is None or req_password is None: + # Many LDAP servers will grant you anonymous access if you log in + # with an empty password, even if you supply a non-anonymous user + # ID, causing .bind_user() to return True. Therefore, only accept + # non-empty passwords. + if req_username in ['', None] or req_password in ['', None]: current_app.logger.debug('Got a request without auth data') return make_auth_required_response() -- GitLab