Moodle
  1. Moodle
  2. MDL-30688

REST simpleserver does not accept the moodlewsrestformat parameter

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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:
    • Rank:
      33511

      Description

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Jérôme Mouneyrac added a comment -
          Show
          Jérôme Mouneyrac added a comment - I wrote this issue: http://tracker.moodle.org/browse/MDL-30696
          Hide
          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
          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
          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
          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
          Sebastian Vassiliou added a comment -

          Thank you very much!

          Show
          Sebastian Vassiliou added a comment - Thank you very much!
          Hide
          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
          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
          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
          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
          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
          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
          Rossiani Wijaya added a comment -

          Hi Jerome,

          Could you provide testing instruction for this issue?

          Thanks

          Show
          Rossiani Wijaya added a comment - Hi Jerome, Could you provide testing instruction for this issue? Thanks
          Hide
          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
          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
          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
          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
          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
          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
          Jérôme Mouneyrac added a comment -

          Ah I see the spacing issue...

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

          spacing fixed. Fix backported to 2.2 too.

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

          Thanks Jerome, this has been integrated now

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

          Test passed on master and 2.2. Well done.

          Show
          Gerard Caulfield added a comment - Test passed on master and 2.2. Well done.
          Hide
          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
          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: