From 0d01c334827ea32f8e70665783cdfe755fe6eff0 Mon Sep 17 00:00:00 2001
From: Ian Christian Myers <ian@iancmyers.com>
Date: Thu, 6 Jun 2013 00:14:32 -0700
Subject: [PATCH] Enabled strong_parameters across all models/controllers.

All models are now using ActiveModel::ForbiddenAttributesProtection, which shifts the responsibility for parameter whitelisting for mass-assignments from the model to the controller. attr_accessible has been disabled and removed as this functionality replaces that.

The require_parameters method in the ApplicationController has been removed in favor of strong_parameters' #require method.

It is important to note that there is still some refactoring required to get all parameters to pass through #require and #permit so that we can guarantee that parameter values are scalar. Currently strong_parameters, in most cases, is only being utilized to require parameters and to whitelist the few places that do mass-assignments.
---
 .../admin/impersonate_controller.rb            |  2 +-
 .../admin/site_customizations_controller.rb    | 10 ++++++++--
 .../admin/site_settings_controller.rb          |  2 +-
 app/controllers/application_controller.rb      |  8 --------
 app/controllers/clicks_controller.rb           | 18 +++++++++++-------
 app/controllers/posts_controller.rb            |  6 +++---
 app/controllers/search_controller.rb           |  4 +---
 app/controllers/session_controller.rb          |  5 +++--
 app/controllers/topics_controller.rb           | 14 ++++++++------
 app/controllers/users_controller.rb            |  8 ++++----
 app/models/admin_log.rb                        |  2 --
 app/models/cas_user_info.rb                    |  2 --
 app/models/category.rb                         |  2 --
 app/models/facebook_user_info.rb               |  1 -
 app/models/invite.rb                           |  1 -
 app/models/post_action.rb                      |  3 ---
 app/models/post_action_type.rb                 |  2 --
 app/models/site_setting.rb                     |  2 --
 app/models/topic_allowed_group.rb              |  1 -
 app/models/topic_allowed_user.rb               |  1 -
 app/models/upload.rb                           |  2 --
 app/models/user.rb                             |  2 --
 app/models/user_action.rb                      |  1 -
 app/models/user_open_id.rb                     |  1 -
 app/models/user_visit.rb                       |  1 -
 config/application.rb                          |  4 ++++
 config/initializers/strong_parameters.rb       |  1 +
 .../admin/impersonate_controller_spec.rb       |  2 +-
 .../admin/site_settings_controller_spec.rb     |  2 +-
 spec/controllers/clicks_controller_spec.rb     | 10 +++++-----
 spec/controllers/posts_controller_spec.rb      |  6 +++---
 spec/controllers/session_controller_spec.rb    |  4 ++--
 spec/controllers/topics_controller_spec.rb     | 14 +++++++-------
 spec/controllers/users_controller_spec.rb      |  6 +++---
 34 files changed, 67 insertions(+), 83 deletions(-)
 create mode 100644 config/initializers/strong_parameters.rb

diff --git a/app/controllers/admin/impersonate_controller.rb b/app/controllers/admin/impersonate_controller.rb
index 6cdea0077..eec39b4aa 100644
--- a/app/controllers/admin/impersonate_controller.rb
+++ b/app/controllers/admin/impersonate_controller.rb
@@ -1,7 +1,7 @@
 class Admin::ImpersonateController < Admin::AdminController
 
   def create
-    requires_parameters(:username_or_email)
+    params.require(:username_or_email)
 
     user = User.find_by_username_or_email(params[:username_or_email]).first
 
diff --git a/app/controllers/admin/site_customizations_controller.rb b/app/controllers/admin/site_customizations_controller.rb
index aded94433..046926a0b 100644
--- a/app/controllers/admin/site_customizations_controller.rb
+++ b/app/controllers/admin/site_customizations_controller.rb
@@ -9,7 +9,7 @@ class Admin::SiteCustomizationsController < Admin::AdminController
   end
 
   def create
-    @site_customization = SiteCustomization.new(params[:site_customization])
+    @site_customization = SiteCustomization.new(site_customization_params)
     @site_customization.user_id = current_user.id
 
     respond_to do |format|
