Moodle
  1. Moodle
  2. MDL-37020

Shibboleth processes wantsurl inconsistently

    Details

    • Testing Instructions:
      Hide
      1. Enable debugging
      2. Open a new browser window (no current Moodle session - including other tabs)
      3. Access /auth/shibboleth/index.php
      4. Verify you see an error about Shibboleth not being setup correctly
      5. Verify you do not see a warning like " Notice: Undefined property: stdClass::$wantsurl" in the output buffer on the page.
      Show
      Enable debugging Open a new browser window (no current Moodle session - including other tabs) Access /auth/shibboleth/index.php Verify you see an error about Shibboleth not being setup correctly Verify you do not see a warning like " Notice: Undefined property: stdClass::$wantsurl" in the output buffer on the page.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-37020-master
    • Rank:
      46559

      Description

      MDL-35153 introduced support for WAYFless URLs in Shibboleth (which is working fine). It does this by checking for an optional "target" parameter passed from an external source. As written the patch doesn't anticipate use cases where both "target" and $SESSION->wantsurl are unset, and you can get a notice like this:

      Notice: Undefined property: stdClass::$wantsurl in .../auth/shibboleth/index.php on line 10

      More serious is that PARAM_LOCALURL behaves unexpectedly when the user isn't already authenticated. During an authentication scenario target can be set with the referring authentication URL. This is properly cleaned by optional_param, but it still results in $SESSION->wantsurl getting set, albeit empty. This means if you've got a bookmarked location on the Moodle instance in question, you're always getting dumped to the front page instead of the deep link unless you're already authenticated.

        Issue Links

          Activity

          Hide
          Damyon Wiese added a comment -

          Added test instructions.

          Show
          Damyon Wiese added a comment - Added test instructions.
          Hide
          Damyon Wiese added a comment -

          Peer review checklist:

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing (I added the testing instructions for you - we really should get a shibboleth test vm setup)
          [Y] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          This patch looks OK to me - and these test instructions allow you to verify the bug without setting up shibboleth.

          Thanks Charles

          Show
          Damyon Wiese added a comment - Peer review checklist: [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing (I added the testing instructions for you - we really should get a shibboleth test vm setup) [Y] Security [-] Documentation [Y] Git [Y] Sanity check This patch looks OK to me - and these test instructions allow you to verify the bug without setting up shibboleth. Thanks Charles
          Hide
          Sam Hemelryk added a comment -

          Thanks Charles, your fix has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Charles, your fix has been integrated now.
          Hide
          Michael de Raadt added a comment -

          Test result: Success.

          Tested in master only.

          Show
          Michael de Raadt added a comment - Test result: Success. Tested in master only.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: