As far as I understand, when a user attempts to login by providing just the username (without the @server.com part), Drupal invokes hook_auth of each module that implements it. If the user is found locally and provided the correct password, it locally loads the user (hook_user $op = 'load' and then $op = 'login')

The problem is that if the user is found locally but the password is incorrect, Drupal invokes the rest of the hook_auths from all the modules. I don't think that behavior is favorable at all, and it goes against everything I've read about hook_auth. It should simply return a incorrect username/password error and stop, since it did find a local user, but the password was incorrect.

In other words, it SHOULDN'T invoke all hook_auth in modules if it finds the user locally, me thinks.

I have a test module called remote_db, and when I try to login with admin and a wrong password, I get the error message in the code below, and below that one I get the default Drupal error. I think it shouldn't run remote_db since admin exists locally, and the point of calling hook_auth is if the user doesn't exist locally.

function remote_db_auth($username,$password)
{
drupal_set_message(t('I should not see this'), 'error');
return FALSE;
}

Comments

michael_kirk’s picture

michael_kirk commented

I think at the point the user logs in, they do exist locally.

When a user is successfully authenticated, if they don't yet exist in drupal, a row in the user table is created, even though drupal doesn't know their password. I'm not sure in which order they are applied, but it seems authentication is the union of all acceptable username/password combinations for your various hook_auths.

This certainly isn't the most secure way to do it, but it is nice (simple) to have most of my users tied to an existing password map, while I create an admin account that exists only within drupal without having to create a user called "admin" in my unix password maps (actually I'm using "root" which would cause even more trouble).

It seems like it would be best to allow each user to be tied to a particular authentication type. Anyone thoughts on that?

michael_kirk’s picture

michael_kirk commented

user_authenticate first tries to authenticate it against a local account. If this fails (if the user does not exist, or if the user does exist and you gave the wrong password) it tries all auth hooks in series. On success the user is registered as a new user if she hasn't already. I'm not sure how you would overcome this without a change to a core module.

http://api.drupal.org/api/function/user_authenticate/5

aniskhan’s picture

aniskhan commented

so what i am saying is that when the user does exist and you gave the wrong password, it shouldn't try all the auth hooks in series. if the user exists locally, doesn't make sense to go on and call all auth hooks, no?

damien tournoud’s picture

damien tournoud commented

The behavior of Drupal 5 is the following:

  • Try to authenticate the user locally.
  • If it fails and the login is in the authmap, try to login the user using the corresponding authentication in the authmap.
  • If it fails and the login is not in the authmap (or has no server part), try each authentication in sequence.

I think it's too late to change the behavior of Drupal 5, so I'm tempted to mark this as "won't fix".

Note that the authentication behavior already changed in Drupal 6, because authentication modules now take full control of the authentication form (see for example the openid implementation).

damien tournoud’s picture

damien tournoud commented

@aniskhan: it makes sense because remote users become local user when they first login. Their remote password could have changed, so Drupal has to query the remote server.

aniskhan’s picture

aniskhan commented

Damien, what about users that were registered locally initially? Why go through all the auth hooks? I think perhaps Drupal should save somewhere if the user was registered locally or by reason of a auth hook.

That way, users that were registered locally won't go through all the auth hooks, and users that were registered because of a auth hook call will be checked against all the auth hooks.

michael_kirk’s picture

michael_kirk commented

It's too late to change this for drupal 5. It could dramatically change the way a site behaves.

Not to mention, there is no upgrade path for all existing site, as all these users who were created by external auth hooks are indistinguishable from locally created users.

That said, I think it is a good idea to associate authentication hooks per user. So joe_local will only authenticate against the drupal native authentication, while joe_remote_user will only check against the particular hook(s) associated with his account.

Maybe it has made, or will make, its way into D6 or D7. Anyone know?

richmck’s picture

richmck commented

I like the functionality as it is.

My scenario is that I have imported users as part of a system migration.

The old system used a different password hashing function that the MD5 that is used in drupal.

This functionality allows me to add a hook that that tries the old hashing function to authenticate of the standard drupal login fails.

Once they update their password, their authentication will not use the hook. Alternatively, my hook can update the password field on successful login.

Without this functionality, I would have to reset everyone's password (ugh)

IMHO -- The writer of the hooks and the administrators need to make sure that they apply the hooks with security in mind.

michael_kirk’s picture

michael_kirk commented

But you don't have a choice to "make sure" only appropriate security hooks are applied. If you have users authenticating from multiple sources, all hooks are attempted, until one succeeds. Effectively you have a different password for each hook that happens to have a username like yours, any one of them allowing a drupal log in.

Maybe it's not a huge benefit to security to fix a constant number of acceptable passwords when you are dealing with an exponential space, but I don't see how associating users with a particular auth hook would hurt anything. Would it incur much overhead to record the associated acceptable auth hook(s) per user?

How would that work? Maybe the first hook that succesfully authenticates the user, the first time they log in.

michael_kirk’s picture

michael_kirk commented

to elaborate my concern:
If you trust a source for authentication of any of your users, you necessarily trust it for all of them. Usually, not a problem, but a risk that could be minimized.

cridenour’s picture

cridenour commented

I could see the potential benefit but with the changes in 6 (and 7) I don't think its required. I fully agree that is is too late to change 5 - too many sites rely on this behavior.

dpearcefl’s picture

dpearcefl commented
Status: Active » Closed (won't fix)

Considering the time elapsed between now and the last comment plus the fact that D5 is no longer supported, I am closing this ticket.