diff --git a/app/controllers/dmails_controller.rb b/app/controllers/dmails_controller.rb index a421c7bb1..ca4455a8a 100644 --- a/app/controllers/dmails_controller.rb +++ b/app/controllers/dmails_controller.rb @@ -5,7 +5,7 @@ class DmailsController < ApplicationController def new if params[:respond_to_id] parent = Dmail.find(params[:respond_to_id]) - check_privilege(parent) + check_show_privilege(parent) @dmail = parent.build_response(:forward => params[:forward]) else @dmail = Dmail.new(dmail_params(:create)) @@ -21,7 +21,7 @@ class DmailsController < ApplicationController def show @dmail = Dmail.find(params[:id]) - check_privilege(@dmail) + check_show_privilege(@dmail) if request.format.html? && @dmail.owner == CurrentUser.user @dmail.update!(is_read: true) @@ -37,7 +37,7 @@ class DmailsController < ApplicationController def update @dmail = Dmail.find(params[:id]) - check_privilege(@dmail) + check_update_privilege(@dmail) @dmail.update(dmail_params(:update)) flash[:notice] = "Dmail updated" @@ -51,10 +51,12 @@ class DmailsController < ApplicationController private - def check_privilege(dmail) - if !dmail.visible_to?(CurrentUser.user, params[:key]) - raise User::PrivilegeError - end + def check_show_privilege(dmail) + raise User::PrivilegeError unless dmail.visible_to?(CurrentUser.user, params[:key]) + end + + def check_update_privilege(dmail) + raise User::PrivilegeError unless dmail.owner == CurrentUser.user end def dmail_params(context) diff --git a/test/functional/dmails_controller_test.rb b/test/functional/dmails_controller_test.rb index d3b756538..6e39189b4 100644 --- a/test/functional/dmails_controller_test.rb +++ b/test/functional/dmails_controller_test.rb @@ -143,6 +143,13 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest assert_equal(false, @dmail.reload.is_deleted) end + should "not allow updating if the dmail is not owned by the current user even with a dmail key" do + put_auth dmail_path(@dmail), @unrelated_user, params: { dmail: { is_deleted: true }, key: @dmail.key } + + assert_response 403 + assert_equal(false, @dmail.reload.is_deleted) + end + should "update user's unread_dmail_count when marking dmails as read or unread" do put_auth dmail_path(@dmail), @user, params: { dmail: { is_read: true } } assert_equal(true, @dmail.reload.is_read)