Skip to content
Snippets Groups Projects
Unverified Commit 325bccc1 authored by azul's avatar azul
Browse files

fix: sanity checks on user params

fixes #8801

Includes a test reproducing 500 on lynx

We now make use of ActionController::Parameters require and permit
methods.
parent fecd710d
No related branches found
No related tags found
1 merge request!50fix: sanity checks on user params
Pipeline #
...@@ -53,7 +53,7 @@ module Api ...@@ -53,7 +53,7 @@ module Api
end end
def update def update
@user.account.update params[:user] @user.account.update user_update_params
respond_with @user respond_with @user
end end
...@@ -67,6 +67,15 @@ module Api ...@@ -67,6 +67,15 @@ module Api
private private
def user_update_params
params.require(:user).permit :login,
:password_verifier,
:password_salt,
:recovery_code_verifier,
:recovery_code_salt,
:public_key
end
def release_handles def release_handles
current_user.is_monitor? || params[:identities] == "destroy" current_user.is_monitor? || params[:identities] == "destroy"
end end
......
...@@ -4,28 +4,42 @@ class Api::UsersControllerTest < ApiControllerTest ...@@ -4,28 +4,42 @@ class Api::UsersControllerTest < ApiControllerTest
test "user can change settings" do test "user can change settings" do
user = find_record :user user = find_record :user
changed_attribs = record_attributes_for :user_with_settings attribs = record_attributes_for(:user)
changed_attribs = attribs.slice 'login',
'password_verifier',
'password_salt'
account_settings = stub account_settings = stub
account_settings.expects(:update).with(changed_attribs) account_settings.expects(:update).with(changed_attribs)
Account.expects(:new).with(user).returns(account_settings) Account.expects(:new).with(user).returns(account_settings)
login user login user
api_put :update, :user => changed_attribs, :id => user.id, :format => :json api_put :update, :user => attribs, :id => user.id, :format => :json
assert_equal user, assigns[:user] assert_equal user, assigns[:user]
assert_response 204 assert_response 204
assert @response.body.blank?, "Response should be blank" assert @response.body.blank?, "Response should be blank"
end end
test "deal with empty settings" do
user = find_record :user
login user
assert_raises ActionController::ParameterMissing do
api_put :update, :id => user.id, :format => :json
end
end
test "admin can update user" do test "admin can update user" do
user = find_record :user user = find_record :user
changed_attribs = record_attributes_for :user_with_settings attribs = record_attributes_for(:user)
changed_attribs = attribs.slice 'login',
'password_verifier',
'password_salt'
account_settings = stub account_settings = stub
account_settings.expects(:update).with(changed_attribs) account_settings.expects(:update).with(changed_attribs)
Account.expects(:new).with(user).returns(account_settings) Account.expects(:new).with(user).returns(account_settings)
login :is_admin? => true login :is_admin? => true
api_put :update, :user => changed_attribs, :id => user.id, :format => :json api_put :update, :user => attribs, :id => user.id, :format => :json
assert_equal user, assigns[:user] assert_equal user, assigns[:user]
assert_response 204 assert_response 204
......
...@@ -46,7 +46,7 @@ class SrpTest < RackTest ...@@ -46,7 +46,7 @@ class SrpTest < RackTest
@password = password @password = password
end end
def update_user(params) def update_user(params = {})
put api_url("users/#{@user.id}.json"), put api_url("users/#{@user.id}.json"),
user_params(params), user_params(params),
auth_headers auth_headers
......
...@@ -19,6 +19,14 @@ class UpdateAccountTest < SrpTest ...@@ -19,6 +19,14 @@ class UpdateAccountTest < SrpTest
assert_login_required assert_login_required
end end
test "empty request" do
authenticate
update_user
refute last_response.successful?
assert_equal 400, last_response.status
assert_equal '', last_response.body
end
test "update password via api" do test "update password via api" do
authenticate authenticate
update_user password: "No! Verify me instead." update_user password: "No! Verify me instead."
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment