From 519427586aea71241601bab1e1ad4726d01ffded Mon Sep 17 00:00:00 2001
From: Serge Paquet <srgpqt@gmail.com>
Date: Sun, 2 Feb 2014 15:41:38 -0500
Subject: [PATCH] use Rails4-style strong parameters mass assignment protection

---
 Gemfile                                |  3 ---
 Gemfile.lock                           |  3 ---
 app/controllers/messages_controller.rb |  9 ++++++++-
 app/controllers/settings_controller.rb | 14 +++++++++++++-
 app/controllers/signup_controller.rb   | 10 +++++++++-
 app/controllers/stories_controller.rb  | 20 ++++++++++++--------
 app/models/comment.rb                  |  2 --
 app/models/invitation.rb               |  2 --
 app/models/invitation_request.rb       |  2 --
 app/models/message.rb                  |  2 --
 app/models/moderation.rb               |  2 --
 app/models/story.rb                    |  3 ---
 app/models/tag_filter.rb               |  2 --
 app/models/user.rb                     |  5 -----
 app/models/vote.rb                     |  2 --
 config/application.rb                  |  9 +++------
 config/environments/development.rb     |  3 ---
 config/environments/test.rb            |  3 ---
 18 files changed, 45 insertions(+), 51 deletions(-)

diff --git a/Gemfile b/Gemfile
index 2eb052d..82f6011 100644
--- a/Gemfile
+++ b/Gemfile
@@ -12,9 +12,6 @@ gem "mysql2", ">= 0.3.14"
 # NOTE: If you use PostgreSQL, you must still leave enabled the above mysql2 gem
 #       for Sphinx full text search to function.
 
-# Use Rails3-style mass assignment security
-gem "protected_attributes"
-
 # Use Uglifier as compressor for JavaScript assets
 gem "uglifier", ">= 1.3.0"
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 8fecf91..1cb4c20 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -58,8 +58,6 @@ GEM
       mini_portile (~> 0.5.0)
     oauth (0.4.7)
     polyglot (0.3.3)
-    protected_attributes (1.0.5)
-      activemodel (>= 4.0.1, < 5.0)
     rack (1.5.2)
     rack-test (0.6.2)
       rack (>= 1.0)
@@ -137,7 +135,6 @@ DEPENDENCIES
   mysql2 (>= 0.3.14)
   nokogiri
   oauth
-  protected_attributes
   rails (= 4.0.2)
   rdiscount
   rspec-rails (~> 2.6)
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index fb3bf7a..6c80d60 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -32,7 +32,7 @@ class MessagesController < ApplicationController
     @cur_url = "/messages"
     @title = "Messages"
 
-    @new_message = Message.new(params[:message])
+    @new_message = Message.new(message_params)
     @new_message.author_user_id = @user.id
 
     @direction = :out
@@ -95,6 +95,13 @@ class MessagesController < ApplicationController
   end
 
 private
+
+  def message_params
+    params.require(:message).permit(
+      :recipient_username, :subject, :body,
+    )
+  end
+
   def find_message
     if @message = Message.where(:short_id => params[:message_id] || params[:id]).first
       if (@message.author_user_id == @user.id ||
diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb
index dffa335..203976c 100644
--- a/app/controllers/settings_controller.rb
+++ b/app/controllers/settings_controller.rb
@@ -22,11 +22,23 @@ class SettingsController < ApplicationController
   def update
     @edit_user = @user.clone
 
-    if @edit_user.update_attributes(params[:user])
+    if @edit_user.update_attributes(user_params)
       flash.now[:success] = "Successfully updated settings."
       @user = @edit_user
     end
 
     render :action => "index"
   end
+
+private
+
+  def user_params
+    params.require(:user).permit(
+      :username, :email, :password, :password_confirmation, :about,
+      :email_replies, :email_messages, :email_mentions,
+      :pushover_replies, :pushover_messages, :pushover_mentions,
+      :pushover_user_key, :pushover_device, :pushover_sound,
+      :mailing_list_enabled,
+    )
+  end
 end
diff --git a/app/controllers/signup_controller.rb b/app/controllers/signup_controller.rb
index b6d436c..400d163 100644
--- a/app/controllers/signup_controller.rb
+++ b/app/controllers/signup_controller.rb
@@ -37,7 +37,7 @@ class SignupController < ApplicationController
 
     @title = "Signup"
 
-    @new_user = User.new(params[:user])
+    @new_user = User.new(user_params)
     @new_user.invited_by_user_id = @invitation.user_id
 
     if @new_user.save
@@ -53,4 +53,12 @@ class SignupController < ApplicationController
       render :action => "invited"
     end
   end
+
+private
+
+  def user_params
+    params.require(:user).permit(
+      :username, :email, :password, :password_confirmation, :about,
+    )
+  end
 end
diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb
index 079b915..d1e4ba8 100644
--- a/app/controllers/stories_controller.rb
+++ b/app/controllers/stories_controller.rb
@@ -12,9 +12,7 @@ class StoriesController < ApplicationController
     @title = "Submit Story"
     @cur_url = "/stories/new"
 
-    # we don't allow the url to be changed, so we have to set it manually
-    @story = Story.new(params[:story].reject{|k,v| k == "url" })
-    @story.url = params[:story][:url]
+    @story = Story.new(story_params)
     @story.user_id = @user.id
 
     if @story.valid? && !(@story.already_posted_story && !@story.seen_previous)
@@ -98,9 +96,7 @@ class StoriesController < ApplicationController
   end
 
   def preview
-    # we don't allow the url to be changed, so we have to set it manually
-    @story = Story.new(params[:story].reject{|k,v| k == "url" })
-    @story.url = params[:story][:url]
+    @story = Story.new(story_params)
     @story.user_id = @user.id
     @story.previewing = true
 
@@ -173,9 +169,10 @@ class StoriesController < ApplicationController
     @story.is_expired = false
     @story.editor_user_id = @user.id
 
-    @story.attributes = params[:story].except(:url)
     if @story.url_is_editable_by_user?(@user)
-      @story.url = params[:story][:url]
+      @story.attributes = story_params
+    else
+      @story.attributes = story_params.except(:url)
     end
 
     if @story.save
@@ -228,6 +225,13 @@ class StoriesController < ApplicationController
 
 private
 
+  def story_params
+    params.require(:story).permit(
+      :title, :url, :description, :moderation_reason, :seen_previous,
+      :tags_a => []
+    )
+  end
+
   def find_story
     Story.where(:short_id => params[:story_id]).first
   end
diff --git a/app/models/comment.rb b/app/models/comment.rb
index ce5d176..fb0870a 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -9,8 +9,6 @@ class Comment < ActiveRecord::Base
   has_one :moderation,
     :class_name => "Moderation"
 
-  attr_accessible :comment, :moderation_reason
-
   attr_accessor :current_vote, :previewing, :indent_level, :highlighted
 
   before_validation :on => :create do
diff --git a/app/models/invitation.rb b/app/models/invitation.rb
index ea99f8e..1977364 100644
--- a/app/models/invitation.rb
+++ b/app/models/invitation.rb
@@ -1,8 +1,6 @@
 class Invitation < ActiveRecord::Base
   belongs_to :user
 
-  attr_accessible nil
-
   validate do
     unless email.to_s.match(/\A[^@ ]+@[^ @]+\.[^ @]+\z/)
       errors.add(:email, "is not valid")
diff --git a/app/models/invitation_request.rb b/app/models/invitation_request.rb
index 1424b0b..5abe529 100644
--- a/app/models/invitation_request.rb
+++ b/app/models/invitation_request.rb
@@ -1,6 +1,4 @@
 class InvitationRequest < ActiveRecord::Base
-  attr_accessible nil
-
   validates :name, :presence => true
   validates :email, :format => { :with => /\A[^@ ]+@[^@ ]+\.[^@ ]+\Z/ }
   validates :memo, :format => { :with => /https?:\/\// }
diff --git a/app/models/message.rb b/app/models/message.rb
index 34274bb..6a783c1 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -11,8 +11,6 @@ class Message < ActiveRecord::Base
 
   attr_accessor :recipient_username
 
-  attr_accessible :recipient_username, :subject, :body
-
   validates_length_of :subject, :in => 1..150
   validates_length_of :body, :maximum => (64 * 1024)
 
diff --git a/app/models/moderation.rb b/app/models/moderation.rb
index 2761403..2b3fa7a 100644
--- a/app/models/moderation.rb
+++ b/app/models/moderation.rb
@@ -6,8 +6,6 @@ class Moderation < ActiveRecord::Base
   belongs_to :comment
   belongs_to :user
 
-  attr_accessible nil
-
   after_create :send_message_to_moderated
 
   def send_message_to_moderated
diff --git a/app/models/story.rb b/app/models/story.rb
index ec02aaf..2a19d59 100644
--- a/app/models/story.rb
+++ b/app/models/story.rb
@@ -20,9 +20,6 @@ class Story < ActiveRecord::Base
     :seen_previous
   attr_accessor :editor_user_id, :moderation_reason
 
-  attr_accessible :title, :description, :tags_a, :moderation_reason,
-    :seen_previous
-
   before_validation :assign_short_id,
     :on => :create
   before_save :log_moderation
diff --git a/app/models/tag_filter.rb b/app/models/tag_filter.rb
index f586af1..b528c07 100644
--- a/app/models/tag_filter.rb
+++ b/app/models/tag_filter.rb
@@ -1,6 +1,4 @@
 class TagFilter < ActiveRecord::Base
   belongs_to :tag
   belongs_to :user
-
-  attr_accessible nil
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index ef4050e..fa32d48 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -36,11 +36,6 @@ class User < ActiveRecord::Base
     end
   end
 
-  attr_accessible :username, :email, :password, :password_confirmation,
-    :about, :email_replies, :pushover_replies, :pushover_user_key,
-    :pushover_device, :pushover_sound, :email_messages, :pushover_messages,
-    :email_mentions, :pushover_mentions, :mailing_list_enabled, :delete_me
-
   before_save :check_session_token
   before_validation :on => :create do
     self.create_rss_token
diff --git a/app/models/vote.rb b/app/models/vote.rb
index 30aa09c..ea7f536 100644
--- a/app/models/vote.rb
+++ b/app/models/vote.rb
@@ -21,8 +21,6 @@ class Vote < ActiveRecord::Base
     "" => "Cancel",
   }
 
-  attr_accessible nil
-
   def self.votes_by_user_for_stories_hash(user, stories)
     votes = {}
 
diff --git a/config/application.rb b/config/application.rb
index 6aaf2f1..99c9cc2 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -24,15 +24,12 @@ module Lobsters
     # config.i18n.default_locale = :de
     config.i18n.enforce_available_locales = true
 
-    # Enforce whitelist mode for mass assignment.
-    # This will create an empty whitelist of attributes available for mass-assignment for all models
-    # in your app. As such, your models will need to explicitly whitelist or blacklist accessible
-    # parameters by using an attr_accessible or attr_protected declaration.
-    config.active_record.whitelist_attributes = true
-
     # Future Rails version will disable implicit joins, so we'll be prepared.
     config.active_record.disable_implicit_join_references = true
 
+    # Raise an exception when using mass assignment with unpermitted attributes
+    config.action_controller.action_on_unpermitted_parameters = :raise
+
     config.cache_store = :memory_store
   end
 end
diff --git a/config/environments/development.rb b/config/environments/development.rb
index de773c7..68406ec 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -22,9 +22,6 @@ Lobsters::Application.configure do
   # Raise an error on page load if there are pending migrations
   config.active_record.migration_error = :page_load
 
-  # Raise exception on mass assignment protection for Active Record models
-  config.active_record.mass_assignment_sanitizer = :strict
-
   # Debug mode disables concatenation and preprocessing of assets.
   # This option may cause significant delays in view rendering with a large
   # number of complex assets.
diff --git a/config/environments/test.rb b/config/environments/test.rb
index 04537e5..1092b6a 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -31,9 +31,6 @@ Lobsters::Application.configure do
   # ActionMailer::Base.deliveries array.
   config.action_mailer.delivery_method = :test
 
-  # Raise exception on mass assignment protection for Active Record models.
-  config.active_record.mass_assignment_sanitizer = :strict
-
   # Print deprecation notices to the stderr.
   config.active_support.deprecation = :stderr
 end