Moodle
  1. Moodle
  2. MDL-11728

Strange setting and behaviour in DB ext auth...

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.3, 1.9
    • Fix Version/s: 2.0
    • Component/s: Authentication
    • Labels:
      None
    • Environment:
      Any using ext db auth
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      In the ext db auth, in the password setting, these values are available:

      • plain text
      • md5
      • sha1
      • internal

      that "internal" setting is used by the is_internal() method to return true/false.

      And, in moodlelib, if one auth method is internal, then update_user_record() is 100% prevented, so nothing is refreshed from ext db.

      Just guessing if that "internal" value in the db auth plugin has another hidden sense. I cannot find it. Everything gets refreshed properly without it and the ext db, by definition, seems to be a not internal auth plugin.

      I would propose to kill such "internal" option, unless somebody have any explanation for current behaviour (I've take a look to the ldap auth - really similar in concep) and it haven't such "internal" posibility at all.

      So:

      • IMO the db auth plugin should return always false.
      • The plugin should be able to run together with the "cron.php" passwords and "auth_db_sync_users.php " for new users and so on but not using the "internal" concept for that.

      Just one opinion. Ciao

        Gliffy Diagrams

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Adding some people here.

          Show
          Eloy Lafuente (stronk7) added a comment - Adding some people here.
          Hide
          Iñaki Arenaza added a comment -

          I think Eloy is 100% right: is_internal() should always return false (auth/db is not an internal auth plugin). The rest of the *_update functions deal with changes as they should (much like ldap auth, for example).

          This chage in the behaviour comes from MDL-9491. We returned false unconditionally before that.

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - I think Eloy is 100% right: is_internal() should always return false (auth/db is not an internal auth plugin). The rest of the *_update functions deal with changes as they should (much like ldap auth, for example). This chage in the behaviour comes from MDL-9491 . We returned false unconditionally before that. Saludos. Iñaki.
          Hide
          Martin Dougiamas added a comment -

          Can we push this to 1.9.1? Please move it back if it's crucial

          Show
          Martin Dougiamas added a comment - Can we push this to 1.9.1? Please move it back if it's crucial
          Hide
          Martín Langhoff added a comment -

          I can explain this (and willtry and look at the bug asap) – auth/db supports cases where the ext DB holds the list of valid usernames, but knows nothing about passwords. So we reference the external DB for acct validity, but moodle does the passwrd management.

          Sounds odd, but it is actually very common – most school-level student mgmt systems don't let students login, so they have no concept of a password. They do have a student identifier that is used as "username".

          Show
          Martín Langhoff added a comment - I can explain this (and willtry and look at the bug asap) – auth/db supports cases where the ext DB holds the list of valid usernames, but knows nothing about passwords. So we reference the external DB for acct validity, but moodle does the passwrd management. Sounds odd, but it is actually very common – most school-level student mgmt systems don't let students login, so they have no concept of a password. They do have a student identifier that is used as "username".
          Hide
          Petr Skoda added a comment -

          to be revisited in 2.0

          Show
          Petr Skoda added a comment - to be revisited in 2.0
          Hide
          Petr Skoda added a comment -

          The is_internal() should is now defined as: plugin is using user table hashes for authentication. I have added a new is_synchronised_with_external() that actually solves the problem described by Eloy. Hopefully this should be a lot more flexible and easier to understand.

          Please test and reopen if necessary, thanks everybody for the cooperation and report!

          Petr

          Show
          Petr Skoda added a comment - The is_internal() should is now defined as: plugin is using user table hashes for authentication. I have added a new is_synchronised_with_external() that actually solves the problem described by Eloy. Hopefully this should be a lot more flexible and easier to understand. Please test and reopen if necessary, thanks everybody for the cooperation and report! Petr

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: