Sanitize html for note fields of tickets, users and organizations.

This commit is contained in:
Martin Edenhofer 2019-02-12 08:38:59 +01:00
parent f28cd627f8
commit dabce01ec2
7 changed files with 28 additions and 0 deletions

View file

@ -7,6 +7,7 @@ class Organization < ApplicationModel
include HasHistory include HasHistory
include HasSearchIndexBackend include HasSearchIndexBackend
include CanCsvImport include CanCsvImport
include ChecksHtmlSanitized
include Organization::ChecksAccess include Organization::ChecksAccess
include Organization::Assets include Organization::Assets
@ -22,6 +23,8 @@ class Organization < ApplicationModel
activity_stream_permission 'admin.role' activity_stream_permission 'admin.role'
sanitized_html :note
private private
def domain_cleanup def domain_cleanup

View file

@ -6,6 +6,7 @@ class Ticket < ApplicationModel
include ChecksClientNotification include ChecksClientNotification
include ChecksLatestChangeObserved include ChecksLatestChangeObserved
include CanCsvImport include CanCsvImport
include ChecksHtmlSanitized
include HasHistory include HasHistory
include HasTags include HasTags
include HasSearchIndexBackend include HasSearchIndexBackend
@ -56,6 +57,8 @@ class Ticket < ApplicationModel
history_relation_object 'Ticket::Article' history_relation_object 'Ticket::Article'
sanitized_html :note
belongs_to :group belongs_to :group
belongs_to :organization belongs_to :organization
has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy, inverse_of: :ticket has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy, inverse_of: :ticket

View file

@ -6,6 +6,7 @@ class User < ApplicationModel
include HasHistory include HasHistory
include HasSearchIndexBackend include HasSearchIndexBackend
include CanCsvImport include CanCsvImport
include ChecksHtmlSanitized
include HasGroups include HasGroups
include HasRoles include HasRoles
@ -66,6 +67,8 @@ class User < ApplicationModel
:groups, :groups,
:user_groups :user_groups
sanitized_html :note
def ignore_search_indexing?(_action) def ignore_search_indexing?(_action)
# ignore internal user # ignore internal user
return true if id == 1 return true if id == 1

View file

@ -0,0 +1,10 @@
RSpec.shared_examples 'HasXssSanitizedNote' do |model_factory:|
describe 'XSS prevention' do
context 'with injected JS' do
subject { create(model_factory, note: 'test 123 <script type="text/javascript">alert("XSS!");</script> <b>some text</b>') }
it 'strips out <script> tag' do
expect(subject.note).to eq('test 123 alert("XSS!"); <b>some text</b>')
end
end
end
end

View file

@ -1,10 +1,12 @@
require 'rails_helper' require 'rails_helper'
require 'models/concerns/can_lookup_examples' require 'models/concerns/can_lookup_examples'
require 'models/concerns/has_search_index_backend_examples' require 'models/concerns/has_search_index_backend_examples'
require 'models/concerns/has_xss_sanitized_note_examples'
RSpec.describe Organization, type: :model do RSpec.describe Organization, type: :model do
it_behaves_like 'CanLookup' it_behaves_like 'CanLookup'
it_behaves_like 'HasSearchIndexBackend', indexed_factory: :organization it_behaves_like 'HasSearchIndexBackend', indexed_factory: :organization
it_behaves_like 'HasXssSanitizedNote', model_factory: :organization
describe '.where_or_cis' do describe '.where_or_cis' do
it 'finds instance by querying multiple attributes case insensitive' do it 'finds instance by querying multiple attributes case insensitive' do
@ -13,4 +15,5 @@ RSpec.describe Organization, type: :model do
expect(organizations).not_to be_blank expect(organizations).not_to be_blank
end end
end end
end end

View file

@ -2,11 +2,13 @@ require 'rails_helper'
require 'models/application_model_examples' require 'models/application_model_examples'
require 'models/concerns/can_be_imported_examples' require 'models/concerns/can_be_imported_examples'
require 'models/concerns/can_lookup_examples' require 'models/concerns/can_lookup_examples'
require 'models/concerns/has_xss_sanitized_note_examples'
RSpec.describe Ticket, type: :model do RSpec.describe Ticket, type: :model do
it_behaves_like 'ApplicationModel' it_behaves_like 'ApplicationModel'
it_behaves_like 'CanBeImported' it_behaves_like 'CanBeImported'
it_behaves_like 'CanLookup' it_behaves_like 'CanLookup'
it_behaves_like 'HasXssSanitizedNote', model_factory: :ticket
subject(:ticket) { create(:ticket) } subject(:ticket) { create(:ticket) }
@ -378,4 +380,5 @@ RSpec.describe Ticket, type: :model do
end end
end end
end end
end end

View file

@ -3,6 +3,7 @@ require 'models/application_model_examples'
require 'models/concerns/has_groups_examples' require 'models/concerns/has_groups_examples'
require 'models/concerns/has_roles_examples' require 'models/concerns/has_roles_examples'
require 'models/concerns/has_groups_permissions_examples' require 'models/concerns/has_groups_permissions_examples'
require 'models/concerns/has_xss_sanitized_note_examples'
require 'models/concerns/can_be_imported_examples' require 'models/concerns/can_be_imported_examples'
require 'models/concerns/can_lookup_examples' require 'models/concerns/can_lookup_examples'
@ -10,6 +11,7 @@ RSpec.describe User do
it_behaves_like 'ApplicationModel' it_behaves_like 'ApplicationModel'
it_behaves_like 'HasGroups', group_access_factory: :agent_user it_behaves_like 'HasGroups', group_access_factory: :agent_user
it_behaves_like 'HasRoles', group_access_factory: :agent_user it_behaves_like 'HasRoles', group_access_factory: :agent_user
it_behaves_like 'HasXssSanitizedNote', model_factory: :user
it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user
it_behaves_like 'CanBeImported' it_behaves_like 'CanBeImported'
it_behaves_like 'CanLookup' it_behaves_like 'CanLookup'
@ -831,4 +833,5 @@ RSpec.describe User do
end end
end end
end end
end end