Moodle
  1. Moodle
  2. MDL-29805

token.php does not return the token sometimes

    Details

      Description

      Some people report that token.php throws the following error message:
      Unsupported redirect detected, script execution terminated
      http://moodle.org/mod/forum/discuss.php?d=185535#p819426
      http://moodle.org/mod/forum/discuss.php?d=185535#p812332

      I'm not sure where this redirection happens:
      http://moodle.org/mod/forum/discuss.php?d=185535#p814206

      When I try to acces to my moodle site form the iPhone official application i get this error message:

      Please check your login detail or ask your site administrator to check moodle configuration.

      Mobile web services are enablen in my site, and user autentication method is set to manual as you said.

      When I try to access to token.php (with the correct params, of course) using chorme, the token is shown in the response:

      "token":"6d056544345..."

      But doing the same with firefox... a window to download token.php appear!! I'm using the latest version of moodle form mac, downloaded today and I'm not using https.

      Taking a look to the generated traffic (usign wireskark) when I try to login using the official mobile web app I noticed the next error:

      Unsupported redirect detected, script execution terminated

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Jérôme Mouneyrac added a comment -

            note that I cannot reproduce so if someone end up to get the issue, please speak up here. Cheers.

            Show
            Jérôme Mouneyrac added a comment - note that I cannot reproduce so if someone end up to get the issue, please speak up here. Cheers.
            Hide
            Jérôme Mouneyrac added a comment -

            Decreasing the priority to Normal as nobody seems to bother anymore (no votes)

            Show
            Jérôme Mouneyrac added a comment - Decreasing the priority to Normal as nobody seems to bother anymore (no votes)
            Hide
            Jérôme Mouneyrac added a comment -

            CAS authentication plugin does a redirection on a different login page when a user try to login in Moodle. I suspect it could be related to the issue. Dongsheng is going to implement the CAS support for the Mobile app. I assign this issue to you Dongsheng. Browse the forum discussions linked in this issue description, it could help you

            Show
            Jérôme Mouneyrac added a comment - CAS authentication plugin does a redirection on a different login page when a user try to login in Moodle. I suspect it could be related to the issue. Dongsheng is going to implement the CAS support for the Mobile app. I assign this issue to you Dongsheng. Browse the forum discussions linked in this issue description, it could help you
            Hide
            unimatrix added a comment -

            This happens because a function called initialise_fullme() in /lib/setuplib.php is checking whether the hostname you are sending your request to matches the hostname defined by $CFG->wwwroot in your config.php. If it doesn't, it calls redirect() which doesn't allow AJAX scripts and thus rejects the redirection.
            So for example, if your $CFG->wwwroot is defined using an IP address and you want to access token.php via your hostname, you will get this error.

            Show
            unimatrix added a comment - This happens because a function called initialise_fullme() in /lib/setuplib.php is checking whether the hostname you are sending your request to matches the hostname defined by $CFG->wwwroot in your config.php. If it doesn't, it calls redirect() which doesn't allow AJAX scripts and thus rejects the redirection. So for example, if your $CFG->wwwroot is defined using an IP address and you want to access token.php via your hostname, you will get this error.
            Hide
            Jérôme Mouneyrac added a comment -

            Thanks unimatrix, I'll have a look to provide a fix.

            Show
            Jérôme Mouneyrac added a comment - Thanks unimatrix, I'll have a look to provide a fix.
            Hide
            Jérôme Mouneyrac added a comment -

            Assigning to me...

            Show
            Jérôme Mouneyrac added a comment - Assigning to me...
            Hide
            Jérôme Mouneyrac added a comment -

            Hi unimatrix,
            as you mention Moodle does not allow you to access it from a different url and it always redirect to the correct $CFG->wwwroot. We could "hack" $_SERVER['HTTP_HOST'] to set it to $CFG->wwwroot but I don't find a good reason to call token.php with a different url. Do you have one in mind?

            Show
            Jérôme Mouneyrac added a comment - Hi unimatrix, as you mention Moodle does not allow you to access it from a different url and it always redirect to the correct $CFG->wwwroot. We could "hack" $_SERVER ['HTTP_HOST'] to set it to $CFG->wwwroot but I don't find a good reason to call token.php with a different url. Do you have one in mind?
            Hide
            unimatrix added a comment -

            The only reason I can think of why somebody would call it from a different URL is not knowing about Moodle's "no redirect in AJAX scripts" policy.
            The way I see it you have two options:
            A) What you suggested: somehow allow it to redirect (which means either changing or breaking the rules).
            B) Not allow it to redirect, but notify the user (through an error message) about what they are doing wrong, to prevent confusion.

            For convenience choose A, for security choose B.

            Show
            unimatrix added a comment - The only reason I can think of why somebody would call it from a different URL is not knowing about Moodle's "no redirect in AJAX scripts" policy. The way I see it you have two options: A) What you suggested: somehow allow it to redirect (which means either changing or breaking the rules). B) Not allow it to redirect, but notify the user (through an error message) about what they are doing wrong, to prevent confusion. For convenience choose A, for security choose B.
            Hide
            Jérôme Mouneyrac added a comment -

            +1 for B. I'll make a patch to throw an explicit error.

            Show
            Jérôme Mouneyrac added a comment - +1 for B. I'll make a patch to throw an explicit error.
            Hide
            unimatrix added a comment -

            B would've been my choice too.

            Show
            unimatrix added a comment - B would've been my choice too.
            Hide
            Rossiani Wijaya added a comment -

            Hi Jerome,

            I got the following error

            Notice: Undefined index: port in /stable/22/lib/setuplib.php on line 703

            As for the new string, it might be better to use "invalid" instead of "wrong".

            Show
            Rossiani Wijaya added a comment - Hi Jerome, I got the following error Notice: Undefined index: port in /stable/22/lib/setuplib.php on line 703 As for the new string, it might be better to use "invalid" instead of "wrong".
            Hide
            Jérôme Mouneyrac added a comment -

            Thanks Rosie, I'll make the changes.

            Show
            Jérôme Mouneyrac added a comment - Thanks Rosie, I'll make the changes.
            Hide
            Sam Hemelryk added a comment -

            Hi Jerome,

            I'm sending this back sorry.
            I think the concept is spot on however I think that we should both create a generic "no redirect" define, and touch up the error message.

            The "no redirect" define idea comes because I don't like adding a check for a single specific script to setuplib.php and because I imagine there will be other circumstances in the future where we will want to prevent that redirect in the same way.
            I considered using AJAX_SCRIPT as token.php defines that and I can't imagine an AJAX request going to the wrong location unless someone was hacking. However after thinking it through I think its probably clearer to have a separate define.
            I was thinking something along the lines of "REQUIRE_CORRECT_ACCESS".
            Then move the error message from webservice.php to error.php

            Also just a thought, it would be clearer in the debug info for the exception if you used the standard representation of a URL and port.
            e.g. www.localhost.com:80

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Jerome, I'm sending this back sorry. I think the concept is spot on however I think that we should both create a generic "no redirect" define, and touch up the error message. The "no redirect" define idea comes because I don't like adding a check for a single specific script to setuplib.php and because I imagine there will be other circumstances in the future where we will want to prevent that redirect in the same way. I considered using AJAX_SCRIPT as token.php defines that and I can't imagine an AJAX request going to the wrong location unless someone was hacking. However after thinking it through I think its probably clearer to have a separate define. I was thinking something along the lines of "REQUIRE_CORRECT_ACCESS". Then move the error message from webservice.php to error.php Also just a thought, it would be clearer in the debug info for the exception if you used the standard representation of a URL and port. e.g. www.localhost.com:80 Cheers Sam
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment - - edited

            Thanks Sam. I did an additional change which is returning errorcode for AJAX_SCRIPT (early init or not). I think it would be nice to return the error code as a client can code automatic response against it.

            PS: I don't think it breaks anything to add errorcode in the retrun value of AJAX_SCRIPT. But just for the info, it's not essential to have the error code (at the end the issue is about to let the client dev knows why the call fails).

            Show
            Jérôme Mouneyrac added a comment - - edited Thanks Sam. I did an additional change which is returning errorcode for AJAX_SCRIPT (early init or not). I think it would be nice to return the error code as a client can code automatic response against it. PS: I don't think it breaks anything to add errorcode in the retrun value of AJAX_SCRIPT. But just for the info, it's not essential to have the error code (at the end the issue is about to let the client dev knows why the call fails).
            Hide
            Sam Hemelryk added a comment -

            Hi Jerome,

            Things look perfect except for one minor thing; as well as checking REQUIRE_CORRECT_ACCESS is defined you must also check it is true.

            if (defined('REQUIRE_CORRECT_ACCESS') && REQUIRE_CORRECT_ACCESS) {
            

            I'll integrated that branches as soon as they've been fixed up and I've left this in integration review in order to allow you to do it this round.

            Many thanks
            Sam

            Show
            Sam Hemelryk added a comment - Hi Jerome, Things look perfect except for one minor thing; as well as checking REQUIRE_CORRECT_ACCESS is defined you must also check it is true. if (defined('REQUIRE_CORRECT_ACCESS') && REQUIRE_CORRECT_ACCESS) { I'll integrated that branches as soon as they've been fixed up and I've left this in integration review in order to allow you to do it this round. Many thanks Sam
            Hide
            Sam Hemelryk added a comment -

            This has been integrated now. I made the required change during integration.

            Show
            Sam Hemelryk added a comment - This has been integrated now. I made the required change during integration.
            Hide
            Jérôme Mouneyrac added a comment -

            Oops, yes it would have been much better to check the value. Thanks for fixing it Sam.

            Show
            Jérôme Mouneyrac added a comment - Oops, yes it would have been much better to check the value. Thanks for fixing it Sam.
            Hide
            Rajesh Taneja added a comment -

            Thanks Jerome, works fine.
            got

            {"error":"Invalid url or port.","stacktrace":null,"debuginfo":null,"errorcode":"requirecorrectaccess"}

            while trying to access it with localhost.

            Show
            Rajesh Taneja added a comment - Thanks Jerome, works fine. got {"error":"Invalid url or port.","stacktrace":null,"debuginfo":null,"errorcode":"requirecorrectaccess"} while trying to access it with localhost.
            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:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: