Uploaded image for project: '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

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting that and suggesting a solution.

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

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

            Show
            maherne Michael Aherne added a comment - I've created a pull request for this (retaining David Tang as the author)
            Hide
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            Thanks Michael, submitting for integration.

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

            Show
            poltawski Dan Poltawski added a comment - Thanks Michael, submitting for integration. Are you able to provide the patches for the stable branches?
            Hide
            maherne 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
            maherne 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
            nebgor 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
            nebgor 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
            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
            samhemelryk Sam Hemelryk added a comment -

            Thanks Michael, this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Michael, this has been integrated now
            Hide
            abgreeve 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
            abgreeve 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

            (not really)

            Closing, thanks!

            Show
            stronk7 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:
                  Fix Release Date:
                  12/Nov/12