From 66a07bd93825dac81782b663a18c6155d3932646 Mon Sep 17 00:00:00 2001 From: James Fleming Date: Mon, 8 Jul 2013 17:09:20 +0200 Subject: [PATCH] Remove attributes protection. Use a blacklist approach instead of a blacklist approach in Profile#receive. Remove attr_accessible from models and make specs pass. --- app/models/account_deletion.rb | 2 -- app/models/aspect.rb | 2 -- app/models/block.rb | 1 - app/models/conversation.rb | 1 - app/models/invitation.rb | 2 -- app/models/o_embed_cache.rb | 1 - app/models/photo.rb | 3 +-- app/models/post.rb | 2 +- app/models/profile.rb | 6 ++---- app/models/reshare.rb | 1 - app/models/status_message.rb | 1 - app/models/user.rb | 2 +- spec/models/photo_spec.rb | 18 ------------------ spec/models/user_spec.rb | 2 +- 14 files changed, 6 insertions(+), 38 deletions(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index c3ca2e142..492929e5b 100644 --- a/app/models/account_deletion.rb +++ b/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 diff --git a/app/models/aspect.rb b/app/models/aspect.rb index ef7e49ae4..c3d9de4a1 100644 --- a/app/models/aspect.rb +++ b/app/models/aspect.rb @@ -3,8 +3,6 @@ # the COPYRIGHT file. class Aspect < ActiveRecord::Base - include ActiveModel::ForbiddenAttributesProtection - belongs_to :user has_many :aspect_memberships, :dependent => :destroy diff --git a/app/models/block.rb b/app/models/block.rb index 146227ad4..361e4d934 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -1,5 +1,4 @@ class Block < ActiveRecord::Base - include ActiveModel::ForbiddenAttributesProtection belongs_to :person belongs_to :user diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 531cd0b9f..8d2f25c25 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -1,7 +1,6 @@ class Conversation < ActiveRecord::Base include Diaspora::Federated::Base include Diaspora::Guid - include ActiveModel::ForbiddenAttributesProtection xml_attr :subject xml_attr :created_at diff --git a/app/models/invitation.rb b/app/models/invitation.rb index 1f2aa9cee..40e5037c9 100644 --- a/app/models/invitation.rb +++ b/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? diff --git a/app/models/o_embed_cache.rb b/app/models/o_embed_cache.rb index 1d0ec6fb9..9aee6d00b 100644 --- a/app/models/o_embed_cache.rb +++ b/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 diff --git a/app/models/photo.rb b/app/models/photo.rb index 7df5ab9bb..f34d6f3fb 100644 --- a/app/models/photo.rb +++ b/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] diff --git a/app/models/post.rb b/app/models/post.rb index d5e466cab..59968356f 100644 --- a/app/models/post.rb +++ b/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] diff --git a/app/models/profile.rb b/app/models/profile.rb index 5a8073d3b..76a56d73e 100644 --- a/app/models/profile.rb +++ b/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 diff --git a/app/models/reshare.rb b/app/models/reshare.rb index 5b3c5341b..e5728429b 100644 --- a/app/models/reshare.rb +++ b/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 diff --git a/app/models/status_message.rb b/app/models/status_message.rb index 13821c4bd..f580be28d 100644 --- a/app/models/status_message.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 1170b0082..92d9cb8e8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,7 +7,6 @@ class User < ActiveRecord::Base include Connecting include Querying include SocialActions - include ActiveModel::ForbiddenAttributesProtection scope :logged_in_since, lambda { |time| where('last_sign_in_at > ?', time) } scope :monthly_actives, lambda { |time = Time.now| logged_in_since(time - 1.month) } @@ -327,6 +326,7 @@ class User < ActiveRecord::Base params[:image_url_small] = photo.url(:thumb_small) end + params.stringify_keys!.slice!(*(Profile.column_names+['tag_string', 'date'])) if self.profile.update_attributes(params) deliver_profile_update true diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index f9e8d2036..3b71e2b6e 100644 --- a/spec/models/photo_spec.rb +++ b/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) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d0ac01f5a..60c1e157c 100644 --- a/spec/models/user_spec.rb +++ b/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