Bug 712859 - LDAP authentication broken
Summary: LDAP authentication broken
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Pulp
Classification: Retired
Component: z_other
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
: Sprint 25
Assignee: Pradeep Kilambi
QA Contact: Preethi Thomas
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-06-13 12:05 UTC by Chris St. Pierre
Modified: 2012-02-24 20:10 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-02-24 20:10:35 UTC
Embargoed:


Attachments (Terms of Use)
Fix arguments passed to LDAPConnection constructor (533 bytes, application/octet-stream)
2011-06-13 12:05 UTC, Chris St. Pierre
no flags Details
Much better LDAP patch (12.54 KB, patch)
2011-06-13 19:36 UTC, Chris St. Pierre
no flags Details | Diff
New AuthenticationLDAP page (3.43 KB, text/plain)
2011-06-13 20:15 UTC, Chris St. Pierre
no flags Details

Description Chris St. Pierre 2011-06-13 12:05:30 UTC
Created attachment 504423 [details]
Fix arguments passed to LDAPConnection constructor

Description of problem:

LDAP authentication is broken in 0.0.181; it produces a traceback:

2011-06-13 07:44:52,641 [ERROR][Dummy-2] @ base.py:59 - Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/pulp/server/webservices/controllers/base.py", line 55, in report_error
    return method(self, *args, **kwargs)
  File "/usr/lib/python2.4/site-packages/pulp/server/webservices/controllers/base.py", line 104, in _auth_decorator
    user = check_user_cert(cert_pem)
  File "/usr/lib/python2.4/site-packages/pulp/server/auth/authentication.py", line 151, in check_user_cert
    return check_username_password(username)
  File "/usr/lib/python2.4/site-packages/pulp/server/auth/authentication.py", line 124, in check_username_password
    return _check_username_password_ldap(username, password)
  File "/usr/lib/python2.4/site-packages/pulp/server/auth/authentication.py", line 83, in _check_username_password_ldap
    status = ldap_server.lookup_user(ldap_base, username)
  File "/usr/lib/python2.4/site-packages/pulp/server/LDAPConnection.py", line 108, in lookup_user
    result = self.lconn.search_s(baseDN, scope, filter)
  File "/usr/lib64/python2.4/site-packages/ldap/ldapobject.py", line 481, in search_s
    return self.search_ext_s(base,scope,filterstr,attrlist,attrsonly,None,None,timeout=self.timeout)
  File "/usr/lib64/python2.4/site-packages/ldap/ldapobject.py", line 474, in search_ext_s
    msgid = self.search_ext(base,scope,filterstr,attrlist,attrsonly,serverctrls,clientctrls,timeout,sizelimit)
  File "/usr/lib64/python2.4/site-packages/ldap/ldapobject.py", line 470, in search_ext
    timeout,sizelimit,
  File "/usr/lib64/python2.4/site-packages/ldap/ldapobject.py", line 94, in _ldap_call
    result = func(*args,**kwargs)
SERVER_DOWN: {'desc': "Can't contact LDAP server"}

The issue appears to be that the arguments to the pulp.server.LDAPConnection.LDAPConnection object are wrong.  A patch is attached.

Version-Release number of selected component (if applicable):

0.0.181

How reproducible:

Always.

Steps to Reproduce:
1. Uncomment the [ldap] section in pulp.conf.  Put in a legitimate LDAP server and base.
2. Watch as stack traces are spewed all over pulp.log.
  
Actual results:

Pulp spews stack traces.

Expected results:

Pulp authenticates against LDAP.

Additional info:

Comment 1 Chris St. Pierre 2011-06-13 15:24:15 UTC
Upon further investigation, LDAP authn is more broken than I thought.  I'm working on a much larger patch, so you can hold off on this one for now.  Thanks.

Comment 2 Chris St. Pierre 2011-06-13 19:36:06 UTC
Created attachment 504526 [details]
Much better LDAP patch

Comment 3 Chris St. Pierre 2011-06-13 19:36:38 UTC
I've attached a newer and better patch to make LDAP authentication
work.  Some notes:

When a new user authenticates via LDAP, they are added to the pulp
database with no password.  If someone tries to authenticate with no
password, their attempt will fail and then get passed to LDAP.  They
are added with the role specified in 'ldap.default_role' in pulp.conf;
this _must_ be set, or they will be added with no role, which does not
permit logging in.

LDAP users can also be added manually with
"pulp-admin user create --username=<username> --ldap", which creates a
user without a password.  This could be used to assign a non-default
initial role to a user.

If LDAP is enabled, all authentications that fail against the Pulp
database will subsequently try against LDAP.  (Before, it was one or
the other.)

Adding LDAP to the Pulp database does result in more cruft.  (I.e.,
old users will be left in the database.)  This sucks.  But given the
Pulp model, this was the most reasonable way I could determine to make
this work.  If a user is removed from LDAP, they will no longer be
able to authenticate to Pulp, since they have no password.

Other things I fixed/added:

- "pulp-admin auth login" can now prompt for a password (rather than
  taking it only on the command line)
- Added support for LDAP/TLS
- Added support for an LDAP filter in pulp.conf to restrict access to
  pulp; support for LDAP groups as a limiter should probably also be
  added
- Fixed logging from LDAPConnection.py
- Removed extra "Usage:" from client usage messages
- LDAP authentication now searches for a user rather than just
  concatenating "uid=%s,<base>", so the user doesn't have to be a
  direct child of the search base.  (lookup_user() already did this,
  but authenticate_user() didn't.)
- Fixed arguments to LDAPConnection constructor (as in original patch)

The patch should apply cleanly to HEAD as of this posting.

Comment 4 Chris St. Pierre 2011-06-13 20:15:21 UTC
Created attachment 504538 [details]
New AuthenticationLDAP page

I've attached another file that should serve to replace https://fedorahosted.org/pulp/wiki/AuthenticationLDAP.  I'm not terrifically familiar with Trac's wiki format, so I hope I don't have any glaring errors in the markup.

Comment 5 Pradeep Kilambi 2011-06-14 18:53:02 UTC
Hi Chris:

Thanks for the patch. I'll review, test and apply your patch to master if all seems well. Feel free to share any test cases you ran that I can double check as well. Will keep you posted

Thanks,

~ Prad

Comment 6 Pradeep Kilambi 2011-06-15 15:28:52 UTC
overall the patch looks good and seem to work as expected. Although I found one issue while testing this which is not related to yours i believe. Here is the bug in case you run into it,

https://bugzilla.redhat.com/show_bug.cgi?id=713493

I'll do some more sanity testing and apply your patch.

Thanks again,

~ Prad

Comment 7 Pradeep Kilambi 2011-06-16 12:43:34 UTC
Patch has been tested and applied 

commit 70421bcbf6c8f82ea94c20787d9ef5c539948a78

I also merged your documentation changes to existing Pulp LDAP integration wikipage.

Thanks again for your contribution!

~ Prad

Comment 8 Jeff Ortel 2011-06-17 21:10:05 UTC
build: 0.192

Comment 9 Preethi Thomas 2011-10-03 18:23:52 UTC
verified
[root@preethi ~]# rpm -q pulp
pulp-0.0.237-1.fc15.noarch

[root@preethi ~]# pulp-admin auth login -u pulpuser1
Enter password: 

[root@preethi ~]# 

# external ldap for user authentication instead of pulp.
[ldap]
uri: ldap://prad.rdu.redhat.com
base: dc=rdu, dc=redhat, dc=com 
tls: no
#default_role: consumer-users
# Uncomment the "filter" directive to set a more restrictive LDAP
# filter to limit the LDAP users who can authenticate to pulp.
#filter: (gidNumber=200)

from pulp.log

2011-10-03 14:31:40,912 13631:140107755525888: pulp.server.auth.authentication:ERROR: authentication:111 This is an ldap user SON([(u'_id', u'0485145d-baa9-4d25-9abe-4ae6545f6270'), (u'name', u'pulpuser1'), (u'roles', []), (u'_ns', u'users'), (u'login', u'pulpuser1'), (u'password', None), (u'id', u'0485145d-baa9-4d25-9abe-4ae6545f6270')])
2011-10-03 14:31:41,173 13631:140107755525888: pulp.server.LDAPConnection:INFO: LDAPConnection:173 Found user with id pulpuser1
2011-10-03 14:31:41,174 13631:140107755525888: pulp.server.LDAPConnection:INFO: LDAPConnection:122 Found user with id pulpuser1 with matching credentials

Comment 10 Preethi Thomas 2012-02-24 20:10:35 UTC
Pulp v1.0 is released
Closed Current Release.


Note You need to log in before you can comment on or make changes to this bug.