From b96fb53a71100ac55dc06cc2fc1992f21dd7ec9e Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Wed, 17 Apr 2019 14:18:02 +0800 Subject: [PATCH] Refactoring: Migrate ticket_xss_test to RSpec --- spec/models/channel/email_parser_spec.rb | 32 ++++ spec/models/ticket/article_spec.rb | 183 ++++++++++++++++++++++ spec/models/ticket_spec.rb | 10 ++ test/unit/ticket_xss_test.rb | 184 ----------------------- 4 files changed, 225 insertions(+), 184 deletions(-) delete mode 100644 test/unit/ticket_xss_test.rb diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 9498865b0..2561a30dd 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -406,6 +406,38 @@ RSpec.describe Channel::EmailParser, type: :model do end end + describe 'XSS protection' do + let(:article) { described_class.new.process({}, raw_mail).second } + + let(:raw_mail) { <<~RAW.chomp } + From: ME Bob + To: customer@example.com + Subject: some subject + Content-Type: #{content_type} + MIME-Version: 1.0 + + no HTML + RAW + + context 'for Content-Type: text/html' do + let(:content_type) { 'text/html' } + + it 'removes injected + SANITIZED + end + end + end + context 'for “delivery failed” notifications (a.k.a. bounce messages)' do let(:ticket) { article.ticket } let(:article) { create(:ticket_article, sender_name: 'Agent', message_id: message_id) } diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb index 888682f0a..42803ad25 100644 --- a/spec/models/ticket/article_spec.rb +++ b/spec/models/ticket/article_spec.rb @@ -57,6 +57,189 @@ RSpec.describe Ticket::Article, type: :model do end end + describe 'XSS protection:' do + subject(:article) { create(:ticket_article, body: body, content_type: 'text/html') } + + context 'when body contains only injected JS' do + let(:body) { <<~RAW.chomp } + + RAW + + it 'removes + RAW + + it 'removes ' } + + it 'does not sanitize title' do + expect(ticket.title).to eq(title) + end + end + describe 'Cti::CallerId syncing:' do subject(:ticket) { build(:ticket) } before { allow(Cti::CallerId).to receive(:build) } diff --git a/test/unit/ticket_xss_test.rb b/test/unit/ticket_xss_test.rb deleted file mode 100644 index a48f29da5..000000000 --- a/test/unit/ticket_xss_test.rb +++ /dev/null @@ -1,184 +0,0 @@ -require 'test_helper' - -class TicketXssTest < ActiveSupport::TestCase - test 'xss via model' do - ticket = Ticket.create( - title: 'test 123 ', - group: Group.lookup(name: 'Users'), - customer_id: 2, - state: Ticket::State.lookup(name: 'new'), - priority: Ticket::Priority.lookup(name: '2 normal'), - updated_by_id: 1, - created_by_id: 1, - ) - assert(ticket, 'ticket created') - - assert_equal('test 123 ', ticket.title, 'ticket.title verify') - assert_equal('Users', ticket.group.name, 'ticket.group verify') - assert_equal('new', ticket.state.name, 'ticket.state verify') - - article1 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: '', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('alert("XSS!");', article1.body, 'article1.body verify - inbound') - - article2 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'please tell me this doesn\'t work: ', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('please tell me this doesn\'t work: alert("XSS!");', article2.body, 'article2.body verify - inbound') - - article3 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'please tell me this doesn\'t work: ada
LINKaaABC', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal("please tell me this doesn't work: ada -
-LINKaaABC
", article3.body, 'article3.body verify - inbound') - - article4 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'please tell me this doesn\'t work: alal', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal("please tell me this doesn't work: alal", article4.body, 'article4.body verify - inbound') - - article5 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/plain', - body: 'please tell me this doesn\'t work: ada
LINKaaABC', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('please tell me this doesn\'t work: ada
LINKaaABC', article5.body, 'article5.body verify - inbound') - - article6 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'some message article helper test1
asdasd
', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('some message article helper test1
-asdasd
-
', article6.body, 'article6.body verify - inbound') - - article7 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'some message article helper test1
asdasd
', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('some message article helper test1
-asdasd
-
', article7.body, 'article7.body verify - inbound') - - article8 = Ticket::Article.create( - ticket_id: ticket.id, - from: 'some_sender@example.com', - to: 'some_recipient@example.com', - subject: 'some subject ', - message_id: 'some@id', - content_type: 'text/html', - body: 'some message article helper test1 abc 123123', - internal: false, - sender: Ticket::Article::Sender.find_by(name: 'Customer'), - type: Ticket::Article::Type.find_by(name: 'email'), - updated_by_id: 1, - created_by_id: 1, - ) - assert_equal('some message article helper test1 abc 123123', article8.body, 'article8.body verify - inbound') - - end - - test 'xss via mail' do - data = 'From: ME Bob -To: customer@example.com -Subject: some subject -Content-Type: text/html -MIME-Version: 1.0 - -no HTML ' - - parser = Channel::EmailParser.new - ticket, article, user = parser.process({}, data) - assert_equal('text/html', ticket.articles.first.content_type) - assert_equal('no HTML alert(\'XSS\')', ticket.articles.first.body) - - data = 'From: ME Bob -To: customer@example.com -Subject: some subject -Content-Type: text/plain -MIME-Version: 1.0 - -no HTML ' - - parser = Channel::EmailParser.new - ticket, article, user = parser.process({}, data) - assert_equal('text/plain', ticket.articles.first.content_type) - assert_equal('no HTML ', ticket.articles.first.body) - - end -end