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
    • Rank:
      30083

      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

        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 Škoda added a comment -

        to be revisited in 2.0

        Show
        Petr Škoda added a comment - to be revisited in 2.0
        Hide
        Petr Škoda 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 Škoda 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: