move to strong_parameters of rails4.

Gemfile

@@ -4,8 +4,7 @@ gem 'rails', '4.0.0.rc1'
 gem 'sass-rails', '4.0.0.rc1'
 gem 'coffee-rails', '~>4.0'
 
-# add these gems to help with the transition:
-gem 'protected_attributes'
+# todo: remove xml api 
 gem 'actionpack-xml_parser', github: 'rails/actionpack-xml_parser'
 
 # See https://github.com/sstephenson/execjs#readme for more supported runtimes

Gemfile.lock

@@ -105,8 +105,6 @@ GEM
     mysql2 (0.3.11)
     nokogiri (1.5.9)
     polyglot (0.3.3)
-    protected_attributes (1.0.1)
-      activemodel (>= 4.0.0.beta, < 5.0)
     rack (1.5.2)
     rack-mini-profiler (0.1.26)
       rack (>= 1.1.3)
@@ -209,7 +207,6 @@ DEPENDENCIES
   jquery-rails
   mocha
   mysql2
-  protected_attributes
   rack-mini-profiler
   rails (= 4.0.0.rc1)
   rails_autolink

app/controllers/application_controller.rb

@@ -6,8 +6,9 @@ require_dependency "login_system"
 require_dependency "tracks/source_view"
 
 class ApplicationController < ActionController::Base
-
-  protect_from_forgery
+  # Prevent CSRF attacks by raising an exception.
+  # For APIs, you may want to use :null_session instead.
+  protect_from_forgery with: :exception
 
   include LoginSystem
   helper_method :current_user, :prefs, :format_date

app/controllers/contexts_controller.rb

@@ -8,7 +8,7 @@ class ContextsController < ApplicationController
   prepend_before_filter :login_or_feed_token_required, :only => [:index]
 
   def index
-    @all_contexts = current_user.contexts
+    @all_contexts    = current_user.contexts
     @active_contexts = current_user.contexts.active
     @hidden_contexts = current_user.contexts.hidden
     @closed_contexts = current_user.contexts.closed
@@ -69,7 +69,7 @@ class ContextsController < ApplicationController
       render_failure "Expected post format is valid xml like so: <context><name>context name</name></context>.", 400
       return
     end
-    @context = current_user.contexts.build(params['context'])
+    @context = current_user.contexts.build(context_params)
     @context.hide! if params['context_state'] && params['context_state']['hide'] == '1'
     @saved = @context.save
     @context_not_done_counts = { @context.id => 0 }
@@ -93,7 +93,7 @@ class ContextsController < ApplicationController
   def update
     process_params_for_update
 
-    @context.attributes = params["context"]
+    @context.attributes = context_params
     @saved = @context.save
     @state_saved = set_state_for_update(@new_state) 
     @saved = @saved && @state_saved
@@ -161,6 +161,12 @@ class ContextsController < ApplicationController
     all_done_todos_for current_user.contexts.find(params[:id])
   end
 
+  private
+
+  def context_params
+    params.require(:context).permit(:name, :position, :state)
+  end
+
   protected
 
   def update_state_counts

app/controllers/notes_controller.rb

@@ -24,7 +24,7 @@ class NotesController < ApplicationController
 
   def create
     @note = current_user.notes.build
-    @note.attributes = params["note"]
+    @note.attributes = note_params
 
     @saved = @note.save
 
@@ -45,7 +45,7 @@ class NotesController < ApplicationController
 
   def update
     @note = current_user.notes.find(params['id'])
-    @note.attributes = params["note"]
+    @note.attributes = note_params
     @saved = @note.save
     respond_to do |format|
       format.html
@@ -69,4 +69,10 @@ class NotesController < ApplicationController
     @source_view = params['_source_view'] || 'note'
   end
 
+  private
+
+  def note_params
+    params.require(:note).permit(:project_id, :body)
+  end
+
 end

app/controllers/preferences_controller.rb

@@ -9,8 +9,8 @@ class PreferencesController < ApplicationController
   def update
     @prefs = current_user.prefs
     @user = current_user
-    user_updated = current_user.update_attributes(params['user'])
-    prefs_updated = current_user.preference.update_attributes(params['prefs'])
+    user_updated = current_user.update_attributes(user_params)
+    prefs_updated = current_user.preference.update_attributes(prefs_params)
     if (user_updated && prefs_updated)
       if !params['user']['password'].blank? # password updated?
         logout_user t('preferences.password_changed')
@@ -33,6 +33,20 @@ class PreferencesController < ApplicationController
 
 private
 
+  def prefs_params
+    params.require(:prefs).permit(
+      :date_format, :week_starts, :show_number_completed, 
+      :show_completed_projects_in_sidebar, :show_hidden_contexts_in_sidebar, 
+      :staleness_starts, :due_style, :locale, :title_date_format, :time_zone, 
+      :show_hidden_projects_in_sidebar, :show_project_on_todo_done, 
+      :review_period, :refresh, :verbose_action_descriptors, 
+      :mobile_todos_per_page, :sms_email, :sms_context_id)
+  end
+
+  def user_params
+    params.require(:user).permit(:login, :first_name, :last_name, :password_confirmation, :password, :auth_type, :open_id_url)
+  end
+
   # Display notification if preferences are successful updated
   def preference_updated
     notify :notice, t('preferences.updated')

app/controllers/projects_controller.rb

@@ -173,7 +173,7 @@ class ProjectsController < ApplicationController
       render_failure "Expected post format is valid xml like so: <project><name>project name</name></project>.", 400
       return
     end
-    @project = current_user.projects.build(params['project'])
+    @project = current_user.projects.build(project_params)
     @go_to_project = params['go_to_project']
     @saved = @project.save
     @project_not_done_counts = { @project.id => 0 }
@@ -212,7 +212,7 @@ class ProjectsController < ApplicationController
       params['project']['name'] = params['value']
     end
 
-    @project.attributes = params['project']
+    @project.attributes = project_params
     @saved = @project.save
     if @saved
       @project.transition_to(@new_state) if @state_changed
@@ -342,4 +342,10 @@ class ProjectsController < ApplicationController
     end
   end
 
+  private
+
+  def project_params
+    params.require(:project).permit(:name, :position, :user_id, :description, :state, :default_context_id, :default_tags)
+  end
+
 end

app/controllers/recurring_todos_controller.rb

@@ -89,7 +89,7 @@ class RecurringTodosController < ApplicationController
       params["recurring_todo"]["weekly_return_"+day]=' ' if params["recurring_todo"]["weekly_return_"+day].nil?
     end
 
-    @saved = @recurring_todo.update_attributes params["recurring_todo"]
+    @saved = @recurring_todo.update_attributes recurring_todo_params
 
     respond_to do |format|
       format.js
@@ -97,7 +97,7 @@ class RecurringTodosController < ApplicationController
   end
 
   def create
-    p = RecurringTodoCreateParamsHelper.new(params)
+    p = RecurringTodoCreateParamsHelper.new(params, recurring_todo_params)
     p.attributes['end_date']=parse_date_per_user_prefs(p.attributes['end_date'])
     p.attributes['start_from']=parse_date_per_user_prefs(p.attributes['start_from'])
 
@@ -207,9 +207,9 @@ class RecurringTodosController < ApplicationController
 
   class RecurringTodoCreateParamsHelper
 
-    def initialize(params)
+    def initialize(params, recurring_todo_params)
       @params = params['request'] || params
-      @attributes = params['request'] && params['request']['recurring_todo']  || params['recurring_todo']
+      @attributes = recurring_todo_params
 
       # make sure all selectors (recurring_period, recurrence_selector,
       # daily_selector, monthly_selector and yearly_selector) are first in hash
@@ -259,6 +259,25 @@ class RecurringTodosController < ApplicationController
 
   private
 
+  def recurring_todo_params
+    params.require(:recurring_todo).permit(
+      # model attributes
+      :context_id, :project_id, :description, :notes, :state, :start_from, 
+      :ends_on, :end_date, :number_of_occurences, :occurences_count, :target, 
+      :show_from_delta, :recurring_period, :recurrence_selector, :every_other1, 
+      :every_other2, :every_other3, :every_day, :only_work_days, :every_count, 
+      :weekday, :show_always,
+      # form attributes
+      :recurring_period, :daily_selector, :monthly_selector, :yearly_selector, 
+      :recurring_target, :daily_every_x_days, :monthly_day_of_week, 
+      :monthly_every_x_day, :monthly_every_x_month2, :monthly_every_x_month, 
+      :monthly_every_xth_day, :recurring_show_days_before, 
+      :recurring_show_always, :weekly_every_x_week, :weekly_return_monday,
+      :yearly_day_of_week, :yearly_every_x_day, :yearly_every_xth_day, 
+      :yearly_month_of_year2, :yearly_month_of_year
+      )
+  end
+
   def init
     @days_of_week = []
     0.upto 6 do |i|

app/controllers/todos/todo_create_params_helper.rb

@@ -5,7 +5,7 @@ module Todos
 
     def initialize(params, user)
       set_params(params)
-      filter_attributes
+      filter_attributes(params)
       filter_tags
       filter_starred
 
@@ -20,8 +20,12 @@ module Todos
       @params = params['request'] || params
     end
 
-    def filter_attributes
-      @attributes = @params['request'] && @params['request']['todo'] || @params['todo']
+    def filter_attributes(params)
+      if params[:request]
+        @attributes = todo_params(params[:request])
+      else
+        @attributes = todo_params(params)
+      end
       @attributes = {} if @attributes.nil?  # make sure there is at least an empty hash
 		end
 
@@ -116,6 +120,24 @@ module Todos
 
     private
 
+    def todo_params(params)
+      # keep :predecessor_dependencies from being filterd (for XML API). 
+      # The permit cannot handle multiple precessors
+      deps = params[:todo][:predecessor_dependencies][:predecessor] if params[:todo][:predecessor_dependencies]
+
+      filtered = params.require(:todo).permit(
+        :context_id, :project_id, :description, :notes, 
+        :due, :show_from, :state, 
+        # XML API
+        :tags => [:tag => [:name]],
+        :context => [:name],
+        :project => [:name])
+
+      # add back :predecessor_dependencies
+      filtered[:predecessor_dependencies] = {:predecessor => deps } unless deps.nil?
+      filtered
+    end
+
     def find_or_create_group(group_type, set, name)
       return set_id_by_name(group_type, set, name) if specified_by_name?(group_type)
       return set_id_by_id_string(group_type, set, @attributes["#{group_type}_id"]) if specified_by_id?(group_type)

app/controllers/todos_controller.rb

@@ -428,7 +428,7 @@ class TodosController < ApplicationController
     determine_changes_by_this_update
     determine_remaining_in_container_count( (@context_changed || @project_changed) ? @original_item : @todo)
     determine_down_count
-    determine_deferred_tag_count(params['_tag_name']) if source_view_is(:tag)
+    determine_deferred_tag_count(sanitize(params['_tag_name'])) if source_view_is(:tag)
 
     @todo.touch_predecessors if @original_item_description != @todo.description
 
@@ -1219,7 +1219,10 @@ class TodosController < ApplicationController
   end
 
   def update_attributes_of_todo
-    @todo.attributes = params["todo"]
+    # TODO: duplication with todo_create_params_helper
+    @todo.attributes = params.require(:todo).permit(
+        :context_id, :project_id, :description, :notes, 
+        :due, :show_from, :state)
   end
 
   def determine_changes_by_this_update

app/controllers/users_controller.rb

@@ -78,7 +78,7 @@ class UsersController < ApplicationController
           return
         end
 
-        user = User.new(params['user'])
+        user = User.new(user_params)
 
         unless user.valid?
           session['new_user'] = user
@@ -108,8 +108,8 @@ class UsersController < ApplicationController
           render_failure "Expected post format is valid xml like so: <user><login>username</login><password>abc123</password></user>.", 400
           return
         end
-        user = User.new(params[:user])
-        user.password_confirmation = params[:user][:password]
+        user = User.new(user_params)
+        user.password_confirmation = user_params[:password]
         saved = user.save
         unless user.new_record?
           render :text => t('users.user_created'), :status => 200
@@ -147,7 +147,7 @@ class UsersController < ApplicationController
 
   def update_password
     # is used for focing password change after sha->bcrypt upgrade
-    current_user.change_password(params[:user][:password], params[:user][:password_confirmation])
+    current_user.change_password(user_params[:password], user_params[:password_confirmation])
     notify :notice, t('users.password_updated')
     redirect_to preferences_path
   rescue Exception => error
@@ -160,7 +160,7 @@ class UsersController < ApplicationController
   end
 
   def update_auth_type
-    current_user.auth_type = params[:user][:auth_type]
+    current_user.auth_type = user_params[:auth_type]
     if current_user.save
       notify :notice, t('users.auth_type_updated')
       redirect_to preferences_path
@@ -179,6 +179,10 @@ class UsersController < ApplicationController
 
   private
 
+  def user_params
+    params.require(:user).permit(:login, :first_name, :last_name, :password_confirmation, :password, :auth_type, :open_id_url)
+  end
+
   def get_new_user
     if session['new_user']
       user = session['new_user']

app/models/context.rb

@@ -1,7 +1,6 @@
 class Context < ActiveRecord::Base
 
-  has_many :todos, -> { order("todos.due IS NULL, todos.due ASC, todos.created_at ASC").includes(:project) }, :dependent => :delete_all
-    
+  has_many :todos, -> { order("todos.due IS NULL, todos.due ASC, todos.created_at ASC").includes(:project) }, :dependent => :delete_all    
   has_many :recurring_todos, :dependent => :delete_all
   belongs_to :user
 
@@ -34,8 +33,6 @@ class Context < ActiveRecord::Base
     end
   end
 
-  attr_protected :user
-
   validates_presence_of :name, :message => "context must have a name"
   validates_length_of :name, :maximum => 255, :message => "context name must be less than 256 characters"
   validates_uniqueness_of :name, :message => "already exists", :scope => "user_id"

app/models/note.rb

@@ -2,7 +2,5 @@ class Note < ActiveRecord::Base
   belongs_to :user
   belongs_to :project
 
-  attr_protected :user
-
   scope :with_body, lambda { |terms| where("body LIKE ?", terms) }
 end

app/models/preference.rb

@@ -1,11 +1,6 @@
 class Preference < ActiveRecord::Base
   belongs_to :user
   belongs_to :sms_context, :class_name => 'Context'
-
-  attr_accessible :date_format, :week_starts, :show_number_completed, :show_completed_projects_in_sidebar,
-    :show_hidden_contexts_in_sidebar, :staleness_starts, :due_style, :locale,
-    :title_date_format, :time_zone, :show_hidden_projects_in_sidebar, :show_project_on_todo_done, :review_period,
-    :refresh, :verbose_action_descriptors, :mobile_todos_per_page, :sms_email, :sms_context_id
     
   def self.due_styles
     { :due_in_n_days => 0, :due_on => 1}

app/models/project.rb

@@ -40,7 +40,6 @@ class Project < ActiveRecord::Base
     end
   end
 
-  attr_protected :user
   attr_accessor :cached_note_count
 
   def self.null_object
@@ -123,7 +122,11 @@ class Project < ActiveRecord::Base
   end
 
   def name=(value)
-    self[:name] = value.gsub(/\s{2,}/, " ").strip
+    if value
+      self[:name] = value.gsub(/\s{2,}/, " ").strip
+    else
+      self[:name] = nil
+    end
   end
 
   def new_record_before_save?

app/models/recurring_todo.rb

@@ -1,6 +1,4 @@
 class RecurringTodo < ActiveRecord::Base
-  attr_protected :user
-
   belongs_to :context
   belongs_to :project
   belongs_to :user

app/models/tag.rb

@@ -10,8 +10,6 @@ class Tag < ActiveRecord::Base
   # rescue the ActiveRecord database constraint errors instead.
   validates_presence_of :name
   validates_uniqueness_of :name, :case_sensitive => false
-
-  attr_accessible :name
   
   before_create :before_create
   

app/models/tagging.rb

@@ -2,9 +2,7 @@
 # The Tagging join model.
 
 class Tagging < ActiveRecord::Base
-  
-  attr_accessible :taggable_id, :tag, :taggable_type
- 
+   
   belongs_to :tag
   belongs_to :taggable, :polymorphic => true, :touch => true
   

app/models/todo.rb

@@ -66,7 +66,7 @@ class Todo < ActiveRecord::Base
 
   aasm :column => :state do
 
-    state :active #, :enter => Proc.new{|t| puts "$$$ activating #{t.aasm_current_state} - #{t.show_from} "}
+    state :active 
     state :project_hidden
     state :completed, :before_enter => Proc.new { |t| t.completed_at = Time.zone.now }, :before_exit => Proc.new { |t| t.completed_at = nil}
     state :deferred,  :after_exit => Proc.new { |t| t[:show_from] = nil }
@@ -103,8 +103,6 @@ class Todo < ActiveRecord::Base
     end
   end
 
-  attr_protected :user
-
   # Description field can't be empty, and must be < 100 bytes Notes must be <
   # 60,000 bytes (65,000 actually, but I'm being cautious)
   validates_presence_of :description

app/models/user.rb

@@ -4,9 +4,7 @@ require 'bcrypt'
 class User < ActiveRecord::Base
   # Virtual attribute for the unencrypted password
   attr_accessor :password
-  attr_protected :is_admin # don't allow mass-assignment for this
   
-  attr_accessible :login, :first_name, :last_name, :password_confirmation, :password, :auth_type, :open_id_url
   #for will_paginate plugin
   cattr_accessor :per_page
   @@per_page = 5

test/controllers/notes_controller_test.rb

@@ -2,13 +2,45 @@ require File.expand_path(File.dirname(__FILE__) + '/../test_helper')
 
 class NotesControllerTest < ActionController::TestCase
   
-  def setup
-  end
-
   def test_get_notes_page
     login_as :admin_user
     get :index
     assert_response 200
   end
+
+  def test_create_new_note
+    login_as :admin_user
+    project = users(:admin_user).projects.first
+    count = users(:admin_user).notes.count
+
+  	post :create, note: {body: "test note", project_id: project.id}, format: :js
+
+  	assert_response 200
+  	assert assigns['saved'], "@saved should be true"
+  	assert count+1, users(:admin_user).notes.reload.count
+  end
+
+  def test_update_note
+  	login_as :admin_user
+
+  	note = users(:admin_user).notes.first
+
+  	assert_not_equal "test", note.body
+  	post :update, id: note.id, note: {body: "test"}, format: :js
+  	assert_equal "test", note.reload.body
+  end
+
+  def test_destroy_note
+  	login_as :admin_user
+
+  	note = users(:admin_user).notes.first
+    count = users(:admin_user).notes.count
+
+    post :destroy, id: note.id, format: :js
+
+    old_note = users(:admin_user).notes.where(id: note.id).first
+    assert_nil old_note
+    assert count-1, users(:admin_user).notes.reload.count
+  end
   
 end

test/integration/context_xml_api_test.rb

@@ -19,7 +19,7 @@ class ContextXmlApiTest < ActionDispatch::IntegrationTest
   end
   
   def test_fails_gracefully_with_invalid_xml_format
-    authenticated_post_xml_to_context_create "<context_name></context_name>"
+    authenticated_post_xml_to_context_create "<context><name></name></context>"
     assert_responses_with_error 'Name context must have a name'
   end
     

test/integration/project_xml_api_test.rb

@@ -21,7 +21,7 @@ class ProjectXmlApiTest < ActionDispatch::IntegrationTest
  end
     
   def test_fails_with_invalid_xml_format2
-    authenticated_post_xml_to_project_create "<request><project></project></request>"
+    authenticated_post_xml_to_project_create "<project><name></name></project>"
     assert_responses_with_error 'Name project must have a name'
   end
   

test/integration/todo_xml_api_test.rb

@@ -61,7 +61,7 @@ class TodoXmlApiTest < ActionDispatch::IntegrationTest
     assert_response :success
     todo = @user.todos.where(:description => "this will succeed 2.0").first
     assert_not_nil todo
-    assert !todo.uncompleted_predecessors.empty?
+    assert !todo.uncompleted_predecessors.empty?, "should have predecessors"
   end
 
   def test_post_create_todo_with_single_dependency