Require password re-entry for sensitive actions (#19851).

Patch by Jens Krämer.

git-svn-id: http://svn.redmine.org/redmine/trunk@14333 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/application_controller.rb

@@ -59,6 +59,8 @@ class ApplicationController < ActionController::Base
   include Redmine::MenuManager::MenuController
   helper Redmine::MenuManager::MenuHelper
 
+  include Redmine::SudoMode::Controller
+
   def session_expiration
     if session[:user_id]
       if session_expired? && !try_to_autologin

app/controllers/auth_sources_controller.rb

@@ -21,6 +21,7 @@ class AuthSourcesController < ApplicationController
 
   before_filter :require_admin
   before_filter :find_auth_source, :only => [:edit, :update, :test_connection, :destroy]
+  require_sudo_mode :update, :destroy
 
   def index
     @auth_source_pages, @auth_sources = paginate AuthSource, :per_page => 25

app/controllers/email_addresses_controller.rb

@@ -18,6 +18,7 @@
 class EmailAddressesController < ApplicationController
   before_filter :find_user, :require_admin_or_current_user
   before_filter :find_email_address, :only => [:update, :destroy]
+  require_sudo_mode :create, :update, :destroy
 
   def index
     @addresses = @user.email_addresses.order(:id).where(:is_default => false).to_a

app/controllers/groups_controller.rb

@@ -22,6 +22,8 @@ class GroupsController < ApplicationController
   before_filter :find_group, :except => [:index, :new, :create]
   accept_api_auth :index, :show, :create, :update, :destroy, :add_users, :remove_user
 
+  require_sudo_mode :add_users, :remove_user, :create, :update, :destroy, :edit_membership, :destroy_membership
+
   helper :custom_fields
   helper :principal_memberships
 

app/controllers/members_controller.rb

@@ -23,6 +23,8 @@ class MembersController < ApplicationController
   before_filter :authorize
   accept_api_auth :index, :show, :create, :update, :destroy
 
+  require_sudo_mode :create, :update, :destroy
+
   def index
     scope = @project.memberships.active
     @offset, @limit = api_offset_and_limit

app/controllers/my_controller.rb

@@ -20,6 +20,9 @@ class MyController < ApplicationController
   # let user change user's password when user has to
   skip_before_filter :check_password_change, :only => :password
 
+  require_sudo_mode :account, only: :post
+  require_sudo_mode :reset_rss_key, :reset_api_key, :show_api_key, :destroy
+
   helper :issues
   helper :users
   helper :custom_fields
@@ -123,6 +126,10 @@ class MyController < ApplicationController
     redirect_to my_account_path
   end
 
+  def show_api_key
+    @user = User.current
+  end
+
   # Create a new API key
   def reset_api_key
     if request.post?

app/controllers/projects_controller.rb

@@ -25,6 +25,7 @@ class ProjectsController < ApplicationController
   before_filter :require_admin, :only => [ :copy, :archive, :unarchive, :destroy ]
   accept_rss_auth :index
   accept_api_auth :index, :show, :create, :update, :destroy
+  require_sudo_mode :destroy
 
   after_filter :only => [:create, :edit, :update, :archive, :unarchive, :destroy] do |controller|
     if controller.request.post?

app/controllers/roles_controller.rb

@@ -23,6 +23,8 @@ class RolesController < ApplicationController
   before_filter :find_role, :only => [:show, :edit, :update, :destroy]
   accept_api_auth :index, :show
 
+  require_sudo_mode :create, :update, :destroy
+
   def index
     respond_to do |format|
       format.html {

app/controllers/settings_controller.rb

@@ -23,6 +23,8 @@ class SettingsController < ApplicationController
 
   before_filter :require_admin
 
+  require_sudo_mode :index, :edit, :plugin
+
   def index
     edit
     render :action => 'edit'

app/controllers/users_controller.rb

@@ -28,6 +28,8 @@ class UsersController < ApplicationController
   include CustomFieldsHelper
   helper :principal_memberships
 
+  require_sudo_mode :create, :update, :destroy
+
   def index
     sort_init 'login', 'asc'
     sort_update %w(login firstname lastname admin created_on last_login_on)

app/helpers/application_helper.rb

@@ -25,6 +25,7 @@ module ApplicationHelper
   include Redmine::I18n
   include GravatarHelper::PublicMethods
   include Redmine::Pagination::Helper
+  include Redmine::SudoMode::Helper
 
   extend Forwardable
   def_delegators :wiki_helper, :wikitoolbar_for, :heads_for_wiki_formatter

app/views/my/_sidebar.html.erb

@@ -21,8 +21,8 @@
 <% if Setting.rest_api_enabled? %>
 <h4><%= l(:label_api_access_key) %></h4>
 <div>
-  <%= link_to_function(l(:button_show), "$('#api-access-key').toggle();")%>
-  <pre id='api-access-key' class='autoscroll'><%= @user.api_key %></pre>
+  <%= link_to l(:button_show), {:action => 'show_api_key'}, :remote => true %>
+  <pre id='api-access-key' class='autoscroll'></pre>
 </div>
 <%= javascript_tag("$('#api-access-key').hide();") %>
 <p>

app/views/my/show_api_key.html.erb

@@ -0,0 +1,10 @@
+<h2><%= l :label_api_access_key %></h2>
+
+<div class="box">
+  <pre><%= @user.api_key %></pre>
+</div>
+
+<p><%= link_to l(:button_back), action: 'account' %></p>
+
+
+

app/views/my/show_api_key.js.erb

@@ -0,0 +1 @@
+$('#api-access-key').html('<%= escape_javascript @user.api_key %>').toggle();

app/views/sudo_mode/_new_modal.html.erb

@@ -0,0 +1,19 @@
+<h3 class="title"><%= l(:label_password_required) %></h3>
+<%= form_tag({}, remote: true) do %>
+
+  <%= hidden_field_tag '_method', request.request_method %>
+  <%= hash_to_hidden_fields @sudo_form.original_fields %>
+  <%= render_flash_messages %>
+  <div class="box tabular">
+    <p>
+      <label for="sudo_password"><%= l :field_password %><span class="required">*</span></label>
+      <%= password_field_tag :sudo_password, nil, size: 25 %>
+    </p>
+  </div>
+
+  <p class="buttons">
+    <%= submit_tag l(:button_confirm_password), onclick: "hideModal(this);"  %>
+    <%= submit_tag l(:button_cancel), name: nil, onclick: "hideModal(this);", type: 'button' %>
+  </p>
+<% end %>
+

app/views/sudo_mode/new.html.erb

@@ -0,0 +1,17 @@
+<h2><%= l :label_password_required %></h2>
+<%= form_tag({}, class: 'tabular') do %>
+
+  <%= hidden_field_tag '_method', request.request_method %>
+  <%= hash_to_hidden_fields @sudo_form.original_fields %>
+
+  <div class="box">
+    <p>
+      <label for="sudo_password"><%= l :field_password %><span class="required">*</span></label>
+      <%= password_field_tag :sudo_password, nil, size: 25 %>
+    </p>
+  </div>
+  <%= submit_tag l(:button_confirm_password) %>
+<% end %>
+<%= javascript_tag "$('#sudo_password').focus();" %>
+
+

app/views/sudo_mode/new.js.erb

@@ -0,0 +1,4 @@
+$('#ajax-modal').html('<%= escape_javascript render partial: 'sudo_mode/new_modal' %>');
+showModal('ajax-modal', '400px');
+$('#sudo_password').focus();
+

config/locales/de.yml

@@ -163,6 +163,7 @@ de:
   button_close: Schließen
   button_collapse_all: Alle einklappen
   button_configure: Konfigurieren
+  button_confirm_password: Kennwort bestätigen
   button_copy: Kopieren
   button_copy_and_follow: Kopieren und Ticket anzeigen
   button_create: Anlegen
@@ -670,6 +671,7 @@ de:
   label_overview: Übersicht
   label_parent_revision: Vorgänger
   label_password_lost: Kennwort vergessen
+  label_password_required: Bitte geben Sie Ihr Kennwort ein
   label_permissions: Berechtigungen
   label_permissions_report: Berechtigungsübersicht
   label_personalize_page: Diese Seite anpassen

config/locales/en.yml

@@ -554,6 +554,7 @@ en:
   label_register: Register
   label_login_with_open_id_option: or login with OpenID
   label_password_lost: Lost password
+  label_password_required: Confirm your password to continue
   label_home: Home
   label_my_page: My page
   label_my_account: My account
@@ -989,6 +990,7 @@ en:
   button_reset: Reset
   button_rename: Rename
   button_change_password: Change password
+  button_confirm_password: Confirm password
   button_copy: Copy
   button_copy_and_follow: Copy and follow
   button_annotate: Annotate

config/routes.rb

@@ -67,6 +67,7 @@ Rails.application.routes.draw do
   match 'my', :controller => 'my', :action => 'index', :via => :get # Redirects to my/page
   match 'my/reset_rss_key', :controller => 'my', :action => 'reset_rss_key', :via => :post
   match 'my/reset_api_key', :controller => 'my', :action => 'reset_api_key', :via => :post
+  match 'my/show_api_key', :controller => 'my', :action => 'show_api_key', :via => :get
   match 'my/password', :controller => 'my', :action => 'password', :via => [:get, :post]
   match 'my/page_layout', :controller => 'my', :action => 'page_layout', :via => :get
   match 'my/add_block', :controller => 'my', :action => 'add_block', :via => :post

lib/redmine/sudo_mode.rb

@@ -0,0 +1,224 @@
+require 'active_support/core_ext/object/to_query'
+require 'rack/utils'
+
+module Redmine
+  module SudoMode
+
+    # timespan after which sudo mode expires when unused.
+    MAX_INACTIVITY = 15.minutes
+
+
+    class SudoRequired < StandardError
+    end
+
+
+    class Form
+      include ActiveModel::Validations
+
+      attr_accessor :password, :original_fields
+      validate :check_password
+
+      def initialize(password = nil)
+        self.password = password
+      end
+
+      def check_password
+        unless password.present? && User.current.check_password?(password)
+          errors[:password] << :invalid
+        end
+      end
+    end
+
+
+    module Helper
+      # Represents params data from hash as hidden fields
+      #
+      # taken from https://github.com/brianhempel/hash_to_hidden_fields
+      def hash_to_hidden_fields(hash)
+        cleaned_hash = hash.reject { |k, v| v.nil? }
+        pairs = cleaned_hash.to_query.split(Rack::Utils::DEFAULT_SEP)
+        tags = pairs.map do |pair|
+          key, value = pair.split('=', 2).map { |str| Rack::Utils.unescape(str) }
+          hidden_field_tag(key, value)
+        end
+        tags.join("\n").html_safe
+      end
+    end
+
+
+    module Controller
+      extend ActiveSupport::Concern
+
+      included do
+        around_filter :sudo_mode
+      end
+
+      # Sudo mode Around Filter
+      #
+      # Checks the 'last used' timestamp from session and sets the
+      # SudoMode::active? flag accordingly.
+      #
+      # After the request refreshes the timestamp if sudo mode was used during
+      # this request.
+      def sudo_mode
+        if api_request?
+          SudoMode.disable!
+        elsif sudo_timestamp_valid?
+          SudoMode.active!
+        end
+        yield
+        update_sudo_timestamp! if SudoMode.was_used?
+      end
+
+      # This renders the sudo mode form / handles sudo form submission.
+      #
+      # Call this method in controller actions if sudo permissions are required
+      # for processing this request. This approach is good in cases where the
+      # action needs to be protected in any case or where the check is simple.
+      #
+      # In cases where this decision depends on complex conditions in the model,
+      # consider the declarative approach using the require_sudo_mode class
+      # method and a corresponding declaration in the model that causes it to throw
+      # a SudoRequired Error when necessary.
+      #
+      # All parameter names given are included as hidden fields to be resubmitted
+      # along with the password.
+      #
+      # Returns true when processing the action should continue, false otherwise.
+      # If false is returned, render has already been called for display of the
+      # password form.
+      #
+      # if @user.mail_changed?
+      #   require_sudo_mode :user or return
+      # end
+      #
+      def require_sudo_mode(*param_names)
+        return true if SudoMode.active?
+
+        if param_names.blank?
+          param_names = params.keys - %w(id action controller sudo_password)
+        end
+
+        process_sudo_form
+
+        if SudoMode.active?
+          true
+        else
+          render_sudo_form param_names
+          false
+        end
+      end
+
+      # display the sudo password form
+      def render_sudo_form(param_names)
+        @sudo_form ||= SudoMode::Form.new
+        @sudo_form.original_fields = params.slice( *param_names )
+        # a simple 'render "sudo_mode/new"' works when used directly inside an
+        # action, but not when called from a before_filter:
+        respond_to do |format|
+          format.html { render 'sudo_mode/new' }
+          format.js   { render 'sudo_mode/new' }
+        end
+      end
+
+      # handle sudo password form submit
+      def process_sudo_form
+        if params[:sudo_password]
+          @sudo_form = SudoMode::Form.new(params[:sudo_password])
+          if @sudo_form.valid?
+            SudoMode.active!
+          else
+            flash.now[:error] = l(:notice_account_wrong_password)
+          end
+        end
+      end
+
+      def sudo_timestamp_valid?
+        session[:sudo_timestamp].to_i > MAX_INACTIVITY.ago.to_i
+      end
+
+      def update_sudo_timestamp!(new_value = Time.now.to_i)
+        session[:sudo_timestamp] = new_value
+      end
+
+      # Before Filter which is used by the require_sudo_mode class method.
+      class SudoRequestFilter < Struct.new(:parameters, :request_methods)
+        def before(controller)
+          method_matches = request_methods.blank? || request_methods.include?(controller.request.method_symbol)
+          if SudoMode.possible? && method_matches
+            controller.require_sudo_mode( *parameters )
+          else
+            true
+          end
+        end
+      end
+
+      module ClassMethods
+
+        # Handles sudo requirements for the given actions, preserving the named
+        # parameters, or any parameters if you omit the :parameters option.
+        #
+        # Sudo enforcement by default is active for all requests to an action
+        # but may be limited to a certain subset of request methods via the
+        # :only option.
+        #
+        # Examples:
+        #
+        # require_sudo_mode :account, only: :post
+        # require_sudo_mode :update, :create, parameters: %w(role)
+        # require_sudo_mode :destroy
+        #
+        def require_sudo_mode(*args)
+          actions = args.dup
+          options = actions.extract_options!
+          filter = SudoRequestFilter.new Array(options[:parameters]), Array(options[:only])
+          before_filter filter, only: actions
+        end
+      end
+    end
+
+
+    # true if the sudo mode state was queried during this request
+    def self.was_used?
+      !!RequestStore.store[:sudo_mode_was_used]
+    end
+
+    # true if sudo mode is currently active.
+    #
+    # Calling this method also turns was_used? to true, therefore
+    # it is important to only call this when sudo is actually needed, as the last
+    # condition to determine wether a change can be done or not.
+    #
+    # If you do it wrong, timeout of the sudo mode will happen too late or not at
+    # all.
+    def self.active?
+      if !!RequestStore.store[:sudo_mode]
+        RequestStore.store[:sudo_mode_was_used] = true
+      end
+    end
+
+    def self.active!
+      RequestStore.store[:sudo_mode] = true
+    end
+
+    def self.possible?
+      !disabled? && User.current.logged?
+    end
+
+    # Turn off sudo mode (never require password entry).
+    def self.disable!
+      RequestStore.store[:sudo_mode_disabled] = true
+    end
+
+    # Turn sudo mode back on
+    def self.enable!
+      RequestStore.store[:sudo_mode_disabled] = nil
+    end
+
+    def self.disabled?
+      !!RequestStore.store[:sudo_mode_disabled]
+    end
+
+  end
+end
+

test/functional/auth_sources_controller_test.rb

@@ -22,6 +22,7 @@ class AuthSourcesControllerTest < ActionController::TestCase
 
   def setup
     @request.session[:user_id] = 1
+    Redmine::SudoMode.disable!
   end
 
   def test_index

test/functional/email_addresses_controller_test.rb

@@ -22,6 +22,7 @@ class EmailAddressesControllerTest < ActionController::TestCase
 
   def setup
     User.current = nil
+    Redmine::SudoMode.disable!
   end
 
   def test_index_with_no_additional_emails

test/functional/groups_controller_test.rb

@@ -22,6 +22,7 @@ class GroupsControllerTest < ActionController::TestCase
 
   def setup
     @request.session[:user_id] = 1
+    Redmine::SudoMode.disable!
   end
 
   def test_index

test/functional/members_controller_test.rb

@@ -23,6 +23,7 @@ class MembersControllerTest < ActionController::TestCase
   def setup
     User.current = nil
     @request.session[:user_id] = 2
+    Redmine::SudoMode.disable!
   end
 
   def test_new

test/functional/my_controller_test.rb

@@ -23,6 +23,7 @@ class MyControllerTest < ActionController::TestCase
 
   def setup
     @request.session[:user_id] = 2
+    Redmine::SudoMode.disable!
   end
 
   def test_index
@@ -253,6 +254,12 @@ class MyControllerTest < ActionController::TestCase
     assert_redirected_to '/my/account'
   end
 
+  def test_show_api_key
+    get :show_api_key
+    assert_response :success
+    assert_select 'pre', User.find(2).api_key
+  end
+
   def test_reset_api_key_with_existing_key
     @previous_token_value = User.find(2).api_key # Will generate one if it's missing
     post :reset_api_key

test/functional/projects_controller_test.rb

@@ -28,6 +28,7 @@ class ProjectsControllerTest < ActionController::TestCase
   def setup
     @request.session[:user_id] = nil
     Setting.default_language = 'en'
+    Redmine::SudoMode.disable!
   end
 
   def test_index_by_anonymous_should_not_show_private_projects

test/functional/roles_controller_test.rb

@@ -23,6 +23,7 @@ class RolesControllerTest < ActionController::TestCase
   def setup
     User.current = nil
     @request.session[:user_id] = 1 # admin
+    Redmine::SudoMode.disable!
   end
 
   def test_index

test/functional/settings_controller_test.rb

@@ -24,6 +24,7 @@ class SettingsControllerTest < ActionController::TestCase
   def setup
     User.current = nil
     @request.session[:user_id] = 1 # admin
+    Redmine::SudoMode.disable!
   end
 
   def test_index

test/functional/users_controller_test.rb

@@ -30,6 +30,7 @@ class UsersControllerTest < ActionController::TestCase
   def setup
     User.current = nil
     @request.session[:user_id] = 1 # admin
+    Redmine::SudoMode.disable!
   end
 
   def test_index

test/integration/admin_test.rb

@@ -26,6 +26,14 @@ class AdminTest < Redmine::IntegrationTest
            :members,
            :enabled_modules
 
+  def setup
+    Redmine::SudoMode.enable!
+  end
+
+  def teardown
+    Redmine::SudoMode.disable!
+  end
+
   def test_add_user
     log_user("admin", "admin")
     get "/users/new"
@@ -36,6 +44,15 @@ class AdminTest < Redmine::IntegrationTest
                     :lastname => "Smith", :mail => "psmith@somenet.foo",
                     :language => "en", :password => "psmith09",
                     :password_confirmation => "psmith09" }
+    assert_response :success
+    assert_nil User.find_by_login("psmith")
+
+    post "/users",
+         :user => { :login => "psmith", :firstname => "Paul",
+                    :lastname => "Smith", :mail => "psmith@somenet.foo",
+                    :language => "en", :password => "psmith09",
+                    :password_confirmation => "psmith09" },
+         :sudo_password => 'admin'
 
     user = User.find_by_login("psmith")
     assert_kind_of User, user

test/integration/sudo_test.rb

@@ -0,0 +1,126 @@
+require File.expand_path('../../test_helper', __FILE__)
+
+class SudoTest < Redmine::IntegrationTest
+  fixtures :projects, :members, :member_roles, :roles, :users
+
+  def setup
+    Redmine::SudoMode.enable!
+  end
+
+  def teardown
+    Redmine::SudoMode.disable!
+  end
+
+  def test_create_member_xhr
+    log_user 'admin', 'admin'
+    get '/projects/ecookbook/settings/members'
+    assert_response :success
+
+    assert_no_difference 'Member.count' do
+      xhr :post, '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}
+    end
+
+    assert_no_difference 'Member.count' do
+      xhr :post, '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: ''
+    end
+
+    assert_no_difference 'Member.count' do
+      xhr :post, '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: 'wrong'
+    end
+
+    assert_difference 'Member.count' do
+      xhr :post, '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: 'admin'
+    end
+    assert User.find(7).member_of?(Project.find(1))
+  end
+
+  def test_create_member
+    log_user 'admin', 'admin'
+    get '/projects/ecookbook/settings/members'
+    assert_response :success
+
+    assert_no_difference 'Member.count' do
+      post '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}
+    end
+
+    assert_no_difference 'Member.count' do
+      post '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: ''
+    end
+
+    assert_no_difference 'Member.count' do
+      post '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: 'wrong'
+    end
+
+    assert_difference 'Member.count' do
+      post '/projects/ecookbook/memberships', membership: {role_ids: [1], user_id: 7}, sudo_password: 'admin'
+    end
+
+    assert_redirected_to '/projects/ecookbook/settings/members'
+    assert User.find(7).member_of?(Project.find(1))
+  end
+
+  def test_create_role
+    log_user 'admin', 'admin'
+    get '/roles'
+    assert_response :success
+
+    get '/roles/new'
+    assert_response :success
+
+    post '/roles', role: { }
+    assert_response :success
+    assert_select 'h2', 'Confirm your password to continue'
+    assert_select 'form[action="/roles"]'
+    assert assigns(:sudo_form).errors.blank?
+
+    post '/roles', role: { name: 'new role', issues_visibility: 'all' }
+    assert_response :success
+    assert_select 'h2', 'Confirm your password to continue'
+    assert_select 'form[action="/roles"]'
+    assert_match /"new role"/, response.body
+    assert assigns(:sudo_form).errors.blank?
+
+    post '/roles', role: { name: 'new role', issues_visibility: 'all' }, sudo_password: 'wrong'
+    assert_response :success
+    assert_select 'h2', 'Confirm your password to continue'
+    assert_select 'form[action="/roles"]'
+    assert_match /"new role"/, response.body
+    assert assigns(:sudo_form).errors[:password].present?
+
+    assert_difference 'Role.count' do
+      post '/roles', role: { name: 'new role', issues_visibility: 'all', assignable: '1', permissions: %w(view_calendar) }, sudo_password: 'admin'
+    end
+    assert_redirected_to '/roles'
+  end
+
+  def test_update_email_address
+    log_user 'jsmith', 'jsmith'
+    get '/my/account'
+    assert_response :success
+    post '/my/account', user: { mail: 'newmail@test.com' }
+    assert_response :success
+    assert_select 'h2', 'Confirm your password to continue'
+    assert_select 'form[action="/my/account"]'
+    assert_match /"newmail@test\.com"/, response.body
+    assert assigns(:sudo_form).errors.blank?
+
+    # wrong password
+    post '/my/account', user: { mail: 'newmail@test.com' }, sudo_password: 'wrong'
+    assert_response :success
+    assert_select 'h2', 'Confirm your password to continue'
+    assert_select 'form[action="/my/account"]'
+    assert_match /"newmail@test\.com"/, response.body
+    assert assigns(:sudo_form).errors[:password].present?
+
+    # correct password
+    post '/my/account', user: { mail: 'newmail@test.com' }, sudo_password: 'jsmith'
+    assert_redirected_to '/my/account'
+    assert_equal 'newmail@test.com', User.find_by_login('jsmith').mail
+
+    # sudo mode should now be active and not require password again
+    post '/my/account', user: { mail: 'even.newer.mail@test.com' }
+    assert_redirected_to '/my/account'
+    assert_equal 'even.newer.mail@test.com', User.find_by_login('jsmith').mail
+  end
+
+end