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

Warning shown when debugging is on and user doesn't exist in LDAP Context (follow up to MDL-41304)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.9, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.8, 2.5.4
    • Component/s: Authentication
    • Labels:
      None
    • Database:
      Any
    • Testing Instructions:
      Hide

      Requirement:

      • LDAP authentication enabled and configured.
      • Debug ALL on
      • LDAP server with at least two different disjoint contexts.
      • A valid LDAP test account that is exists in the second context (but no in the first one).

      Login via LDAP with the test account that exists in only the second context.

      If display errors is on the default redirect after login will show the error. Otherwise, the warnings will be in PHP error log.

      Show
      Requirement: LDAP authentication enabled and configured. Debug ALL on LDAP server with at least two different disjoint contexts. A valid LDAP test account that is exists in the second context (but no in the first one). Login via LDAP with the test account that exists in only the second context. If display errors is on the default redirect after login will show the error. Otherwise, the warnings will be in PHP error log.
    • Workaround:
      Hide

      Change debugging to MINIMAL or NONE.

      Show
      Change debugging to MINIMAL or NONE.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      wip_master_mdl-42723

      Description

      The fix for MDL-41304 is incomplete (as stated there). As the fix has been integrated and the bug closed, I'm opening this bug to fix the remaining cases.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            (argh, cloning issues seems to copy across a load of info it shouldn't.. must create a bug about this)

            Show
            poltawski Dan Poltawski added a comment - (argh, cloning issues seems to copy across a load of info it shouldn't.. must create a bug about this)
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            FYI: I've been testing this and I'm unable to reproduce the errors. I've here two contexts, separated:

            ou=teachers,ou=people,dc=example,dc=com
            ou=students,ou=people,dc=example,dc=com
            (search subcontexts = no)

            And I'm able to login (1st time) both with teacher and students accounts, without getting any error/warn on screen nor logs.

            I only have been able to reproduce some warnings if I get rid of the second context (students) and then try to log as such (only looking in teachers). Or, in other words, if I try to log with an incorrect login. The warnings are:

            Warning: ldap_list(): Search: No such object in lib/ldaplib.php on line 258
             
            and
             
            Warning: ldap_first_entry() expects parameter 2 to be resource, boolean given in lib/ldaplib.php on line 261

            With your patch applied I don't get the warnings any more and the login page is shown again. So I'm going to assume that's the expected behavior and apply the patches.

            Still not sure why it does not present any warning when I've the 2 contexts and log with a user in the 2nd one (that seems to be the use case in testing instructions), so any clarification would be really welcome.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - FYI: I've been testing this and I'm unable to reproduce the errors. I've here two contexts, separated: ou=teachers,ou=people,dc=example,dc=com ou=students,ou=people,dc=example,dc=com (search subcontexts = no) And I'm able to login (1st time) both with teacher and students accounts, without getting any error/warn on screen nor logs. I only have been able to reproduce some warnings if I get rid of the second context (students) and then try to log as such (only looking in teachers). Or, in other words, if I try to log with an incorrect login. The warnings are: Warning: ldap_list(): Search: No such object in lib/ldaplib.php on line 258   and   Warning: ldap_first_entry() expects parameter 2 to be resource, boolean given in lib/ldaplib.php on line 261 With your patch applied I don't get the warnings any more and the login page is shown again. So I'm going to assume that's the expected behavior and apply the patches. Still not sure why it does not present any warning when I've the 2 contexts and log with a user in the 2nd one (that seems to be the use case in testing instructions), so any clarification would be really welcome. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm going to push this. As said, I've got some warnings but unrelated to multiple disjoint contexts. Just if the user does not exist they are shown. The patch solves that and everything continues working ok here. So accepting it.

            But please, clarify if testing instructions are correct or no!

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm going to push this. As said, I've got some warnings but unrelated to multiple disjoint contexts. Just if the user does not exist they are shown. The patch solves that and everything continues working ok here. So accepting it. But please, clarify if testing instructions are correct or no! Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (24, 25 and master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (24, 25 and master), thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            as far as I've here the testing environment ready, I get this. And I halt it awaiting for clarification about my comments above and current testing instructions.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - as far as I've here the testing environment ready, I get this. And I halt it awaiting for clarification about my comments above and current testing instructions.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm considering this passed. I've tested it twice, with existing users, with multiple and only one context, with wrong credentials.. and all the combinations seems to work ok (no more warnings ever).

            So, passed (no matter the testing instructions were misleading).

            Ciao

            PS: I've contacted with Iñaki (using Twitter), but he was off (traveling). NP.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm considering this passed. I've tested it twice, with existing users, with multiple and only one context, with wrong credentials.. and all the combinations seems to work ok (no more warnings ever). So, passed (no matter the testing instructions were misleading). Ciao PS: I've contacted with Iñaki (using Twitter), but he was off (traveling). NP.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            It's Friday, I'm tired so I won't be very imaginative today.

            No matter of that, yes, you did it! Thanks for your collaboration!

            Closing this as fixed!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - It's Friday, I'm tired so I won't be very imaginative today. No matter of that, yes, you did it! Thanks for your collaboration! Closing this as fixed!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14