Status

Webtools Graveyard
Verbatim
RESOLVED FIXED
10 years ago
2 years ago

People

(Reporter: clouserw, Assigned: dschafer)

Tracking

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

10 years ago
Currently Pootle's authentication is a single flat file.  Since all our other systems use LDAP accounts we'll want to use it as well.
(Assignee)

Comment 1

10 years ago
Created attachment 324685 [details]
A set of LDAP functions in Python

This is a straight conversion of http://svn.mozilla.org/addons/trunk/site/app/controllers/components/ldap.php into python.

LDAP_HOST, ANON_BIND and ANON_BIND_PW need to be filled in for this to work.
Assignee: nobody → dschafer
Status: NEW → ASSIGNED

Comment 2

10 years ago
Some general python comments:

I'd make the config params arguments to __init__. Class names are commonly CamelCased in python. Throw exceptions rather than returning False. You should add doc strings to the methods. Which of those are actually for external use? Like, some of the methods look like they should just do something like

if not self.ldo:
  self.connect()

Usually (foo, bar) is easier to read, namely, ' ' after a comma in arguments. 'if' conditions are not in braces in python, too.

There's no point in 
#!/usr/bin/env python
as there's no standalone functionality.

Is there an inconsistency between username and email?

Never used ldap myself, is initDn() a worthwhile independent method?

How do we intend to secure the connection, via ldaps or con.start_tls_s()?

Oh, and is

import ldap

referring to http://python-ldap.sourceforge.net/? Looks like it, just want to make sure.

HTH.
(Assignee)

Comment 3

10 years ago
Created attachment 324709 [details]
Improved LDAP module

Thanks for the Python advice; I haven't used Python in a while, so that code really was a straight PHP->Python translation.  Is this new version better?

I'm not sure about secure connections; that's something not included in the original php function.
Attachment #324685 - Attachment is obsolete: true
(Assignee)

Comment 4

10 years ago
Created attachment 325051 [details]
Expanded LDAP module

A much expanded and generalized LDAP module.  As much of the LDAP-directory-specific code as possible is factored out into private methods.
Attachment #324709 - Attachment is obsolete: true
(Assignee)

Comment 5

10 years ago
Created attachment 325460 [details] [diff] [review]
Patch to add LDAP login to Pootle using flat preferences file

This patch uses the flat users.prefs file for preferences, but does login verification with LDAP.

There are a few weird things this patch does because it needs to use the flat file.  The usernames in the flat file should be LDAP e-mail addresses, but with "s/\./D0T/" (so my flat file entry is "dschafer@mozillaD0Tcom"); this is to avoid the weird way jToolkit preference methods deal with . in key names.  Clearly, this is a very temporary fix, but since we will migrate to using databases for preferences very soon (and certainly before anything goes live), I decided using that replacement method would be significantly easier than trying to redo a lot of jToolkit preference code (that we really don't need to change in the long run).

As mentioned in the docstrings for this patch, a ldapsettings.py file will need to be created in the python path which contains the LDAP server name and anonymous login credentials.

The way validation is done is slightly different now; we can't easily verify if a session ID is valid, since we don't have the MD5 of the password.  I believe that checking if it is in the sessioncache works and is secure, but this should be verified.
Attachment #325051 - Attachment is obsolete: true
Attachment #325460 - Flags: review?(clouserw)
(Assignee)

Updated

10 years ago
Attachment #325460 - Flags: review?(clouserw)
(Assignee)

Comment 6

10 years ago
Created attachment 326349 [details] [diff] [review]
Improved LDAP module with auto-registration

This patch is similar to the previous one, but adds UI registration.  Additionally, it adds an auto-registration feature; if a person logs in with a valid LDAP account, but has never used Pootle before, it should create a new line in the preferences file for them.  It maintains the terribly-hackish D0T substitution from the previous patch (in many more places); as soon as databases become the preferences mechanism, this will all be fixed.

There is one major issue with this patch; currently, validate does not really do any validation if the user is already logged in, which means it might be possible to spoof a session string.  I'll be trying to fix this issue this afternoon.
Attachment #325460 - Attachment is obsolete: true
Attachment #326349 - Flags: review?(clouserw)
(Assignee)

Comment 7

10 years ago
Created attachment 326822 [details] [diff] [review]
Most recent LDAP patch

The most recent version of ldap.patch
Attachment #326349 - Attachment is obsolete: true
Attachment #326822 - Flags: review?(clouserw)
Attachment #326349 - Flags: review?(clouserw)
(Assignee)

Comment 8

10 years ago
LDAP support is in r7679 to the Mozootle branch.  There are a few issues to work out (regarding the storing of LDAP user preferences in users.prefs), but I think that all falls under bug 442055, so I'm going to mark this FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Updated

10 years ago
Attachment #326822 - Flags: review?(clouserw)
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.