Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.1.4, 2.2
-
Component/s: Authentication, Forum
-
Database:Any
-
Testing Instructions:
-
Workaround:
-
Affected Branches:MOODLE_21_STABLE, MOODLE_22_STABLE
-
Fixed Branches:MOODLE_21_STABLE, MOODLE_22_STABLE
-
Pull from Repository:
-
Pull Master Branch:
MDL-30722_forum_unsubscribe -
Pull Master Diff URL:
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:
- Log out
- Log in as a guest
- try to unsubscribe from all forums (http://localhost/mod/forum/unsubscribeall.php)
you are not redirected to login page.
Gliffy Diagrams
Activity
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.
Added some testing instructions to figure out what the correct behaviour should be.
I've added a master branch for peer review. I fixed the set_context() problem at the same time.
The code looks fine. Tested the context issue that Michael mentioned and this fixes that as well.
Thanks Andrew.
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 
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
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.
Fair enough, integrating...
Integrated, thanks!
Sorry, wrong Jason. Fixing.
Worked fine
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
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 resultline 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()