Modify

Opened 11 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  
    2828
    2929    def check_permission(self, perm, operation):
    3030        # Permission table for screenshot tags.
    31         permissions = {'view' : 'WIKI_VIEW', 'modify' : 'WIKI_ADMIN'}
     31        permissions = {'view' : 'SCREENSHOTS_VIEW',
     32                       'modify' : 'SCREENSHOTS_EDIT'}
    3233
    3334        # First check permissions in default provider then for screenshots.
    3435        return super(ScreenshotsTagProvider, self).check_permission(perm,

Attachments (0)

Change History (8)

comment:1 Changed 11 years ago by 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?

comment:2 Changed 11 years ago by Steffen Hoffmann

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

comment:3 Changed 11 years ago by 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.

comment:4 in reply to:  3 ; Changed 11 years ago by Steffen Hoffmann

Summary: Use plugin's own permissions for tagging actions instead unrelated WIKI_* permissionsUse 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 in reply to:  4 Changed 11 years ago by 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.

comment:6 Changed 11 years ago by Steffen Hoffmann

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

comment:7 in reply to:  3 Changed 11 years ago by 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.

comment:8 Changed 11 years ago by Steffen Hoffmann

Resolution: fixed
Status: newclosed

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Radek Bartoň.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.