Ticket #11501 (new enhancement)
User authentication via LDAP
| Reported by: | rmartinjak | Owned by: | jason, mpatel, was |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | notebook | Keywords: | notebook, auth, ldap |
| Cc: | rkirov, kini, jasonbhill, novoselt | Work issues: | |
| Report Upstream: | Reported upstream. Developers acknowledge bug. | Reviewers: | |
| Authors: | Robin Martinjak | Merged in: | |
| Dependencies: | Stopgaps: |
Description
Support (optional) user authentication via LDAP or other external backends.
This will be useful for i.e. universities (especially once the notebook is scalable).
Users should also be able to search for (i.e. when adding collaborators to a ws) that are available but not yet known to sage.
relevant tickets:
- #11080 (move notebook to flask/wsgi-based notebook)
Attachments
Change History
comment:2 Changed 2 years ago by rmartinjak
the first comment should actually say:
patch is based on Rado Kirov's "rkirov-flask" repo.
requires module "python-ldap" (available via easy_install like in Rado's install instructions)
(and the list of changes is missing newlines, sorry for that)
comment:3 follow-up: ↓ 4 Changed 2 years ago by ijstokes
Copied over from discussion on sage-notebook.
- Which python-ldap instructions from Rado are you referring to? I had trouble easy_installing python-ldap with Sage 4.7, and had to download it, configure setup.cfg by hand, and compile it. I had to get a recent version of OpenLDAP and BerkeleyDB for this to work. If those steps are not required, I'd love to know what I did wrong.
- Why did you do:
class ExtAuthUserManager?(OpenIDUserManager):
It would seem to me that this should be reversed. ExtAuthUserManager? should be the generic class, and OpenIDUserManager should turn into an OpenIDAuth otion in the _auth_methods dict.
- I would suggest making a list of _auth_methods keys which can be ordered so it is predictable which auth_method is being used first, second, etc.
- It doesn't seem very nice to have OpenID-specific references anywhere inside ExtAuthUserManager?. (init and a few OpenID specific methods).
- Can you point me at some details of how the "conf" system works? Where does this come from on disk? What is passed to the constructor?
- I would have the conf file specify the full URL to the LDAP host, rather than have "host" and "port" configs. We use:
which is impossible to configure for in your current code: it is fixed with "ldap".
- Please can you provide an example configuration file? In particular, I'm interested in:
result = self._ldap_search("(%s=%s)" % (self._confldap_username_attrib?, username), attrlist)
My best guess is that "ldap_username_attrib" will typically be "uid", but I can tell you that in our LDAP server the same user may have multiple entries for different LDAP namespaces. We have one namespace for system users, and another for web portal users, so my username ijstokes shows up twice:
uid=ijstokes,cn=users,cn=portal,dc=nebiogrid,dc=org uid=ijstokes,cn=users,cn=accounts,dc=nebiogrid,dc=org
Plus on a big LDAP server this is an expensive search. The admin will know where users are kept, and the search should be limited just to that part of the LDAP tree. In my LDAP implementation, I did a synchronous search as follows:
results = self._ldap_con.search_s("uid=%s,cn=users,cn=portal,dc=nebiogrid,dc=org" % username, ldap.SCOPE_BASE, attrlist=['uid','cn','mail'])
- I think you should just setup the LDAP connection once in the constructor. I *think* Python LDAP will manage the connection intelligently.
- check_password seems a bit convoluted -- it does two binds: once to get the DN, and once to check the password. I'd prefer the model above.
- Sage stores a little bit of additional per-user state: suspended/active and worksheet timeouts or auto-save (can't remember). TTBOMK the OpenID system gets around this by creating a Sage user when someone authenticates for the first time. Do you inherit the same behavior? I can't see where this happens.
comment:4 in reply to: ↑ 3 Changed 2 years ago by rmartinjak
Replying to ijstokes:
Copied over from discussion on sage-notebook. 1. Which python-ldap instructions from Rado are you referring to? I had trouble easy_installing python-ldap with Sage 4.7, and had to download it, configure setup.cfg by hand, and compile it. I had to get a recent version of OpenLDAP and BerkeleyDB for this to work. If those steps are not required, I'd love to know what I did wrong.
My bad, the comment went horribly wrong.
I meant Rado's instruction for getting his flask notebook running (he also uses easy_install from sage's ipython) on top of which my patch is based.
To get python-ldap easy_install'ed you need OpenLDAP's libldap but I can't confirm if that suffices.
- Why did you do: class ExtAuthUserManager(OpenIDUserManager): It would seem to me that this should be reversed. ExtAuthUserManager should be the generic class, and OpenIDUserManager should turn into an OpenIDAuth otion in the _auth_methods dict.
You are right. I'm not experienced with OpenID so I have no idea if/how this can be done.
- I would suggest making a list of _auth_methods keys which can be ordered so it is predictable which auth_method is being used first, second, etc.
I'd favour that too but unfortunately lists of preset values can not be configured in the UI (yet).
- It doesn't seem very nice to have OpenID-specific references anywhere inside ExtAuthUserManager. (init and a few OpenID specific methods).
Indeed. I didn't want to break functionality and OpenID seems to be a quite special case. This should be changed if possible
- Can you point me at some details of how the "conf" system works? Where does this come from on disk? What is passed to the constructor?
A reference to a ServerConfiguration (server_conf.py object is passed. Configuration is done in the browser UI (settings -> notebook settings).
- I would have the conf file specify the full URL to the LDAP host, rather than have "host" and "port" configs. We use: ldaps://hostname
Right! I will change that immediately, completely forgot about ldaps
- Please can you provide an example configuration file? In particular, I'm interested in: result = self._ldap_search ![...] 8. I think you should just setup the LDAP connection once in the constructor. I *think* Python LDAP will manage the connection intelligently.
- check_password seems a bit convoluted -- it does two binds: once to get the DN, and once to check the password. I'd prefer the model above.
The first bind is with a "generic" DN (i.e. a non-user account).
After the bind succeeds, LDAP is queried for an object which (italic == configurable item):
- is below ldap_basedn (this could be cn=users,cn=portal,dc=nebiogrid,dc=org)
- attribute ldap_username_attribute exactly matches <username>
The generic DN is then unbound and either one ldap object or "None" is returned. After unbinding the connection must be reset with ldap.initialize
If a unique object is returned, we use that object's DN and the provided <password> to try and bind with ldap. If that succeeds, the user has successfully logged in.
See this screenshot for a config example: http://rmartinjak.de/notebooksettings.png
- Sage stores a little bit of additional per-user state: suspended/active and worksheet timeouts or auto-save (can't remember). TTBOMK the OpenID system gets around this by creating a Sage user when someone authenticates for the first time. Do you inherit the same behavior? I can't see where this happens.
Done in ExtAuthUM's "_user()"
comment:5 follow-up: ↓ 6 Changed 22 months ago by dimpase
they say I have to build ldap 2.13 from source on MacOSX 10.6, otherwise ldap doesn't work. see http://stackoverflow.com/questions/6475118/python-ldap-os-x-10-6-and-python-2-6
comment:6 in reply to: ↑ 5 Changed 22 months ago by dimpase
Replying to dimpase:
they say I have to build ldap 2.13 from source on MacOSX 10.6, otherwise ldap doesn't work. see http://stackoverflow.com/questions/6475118/python-ldap-os-x-10-6-and-python-2-6
oops, I meant 2.3.13 After I took the source and did
sage -sh python setup.py install
in its source directory, I was able to get to
File "/usr/local/src/sage/current/devel/sagenb/flask_version/authentication.py", line 53, in login
elif g.notebook.user_manager().check_password(username, password):
File "/usr/local/src/sage/sage-4.7.alpha5/devel/rkirov-flask/sagenb/notebook/user_manager.py", line 503, in check_password
return self._check_password(username, password)
File "/usr/local/src/sage/sage-4.7.alpha5/devel/rkirov-flask/sagenb/notebook/user_manager.py", line 612, in _check_password
u = self._auth_methods[a].check_user(username)
File "/usr/local/src/sage/sage-4.7.alpha5/devel/rkirov-flask/sagenb/notebook/user_manager.py", line 729, in check_user
u = self._get_ldapuser(username)
File "/usr/local/src/sage/sage-4.7.alpha5/devel/rkirov-flask/sagenb/notebook/user_manager.py", line 709, in _get_ldapuser
result = self._ldap_search("(%s=%s)" % (self._conf['ldap_username_attrib'], username), attrlist)
File "/usr/local/src/sage/sage-4.7.alpha5/devel/rkirov-flask/sagenb/notebook/user_manager.py", line 690, in _ldap_search
raise ValueError, "invalid LDAP credentials"
exceptions.ValueError: invalid LDAP credentials
after attempting to log in (well, it could well be that the "Bind DN" is set wrongly in my case).
by the way, there is obvious typo in the patch in flask_version/authentication.py
password = request.form['password'True]
we guessed that "'True" should be gone there.
comment:9 Changed 22 months ago by kini
Just by the way, we are currently using this patch (rebased and modified a bit) to do authentication for our Sage-based undergraduate class. Thanks for your work, rmartinjak! Hopefully we can eventually review this and get it into the main notebook codebase (but that will probably take some more discussion).
comment:10 Changed 21 months ago by rmartinjak
I'm glad to hear that.
Actually, the username normalization to ascii is problematic, as it allows logging in with invalid usernames (i.e. rmärtinjak or rm*rtinjak instead of rmartinjak)
I attached a newer class that uses python-ldaps filter_format() method to build valid ldap queries
@dimpase: you're right with the typo and throwing an exception with "invalid LDAP credentials" occurs when either your bind DN or the corresponding bind password is wrong (as you suspected)
comment:12 Changed 21 months ago by dimpase
One more thing that we just noticed: our LDAP has case-insensitive login names, resulting in nb creating separate sws directories for, say, user bond007, if he logs in as BOND007, or as Bond007, or as bond07.
This has to be a feature of ldapauth, to canonise login names in some way for such LDAP (our is an AD Windows thing).
comment:14 Changed 11 months ago by kcrisman
- Report Upstream changed from N/A to Reported upstream. Developers acknowledge bug.
This is now basically happening upstream. See this pull request, apparently unrelated to anything here, and the continuation of this material at this fork by rmartinjak.
comment:15 Changed 11 months ago by kini
Correction: the pull request is based on rmartinjak's patch from this ticket, though it was done by someone else (Konstantin Podshumok, who also submitted a nice gettext cleanup and Russian translation).
Personally I am wondering what happened to ijstokes's list of concerns, all of which seem very reasonable to me.
comment:16 follow-up: ↓ 17 Changed 11 months ago by kini
Looking at rmartinjak's current "ldap" branch on sagenb, it seems that some of ijstokes's comments have been addressed... OpenIDUserManager now inherits from ExtAuthUserManager rather than vice versa, for example. So apparently the patch here is out of date.
comment:17 in reply to: ↑ 16 Changed 11 months ago by rmartinjak
Replying to kini:
Looking at rmartinjak's current "ldap" branch on sagenb , it seems that some of ijstokes's comments have been addressed... OpenIDUserManager now inherits from ExtAuthUserManager rather than vice versa, for example. So apparently the patch here is out of date.
This is true, anything but "3." (ordered list of auth methods) should've been taken care of. Maybe also part 1, getting python-ldap, might be an issue. Building it requires libldap (with headers) iirc
comment:18 Changed 10 months ago by pipedream
Note from this thread https://groups.google.com/forum/?fromgroups#!topic/sage-support/6DmaZW8cY98 That python-dev needs to be installed (on Ubuntu) to use the instructions in this ticket else import crypto fails.


patch is based on Rado Kirov's module "python-ldap" (available via easy_install like in Rado's install instructions)
major changes in the patch:
user_manager.py: renamed "valid_login_names" to "known_users" (imho the old name is kind of misleading now) new UserManager? (ExtAuthUsermanager?) new abstract class AuthMethod? (all auth methods used by ExtAuthUM should implement its methods) class LdapAuthMethod? for LDAP authentication
user.py new User.account_type: "external" new methods may_change_email(), may_change_password()
conf.py/server_conf.py added (optional) key POS that allows ordering of items in a config group added config items for LDAP auth made openid login configurable
account_settings.html users only see the "change email/password" form if above mentioned methods return true
worksheet_share.html added a user lookup form