dmails: fix users being able to update other user's dmails.

Fix it being possible to mark dmails belonging to other users as read or
deleted. Anyone who had a permalink to a dmail could update the dmail.
This commit is contained in:
evazion
2020-02-02 23:36:05 -06:00
parent 73219f38ce
commit b8aa223ecb
2 changed files with 16 additions and 7 deletions

View File

@@ -5,7 +5,7 @@ class DmailsController < ApplicationController
def new def new
if params[:respond_to_id] if params[:respond_to_id]
parent = Dmail.find(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]) @dmail = parent.build_response(:forward => params[:forward])
else else
@dmail = Dmail.new(dmail_params(:create)) @dmail = Dmail.new(dmail_params(:create))
@@ -21,7 +21,7 @@ class DmailsController < ApplicationController
def show def show
@dmail = Dmail.find(params[:id]) @dmail = Dmail.find(params[:id])
check_privilege(@dmail) check_show_privilege(@dmail)
if request.format.html? && @dmail.owner == CurrentUser.user if request.format.html? && @dmail.owner == CurrentUser.user
@dmail.update!(is_read: true) @dmail.update!(is_read: true)
@@ -37,7 +37,7 @@ class DmailsController < ApplicationController
def update def update
@dmail = Dmail.find(params[:id]) @dmail = Dmail.find(params[:id])
check_privilege(@dmail) check_update_privilege(@dmail)
@dmail.update(dmail_params(:update)) @dmail.update(dmail_params(:update))
flash[:notice] = "Dmail updated" flash[:notice] = "Dmail updated"
@@ -51,10 +51,12 @@ class DmailsController < ApplicationController
private private
def check_privilege(dmail) def check_show_privilege(dmail)
if !dmail.visible_to?(CurrentUser.user, params[:key]) raise User::PrivilegeError unless dmail.visible_to?(CurrentUser.user, params[:key])
raise User::PrivilegeError end
end
def check_update_privilege(dmail)
raise User::PrivilegeError unless dmail.owner == CurrentUser.user
end end
def dmail_params(context) def dmail_params(context)

View File

@@ -143,6 +143,13 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest
assert_equal(false, @dmail.reload.is_deleted) assert_equal(false, @dmail.reload.is_deleted)
end 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 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 } } put_auth dmail_path(@dmail), @user, params: { dmail: { is_read: true } }
assert_equal(true, @dmail.reload.is_read) assert_equal(true, @dmail.reload.is_read)