From 130d8379524aeba8928ca2ab1cc284ad14fe5607 Mon Sep 17 00:00:00 2001 From: Ian Christian Myers Date: Tue, 4 Jun 2013 23:45:25 -0700 Subject: [PATCH 1/4] Implemented strong_parameters for Category/CategoriesController. Category now requires parameters to be permitted by strong_parameters using #require or #permit for mass-assignment. Missing required parameters now throw a ActionController::ParameterMissing execption instead of the Discourse::InvalidParameters execption. --- app/controllers/categories_controller.rb | 8 +++++--- app/models/category.rb | 2 ++ spec/controllers/categories_controller_spec.rb | 12 ++++++------ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 9790b02dbe8..65256a61762 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -27,7 +27,6 @@ class CategoriesController < ApplicationController end def create - requires_parameters(*required_param_keys) guardian.ensure_can_create!(Category) @category = Category.create(category_params.merge(user: current_user)) @@ -37,7 +36,6 @@ class CategoriesController < ApplicationController end def update - requires_parameters(*required_param_keys) guardian.ensure_can_edit!(@category) json_result(@category, serializer: CategorySerializer) { |cat| cat.update_attributes(category_params) } end @@ -59,7 +57,11 @@ class CategoriesController < ApplicationController end def category_params - params.slice(*category_param_keys) + required_param_keys.each do |key| + params.require(key) + end + + params.permit(*category_param_keys) end def fetch_category diff --git a/app/models/category.rb b/app/models/category.rb index 85943642c8d..269038385ec 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1,4 +1,6 @@ class Category < ActiveRecord::Base + include ActiveModel::ForbiddenAttributesProtection + belongs_to :topic, dependent: :destroy belongs_to :topic_only_relative_url, select: "id, title, slug", diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb index 7e05df58ddf..fba156c5223 100644 --- a/spec/controllers/categories_controller_spec.rb +++ b/spec/controllers/categories_controller_spec.rb @@ -19,15 +19,15 @@ describe CategoriesController do end it 'raises an exception when the name is missing' do - lambda { xhr :post, :create, color: 'ff0', text_color: 'fff' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :post, :create, color: 'ff0', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing) end it 'raises an exception when the color is missing' do - lambda { xhr :post, :create, name: 'hello', text_color: 'fff' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :post, :create, name: 'hello', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing) end it 'raises an exception when the text color is missing' do - lambda { xhr :post, :create, name: 'hello', color: 'ff0' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :post, :create, name: 'hello', color: 'ff0' }.should raise_error(ActionController::ParameterMissing) end describe 'failure' do @@ -106,15 +106,15 @@ describe CategoriesController do end it "requires a name" do - lambda { xhr :put, :update, id: @category.slug, color: 'fff', text_color: '0ff' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :put, :update, id: @category.slug, color: 'fff', text_color: '0ff' }.should raise_error(ActionController::ParameterMissing) end it "requires a color" do - lambda { xhr :put, :update, id: @category.slug, name: 'asdf', text_color: '0ff' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :put, :update, id: @category.slug, name: 'asdf', text_color: '0ff' }.should raise_error(ActionController::ParameterMissing) end it "requires a text color" do - lambda { xhr :put, :update, id: @category.slug, name: 'asdf', color: 'fff' }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :put, :update, id: @category.slug, name: 'asdf', color: 'fff' }.should raise_error(ActionController::ParameterMissing) end describe 'failure' do From 3b245031a4f07dfc5ef8c2363478759d9af7993c Mon Sep 17 00:00:00 2001 From: Ian Christian Myers Date: Wed, 5 Jun 2013 00:04:03 -0700 Subject: [PATCH 2/4] Implemented strong_parameters for Invite/InvitesController. The email parameter is now required using strong parameters and will throw ActionController::ParameterMissing if it is missing. If the email address is incorrect or invalid, Discourse::InvalidParameters will still be thrown. --- app/controllers/invites_controller.rb | 2 +- app/models/invite.rb | 1 + spec/controllers/invites_controller_spec.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index dbaf94e7fb2..1477f5734d4 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -29,7 +29,7 @@ class InvitesController < ApplicationController end def destroy - requires_parameter(:email) + params.require(:email) invite = Invite.where(invited_by_id: current_user.id, email: params[:email]).first raise Discourse::InvalidParameters.new(:email) if invite.blank? diff --git a/app/models/invite.rb b/app/models/invite.rb index 9a5acc73b0b..b065a9ba9ce 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -1,6 +1,7 @@ require_dependency 'trashable' class Invite < ActiveRecord::Base + include ActiveModel::ForbiddenAttributesProtection include Trashable belongs_to :user diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 473553cf2d8..29c3a3da2aa 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -17,7 +17,7 @@ describe InvitesController do it 'raises an error when the email is missing' do - lambda { delete :destroy }.should raise_error(Discourse::InvalidParameters) + lambda { delete :destroy }.should raise_error(ActionController::ParameterMissing) end it "raises an error when the email cannot be found" do From f50b6488444a38a4fdf017caa0d546a6895e59f0 Mon Sep 17 00:00:00 2001 From: Ian Christian Myers Date: Wed, 5 Jun 2013 00:23:51 -0700 Subject: [PATCH 3/4] Implemented strong_parameters for PostAction/PostActionsController. PostActionsController now uses strong_parameters' #require to require certain parameters. ActionController::ParameterMissing is now thrown when a reqired parameter is missing, rather than Discourse::InvalidParameters. --- app/controllers/post_actions_controller.rb | 4 ++-- app/models/post_action.rb | 1 + spec/controllers/post_actions_controller_spec.rb | 12 ++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb index bc781934423..5174a1800d1 100644 --- a/app/controllers/post_actions_controller.rb +++ b/app/controllers/post_actions_controller.rb @@ -70,7 +70,7 @@ class PostActionsController < ApplicationController private def fetch_post_from_params - requires_parameter(:id) + params.require(:id) finder = Post.where(id: params[:id]) # Include deleted posts if the user is a moderator (to guardian ?) @@ -81,7 +81,7 @@ class PostActionsController < ApplicationController end def fetch_post_action_type_id_from_params - requires_parameter(:post_action_type_id) + params.require(:post_action_type_id) @post_action_type_id = params[:post_action_type_id].to_i end end diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 46991c3c5a0..4ac3a9ec6cd 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -5,6 +5,7 @@ require_dependency 'trashable' class PostAction < ActiveRecord::Base class AlreadyActed < StandardError; end + include ActiveModel::ForbiddenAttributesProtection include RateLimiter::OnCreateRecord include Trashable diff --git a/spec/controllers/post_actions_controller_spec.rb b/spec/controllers/post_actions_controller_spec.rb index 3c0fed3d2ae..9c7b8e8df4a 100644 --- a/spec/controllers/post_actions_controller_spec.rb +++ b/spec/controllers/post_actions_controller_spec.rb @@ -14,11 +14,11 @@ describe PostActionsController do end it 'raises an error when the id is missing' do - lambda { xhr :post, :create, post_action_type_id: PostActionType.types[:like] }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :post, :create, post_action_type_id: PostActionType.types[:like] }.should raise_error(ActionController::ParameterMissing) end it 'raises an error when the post_action_type_id index is missing' do - lambda { xhr :post, :create, id: @post.id }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :post, :create, id: @post.id }.should raise_error(ActionController::ParameterMissing) end it "fails when the user doesn't have permission to see the post" do @@ -70,7 +70,7 @@ describe PostActionsController do let!(:user) { log_in } it 'raises an error when the post_action_type_id is missing' do - lambda { xhr :delete, :destroy, id: post.id }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :delete, :destroy, id: post.id }.should raise_error(ActionController::ParameterMissing) end it "returns 404 when the post action type doesn't exist for that user" do @@ -116,7 +116,7 @@ describe PostActionsController do let!(:user) { log_in(:moderator) } it "raises an error without a post_action_type_id" do - -> { xhr :post, :clear_flags, id: flagged_post.id }.should raise_error(Discourse::InvalidParameters) + -> { xhr :post, :clear_flags, id: flagged_post.id }.should raise_error(ActionController::ParameterMissing) end it "raises an error when the user doesn't have access" do @@ -160,13 +160,13 @@ describe PostActionsController do it 'raises an error without an id' do lambda { xhr :get, :users, post_action_type_id: PostActionType.types[:like] - }.should raise_error(Discourse::InvalidParameters) + }.should raise_error(ActionController::ParameterMissing) end it 'raises an error without a post action type' do lambda { xhr :get, :users, id: post.id - }.should raise_error(Discourse::InvalidParameters) + }.should raise_error(ActionController::ParameterMissing) end it "fails when the user doesn't have permission to see the post" do From 41528f5d1157fa615453dcca2735d59677aaf1f1 Mon Sep 17 00:00:00 2001 From: Ian Christian Myers Date: Wed, 5 Jun 2013 00:55:55 -0700 Subject: [PATCH 4/4] Implemented strong_parameters for Upload/UploadsController. The topic_id param is now required using strong_parameters' #require method. If the parameter is missing ActionController::ParameterMissing will be raised instead of Discourse::InvalidParameters. --- app/controllers/uploads_controller.rb | 2 +- app/models/upload.rb | 2 ++ spec/controllers/uploads_controller_spec.rb | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index c46dde316a3..db82d1eed95 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -2,7 +2,7 @@ class UploadsController < ApplicationController before_filter :ensure_logged_in def create - requires_parameter(:topic_id) + params.require(:topic_id) file = params[:file] || params[:files].first # only supports images for now diff --git a/app/models/upload.rb b/app/models/upload.rb index de7226a5334..f47d0a28e68 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -5,6 +5,8 @@ require 's3' require 'local_store' class Upload < ActiveRecord::Base + include ActiveModel::ForbiddenAttributesProtection + belongs_to :user belongs_to :topic diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index d3a971ff573..bbbef2914d2 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -16,7 +16,7 @@ describe UploadsController do context 'missing params' do it 'raises an error without the topic_id param' do - -> { xhr :post, :create }.should raise_error(Discourse::InvalidParameters) + -> { xhr :post, :create }.should raise_error(ActionController::ParameterMissing) end end