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

          Attachments

            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