jro_apps/redmine Commit - r13956:c2fca3799927

Don't use SudoMode.disable! to skip API requests (#19851)....

Jean-Philippe Lang -

r13956:c2fca3799927

parent child

Context file:

r13956:c2fca3799927

Collapse all files

lib/redmine/sudo_mode.rb +4 -4

             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?
+                    if sudo_timestamp_valid?
-                      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
+                      if controller.api_request?
+                        true
+                      elsif 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?
                   enabled? && 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.enabled?
                   Redmine::Configuration['sudo_mode'] && !RequestStore.store[:sudo_mode_disabled]
                 end
               end
             end

test/integration/sudo_mode_test.rb +15 0

             require File.expand_path('../../test_helper', __FILE__)
             class SudoTest < Redmine::IntegrationTest
               fixtures :projects, :members, :member_roles, :roles, :users
               def setup
                 Redmine::SudoMode.stubs(:enabled?).returns(true)
               end
               def test_add_user
                 log_user("admin", "admin")
                 get "/users/new"
                 assert_response :success
                 post "/users",
                      :user => { :login => "psmith", :firstname => "Paul",
                                 :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'
                 assert_response 302
                 user = User.find_by_login("psmith")
                 assert_kind_of User, user
               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
+              def test_sudo_mode_should_skip_api_requests
+                with_settings :rest_api_enabled => '1' do
+                  assert_difference('User.count') do
+                    post '/users.json', {
+                      :user => {
+                        :login => 'foo', :firstname => 'Firstname', :lastname => 'Lastname',
+                        :mail => 'foo@example.net', :password => 'secret123',
+                        :mail_notification => 'only_assigned'}
+                      },
+                      credentials('admin')
+                    assert_response :created
+                  end
+                end
+              end
             end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages