Description

Some auth methods can not be chained with others due to usage of CancelLogin. Below, this is described for ldap_login, but the problem is more generic.

ldap_login.py returns CancelLogin on authentication failure (instead of ContinueLogin as other modules). This makes it impossible to chain it before other authentication modules, e.g. MoinAuth. -- Patrick Cernko <pcernko@mpi-sws.org>

Steps to reproduce

  1. configure authentication chain with LDAP as primary and MoinAuth as secondary authentication:

        auth = [ LDAPAuth(...), MoinAuth(), ]
  2. Create a new Moinauth user
  3. Try authenticating:

Fails as LDAP as first authentication module cancels the authentication instead of letting MoinAuth continue (successfully).

Component selection

Details

ll in the details here:

MoinMoin Version

1.9.2

OS and Version

Debian/lenny

Python Version

2.5.2

Server Setup

Apache2 with CGI

Server Details

Language you are using the wiki in (set in the browser/UserPreferences)

english

Workaround

This patch fixes the problem (at least for me):

--- ldap_login.py.orig  2010-02-28 16:28:45.000000000 +0100
+++ ldap_login.py       2010-03-20 14:52:29.000000000 +0100
@@ -243,7 +243,7 @@

             except ldap.INVALID_CREDENTIALS, err:
                 logging.debug("invalid credentials (wrong password?) for dn %r (username: %r)" % (dn, username))
-                return CancelLogin(_("Invalid username or password."))
+                return ContinueLogin(user_obj, _("Invalid username or password."))

             if u and self.autocreate:
                 logging.debug("calling create_or_update to autocreate user %r" % u.name)

Discussion

Please give more details about the scenario where the patch makes sense.

Currently (without patch) it behaves like:

So, in what scenario does continuing with a wrong password make sense?

Answering that myself:

If configured so, moin uses the login username/password to do the initial bind to ldap. If the user is not in ldap, this will fail with invalid credentials exception.

uncleremus, 2012-05-25: Your description of the behavior above is only correct for LDAP bind with bind DN. LDAPAuth also supports binding with the user's name and password. With that method, "invalid credentials" will be returned also in the first case (no such user in LDAP), causing the authentication to cancel. That's certainly not desirable behavior. Binding to LDAP with the user's name is much preferrable IMO.

I have made a very similar but slightly more elaborate patch in MoinMoinPatch/AddLdapAuthSasl

Discussing doing a general change

How about not using CancelLogin any more? What about:

  1. checking if we already have an authenitcated user from previous auth methods. if we have nothing else to do, we can do short-circuit and just ContinueLogin with that user.

  2. auth method tries to determine an authorized user somehow by itself (then it returns ContinueLogin with that user) or it does ContiueLogin with None.

/!\ If we want to change that, it should be done systematically and in every auth method.

(!) May be we should start to explain why one uses an external auth and mixes it with an internal one.

uncleremus, 2012-05-25: The answer to the last question is simple. You can have a few local users (e.g. the superuser, or in my case, a Wiki migrated to a different environment) want future users to be able to login with their LDAP credentials, while maintaining the availablility of the local accounts for old users.

Plan


CategoryMoinMoinBug

MoinMoin: MoinMoinBugs/ChainingAuthMethods (last edited 2012-05-25 15:07:03 by uncleremus)