2018-12-13 09:06:44 +00:00
|
|
|
|
require 'rails_helper'
|
2019-01-22 16:35:01 +00:00
|
|
|
|
require 'models/application_model_examples'
|
2019-01-28 06:04:05 +00:00
|
|
|
|
require 'models/concerns/can_be_imported_examples'
|
2019-10-21 10:44:52 +00:00
|
|
|
|
require 'models/concerns/can_csv_import_examples'
|
2019-06-28 13:07:14 +00:00
|
|
|
|
require 'models/concerns/has_history_examples'
|
2019-03-13 23:51:22 +00:00
|
|
|
|
require 'models/concerns/has_object_manager_attributes_validation_examples'
|
2019-01-22 16:35:01 +00:00
|
|
|
|
|
|
|
|
|
RSpec.describe Ticket::Article, type: :model do
|
2020-09-30 09:07:01 +00:00
|
|
|
|
subject(:article) { create(:ticket_article) }
|
|
|
|
|
|
2019-01-24 10:13:04 +00:00
|
|
|
|
it_behaves_like 'ApplicationModel'
|
2019-01-28 06:04:05 +00:00
|
|
|
|
it_behaves_like 'CanBeImported'
|
2019-10-21 10:44:52 +00:00
|
|
|
|
it_behaves_like 'CanCsvImport'
|
2019-06-28 13:07:14 +00:00
|
|
|
|
it_behaves_like 'HasHistory'
|
2019-03-13 23:51:22 +00:00
|
|
|
|
it_behaves_like 'HasObjectManagerAttributesValidation'
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-04-04 03:01:20 +00:00
|
|
|
|
describe 'Callbacks, Observers, & Async Transactions -' do
|
2019-02-05 09:59:13 +00:00
|
|
|
|
describe 'NULL byte handling (via ChecksAttributeValuesAndLength concern):' do
|
2019-02-05 07:33:40 +00:00
|
|
|
|
it 'removes them from #subject on creation, if necessary (postgres doesn’t like them)' do
|
2019-02-05 09:59:13 +00:00
|
|
|
|
expect(create(:ticket_article, subject: "com test 1\u0000"))
|
|
|
|
|
.to be_persisted
|
|
|
|
|
end
|
|
|
|
|
|
2019-02-05 07:33:40 +00:00
|
|
|
|
it 'removes them from #body on creation, if necessary (postgres doesn’t like them)' do
|
2019-02-05 09:59:13 +00:00
|
|
|
|
expect(create(:ticket_article, body: "some\u0000message 123"))
|
|
|
|
|
.to be_persisted
|
|
|
|
|
end
|
2019-01-04 15:29:56 +00:00
|
|
|
|
end
|
|
|
|
|
|
2019-04-04 03:01:20 +00:00
|
|
|
|
describe 'Setting of ticket.create_article_{sender,type}' do
|
|
|
|
|
let!(:ticket) { create(:ticket) }
|
|
|
|
|
|
|
|
|
|
context 'on creation' do
|
|
|
|
|
context 'of first article on a ticket' do
|
|
|
|
|
subject(:article) do
|
|
|
|
|
create(:ticket_article, ticket: ticket, sender_name: 'Agent', type_name: 'email')
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
it 'sets ticket sender/type attributes based on article sender/type' do
|
|
|
|
|
expect { article }
|
|
|
|
|
.to change { ticket.reload.create_article_sender&.name }.to('Agent')
|
|
|
|
|
.and change { ticket.reload.create_article_type&.name }.to('email')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'of subsequent articles on a ticket' do
|
|
|
|
|
subject(:article) do
|
|
|
|
|
create(:ticket_article, ticket: ticket, sender_name: 'Customer', type_name: 'twitter status')
|
|
|
|
|
end
|
|
|
|
|
|
2019-04-15 01:41:17 +00:00
|
|
|
|
let!(:first_article) do
|
|
|
|
|
create(:ticket_article, ticket: ticket, sender_name: 'Agent', type_name: 'email')
|
|
|
|
|
end
|
|
|
|
|
|
2019-04-04 03:01:20 +00:00
|
|
|
|
it 'does not modify ticket’s sender/type attributes' do
|
|
|
|
|
expect { article }
|
|
|
|
|
.to not_change { ticket.reload.create_article_sender.name }
|
|
|
|
|
.and not_change { ticket.reload.create_article_type.name }
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
2019-04-17 06:18:02 +00:00
|
|
|
|
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 }
|
|
|
|
|
<script type="text/javascript">alert("XSS!");</script>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'removes <script> tags' do
|
|
|
|
|
expect(article.body).to eq('alert("XSS!");')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains injected JS amidst other text' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
please tell me this doesn't work: <script type="text/javascript">alert("XSS!");</script>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'removes <script> tags' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
please tell me this doesn't work: alert("XSS!");
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains invalid HTML tags' do
|
|
|
|
|
let(:body) { '<some_not_existing>ABC</some_not_existing>' }
|
|
|
|
|
|
|
|
|
|
it 'removes invalid tags' do
|
|
|
|
|
expect(article.body).to eq('ABC')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains restricted HTML attributes' do
|
|
|
|
|
let(:body) { '<div class="adasd" id="123" data-abc="123"></div>' }
|
|
|
|
|
|
|
|
|
|
it 'removes restricted attributes' do
|
|
|
|
|
expect(article.body).to eq('<div></div>')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains JS injected into href attribute' do
|
|
|
|
|
let(:body) { '<a href="javascript:someFunction()">LINK</a>' }
|
|
|
|
|
|
|
|
|
|
it 'removes <a> tags' do
|
|
|
|
|
expect(article.body).to eq('LINK')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains an unclosed <div> element' do
|
|
|
|
|
let(:body) { '<div>foo' }
|
|
|
|
|
|
|
|
|
|
it 'closes it' do
|
|
|
|
|
expect(article.body).to eq('<div>foo</div>')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains a plain link (<a> element)' do
|
|
|
|
|
let(:body) { '<a href="https://example.com">foo</a>' }
|
|
|
|
|
|
|
|
|
|
it 'adds sanitization attributes' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
<a href="https://example.com" rel="nofollow noreferrer noopener" target="_blank" title="https://example.com">foo</a>
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
2020-07-29 14:54:40 +00:00
|
|
|
|
|
|
|
|
|
context 'when a sanitization attribute is present' do
|
|
|
|
|
# ATTENTION: We use `target` here because re-sanitization would change the order of attributes
|
|
|
|
|
let(:body) { '<a href="https://example.com" target="_blank">foo</a>' }
|
|
|
|
|
|
|
|
|
|
it 'adds sanitization attributes' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
<a href="https://example.com" rel="nofollow noreferrer noopener" target="_blank" title="https://example.com">foo</a>
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when changing an unrelated attribute' do
|
|
|
|
|
|
|
|
|
|
it "doesn't re-sanitizes the body" do
|
|
|
|
|
expect { article.update!(message_id: 'test') }.not_to change { article.reload.body }
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
2019-04-17 06:18:02 +00:00
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'for all cases above, combined' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
please tell me this doesn't work: <table>ada<tr></tr></table>
|
|
|
|
|
<div class="adasd" id="123" data-abc="123"></div>
|
|
|
|
|
<div>
|
|
|
|
|
<a href="javascript:someFunction()">LINK</a>
|
|
|
|
|
<a href="http://lalal.de">aa</a>
|
|
|
|
|
<some_not_existing>ABC</some_not_existing>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'performs all sanitizations' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
please tell me this doesn't work: <table>ada<tr></tr>
|
|
|
|
|
</table>
|
|
|
|
|
<div></div>
|
|
|
|
|
<div>
|
|
|
|
|
LINK
|
|
|
|
|
<a href="http://lalal.de" rel="nofollow noreferrer noopener" target="_blank" title="http://lalal.de">aa</a>
|
|
|
|
|
ABC</div>
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'for content_type: "text/plain"' do
|
|
|
|
|
subject(:article) { create(:ticket_article, body: body, content_type: 'text/plain') }
|
|
|
|
|
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
please tell me this doesn't work: <table>ada<tr></tr></table>
|
|
|
|
|
<div class="adasd" id="123" data-abc="123"></div>
|
|
|
|
|
<div>
|
|
|
|
|
<a href="javascript:someFunction()">LINK</a>
|
|
|
|
|
<a href="http://lalal.de">aa</a>
|
|
|
|
|
<some_not_existing>ABC</some_not_existing>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'performs no sanitizations' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
please tell me this doesn't work: <table>ada<tr></tr></table>
|
|
|
|
|
<div class="adasd" id="123" data-abc="123"></div>
|
|
|
|
|
<div>
|
|
|
|
|
<a href="javascript:someFunction()">LINK</a>
|
|
|
|
|
<a href="http://lalal.de">aa</a>
|
|
|
|
|
<some_not_existing>ABC</some_not_existing>
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains <video> element' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
please tell me this doesn't work: <video>some video</video><foo>alal</foo>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'leaves it as-is' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
please tell me this doesn't work: <video>some video</video>alal
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains CSS in style attribute' do
|
|
|
|
|
context 'for cid-style email attachment' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
<img style="width: 85.5px; height: 49.5px" src="cid:15.274327094.140938@zammad.example.com">
|
|
|
|
|
asdasd
|
|
|
|
|
<img src="cid:15.274327094.140939@zammad.example.com">
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'adds terminal semicolons to style rules' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
<img style="width: 85.5px; height: 49.5px;" src="cid:15.274327094.140938@zammad.example.com">
|
|
|
|
|
asdasd
|
|
|
|
|
<img src="cid:15.274327094.140939@zammad.example.com">
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'for relative-path-style email attachment' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
<img style="width: 85.5px; height: 49.5px" src="api/v1/ticket_attachment/123/123/123">
|
|
|
|
|
asdasd
|
|
|
|
|
<img src="api/v1/ticket_attachment/123/123/123">
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'adds terminal semicolons to style rules' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
<img style="width: 85.5px; height: 49.5px;" src="api/v1/ticket_attachment/123/123/123">
|
|
|
|
|
asdasd
|
|
|
|
|
<img src="api/v1/ticket_attachment/123/123/123">
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains <body> elements' do
|
|
|
|
|
let(:body) { '<body>123</body>' }
|
|
|
|
|
|
|
|
|
|
it 'removes <body> tags' do
|
|
|
|
|
expect(article.body).to eq('123')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'when body contains onclick attributes in <a> elements' do
|
|
|
|
|
let(:body) { <<~RAW.chomp }
|
|
|
|
|
<a href="#" onclick="some_function();">abc</a>
|
|
|
|
|
<a href="https://example.com" oNclIck="some_function();">123</a>
|
|
|
|
|
RAW
|
|
|
|
|
|
|
|
|
|
it 'removes onclick attributes' do
|
|
|
|
|
expect(article.body).to eq(<<~SANITIZED.chomp)
|
|
|
|
|
<a href="#">abc</a>
|
|
|
|
|
<a href="https://example.com" rel="nofollow noreferrer noopener" target="_blank" title="https://example.com">123</a>
|
|
|
|
|
SANITIZED
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
2019-04-10 17:17:42 +00:00
|
|
|
|
describe 'DoS protection:' do
|
|
|
|
|
context 'when #body exceeds 1.5MB' do
|
|
|
|
|
subject(:article) { create(:ticket_article, body: body) }
|
2019-04-15 01:41:17 +00:00
|
|
|
|
|
2019-04-10 17:17:42 +00:00
|
|
|
|
let(:body) { 'a' * 2_000_000 }
|
|
|
|
|
|
|
|
|
|
context 'for "web" thread', application_handle: 'web' do
|
|
|
|
|
it 'raises an Unprocessable Entity error' do
|
|
|
|
|
expect { article }.to raise_error(Exceptions::UnprocessableEntity)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
2020-07-10 14:54:04 +00:00
|
|
|
|
context 'for import' do
|
|
|
|
|
before do
|
|
|
|
|
Setting.set('import_mode', true)
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
it 'truncates body to 1.5 million chars' do
|
|
|
|
|
expect(article.body.length).to eq(1_500_000)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
2019-04-10 17:17:42 +00:00
|
|
|
|
context 'for "test.postmaster" thread', application_handle: 'test.postmaster' do
|
|
|
|
|
it 'truncates body to 1.5 million chars' do
|
|
|
|
|
expect(article.body.length).to eq(1_500_000)
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'with NULL bytes' do
|
2020-09-30 09:07:01 +00:00
|
|
|
|
let(:body) { "\u0000#{'a' * 2_000_000}" }
|
2019-04-10 17:17:42 +00:00
|
|
|
|
|
|
|
|
|
it 'still removes them, if necessary (postgres doesn’t like them)' do
|
|
|
|
|
expect(article).to be_persisted
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
it 'still truncates body' do
|
|
|
|
|
expect(article.body.length).to eq(1_500_000)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
2019-02-05 07:33:40 +00:00
|
|
|
|
describe 'Cti::Log syncing:' do
|
|
|
|
|
context 'with existing Log records' do
|
|
|
|
|
context 'for an incoming call from an unknown number' do
|
|
|
|
|
let!(:log) { create(:'cti/log', :with_preferences, from: '491111222222', direction: 'in') }
|
|
|
|
|
|
|
|
|
|
context 'with that number in #body' do
|
|
|
|
|
subject(:article) { build(:ticket_article, body: <<~BODY) }
|
|
|
|
|
some message
|
|
|
|
|
+49 1111 222222
|
|
|
|
|
BODY
|
|
|
|
|
|
|
|
|
|
it 'does not modify any Log records (because CallerIds from article bodies have #level "maybe")' do
|
|
|
|
|
expect do
|
|
|
|
|
article.save
|
|
|
|
|
Observer::Transaction.commit
|
|
|
|
|
Scheduler.worker(true)
|
|
|
|
|
end.not_to change { log.reload.attributes }
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
Refactoring: Automatic RSpec VCR cassette name helper
This commit was prepared to support upcoming additions to the test suite
(specifically, better coverage for existing Twitter functionality).
These upcoming changes will depend heavily on VCR.[0]
(VCR is a Ruby gem that makes it easier to write and run tests
that call out to external services over HTTP
by "recording" HTTP transactions to a YAML file
and "replaying" them later.)
VCR is widely-used (4600 GitHub stars), but its API is a little clumsy--
You have to manually specify the name of a "cassette" file every time:
it 'does something' do
VCR.use_cassette('path/to/cassette') do
...
end
end
This commit adds an RSpec metadata config option
as a shorthand for the syntax above:
it 'does something', :use_vcr do
...
end
This config option automatically generates a cassette filename
based on the description of the example it's applied to.
=== Analysis of alternative approaches
Ideally, these auto-generated cassette filenames should be unique:
if filenames collide, multiple examples will share the same cassette.
A first attempt generated names based on `example.full_description`,
but that led to errors:
Errno::ENAMETOOLONG:
File name too long @ rb_sysopen - /opt/zammad/test/data/vcr_cassettes/models/ticket/article/ticket_article_callbacks_observers_async_transactions_-_auto-setting_of_outgoing_twitter_article_attributes_via_bg_jobs_when_the_original_channel_specified_in_ticket_preferences_was_deleted_but_a_new_one_with_the_same_screen_name_exists_sets_appropriate_status_attributes_on_the_new_channel.yml
Another idea was to use MD5 digests of the above,
but in fact both of these approaches share another problem:
even minor changes to the description could break tests
(unless the committer remembers to rename the cassette file to match):
an altered description means VCR will record a new cassette file
instead of replaying from the original.
(Normally, this would only slow down the test instead of breaking it,
but sometimes we modify tests and cassettes after recording them
to hide sensitive data like API keys or login credentials.)
The approach taken by this commit was to use partial descriptions,
combining the parent `describe`/`context` label with the `it` label.
This does not guarantee uniqueness--
even in the present refactoring, it produced a filename collision--
but it's a good middle ground.
[0]: https://relishapp.com/vcr/vcr/docs
2019-11-12 08:17:21 +00:00
|
|
|
|
describe 'Auto-setting of outgoing Twitter article attributes (via bg jobs):', use_vcr: :with_oauth_headers do
|
2019-02-05 09:59:13 +00:00
|
|
|
|
subject!(:twitter_article) { create(:twitter_article, sender_name: 'Agent') }
|
2019-04-15 01:41:17 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
let(:channel) { Channel.find(twitter_article.ticket.preferences[:channel_id]) }
|
Refactoring: Automatic RSpec VCR cassette name helper
This commit was prepared to support upcoming additions to the test suite
(specifically, better coverage for existing Twitter functionality).
These upcoming changes will depend heavily on VCR.[0]
(VCR is a Ruby gem that makes it easier to write and run tests
that call out to external services over HTTP
by "recording" HTTP transactions to a YAML file
and "replaying" them later.)
VCR is widely-used (4600 GitHub stars), but its API is a little clumsy--
You have to manually specify the name of a "cassette" file every time:
it 'does something' do
VCR.use_cassette('path/to/cassette') do
...
end
end
This commit adds an RSpec metadata config option
as a shorthand for the syntax above:
it 'does something', :use_vcr do
...
end
This config option automatically generates a cassette filename
based on the description of the example it's applied to.
=== Analysis of alternative approaches
Ideally, these auto-generated cassette filenames should be unique:
if filenames collide, multiple examples will share the same cassette.
A first attempt generated names based on `example.full_description`,
but that led to errors:
Errno::ENAMETOOLONG:
File name too long @ rb_sysopen - /opt/zammad/test/data/vcr_cassettes/models/ticket/article/ticket_article_callbacks_observers_async_transactions_-_auto-setting_of_outgoing_twitter_article_attributes_via_bg_jobs_when_the_original_channel_specified_in_ticket_preferences_was_deleted_but_a_new_one_with_the_same_screen_name_exists_sets_appropriate_status_attributes_on_the_new_channel.yml
Another idea was to use MD5 digests of the above,
but in fact both of these approaches share another problem:
even minor changes to the description could break tests
(unless the committer remembers to rename the cassette file to match):
an altered description means VCR will record a new cassette file
instead of replaying from the original.
(Normally, this would only slow down the test instead of breaking it,
but sometimes we modify tests and cassettes after recording them
to hide sensitive data like API keys or login credentials.)
The approach taken by this commit was to use partial descriptions,
combining the parent `describe`/`context` label with the `it` label.
This does not guarantee uniqueness--
even in the present refactoring, it produced a filename collision--
but it's a good middle ground.
[0]: https://relishapp.com/vcr/vcr/docs
2019-11-12 08:17:21 +00:00
|
|
|
|
let(:run_bg_jobs) { -> { Scheduler.worker(true) } }
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'sets #from to sender’s Twitter handle' do
|
|
|
|
|
expect(&run_bg_jobs)
|
|
|
|
|
.to change { twitter_article.reload.from }
|
|
|
|
|
.to('@example')
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'sets #to to recipient’s Twitter handle' do
|
|
|
|
|
expect(&run_bg_jobs)
|
|
|
|
|
.to change { twitter_article.reload.to }
|
|
|
|
|
.to('') # Tweet in VCR cassette is addressed to no one
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
Fixes #2715 - Twitter status URLs are broken
This commit is a complement of its parent:
the parent fixes broken links to Twitter DMs,
while this one fixes broken links to Twitter statuses.
When importing tweets (articles of type "twitter status"),
Zammad generates a source link URL using the following format:
https://twitter.com/statuses/:id
and then stores that URL under `article.preferences[:links]`.
This URL template worked until as recently as 2017[0],
but currently fails for users actively logged in to Twitter.
Now, the correct URL template appears to be
https://twitter.com/:user_id/status/:id
where `:user_id` is not strict, and may be any word (\w+) <= 20 chars.
Try it yourself:
$ curl https://twitter.com/elonmusk/status/1069382411899817990
<html><body>You are being <a href="https://twitter.com/medenhofer/status/1069382411899817990">redirected</a>.</body></html>
In this commit, we replace `:user_id` with a single underscore (`_`).
This behavior is not officially documented anywhere (as far as I know),
but it works (for now).
This commit also extends the previous commit's DB migration/bg job
to rectify existing, broken tweet URLs stored in the database.
For performance purposes, this migration is performed in the background
and limited to the latest 10,000 Twitter articles.
[0]: https://stackoverflow.com/questions/41786123
GitHub: https://github.com/zammad/zammad/issues/2715
2019-11-14 06:03:34 +00:00
|
|
|
|
it 'sets #message_id to tweet ID (https://twitter.com/_/status/<id>)' do
|
2019-02-05 09:59:13 +00:00
|
|
|
|
expect(&run_bg_jobs)
|
|
|
|
|
.to change { twitter_article.reload.message_id }
|
|
|
|
|
.to('1069382411899817990')
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'sets #preferences with tweet metadata' do
|
|
|
|
|
expect(&run_bg_jobs)
|
|
|
|
|
.to change { twitter_article.reload.preferences }
|
|
|
|
|
.to(hash_including('twitter', 'links'))
|
|
|
|
|
|
|
|
|
|
expect(twitter_article.preferences[:links].first)
|
|
|
|
|
.to include(
|
|
|
|
|
'name' => 'on Twitter',
|
|
|
|
|
'target' => '_blank',
|
Fixes #2715 - Twitter status URLs are broken
This commit is a complement of its parent:
the parent fixes broken links to Twitter DMs,
while this one fixes broken links to Twitter statuses.
When importing tweets (articles of type "twitter status"),
Zammad generates a source link URL using the following format:
https://twitter.com/statuses/:id
and then stores that URL under `article.preferences[:links]`.
This URL template worked until as recently as 2017[0],
but currently fails for users actively logged in to Twitter.
Now, the correct URL template appears to be
https://twitter.com/:user_id/status/:id
where `:user_id` is not strict, and may be any word (\w+) <= 20 chars.
Try it yourself:
$ curl https://twitter.com/elonmusk/status/1069382411899817990
<html><body>You are being <a href="https://twitter.com/medenhofer/status/1069382411899817990">redirected</a>.</body></html>
In this commit, we replace `:user_id` with a single underscore (`_`).
This behavior is not officially documented anywhere (as far as I know),
but it works (for now).
This commit also extends the previous commit's DB migration/bg job
to rectify existing, broken tweet URLs stored in the database.
For performance purposes, this migration is performed in the background
and limited to the latest 10,000 Twitter articles.
[0]: https://stackoverflow.com/questions/41786123
GitHub: https://github.com/zammad/zammad/issues/2715
2019-11-14 06:03:34 +00:00
|
|
|
|
'url' => "https://twitter.com/_/status/#{twitter_article.message_id}"
|
2019-02-05 09:59:13 +00:00
|
|
|
|
)
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'does not change #cc' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.cc }
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'does not change #subject' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.subject }
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'does not change #content_type' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.content_type }
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'does not change #body' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.body }
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'does not change #sender' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.sender }
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
it 'does not change #type' do
|
|
|
|
|
expect(&run_bg_jobs).not_to change { twitter_article.reload.type }
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
it 'sets appropriate status attributes on the ticket’s channel' do
|
|
|
|
|
expect(&run_bg_jobs)
|
|
|
|
|
.to change { channel.reload.attributes }
|
|
|
|
|
.to hash_including(
|
|
|
|
|
'status_in' => nil,
|
|
|
|
|
'last_log_in' => nil,
|
|
|
|
|
'status_out' => 'ok',
|
|
|
|
|
'last_log_out' => ''
|
|
|
|
|
)
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
context 'when the original channel (specified in ticket.preferences) was deleted' do
|
|
|
|
|
context 'but a new one with the same screen_name exists' do
|
|
|
|
|
let(:new_channel) { create(:twitter_channel) }
|
|
|
|
|
|
|
|
|
|
before do
|
|
|
|
|
channel.destroy
|
|
|
|
|
|
|
|
|
|
expect(new_channel.options[:user][:screen_name])
|
|
|
|
|
.to eq(channel.options[:user][:screen_name])
|
2018-12-13 09:06:44 +00:00
|
|
|
|
end
|
|
|
|
|
|
2019-02-05 09:59:13 +00:00
|
|
|
|
it 'sets appropriate status attributes on the new channel' do
|
2018-12-13 09:06:44 +00:00
|
|
|
|
expect(&run_bg_jobs)
|
2019-02-05 09:59:13 +00:00
|
|
|
|
.to change { new_channel.reload.attributes }
|
2018-12-13 09:06:44 +00:00
|
|
|
|
.to hash_including(
|
2018-12-19 17:31:51 +00:00
|
|
|
|
'status_in' => nil,
|
|
|
|
|
'last_log_in' => nil,
|
|
|
|
|
'status_out' => 'ok',
|
|
|
|
|
'last_log_out' => ''
|
2018-12-13 09:06:44 +00:00
|
|
|
|
)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
2019-07-03 16:12:05 +00:00
|
|
|
|
|
|
|
|
|
describe 'Sending of outgoing emails', performs_jobs: true do
|
|
|
|
|
subject(:article) { create(:ticket_article, type_name: type, sender_name: sender) }
|
|
|
|
|
|
|
|
|
|
shared_examples 'sends email' do
|
|
|
|
|
it 'dispatches an email on creation (via TicketArticleCommunicateEmailJob)' do
|
|
|
|
|
expect { article }
|
|
|
|
|
.to have_enqueued_job(TicketArticleCommunicateEmailJob)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
shared_examples 'does not send email' do
|
|
|
|
|
it 'does not dispatch an email' do
|
|
|
|
|
expect { article }
|
|
|
|
|
.not_to have_enqueued_job(TicketArticleCommunicateEmailJob)
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'with "application_server" application handle', application_handle: 'application_server' do
|
|
|
|
|
context 'for type: "email"' do
|
|
|
|
|
let(:type) { 'email' }
|
|
|
|
|
|
|
|
|
|
context 'from sender: "Agent"' do
|
|
|
|
|
let(:sender) { 'Agent' }
|
|
|
|
|
|
|
|
|
|
include_examples 'sends email'
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'from sender: "Customer"' do
|
|
|
|
|
let(:sender) { 'Customer' }
|
|
|
|
|
|
|
|
|
|
include_examples 'does not send email'
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'for any other type' do
|
|
|
|
|
let(:type) { 'sms' }
|
|
|
|
|
|
|
|
|
|
context 'from any sender' do
|
|
|
|
|
let(:sender) { 'Agent' }
|
|
|
|
|
|
|
|
|
|
include_examples 'does not send email'
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'with "*.postmaster" application handle', application_handle: 'scheduler.postmaster' do
|
|
|
|
|
context 'for any type' do
|
|
|
|
|
let(:type) { 'email' }
|
|
|
|
|
|
|
|
|
|
context 'from any sender' do
|
|
|
|
|
let(:sender) { 'Agent' }
|
|
|
|
|
|
|
|
|
|
include_examples 'does not send email'
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
end
|
2019-02-10 08:40:55 +00:00
|
|
|
|
|
|
|
|
|
describe 'clone attachments' do
|
|
|
|
|
context 'of forwarded article' do
|
|
|
|
|
context 'via email' do
|
|
|
|
|
|
|
|
|
|
it 'only need to clone attached attachments' do
|
|
|
|
|
article_parent = create(:ticket_article,
|
|
|
|
|
type: Ticket::Article::Type.find_by(name: 'email'),
|
|
|
|
|
content_type: 'text/html',
|
|
|
|
|
body: '<img src="cid:15.274327094.140938@zammad.example.com"> some text',)
|
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file1_normally_should_be_an_image',
|
|
|
|
|
filename: 'some_file1.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
'Content-ID' => '15.274327094.140938@zammad.example.com',
|
|
|
|
|
'Content-Disposition' => 'inline',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file2_normally_should_be_an_image',
|
|
|
|
|
filename: 'some_file2.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
'Content-ID' => '15.274327094.140938_not_reffered@zammad.example.com',
|
|
|
|
|
'Content-Disposition' => 'inline',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
2019-03-11 17:17:08 +00:00
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file3_normally_should_be_an_image',
|
|
|
|
|
filename: 'some_file3.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
'Content-Disposition' => 'attached',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
2019-02-10 08:40:55 +00:00
|
|
|
|
article_new = create(:ticket_article)
|
|
|
|
|
UserInfo.current_user_id = 1
|
|
|
|
|
|
|
|
|
|
attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_attached_attachments: true)
|
|
|
|
|
|
2019-03-11 17:17:08 +00:00
|
|
|
|
expect(attachments.count).to eq(2)
|
2019-02-10 08:40:55 +00:00
|
|
|
|
expect(attachments[0].filename).to eq('some_file2.jpg')
|
2019-03-11 17:17:08 +00:00
|
|
|
|
expect(attachments[1].filename).to eq('some_file3.jpg')
|
2019-02-10 08:40:55 +00:00
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
|
|
|
|
|
context 'of trigger' do
|
|
|
|
|
context 'via email notifications' do
|
|
|
|
|
it 'only need to clone inline attachments used in body' do
|
|
|
|
|
article_parent = create(:ticket_article,
|
|
|
|
|
type: Ticket::Article::Type.find_by(name: 'email'),
|
|
|
|
|
content_type: 'text/html',
|
|
|
|
|
body: '<img src="cid:15.274327094.140938@zammad.example.com"> some text',)
|
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file1_normally_should_be_an_image',
|
|
|
|
|
filename: 'some_file1.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
'Content-ID' => '15.274327094.140938@zammad.example.com',
|
|
|
|
|
'Content-Disposition' => 'inline',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file2_normally_should_be_an_image',
|
|
|
|
|
filename: 'some_file2.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
'Content-ID' => '15.274327094.140938_not_reffered@zammad.example.com',
|
|
|
|
|
'Content-Disposition' => 'inline',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
2019-03-11 17:17:08 +00:00
|
|
|
|
|
|
|
|
|
# #2483 - #{article.body_as_html} now includes attachments (e.g. PDFs)
|
|
|
|
|
# Regular attachments do not get assigned a Content-ID, and should not be copied in this use case
|
|
|
|
|
Store.add(
|
|
|
|
|
object: 'Ticket::Article',
|
|
|
|
|
o_id: article_parent.id,
|
|
|
|
|
data: 'content_file3_with_no_content_id',
|
|
|
|
|
filename: 'some_file3.jpg',
|
|
|
|
|
preferences: {
|
|
|
|
|
'Content-Type' => 'image/jpeg',
|
|
|
|
|
'Mime-Type' => 'image/jpeg',
|
|
|
|
|
},
|
|
|
|
|
created_by_id: 1,
|
|
|
|
|
)
|
|
|
|
|
|
2019-02-10 08:40:55 +00:00
|
|
|
|
article_new = create(:ticket_article)
|
|
|
|
|
UserInfo.current_user_id = 1
|
|
|
|
|
|
|
|
|
|
attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_inline_attachments: true )
|
|
|
|
|
|
|
|
|
|
expect(attachments.count).to eq(1)
|
|
|
|
|
expect(attachments[0].filename).to eq('some_file1.jpg')
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
|
|
|
|
end
|
2018-12-13 09:06:44 +00:00
|
|
|
|
end
|