Edgewall Software
Modify

Opened 15 years ago

Closed 2 days ago

Last modified 36 hours ago

#948 closed enhancement (duplicate)

[patch] Add more control over attachments for the average user

Reported by: Christian Boos Owned by:
Priority: normal Milestone:
Component: general Version: devel
Severity: normal Keywords: patch attachment permission
Cc: dserodio@…, mbertier@…, rsalveti@…, vnbang2003@…, luke-trac@…, andrex@… Branch:
Release Notes:
API Changes:

Description

Currently, the average user can simply add attachments, and has no further control over the files that he has previously attached.

I would suggest that a user should be able to:

  • delete a file attached by himself (Note: this would imply that an anonymous user could delete files added by another anonymous user)
  • replace a file attached by himself by a newer version, simply by keeping a new Replace existing file checkbox checked, on the attachment form

Of course, the users with the TICKET_ADMIN and WIKI_DELETE would still be able to remove any attachment in the appropriate module.

See the attachment:attachment_del_replace.1077.diff for the suggested implementation.

In addition to the above changes, the patch also contains a small enhancement to the display of an attachment (as I have now the attachment information available, I show it in a way similar to the file revision information).

Attachments (1)

attachment_del_replace.1077.diff (10.3 KB ) - added by cboos@… 15 years ago.
Suggested implementation for #948 (diff to [1077])

Download all attachments as: .zip

Change History (38)

by cboos@…, 15 years ago

Suggested implementation for #948 (diff to [1077])

comment:1 by cboos@…, 15 years ago

Oops, there's a spurious print debug statement in the patch… If only I could replace that attachment with a fixed one :)

comment:2 by Jonas Borgström, 15 years ago

Milestone: 0.9

Looks interesting, no time to include it in Trac 0.8 though…

comment:3 by anonymous, 15 years ago

You might want to follow the example of Bugzilla here: attachments can obsolete other attachments, but they cannot be deleted.

comment:4 by Christian Boos, 15 years ago

Owner: changed from Jonas Borgström to Christian Boos
Status: newassigned

comment:5 by vittorio, 14 years ago

Severity: normalenhancement

comment:6 by anonymous, 14 years ago

I would suggest that the user be able to do what the trac project administrator determines that the user can do. This would probably mean that the trac program would need to implement a range of permissions for giving finer grained control of file attachment permissions to the trac administrator. For instance the trac administrator may determine that a user can delete all file attachments, or the user may just be able to delete his own file attachments. Point is that it should be up to the administrator to determine the permissions and not the program. I think that well thought out file attachment permissions will make the system much more robust. Just my opinion.

comment:7 by Christian Boos, 14 years ago

Resolution: fixed
Status: assignedclosed

The original idea was (re-)implemented in [1874], but only for the replace part. That way, attachments can't be deleted if the user has not this specific permission.

comment:8 by ziga, 14 years ago

Maybe it's just me, but I think this doesn't work. I still have multiple attacments listed when I check the checkbox! I replaced .cs and compiled .py files. What else is there to do?

comment:9 by dserodio@…, 14 years ago

Why is this fixed? AFAIK, there's still no way to delete an attachment, and I do have TRAC_ADMIN permission.

comment:10 by dserodio@…, 14 years ago

Cc: dserodio@… added

comment:11 by Christian Boos, 14 years ago

Don't you see the Delete Attachment button at the bottom of the attachment page?

comment:12 by dserodio@…, 14 years ago

Oh, now I see. I was looking for it on the Wiki page, besides the attachment, not in the attachment page itself.

Thanks. Now I went to the FAQ to add it and saw that it's already there.

/me hides in shame

comment:13 by Max , 14 years ago

Resolution: fixed
Status: closedreopened

I second the opinion of anonymous, to wit:

I would suggest that the user be able to do what the trac project administrator determines that the user can do. This would probably mean that the trac program would need to implement a range of permissions for giving finer grained control of file attachment permissions to the trac administrator. For instance the trac administrator may determine that a user can delete all file attachments, or the user may just be able to delete his own file attachments. Point is that it should be up to the administrator to determine the permissions and not the program. I think that well thought out file attachment permissions will make the system much more robust. Just my opinion.

I do not want to grant all of my dev users the WIKI_DELETE permission that would allow them to delete all attachments (and other wiki elements); it's too powerful. Nor do I want them coming to me or one of the other admins if they replace their attachment with a newer version and then want to get rid of the deprecated attachment from the attachment list. It is an occasional nuisance, true; but why should the development team be beholden to me for so trivial an alteration as removing an attachment that they themselves added, if I think them trustworthy enough not to sabotage their own wiki entries by deleting all their attachments in a fit of pique?

Could this not be solved by adding the appropriate code and adding the permission type WIKI_ATTACHMENT_DELETE?

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

comment:14 by mbertier@…, 13 years ago

I definitely agree with Max. We should also have a TICKET_DELETE_ATTACHMENT permission.

comment:15 by anonymous, 13 years ago

Cc: mbertier@… added

comment:16 by anonymous, 13 years ago

Cc: rsalveti@… added

comment:17 by sid <sid.wiesner@…>, 13 years ago

#2384 was marked as a duplicate of this ticket.

comment:18 by sid <sid.wiesner@…>, 13 years ago

Milestone: 0.9

Removing the 0.9 milestone — it has been closed and it doesn't make sense to have an open ticket in it still.

comment:19 by Christian Boos, 13 years ago

Keywords: permission added
Milestone: 0.12

Related to the PermissionPolicy topic.

comment:20 by Noah Kantrowitz (coderanger) <coderanger@…>, 13 years ago

You can also check out the SelfDelete plugin.

comment:21 by Alec Thomas, 12 years ago

Milestone: 0.120.11
Owner: changed from Christian Boos to Alec Thomas
Status: reopenednew

TracDev/SecurityBranch merged in r5514. The requirements from this ticket could now be implemented within the trac.perm.IPermissionPolicy interface: when a check for TICKET_ADMIN on a ticket attachment is passed to a policy implementation it could check the creator of the ticket is the same as the current user and deny/allow accordingly. Similarly for Wiki attachments and WIKI_DELETE.

A sample plugin implementing authz based access control is included as an example.

Ideally though, each "realm" of objects in Trac would define its own set of permissions and a policy would be able to act on that. eg. attachments might have CREATE, DELETE, REPLACE, each of which could be allowed/denied by the policy.

comment:22 by anonymous, 12 years ago

Cc: vnbang2003@… added

comment:23 by luke-trac@…, 11 years ago

Cc: luke-trac@… added

comment:24 by Christian Boos, 11 years ago

Milestone: 0.11-retriage0.13

I don't see happening that soon, but definitely needed.

comment:25 by Andrew Schulman <andrex@…>, 9 years ago

Cc: andrex@… added

It would also be useful if users could edit the file description.

comment:26 by ivoras@…, 9 years ago

How about versioning attachments (i.e. when an attachments is replaced)? It could be done simply either by adding version prefixes to old files (e.g. 01_myfile.doc, 02_myfile.doc…) or by creating directories with version name containing the old versions (e.g. 01/myfile.doc, 01/myfile.doc).

comment:27 by anonymous, 8 years ago

As we had the problem that different people are adding and replacing attachments, but cannot be given permission TICKET_ADMIN, I programmed a little plugin for this issue. See https://trac-hacks.org/wiki/AttachmentPolicyPlugin

IMHO it is very confusing that Trac says "[…] Replacing other's attachments requires ATTACHMENT_DELETE permission.", but there is no permission ATTACHMENT_DELETE in the admin-permission panel. I also thought that replacing any attachment is the default action when having permission TICKET_APPEND.

comment:28 by anonymous, 8 years ago

Why is this request still open after 7 years? even with code submitted to enable it? This is an open and honest question, without whine or sarcasm I really would like to understand what the blocking factors are.

To my, user-centric, way of thinking WikiPage#attachment is a special class of WikiPage>SubPage and could be handled the same way. Have ATTACH_CREATE,_MODIFY,_DELETE permissions, and then version the changes as per #comment:26.

comment:29 by Christian Boos, 8 years ago

As you can imagine, the issue is less trivial than it seems, mainly as the notion of "your own attachments" when you're not logged-in is not well defined. When you're authenticated, what we could do is a bit clearer, though in the coarse grain permission model we don't have a proper way to express a rule saying "you can do that on your own things". This would require a PermissionPolicy, but then not every resource has an owner, and for those who have, you don't have a standardized way to query the owner.

Writing a PermissionPolicy that implements this check for attachments only and for authenticated users would probably be trivial, but not that useful either, as authenticated users usually have more powerful permissions, like ATTACHMENT_DELETE, etc. I can imagine this might still be useful on sites with lots of authenticated users (TH:AccountManagerPlugin) but then I'm not really concerned personally.

And with that you hit the other reason why this hasn't been fixed in 7 years: Trac is a community based project, meaning that the areas where it progresses are those for which some community members feel motivated enough to provide a good solution, in terms of ideas, code, testing, and documentation.

I realize the investment in time and effort needed to provide a decent solution like the one I outlined above might be perceived as being overkill for the anticipated benefit of the feature or fix, but if you rather see this as a learning experience and a way to contribute your own piece to the edifice, then it might be well worth it ;-)

comment:30 by framay <franz.mayer@…>, 8 years ago

Well, I haven't read all the comments and requirements in this ticket, but AttachmentPolicyPlugin would provide some of the functionality.

comment:31 by michael@…, 7 years ago

Could we have a cut-down version of this feature in the short term? That is, to grant authenticated users the right to delete their own attachments by default?

I can see that the issue of people who are not logged on is more complex; if it's easy to implement, they might be permitted to delete attachments within an editing session.

Consider as a use case my current situation:

  • Modest Wiki skill.
  • Authenticated.
  • Tried to use an image, but PDF format is not permitted for images.
  • Using an image is a two stage process (upload, use) so preview doesn't help
  • Couldn't replace the PDF with a JPG and can't delete the PDF, so the page is a mess
  • Don't want to ask the bank's sys-admin team for admin rights …

comment:32 by Ryan J Ollos, 4 years ago

Owner: Alec Thomas removed

comment:33 by Ryan J Ollos, 4 years ago

Reporter: changed from cboos@… to Christian Boos

comment:34 by figaro, 4 years ago

Keywords: patch added

comment:35 by knavero, 2 days ago

Has there been any updates on this? I'm interested in this permission as well.

comment:36 by anonymous, 2 days ago

Milestone: next-major-releases
Resolution: duplicate
Status: newclosed

This was already implemented in #12719: The LegacyAttachmentPolicy allows authenticated users to delete their own attachments since Trac 1.3.2.

in reply to:  36 comment:37 by Ryan J Ollos, 36 hours ago

Replying to anonymous:

This was already implemented in #12719: The LegacyAttachmentPolicy allows authenticated users to delete their own attachments since Trac 1.3.2.

Thanks for noticing. I had already forgotten about the change.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from (none) to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.