@@ -25,7 +25,7 @@ class Admin::SiteCustomizationsController < Admin::AdminController
     @site_customization = SiteCustomization.find(params[:id])
 
     respond_to do |format|
-      if @site_customization.update_attributes(params[:site_customization])
+      if @site_customization.update_attributes(site_customization_params)
         format.json { head :no_content }
       else
         format.json { render json: @site_customization.errors, status: :unprocessable_entity }
@@ -42,4 +42,10 @@ class Admin::SiteCustomizationsController < Admin::AdminController
     end
   end
 
+  private
+
+    def site_customization_params
+      params.require(:site_customization).permit(:name, :stylesheet, :header, :position, :enabled, :key, :override_default_style, :stylesheet_baked)
+    end
+
 end
diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb
index 5c3c8ccbc..c827ef885 100644
--- a/app/controllers/admin/site_settings_controller.rb
+++ b/app/controllers/admin/site_settings_controller.rb
@@ -7,7 +7,7 @@ class Admin::SiteSettingsController < Admin::AdminController
   end
 
   def update
-    requires_parameter(:value)
+    params.require(:value)
     SiteSetting.send("#{params[:id]}=", params[:value])
     render nothing: true
   end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index a07f90da0..b0a6b3469 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -257,14 +257,6 @@ class ApplicationController < ActionController::Base
       Rack::MiniProfiler.authorize_request
     end
 
-    def requires_parameters(*required)
-      required.each do |p|
-        raise Discourse::InvalidParameters.new(p) unless params.has_key?(p)
-      end
-    end
-
-    alias :requires_parameter :requires_parameters
-
     def store_incoming_links
       IncomingLink.add(request,current_user) unless request.xhr?
     end
diff --git a/app/controllers/clicks_controller.rb b/app/controllers/clicks_controller.rb
index 9515247a9..3bc093f01 100644
--- a/app/controllers/clicks_controller.rb
+++ b/app/controllers/clicks_controller.rb
@@ -3,14 +3,11 @@ class ClicksController < ApplicationController
   skip_before_filter :check_xhr
 
   def track
-    requires_parameter(:url)
-    if params[:topic_id].present? || params[:post_id].present?
-      args = {url: params[:url], ip: request.remote_ip}
-      args[:user_id] = current_user.id if current_user.present?
-      args[:post_id] = params[:post_id].to_i if params[:post_id].present?
-      args[:topic_id] = params[:topic_id].to_i if params[:topic_id].present?
+    params = track_params.merge(ip: request.remote_ip)
 
-      TopicLinkClick.create_from(args)
+    if params[:topic_id].present? || params[:post_id].present?
+      params.merge!({ user_id: current_user.id }) if current_user.present?
+      TopicLinkClick.create_from(params)
     end
 
     # Sometimes we want to record a link without a 302. Since XHR has to load the redirected
@@ -22,4 +19,11 @@ class ClicksController < ApplicationController
     end
   end
 
+  private
+
+    def track_params
+      params.require(:url)
+      params.permit(:url, :post_id, :topic_id, :redirect)
+    end
+
 end
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index c64631a0e..4616a76cd 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -25,7 +25,7 @@ class PostsController < ApplicationController
   end
 
   def create
