Moodle

create_user_record requires ipaddress, does not work from scripts

Details

  • Type: Bug Bug
  • Status: Closed 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

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

Attachments

  1. Text File
    mdl-19628-18.diff
    04/Aug/09 9:12 PM
    0.5 kB
    Iñaki Arenaza
  2. Text File
    mdl-19628-19.diff
    04/Aug/09 9:12 PM
    0.5 kB
    Iñaki Arenaza
  3. Text File
    mdl-19628-20.diff
    04/Aug/09 9:12 PM
    0.5 kB
    Iñaki Arenaza

Issue Links

This issue has been marked as being related by:
MDL-17682 Moodle cannot create a new record on user table when a new LDAP entry wants to authenticate Critical Closed
MDL-20110 Cron never runs through the CLI if 'allowedip' has been set Major Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.5#665-sha1:422aead) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.