Opened 6 years ago

#9276 new enhancement

Reported by: Owned by: David.Byrne@… Franz normal AttachmentPolicyPlugin normal 0.12

Description

I could use something like this if it was extended further. I need to allow some users the ability to attach files to milestones without being able to modify milestones. I was wondering if this could be enhanced to match the permissions map. This would create 9 total permissions - TICKET_ATTACHMENT_CREATE, WIKI_ATTACHMENT_CREATE, MILESTONE_ATTACHMENT_CREATE, TICKET_ATTACHMENT_VIEW, WIKI_ATTACHMENT_VIEW, MILESTONE_ATTACHMENT_VIEW, TICKET_ATTACHMENT_DELETE, WIKI_ATTACHMENT_DELETE, MILESTONE_ATTACHMENT_DELETE. The view ones are probably not as important since they map to being able to view the associated entity. Your plugin already handles the TICKET_ATTACHMENT_DELETE. My need is on the MILESTONE_ATTACHMENT_CREATE. If this is something that you cannot get to shortly, please let me know. I may be able to give you a patch file for handling this.

comment:1 in reply to:  description Changed 6 years ago by Franz

Well, most of the mentioned permissions are already in some other permissions included:

• TICKET_ATTACHMENT_CREATE: TICKET_APPEND
• MILESTONE_ATTACHMENT_CREATE: MILESTONE_MODIFY

and some more, see TracPermissions for all permissions for trac

The AttachmentPolicyPlugin works also for attachments of milestones and wiki-pages - so it implies these permissions: TICKET_ATTACHMENT_DELETE, WIKI_ATTACHMENT_DELETE, MILESTONE_ATTACHMENT_DELETE

Last edited 3 months ago by Ryan J Ollos (previous) (diff)

comment:2 follow-up:  3 Changed 6 years ago by David.Byrne@…

I understand, but you're missing my point. I need to allow certain users to attach files to a milestone without being able to modify the milestone. This is similar to the reason that you created this plugin to begin with. You needed to allow users to delete ticket attachments without giving them TICKET_ADMIN. I've seen the mapping in the LegacyAttachmentPolicy. Also, it appears to me that you specifically reference TICKET_ATTACHMENT_DELETE in your code, so I'm not sure how it already handles MILESTONE_ATTACHMENT_DELETE or WIKI_ATTACHMENT_DELETE permissions. As I said prior, if you cannot get to this, I will just make the modifications locally. I just hate to see multiple plugins on trac-hacks that do pretty much the same thing.

comment:3 in reply to:  2 Changed 6 years ago by Franz

I understand, but you're missing my point.

I understand your point and it shouldn't be very difficult, at least for all *_DELETE permissions, since this is just a copy of the existing code. It is a bit of work though. Please note that if you extend the plugin there should be an ATTACHMENT_ALL_DELETE permission, which includes all *_DELETE permissions.

Also, it appears to me that you specifically reference TICKET_ATTACHMENT_DELETE in your code, so I'm not sure how it already handles MILESTONE_ATTACHMENT_DELETE or WIKI_ATTACHMENT_DELETE permissions.

I haven't known until yesterday that I'm able to attach documents to milestones, so I tried to attach a file and delete it with a different user and it worked with the AttachmentPolicyPlugin. But don't ask me for internas - maybe Trac uses just TICKET_ATTACHMENT_DELETE for every attachment action.

comment:4 Changed 6 years ago by anonymous

Actually, the reason it somewhat works is because you are not checking the parent node. The attachment is being passed in and your code is not looking to see what the attachment is attached to. I looked at the LegacyAttachment code and there is a parent check via resource.parent.realm. If the attachment is attached to a ticket, this value will be 'ticket'. If attached to a wiki page, the value will be 'wiki'. The actual permission that Trac is looking for is 'ATTACHMENT_DELETE' in every delete case.

comment:5 follow-up:  6 Changed 4 years ago by David.Byrne@…

Priority: high → normal

I was the one to originally create this ticket (my email address has changed). I implemented this functionality. If you would like, I can attach the modified code to this ticket. I implemented all of the above (except for the view permissions) and added ATTACHMENT_ADMIN that would allow CREATE or DELETE for all of the types. I've also upgraded to Trac 1.0.1 recently.

Changed 4 years ago by David.Byrne@…

modified attachment_policy.py

comment:6 in reply to:  5 Changed 4 years ago by Franz

I was the one to originally create this ticket (my email address has changed). I implemented this functionality. If you would like, I can attach the modified code to this ticket. I implemented all of the above (except for the view permissions) and added ATTACHMENT_ADMIN that would allow CREATE or DELETE for all of the types. I've also upgraded to Trac 1.0.1 recently.

Yes, that would be great. You could attach the patch to this ticket as described in SubmittingPatches on Trac. Thus I could merge it to the existing source code and test your it, before submitting it to existing plugin.

Thank you.

Changed 4 years ago by David.Byrne@…

unified diff of change

comment:7 follow-up:  8 Changed 4 years ago by David.Byrne@…

I've attached a unified diff of the changes, but I'm not sure if it is set up right. I have your original file in my packages directory along with my modified file. I did the diff of the two files, but I'm not sure if it is usable in that state or not. Please let me know if this works. If it doesn't, I will see what I can do to get you the patch in a usable format.

comment:8 in reply to:  7 ; follow-up:  9 Changed 4 years ago by Franz

I've attached a unified diff of the changes, but I'm not sure if it is set up right. I have your original file in my packages directory along with my modified file. I did the diff of the two files, but I'm not sure if it is usable in that state or not. Please let me know if this works. If it doesn't, I will see what I can do to get you the patch in a usable format.

Sorry, I have overlooked your attached file attachment_policy.py. Your patch cannot be displayed by Trac-Hacks (don#T know why), but when downloading in raw format it looks good. Thank you for your work - at a first sight it looks well implemented.

However, I am not convinced about that solution, because it needs so many permissions. On the other hand it's the most flexible way of setting user rights.

For everybody who loves / needs David's solution, can download attachment_policy.py and replace it in source code and rebuild the plugin.

@David, if you like you can also attach an egg to this ticket so users can install the plugin easier. If you do so, please use a different version (e.g. 0.1.1) in setup.py and consider adding your name in the license clause (for license, see ticket #11085).

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:9 in reply to:  8 Changed 4 years ago by Ryan J Ollos

Your patch cannot be displayed by Trac-Hacks (don#T know why), ...

It might be captured in #6840, which I'm not sure of the cause (real fix is #10193). Anyway, it seems to be fairly common for the preview to fail on t-h.o.

Modify Ticket

Change Properties