Merge branch 'jflemingprod-feature/4143-port_to_strong_parameters' into develop

Gemfile

@@ -65,6 +65,11 @@ gem 'redcarpet',      '3.0.0'
 gem 'roxml',          '3.1.6'
 gem 'ruby-oembed',    '0.8.8'
 
+
+# Please remove when migrating to Rails 4
+gem 'strong_parameters'
+
+
 # Services
 
 gem 'omniauth',          '1.1.4'

Gemfile.lock

@@ -380,6 +380,10 @@ GEM
       multi_json (~> 1.0)
       rack (~> 1.0)
       tilt (~> 1.1, != 1.3.0)
+    strong_parameters (0.2.1)
+      actionpack (~> 3.0)
+      activemodel (~> 3.0)
+      railties (~> 3.0)
     subexec (0.2.3)
     temple (0.6.6)
     thor (0.18.1)
@@ -486,6 +490,7 @@ DEPENDENCIES
   sinon-rails (= 1.7.3)
   slim (= 1.3.9)
   spork (= 1.0.0rc3)
+  strong_parameters
   timecop (= 0.6.1)
   twitter (= 4.8.1)
   typhoeus (= 0.6.3)

app/controllers/aspects_controller.rb

@@ -10,7 +10,7 @@ class AspectsController < ApplicationController
              :json
 
   def create
-    @aspect = current_user.aspects.build(params[:aspect])
+    @aspect = current_user.aspects.build(aspect_params)
     aspecting_person_id = params[:aspect][:person_id]
 
     if @aspect.save
@@ -92,7 +92,7 @@ class AspectsController < ApplicationController
   def update
     @aspect = current_user.aspects.where(:id => params[:id]).first
 
-    if @aspect.update_attributes!(params[:aspect])
+    if @aspect.update_attributes!(aspect_params)
       flash[:notice] = I18n.t 'aspects.update.success', :name => @aspect.name
     else
       flash[:error] = I18n.t 'aspects.update.failure', :name => @aspect.name
@@ -121,4 +121,8 @@ class AspectsController < ApplicationController
       @contact = current_user.share_with(@person, @aspect)
     end
   end
+
+  def aspect_params
+    params.require(:aspect).permit(:name, :contacts_visible, :order_id)
+  end
 end

app/controllers/blocks_controller.rb

@@ -4,7 +4,7 @@ class BlocksController < ApplicationController
   respond_to :html, :json
 
   def create
-    block = current_user.blocks.new(params[:block])
+    block = current_user.blocks.new(block_params)
 
     if block.save
       disconnect_if_contact(block.person)
@@ -39,4 +39,8 @@ class BlocksController < ApplicationController
       current_user.disconnect(contact, :force => true)
     end
   end
+
+  def block_params
+    params.require(:block).permit(:person_id)
+  end
 end

app/controllers/conversations_controller.rb

@@ -34,13 +34,14 @@ class ConversationsController < ApplicationController
       person_ids = Contact.where(:id => params[:contact_ids].split(',')).map(&:person_id)
     end
 
-    params[:conversation][:participant_ids] = [*person_ids] | [current_user.person_id]
+    @conversation = Conversation.new
-    params[:conversation][:author] = current_user.person
+    @conversation.subject = params[:conversation][:subject]
-    message_text = params[:conversation].delete(:text)
+    @conversation.participant_ids = [*person_ids] | [current_user.person_id]
-    params[:conversation][:messages_attributes] = [ {:author => current_user.person, :text => message_text }]
+    @conversation.author = current_user.person
+    message_text = params[:conversation][:text]
+    @conversation.messages_attributes = [ {:author => current_user.person, :text => message_text }]
 
     @response = {}
-    @conversation = Conversation.new(params[:conversation])
     if person_ids.present? && @conversation.save
       Postzord::Dispatcher.build(current_user, @conversation).post
       @response[:success] = true

app/controllers/invitations_controller.rb

@@ -50,7 +50,7 @@ class InvitationsController < ApplicationController
   end
 
   def create
-    emails = params[:email_inviter][:emails].split(',').map(&:strip).uniq
+    emails = inviter_params[:emails].split(',').map(&:strip).uniq
 
     valid_emails, invalid_emails = emails.partition { |email| valid_email?(email) }
 
@@ -60,8 +60,7 @@ class InvitationsController < ApplicationController
     unless valid_emails.empty?
       Workers::Mail::InviteEmail.perform_async(valid_emails.join(','),
                                                current_user.id,
-                                               params[:email_inviter])
+                                               inviter_params)
-
     end
 
     if emails.empty?