-    requires_parameter(:post)
+    params.require(:post)
 
     post_creator = PostCreator.new(current_user,
                                    raw: params[:post][:raw],
@@ -55,7 +55,7 @@ class PostsController < ApplicationController
   end
 
   def update
-    requires_parameter(:post)
+    params.require(:post)
 
     post = Post.where(id: params[:id]).first
     post.image_sizes = params[:image_sizes] if params[:image_sizes].present?
@@ -138,7 +138,7 @@ class PostsController < ApplicationController
 
   def destroy_many
 
-    requires_parameters(:post_ids)
+    params.require(:post_ids)
 
     posts = Post.where(id: params[:post_ids])
     raise Discourse::InvalidParameters.new(:post_ids) if posts.blank?
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index 8bf2b3388..a7ba00121 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -7,7 +7,7 @@ class SearchController < ApplicationController
   end
 
   def query
-    requires_parameter(:term)
+    params.require(:term)
 
     search_args = {guardian: guardian}
     search_args[:type_filter] = params[:type_filter] if params[:type_filter].present?
@@ -35,6 +35,4 @@ class SearchController < ApplicationController
     render_json_dump(search.execute.as_json)
   end
 
-
-
 end
diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb
index e78be13b3..19ea421e8 100644
--- a/app/controllers/session_controller.rb
+++ b/app/controllers/session_controller.rb
@@ -6,7 +6,8 @@ class SessionController < ApplicationController
   skip_before_filter :redirect_to_login_if_required
 
   def create
-    requires_parameter(:login, :password)
+    params.require(:login)
+    params.require(:password)
 
     login = params[:login]
     login = login[1..-1] if login[0] == "@"
@@ -47,7 +48,7 @@ class SessionController < ApplicationController
   end
 
   def forgot_password
-    requires_parameter(:login)
+    params.require(:login)
 
     user = User.where('username_lower = :username or email = :email', username: params[:login].downcase, email: Email.downcase(params[:login])).first
     if user.present?
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index d9be939db..284b8175f 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -78,7 +78,8 @@ class TopicsController < ApplicationController
   end
 
   def similar_to
-    requires_parameters(:title, :raw)
+    params.require(:title)
+    params.require(:raw)
     title, raw = params[:title], params[:raw]
 
     raise Discourse::InvalidParameters.new(:title) if title.length < SiteSetting.min_title_similar_length
@@ -89,7 +90,8 @@ class TopicsController < ApplicationController
   end
 
   def status
-    requires_parameters(:status, :enabled)
+    params.require(:status)
+    params.require(:enabled)
 
     raise Discourse::InvalidParameters.new(:status) unless %w(visible closed pinned archived).include?(params[:status])
     @topic = Topic.where(id: params[:topic_id].to_i).first
@@ -115,7 +117,7 @@ class TopicsController < ApplicationController
   end
 
   def autoclose
-    requires_parameter(:auto_close_days)
+    raise Discourse::InvalidParameters.new(:auto_close_days) unless params.has_key?(:auto_close_days)
     @topic = Topic.where(id: params[:topic_id].to_i).first
     guardian.ensure_can_moderate!(@topic)
     @topic.auto_close_days = params[:auto_close_days]
@@ -136,7 +138,7 @@ class TopicsController < ApplicationController
   end
 
   def invite
-    requires_parameter(:user)
+    params.require(:user)
     topic = Topic.where(id: params[:topic_id]).first
     guardian.ensure_can_invite_to!(topic)
 
@@ -154,7 +156,7 @@ class TopicsController < ApplicationController
   end
 
   def merge_topic
-    requires_parameters(:destination_topic_id)
+    params.require(:destination_topic_id)
 
     topic = Topic.where(id: params[:topic_id]).first
     guardian.ensure_can_move_posts!(topic)
@@ -168,7 +170,7 @@ class TopicsController < ApplicationController
   end
 
   def move_posts
-    requires_parameters(:post_ids)
+    params.require(:post_ids)
 
     topic = Topic.where(id: params[:topic_id]).first
     guardian.ensure_can_move_posts!(topic)
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 4538c2e2c..82a185d2e 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -65,7 +65,7 @@ class UsersController < ApplicationController
   end
 
   def username
-    requires_parameter(:new_username)
+    params.require(:new_username)
 
     user = fetch_user_from_params
     guardian.ensure_can_edit!(user)
@@ -86,14 +86,14 @@ class UsersController < ApplicationController
   end
 
   def is_local_username
-    requires_parameter(:username)
+    params.require(:username)
     u = params[:username].downcase
     r = User.exec_sql('select 1 from users where username_lower = ?', u).values
     render json: {valid: r.length == 1}
   end
 
   def check_username
-    requires_parameter(:username)
+    params.require(:username)
 
     validator = UsernameValidator.new(params[:username])
     if !validator.valid_format?
@@ -259,7 +259,7 @@ class UsersController < ApplicationController
   end
 
   def change_email
-    requires_parameter(:email)
+    params.require(:email)
     user = fetch_user_from_params
     guardian.ensure_can_edit!(user)
     lower_email = Email.downcase(params[:email]).strip
diff --git a/app/models/admin_log.rb b/app/models/admin_log.rb
index 63198f70a..71dd383a1 100644
--- a/app/models/admin_log.rb
+++ b/app/models/admin_log.rb
@@ -2,8 +2,6 @@
 # like deleting users, changing site settings, etc.
 # Use the AdminLogger class to log records to this table.
 class AdminLog < ActiveRecord::Base
-  attr_accessible :action, :admin_id, :target_user_id, :details
-
   belongs_to :admin,        class_name: 'User'
   belongs_to :target_user,  class_name: 'User' # can be nil
 
diff --git a/app/models/cas_user_info.rb b/app/models/cas_user_info.rb
index 633566eb6..877188f28 100644
--- a/app/models/cas_user_info.rb
+++ b/app/models/cas_user_info.rb
@@ -1,5 +1,3 @@
 class CasUserInfo < ActiveRecord::Base
-  attr_accessible :email, :cas_user_id, :first_name, :gender, :last_name, :name, :user_id, :username, :link
   belongs_to :user
-
 end
diff --git a/app/models/category.rb b/app/models/category.rb
index 269038385..85943642c 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -1,6 +1,4 @@
 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/app/models/facebook_user_info.rb b/app/models/facebook_user_info.rb
index 2378fa45c..b0c0a8123 100644
--- a/app/models/facebook_user_info.rb
+++ b/app/models/facebook_user_info.rb
@@ -1,5 +1,4 @@
 class FacebookUserInfo < ActiveRecord::Base
-  attr_accessible :email, :facebook_user_id, :first_name, :gender, :last_name, :name, :user_id, :username, :link
   belongs_to :user
 end
 
diff --git a/app/models/invite.rb b/app/models/invite.rb
index b065a9ba9..9a5acc73b 100644
--- a/app/models/invite.rb
+++ b/app/models/invite.rb
@@ -1,7 +1,6 @@
 require_dependency 'trashable'
 
 class Invite < ActiveRecord::Base
-  include ActiveModel::ForbiddenAttributesProtection
   include Trashable
 
   belongs_to :user
diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 80244c4fd..9f6a05fd9 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -5,12 +5,9 @@ require_dependency 'trashable'
 class PostAction < ActiveRecord::Base
   class AlreadyActed < StandardError; end
 
-  include ActiveModel::ForbiddenAttributesProtection
   include RateLimiter::OnCreateRecord
   include Trashable
 
-  attr_accessible :post_action_type_id, :post_id, :user_id, :post, :user, :post_action_type, :message, :related_post_id, :staff_took_action
-
   belongs_to :post
   belongs_to :user
   belongs_to :post_action_type
diff --git a/app/models/post_action_type.rb b/app/models/post_action_type.rb
index c9cf02a79..542cc04ea 100644
--- a/app/models/post_action_type.rb
+++ b/app/models/post_action_type.rb
@@ -1,8 +1,6 @@
 require_dependency 'enum'
 
 class PostActionType < ActiveRecord::Base
-  attr_accessible :id, :is_flag, :name_key, :icon
-
   class << self
     def ordered
       order('position asc').all
diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index fc7c4b58e..b9ee60a47 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -6,8 +6,6 @@ class SiteSetting < ActiveRecord::Base
   validates_presence_of :name
   validates_presence_of :data_type
 
-  attr_accessible :description, :name, :value, :data_type
-
   # settings available in javascript under Discourse.SiteSettings
   client_setting(:title, "Discourse")
   client_setting(:logo_url, '/assets/d-logo-sketch.png')
diff --git a/app/models/topic_allowed_group.rb b/app/models/topic_allowed_group.rb
index 9b6cea599..c001b7f6c 100644
--- a/app/models/topic_allowed_group.rb
+++ b/app/models/topic_allowed_group.rb
@@ -1,7 +1,6 @@
 class TopicAllowedGroup < ActiveRecord::Base
   belongs_to :topic
   belongs_to :group
-  attr_accessible :group_id, :user_id
 
   validates_uniqueness_of :topic_id, scope: :group_id
 end
diff --git a/app/models/topic_allowed_user.rb b/app/models/topic_allowed_user.rb
index eff27af14..0f33f079a 100644
--- a/app/models/topic_allowed_user.rb
+++ b/app/models/topic_allowed_user.rb
@@ -1,7 +1,6 @@
 class TopicAllowedUser < ActiveRecord::Base
   belongs_to :topic
   belongs_to :user
-  attr_accessible :topic_id, :user_id
 
   validates_uniqueness_of :topic_id, scope: :user_id
 end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index f47d0a28e..de7226a53 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -5,8 +5,6 @@ require 's3'
 require 'local_store'
 
 class Upload < ActiveRecord::Base
-  include ActiveModel::ForbiddenAttributesProtection
-
   belongs_to :user
   belongs_to :topic
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 64d8c779b..a03c0c439 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -7,8 +7,6 @@ require_dependency 'discourse'
 require_dependency 'post_destroyer'
 
 class User < ActiveRecord::Base
-  attr_accessible :name, :username, :password, :email, :bio_raw, :website
-
   has_many :posts
   has_many :notifications
   has_many :topic_users
diff --git a/app/models/user_action.rb b/app/models/user_action.rb
index 123857fce..de7d6f1c3 100644
--- a/app/models/user_action.rb
+++ b/app/models/user_action.rb
@@ -2,7 +2,6 @@ class UserAction < ActiveRecord::Base
   belongs_to :user
   belongs_to :target_post, class_name: "Post"
   belongs_to :target_topic, class_name: "Topic"
-  attr_accessible :acting_user_id, :action_type, :target_topic_id, :target_post_id, :target_user_id, :user_id
 
   validates_presence_of :action_type
   validates_presence_of :user_id
diff --git a/app/models/user_open_id.rb b/app/models/user_open_id.rb
index 14246041e..860734081 100644
--- a/app/models/user_open_id.rb
+++ b/app/models/user_open_id.rb
@@ -1,6 +1,5 @@
 class UserOpenId < ActiveRecord::Base
   belongs_to :user
-  attr_accessible :email, :url, :user_id, :active
 
   validates_presence_of :email
   validates_presence_of :url
diff --git a/app/models/user_visit.rb b/app/models/user_visit.rb
index 6362c2092..9f236f43d 100644
--- a/app/models/user_visit.rb
+++ b/app/models/user_visit.rb
@@ -1,5 +1,4 @@
 class UserVisit < ActiveRecord::Base
-  attr_accessible :visited_at, :user_id
 
   # A list of visits in the last month by day
   def self.by_day(sinceDaysAgo=30)
diff --git a/config/application.rb b/config/application.rb
index 5d24e8684..bda758b20 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -114,6 +114,10 @@ module Discourse
     config.ember.ember_location = "#{Rails.root}/app/assets/javascripts/external_production/ember.js"
     config.ember.handlebars_location = "#{Rails.root}/app/assets/javascripts/external/handlebars-1.0.rc.3.js"
 
+    # Since we are using strong_parameters, we can disable and remove
+    # attr_accessible.
+    config.active_record.whitelist_attributes = false
+
     # So open id logs somewhere sane
     config.after_initialize do
       OpenID::Util.logger = Rails.logger
diff --git a/config/initializers/strong_parameters.rb b/config/initializers/strong_parameters.rb
new file mode 100644
index 000000000..c25ce4c2b
--- /dev/null
+++ b/config/initializers/strong_parameters.rb
@@ -0,0 +1 @@
+ActiveRecord::Base.send(:include, ActiveModel::ForbiddenAttributesProtection)
\ No newline at end of file
diff --git a/spec/controllers/admin/impersonate_controller_spec.rb b/spec/controllers/admin/impersonate_controller_spec.rb
index 835a96170..45a6aecb3 100644
--- a/spec/controllers/admin/impersonate_controller_spec.rb
+++ b/spec/controllers/admin/impersonate_controller_spec.rb
@@ -21,7 +21,7 @@ describe Admin::ImpersonateController do
     context 'create' do
 
       it 'requires a username_or_email parameter' do
-        lambda { xhr :put, :create }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :create }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'returns 404 when that user does not exist' do
diff --git a/spec/controllers/admin/site_settings_controller_spec.rb b/spec/controllers/admin/site_settings_controller_spec.rb
index 4724518ec..36d0b3982 100644
--- a/spec/controllers/admin/site_settings_controller_spec.rb
+++ b/spec/controllers/admin/site_settings_controller_spec.rb
@@ -26,7 +26,7 @@ describe Admin::SiteSettingsController do
     context 'update' do
 
       it 'requires a value parameter' do
-        lambda { xhr :put, :update, id: 'test_setting' }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :update, id: 'test_setting' }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'sets the value when the param is present' do
diff --git a/spec/controllers/clicks_controller_spec.rb b/spec/controllers/clicks_controller_spec.rb
index 9c7f252c7..1c127d922 100644
--- a/spec/controllers/clicks_controller_spec.rb
+++ b/spec/controllers/clicks_controller_spec.rb
@@ -6,7 +6,7 @@ describe ClicksController do
 
     context 'missing params' do
       it 'raises an error without the url param' do
-        lambda { xhr :get, :track, post_id: 123 }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :get, :track, post_id: 123 }.should raise_error(ActionController::ParameterMissing)
       end
 
       it "redirects to the url even without the topic_id or post_id params" do
@@ -24,7 +24,7 @@ describe ClicksController do
 
       context 'with a post_id' do
         it 'calls create_from' do
-          TopicLinkClick.expects(:create_from).with(url: 'http://discourse.org', post_id: 123, ip: '192.168.0.1')
+	  TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1')
           xhr :get, :track, url: 'http://discourse.org', post_id: 123
           response.should redirect_to("http://discourse.org")
         end
@@ -36,13 +36,13 @@ describe ClicksController do
         end
 
         it 'will pass the user_id to create_from' do
-          TopicLinkClick.expects(:create_from).with(url: 'http://discourse.org', post_id: 123, ip: '192.168.0.1')
+	  TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1')
           xhr :get, :track, url: 'http://discourse.org', post_id: 123
           response.should redirect_to("http://discourse.org")
         end
 
         it "doesn't redirect with the redirect=false param" do
-          TopicLinkClick.expects(:create_from).with(url: 'http://discourse.org', post_id: 123, ip: '192.168.0.1')
+	  TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1', 'redirect' => 'false')
           xhr :get, :track, url: 'http://discourse.org', post_id: 123, redirect: 'false'
           response.should_not be_redirect
         end
@@ -51,7 +51,7 @@ describe ClicksController do
 
       context 'with a topic_id' do
         it 'calls create_from' do
-          TopicLinkClick.expects(:create_from).with(url: 'http://discourse.org', topic_id: 789, ip: '192.168.0.1')
+	  TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'topic_id' => '789', 'ip' => '192.168.0.1')
           xhr :get, :track, url: 'http://discourse.org', topic_id: 789
           response.should redirect_to("http://discourse.org")
         end
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index 35f9c59d7..587f5b5a5 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -141,7 +141,7 @@ describe PostsController do
       let!(:post2) { Fabricate(:post, topic_id: post1.topic_id, user: poster, post_number: 3) }
 
       it "raises invalid parameters no post_ids" do
-        lambda { xhr :delete, :destroy_many }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :delete, :destroy_many }.should raise_error(ActionController::ParameterMissing)
       end
 
       it "raises invalid parameters with missing ids" do
@@ -193,7 +193,7 @@ describe PostsController do
         update_params.delete(:post)
         lambda {
           xhr :put, :update, update_params
-        }.should raise_error(Discourse::InvalidParameters)
+	}.should raise_error(ActionController::ParameterMissing)
       end
 
       it "raises an error when the user doesn't have permission to see the post" do
@@ -258,7 +258,7 @@ describe PostsController do
       let(:new_post) { Fabricate.build(:post, user: user) }
 
       it "raises an exception without a post parameter" do
-        lambda { xhr :post, :create }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :post, :create }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'calls the post creator' do
diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb
index 285f520e2..6ec78a592 100644
--- a/spec/controllers/session_controller_spec.rb
+++ b/spec/controllers/session_controller_spec.rb
@@ -13,7 +13,7 @@ describe SessionController do
       end
 
       it "raises an error when the login isn't present" do
-        lambda { xhr :post, :create }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :post, :create }.should raise_error(ActionController::ParameterMissing)
       end
 
       describe 'invalid password' do
@@ -114,7 +114,7 @@ describe SessionController do
   describe '.forgot_password' do
 
     it 'raises an error without a username parameter' do
-      lambda { xhr :post, :forgot_password }.should raise_error(Discourse::InvalidParameters)
+      lambda { xhr :post, :forgot_password }.should raise_error(ActionController::ParameterMissing)
     end
 
     context 'for a non existant username' do
diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb
index fe0ea2d0f..1247eeaa9 100644
--- a/spec/controllers/topics_controller_spec.rb
+++ b/spec/controllers/topics_controller_spec.rb
@@ -13,7 +13,7 @@ describe TopicsController do
       let(:topic) { p1.topic }
 
       it "raises an error without postIds" do
-        lambda { xhr :post, :move_posts, topic_id: topic.id, title: 'blah' }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :post, :move_posts, topic_id: topic.id, title: 'blah' }.should raise_error(ActionController::ParameterMissing)
       end
 
       it "raises an error when the user doesn't have permission to move the posts" do
@@ -106,7 +106,7 @@ describe TopicsController do
       let(:topic) { p1.topic }
 
       it "raises an error without destination_topic_id" do
-        lambda { xhr :post, :merge_topic, topic_id: topic.id }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :post, :merge_topic, topic_id: topic.id }.should raise_error(ActionController::ParameterMissing)
       end
 
       it "raises an error when the user doesn't have permission to merge" do
@@ -144,11 +144,11 @@ describe TopicsController do
     let(:raw) { 'this body is long enough to search for' }
 
     it "requires a title" do
-      -> { xhr :get, :similar_to, raw: raw }.should raise_error(Discourse::InvalidParameters)
+      -> { xhr :get, :similar_to, raw: raw }.should raise_error(ActionController::ParameterMissing)
     end
 
     it "requires a raw body" do
-      -> { xhr :get, :similar_to, title: title }.should raise_error(Discourse::InvalidParameters)
+      -> { xhr :get, :similar_to, title: title }.should raise_error(ActionController::ParameterMissing)
     end
 
     it "raises an error if the title length is below the minimum" do
@@ -218,11 +218,11 @@ describe TopicsController do
       end
 
       it 'requires the status parameter' do
-        lambda { xhr :put, :status, topic_id: @topic.id, enabled: true }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :status, topic_id: @topic.id, enabled: true }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'requires the enabled parameter' do
-        lambda { xhr :put, :status, topic_id: @topic.id, status: 'visible' }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :status, topic_id: @topic.id, status: 'visible' }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'raises an error with a status not in the whitelist' do
@@ -526,7 +526,7 @@ describe TopicsController do
       end
 
       it 'requires an email parameter' do
-        lambda { xhr :post, :invite, topic_id: @topic.id }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :post, :invite, topic_id: @topic.id }.should raise_error(ActionController::ParameterMissing)
       end
 
       describe 'without permission' do
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index da948eb8a..f073db951 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -178,7 +178,7 @@ describe UsersController do
       let!(:user) { log_in }
 
       it 'raises an error without an email parameter' do
-        lambda { xhr :put, :change_email, username: user.username }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :change_email, username: user.username }.should raise_error(ActionController::ParameterMissing)
       end
 
       it "raises an error if you can't edit the user" do
@@ -489,7 +489,7 @@ describe UsersController do
       let(:new_username) { "#{user.username}1234" }
 
       it 'raises an error without a new_username param' do
-        lambda { xhr :put, :username, username: user.username }.should raise_error(Discourse::InvalidParameters)
+	lambda { xhr :put, :username, username: user.username }.should raise_error(ActionController::ParameterMissing)
       end
 
       it 'raises an error when you don\'t have permission to change the user' do
@@ -518,7 +518,7 @@ describe UsersController do
     end
 
     it 'raises an error without a username parameter' do
-      lambda { xhr :get, :check_username }.should raise_error(Discourse::InvalidParameters)
+      lambda { xhr :get, :check_username }.should raise_error(ActionController::ParameterMissing)
     end
 
     shared_examples_for 'when username is unavailable locally' do