Modify

Opened 5 years ago

Closed 3 months ago

#9952 closed task (wontfix)

Remove permissions checking from _get_boxes in prefs.py

Reported by: Ryan J Ollos Owned by:
Priority: normal Component: AnnouncerPlugin
Severity: normal Keywords:
Cc: Robert Corsaro, Steffen Hoffmann Trac Release: 0.12

Description

Some background on how I arrived at opening this ticket: I'm using PrivateTicketsPlugin on my Trac instance to grant ticket viewing permissions to a subset of my users. I'd like those users to be part of a joinable group, which I'd subscribe them to at the time I create their accounts. On attempting to setup this up this evening, I ran into the issue that the Group Subscription panel was not viewable by the users that lacked TICKET_VIEW permission. To grant them permission, I had to make some local modifications to the source code. In the course of making those modifications, I ran into what seems to be an inconsistency in how the source code is organized.

There seems to be an inconsistency in how permissions checking is done in the source code. Permissions checking is done in subscribers.py, for example:

def get_announcement_preference_boxes(self, req):
    if req.perm.has_permission('WIKI_VIEW'):
        yield "general_wiki", _("General Wiki Announcements")

Therefore, I expected permissions checking for the Group Subscription panel to also be done in subscribers.py, but it is not:

def get_announcement_preference_boxes(self, req):
    if req.authname == "anonymous" and 'email' not in req.session:
        return
    if self.joinable_groups:
        yield "joinable_groups", _("Group Subscriptions")

What I found is that additional permissions checking is done in prefs.py:

def _get_boxes(self, req):
    for pr in self.preference_boxes:
        boxes = pr.get_announcement_preference_boxes(req)
        boxdata = {}
        if boxes:
            for boxname, boxlabel in boxes:
                if boxname == 'general_wiki' and not req.perm.has_permission('WIKI_VIEW'):
                continue
                if (boxname == 'legacy' or boxname == 'joinable_groups') and not req.perm.has_permission('TICKET_VIEW'):
                    continue
                yield ((boxname, boxlabel) +
                    pr.render_announcement_preference_box(req, boxname))

It looks like the following permissions check is redundant with a permissions check in subscribers.py:

if boxname == 'general_wiki' and not req.perm.has_permission('WIKI_VIEW'):
    continue

and for consistency this permissions check should be moved into subscribers.py:

if (boxname == 'legacy' or boxname == 'joinable_groups') and not req.perm.has_permission('TICKET_VIEW'):
    continue

Therefore, I'd recommend deleting the former and making the change suggested in the latter. I was hoping to get some feedback to see if I'm overlooking something before generating a patch.

Attachments (0)

Change History (4)

comment:1 Changed 5 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added
Owner: changed from Steffen Hoffmann to Ryan J Ollos
Status: newassigned

comment:2 Changed 4 years ago by Ryan J Ollos

Status: assignednew

comment:3 Changed 7 months ago by Ryan J Ollos

Owner: Ryan J Ollos deleted

comment:4 Changed 3 months ago by Ryan J Ollos

Resolution: wontfix
Status: newclosed

Please upgrade to Trac 1.2, which has integrated the core of AnnouncerPlugin. Please raise the issue on the trac:MailingList if you encounter the issue with Trac 1.2.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.