diff --git a/Gemfile b/Gemfile index f6a560f..fa75df1 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,3 @@ -source :rubygems +source 'https://rubygems.org' -gemspec \ No newline at end of file +gemspec diff --git a/lib/resources_controller.rb b/lib/resources_controller.rb index 58a2cfd..ee9d155 100644 --- a/lib/resources_controller.rb +++ b/lib/resources_controller.rb @@ -22,6 +22,9 @@ # # class ForumsController < ApplicationController # resources_controller_for :forums +# def forum_params +# params.fetch('forum',{}).permit(%i(name description ...)) +# end # end # # Your controller will get the standard CRUD actions, @forum will be set in member actions, @forums in @@ -30,9 +33,15 @@ # ==== Example 2: Specifying enclosing resources # class PostsController < ApplicationController # resources_controller_for :posts, :in => :forum +# def resource_params +# params.fetch('post',{}).permit(%i(title body ...)) +# end # end # -# As above, but the controller will load @forum on every action, and use @forum to find and create @posts +# As above, but the controller will load @forum on every action, and use @forum to find and create @posts. +# Note also the use of resource_params instead of post_params (named by the resource +# name); either version works, but one must be defined. See the discussion of +# strong parameters below. # # ==== Wildcard enclosing resources # All of the above examples will work for any routes that match what it specified @@ -76,6 +85,7 @@ # resources_controller_for :account, :class => User, :singleton => true do # @current_user # end +# def account_params; ...; end # end # # Your controller will use the block to find the resource. The @account will be assigned to @current_user @@ -85,6 +95,7 @@ # # class PostsController < ApplicationController # resources_controller_for :posts +# def post_params; ...; end # end # # This will now work for /users/2/posts. @@ -112,6 +123,7 @@ # # class ImageController < ApplicationController # resources_controller_for :image, :singleton => true +# def image_params; ...; end # end # # When invoked with /users/3/image RC will find @user, and use @user.image to find the resource, and @@ -159,26 +171,32 @@ # map_enclosing_resource :account, :singleton => true, :find => :current_user # # def current_user # get it from session or whatnot +# end # end # # class ForumsController < AplicationController # resources_controller_for :forums +# def forum_params; ...; end # end # # class PostsController < AplicationController # resources_controller_for :posts +# def post_params; ...; end # end # # class UsersController < AplicationController # resources_controller_for :users +# def user_params; ...; end # end # # class ImageController < AplicationController # resources_controller_for :image, :singleton => true +# def image_params; ...; end # end # # class AccountController < ApplicationController # resources_controller_for :account, :singleton => true, :find => :current_user +# def account_params; ...; end # end # # This is how the app will handle the following routes: @@ -212,6 +230,42 @@ # @post = @account.posts.find(3) # @post.update_attributes(params[:post]) # +# === Strong parameters +# +# Note that in all the examples above, the controller must define a method +# resource_params, or #{resource_name}_params, which will be used to select +# parameters out of the params fed into the controller by the standard +# create and update actions. (This is invoked by a parent resource_params +# method in lib/resource_controller/resource_methods.rb, which dispatches +# to whichever available, with #{resource_name}_params preferred if they +# both are.) +# +# The method can be inherited from a base class, not defined by a controller +# itself. Thus, if you have several controllers that take a common set of +# attributes, you can define resource_params in a controller base class and not +# bother with a bunch of identical resource_params definitions in the +# definitions of the individual controllers. +# +# (It also means that if, for some reason, you want to avoid defining any +# such methods at all, you can put +# +# def resource_params +# params.fetch(resource_name, {}).permit! +# end +# +# in ApplicationController. However, this is not recommended for the same +# reasons permit! (which permits whatever the client supplied) is not +# recommended -- a hostile user can use browser dev tools to add extra +# inputs to your forms, and set whatever attributes they like, whether you +# expected them to or not. Easy exploits for this are setting 'is_admin' on a +# User object, or altering the blog_id on a Post to put it on a blog where the +# user doesn't have publication rights. And that's just within the context +# of the sample app above. A reasonable rule of thumb might be to not give +# any user access to a controller with permit! unless the same user has +# access to the production database SQL prompt. And then remember that a +# web client with access to your trusted user's credentials might not be +# your trusted user!) +# # === Views # # Ok - so how do I write the views? diff --git a/lib/resources_controller/actions.rb b/lib/resources_controller/actions.rb index 3dd1a36..7acc0cf 100644 --- a/lib/resources_controller/actions.rb +++ b/lib/resources_controller/actions.rb @@ -121,14 +121,4 @@ def destroy end end - - # Never trust parameters from the scary internet, only allow the white list through. - def resource_params - if self.respond_to?("#{resource_name}_params", true) - return self.send("#{resource_name}_params") - else - return params.require(resource_name).permit( *(resource_service.content_columns.map(&:name) - [ 'updated_at', 'created_at' ]) ) - end - end - end diff --git a/lib/resources_controller/resource_methods.rb b/lib/resources_controller/resource_methods.rb index 660b95e..358ee52 100644 --- a/lib/resources_controller/resource_methods.rb +++ b/lib/resources_controller/resource_methods.rb @@ -13,12 +13,11 @@ def find_resource(id = nil) resource_service.find id end - # makes a new resource, if attributes are not supplied, determine them from the - # params hash and the current resource_class, or resource_name (the latter left in for BC) + # makes a new resource. If attributes are not supplied, we try to get them + # from 'resource_params', if available. def new_resource(attributes = nil, &block) - if attributes.blank? && respond_to?(:params) && params.is_a?(ActionController::Parameters) - resource_form_name = ActiveModel::Naming.singular(resource_class) - attributes = params[resource_form_name] || params[resource_name] || {} + if attributes.blank? && respond_to?(:resource_params) + attributes = resource_params end resource_service.new attributes, &block end @@ -34,8 +33,10 @@ def destroy_resource(id = nil) def resource_params if self.respond_to?("#{resource_name}_params", true) return self.send("#{resource_name}_params") + elsif defined?(super) + return super else - return params.fetch(resource_name, {}).permit( *(resource_service.content_columns.map(&:name) - [ 'updated_at', 'created_at' ]) ) + raise NoMethodError, "resource_params and #{resource_name}_params both unimplemented" end end diff --git a/spec/app.rb b/spec/app.rb index 8cd00a5..5cd8d25 100644 --- a/spec/app.rb +++ b/spec/app.rb @@ -1,3 +1,4 @@ +# coding: utf-8 # Testing app setup module ResourcesControllerTest @@ -95,6 +96,7 @@ class Application < Rails::Application create_table :infos, :force => true do |t| t.column "user_id", :integer + t.column "info", :string end create_table :addresses, :force => true do |t| @@ -200,16 +202,14 @@ class ApplicationController < ActionController::Base def current_user @current_user end - - def resource_params - params - end end module Admin class ForumsController < ApplicationController resources_controller_for :forums - + def resource_params + params['forum'].try(:permit, %i(owner_id)) + end end class InterestsController < ApplicationController @@ -238,10 +238,16 @@ def account_params class InfosController < ApplicationController resources_controller_for :info, :singleton => true, :only => [:show, :edit, :update] + def resource_params + params['info'].try(:permit, %i(attr other_attr)) # no real attrs! + end end class TagsController < ApplicationController resources_controller_for :tags + def resource_params + params['tag'].try(:permit, :name) + end end class UsersController < ApplicationController diff --git a/spec/controllers/admin_forums_controller_spec.rb b/spec/controllers/admin_forums_controller_spec.rb index 12556ab..9396ca6 100644 --- a/spec/controllers/admin_forums_controller_spec.rb +++ b/spec/controllers/admin_forums_controller_spec.rb @@ -433,11 +433,11 @@ def do_get end def do_post - post :create, params: { :forum => {:name => 'Forum'} } + post :create, params: { :forum => {:owner_id => '1'} } end it "should create a new forum" do - expect(Forum).to receive(:new).with({'name' => 'Forum'}).and_return(@mock_forum) + expect(Forum).to receive(:new).with(ac_permitted(:owner_id => '1')).and_return(@mock_forum) do_post end @@ -463,11 +463,11 @@ def do_post end def do_post - post :create, params: { :forum => {:name => 'Forum'} }, xhr: true + post :create, params: { :forum => {:owner_id => '1'} }, xhr: true end it "should create a new forum" do - expect(Forum).to receive(:new).with({'name' => 'Forum'}).and_return(@mock_forum) + expect(Forum).to receive(:new).with(ac_permitted(:owner_id => '1')).and_return(@mock_forum) do_post end @@ -492,12 +492,13 @@ def do_post before(:each) do @mock_forum = double('Forum').as_null_object + @update_params = { :owner_id => "1" } allow(@mock_forum).to receive(:to_param).and_return("1") allow(Forum).to receive(:find).and_return(@mock_forum) end def do_update - put :update, params: { :id => "1" } + put :update, params: { :id => "1", :forum => @update_params } end it "should find the forum requested" do @@ -511,7 +512,9 @@ def do_update end it "should update the found forum" do - expect(@mock_forum).to receive(:update).and_return(true) + expected_params = ActionController::Parameters.new(@update_params) + expected_params.permit(:owner_id) + expect(@mock_forum).to receive(:update).with(expected_params).and_return(true) do_update expect(assigns(:forum)).to eq(@mock_forum) end diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb index 0475e78..ddae4e0 100644 --- a/spec/controllers/comments_controller_spec.rb +++ b/spec/controllers/comments_controller_spec.rb @@ -298,11 +298,11 @@ def do_get end def do_post - post :create, params: { :comment => {:name => 'Comment'}, :forum_id => '3', :post_id => '2' } + post :create, params: { :comment => {:user_id => '3'}, :forum_id => '3', :post_id => '2' } end it "should build a new comment" do - expect(@post_comments).to receive(:build).with({'name' => 'Comment'}).and_return(@comment) + expect(@post_comments).to receive(:build).with(ac_permitted('user_id' => '3')).and_return(@comment) do_post end diff --git a/spec/controllers/forums_controller_spec.rb b/spec/controllers/forums_controller_spec.rb index 03d0992..9596b02 100644 --- a/spec/controllers/forums_controller_spec.rb +++ b/spec/controllers/forums_controller_spec.rb @@ -204,12 +204,12 @@ def do_get end def do_post - post :create, params: { :forum => {:name => 'Forum'}, :resource_path => '/forums', :resource_method => :post } + post :create, params: { :forum => {:title => 'Forum'}, :resource_path => '/forums', :resource_method => :post } end it "should create a new forum" do - expect(Forum).to receive(:new).with({'name' => 'Forum'}).and_return(@mock_forum) + expect(Forum).to receive(:new).with(ac_permitted('title' => 'Forum')).and_return(@mock_forum) do_post end @@ -490,11 +490,11 @@ def do_get end def do_post - post :create, params: { :forum => {:name => 'Forum'} } + post :create, params: { :forum => {:title => 'Forum'} } end it "should create a new forum" do - expect(Forum).to receive(:new).with({'name' => 'Forum'}).and_return(@mock_forum) + expect(Forum).to receive(:new).with(ac_permitted('title' => 'Forum')).and_return(@mock_forum) do_post end @@ -520,11 +520,11 @@ def do_post end def do_post - post :create, params: { :forum => {:name => 'Forum'} }, xhr: true + post :create, params: { :forum => {:title => 'Forum'} }, xhr: true end it "should create a new forum" do - expect(Forum).to receive(:new).with({'name' => 'Forum'}).and_return(@mock_forum) + expect(Forum).to receive(:new).with(ac_permitted('title' => 'Forum')).and_return(@mock_forum) do_post end diff --git a/spec/controllers/tags_controller_via_forum_spec.rb b/spec/controllers/tags_controller_via_forum_spec.rb index a43fada..d194d65 100644 --- a/spec/controllers/tags_controller_via_forum_spec.rb +++ b/spec/controllers/tags_controller_via_forum_spec.rb @@ -151,7 +151,7 @@ def do_get end it "should build a new tag with params" do - expect(@forum_tags).to receive(:build).with("name" => "hello").and_return(@tag) + expect(@forum_tags).to receive(:build).with(ac_permitted("name" => "hello")).and_return(@tag) do_get end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dc6bd86..48c3826 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,11 +9,18 @@ # just about every test fail. ActionController::Base.view_paths = [File.join(File.dirname(__FILE__), "app/views")] +module SpecConveniences + def ac_permitted(hsh) + ActionController::Parameters.new(hsh).permit! + end +end + RSpec.configure do |config| config.use_transactional_fixtures = true config.use_instantiated_fixtures = false config.infer_spec_type_from_file_location! config.expect_with(:rspec) { |c| c.syntax = [ :should, :expect ] } + config.include SpecConveniences # defined above end require 'rails-controller-testing'