Moodle
  1. Moodle
  2. MDL-19628

create_user_record requires ipaddress, does not work from scripts

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.5
    • Fix Version/s: 1.9.10, 2.0
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      31625

      Description

      If you call create_user_record from a script or anywhere where $_SERVER['REMOTE_ADDR'] and such are not defined. It will try to insert a user record where lastip is null. This doesn't work because the user table has a non-null constraint on lastip. As a result, things like ldap authentication are broken (see last few comments on MDL-17682).

      Suggestions:

      • Remove non-null constraint. This makes the most sense to me because a user that has just be created by a script shouldn't have a lastip.
      • default to '127.0.0.1' in case getremoteaddr() returns null
      1. mdl-19628-18.diff
        0.5 kB
        Iñaki Arenaza
      2. mdl-19628-19.diff
        0.5 kB
        Iñaki Arenaza
      3. mdl-19628-20.diff
        0.5 kB
        Iñaki Arenaza

        Issue Links

          Activity

          Hide
          Alan Trick added a comment -

          This is also an issue with create_course and enrolment modules.

          Show
          Alan Trick added a comment - This is also an issue with create_course and enrolment modules.
          Hide
          Iñaki Arenaza added a comment -

          I think a better/safer fix is to change create_user_record() to check for a null return value from getremoteaddr and change it to an empty string, which is a valid database value. After all, the only use of getremoteaddr that has trouble with a null return value is create_user_record.

          Something like the attached patch (for 1.8.x, 1.9.x and HEAD) could do it.

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - I think a better/safer fix is to change create_user_record() to check for a null return value from getremoteaddr and change it to an empty string, which is a valid database value. After all, the only use of getremoteaddr that has trouble with a null return value is create_user_record. Something like the attached patch (for 1.8.x, 1.9.x and HEAD) could do it. Saludos. Iñaki.
          Hide
          Alan Trick added a comment -

          Does any of the Moodle code expect it to be a well-formed IPv4 address?

          Show
          Alan Trick added a comment - Does any of the Moodle code expect it to be a well-formed IPv4 address?
          Hide
          Iñaki Arenaza added a comment -

          I don't think so (but I may be wrong).

          Saludos. Iñaki.

          Show
          Iñaki Arenaza added a comment - I don't think so (but I may be wrong). Saludos. Iñaki.
          Hide
          Paul Holden added a comment -

          The problem in this bug is related to the one I've linked in that the function getremoteaddr() doesn't return a value when called from the CLI.

          Show
          Paul Holden added a comment - The problem in this bug is related to the one I've linked in that the function getremoteaddr() doesn't return a value when called from the CLI.
          Hide
          Martin Dougiamas added a comment -

          I'm changing getremoteaddr() to return "0.0.0.0" instead of null by default, and checking all the calls (it was a bit of a mess, quite a few things would get upset with the null). 127.0.0.1 is not really safe, because in some checks that use this function the code will assume the client is the local machine. The default can be changed via a new parameter in case you really want a null.

          eg getremoteaddr(null)

          Show
          Martin Dougiamas added a comment - I'm changing getremoteaddr() to return "0.0.0.0" instead of null by default, and checking all the calls (it was a bit of a mess, quite a few things would get upset with the null). 127.0.0.1 is not really safe, because in some checks that use this function the code will assume the client is the local machine. The default can be changed via a new parameter in case you really want a null. eg getremoteaddr(null)

            People

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

              Dates

              • Created:
                Updated:
                Resolved: