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

Warning shown when debugging is on and user doesn't exist in LDAP Context

    Details

    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide

      Requirement:

      • Debug ALL on
      • "Search subcontexts" is enable in LDAP settings
      • LDAP account in say staff context and different context for students

      Login via LDAP with an account that exists in only one context.
      If display errors is on the default redirect after login will show the error or in the error_log.

      Show
      Requirement: Debug ALL on "Search subcontexts" is enable in LDAP settings LDAP account in say staff context and different context for students Login via LDAP with an account that exists in only one context. If display errors is on the default redirect after login will show the error or in the error_log.
    • Workaround:
      Hide

      Change debugging to MINIMAL or NONE.

      Show
      Change debugging to MINIMAL or NONE.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      mdl41304-master

      Description

      We have recently turned on full debugging in our TEST environment and found the warning message prevents redirection:

      Warning: ldap_search(): Search: No such object in /var/www/moodle/lib/ldaplib.php on line 241
       
      Warning: ldap_first_entry() expects parameter 2 to be resource, boolean given in /var/www/moodle/lib/ldaplib.php on line 248
      

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting that and providing patches. Feel free to put that up for peer review.

            You might want to add a bit more detail to the testing instructions. Most of the devs around here have limited LDAP experience.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting that and providing patches. Feel free to put that up for peer review. You might want to add a bit more detail to the testing instructions. Most of the devs around here have limited LDAP experience.
            Hide
            markn Mark Nelson added a comment -

            Hi Tim,

            We tend to avoid using @ to suppress warnings with any newly introduced code, however in this case I see no other alternative. The logic seems fine to me.

            Thanks for your contribution.

            Regards,

            Mark

            Show
            markn Mark Nelson added a comment - Hi Tim, We tend to avoid using @ to suppress warnings with any newly introduced code, however in this case I see no other alternative. The logic seems fine to me. Thanks for your contribution. Regards, Mark
            Hide
            tlock Tim Lock added a comment -

            Hi Mark,

            Thanks and I agree that silencing errors with an @ should be avoided if possible.

            Show
            tlock Tim Lock added a comment - Hi Mark, Thanks and I agree that silencing errors with an @ should be avoided if possible.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Tim - i've integrated it to master, 25 and 24

            Show
            poltawski Dan Poltawski added a comment - Thanks Tim - i've integrated it to master, 25 and 24
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for fixing this Tim,

            Passing test...

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Tim, Passing test...
            Hide
            poltawski Dan Poltawski added a comment -

            Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke:

            A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

            Show
            poltawski Dan Poltawski added a comment - Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke: A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"
            Hide
            iarenaza Iñaki Arenaza added a comment -

            I hate to re-open this, but the fix is incomplete. If you configure the LDAP auth plugin not to search subcontexts, you get the same warnings (under the same circumstances). So the check for $ldap_result and the break sentence should go after the "if then else" block, before the call to ldap_first_entry(). And we would also need to silence the call to ldap_list() for the same reasons.

            I'll prepare patches in a few hours and will put the links to github here.

            Show
            iarenaza Iñaki Arenaza added a comment - I hate to re-open this, but the fix is incomplete. If you configure the LDAP auth plugin not to search subcontexts, you get the same warnings (under the same circumstances). So the check for $ldap_result and the break sentence should go after the "if then else" block, before the call to ldap_first_entry(). And we would also need to silence the call to ldap_list() for the same reasons. I'll prepare patches in a few hours and will put the links to github here.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Iñaki,

            Can we create a new issue for this, as reopening the issue doesn't really work with our workflow.

            thanks

            Show
            poltawski Dan Poltawski added a comment - Hi Iñaki, Can we create a new issue for this, as reopening the issue doesn't really work with our workflow. thanks
            Hide
            iarenaza Iñaki Arenaza added a comment -

            Sure, Dan.

            Show
            iarenaza Iñaki Arenaza added a comment - Sure, Dan.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks, sending this back to closed. Please link it to here.

            Show
            poltawski Dan Poltawski added a comment - Thanks, sending this back to closed. Please link it to here.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Sep/13