Opened 12 years ago
Closed 11 years ago
#10940 closed defect (fixed)
Use plugin's own permissions for tagging actions instead of unrelated WIKI_* permissions
Reported by: | Steffen Hoffmann | Owned by: | Radek Bartoň |
---|---|---|---|
Priority: | normal | Component: | ScreenshotsPlugin |
Severity: | major | Keywords: | tag provider permission |
Cc: | Trac Release: | 0.12 |
Description
TracTags support is currently flawed in the following way:
If you only grant SCREENSHOTS_EDIT to 'authenticated', adding screenshots is broken. Relevant DEBUG log passage:
14:57:54,402 Trac[core] DEBUG: action: post-add 14:57:54,402 Trac[core] DEBUG: actions: ['post-add', 'view'] 14:57:54,403 Trac[core] DEBUG: input filename: ('20130310_acct_mgr-uid-change_dev.png',) 14:57:54,403 Trac[core] DEBUG: output filename: (u'20130310_acct_mgr-uid-change_dev.png',) 14:57:54,406 Trac[core] DEBUG: screenshot: {'height': 566, 'width': 1090, 'name': u'Test', 'file': u'20130310_acct_mgr-uid-change_dev.png', 'time': 1363442274, 'priority': 0, 'author': u'test3', 'tags': u'AccountManager', 'description': u'file for testing'} 14:57:54,406 Trac[api] DEBUG: INSERT INTO screenshot (height, width, name, file, time, priority, author, tags, description) VALUES (566, 1090, Test, 20130310_acct_mgr-uid-change_dev.png, 1363442274, 0, test3, AccountManager, file for testing) 14:57:54,408 Trac[api] DEBUG: SELECT id, name, description, time, author, tags, file, width, height, priority FROM screenshot WHERE time = (1363442274,) 14:57:54,409 Trac[api] DEBUG: SELECT component FROM screenshot_component WHERE screenshot = 2 14:57:54,410 Trac[api] DEBUG: SELECT version FROM screenshot_version WHERE screenshot = 2 14:57:54,410 Trac[core] DEBUG: path: /data/trac/sandbox_1.0-pg/conf/screenshots/2 14:57:54,412 Trac[core] DEBUG: filename: /data/trac/sandbox_1.0-pg/conf/screenshots/2/20130310_acct_mgr-uid-change_dev-1090x566.png 14:57:54,413 Trac[perm] DEBUG: No policy allowed test3 performing WIKI_ADMIN on <Resource u'screenshots:2'> 14:57:54,414 Trac[main] WARNING: [127.0.0.1] HTTPForbidden: 403 Forbidden (Unzureichende Berechtigungen zum Ausführen dieser Operation.)
Especially note the check for WIKI_ADMIN that looks misplaced.
After some research I found that
- disabling only tracscreenshots.tags.ScreenshotsTags component or
- the following mini-patch
fixes this.
-
.py
old new 28 28 29 29 def check_permission(self, perm, operation): 30 30 # Permission table for screenshot tags. 31 permissions = {'view' : 'WIKI_VIEW', 'modify' : 'WIKI_ADMIN'} 31 permissions = {'view' : 'SCREENSHOTS_VIEW', 32 'modify' : 'SCREENSHOTS_EDIT'} 32 33 33 34 # First check permissions in default provider then for screenshots. 34 35 return super(ScreenshotsTagProvider, self).check_permission(perm,
Attachments (0)
Change History (8)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
Additional note: import sets
has been obsoleted by [4195], so I suggest to remove it.
comment:3 follow-ups: 4 7 Changed 12 years ago by
comment:4 follow-up: 5 Changed 12 years ago by
Summary: | Use plugin's own permissions for tagging actions instead unrelated WIKI_* permissions → Use plugin's own permissions for tagging actions instead of unrelated WIKI_* permissions |
---|
Replying to rjollos:
hasienda: Blackhex has previously given me the okay to take over maintenance of this plugin, so please feel free to commit your patch.
Ah, didn't know that. My last contact with Blackhex was during initial i18n work for this plugin. But sure, I'll take care for it ASAP.
I don't know about your plans for doing formal releases here. So should I close the ticket on commit too?
comment:5 Changed 12 years ago by
Replying to hasienda:
I don't know about your plans for doing formal releases here. So should I close the ticket on commit too?
I don't think I'll have the bandwidth to take over maintenance of this one. I'll probably commit a fix here and there as tickets get raised, at least until we find a formal maintainer. You are welcome to take over maintenance, but I know you have a bit on your plate as well ;)
As for closing the ticket, I think that would be least confusing to incoming authors, but I'm fine with whatever you feel is best. Unfortunately, there are 0.11
and 0.12
branches for this plugin, so it's probably best to push your change to both (assuming it's applicable to both). In the past I have observed that Blackhex has done pretty significant rework between version-based branches, so I'm not sure how much the 0.11
and 0.12
branches differ.
comment:6 Changed 11 years ago by
#11191 has been closed as a duplicate of this ticket. I should really check issue more carefully myself, sorry.
The offending set of permissions originates from changeset [4195]. Wonder, why I didn't see this for years.
On second thought it might be good to make the change listener invocations a bit more robust in order to not disturb the whole action on failure. Would you welcome such a re-work?