@@ -99,4 +98,8 @@ class InvitationsController < ApplicationController
     session[key] = nil
     return value
   end
+
+  def inviter_params
+    params.require(:email_inviter).permit(:message, :locale, :emails)
+  end
 end

app/controllers/photos_controller.rb

@@ -41,7 +41,7 @@ class PhotosController < ApplicationController
   def create
     rescuing_photo_errors do
       if remotipart_submitted?
-        @photo = current_user.build_post(:photo, params[:photo])
+        @photo = current_user.build_post(:photo, photo_params)
         if @photo.save
           respond_to do |format|
             format.json { render :json => {"success" => true, "data" => @photo.as_api_response(:backbone)} }
@@ -114,7 +114,7 @@ class PhotosController < ApplicationController
   def update
     photo = current_user.photos.where(:id => params[:id]).first
     if photo
-      if current_user.update_post( photo, params[:photo] )
+      if current_user.update_post( photo, photo_params )
         flash.now[:notice] = I18n.t 'photos.update.notice'
         respond_to do |format|
           format.js{ render :json => photo, :status => 200 }
@@ -133,6 +133,10 @@ class PhotosController < ApplicationController
 
   private
 
+  def photo_params
+    params.require(:photo).permit(:public, :text, :pending, :user_file, :image_url, :aspect_ids, :set_profile_photo)
+  end
+
   def file_handler(params)
     # For XHR file uploads, request.params[:qqfile] will be the path to the temporary file
     # For regular form uploads (such as those made by Opera), request.params[:qqfile] will be an UploadedFile which can be returned unaltered.

app/controllers/profiles_controller.rb

@@ -33,7 +33,7 @@ class ProfilesController < ApplicationController
 
   def update
     # upload and set new profile photo
-    @profile_attrs = params[:profile] || {}
+    @profile_attrs = profile_params
     
     munge_tag_string
 
@@ -78,4 +78,8 @@ class ProfilesController < ApplicationController
     end
     @profile_attrs[:tag_string] = (params[:tags]) ? params[:tags].gsub(',',' ') : ""
   end
+
+  def profile_params
+    params.require(:profile).permit(:first_name, :last_name, :gender, :bio, :location, :searchable, :tag_string, :nsfw, :date => [:year, :month, :day]) || {}
+  end
 end

app/controllers/registrations_controller.rb

@@ -9,7 +9,7 @@ class RegistrationsController < Devise::RegistrationsController
   before_filter -> { @css_framework = :bootstrap }, only: [:new]
 
   def create
-    @user = User.build(params[:user])
+    @user = User.build(user_params)
     @user.process_invite_acceptence(invite) if invite.present?
 
     if @user.save
@@ -54,4 +54,8 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   helper_method :invite
+
+  def user_params
+    params.require(:user).permit(:username, :email, :getting_started, :password, :password_confirmation, :language, :disable_mail, :invitation_service, :invitation_identifier, :show_community_spotlight_in_stream, :auto_follow_back, :auto_follow_back_aspect_id, :remember_me)
+  end
 end

app/controllers/users_controller.rb

@@ -24,7 +24,7 @@ class UsersController < ApplicationController
     password_changed = false
     @user = current_user
 
-    if u = params[:user]
+    if u = user_params
       u.delete(:password) if u[:password].blank?
       u.delete(:password_confirmation) if u[:password].blank? and u[:password_confirmation].blank?
       u.delete(:language) if u[:language].blank?
@@ -125,7 +125,8 @@ class UsersController < ApplicationController
 
   def getting_started_completed
     user = current_user
-    user.update_attributes(:getting_started => false)
+    user.getting_started = false
+    user.save
     redirect_to stream_path
   end
 
@@ -157,4 +158,10 @@ class UsersController < ApplicationController
     end
     redirect_to edit_user_path
   end
+
+  private
+
+  def user_params
+    params.fetch(:user).permit(:username, :email, :current_password, :password, :password_confirmation, :language, :disable_mail, :invitation_service, :invitation_identifier, :show_community_spotlight_in_stream, :auto_follow_back, :auto_follow_back_aspect_id, :remember_me, :email_preferences => [:also_commented, :mentioned, :comment_on_post, :private_message, :started_sharing, :liked, :reshared])
+  end
 end

app/models/account_deletion.rb

@@ -9,8 +9,6 @@ class AccountDeletion < ActiveRecord::Base
   belongs_to :person
   after_create :queue_delete_account
 
-  attr_accessible :person
-
   xml_name :account_deletion
   xml_attr :diaspora_handle
 

app/models/aspect.rb

@@ -16,8 +16,6 @@ class Aspect < ActiveRecord::Base
 
   validates_uniqueness_of :name, :scope => :user_id, :case_sensitive => false
 
-  attr_accessible :name, :contacts_visible, :order_id
-
   before_validation do
     name.strip!
   end

app/models/invitation.rb

@@ -9,8 +9,6 @@ class Invitation < ActiveRecord::Base
   belongs_to :recipient, :class_name => 'User'
   belongs_to :aspect
 
-  attr_accessible :sender, :recipient, :aspect, :language, :service, :identifier, :admin, :message
-
   before_validation :set_email_as_default_service
 
  # before_create :share_with_exsisting_user, :if => :recipient_id?

app/models/o_embed_cache.rb

@@ -1,6 +1,5 @@
 class OEmbedCache < ActiveRecord::Base
   serialize :data
-  attr_accessible :url
   validates :data, :presence => true
 
   has_many :posts

app/models/photo.rb

@@ -41,7 +41,6 @@ class Photo < ActiveRecord::Base
   validates_associated :status_message
   delegate :author_name, to: :status_message, prefix: true
 
-  attr_accessible :text, :pending
   validate :ownership_of_status_message
 
   before_destroy :ensure_user_picture
@@ -69,7 +68,7 @@ class Photo < ActiveRecord::Base
   end
 
   def self.diaspora_initialize(params = {})
-    photo = self.new params.to_hash
+    photo = self.new params.to_hash.slice(:text, :pending)
     photo.author = params[:author]
     photo.public = params[:public] if params[:public]
     photo.pending = params[:pending] if params[:pending]

app/models/post.rb

@@ -116,7 +116,7 @@ class Post < ActiveRecord::Base
   #############
 
   def self.diaspora_initialize(params)
-    new_post = self.new params.to_hash
+    new_post = self.new params.to_hash.stringify_keys.slice(*self.column_names)
     new_post.author = params[:author]
     new_post.public = params[:public] if params[:public]
     new_post.pending = params[:pending] if params[:pending]

app/models/profile.rb

@@ -38,9 +38,6 @@ class Profile < ActiveRecord::Base
   validate :max_tags
   validate :valid_birthday
 
-  attr_accessible :first_name, :last_name, :image_url, :image_url_medium,
-    :image_url_small, :birthday, :gender, :bio, :location, :searchable, :date, :tag_string, :nsfw
-
   belongs_to :person
   before_validation do
     self.tag_string = self.tag_string.split[0..4].join(' ')
@@ -57,7 +54,8 @@ class Profile < ActiveRecord::Base
 
   def receive(user, person)
     Rails.logger.info("event=receive payload_type=profile sender=#{person} to=#{user}")
-    person.profile.update_attributes self.attributes.merge(:tag_string => self.tag_string)
+    profiles_attr = self.attributes.merge('tag_string' => self.tag_string).slice('diaspora_handle', 'first_name', 'last_name', 'image_url', 'image_url_small', 'image_url_medium', 'birthday', 'gender', 'bio', 'location', 'searchable', 'nsfw', 'tag_string')
+    person.profile.update_attributes(profiles_attr) 
 
     person.profile
   end

app/models/reshare.rb

@@ -6,7 +6,6 @@ class Reshare < Post
 
   belongs_to :root, :class_name => 'Post', :foreign_key => :root_guid, :primary_key => :guid
   validate :root_must_be_public
-  attr_accessible :root_guid, :public
   validates_presence_of :root, :on => :create
   validates_uniqueness_of :root_guid, :scope => :author_id
   delegate :author, to: :root, prefix: true

app/models/status_message.rb

@@ -25,7 +25,6 @@ class StatusMessage < Post
   # therefore, we put the validation in a before_destory callback instead of a validation
   before_destroy :presence_of_content
 
-  attr_accessible :text, :provider_display_name, :frame_name
   attr_accessor :oembed_url
 
   before_create :filter_mentions

app/models/user.rb

@@ -63,25 +63,9 @@ class User < ActiveRecord::Base
 
   has_many :notifications, :foreign_key => :recipient_id
 
-
   before_save :guard_unconfirmed_email,
               :save_person!
 
-  attr_accessible :username,
-                  :email,
-                  :getting_started,
-                  :password,
-                  :password_confirmation,
-                  :language,
-                  :disable_mail,
-                  :invitation_service,
-                  :invitation_identifier,
-                  :show_community_spotlight_in_stream,
-                  :auto_follow_back,
-                  :auto_follow_back_aspect_id,
-                  :remember_me
-
-
   def self.all_sharing_with_person(person)
     User.joins(:contacts).where(:contacts => {:person_id => person.id})
   end
@@ -342,6 +326,8 @@ class User < ActiveRecord::Base
       params[:image_url_small] = photo.url(:thumb_small)
     end
 
+    params.stringify_keys!
+    params.slice!(*(Profile.column_names+['tag_string', 'date']))
     if self.profile.update_attributes(params)
       deliver_profile_update
       true
@@ -356,7 +342,7 @@ class User < ActiveRecord::Base
 
   ###Helpers############
   def self.build(opts = {})
-    u = User.new(opts)
+    u = User.new(opts.except(:person))
     u.setup(opts)
     u
   end

config/application.rb

@@ -49,7 +49,7 @@ module Diaspora
     # 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
+    #config.active_record.whitelist_attributes = false
 
     # Enable the asset pipeline
     config.assets.enabled = true

config/initializers/strong_parameters.rb

@@ -0,0 +1,2 @@
+# Please remove when migrating to Rails 4
+ActiveRecord::Base.send(:include, ActiveModel::ForbiddenAttributesProtection)

spec/controllers/photos_controller_spec.rb

@@ -54,6 +54,20 @@ describe PhotosController do
       }.should change(Photo, :count).by(1)
     end
 
+    it "doesn't allow mass assignment of person" do
+      new_user = FactoryGirl.create(:user)
+      @params[:photo][:author] = new_user
+      post :create, @params
+      Photo.last.author.should == alice.person
+    end
+
+    it "doesn't allow mass assignment of person_id" do
+      new_user = FactoryGirl.create(:user)
+      @params[:photo][:author_id] = new_user.id
+      post :create, @params
+      Photo.last.author.should == alice.person
+    end
+
     it 'can set the photo as the profile photo' do
       old_url = alice.person.profile.image_url
       @params[:photo][:set_profile_photo] = true
@@ -137,7 +151,14 @@ describe PhotosController do
       @alices_photo.reload.text.should == "now with lasers!"
     end
 
-    it "doesn't overwrite random attributes" do
+    it "doesn't allow mass assignment of person" do
+      new_user = FactoryGirl.create(:user)
+      params = { :text => "now with lasers!", :author => new_user }
+      put :update, :id => @alices_photo.id, :photo => params
+      @alices_photo.reload.author.should == alice.person
+    end
+
+    it "doesn't allow mass assignment of person_id" do
       new_user = FactoryGirl.create(:user)
       params = { :text => "now with lasers!", :author_id => new_user.id }
       put :update, :id => @alices_photo.id, :photo => params

spec/controllers/profiles_controller_spec.rb

@@ -71,7 +71,8 @@ describe ProfilesController do
 
     it 'sets tags' do
       params = { :id => eve.person.id,
-                 :tags => '#apples #oranges'}
+                 :tags => '#apples #oranges',
+                 :profile => {:tag_string => ''} }
 
       put :update, params
       eve.person(true).profile.tag_list.to_set.should == ['apples', 'oranges'].to_set

spec/models/photo_spec.rb

@@ -26,24 +26,6 @@ describe Photo do
     @saved_photo.save
   end
 
-  describe "protected attributes" do
-    it "doesn't allow mass assignment of person" do
-      @photo.save!
-      @photo.update_attributes(:author => FactoryGirl.build(:person))
-      @photo.reload.author.should == @user.person
-    end
-    it "doesn't allow mass assignment of person_id" do
-      @photo.save!
-      @photo.update_attributes(:author_id => FactoryGirl.build(:person).id)
-      @photo.reload.author.should == @user.person
-    end
-    it 'allows assignment of text' do
-      @photo.save!
-      @photo.update_attributes(:text => "this is awesome!!")
-      @photo.reload.text.should == "this is awesome!!"
-    end
-  end
-
   describe 'after_create' do
     it 'calls #queue_processing_job' do
       @photo.should_receive(:queue_processing_job)

spec/models/user_spec.rb

@@ -466,7 +466,7 @@ describe User do
     end
 
     it 'dispatches the profile when tags are set' do
-      @params = {:tags => '#what #hey'}
+      @params = {:tag_string => '#what #hey'}
       mailman = Postzord::Dispatcher.build(alice, Profile.new)
       Postzord::Dispatcher.should_receive(:build).and_return(mailman)
       alice.update_profile(@params).should be_true