Details

    • Rank:
      44759

      Description

      New mobile admin page:

      • with CSS url (also return this information in the web service get_site_info)
      • move enable web service to this admin page

      Go to Admin > Mobile

      • check that you can enter and save a css url (just enter any url)
      • check that you can enable/disable mobile settings. If you have no service except the mobile one (Admin > plugins > Web service > Manage service), enabling/disabling this setting should enable/disable the REST and XMLRPC protocol (Admin > plugins > Web service > Manage protocols).

        Issue Links

          Activity

          Hide
          Jérôme Mouneyrac added a comment -

          Juan said:

          I was thinking that maybe is a good idea move the current enable mobile service checkbox to the new admin form

          +1

          Show
          Jérôme Mouneyrac added a comment - Juan said: I was thinking that maybe is a good idea move the current enable mobile service checkbox to the new admin form +1
          Hide
          Michael de Raadt added a comment -

          Don't forget to add 'ui_change' and 'docs_required' labels when this is sent to integration.

          Show
          Michael de Raadt added a comment - Don't forget to add 'ui_change' and 'docs_required' labels when this is sent to integration.
          Hide
          Jérôme Mouneyrac added a comment -

          Juan, can you confirm that:

          • CSS url is empty by default?
          • Alternate login is empty by default?
          Show
          Jérôme Mouneyrac added a comment - Juan, can you confirm that: CSS url is empty by default? Alternate login is empty by default?
          Hide
          Juan Leyva added a comment -

          Hi Jerome,

          yes I confirm that CSS url and alternate auth url are empty by default.

          Regarding "alternate auth url" I was thinking that returning it in the get_site_info doesn't make sense, because you need to be authenticated previously to call this WS for getting this setting, so, at least the first time you log in you will authenticate against the standard login/token.php file (and that's not make sense)

          I think that the correct way for implementing the "alternate auth url" issue should be modifying the login/token.php so when this setting is present, instead of doing this

          $user = authenticate_user_login($username, $password);

          it should be done something like a curl call to the alternate auth url sending username and password (alternate auth url must be under https for strict security), the response should be a JSON object indicating true or false

          this makes sense for you?

          Regards

          Show
          Juan Leyva added a comment - Hi Jerome, yes I confirm that CSS url and alternate auth url are empty by default. Regarding "alternate auth url" I was thinking that returning it in the get_site_info doesn't make sense, because you need to be authenticated previously to call this WS for getting this setting, so, at least the first time you log in you will authenticate against the standard login/token.php file (and that's not make sense) I think that the correct way for implementing the "alternate auth url" issue should be modifying the login/token.php so when this setting is present, instead of doing this $user = authenticate_user_login($username, $password); it should be done something like a curl call to the alternate auth url sending username and password (alternate auth url must be under https for strict security), the response should be a JSON object indicating true or false this makes sense for you? Regards
          Hide
          Jérôme Mouneyrac added a comment -

          Thanks Juan I understand, it makes sens now. However I'll have a better look because I'm not sure about:

          • identity providers that don't request password (Google, Mnet...) in the mobile use case
          • how the existing web alternate url interacts with auth plugins.
            I'll look to all that and come back to you.
          Show
          Jérôme Mouneyrac added a comment - Thanks Juan I understand, it makes sens now. However I'll have a better look because I'm not sure about: identity providers that don't request password (Google, Mnet...) in the mobile use case how the existing web alternate url interacts with auth plugins. I'll look to all that and come back to you.
          Hide
          Jérôme Mouneyrac added a comment -

          I've been thinking about that...

          • For identity providers without "login/password", they normally require some kind of "access token" which is like a password at the end. So in fact it does not change much. What we should do is to call a new auth plugin method like get_web_service_token($login, $password) that does either the authenticate_user_login() or either execute some code related to the identity provider. Each auth plugin should have this method.
          • For the mobilealternateloginurl, is it just something to extend alternateloginurl? If yes then the curl call to this mobilealternateloginurl should return a login/password too? It's what alternateloginurl does. I may be misunderstanding something
          Show
          Jérôme Mouneyrac added a comment - I've been thinking about that... For identity providers without "login/password", they normally require some kind of "access token" which is like a password at the end. So in fact it does not change much. What we should do is to call a new auth plugin method like get_web_service_token($login, $password) that does either the authenticate_user_login() or either execute some code related to the identity provider. Each auth plugin should have this method. For the mobilealternateloginurl, is it just something to extend alternateloginurl? If yes then the curl call to this mobilealternateloginurl should return a login/password too? It's what alternateloginurl does. I may be misunderstanding something
          Hide
          Juan Leyva added a comment -

          Thanks Jerome for your feedback.

          I will try to clarify myself:

          I always have talked about alternate auth url (not login url), the auth url is an authentication endpoint that return true/false when an user try to login. This is the reason I was thinking in hacking the login/token.php script.

          As you mentioned before, instead hacking the login/token.php one possibility is to extend auth plugins adding a new method get_web_service_token, this is ok for custom auth plugins but what about core plugins like Shibboleth?

          I think that there is not a common way for implementing a fake Shibboleth auth so different institutions may be forced to implement their own solution, so if we place the authentication inside the auth plugin they will have to change the core with their own customizations.

          Using and authentication hook in the login/token.php is in some ways cleaner but is not as elegant as the solution you pointed out.

          I think that the main problem is what to do with Shibboleth auth, we need an alternate way to do user authentication that works fine with Shibboleth

          Regards

          Regards

          Show
          Juan Leyva added a comment - Thanks Jerome for your feedback. I will try to clarify myself: I always have talked about alternate auth url (not login url), the auth url is an authentication endpoint that return true/false when an user try to login. This is the reason I was thinking in hacking the login/token.php script. As you mentioned before, instead hacking the login/token.php one possibility is to extend auth plugins adding a new method get_web_service_token, this is ok for custom auth plugins but what about core plugins like Shibboleth? I think that there is not a common way for implementing a fake Shibboleth auth so different institutions may be forced to implement their own solution, so if we place the authentication inside the auth plugin they will have to change the core with their own customizations. Using and authentication hook in the login/token.php is in some ways cleaner but is not as elegant as the solution you pointed out. I think that the main problem is what to do with Shibboleth auth, we need an alternate way to do user authentication that works fine with Shibboleth Regards Regards
          Hide
          Juan Leyva added a comment -

          From the dev chat...

          [11:48:37] <jleyva@jabber.org> Jerome, about the mobile auth issue
          [11:49:01] <mouneyrac> yes (MDL-35998)
          [11:49:01] <moodlebot> http://tracker.moodle.org/browse/MDL-35998 New mobile menu: with two options CSS and Alternate auth url + existing enable mobile web services - Assignee: Jerome Mouneyrac Type: Sub-task Status: Waiting for peer review V:0 W:1 Priority: Blocker
          [11:49:21] <jleyva@jabber.org> I'm not very sure but I think that the only auth plugin that requires additional interaction to the user (it gets redirected to a protected directory) to work is the Shibboleth one
          [11:49:50] <jleyva@jabber.org> so, the only auth system that will not work properly for the mobile app will be then Shibboleth
          [11:50:36] <jleyva@jabber.org> do you know how Shibboleth auth works in Moodle?
          [11:50:49] <mouneyrac> Juan: I know nothing about shibbolteh. Is that widely spread? Why does it require an external login?
          [11:51:16] <jleyva@jabber.org> is the way it was implemented, the user is redirected automatically or using a link to a protected directory
          [11:51:27] *** Dan Marsden (dan@catalyst.net.nz/Office) has left the room
          [11:51:36] <jleyva@jabber.org> the protected directory has, usally, the Shibolleth auth Apache's module
          [11:52:07] <jleyva@jabber.org> so the user is prompted to enter username and password, and the server sets a cookie or server variable
          [11:52:42] <jleyva@jabber.org> that's the reason it's not going to work from a mobile app
          [11:52:56] *** Sam Hemelryk (samhemelryk@moodle.org/Sam’s MacBook Pro) has joined the room as a participant
          [11:53:12] <mouneyrac> that seems weird
          [11:53:24] <mouneyrac> but is the username know by Moodle?
          [11:54:01] *** Sam Hemelryk (samhemelryk@moodle.org/Sam’s MacBook Pro) has left the room
          [11:54:02] <mouneyrac> what we can do if that we detect auth method == shibolleth and do the curl call in this case
          [11:54:05] <jleyva@jabber.org> the username and other data is stored in SERVER variables
          [11:54:14] <jleyva@jabber.org> I like a lot your idea of extending auth plugins, but I'm not sure if we can do a fake login to the protected directory using a curl call
          [11:54:15] <mouneyrac> for the other auth plugin it will work
          [11:54:42] <jleyva@jabber.org> yes
          [11:55:05] <jleyva@jabber.org> but the point here is, are you going to add a condition to login/token or extend auth plugins for the shibolleth unique case?
          [11:55:05] <mouneyrac> ok it seems reasonable
          [11:55:26] <jleyva@jabber.org> +1 for extending auth plugins, because it will help custom auth plugins to handle the mobile auth
          [11:55:33] <mouneyrac> we need to extend the auth for supporting mutlple auth introduce by oauth2 plugin
          [11:56:35] *** ankit (ankit@moodle.org/5519828981350439913425172) has left the room
          [11:57:44] *** Cindereloy (stronk7@contiento.com/@ unlockingme) has left the room
          [11:59:02] *** tsala (tsala@moodle.org/developers) has left the room
          [11:59:03] <jleyva@jabber.org> I'm reading about some guys that has implemented curl calls against shibboleths directories, it seems to be possible
          [11:59:14] *** Dan P (poltawski@moodle.org/Adium Macbook Air) has left the room
          [11:59:23] <jleyva@jabber.org> In fact, is a standard Apache's protected directory
          [12:00:03] <mouneyrac> ls
          [12:00:12] <mouneyrac> sorry
          [12:00:39] <jleyva@jabber.org> some code here http://shibboleth.1660669.n2.nabble.com/php-class-to-curl-shib-protected-site-td2439110.html , easy to implement apparently
          [12:01:23] <jleyva@jabber.org> so Jerome, then are you going add the extra function to auth plugins? and also implement the shibboleth one? The last can be done for a next minor release I suppose
          [12:01:46] <jleyva@jabber.org> There are some guys interested that may help
          [12:01:59] <jleyva@jabber.org> see MOBILE-153
          [12:01:59] <moodlebot> http://tracker.moodle.org/browse/MOBILE-153 Move UMM to MM - Assignee: Juan Leyva Type: New Feature Status: Open Priority: Blocker
          [12:02:03] <jleyva@jabber.org> (last comments)
          [12:02:18] <mouneyrac> yes I'll have to add the auth method for MDL-34426
          [12:02:18] <moodlebot> http://tracker.moodle.org/browse/MDL-34426 Port Google/Facebook Oauth2 plugin into core - Assignee: Jerome Mouneyrac Type: Task Status: Waiting for integration review V:1 W:4 Priority: Major
          [12:03:57] <mouneyrac> it would be good if the app support oauth2 authentication with google facebook too
          [12:05:04] <mouneyrac> the idea is to request the Google access token from the html5, then send it to token.php that send a ws token back after verifying the access token with Google (throught the new auth method)

          Show
          Juan Leyva added a comment - From the dev chat... [11:48:37] <jleyva@jabber.org> Jerome, about the mobile auth issue [11:49:01] <mouneyrac> yes ( MDL-35998 ) [11:49:01] <moodlebot> http://tracker.moodle.org/browse/MDL-35998 New mobile menu: with two options CSS and Alternate auth url + existing enable mobile web services - Assignee: Jerome Mouneyrac Type: Sub-task Status: Waiting for peer review V:0 W:1 Priority: Blocker [11:49:21] <jleyva@jabber.org> I'm not very sure but I think that the only auth plugin that requires additional interaction to the user (it gets redirected to a protected directory) to work is the Shibboleth one [11:49:50] <jleyva@jabber.org> so, the only auth system that will not work properly for the mobile app will be then Shibboleth [11:50:36] <jleyva@jabber.org> do you know how Shibboleth auth works in Moodle? [11:50:49] <mouneyrac> Juan: I know nothing about shibbolteh. Is that widely spread? Why does it require an external login? [11:51:16] <jleyva@jabber.org> is the way it was implemented, the user is redirected automatically or using a link to a protected directory [11:51:27] *** Dan Marsden (dan@catalyst.net.nz/Office) has left the room [11:51:36] <jleyva@jabber.org> the protected directory has, usally, the Shibolleth auth Apache's module [11:52:07] <jleyva@jabber.org> so the user is prompted to enter username and password, and the server sets a cookie or server variable [11:52:42] <jleyva@jabber.org> that's the reason it's not going to work from a mobile app [11:52:56] *** Sam Hemelryk (samhemelryk@moodle.org/Sam’s MacBook Pro) has joined the room as a participant [11:53:12] <mouneyrac> that seems weird [11:53:24] <mouneyrac> but is the username know by Moodle? [11:54:01] *** Sam Hemelryk (samhemelryk@moodle.org/Sam’s MacBook Pro) has left the room [11:54:02] <mouneyrac> what we can do if that we detect auth method == shibolleth and do the curl call in this case [11:54:05] <jleyva@jabber.org> the username and other data is stored in SERVER variables [11:54:14] <jleyva@jabber.org> I like a lot your idea of extending auth plugins, but I'm not sure if we can do a fake login to the protected directory using a curl call [11:54:15] <mouneyrac> for the other auth plugin it will work [11:54:42] <jleyva@jabber.org> yes [11:55:05] <jleyva@jabber.org> but the point here is, are you going to add a condition to login/token or extend auth plugins for the shibolleth unique case? [11:55:05] <mouneyrac> ok it seems reasonable [11:55:26] <jleyva@jabber.org> +1 for extending auth plugins, because it will help custom auth plugins to handle the mobile auth [11:55:33] <mouneyrac> we need to extend the auth for supporting mutlple auth introduce by oauth2 plugin [11:56:35] *** ankit (ankit@moodle.org/5519828981350439913425172) has left the room [11:57:44] *** Cindereloy (stronk7@contiento.com/@ unlockingme) has left the room [11:59:02] *** tsala (tsala@moodle.org/developers) has left the room [11:59:03] <jleyva@jabber.org> I'm reading about some guys that has implemented curl calls against shibboleths directories, it seems to be possible [11:59:14] *** Dan P (poltawski@moodle.org/Adium Macbook Air) has left the room [11:59:23] <jleyva@jabber.org> In fact, is a standard Apache's protected directory [12:00:03] <mouneyrac> ls [12:00:12] <mouneyrac> sorry [12:00:39] <jleyva@jabber.org> some code here http://shibboleth.1660669.n2.nabble.com/php-class-to-curl-shib-protected-site-td2439110.html , easy to implement apparently [12:01:23] <jleyva@jabber.org> so Jerome, then are you going add the extra function to auth plugins? and also implement the shibboleth one? The last can be done for a next minor release I suppose [12:01:46] <jleyva@jabber.org> There are some guys interested that may help [12:01:59] <jleyva@jabber.org> see MOBILE-153 [12:01:59] <moodlebot> http://tracker.moodle.org/browse/MOBILE-153 Move UMM to MM - Assignee: Juan Leyva Type: New Feature Status: Open Priority: Blocker [12:02:03] <jleyva@jabber.org> (last comments) [12:02:18] <mouneyrac> yes I'll have to add the auth method for MDL-34426 [12:02:18] <moodlebot> http://tracker.moodle.org/browse/MDL-34426 Port Google/Facebook Oauth2 plugin into core - Assignee: Jerome Mouneyrac Type: Task Status: Waiting for integration review V:1 W:4 Priority: Major [12:03:57] <mouneyrac> it would be good if the app support oauth2 authentication with google facebook too [12:05:04] <mouneyrac> the idea is to request the Google access token from the html5, then send it to token.php that send a ws token back after verifying the access token with Google (throught the new auth method)
          Hide
          Jérôme Mouneyrac added a comment - - edited

          In order to lose no time, I'm going to push this to integration without the alternate auth url so we got it. I'll create a different issue for alternate auth url.

          Update: issue created => MDL-36114

          Show
          Jérôme Mouneyrac added a comment - - edited In order to lose no time, I'm going to push this to integration without the alternate auth url so we got it. I'll create a different issue for alternate auth url. Update: issue created => MDL-36114
          Hide
          Frédéric Massart added a comment -

          admin/settings/mobile.php

          • Line 28, the comment should start with a capital and end with a period.

          lib/admin.php

          • Line 6753: As you're introducing this new variable, you could probably add the PHP Docs there.
          • Line 6759: We should probably not rename a function, but I guess as it is private it should not create any issue.
          • Lines 6762, 6772, 6773: Coding style issues
          • Line 6781: Wouldn't it be safer to use OR instead of AND there? In a certain way, we do not need both for the WS to work

          General

          I am not convinced about the place where the new setting page is in the navigation. It looks like the CSS url in for the appearance only. And the other setting is related to the Web services. If mobile will grow with more settings then it's probably worth to set it somewhere else, or in a less important place. (Just mentioning my thoughts)

          Also I wonder if it wouldn't be better to have to params to enable separately XMLRPC or REST. Instead of enabling/disabling them both.

          Apart from those minor points, seems all good.

          Show
          Frédéric Massart added a comment - admin/settings/mobile.php Line 28, the comment should start with a capital and end with a period. lib/admin.php Line 6753: As you're introducing this new variable, you could probably add the PHP Docs there. Line 6759: We should probably not rename a function, but I guess as it is private it should not create any issue. Lines 6762, 6772, 6773: Coding style issues Line 6781: Wouldn't it be safer to use OR instead of AND there? In a certain way, we do not need both for the WS to work General I am not convinced about the place where the new setting page is in the navigation. It looks like the CSS url in for the appearance only. And the other setting is related to the Web services. If mobile will grow with more settings then it's probably worth to set it somewhere else, or in a less important place. (Just mentioning my thoughts) Also I wonder if it wouldn't be better to have to params to enable separately XMLRPC or REST. Instead of enabling/disabling them both. Apart from those minor points, seems all good.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          For REST/XMLRPC: Fred remark is good (less opened entry points), but I decided to privilege the admin user experience (one click without choice).

          After thinking about the Mobile menu, I may have been extreme with the mobile menu on root level. I thought it would be a good promotion to show the mobile menu and present it as a major feature. I will move the mobile settings into Admin > Server. Even thought it's very few settings in this menu, and even thought they concern different things (appeareance, web services, login ...) I prefer to group them in a same menu.

          Thanks for your review Fred. I'll fix all that.

          Show
          Jérôme Mouneyrac added a comment - - edited For REST/XMLRPC: Fred remark is good (less opened entry points), but I decided to privilege the admin user experience (one click without choice). After thinking about the Mobile menu, I may have been extreme with the mobile menu on root level. I thought it would be a good promotion to show the mobile menu and present it as a major feature. I will move the mobile settings into Admin > Server. Even thought it's very few settings in this menu, and even thought they concern different things (appeareance, web services, login ...) I prefer to group them in a same menu. Thanks for your review Fred. I'll fix all that.
          Hide
          Aparup Banerjee added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Hi Jerome,

          I've just been looking at this now.
          The code itself is good, the only thing that I just keep coming back to is the location of this new mobile settings page.
          I'll ping Martin when I see him come online today and get his thoughts, he's usually very onto it when placing these items.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Jerome, I've just been looking at this now. The code itself is good, the only thing that I just keep coming back to is the location of this new mobile settings page. I'll ping Martin when I see him come online today and get his thoughts, he's usually very onto it when placing these items. Many thanks Sam
          Hide
          Sam Hemelryk added a comment -

          So I remember the language strings for the new setting need review. There was a z in there.

          Show
          Sam Hemelryk added a comment - So I remember the language strings for the new setting need review. There was a z in there.
          Hide
          Martin Dougiamas added a comment -

          Yeah. Just had another look at the menus.

          I do think we need to consolidate all the Mobile stuff together at some point in the future (Search for "mobile" in the admin block and look at the confusion that results).

          However for now, I think it would be best to

          a) Keep the switch on the External services page as it was (there are links to it from overview etc which are going to be a pain to change) and
          b) Move the new Mobile page (as is) to be a new one under Web Services.

          This does mean we have the "Enable" switch twice but that's OK. At least people can find things.

          In future we really do need to tidy the whole thing up a lot.

          Show
          Martin Dougiamas added a comment - Yeah. Just had another look at the menus. I do think we need to consolidate all the Mobile stuff together at some point in the future (Search for "mobile" in the admin block and look at the confusion that results). However for now, I think it would be best to a) Keep the switch on the External services page as it was (there are links to it from overview etc which are going to be a pain to change) and b) Move the new Mobile page (as is) to be a new one under Web Services. This does mean we have the "Enable" switch twice but that's OK. At least people can find things. In future we really do need to tidy the whole thing up a lot.
          Hide
          Jérôme Mouneyrac added a comment -

          done

          Show
          Jérôme Mouneyrac added a comment - done
          Hide
          Juan Leyva added a comment -

          Hi Jerome,

          the current moodle_webservice_get_site_info doesn't return the site default language

          I think that we need this information returned because we can't rely only in user's language

          Regards

          Show
          Juan Leyva added a comment - Hi Jerome, the current moodle_webservice_get_site_info doesn't return the site default language I think that we need this information returned because we can't rely only in user's language Regards
          Hide
          Jérôme Mouneyrac added a comment -

          Hi Juan,
          a web service token is linked to a user, why do you need to know the site language? I don't mind to send the site language but I don't see why Moodle mobile app needs to know about the site language.

          Show
          Jérôme Mouneyrac added a comment - Hi Juan, a web service token is linked to a user, why do you need to know the site language? I don't mind to send the site language but I don't see why Moodle mobile app needs to know about the site language.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          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
          Jason Fowler added a comment -

          Unit test passed

          Show
          Jason Fowler added a comment - Unit test passed
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!
          Show
          Jérôme Mouneyrac added a comment - Doc updated http://docs.moodle.org/24/en/Mobile_web_services

            People

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

              Dates

              • Created:
                Updated:
                Resolved: