Modify

Opened 9 years ago

Closed 2 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 6 years ago by Steffen Hoffmann

Owner: changed from Alec Thomas to Steffen Hoffmann
Type: defectenhancement

While I care about permission checks I'll consider this as request for improvement. And please stick to trunk or upcoming tractags-0.7 after it's release, because tractags-0.6 is almost obsolete right now.

comment:2 Changed 6 years ago by Daniel Kahn Gillmor

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 in reply to:  description Changed 5 years ago by Steffen Hoffmann

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

  1. more thinking and adjustments outside current control of TagsPlugin or
  2. 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:4 Changed 4 years ago by Steffen Hoffmann

In 13851:

TagsPlugin: Use tags for fine-grained permissions on tagged resources, refs #3891.

So an idea for the days, when permission providers for Trac were born, finally
has come true.

comment:5 Changed 4 years ago by Steffen Hoffmann

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 4 years ago by Steffen Hoffmann

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.

comment:7 Changed 2 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

In 14945:

0.8: Tag 0.8 release

Fixes #1304, #1344, #3660, #3891, #9064, #9797, #11661, #11690, #11695, #11888, #11950, #11954, #11968, #12202, #12292, #12434, #12486.

Refs #12415.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
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.