Project

General

Profile

« Previous | Next » 

Revision bd622a22

Added by Dominic Cleal about 8 years ago

refs #14691 - user editing self should not change User.current

Rather than changing the behaviour of #to_label to return persisted
data, the User.current object should not be modified with unsaved data
from the form submission or API update.

User.current is used for authz as well as for display purposes, so
shouldn't be changed. Parameter filtering protects privilege escalation
in this case.

View differences:

app/controllers/api/v1/users_controller.rb
private
def find_resource
editing_self? ? @user = User.current : super
editing_self? ? @user = User.find(User.current.id) : super
end
end
end
app/controllers/api/v2/users_controller.rb
private
def find_resource
editing_self? ? @user = User.current : super
editing_self? ? @user = User.find(User.current.id) : super
end
def allowed_nested_id
app/controllers/users_controller.rb
private
def find_resource(permission = :view_users)
editing_self? ? User.current : User.authorized(permission).except_hidden.find(params[:id])
editing_self? ? User.find(User.current.id) : User.authorized(permission).except_hidden.find(params[:id])
end
def login_user(user)
app/models/user.rb
end
def to_label
(firstname.present? || lastname.present?) ? "#{firstname_was} #{lastname_was}" : login
(firstname.present? || lastname.present?) ? "#{firstname} #{lastname}" : login
end
alias_method :name, :to_label
test/functional/api/v1/users_controller_test.rb
end
assert_response :success
end
test '#update should not be editing User.current' do
user = User.create :login => "foo", :mail => "foo@bar.com", :auth_source => auth_sources(:one)
as_user user do
put :update, { :id => user.id, :user => valid_attrs }
end
assert_equal user, assigns(:user)
refute_equal user.object_id, assigns(:user).object_id
assert_response :success
end
end
test/functional/api/v2/users_controller_test.rb
end
assert_response :success
end
test '#update should not be editing User.current' do
user = User.create :login => "foo", :mail => "foo@bar.com", :auth_source => auth_sources(:one)
as_user user do
put :update, { :id => user.id, :user => valid_attrs }
end
assert_equal user, assigns(:user)
refute_equal user.object_id, assigns(:user).object_id
assert_response :success
end
end
test/functional/users_controller_test.rb
assert_response :success
end
test 'user should not be editing User.current' do
user = users(:one)
User.expects(:current).at_least_once.returns(user)
get :edit, { :id => user.id }
assert_equal user, assigns(:user)
refute_equal user.object_id, assigns(:user).object_id
assert_response :success
end
test 'non admin user should be able to update itself' do
User.current = users(:one)
put :update, { :id => users(:one).id, :user => { :firstname => 'test' } }
test/unit/user_test.rb
user.timezone = ''
assert user.valid?
end
test '.to_label should return persisted name' do
user = users(:one)
user.firstname = 'First'
user.lastname = 'Last'
refute_equal('First Last', user.to_label, 'User to_label should not change until saved')
user.save
assert_equal('First Last', user.to_label, 'User to_label should change after saved')
end
end

Also available in: Unified diff