Moodle
  1. Moodle
  2. MDL-31629

auth shibboleth print_error() does not use its own language file

    Details

    • Testing Instructions:
      Hide

      Since it's not easy to make a working Shibboelth authentication break (messing with attributes or IdP), simply duplicate a "print_error('shib_not_set_up_error', 'auth');" line outside of an condition check to show the broken behavior is the quickest way to test this.

      Show
      Since it's not easy to make a working Shibboelth authentication break (messing with attributes or IdP), simply duplicate a "print_error('shib_not_set_up_error', 'auth');" line outside of an condition check to show the broken behavior is the quickest way to test this.
    • Workaround:
      Hide

      update the module parameter for the 3 print_error calls in auth/shibboleth/index.php from 'auth' to 'auth_shibboleth'.

      Show
      update the module parameter for the 3 print_error calls in auth/shibboleth/index.php from 'auth' to 'auth_shibboleth'.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-31629-master

      Description

      The 3 print_error() calls in auth/shibboleth/index.php should be pulling strings from its own module language file instead of "auth". This results missing error messages when errors occur.

      The fix is to update the module parameter for the 3 print_error calls from 'auth' to 'auth_shibboleth'.

        Gliffy Diagrams

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and suggesting a solution.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and suggesting a solution.
          Hide
          Michael Aherne added a comment -

          I've created a pull request for this (retaining David Tang as the author)

          Show
          Michael Aherne added a comment - I've created a pull request for this (retaining David Tang as the author)
          Hide
          Dan Poltawski added a comment -

          I think this can be fine to be 'tested' with code review.

          e.g.
          git grep shib_not_set_up_error
          auth/shibboleth/index.php: print_error('shib_not_set_up_error', 'auth');
          auth/shibboleth/index.php: print_error('shib_not_set_up_error', 'auth');
          auth/shibboleth/lang/en/auth_shibboleth.php:$string['shib_not_set_up_error'] = 'Shibboleth authentication ..

          Show
          Dan Poltawski added a comment - I think this can be fine to be 'tested' with code review. e.g. git grep shib_not_set_up_error auth/shibboleth/index.php: print_error('shib_not_set_up_error', 'auth'); auth/shibboleth/index.php: print_error('shib_not_set_up_error', 'auth'); auth/shibboleth/lang/en/auth_shibboleth.php:$string ['shib_not_set_up_error'] = 'Shibboleth authentication ..
          Hide
          Dan Poltawski added a comment -

          Thanks Michael, submitting for integration.

          Are you able to provide the patches for the stable branches?

          Show
          Dan Poltawski added a comment - Thanks Michael, submitting for integration. Are you able to provide the patches for the stable branches?
          Hide
          Michael Aherne added a comment -

          Thanks, Dan! I've created branches for 2.2 and 2.3 stable, but I wasn't sure about the status of 2.0 and 2.1 regarding updates. I'm happy to port the change to them too if you can confirm that they're still under development.

          Show
          Michael Aherne added a comment - Thanks, Dan! I've created branches for 2.2 and 2.3 stable, but I wasn't sure about the status of 2.0 and 2.1 regarding updates. I'm happy to port the change to them too if you can confirm that they're still under development.
          Hide
          Aparup Banerjee 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
          Aparup Banerjee 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
          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
          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
          Sam Hemelryk added a comment -

          Thanks Michael, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Michael, this has been integrated now
          Hide
          Adrian Greeve added a comment -

          Tested on the 2.2, 2.3 and integration branches.
          The error strings point to the right place in the language file.
          Test passed.

          Show
          Adrian Greeve added a comment - Tested on the 2.2, 2.3 and integration branches. The error strings point to the right place in the language file. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: