Finally worked out what was phasing me with authentication methods.
Administrators are 'forced' to use only the internal authentication methods - all administrators (line 770 in moodlelib.php). That's why a user created manually (or probably by any method) will always be able to log in after the auth method is changed ONLY if they are an administrator. Ordinary users will no longer be able to login if the method is changed of course. This is why I thought that users that had already been created continued to work after we had swapped to LDAP (we had quite a few admins), and this confused the hell out of me.
But... its seems to be that the downside of this is that if I promote an LDAP user to administrator, their LDAP login details will from then on be completely ignored. If their password is changed in the LDAP database it will not be reflected in Moodle.
Is this a bug or a feature? I am working in this area at the moment to make it so manual authorisation is always available, and I'd like to get some opinions on this one.
It's a feature - a safeguard in case the other method of authentication doesn't work. For example, when you are configuring authentication for a new Moodle installation, it can happen that you screw up. In that case, without this safeguard, you wouldn't be able to log in and correct your mistake. And it is not true that the password won't be updated. If the administrator logs in with their newly changed LDAP password, their login will be accepted, and the local password will be updated to reflect this. The logic goes as follows:
- Try the local password first.
- This fails, so try LDAP next.
- This succeeds, so the administrator is logged in.
- After every successful login, the local password is updated with the one used for the successful login, in this case the new LDAP password.
mmmm......
I don't agree. If an administrator's password is changed in LDAP (perhaps, for example, because you want to keep them out) and they log in with their old password - they *still* get into Moodle. Yes, they would be able to log in with their new LDAP password because *then* the local password will fail, but the point is that changing the LDAP password will not prevent a user logging in (with the existing password). Is this good security? Does it matter? I'm not sure, but it's certainly counter-intuitive.
My take would be to make it so that only the 'master' administrator gets this privilege, as they must have been created manually any way.
I don't agree. If an administrator's password is changed in LDAP (perhaps, for example, because you want to keep them out) and they log in with their old password - they *still* get into Moodle. Yes, they would be able to log in with their new LDAP password because *then* the local password will fail, but the point is that changing the LDAP password will not prevent a user logging in (with the existing password). Is this good security? Does it matter? I'm not sure, but it's certainly counter-intuitive.
My take would be to make it so that only the 'master' administrator gets this privilege, as they must have been created manually any way.
No. Unless something has recently changed in weblib.php, in your hypothetical case of a rogue administrator, you just need an extra step to lock him out. Namely (1) first change his LDAP password, then (2) log in as him with the new password. Step (2) will purge his old password from the local Moodle database.
yes I understand that entirely. BUT... who would have known that? 
I just don't think it's a good decision, it doesn't (to me) seem logical from a *users* point of view
I'm prepared to be shouted down - anybody else (Martin) got an opinion?
I'm wanting to do some work on this part of the code to make a start on providing multiple authentication methods. The first pass will be to make it possible to add manual users even if another method is in use. This admin thing is not really independent of that.
I just don't think it's a good decision, it doesn't (to me) seem logical from a *users* point of view
I'm wanting to do some work on this part of the code to make a start on providing multiple authentication methods. The first pass will be to make it possible to add manual users even if another method is in use. This admin thing is not really independent of that.
In the new system, if you add a user manually, then the "auth" field in the "user" table will be set to "manual". Therefore LDAP or other methods won't be checked or used, regardless of whether the user is an admin or not. The primary admin will ALWAYS be "manual" so that safety precaution is preserved.
If you allow LDAP users, then their "auth" field will be "ldap", even if you promote them to being admins. Their login details will always be treated as external, and the internal copy of the password will NEVER be used. So this covers your rogue admin.
Everyone happy with this?
If you allow LDAP users, then their "auth" field will be "ldap", even if you promote them to being admins. Their login details will always be treated as external, and the internal copy of the password will NEVER be used. So this covers your rogue admin.
Everyone happy with this?
Why can't I think that clearly at 11pm? 
Yes, I like that!
Another small thing: do you have any thoughts on how the auth field will be set for 'historical' users created before the auth field existed? My thinking is to start with it empty, try (on first incarnation of the new system) Manual then the chosen authmethod (or should that be the other way around). Wherever it works, that method is plugged in for future use.
Another small thing: do you have any thoughts on how the auth field will be set for 'historical' users created before the auth field existed? My thinking is to start with it empty, try (on first incarnation of the new system) Manual then the chosen authmethod (or should that be the other way around). Wherever it works, that method is plugged in for future use.
Since only one method is allowed right now, you can safely set everyone's field to whatever the current $CFG->auth is. Except for all the admins, of course, who should be set to "manual". This way the upgraded system will behave exactly as it does now and no-one will get any sudden surprises.
Sites that really need to re-match the admins against LDAP accounts (I doubt there will be many sites that need to do this) can just flip the "auth" flags on those accounts manually in the database table.
Thinking about it, there is no need to implement the "falling back" of accounts to manual that we talked about as the first step. Manual accounts are marked as manual already so we just do it.
This should be in 1.4, since it won't require any GUI changes. (Hopefully you can send me some tested patches in the next few days!
).
In 1.5 we can look at a multi-authentication scheme where the admin can say "try to match the accounts against this list of authentication sources, in this order". The list could contain multiple LDAP servers, multiple IMAP servers, etc ...
Sites that really need to re-match the admins against LDAP accounts (I doubt there will be many sites that need to do this) can just flip the "auth" flags on those accounts manually in the database table.
Thinking about it, there is no need to implement the "falling back" of accounts to manual that we talked about as the first step. Manual accounts are marked as manual already so we just do it.
This should be in 1.4, since it won't require any GUI changes. (Hopefully you can send me some tested patches in the next few days!
In 1.5 we can look at a multi-authentication scheme where the admin can say "try to match the accounts against this list of authentication sources, in this order". The list could contain multiple LDAP servers, multiple IMAP servers, etc ...
OK, none of this should present a problem. I should have this sorted out by Monday/Tuesday.
It had just occurred to me that multiple instances of each auth type would be the 'right' way to do it in 1.5. Sigh! It'll be good if it works
It had just occurred to me that multiple instances of each auth type would be the 'right' way to do it in 1.5. Sigh! It'll be good if it works
Howard,
I was in the area today anyway, and just checked in changes to CVS to:
...so that means all you have to do is work on the actual authentication logic in lib/moodlelib.php.
Something very roughly like
If the username already exists in the user table
I was in the area today anyway, and just checked in changes to CVS to:
- create the new "auth" field in the user table
- populate upgraded systems with sensible values (admin = manual, everyone else = $CFG->auth)
- add correct auth values to this field whenever new accounts are created
...so that means all you have to do is work on the actual authentication logic in lib/moodlelib.php.
Something very roughly like
If the username already exists in the user table
- get $user->auth
- if the auth field is internal (use the function is_internal_auth())
- check the password and return result
- else the auth is external so
- check the username/password against external source
- if result was good, update internal copy of data and return true
- else if ($CFG->auth_allow_cached_results) and internal copy matches, return true (this option will allow Moodle to cope with an external source being down for some reason)
- else return false
- if the $CFG->auth is "none" just create the account
- else if the $CFG->auth is internal then return false
- else if the $CFG->auth is external then
- check the username/password against external
- if the result is good create internal account and return true
Ah - that saves me a job!! I'll check out the code and see.
I has already 'bodged up' the database on my local system and have written most of the above, so it should be a quick job now. Just got to remember to test it
I has already 'bodged up' the database on my local system and have written most of the above, so it should be a quick job now. Just got to remember to test it
Martin,
Why do I care (or why is it a special case) that we check if the authentication method is 'internal'? They all have individual methods, exactly the same as the 'external' methods. I can't understand why they need to be treated differently.
I assume you spotted something I didn't that drove you to writing that is_internal_auth() method.
Confused again.....
Why do I care (or why is it a special case) that we check if the authentication method is 'internal'? They all have individual methods, exactly the same as the 'external' methods. I can't understand why they need to be treated differently.
I assume you spotted something I didn't that drove you to writing that is_internal_auth() method.
Confused again.....
The is_internal_auth() function is old and is used all over the place (such as password-changing, for example).
Quite true, you could just call auth_user_login() for internal and external, as now.
I was just thinking of efficiency since you'll already have pulled $user once anyway at the beginning to determine the authentication method to use. It may be that the auth_user_login() would have to change somewhat ... perhaps an optional third parameter containing a predefined $user object?... it's not a huge deal to pull the user object twice, though, and I've not pondered the details ... I was hoping you could work these out ...
Quite true, you could just call auth_user_login() for internal and external, as now.
I was just thinking of efficiency since you'll already have pulled $user once anyway at the beginning to determine the authentication method to use. It may be that the auth_user_login() would have to change somewhat ... perhaps an optional third parameter containing a predefined $user object?... it's not a huge deal to pull the user object twice, though, and I've not pondered the details ... I was hoping you could work these out ...
Oh ok - you actually *don't* need to pull the object twice, so that's ok. I was just worried that you had spotted something vital that I hadn't, which would be more than likely