summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAzul <azul@riseup.net>2017-09-17 09:54:55 +0200
committerAzul <azul@riseup.net>2017-10-24 13:33:03 +0200
commit325bccc1649c928d512ce7c7b11e14566a8c9eeb (patch)
tree4a9adacadce129529bed44792e6a4de1dc158519
parentfecd710de6c574ac8e2b0c45ad9e081badd59b61 (diff)
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.
-rw-r--r--app/controllers/api/users_controller.rb11
-rw-r--r--test/functional/api/users_controller_test.rb22
-rw-r--r--test/integration/api/srp_test.rb2
-rw-r--r--test/integration/api/update_account_test.rb8
4 files changed, 37 insertions, 6 deletions
diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index 709e076..cb7b7bc 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -53,7 +53,7 @@ module Api
end
def update
- @user.account.update params[:user]
+ @user.account.update user_update_params
respond_with @user
end
@@ -67,6 +67,15 @@ module Api
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
current_user.is_monitor? || params[:identities] == "destroy"
end
diff --git a/test/functional/api/users_controller_test.rb b/test/functional/api/users_controller_test.rb
index 88ecae0..ee183f9 100644
--- a/test/functional/api/users_controller_test.rb
+++ b/test/functional/api/users_controller_test.rb
@@ -4,28 +4,42 @@ class Api::UsersControllerTest < ApiControllerTest
test "user can change settings" do
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.expects(:update).with(changed_attribs)
Account.expects(:new).with(user).returns(account_settings)
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_response 204
assert @response.body.blank?, "Response should be blank"
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
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.expects(:update).with(changed_attribs)
Account.expects(:new).with(user).returns(account_settings)
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_response 204
diff --git a/test/integration/api/srp_test.rb b/test/integration/api/srp_test.rb
index b9605f9..ef5d9b8 100644
--- a/test/integration/api/srp_test.rb
+++ b/test/integration/api/srp_test.rb
@@ -46,7 +46,7 @@ class SrpTest < RackTest
@password = password
end
- def update_user(params)
+ def update_user(params = {})
put api_url("users/#{@user.id}.json"),
user_params(params),
auth_headers
diff --git a/test/integration/api/update_account_test.rb b/test/integration/api/update_account_test.rb
index f083dbc..dd28b06 100644
--- a/test/integration/api/update_account_test.rb
+++ b/test/integration/api/update_account_test.rb
@@ -19,6 +19,14 @@ class UpdateAccountTest < SrpTest
assert_login_required
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
authenticate
update_user password: "No! Verify me instead."