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

Duplicate submission of login clears redirection parameter in session

    XMLWordPrintable

    Details

    • Affected Branches:
      MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE

      Description

      The summary isn't totally correct. This is:
      "When accessing a moodle page that requires login, upon redirection to the login page, duplicate submission of this login, where the first submission clears a redirection parameter, results in the secondary (i.e. actual submissions with a browser still attached) not being redirected."

      Steps to reproduce:
      1. Make sure you are not logged into moodle
      2. Request a moodle page that requires login
      3. [Moodle redirects you to login page]
      4. Enter credentials and submit form multiple times.
      5. [Moodle redirects to self using testsession with $SESSION->wantsurl set to the original requested URL]
      6. [Moodle checks testsession, unsets $SESSION->wantsurl and redirects to the original page]
      7. [The secondary submissions are not redirected because wantsurl is now unset in the session]

      Comments:
      Duplicate submission is a basic problem of the HTTP paradigm. While it is a basic problem, the solutions aren't always so straightforward. Most of the time duplicate submission problems are worked around by preventing successive submissions on the client side via javascript. This may be the eventual solution to this problem.

      The way that this works is by immediately disabling the submit button as the first action of an event attached to form submission. This isn't foolproof, but it does simply prevent multiple form submissions.

      Another idea might be to reduce the amount of time of the race condition on the resulting page. By including config.php (and setup.php and all that entails) prior to checking the testsession URI parameter, we are not minimising the time available for concurrency issues. (This is akin to the javascript solution of disabling the button as the first item in the event, rather than just any old place).

      Battling with concurrency on multiple requests probably isn't worth it though. Possibly the superior solution is for the target page to clear wantsurl, rather than the login page. (Something like, if currenturl == wantsurl, then clear wantsurl or similar). I'm sure that there are obvious flaws with this approach too.

      The thing is for sure though, we can't unset wantsurl in the session on /login/index.php where it is currently (unless the client side javascript is used. I'm not sure how clientside javascript manipulating buttons is viewed by those who champion accessibility.)

      Impact:
      For the majority of cases, wantsurl is a handy little feature, and nobody is too annoyed when it doesn't work. (e.g. Redirecting to homepage instead of the requested course page.) For SSO implementation, where redirects are integral to authentication, this "bug" is DEVASTATING.

      Cheers,

      -David

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              davidaylmer David Aylmer
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan, Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan, Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              7 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated: