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
    • Rank:
      33598

      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.

        Activity

        Hide
        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
        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
        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
        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
        Andrew Davis added a comment -

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

        Show
        Andrew Davis added a comment - Added some testing instructions to figure out what the correct behaviour should be.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Eloy Lafuente (stronk7) added a comment -

        Fair enough, integrating...

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

        Integrated, thanks!

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

        Sorry, wrong Jason. Fixing.

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

        Worked fine

        Show
        Jason Fowler added a comment - Worked fine
        Hide
        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
        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: