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

[Ryan J Ollos]

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.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11682 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

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.)

[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).

[Christian Boos]

Fine by me. Patch applied in r11697.

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

[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.

[Ryan J Ollos]

#7730 closed as a duplicate.