authmap entry no longer needed?

Well, I found it confusing because you're not actually intending to use the authmap hooks. You can't use them all the time because they don't give the level of flexibility needed, as you know; that's why the hack is needed to call another auth function after user_authenticate().

I (and I'm guessing many others) don't consider LDAP authentication to be external authentication in the sense that the authmap stuff was designed for. It's designed for remote systems with hostnames where the passwords are copied into the local database. LDAP is--at least often, if not usually--used such that you want to check the password each time, not have it stored again in drupal's database. As it stands, when you change your password in LDAP, a malicious user (the one who made you need to change your password in the first place) can continue logging in to Drupal with your old password until you actually log in to drupal again. Imagine if all the services on my campus worked like that! Not to mention having another copy of a password stored somewhere, possibly with a weaker encryption.

So, I tweaked the code to blank the password after the user is created. This means that local authentication always fails (good) and it then tries the authmap method for old users and the hacked-in method for new users.

This means that ldap_integration_auth() gets called through two different paths, which is a bit confusing in the first place, but is made even more so because in one case it is called with a username and server and in another case with just a username (which may or may not include a server depending on what the user typed in). There's code to deal with that case.

Anyway, I found all that unnecessarily confusing. And the reason for the patch was just to point out that if you stop acting as an external auth module (which I don't really think this module is) it all keeps on working just fine since the hacked in auth call gets called anyway whether it's a new user or not and just carries on doing its thing; the authmap entry isn't gaining us anything.

Hope that clarifies...

I still don't believe the authmap stuff is fine as it is, actually.

The authmap functionality is being fundamentally misused here: it was designed to allow users with accounts on other drupal systems to log in with their user ID's from those systems, by using username@remote.server.com.

What we want with the LDAP module is for users with accounts on *this* system (which happen to be authenticated via ldap) to log in with their userid's from *this* system, by using "username".

If the authmap system was the solution to this problem, then we wouldn't need the other hack to hook into the login process. But if you took out the hack to the login process, this module would not work.

If you take out all the authmap code, however, this module continues to work just fine *and* the code is simpler. In almost every ldap login case, the authmap code doesn't even get called. And in the few cases when it does, the very next thing that would happen if it wasn't there is that the login hack would get called and do exactly the same thing. The authmap stuff is totally extraneous.

But I don't feel a need to reopen this if the developers don't think it needs fixing; I stopped using ldap_integration some time ago because of this and a few other issues and wrote my own ldap module to implement some site-specific behaviour here at work. Since I didn't need to avoid patching drupal core, I was able to add one line to the user.module and end up with a much cleaner implementation.