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
    • Rank:
      38199

      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'.

        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: