Defect #35634

Attachments deletable even though issue edit not permitted

Added by D G about 1 month ago. Updated about 1 month ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:

0%

Category:Permissions and roles
Target version:4.1.5
Resolution:Fixed Affected version:4.2.1

Description

If role A has permission to edit issues in tracker X but not in tracker Y, members of this role can delete attachments in issues with tracker Y.

This commit fixes this for attachments_editable? r15476. Likewise this should be done in /app/models/issue.rb:

  # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_deletable?
  def attachments_deletable?(user=User.current)
    attributes_editable?(user)
  end

0002-Fixed-attachments-deletable-by-user-without-edit-per.patch Magnifier (2.62 KB) Marius BALTEANU, 2021-08-01 13:32

0001-Adds-tests-for-issue-attachment-edit-by-user-without.patch Magnifier (2.07 KB) Marius BALTEANU, 2021-08-01 13:32

Associated revisions

Revision 21141
Added by Marius BALTEANU about 1 month ago

Adds tests for issue attachment edit by user without edit issue permission on tracker (#35634).

Revision 21142
Added by Marius BALTEANU about 1 month ago

Fixed attachments deletable by user without edit issue permission on tracker (#35634).

Revision 21143
Added by Marius BALTEANU about 1 month ago

Merged r21141 to 4.2-stable (#35634).

Revision 21144
Added by Marius BALTEANU about 1 month ago

Merged r21142 to 4.2-stable (#35634).

Revision 21145
Added by Marius BALTEANU about 1 month ago

Merged r21141 to 4.1-stable (#35634).

Revision 21146
Added by Marius BALTEANU about 1 month ago

Merged r21142 to 4.1-stable (#35634).

Revision 21152
Added by Marius BALTEANU about 1 month ago

Fixed "Layout/IndentationWidth: Use 2 (not 4) spaces for indentation." introduced by r21141 (#35634).

Revision 21154
Added by Marius BALTEANU about 1 month ago

Merged r21152 to 4.2-stable (#35634).

Revision 21155
Added by Marius BALTEANU about 1 month ago

Merged r21152 to 4.1-stable (#35634).

History

#1 Updated by D G about 1 month ago

#24623 solves this issue but does a lot more...

#2 Updated by Mischa The Evil about 1 month ago

  • Status changed from New to Confirmed

Issue and given fix confirmed. Adding the given method to Issue fixes this by properly taking tracker permissions into account (i.e. :edit_issue permission needed for tracker Y to delete attachments attached to an issue with set tracker Y).

D G wrote:

#24623 solves this issue but does a lot more...

Indeed. It implements full CRUD-permissions for issue attachments. I think it would be nice to have it integrated into the core, however the provided patch as-is is pretty big and, sadly, is currently outdated and will likely break the existing test suite and comes without any test coverage for the new features itself.
I've had a quick look into the patch implementation though and it all seems properly implemented (albeit to obviously outdated checkouts). It shouldn't be all too hard to rebase it onto the current trunk. Updating the test suite for the patch and testing (for) and handling (of) any possible edge-cases will be most of the required work to get it ready for a decision on core integration.

#3 Updated by Marius BALTEANU about 1 month ago

  • Assignee set to Marius BALTEANU
  • Target version set to 4.1.5

#4 Updated by Marius BALTEANU about 1 month ago

I've added two patches:
  • first one adds tests for r15476
  • second one fixes this issue and add tests.

#6 Updated by Marius BALTEANU about 1 month ago

  • Status changed from Confirmed to Resolved

Patches committed.

#7 Updated by Marius BALTEANU about 1 month ago

  • Status changed from Resolved to Closed
  • Resolution set to Fixed

Also available in: Atom PDF