Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29000

Forgot password tool performs case-sensitive search on username and e-mail

    Details

    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide

      To test, create a Moodle user account with an e-mail address (i.e. test@localhost.com) and run a forgot password test with both 'test@localhost.com' and 'Test@LocaLHOST.com' - you will only receive one e-mail for 'test@localhost.com'.

      Show
      To test, create a Moodle user account with an e-mail address (i.e. test@localhost.com) and run a forgot password test with both 'test@localhost.com' and 'Test@LocaLHOST.com' - you will only receive one e-mail for 'test@localhost.com'.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w33_MDL-29000_m22_forgetcase

      Description

      When using the forgot password tool, it performs a check using case-sensitive strings when using PgSQL because using WHERE $field=$value in PgSQL checks for case-sensitivity. I am unable to replicate using MySQL as MySQL does not check for case-sensitivity when using WHERE $field=$value.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          Nice. Thanks.

          Show
          salvetore Michael de Raadt added a comment - Nice. Thanks.
          Hide
          skodak Petr Skoda added a comment - - edited

          hehe, bloody mysql caused this...

          username - the problem is that we lowercase it in login form, but technically there might be some non-lowercase case-sensitive usernames (probably some custom SSO auth); my 1 to simply lowercase the username to make it work exactly the same as login page ( some note explaining why)

          email - the first part of email address (mailbox) is actually case sensitive, the domain is not - we can not lowercase here. There is another problem with special LIKE characters that can be part of the email or submitted text: "_" and "%", I think these should be properly escaped, our DML can do that (I hope).

          Are you going to work on this or should I?

          Show
          skodak Petr Skoda added a comment - - edited hehe, bloody mysql caused this... username - the problem is that we lowercase it in login form, but technically there might be some non-lowercase case-sensitive usernames (probably some custom SSO auth); my 1 to simply lowercase the username to make it work exactly the same as login page ( some note explaining why) email - the first part of email address (mailbox) is actually case sensitive, the domain is not - we can not lowercase here. There is another problem with special LIKE characters that can be part of the email or submitted text: "_" and "%", I think these should be properly escaped, our DML can do that (I hope). Are you going to work on this or should I?
          Hide
          skodak Petr Skoda added a comment -

          I think the proposed patch above would create serious regressions, sorry.

          Hmm, maybe we could fix this directly in the forgot password tool or some new vague_user_search() function ...

          Show
          skodak Petr Skoda added a comment - I think the proposed patch above would create serious regressions, sorry. Hmm, maybe we could fix this directly in the forgot password tool or some new vague_user_search() function ...
          Hide
          skodak Petr Skoda added a comment -

          Hmm, the use of $user = get_complete_user_data('username', $p_username); in the forgot user script seems completely wrong - it is supposed to be used when constructing $USER global, we should probably fix the docs there...

          Show
          skodak Petr Skoda added a comment - Hmm, the use of $user = get_complete_user_data('username', $p_username); in the forgot user script seems completely wrong - it is supposed to be used when constructing $USER global, we should probably fix the docs there...
          Hide
          skodak Petr Skoda added a comment -

          Replaced integration links:
          https://github.com/jasonilicic/moodle/commit/091f1b4786daad73c7663270261276d2316b5003
          https://github.com/jasonilicic/moodle/commit/703a0c5d2e73c98c96f2657b99f44a4fe86a5629

          To integrators: My commit above should be cherry picked to all 2.x branches if approved, thanks.

          Show
          skodak Petr Skoda added a comment - Replaced integration links: https://github.com/jasonilicic/moodle/commit/091f1b4786daad73c7663270261276d2316b5003 https://github.com/jasonilicic/moodle/commit/703a0c5d2e73c98c96f2657b99f44a4fe86a5629 To integrators: My commit above should be cherry picked to all 2.x branches if approved, thanks.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated and backported to 20 and 21 stable, thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated and backported to 20 and 21 stable, thanks!
          Hide
          andyjdavis Andrew Davis added a comment -

          Seems to work as expected although the testing instructions are backwards and describe the buggy rather than the desired behavior.

          Show
          andyjdavis Andrew Davis added a comment - Seems to work as expected although the testing instructions are backwards and describe the buggy rather than the desired behavior.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                10/Oct/11