Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#11067 closed defect (fixed)

Mainnav Wiki and metanav Help/Guide visibility should be determined by fine-grained permissions

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: wiki system Version: 1.0dev
Severity: normal Keywords: permissions authzpolicy
Cc:
Release Notes:

Take into account fine grained permission before adding mainnav Wiki and metanav Help/Guide links, using the WikiStart and TracGuide resources, respectively.

API Changes:

Description

The Wiki mainnav entry is hidden and the Help/Guide metanav entry is shown for an anonymous user when the following TracFineGrainedPermissions configuration is specified:

[wiki:WikiStart@*]
anonymous = WIKI_VIEW

[wiki:*]
* =

However, Wiki should be shown and Help/Guide should be hidden. This has been discussed on the mailing list.

Attachments (1)

t11067-r11682-1.patch (910 bytes ) - added by Ryan J Ollos <ryan.j.ollos@…> 6 years ago.
Patch against r11682 of the trunk.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

Attachment: t11067-r11682-1.patch added

Patch against r11682 of the trunk.

comment:1 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

Keywords: permissions authzpolicy added

t11067-r11682-1.patch is the first attempt to fix the issue. My reasoning is that the Wiki mainnav entry points to WikiStart, so we should be checking fine-grained access to WikiStart before showing the Wiki mainnav entry. The same for the Help/Guide metanav entry and TracGuide.

However, what I just said is not strictly true, because the user could be changing where the navigation entry points through a configuration such as the following:

[mainnav]
wiki.href = /wiki/TracSupport

Fixing the issue for various mainnav/metanav configurations will require more extensive changes and is complex. If the user specifies a wiki page in wiki.href (e.g. /wiki/TracSupport), then we can easily check for fine-grained access to that wiki page by accessing the mainnav configuration in wiki.web_ui.get_navigation_items, but it is possible that the user would point to another realm or somewhere else entirely. The only sensible thing to do is to parse wiki.href into a realm and id, construct a Resource object and perform the fine-grained permissions checks. I'd be happy to work up a patch for that, but it seems like a lot of work, so I'd like to get some feedback on what makes sense before going down the route.

I think the patch would look something like this (untested, more changes would be needed to handle URLs pointing outside of Trac such as those shown in TracNavigation#Notes):

def get_navigation_items(self, req):
    # returns None for label if item not enabled
    label, realm, id = self.get_mainnav_config('wiki')
    if label is not None:
        resource = Resource(realm, id)
        if 'WIKI_VIEW' in req.perm(resource):
            yield ('mainnav', realm,
                   tag.a(label, href=req.href(realm, id), accesskey=1))
    # returns None for label if item not enabled
    label, realm, id = self.get_mainnav_config('help')
    if label is not None:
        resource = Resource(realm, id)
        if 'WIKI_VIEW' in req.perm(resource):
            yield ('mainnav', realm,
                   tag.a(label, href=req.href(realm, id), accesskey=1))

If t11067-r11682-1.patch will be accepted and you'd like some functional tests then I'd be happy to add those.

(Apparently I confused the CC field with the Keywords field when creating the ticket, and don't have permissions to fix it now.)

comment:2 Changed 6 years ago by Remy Blank

Cc: authz permissions removed

comment:3 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

I spent some more time looking into the code and found that the Wiki and Help/Guide navigation entries are actually pretty unique in that they are the only navigation entries that, by default (without an overriding [mainnav] / [metanav] configuration), direct to a specific resource id. All of the other entries direct to a "parent" page that doesn't have a resource id. For example, initially I thought Admin would follow the same pattern, but a user will see the Admin entry if they have access to at least one admin panel. I suppose this has been fairly well tested already since users can have PERMISSION_GRANT and PERMISSION_REVOKE, enabling them to see only the Permissions admin panel. I found that fine-grained permission checks are not implemented for any of the admin panels, but that can be the subject of a different ticket (#11069).

I propose scheduling the fix in t11067-r11682-1.patch for 1.0.2, and the more extensive work of pulling parts of Chrome.prepare_request into get_navigation_items for next-dev-1.1.x, in a separate ticket (anticipating several iterations of patches).

comment:4 Changed 6 years ago by Christian Boos

Milestone: 1.0.2
Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Fine by me. Patch applied in r11697.

About the outline of code in comment:1, look into using get_resource_url

comment:5 Changed 6 years ago by Christian Boos

Owner: set to Ryan J Ollos <ryan.j.ollos@…>

comment:6 in reply to:  4 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

Replying to cboos:

About the outline of code in comment:1, look into using get_resource_url

Thanks, I will take a look while working on #11078.

comment:7 Changed 5 years ago by Ryan J Ollos

Owner: changed from Ryan J Ollos <ryan.j.ollos@…> to Ryan J Ollos
Reporter: changed from Ryan J Ollos <ryan.j.ollos@…> to Ryan J Ollos

comment:8 Changed 5 years ago by Ryan J Ollos

#7730 closed as a duplicate.

comment:9 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.