Modify

Opened 6 years ago

Last modified 8 months ago

#3891 new enhancement

TagSystem.get_tags(req, resource) should not need a full req object.

Reported by: dkgdkg Owned by: hasienda
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 (6)

comment:1 Changed 3 years ago by hasienda

  • Owner changed from athomas to hasienda
  • Type changed from defect to enhancement

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 3 years ago by dkgdkg

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 2 years ago by hasienda

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 8 months ago by hasienda

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 8 months ago by hasienda

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 8 months ago by hasienda

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.

Add Comment

Modify Ticket

Action
as new The owner will remain hasienda.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.