Edgewall Software
Modify

Ticket #948 (new enhancement)

Opened 7 years ago

Last modified 8 months ago

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

Reported by: cboos@… Owned by: athomas
Priority: normal Milestone: next-major-0.1X
Component: general Version: devel
Severity: normal Keywords: attachment permission
Cc: dserodio@…, mbertier@…, rsalveti@…, vnbang2003@…, luke-trac@…, andrex@…
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

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

Download all attachments as: .zip

Change History

Changed 7 years ago by cboos@…

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

comment:1 Changed 7 years ago by cboos@…

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

comment:2 Changed 7 years ago by jonas

  • Milestone set to 0.9

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

comment:3 Changed 7 years ago by anonymous

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

comment:4 Changed 7 years ago by cboos

  • Owner changed from jonas to cboos
  • Status changed from new to assigned

comment:5 Changed 7 years ago by vittorio

  • Severity changed from normal to enhancement

comment:6 Changed 7 years ago by anonymous

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 Changed 7 years ago by cboos

  • Resolution set to fixed
  • Status changed from assigned to closed

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 Changed 6 years ago by ziga

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 Changed 6 years ago by dserodio@…

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

comment:10 Changed 6 years ago by dserodio@…

  • Cc dserodio@… added

comment:11 Changed 6 years ago by cboos

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

comment:12 Changed 6 years ago by dserodio@…

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 Changed 6 years ago by Max

  • Resolution fixed deleted
  • Status changed from closed to reopened

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?

comment:14 Changed 6 years ago by mbertier@…

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

comment:15 Changed 6 years ago by anonymous

  • Cc mbertier@… added

comment:16 Changed 6 years ago by anonymous

  • Cc rsalveti@… added

comment:17 Changed 5 years ago by sid <sid.wiesner@…>

#2384 was marked as a duplicate of this ticket.

comment:18 Changed 5 years ago by sid <sid.wiesner@…>

  • Milestone 0.9 deleted

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 Changed 5 years ago by cboos

  • Keywords permission added
  • Milestone set to 0.12

Related to the PermissionPolicy topic.

comment:20 Changed 5 years ago by Noah Kantrowitz (coderanger) <coderanger@…>

You can also check out the SelfDelete plugin.

comment:21 Changed 5 years ago by athomas

  • Milestone changed from 0.12 to 0.11
  • Owner changed from cboos to athomas
  • Status changed from reopened to new

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 Changed 5 years ago by anonymous

  • Cc vnbang2003@… added

comment:23 Changed 4 years ago by luke-trac@…

  • Cc luke-trac@… added

comment:24 Changed 3 years ago by cboos

  • Milestone changed from 0.11-retriage to 0.13

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

comment:25 Changed 17 months ago by Andrew Schulman <andrex@…>

  • Cc andrex@… added

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

comment:26 Changed 12 months ago by ivoras@…

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 Changed 8 months ago by anonymous

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.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as new
as The resolution will be set. Next status will be 'closed'
to The owner will be changed from athomas. Next status will be 'new'
The owner will be changed from athomas to anonymous. Next status will be 'assigned'
Author


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

 
Note: See TracTickets for help on using tickets.