Defect #35634: Attachments deletable even though issue edit not permitted - Redmine

Actions

Copy link

Defect #35634

closed

Attachments deletable even though issue edit not permitted

Added by D G over 3 years ago. Updated over 3 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Marius BĂLTEANU

Category:

Permissions and roles

Target version:

4.1.5

Start date:

Due date:

% Done:

Estimated time:

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

Files

Download all files

0002-Fixed-attachments-deletable-by-user-without-edit-per.patch (2.62 KB) 0002-Fixed-attachments-deletable-by-user-without-edit-per.patch		Marius BĂLTEANU, 2021-08-01 13:32
0001-Adds-tests-for-issue-attachment-edit-by-user-without.patch (2.07 KB) 0001-Adds-tests-for-issue-attachment-edit-by-user-without.patch		Marius BĂLTEANU, 2021-08-01 13:32

Actions

Copy link

Updated by D G over 3 years ago

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

Actions

Copy link

Updated by Mischa The Evil over 3 years 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.

Actions

Copy link