Moodle
  1. Moodle
  2. MDL-9399

Inclusion of NTLM Auth Module in MOODLE HEAD

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Authentication
    • Labels:
      None
    • Rank:
      11950

      Description

      Now that Multi-auth is in 1.8 and I have ported the changes over to the NTLM module, we can now plan to include it in Moodle HEAD - ready for 1.9 release!

      I have opened this tracker to allow feedback from testers - I'm especially interested to hear from people testing it in 1.9 - see the link:
      http://download.moodle.org/plugins/auth/ntlm.zip
      to download the current version for 1.9/HEAD

      There are a couple of things to be aware of.... - when upgrading from versions prior to 1.8 when the ntlm module has been installed - after the upgrade you will need to use a manual account to authenticate eg "admin" and then re-configure the settings for the NTLM module as these don't get carried over correctly during the upgrade (doesn't affect people already running 1.8 though...)

      Also - I found an issue with the global var that stated the number of subnet/masks to use, so I have hardcoded it to 8 in a couple of places - I want to fix this before inclusion in Moodle Head.... - hopefully sometime this week.....

      please let me know how you go with testing!

      thanks!

      Dan

      1. 0001-Fix-typo-in-ntlmsso_finish.patch
        0.8 kB
        Iñaki Arenaza
      2. 0002-If-the-cache-flag-is-not-set-it-doesn-t-make-sense.patch
        0.9 kB
        Iñaki Arenaza
      3. 0003-user_login-was-not-converted-to-using-get_cache_fl.patch
        4 kB
        Iñaki Arenaza
      4. auth_ldap_ntlm.patch
        22 kB
        Martín Langhoff
      5. auth.php.patch
        1.0 kB
        Janne Mikkonen
      6. auth.php.patch
        1.0 kB
        Janne Mikkonen

        Issue Links

          Activity

          Hide
          Ian Fogarty added a comment -

          Hi All

          I have put the 1.8 NTLM on to the 1.9 HEAD (as of yesterday - 18/04/07) but came up with the following problems...
          If the user already exists in the user database, NTLM login works perfectly but if the user is logging in for the first time, then they receive a NTLM has not been enabled on this page.

          The solution I have found to this is to login the user first on the offcampuslogin page and then get them to re-login via the NTLM page.

          To ensure this was the case, I have deleted (via SQL) all NTLM users between tests and also the session2 table.

          In addition to this, I also noticed a problem when you go back in to edit the details, the user fields are not recalled from the DB. Although all the LDAP settings are present the actual user fields (e.g. givenName, sn, mail etc) do not get re-displayed.

          Hope some of this helps.

          Ian

          Show
          Ian Fogarty added a comment - Hi All I have put the 1.8 NTLM on to the 1.9 HEAD (as of yesterday - 18/04/07) but came up with the following problems... If the user already exists in the user database, NTLM login works perfectly but if the user is logging in for the first time, then they receive a NTLM has not been enabled on this page. The solution I have found to this is to login the user first on the offcampuslogin page and then get them to re-login via the NTLM page. To ensure this was the case, I have deleted (via SQL) all NTLM users between tests and also the session2 table. In addition to this, I also noticed a problem when you go back in to edit the details, the user fields are not recalled from the DB. Although all the LDAP settings are present the actual user fields (e.g. givenName, sn, mail etc) do not get re-displayed. Hope some of this helps. Ian
          Hide
          Dan Marsden added a comment -

          Hi Ian,

          thanks for the details - I haven't forgotten about it! - I think it's because the password is blank when using the oncampus page, so some of the processes don't run...

          I'm stuck in Peoplesoft Training with oracle over the next couple of weeks - but I've got a big list of things I'm trying to cover off after-hours!

          Dan

          Show
          Dan Marsden added a comment - Hi Ian, thanks for the details - I haven't forgotten about it! - I think it's because the password is blank when using the oncampus page, so some of the processes don't run... I'm stuck in Peoplesoft Training with oracle over the next couple of weeks - but I've got a big list of things I'm trying to cover off after-hours! Dan
          Hide
          Dan Marsden added a comment -

          Hi Ian,

          should be fixed now - I've patched HEAD - will probably take a while for the files to get updated in the zip downloads....

          Thanks!

          Dan

          Show
          Dan Marsden added a comment - Hi Ian, should be fixed now - I've patched HEAD - will probably take a while for the files to get updated in the zip downloads.... Thanks! Dan
          Hide
          Dan Marsden added a comment -

          Hi Skodak,

          you're the maintainer for the auth stuff aren't you? - any objections with this going in HEAD sometime next week?

          Dan

          Show
          Dan Marsden added a comment - Hi Skodak, you're the maintainer for the auth stuff aren't you? - any objections with this going in HEAD sometime next week? Dan
          Hide
          Petr Škoda added a comment - - edited

          I have quickly looked at the code, it might be better to alter normal ldap first and make it reusable - both cas and ntlm could IMHO use it without code duplication.
          The idea is to use $this->authtype everywhere in ldap plugin and then just wrap it in cas or ntlm == have it initialized in one private property and call its methods as needed.

          Could you have a look at it?

          Show
          Petr Škoda added a comment - - edited I have quickly looked at the code, it might be better to alter normal ldap first and make it reusable - both cas and ntlm could IMHO use it without code duplication. The idea is to use $this->authtype everywhere in ldap plugin and then just wrap it in cas or ntlm == have it initialized in one private property and call its methods as needed. Could you have a look at it?
          Hide
          Dan Marsden added a comment -

          you're right - it would make more sense like that...... - I don't think I'll have time to redevelop it and test it extensively enough for 1.9 though, - I wouldn't want it to hold up the 1.9 release!

          I'm thinking the logical place for the ip address stuff won't be in the ldap module itself, but the global auth settings, - eg where we currently set an alt login screen - that way all modules could make use of the IP address stuff.

          I'm not all that familiar with CAS and how it uses LDAP - I'll have to have a good look at it, to see how LDAP would need to be modified to allow it to be reused.

          I'll get onto this sometime soon.....

          thanks,

          Dan

          Show
          Dan Marsden added a comment - you're right - it would make more sense like that...... - I don't think I'll have time to redevelop it and test it extensively enough for 1.9 though, - I wouldn't want it to hold up the 1.9 release! I'm thinking the logical place for the ip address stuff won't be in the ldap module itself, but the global auth settings, - eg where we currently set an alt login screen - that way all modules could make use of the IP address stuff. I'm not all that familiar with CAS and how it uses LDAP - I'll have to have a good look at it, to see how LDAP would need to be modified to allow it to be reused. I'll get onto this sometime soon..... thanks, Dan
          Hide
          Iñaki Arenaza added a comment -

          I keep on intending to refactor all the LDAP-based plugins in Moodle since nearly a year, but I never start doing it . CAS, NTLM and LDAP auth and enrol share (or should share) most of the LDAP core functions and kowledge (like defaults for all the different LDAP servers). All of them could go into .../lib/ldaplib.php and be user everywhere you need them. Just one place to fix and add new features, unlike the current 'mess'

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - I keep on intending to refactor all the LDAP-based plugins in Moodle since nearly a year, but I never start doing it . CAS, NTLM and LDAP auth and enrol share (or should share) most of the LDAP core functions and kowledge (like defaults for all the different LDAP servers). All of them could go into .../lib/ldaplib.php and be user everywhere you need them. Just one place to fix and add new features, unlike the current 'mess' Saludos. Iñaki.
          Hide
          Iñaki Arenaza added a comment - - edited

          It seems there is a small bug in the 1.8 code (haven't tested 1.9dev yet). I have tested the .zip from the modules donwload page, not the CVS version (although the date of the last CVS modification on the 1.8 branch is the same of the .zip file) and a seconf more serious bug.

          The small bug is config.html uses $CFG but the variable is undefined. We need to define $CFG as a global variable in auth_plugin_ntlm::config_form before including config.html. That fixes the issue.

          The second one is more serious as it wipes your NTLM data mappings everytime you save the NTLM configuration. This happes because near the end of config.html we have a line that reads:

          print_auth_lock_options('ldap', $user_fields, $help, true, true);

          that should read:

          print_auth_lock_options('ntlm', $user_fields, $help, true, true);

          to use the NTLM data mappings, instead of the LDAP ones.

          With this to fixes I've been able to use the plugin without problems in my test environment.

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - - edited It seems there is a small bug in the 1.8 code (haven't tested 1.9dev yet). I have tested the .zip from the modules donwload page, not the CVS version (although the date of the last CVS modification on the 1.8 branch is the same of the .zip file) and a seconf more serious bug. The small bug is config.html uses $CFG but the variable is undefined. We need to define $CFG as a global variable in auth_plugin_ntlm::config_form before including config.html. That fixes the issue. The second one is more serious as it wipes your NTLM data mappings everytime you save the NTLM configuration. This happes because near the end of config.html we have a line that reads: print_auth_lock_options('ldap', $user_fields, $help, true, true); that should read: print_auth_lock_options('ntlm', $user_fields, $help, true, true); to use the NTLM data mappings, instead of the LDAP ones. With this to fixes I've been able to use the plugin without problems in my test environment. Saludos. Iñaki.
          Hide
          Dan Marsden added a comment -

          Thanks Iñaki!

          I've got a big list of stuff that I planned to do over the past month - including work on the ntlm module - but our family have all ended up being hit at some point with "bugs" - (I'm just coming out of Influenza A) - If you get a chance, could you commit the changes to CVS? - it will be a problem with 1.9 code as well.

          thanks!

          Dan

          Show
          Dan Marsden added a comment - Thanks Iñaki! I've got a big list of stuff that I planned to do over the past month - including work on the ntlm module - but our family have all ended up being hit at some point with "bugs" - (I'm just coming out of Influenza A) - If you get a chance, could you commit the changes to CVS? - it will be a problem with 1.9 code as well. thanks! Dan
          Hide
          Iñaki Arenaza added a comment -

          Uh! I don't have a CVS account, so I'm afraid I won't be able to do it. Maybe I should ask for one...

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - Uh! I don't have a CVS account, so I'm afraid I won't be able to do it. Maybe I should ask for one... Saludos. Iñaki.
          Hide
          Petr Škoda added a comment -

          My +1 for that Iñaki

          Show
          Petr Škoda added a comment - My +1 for that Iñaki
          Hide
          Dan Marsden added a comment -

          wow! - I think it would be a great idea! - your contributions are much appreciated, and it would be even more efficient if you had the ability to do it yourself! - I've really appreciated a lot of the patches you have made available in the forums - esp the LDAP stuff! - your knowledge in this area seems to be pretty impressive!

          thanks!

          Dan

          Show
          Dan Marsden added a comment - wow! - I think it would be a great idea! - your contributions are much appreciated, and it would be even more efficient if you had the ability to do it yourself! - I've really appreciated a lot of the patches you have made available in the forums - esp the LDAP stuff! - your knowledge in this area seems to be pretty impressive! thanks! Dan
          Hide
          Dan Marsden added a comment -

          not going to make it into 1.9 - will aim for 2.0 (as long as the family doesn't get sick again!)

          Dan

          Show
          Dan Marsden added a comment - not going to make it into 1.9 - will aim for 2.0 (as long as the family doesn't get sick again!) Dan
          Hide
          Iñaki Arenaza added a comment -

          Hi Dan,

          I have commited my fixes and a couple of others from Rod Ward (from the end of this thread).

          This is the first time I mess with Moodle's CVS so I hope I didn't break anything

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - Hi Dan, I have commited my fixes and a couple of others from Rod Ward (from the end of this thread). This is the first time I mess with Moodle's CVS so I hope I didn't break anything Saludos. Iñaki.
          Hide
          Martín Langhoff added a comment -

          Hi! I'm taking this tracker item over to land the code we've been cooking in http://moodle.org/mod/forum/discuss.php?d=80104

          I have it ready to merge at http://repo.or.cz/w/moodle-pu.git?a=shortlog;h=mdl19-authldap-3

          Show
          Martín Langhoff added a comment - Hi! I'm taking this tracker item over to land the code we've been cooking in http://moodle.org/mod/forum/discuss.php?d=80104 I have it ready to merge at http://repo.or.cz/w/moodle-pu.git?a=shortlog;h=mdl19-authldap-3
          Hide
          Petr Škoda added a comment -

          Could you please post one-file-patch in unified format as attachment? I am still not able to get it from that git link, sorry

          Show
          Petr Škoda added a comment - Could you please post one-file-patch in unified format as attachment? I am still not able to get it from that git link, sorry
          Hide
          Martín Langhoff added a comment -

          I've attached the whole patch for review

          In reviewing it earlier today I noticed there's a small refactor it needs around the process that happens when a user logs in. There was a big chunk repeated in login/index and in every bit of code that did SSO of any kind. I've refactored into a new function called complete_user_login() that deals with session setup.

          Show
          Martín Langhoff added a comment - I've attached the whole patch for review In reviewing it earlier today I noticed there's a small refactor it needs around the process that happens when a user logs in. There was a big chunk repeated in login/index and in every bit of code that did SSO of any kind. I've refactored into a new function called complete_user_login() that deals with session setup.
          Hide
          Martín Langhoff added a comment -

          repost the comment so it goes out to watchers...

          ==-
          I've attached the whole patch for review

          In reviewing it earlier today I noticed there's a small refactor it needs around the process that happens when a user logs in. There was a big chunk repeated in login/index and in every bit of code that did SSO of any kind. I've refactored into a new function called complete_user_login() that deals with session setup.

          Show
          Martín Langhoff added a comment - repost the comment so it goes out to watchers... = =- I've attached the whole patch for review In reviewing it earlier today I noticed there's a small refactor it needs around the process that happens when a user logs in. There was a big chunk repeated in login/index and in every bit of code that did SSO of any kind. I've refactored into a new function called complete_user_login() that deals with session setup.
          Hide
          Martín Langhoff added a comment -
          Show
          Martín Langhoff added a comment - Also updated http://docs.moodle.org/en/NTLM_authentication
          Hide
          Martín Langhoff added a comment -

          Landed in HEAD and MOODLE_19_STABLE – now time for the testeeeeerrrrrrs...!

          Show
          Martín Langhoff added a comment - Landed in HEAD and MOODLE_19_STABLE – now time for the testeeeeerrrrrrs...!
          Hide
          Iñaki Arenaza added a comment -

          Patches for a couple of typos, and user_login() not being converted to use the get_cache_flags() thing (it was still using get_config()).

          With those patches, it works with all the combinations of:

          • IIS 6.0 running on W2003
          • Apache 2.2.4 running on W2003

          and:

          • IE 7.0 (W2003)
          • Firefox 2.0.4 (W2003)
          • Firefox 2.0.9 (Debian etch).

          I'll give it a go with Apache 2.x on Ubuntu 7.10 in a few minutes, just to make sure it works there to.

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - Patches for a couple of typos, and user_login() not being converted to use the get_cache_flags() thing (it was still using get_config()). With those patches, it works with all the combinations of: IIS 6.0 running on W2003 Apache 2.2.4 running on W2003 and: IE 7.0 (W2003) Firefox 2.0.4 (W2003) Firefox 2.0.9 (Debian etch). I'll give it a go with Apache 2.x on Ubuntu 7.10 in a few minutes, just to make sure it works there to. Saludos. Iñaki.
          Hide
          Martín Langhoff added a comment -

          Beauty - thanks I~naki for the extra pair of eyes and apologies - I think you caught something I had as a "stashed" incomplete commit in my repo (blush!). Reviewed and committed. 1.9, wohoo!

          Show
          Martín Langhoff added a comment - Beauty - thanks I~naki for the extra pair of eyes and apologies - I think you caught something I had as a "stashed" incomplete commit in my repo (blush!). Reviewed and committed. 1.9, wohoo!
          Hide
          Janne Mikkonen added a comment -

          Add kerberos based authentication support. With a really few minor changes you'll be able to support kerberos based authentication ( apache, mod_auth_kerb ).

          Show
          Janne Mikkonen added a comment - Add kerberos based authentication support. With a really few minor changes you'll be able to support kerberos based authentication ( apache, mod_auth_kerb ).
          Hide
          Janne Mikkonen added a comment -

          Needed stuff to add kerberos based authentication support.

          Show
          Janne Mikkonen added a comment - Needed stuff to add kerberos based authentication support.
          Hide
          Dan Marsden added a comment -

          Is that all that's needed? - Cool! -

          As it's such a minor change, we should be able to get this in 1.9 and 2.0 - Martín?

          Show
          Dan Marsden added a comment - Is that all that's needed? - Cool! - As it's such a minor change, we should be able to get this in 1.9 and 2.0 - Martín?
          Hide
          Martín Langhoff added a comment -

          Dan - I think we're in deep freeze mode

          Janne - the patch changes the username unconditionally, even if kerberos is not expected/enabled? Why?

          Show
          Martín Langhoff added a comment - Dan - I think we're in deep freeze mode Janne - the patch changes the username unconditionally, even if kerberos is not expected/enabled? Why?
          Hide
          Janne Mikkonen added a comment -

          Well, here's flipped one...

          Show
          Janne Mikkonen added a comment - Well, here's flipped one...
          Hide
          Dan Marsden added a comment -

          ntlm is now part of LDAP Auth - closing this bug

          Show
          Dan Marsden added a comment - ntlm is now part of LDAP Auth - closing this bug

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: