From 813cdf2d8538c36fa69e2bc7897d4bc95cb5763f Mon Sep 17 00:00:00 2001
From: Holger Just <holger@plan.io>
Date: Thu, 30 Mar 2017 14:47:07 +0200
Subject: [PATCH] Deny edit/update/delete for anonymous user

---
 app/controllers/users_controller.rb      |  7 +++++--
 app/views/users/show.html.erb            |  2 +-
 test/functional/users_controller_test.rb | 20 ++++++++++++++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 60600a252..e3baa9b9d 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -20,7 +20,8 @@ class UsersController < ApplicationController
   self.main_menu = false
 
   before_action :require_admin, :except => :show
-  before_action :find_user, :only => [:show, :edit, :update, :destroy]
+  before_action ->{ find_user(false) }, :only => :show
+  before_action :find_user, :only => [:edit, :update, :destroy]
   accept_api_auth :index, :show, :create, :update, :destroy
 
   helper :sort
@@ -174,10 +175,12 @@ class UsersController < ApplicationController
 
   private
 
-  def find_user
+  def find_user(logged = true)
     if params[:id] == 'current'
       require_login || return
       @user = User.current
+    elsif logged
+      @user = User.logged.find(params[:id])
     else
       @user = User.find(params[:id])
     end
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 9bb5d6667..b44ed6a3f 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -1,5 +1,5 @@
 <div class="contextual">
-<%= link_to(l(:button_edit), edit_user_path(@user), :class => 'icon icon-edit') if User.current.admin? %>
+<%= link_to(l(:button_edit), edit_user_path(@user), :class => 'icon icon-edit') if User.current.admin? && @user.logged? %>
 </div>
 
 <h2><%= avatar @user, :size => "50" %> <%= @user.name %></h2>
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 0dbd12a81..49d69f84a 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -342,6 +342,12 @@ class UsersControllerTest < Redmine::ControllerTest
     assert_select 'a', :text => 'Activate'
   end
 
+  def test_edit_should_be_denied_for_anonymous
+    assert User.find(6).anonymous?
+    get :edit, :params => {:id => 6}
+    assert_response 404
+  end
+
   def test_update
     ActionMailer::Base.deliveries.clear
     put :update, :params => {
@@ -593,6 +599,12 @@ class UsersControllerTest < Redmine::ControllerTest
     assert_nil ActionMailer::Base.deliveries.last
   end
 
+  def test_update_should_be_denied_for_anonymous
+    assert User.find(6).anonymous?
+    put :update, :params => {:id => 6}
+    assert_response 404
+  end
+
   def test_destroy
     assert_difference 'User.count', -1 do
       delete :destroy, :params => {:id => 2}
@@ -610,6 +622,14 @@ class UsersControllerTest < Redmine::ControllerTest
     assert_response 403
   end
 
+  def test_destroy_should_be_denied_for_anonymous
+    assert User.find(6).anonymous?
+    assert_no_difference 'User.count' do
+      put :destroy, :params => {:id => 6}
+    end
+    assert_response 404
+  end
+
   def test_destroy_should_redirect_to_back_url_param
     assert_difference 'User.count', -1 do
       delete :destroy, :params => {:id => 2, :back_url => '/users?name=foo'}
-- 
2.12.0