Opened 16 years ago
Closed 9 years ago
#3891 closed enhancement (fixed)
TagSystem.get_tags(req, resource) should not need a full req object.
Reported by: | Daniel Kahn Gillmor | Owned by: | Steffen Hoffmann |
---|---|---|---|
Priority: | normal | Component: | TagsPlugin |
Severity: | normal | Keywords: | permissions tags |
Cc: | Trac Release: | 0.11 |
Description
I'm trying to implement an IPermissionProvider on Trac 0.11 using TagsPlugin 0.6. My goal is to make it so that wiki resources given a simple tag (e.g. public
) are visible to anonymous users.
This means that i'd like to call TagSystem.get_tags()
from within check_permissions()
. but get_tags()
takes a req and a resource, and check_permissions()
doesn't have access to a req.
coderanger
suggested some stack hackery to find the req as a workaround. I did that, but even with that working, there is a recursion problem: the call to get_tags
itself checks permissions on the object to get a list of the tags. This leads to infinite regress.
It might make more sense if get_tags
took arguments of (perm, resource)
instead, and if perm
was optional. This way additional plugins could reason about the tags of a given resource without requiring that the user in question already have access to them.
Attachments (0)
Change History (7)
comment:1 Changed 13 years ago by
Owner: | changed from Alec Thomas to Steffen Hoffmann |
---|---|
Type: | defect → enhancement |
comment:2 Changed 13 years ago by
I'll certainly stick to trunk or tractags-0.7 -- this ticket was opened 3 years ago, which is why it focuses on 0.6.
comment:3 Changed 12 years ago by
Replying to dkgdkg:
I'm trying to implement an IPermissionProvider on Trac 0.11 using TagsPlugin 0.6. My goal is to make it so that wiki resources given a simple tag (e.g.
public
) are visible to anonymous users.
Eventually I noticed a conflict between your approach and current policy: Keywords/tags are meant to be part of the tagged resource, and they're disclosed only to users, that have at least *_VIEW permission on the resource itself.
Making tags part of your access control means, that you'll have quite different ideas and requirements for how things should work. Sure, we could grant unconditional tags access to an imagined ITagPermissionProvider
. But you'll have to make sure, that i.e. tagging is an authoritative operation then, and tags can only set by privileged users. This will likely cause issues. I.e. think of ticket keywords, that are commonly used as tags as well, but controlling their change permission must be synced with TICKET_* permissions too.
To summarize, access control is currently pretty far off from the standard tag use, and a working solution will require
- more thinking and adjustments outside current control of TagsPlugin or
- radical changes and extensions towards interfering with permission checks or taking-over control of permission check results inside the tagged realms entirely.
While I'm not at all against this feature request, I just want to point out the invasive nature of required changes, to enable such functionality without creating a bunch of related issues.
comment:5 Changed 10 years ago by
Notes on the current implementation:
As noticed by the reporter, the feature requires a new API method circumventing permission checks to prevent infinite recursion while retrieving resource tags for checking tagged permissions. For general support of this feature, the new ITagProvider
method needs to get adopted by all tag providers.
ToDo:
- add code for backwards-compatibility and robustness with tag providers missing
resource_tags
- deny tags that would allow permission escalation on tag change time
- probably even limit permissions available for tagging (i.e. by blacklisting *_ADMIN)
- propagate new feature to tag providers, that do not inherit its implementation from
tractags.api.DefaultTagProvider
comment:6 Changed 10 years ago by
Of course you'll not only need to enable TagPolicy
component, but add it to permission_policies
option in the [trac]
section of your trac.ini too, in order to let permission checks run through it.
While I care about permission checks I'll consider this as request for improvement. And please stick to
trunk
or upcomingtractags-0.7
after it's release, becausetractags-0.6
is almost obsolete right now.