Use plugin's own permissions for tagging actions instead of unrelated WIKI_* permissions

[Steffen Hoffmann]

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

@@ -28 +28 @@
 
     def check_permission(self, perm, operation):
         # Permission table for screenshot tags.
-        permissions = {'view' : 'WIKI_VIEW', 'modify' : 'WIKI_ADMIN'}
+        permissions = {'view' : 'SCREENSHOTS_VIEW',
+                       'modify' : 'SCREENSHOTS_EDIT'}
 
         # First check permissions in default provider then for screenshots.
         return super(ScreenshotsTagProvider, self).check_permission(perm,

[Steffen Hoffmann]

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?

[Steffen Hoffmann]

Additional note: import sets has been obsoleted by [4195], so I suggest to remove it.

[Ryan J Ollos]

hasienda: Blackhex has previously given me the okay to take over maintenance of this plugin, so please feel free to commit your patch.

[Steffen Hoffmann]

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?

[Ryan J Ollos]

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.

[Steffen Hoffmann]

#11191 has been closed as a duplicate of this ticket. I should really check issue more carefully myself, sorry.

[Steffen Hoffmann]

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.

After stumbling about it again I'll take care for it now.

[Steffen Hoffmann]

In 13303:

ScreenshotsPlugin: Use own actions instead of 'WIKI_*' for tagging action permission checks, closes #10940.

Fixing the issue for the two most current branches simultaneously.