Commit 09f41a8349ca42cc70a4433bbe7e0dfaabd25df8
1 parent
741b63db
Exists in
master
and in
22 other branches
ActionItem192: fixing xss_terminate plugin and some functionals tests
git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@1724 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
22 changed files
with
115 additions
and
83 deletions
Show diff stats
app/models/community.rb
app/models/create_enterprise.rb
| ... | ... | @@ -40,7 +40,7 @@ class CreateEnterprise < Task |
| 40 | 40 | # check for explanation when rejecting |
| 41 | 41 | validates_presence_of :reject_explanation, :if => (lambda { |record| record.status == Task::Status::CANCELLED } ) |
| 42 | 42 | |
| 43 | - xss_terminate :only => [ :acronym, :address, :contact_person, :contact_phone, :economic_activity, :foundation_year, :legal_form, :management_information, :name ], :on => 'validation' | |
| 43 | + xss_terminate :only => [ :acronym, :address, :contact_person, :contact_phone, :economic_activity, :legal_form, :management_information, :name ], :on => 'validation' | |
| 44 | 44 | |
| 45 | 45 | def validate |
| 46 | 46 | if self.region && self.target | ... | ... |
app/models/organization_info.rb
| ... | ... | @@ -5,9 +5,7 @@ class OrganizationInfo < ActiveRecord::Base |
| 5 | 5 | |
| 6 | 6 | validates_format_of :contact_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |info| ! info.contact_email.nil? }) |
| 7 | 7 | |
| 8 | - xss_terminate :only => [ :acronym, :contact_person, :contact_email, :foundation_year, :legal_form, :economic_activity, :management_information ] | |
| 9 | - | |
| 10 | - #xss_terminate :only => [ :acronym, :contact_person, :contact_phone, :economic_activity, :foundation_year, :legal_form, :management_information, :address, :name ] | |
| 8 | + xss_terminate :only => [ :acronym, :contact_person, :contact_email, :legal_form, :economic_activity, :management_information ] | |
| 11 | 9 | |
| 12 | 10 | def summary |
| 13 | 11 | # FIXME diplays too few fields | ... | ... |
app/models/text_article.rb
app/models/tiny_mce_article.rb
test/functional/admin_controller_test.rb
| ... | ... | @@ -11,16 +11,9 @@ class AdminControllerTest < Test::Unit::TestCase |
| 11 | 11 | @response = ActionController::TestResponse.new |
| 12 | 12 | end |
| 13 | 13 | |
| 14 | - def test_local_files_reference | |
| 15 | - assert_local_files_reference | |
| 16 | - end | |
| 17 | - | |
| 18 | - def test_valid_xhtml | |
| 19 | - assert_valid_xhtml | |
| 20 | - end | |
| 21 | - | |
| 22 | 14 | # Replace this with your real tests. |
| 23 | 15 | def test_truth |
| 24 | 16 | assert true |
| 25 | 17 | end |
| 18 | + | |
| 26 | 19 | end | ... | ... |
test/functional/catalog_controller_test.rb
| ... | ... | @@ -12,7 +12,8 @@ class CatalogControllerTest < Test::Unit::TestCase |
| 12 | 12 | end |
| 13 | 13 | |
| 14 | 14 | def test_local_files_reference |
| 15 | - assert_local_files_reference | |
| 15 | + user = create_user('user_test').person | |
| 16 | + assert_local_files_reference :get, :index, :profile => user.identifier | |
| 16 | 17 | end |
| 17 | 18 | |
| 18 | 19 | def test_valid_xhtml | ... | ... |
test/functional/cms_controller_test.rb
| ... | ... | @@ -20,7 +20,7 @@ class CmsControllerTest < Test::Unit::TestCase |
| 20 | 20 | attr_reader :profile |
| 21 | 21 | |
| 22 | 22 | def test_local_files_reference |
| 23 | - assert_local_files_reference | |
| 23 | + assert_local_files_reference :get, :index, :profile => profile.identifier | |
| 24 | 24 | end |
| 25 | 25 | |
| 26 | 26 | def test_valid_xhtml |
| ... | ... | @@ -267,5 +267,40 @@ class CmsControllerTest < Test::Unit::TestCase |
| 267 | 267 | assert_not_includes saved.categories, c2 |
| 268 | 268 | assert_includes saved.categories, c3 |
| 269 | 269 | end |
| 270 | + | |
| 271 | + should 'filter html from textile article name' do | |
| 272 | + post :new, :type => 'TextileArticle', :profile => profile.identifier, :article => { :name => 'a <strong>test</strong> article', :body => 'the text of the article ...' } | |
| 273 | + assert_sanitized assigns(:article).name | |
| 274 | + end | |
| 275 | + | |
| 276 | + should 'filter html from textile article abstract' do | |
| 277 | + post :new, :type => 'TextileArticle', :profile => profile.identifier, :article => { :name => 'article', :abstract => '<strong>abstract</strong>', :body => 'the text of the article ...' } | |
| 278 | + assert_sanitized assigns(:article).abstract | |
| 279 | + end | |
| 280 | + | |
| 281 | + should 'filter html from textile article body' do | |
| 282 | + post :new, :type => 'TextileArticle', :profile => profile.identifier, :article => { :name => 'article', :abstract => 'abstract', :body => 'the <b>text</b> of <a href=#>the</a> article ...' } | |
| 283 | + assert_sanitized assigns(:article).body | |
| 284 | + end | |
| 285 | + | |
| 286 | + should 'filter html with white_list from tiny mce article name' do | |
| 287 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => "<strong>test</strong>", :body => 'the text of the article ...' } | |
| 288 | + assert_equal "<strong>test</strong>", assigns(:article).name | |
| 289 | + end | |
| 290 | + | |
| 291 | + should 'filter html with white_list from tiny mce article abstract' do | |
| 292 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'article', :abstract => "<script>alert('test')</script> article", :body => 'the text of the article ...' } | |
| 293 | + assert_equal " article", assigns(:article).abstract | |
| 294 | + end | |
| 295 | + | |
| 296 | + should 'filter html with white_list from tiny mce article body' do | |
| 297 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'article', :abstract => 'abstract', :body => "the <script>alert('text')</script> of article ..." } | |
| 298 | + assert_equal "the of article ...", assigns(:article).body | |
| 299 | + end | |
| 300 | + | |
| 301 | + should 'not filter html tags permitted from tiny mce article body' do | |
| 302 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'article', :abstract => 'abstract', :body => "<b>the</b> <script>alert('text')</script> <strong>of</strong> article ..." } | |
| 303 | + assert_equal "<b>the</b> <strong>of</strong> article ...", assigns(:article).body | |
| 304 | + end | |
| 270 | 305 | |
| 271 | 306 | end | ... | ... |
test/functional/consumed_products_controller_test.rb
| ... | ... | @@ -18,7 +18,7 @@ class ConsumedProductsControllerTest < Test::Unit::TestCase |
| 18 | 18 | attr_reader :profile |
| 19 | 19 | |
| 20 | 20 | def test_local_files_reference |
| 21 | - assert_local_files_reference | |
| 21 | + assert_local_files_reference :get, :index, :profile => profile.identifier | |
| 22 | 22 | end |
| 23 | 23 | |
| 24 | 24 | def test_valid_xhtml | ... | ... |
test/functional/content_viewer_controller_test.rb
| ... | ... | @@ -18,7 +18,9 @@ class ContentViewerControllerTest < Test::Unit::TestCase |
| 18 | 18 | attr_reader :profile |
| 19 | 19 | |
| 20 | 20 | def test_local_files_reference |
| 21 | - assert_local_files_reference | |
| 21 | + page = profile.articles.build(:name => 'test') | |
| 22 | + page.save! | |
| 23 | + assert_local_files_reference :get, :view_page, :profile => profile.identifier, :page => [ 'test' ] | |
| 22 | 24 | end |
| 23 | 25 | |
| 24 | 26 | def test_valid_xhtml | ... | ... |
test/functional/enterprise_editor_controller_test.rb
| ... | ... | @@ -12,7 +12,8 @@ class EnterpriseEditorControllerTest < Test::Unit::TestCase |
| 12 | 12 | end |
| 13 | 13 | |
| 14 | 14 | def test_local_files_reference |
| 15 | - assert_local_files_reference | |
| 15 | + user = create_user('test_user').person | |
| 16 | + assert_local_files_reference :get, :index, :profile => user.identifier | |
| 16 | 17 | end |
| 17 | 18 | |
| 18 | 19 | def test_valid_xhtml | ... | ... |
test/functional/enterprise_registration_controller_test.rb
| ... | ... | @@ -119,11 +119,6 @@ all_fixtures |
| 119 | 119 | assert_sanitized assigns(:create_enterprise).acronym |
| 120 | 120 | end |
| 121 | 121 | |
| 122 | - should 'filter html from foundation_year' do | |
| 123 | - post :index, :create_enterprise => { 'name' => 'name', 'identifier' => 'mynew', :foundation_year => '<b>foundation_year</b>' } | |
| 124 | - assert_sanitized assigns(:create_enterprise).foundation_year | |
| 125 | - end | |
| 126 | - | |
| 127 | 122 | should 'filter html from legal_form' do |
| 128 | 123 | post :index, :create_enterprise => { 'name' => 'name', 'identifier' => 'mynew', :legal_form => '<b>legal_form</b>' } |
| 129 | 124 | assert_sanitized assigns(:create_enterprise).legal_form | ... | ... |
test/functional/enterprise_validation_test.rb
test/functional/manage_products_controller_test.rb
test/functional/memberships_controller_test.rb
| ... | ... | @@ -17,7 +17,7 @@ class MembershipsControllerTest < Test::Unit::TestCase |
| 17 | 17 | attr_reader :profile |
| 18 | 18 | |
| 19 | 19 | def test_local_files_reference |
| 20 | - assert_local_files_reference | |
| 20 | + assert_local_files_reference :get, :index, :profile => profile.identifier | |
| 21 | 21 | end |
| 22 | 22 | |
| 23 | 23 | def test_valid_xhtml | ... | ... |
test/functional/my_profile_controller_test.rb
| ... | ... | @@ -21,7 +21,10 @@ class MyProfileControllerTest < Test::Unit::TestCase |
| 21 | 21 | end |
| 22 | 22 | |
| 23 | 23 | def test_local_files_reference |
| 24 | - assert_local_files_reference | |
| 24 | + @controller = OnlyForPersonTestController.new | |
| 25 | + user = create_user('test_user').person | |
| 26 | + assert_local_files_reference :get, :index, :profile => user.identifier | |
| 27 | + #get :index, :profile => user.identifier | |
| 25 | 28 | end |
| 26 | 29 | |
| 27 | 30 | def test_valid_xhtml | ... | ... |
test/functional/profile_design_controller_test.rb
test/functional/profile_editor_controller_test.rb
test/functional/profile_members_controller_test.rb
| ... | ... | @@ -12,7 +12,8 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 12 | 12 | end |
| 13 | 13 | |
| 14 | 14 | def test_local_files_reference |
| 15 | - assert_local_files_reference | |
| 15 | + user = create_user('test_user').person | |
| 16 | + assert_local_files_reference :get, :index, :profile => user.identifier | |
| 16 | 17 | end |
| 17 | 18 | |
| 18 | 19 | def test_valid_xhtml | ... | ... |
test/functional/public_controller_test.rb
| ... | ... | @@ -12,14 +12,6 @@ class PublicControllerTest < Test::Unit::TestCase |
| 12 | 12 | @response = ActionController::TestResponse.new |
| 13 | 13 | end |
| 14 | 14 | |
| 15 | - def test_local_files_reference | |
| 16 | - assert_local_files_reference | |
| 17 | - end | |
| 18 | - | |
| 19 | - def test_valid_xhtml | |
| 20 | - assert_valid_xhtml | |
| 21 | - end | |
| 22 | - | |
| 23 | 15 | # Replace this with your real tests. |
| 24 | 16 | def test_truth |
| 25 | 17 | assert true | ... | ... |
test/functional/system_controller_test.rb
| ... | ... | @@ -11,14 +11,6 @@ class SystemControllerTest < Test::Unit::TestCase |
| 11 | 11 | @response = ActionController::TestResponse.new |
| 12 | 12 | end |
| 13 | 13 | |
| 14 | - def test_local_files_reference | |
| 15 | - assert_local_files_reference | |
| 16 | - end | |
| 17 | - | |
| 18 | - def test_valid_xhtml | |
| 19 | - assert_valid_xhtml | |
| 20 | - end | |
| 21 | - | |
| 22 | 14 | # Replace this with your real tests. |
| 23 | 15 | def test_truth |
| 24 | 16 | assert true | ... | ... |
vendor/plugins/xss_terminate/lib/xss_terminate.rb
| ... | ... | @@ -15,65 +15,78 @@ module XssTerminate |
| 15 | 15 | module ClassMethods |
| 16 | 16 | |
| 17 | 17 | def xss_terminate(options = {}) |
| 18 | + options[:with] ||= 'full' | |
| 19 | + filter_with = 'sanitize_fields_with_' + options[:with] | |
| 18 | 20 | # :on is util when before_filter dont work for model |
| 19 | 21 | case options[:on] |
| 20 | 22 | when 'create' |
| 21 | - before_create :sanitize_fields | |
| 23 | + before_create filter_with | |
| 22 | 24 | when 'validation' |
| 23 | - before_validation :sanitize_fields | |
| 25 | + before_validation filter_with | |
| 24 | 26 | else |
| 25 | - before_save :sanitize_fields | |
| 27 | + before_save filter_with | |
| 26 | 28 | end |
| 27 | - | |
| 28 | - sanitizer = case options[:with] | |
| 29 | - when 'html5lib' | |
| 30 | - HTML5libSanitize.new | |
| 31 | - when 'white_list' | |
| 32 | - RailsSanitize.white_list_sanitizer | |
| 33 | - else | |
| 34 | - RailsSanitize.full_sanitizer | |
| 35 | - end | |
| 36 | - | |
| 37 | - write_inheritable_attribute(:xss_terminate_options, { | |
| 29 | + write_inheritable_attribute("xss_terminate_#{options[:with]}_options".to_sym, { | |
| 38 | 30 | :except => (options[:except] || []), |
| 39 | - :only => (options[:only] || options[:sanitize] || []), | |
| 40 | - :sanitizer => sanitizer, | |
| 41 | - | |
| 42 | - :html5lib_sanitize => (options[:html5lib_sanitize] || []) | |
| 31 | + :only => (options[:only] || options[:sanitize] || []) | |
| 43 | 32 | }) |
| 44 | - | |
| 45 | - class_inheritable_reader :xss_terminate_options | |
| 46 | - | |
| 33 | + class_inheritable_reader "xss_terminate_#{options[:with]}_options".to_sym | |
| 47 | 34 | include XssTerminate::InstanceMethods |
| 48 | 35 | end |
| 36 | + | |
| 49 | 37 | end |
| 50 | 38 | |
| 51 | 39 | module InstanceMethods |
| 52 | 40 | |
| 53 | - def sanitize_fields | |
| 41 | + def sanitize_field(sanitizer, field, serialized = false) | |
| 42 | + field = field.to_sym | |
| 43 | + if serialized | |
| 44 | + puts field | |
| 45 | + self[field].each_key { |key| | |
| 46 | + key = key.to_sym | |
| 47 | + self[field][key] = sanitizer.sanitize(self[field][key]) | |
| 48 | + } | |
| 49 | + else | |
| 50 | + if self[field] | |
| 51 | + self[field] = sanitizer.sanitize(self[field]) | |
| 52 | + else | |
| 53 | + self.send("#{field}=", sanitizer.sanitize(self.send("#{field}"))) | |
| 54 | + end | |
| 55 | + end | |
| 56 | + end | |
| 54 | 57 | |
| 55 | - columns = self.class.columns.select{ |i| i.type == :string || i.type == :text }.map{ |i| i.name } | |
| 58 | + def sanitize_columns(with = :full) | |
| 56 | 59 | columns_serialized = self.class.serialized_attributes.keys |
| 60 | + only = eval "xss_terminate_#{with}_options[:only]" | |
| 61 | + except = eval "xss_terminate_#{with}_options[:except]" | |
| 62 | + unless except.empty? | |
| 63 | + only.delete_if{ |i| except.include?( i.to_sym ) } | |
| 64 | + end | |
| 65 | + return only, columns_serialized | |
| 66 | + end | |
| 57 | 67 | |
| 58 | - if !xss_terminate_options[:only].empty? | |
| 59 | - columns = columns.select{ |i| xss_terminate_options[:only].include?( i.to_sym ) } | |
| 60 | - elsif !xss_terminate_options[:except].empty? | |
| 61 | - columns.delete_if{ |i| xss_terminate_options[:except].include?( i.to_sym ) } | |
| 68 | + def sanitize_fields_with_full | |
| 69 | + sanitizer = RailsSanitize.full_sanitizer | |
| 70 | + columns, columns_serialized = sanitize_columns(:full) | |
| 71 | + columns.each do |column| | |
| 72 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | |
| 62 | 73 | end |
| 74 | + end | |
| 63 | 75 | |
| 76 | + def sanitize_fields_with_white_list | |
| 77 | + sanitizer = RailsSanitize.white_list_sanitizer | |
| 78 | + columns, columns_serialized = sanitize_columns(:white_list) | |
| 64 | 79 | columns.each do |column| |
| 65 | - field = column.to_sym | |
| 66 | - if columns_serialized.include?(column) | |
| 67 | - next unless self[field] | |
| 68 | - self[field].each_key { |key| | |
| 69 | - key = key.to_sym | |
| 70 | - self[field][key] = xss_terminate_options[:sanitizer].sanitize(self[field][key]) | |
| 71 | - } | |
| 72 | - else | |
| 73 | - self[field] = xss_terminate_options[:sanitizer].sanitize(self[field]) | |
| 74 | - end | |
| 80 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | |
| 75 | 81 | end |
| 82 | + end | |
| 76 | 83 | |
| 84 | + def sanitize_fields_with_html5lib | |
| 85 | + sanitizer = HTML5libSanitize.new | |
| 86 | + columns = sanitize_columns(:html5lib) | |
| 87 | + columns.each do |column| | |
| 88 | + sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column)) | |
| 89 | + end | |
| 77 | 90 | end |
| 78 | 91 | |
| 79 | 92 | end | ... | ... |