IPermissionPolicy unable to grant WIKI_VIEW access

[dkg-debian.org@…]

I'm working on a simple plugin which provides an IPermissionPolicy interface. The goal is for it to grant WIKI_VIEW privileges for certain objects, but not for the wiki as a whole. The current code (0.11dev, as of r6060) doesn't seem to allow this functionality except for the start page — and then only when accessed as the root, not as /wiki/WikiStart.

Attempts to access /wiki/whatever (when the IPermissionPolicy object grants permission, but the user does not have generic WIKI_VIEW) fail with the error message:

No handler matched request to /wiki/whatever

The following patch seems correct to me, and seems to make things to work the way i expected them to (i.e. the IPermissionPolicy is now capable of granting WIKI_VIEW on specific resources and those resources are actually viewable, while others are not):

Index: trac/wiki/web_ui.py
===================================================================
--- trac/wiki/web_ui.py (revision 6060)
+++ trac/wiki/web_ui.py (working copy)
@@ -94,7 +94,7 @@

     def match_request(self, req):
         match = re.match(r'^/wiki(?:/(.*)|$)', req.path_info)
-        if 'WIKI_VIEW' in req.perm('wiki') and match:
+        if match:
             if match.group(1):
                 req.args['page'] = match.group(1)
             return 1

I believe this just removes a shortcut that's no longer relevant under the new permissions model. It does not appear to grant additional, unwarranted access privileges in the cases i've tested, though i'd welcome any corrections.

Thanks for trac!

[osimons <simon-code@…>]

That makes the permission policy work the other way around from what the current model implements. In the current model you cannot grant a permission through a security policy - through your plugin you can only confirm access, deny access or be ignorant of the request.

So, in your case you need to grant WIKI_VIEW access to the users/groups in question, and then use your policy to deny access to the various pages that should be restricted.

I do not see that there will be any move to remove the 'core' permissions for each module, even though they are enhanced by the new security model.

I am closing the ticket as 'worksforme' as your needs should be served by reversing strategy for your plugin policy. Please reopen if this is not the case.

[osimons <simon-code@…>]

Bummer - resolution wrong…

[osimons <simon-code@…>]

Trying to close again with correct resolution this time - promise.

[dkg-debian.org@…]

I understand your explanation, but the permission policy you're describing doesn't seem to jibe with the one described at TracDev/SecurityBranch. here's a couple reasons why i don't think it lines up:

• IPermissionPolicy objects are supposed to be able to return True, granting access to the requested action. Since viewing a wiki page should only require WIKI_VIEW for that page context (and not for the entire wiki), i don't see why i should have to grant context="wiki" WIKI_VIEW privileges to those users who should only have the privilege in the context of a couple pages.
• Users can already view the root page (/) without having a generic WIKI_VIEW privilege, if another IPermissionPolicy object grants me access to that context. Are you saying this a bug? Should this not be possible? They are actually viewing a wiki page (and using the wiki processor) which would normally be denied them.
• What if the administrator chooses to remove the DefaultPermissionPolicy from the list of permission_policies — in this case, nothing that chooses to authorize a narrow-context WIKI_VIEW check_permission will succeed in granting that access unless it is also willing to grant the wide-context (i.e. req.perm("wiki")) WIKI_VIEW check_permission. Given that the policy is default fail-secure (i.e. return False if no permission policy grants access), it seems odd to ask replacement PermissionPolicy authors to grant wide-context WIKI_VIEW permissions to users who they do not want to view the whole wiki.

Or am i misunderstanding what the context denoted by req.perm("wiki") means here?

Would you consider it a bug in the authz_policy plugin if (starting from ​the default example), the following stanza in authz_file failed to let anonymous users read PublicPage?

[wiki:PublicPage@*]
* = VIEW

[Christian Boos]

Not granting WIKI_VIEW on 'wiki' (or on the toplevel for that matter) means you prevent any wiki page to be seen. So in your plugin, you could return True for the 'WIKI_VIEW' in req.perm("wiki") case, if the user has at least the permission to view some pages.

OTOH, a more specific check will anyway be made when trying to access the specific page, so the more global check on the 'wiki' realm is not really needed. As you noticed, this would also complicate things in the authz_policy plugin if we would handle this correctly.

Open point.

[dkg-debian.org@…]

I hadn't understood that about req.perm('wiki'). There doesn't seem to be anything similar in the codebase for any other trac componenents ("components" is probably the wrong term — what's the right term?). That is, there is no req.perm('ticket'), no req.perm('milestone') etc. All the other invocations of req.perm seem to pass in either a straight context, or they use the multiple-argument approach (i'm assuming that multi-argument call is of narrower scope than 'milestone', etc). Why is 'wiki' special?

From a more practical point of view: If the top-level check remains, how should an IPermissionPolicy author tell that req.perm was just passed 'wiki' from the perspective of check_permission()? Is the context argument just a string to test against with ==? Sorry for the newbie question. The few example policies i've looked at don't include tests like this.

[Christian Boos]

Your questioning is valid. Currently, the code (both old trunk and newer context-refactoring branch) do allow for testing the "realm" only, by creating a Context (in trunk) or a Resource (in context-refactoring) with the .realm set to a string but the .id set to None. This is equivalent to testing against req.perm(realm) (in both branches), and checking for this condition in the IPermissionPolicy component is simply done by verifying whether id is None or not.

Now, it's true that this flexibility is perhaps not needed at this point, given that the permissions names also always convey the realm information: we have WIKI_VIEW, TICKET_VIEW etc. but this allows for simplifying permission names like VIEW or CREATE permission in the future. So I think it's a good thing if IPermissionPolicy components have the correct understanding of the .id == None case.

Nevertheless, I now think it's OK to remove the single use case of req.perm("wiki").

[dkg-debian.org@…]

Thanks for the followup, cboos.

I'm not sure that the permission name containing the realm information is actually relevant: most !IPermissionPolicies will probably return None when WIKI_VIEW is requested and .realm == 'ticket', right?

cboos said:

I think it's a good thing if !IPermissionPolicy components have the correct understanding of the .id == None case.

Just to clarify what that correct understanding is, here's a stab at a precise definition (please correct any misconceptions!):

For a given request with a specific permission, if the .realm is set to a string, but .id == None, an implementation of !IPermissionPolicy should return:

• True if that policy itself could ever return True for a more-specific context (i.e. with the same .realm but with the .id actually set) with the same permission.
• False if that policy itself could never return True for a more-specific context with the same permission, but might sometimes return False for a more-specific context with the same permission.
• None if that policy itself will always return None for a more-specific context with the same permission.

Is that correct? There's a weird asymmetry between the True and False cases above that i'm not sure how to clean up properly.

[Christian Boos]

I'd formulate it a bit differently:

When checking a permission on a realm resource (i.e. when .id is None), the IPermissionPolicy will return:

• True if the permission is by default available for resources in that realm (the permission still could be denied for individual resources)
• False if the permission is by default not available for resources in that realm (there still could be granted for individual resources)
• None if no decision can be made without a more specific resource

This also matches the behavior of the default permission policy, which simply doesn't inspect the .id (or even the resource) at all.

[dkg-debian.org@…]

OK, this makes sense to me (though i'm not certain where in the trac codebase such a request would be useful). So for a minimalist IPermissionPolicy that's wants to narrowly affect the permissions settings, it would likely return None when .id is None, right?

Thanks for all the clarification.

[Christian Boos]

IPermissionPolicy documentation expanded in [6157].

[dkg-debian.org@…]

i don't think this is fixed, because it looks like the id-less check for match_request​ is still present.

Consider a minimal policy "PublicPolicy" which is supposed to only enable access to, say, wiki pages that are prefixed with the word "Public" (e.g. access is granted to wiki/PublicBlahBlah, but the policy is deliberately agnostic about wiki/BlahBlah).

PublicPolicy is designed to be layered into a stack, and perform only this enabling role, making no assumptions or declarations about other permissions. It should return True on Public* wiki pages, None on everything else, and None for when .id is None (because it can make no decision without a more specific resource).

As the match_request function stands, if no policy beneath Public_Policy grants WIKI_VIEW generically to the wiki realm, PublicPolicy will fail to grant access, because the wiki handler won't even match the request.

[Christian Boos]

Thanks for the use case, this clarifies things. That policy should obviously return True instead of None when checking for the realm.

I'll update the API doc accordingly and add a sample plugin.

[dkg-debian.org@…]

At the risk of beating a dead horse, it's not "obviously" clear to me that a policy that's trying to be minimal should return True during a .id == None permission test. What about a similar policy whose job is to deny public access to pages with a prefix of Private? What about a policy that is supposed to do both of these things, but nothing else?

Could you try re-stating your earlier definition of what such a permission test is supposed to mean in light of these use cases?

If every permission test was against a specific resource (i.e. had a non-None .id), this confusion wouldn't be an issue.

afaict, the WIKI_VIEW test​ is the only test that has .id == None in the trac codebase. I still don't understand what the benefit of it is.

[Christian Boos]

Please have a look at attachment:public-wiki-plugin.patch​.

To follow-up on your extended examples, a "Private…" wiki page policy whose job would be to only deny stuff should return None for the realm check (i.e. it's not its job to allow for anything, but it will eventually let some resources go through if other policies allow for them).

Again, the whole story about checking the realm can maybe be better understood once you realize that pre-0.11 pretty much every permission check was a realm permission check, with the realm name being part of the action itself.

We still have the realm name within the permission name at this point, that's why it seems at first a bit odd to ask for if 'WIKI_VIEW' in req.perm('wiki'), but this really should be understood as if 'view' in req.perm('wiki'), both being equivalent to the legacy check if 'WIKI_VIEW' in req.perm.

[Christian Boos]

Clean-up the IPermissionPlugin API docs and add a sample plugin

[dkg-debian.org@…]

Thanks for the clarification, cboos. I still find the id-less check confusing, because of the asymmetry between True and False responses, but i think i understand it now. FWIW, i'm not troubled at all by the idea that the permissions might change from WIKI_VIEW to plain ol' VIEW. That part makes a lot of sense to me: now that we've got a realm to test against, why bother having the realm encoded in the permission itself?

This seems like the key insight to me:

---

Note that when checking a permission on a realm resource (i.e. when .id is None), this usually corresponds to some preliminary check done before making a fine-grained check on some resource.

---

If this is the only reason to do this check, why have it at all? Why not defer or decline the preliminary check entirely, and rely solely on the fine-grained check? If there are other use cases for the .id-less check, what are they? Do they conform to the same semantics?

Thanks for the example policy, btw!

[osimons <simon-code@…>]

Replying to dkg-debian.org@fifthhorseman.net:

If this is the only reason to do this check, why have it at all? Why not defer or decline the preliminary check entirely, and rely solely on the fine-grained check? If there are other use cases for the .id-less check, what are they? Do they conform to the same semantics?

An easy-to-understand example is the permission check done to see if the Wiki menu should be displayed for the user. No page is accessed, but more like 'can this user view any wiki pages?'. Testing every resource in every module for fine-grained permissions on every page load will probably have a slight performance penalty…

[dkg-debian.org@…]

Replying to osimons <simon-code@bvnetwork.no>:

An easy-to-understand example is the permission check done to see if the Wiki menu should be displayed for the user. No page is accessed, but more like 'can this user view any wiki pages?'. Testing every resource in every module for fine-grained permissions on every page load will probably have a slight performance penalty…

Heh. Point taken. This use case clarifies why this call might be relevant, but it raises another question: if the .id-less realm check is used to determine whether to show the wiki nav item, but no policy actually grants access to wiki/ or wiki/WikiStart, then where does clicking on the Wiki link take the user? To an "access denied" page? Then why bother showing the nav item?

Wouldn't it make more sense for the Wiki menu nav item test the specific target of its link to know whether to render the nav item, instead of testing a realm-generic permission?

[Christian Boos]

Replying to dkg-debian.org@fifthhorseman.net:

… Then why bother showing the nav item?

Wouldn't it make more sense for the Wiki menu nav item test the specific target of its link to know whether to render the nav item, instead of testing a realm-generic permission?

Ideally yes, but as that target is overridable (see TracNavigation), that's not directly possible at the navigation contributor level. It's indeed a problem (but hey, a small one ;-) ).

[Christian Boos]

Applied attachment:public-wiki-plugin.patch​ in r6188.

[dkg-debian.org@…]

Replying to cboos:

Ideally yes, but as that target is overridable (see TracNavigation), that's not directly possible at the navigation contributor level. It's indeed a problem (but hey, a small one ;-) ).

Even though it's overridable, there must be some way for trac to determine what the target will be. Is there any reason why this determination couldn't be done at the time the permission test is made so that the permission test is done against a specific .id? Should i open this as a separate ticket?

Thanks for clarifying the documentation for the IPermissionPolicy. I confess i still find the .id-less permission tests non-intuitive (which is why i'm pushing for their removal — without them, trac 0.11 has one of the cleanest extensible permission frameworks i've seen). but it helps to have a good example and clear documentation.

[osimons]

Replying to dkg-debian.org@fifthhorseman.net:

Even though it's overridable, there must be some way for trac to determine what the target will be. Is there any reason why this determination couldn't be done at the time the permission test is made so that the permission test is done against a specific .id? Should i open this as a separate ticket?

Thanks for clarifying the documentation for the IPermissionPolicy. I confess i still find the .id-less permission tests non-intuitive (which is why i'm pushing for their removal — without them, trac 0.11 has one of the cleanest extensible permission frameworks i've seen). but it helps to have a good example and clear documentation.

Just to put this issue to rest - for now at least. Talked about wiki navigation above, and that was in fact a bad example as (ignoring details) there is in fact one resource to display when clicking the navigation. Not so for a lot of other items such as 'View Tickets' - how to test for it's display? Same with search - what should be the criteria for letting the user search wiki pages or tickets? Or tick off to see wiki pages in timeline?

I understand your interest in seeing it removed, but with current model I see no other option than to live with the fact that for some needs, permission checking on realm only must be possible. We should however make sure that any permission request that actually deals with a specific resource in any form, actually do use the full resource descriptor with id (and version).

[fermulator@…]

Hey everyone,

I've just finished reading through ticket #6680 & this ticket, #6211. I still do not see a solution to the following situation:

1. We have user "public_user". This user is not an anonymous user, but instead an authenticated user with a publicly known password.
2. public_user would like to only access wiki pages like: /wiki/Public/*. All other access (wiki, tickets, timeline, etc.) should be denied.

So far I have the following configuration: trac default permission model:

administrators = TRAC_ADMIN
anonymous = BROWSER_VIEW, CHANGESET_VIEW, FILE_VIEW, LOG_VIEW, REPORT_SQL_VIEW, REPORT_VIEW
users = MILESTONE_VIEW, ROADMAP_VIEW, SEARCH_VIEW, TICKET_CREATE, TICKET_MODIFY, TICKET_VIEW, TIMELINE_VIEW, WIKI_CREATE, WIKI_MODIFY, WIKI_VIEW
admin1 = administrators
admin2 = administrators
user1 = users
user2 = users
user3 = users
etc.

Note that public_user is not a member of the default permission model, effectively disallowing all access to this authenticated user.

Now, in authzpolicy.conf, we allow public_user access to only the required wiki portion:

[wiki:Public/*]
public_user = WIKI_VIEW

The following results occur:

• When public_user tries to browse to ​http://myproject/trac, we receive: WIKI_VIEW privileges are required to perform this operation on WikiStart
• When public_user tries to browse to ​http://myproject/trac/wiki/WikiStart, we receive: No handler matched request to /wiki/WikiStart
• When public_user tries to browse to ​http://myproject/trac/wiki/Public, we recieve: No handler matched request to /wiki/Public
• When public_user tries to browse to ​http://myproject/trac/wiki/Public/Docs, we receive: No handler matched request to /wiki/Public/Docs

From all of the comments and discussions about this issue, it appears to be the issue that we have no way of granting public_user access to the wiki realm without granting access to ALL wiki pages …?

How can we accomplish what I'm trying to do here?

[Christian Boos]

Will look at this again, together with #6644 and #6680.

[Christian Boos]

See also #7730.

[Christian Boos]

Removing keyword, as no patch addresses the reopening made in comment:23.

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Ryan J Ollos]

Milestone renamed