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

REST simpleserver does not accept the moodlewsrestformat parameter

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2.2
    • Component/s: Web Services
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Call the simpleserver.php server instead of the server.php.
      Use the REST client demo and call simpleserver.php instead server.php. Set wsusername/wspassword parameters instead of the token parameter. That should be the only things to change in your REST demo client (in the condition that it was already preset and working). Also the user must have 'webservice' authentication method set. Note that you can not log into Moodle when this authentication method is set.

      Show
      Call the simpleserver.php server instead of the server.php. Use the REST client demo and call simpleserver.php instead server.php. Set wsusername/wspassword parameters instead of the token parameter. That should be the only things to change in your REST demo client (in the condition that it was already preset and working). Also the user must have 'webservice' authentication method set. Note that you can not log into Moodle when this authentication method is set.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:

      Description

      server.php accepts the new moodlewsrestformat parameter to allow json output, however simpleserver.php is lacking that.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks for reporting that Sebastian. I'll add it to STABLE backlog.

            Sebastian, can I ask you why you didn't use the token authentication? I now think sending username/password was in fact a bad idea to let you do that specially if the server is not HTTPS.

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks for reporting that Sebastian. I'll add it to STABLE backlog. Sebastian, can I ask you why you didn't use the token authentication? I now think sending username/password was in fact a bad idea to let you do that specially if the server is not HTTPS.
            Hide
            hal9000 Sebastian Vassiliou added a comment -

            Hi Jerome,

            I didn't use token cause the other one is "simple"
            I'm developing a Moodle app for Windows Phone and found it unpractical for the user to have him enter a huge token. And if I want the app to get a token based on username/password, I would have to send those to moodle anyway, right?

            Show
            hal9000 Sebastian Vassiliou added a comment - Hi Jerome, I didn't use token cause the other one is "simple" I'm developing a Moodle app for Windows Phone and found it unpractical for the user to have him enter a huge token. And if I want the app to get a token based on username/password, I would have to send those to moodle anyway, right?
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Hi Sebastian,
            your client can get the user token from /login/token.php. So you send the username/password only once the first time, and not for every request.

            See: http://docs.moodle.org/dev/Creating_a_web_service_client#How_to_get_a_user_token

            Show
            jerome Jérôme Mouneyrac added a comment - Hi Sebastian, your client can get the user token from /login/token.php. So you send the username/password only once the first time, and not for every request. See: http://docs.moodle.org/dev/Creating_a_web_service_client#How_to_get_a_user_token
            Hide
            hal9000 Sebastian Vassiliou added a comment -

            Ok thanks. Since I am also using OKtech and a custom service with more functions enabled than the mobile service, I guess I will have to manually set the shortname for those in the DB and get one token per service, correct?

            Show
            hal9000 Sebastian Vassiliou added a comment - Ok thanks. Since I am also using OKtech and a custom service with more functions enabled than the mobile service, I guess I will have to manually set the shortname for those in the DB and get one token per service, correct?
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Yes, it's correct. Thanks to remind me that username/password have access to all service. I think this authentication should be deprecated/removed from 2.3. I'm going to write an issue for that.

            Show
            jerome Jérôme Mouneyrac added a comment - Yes, it's correct. Thanks to remind me that username/password have access to all service. I think this authentication should be deprecated/removed from 2.3. I'm going to write an issue for that.
            Hide
            jerome Jérôme Mouneyrac added a comment -
            Show
            jerome Jérôme Mouneyrac added a comment - I wrote this issue: http://tracker.moodle.org/browse/MDL-30696
            Hide
            hal9000 Sebastian Vassiliou added a comment -

            Thank you Jerome, I will make appropriate changes to the App
            Really looking forward to see that Web Services Roadmap being implemented, but at least the get_content function of moodle 2.2 kind of saved my day.
            Btw, do you know if I can name the App "Moodle" and if I can use the Moodle logo, or will there be conflicts?

            Show
            hal9000 Sebastian Vassiliou added a comment - Thank you Jerome, I will make appropriate changes to the App Really looking forward to see that Web Services Roadmap being implemented, but at least the get_content function of moodle 2.2 kind of saved my day. Btw, do you know if I can name the App "Moodle" and if I can use the Moodle logo, or will there be conflicts?
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            "Moodle" and also the logo are trademark - so they can't be used. You can contact Moodle.com for more information: http://moodle.com/feedback/. I don't know much more about this subject.

            Show
            jerome Jérôme Mouneyrac added a comment - - edited "Moodle" and also the logo are trademark - so they can't be used. You can contact Moodle.com for more information: http://moodle.com/feedback/ . I don't know much more about this subject.
            Hide
            hal9000 Sebastian Vassiliou added a comment -

            Thank you very much!

            Show
            hal9000 Sebastian Vassiliou added a comment - Thank you very much!
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Hi Sebastian, in fact I have to change what I said. The simple authentication is not "not recommended" it just doesn't serve the same purpose that the token authentication. The simple authentication can not be used for web user: http://docs.moodle.org/dev/External_services_security#Simplified_web_services. Sorry for the misleading, we'll not deprecate the simple authentication method. Actually we should rename it

            At the end, it is the same result for you. You have to use the token authentication for your Mobile app.

            Show
            jerome Jérôme Mouneyrac added a comment - Hi Sebastian, in fact I have to change what I said. The simple authentication is not "not recommended" it just doesn't serve the same purpose that the token authentication. The simple authentication can not be used for web user: http://docs.moodle.org/dev/External_services_security#Simplified_web_services . Sorry for the misleading, we'll not deprecate the simple authentication method. Actually we should rename it At the end, it is the same result for you. You have to use the token authentication for your Mobile app.
            Hide
            hal9000 Sebastian Vassiliou added a comment -

            Yeah I realized I need to do that anyway yesterday, when I tried to download a file Will make the switch today, will surely be easy to implement.
            Thank you for your support!

            Show
            hal9000 Sebastian Vassiliou added a comment - Yeah I realized I need to do that anyway yesterday, when I tried to download a file Will make the switch today, will surely be easy to implement. Thank you for your support!
            Hide
            jerome Jérôme Mouneyrac added a comment -

            $_REQUEST part of the patch need to be removed if MDL-30495 do not use $_REQUEST anymore in the web service structure.

            Show
            jerome Jérôme Mouneyrac added a comment - $_REQUEST part of the patch need to be removed if MDL-30495 do not use $_REQUEST anymore in the web service structure.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Jerome,

            Could you provide testing instruction for this issue?

            Thanks

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Jerome, Could you provide testing instruction for this issue? Thanks
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Done, you can come over if this is complicated. It's not a recommended way to use web services so there is no documentation about it.

            Show
            jerome Jérôme Mouneyrac added a comment - Done, you can come over if this is complicated. It's not a recommended way to use web services so there is no documentation about it.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Jerome,

            It would be better if you combine the if/elseif check for moodlewsrestformat, it could save some site processing. Also, I think it is unnecessary to check for $_REQUEST, since the param could only be $_GET or $_POST.

            if (isset($_GET)) {
              unset()
            } else if (isset($_POST)) {
              unset()
            }

            Finally, please fix the spacing for patch.

            Other than that, the patch is working great.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Jerome, It would be better if you combine the if/elseif check for moodlewsrestformat, it could save some site processing. Also, I think it is unnecessary to check for $_REQUEST, since the param could only be $_GET or $_POST. if (isset($_GET)) { unset() } else if (isset($_POST)) { unset() } Finally, please fix the spacing for patch. Other than that, the patch is working great.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks Rosie for the peer review.

            I let $_REQUEST/$_GET/$_POST together because POST/GET cleaning is appearing with MDL-30495 and I don't think it breaks anything to cleanup $_REQUEST. But if it is really bad, integration can fail and I can remove it. Then it would need to wait for MDL-30495 to be integrated.

            About $_GET and $_POST they can be filled with the same key. It is true that you can not do GET and POST HTTP request in one request. However PHP will put all params contained in the URL into the $_GET even thought there are $_POST parameters.

            What spacing problem are you talking about?

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks Rosie for the peer review. I let $_REQUEST/$_GET/$_POST together because POST/GET cleaning is appearing with MDL-30495 and I don't think it breaks anything to cleanup $_REQUEST. But if it is really bad, integration can fail and I can remove it. Then it would need to wait for MDL-30495 to be integrated. About $_GET and $_POST they can be filled with the same key. It is true that you can not do GET and POST HTTP request in one request. However PHP will put all params contained in the URL into the $_GET even thought there are $_POST parameters. What spacing problem are you talking about?
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Ah I see the spacing issue...

            Show
            jerome Jérôme Mouneyrac added a comment - Ah I see the spacing issue...
            Hide
            jerome Jérôme Mouneyrac added a comment -

            spacing fixed. Fix backported to 2.2 too.

            Show
            jerome Jérôme Mouneyrac added a comment - spacing fixed. Fix backported to 2.2 too.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jerome, this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jerome, this has been integrated now
            Hide
            gerry Gerard Caulfield added a comment -

            Test passed on master and 2.2. Well done.

            Show
            gerry Gerard Caulfield added a comment - Test passed on master and 2.2. Well done.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks!

            Closing as fixed, heading to zzzZZZzzz, niao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks! Closing as fixed, heading to zzzZZZzzz, niao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12