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

Unsubscribe from all forums responding correctly to guests

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      Ensure your site has guest access and autologin guest enabled in your site settings.

      Make sure you are completely logged out
      Go to /mod/forum/unsubscribeall.php. You should be redirected to the login page.

      Login as guest.
      Go to localhost/moodle/dev/master/mod/forum/unsubscribeall.php
      You should be redirected to your site home page.

      Login in as a proper user.
      Go to localhost/moodle/dev/master/mod/forum/unsubscribeall.php
      The unsubscribe from all forums confirmation screen should be displayed with no warnings or errors.

      Show
      Ensure your site has guest access and autologin guest enabled in your site settings. Make sure you are completely logged out Go to /mod/forum/unsubscribeall.php. You should be redirected to the login page. Login as guest. Go to localhost/moodle/dev/master/mod/forum/unsubscribeall.php You should be redirected to your site home page. Login in as a proper user. Go to localhost/moodle/dev/master/mod/forum/unsubscribeall.php The unsubscribe from all forums confirmation screen should be displayed with no warnings or errors.
    • Workaround:
      Hide

      in /mod/forum/unsubscribeall.php
      replace:

      require_login();

      by

      require_login(NULL, false);

      Show
      in /mod/forum/unsubscribeall.php replace: require_login(); by require_login(NULL, false);
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-30722_forum_unsubscribe

      Description

      When to try to unsubscribe from all forums and you are not logged in then you should be redirected to login page, but guests are not.

      Reproduction steps:

      1. Log out
      2. Log in as a guest
      3. try to unsubscribe from all forums (http://localhost/mod/forum/unsubscribeall.php)

      you are not redirected to login page.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          In a quick test I encountered this related error.

          Coding problem: $PAGE->context was not set. You may have forgotten to call require_login() or $PAGE->set_context(). The page may not display correctly as a result
           
              line 345 of \lib\pagelib.php: call to debugging()
              line 617 of \lib\pagelib.php: call to moodle_page->magic_get_context()
              line 1216 of \lib\weblib.php: call to moodle_page->__get()
              line 920 of \lib\pagelib.php: call to format_string()
              line 42 of \mod\forum\unsubscribeall.php: call to moodle_page->set_title()

          Show
          salvetore Michael de Raadt added a comment - In a quick test I encountered this related error. Coding problem: $PAGE->context was not set. You may have forgotten to call require_login() or $PAGE->set_context(). The page may not display correctly as a result   line 345 of \lib\pagelib.php: call to debugging() line 617 of \lib\pagelib.php: call to moodle_page->magic_get_context() line 1216 of \lib\weblib.php: call to moodle_page->__get() line 920 of \lib\pagelib.php: call to format_string() line 42 of \mod\forum\unsubscribeall.php: call to moodle_page->set_title()
          Hide
          salvetore Michael de Raadt added a comment -

          I found that when I was not logged in at all it would send me to the login page.

          When I was logged in as a guest and I tried to get to the unsubscribe page, I was redirected to the site home page.

          When I was logged in and entered the URL directly the error I described above appeared.

          Show
          salvetore Michael de Raadt added a comment - I found that when I was not logged in at all it would send me to the login page. When I was logged in as a guest and I tried to get to the unsubscribe page, I was redirected to the site home page. When I was logged in and entered the URL directly the error I described above appeared.
          Hide
          andyjdavis Andrew Davis added a comment -

          Added some testing instructions to figure out what the correct behaviour should be.

          Show
          andyjdavis Andrew Davis added a comment - Added some testing instructions to figure out what the correct behaviour should be.
          Hide
          andyjdavis Andrew Davis added a comment -

          I've added a master branch for peer review. I fixed the set_context() problem at the same time.

          Show
          andyjdavis Andrew Davis added a comment - I've added a master branch for peer review. I fixed the set_context() problem at the same time.
          Hide
          abgreeve Adrian Greeve added a comment -

          The code looks fine. Tested the context issue that Michael mentioned and this fixes that as well.
          Thanks Andrew.

          Show
          abgreeve Adrian Greeve added a comment - The code looks fine. Tested the context issue that Michael mentioned and this fixes that as well. Thanks Andrew.
          Hide
          stronk7 Eloy Lafuente (stronk7) 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
          stronk7 Eloy Lafuente (stronk7) 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
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited

          WOW, but... that script deletes ALL forum subscriptions in the whole site (for the current user), no matter if:

          • the user is enrolled
          • the course is hidden
          • there are no controls of the 'mod/forum:managesubscriptions' capability
          • nor stuff like forum_get_subscribed_forums() is used at all.

          Also, I can imagine the logic beyond allowing to unsubscribe from email... but for the whole site? Why not only for the course? If we want to allow the user to unsubscribe from all, it should be some option into his profile pages/settings block. But not from email and for sure, not missing all those checks commented in the paragraph above.

          I'm going to keep this open, just in case you want to advance here and fix access not only for guests.

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited WOW, but... that script deletes ALL forum subscriptions in the whole site (for the current user), no matter if: the user is enrolled the course is hidden there are no controls of the 'mod/forum:managesubscriptions' capability nor stuff like forum_get_subscribed_forums() is used at all. Also, I can imagine the logic beyond allowing to unsubscribe from email... but for the whole site? Why not only for the course? If we want to allow the user to unsubscribe from all, it should be some option into his profile pages/settings block. But not from email and for sure, not missing all those checks commented in the paragraph above. I'm going to keep this open, just in case you want to advance here and fix access not only for guests. Ciao
          Hide
          andyjdavis Andrew Davis added a comment -

          Id rather fix any access control issues in a separate issue. I have raised MDL-31460 to do that. mod/forum/unsubscribeall.php responding correctly to guests is a nice contained issue. Making sense of what checks should be performed and determining whether that script should exist at all is vague enough that it could take some time to get right.

          Show
          andyjdavis Andrew Davis added a comment - Id rather fix any access control issues in a separate issue. I have raised MDL-31460 to do that. mod/forum/unsubscribeall.php responding correctly to guests is a nice contained issue. Making sense of what checks should be performed and determining whether that script should exist at all is vague enough that it could take some time to get right.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Fair enough, integrating...

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Fair enough, integrating...
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          moodle.com moodle.com added a comment -

          Sorry, wrong Jason. Fixing.

          Show
          moodle.com moodle.com added a comment - Sorry, wrong Jason. Fixing.
          Hide
          phalacee Jason Fowler added a comment -

          Worked fine

          Show
          phalacee Jason Fowler added a comment - Worked fine
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers.

          Many thanks & ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers. Many thanks & ciao

            People

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

              Dates

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