Allow accessing attachments of merged tickets by old ticket_id of merged ticket.

app/controllers/ticket_articles_controller.rb

@@ -214,7 +214,16 @@ class TicketArticlesController < ApplicationController
     end
     article = Ticket::Article.find(params[:article_id])
     if ticket.id != article.ticket_id
-      raise Exceptions::NotAuthorized, 'No access, article_id/ticket_id is not matching.'
+
+      # check if requested ticket got merged
+      if ticket.state.state_type.name != 'merged'
+        raise Exceptions::NotAuthorized, 'No access, article_id/ticket_id is not matching.'
+      end
+
+      ticket = article.ticket
+      if !ticket_permission(ticket)
+        raise Exceptions::NotAuthorized, "No access, for ticket_id '#{ticket.id}'."
+      end
     end
 
     list = article.attachments || []

test/controllers/ticket_article_attachments_controller_test.rb

@@ -0,0 +1,144 @@
+# encoding: utf-8
+require 'test_helper'
+
+class TicketArticleAttachmentsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+
+    # create agent
+    roles  = Role.where(name: %w(Admin Agent))
+    groups = Group.all
+
+    UserInfo.current_user_id = 1
+    @admin = User.create_or_update(
+      login: 'tickets-admin',
+      firstname: 'Tickets',
+      lastname: 'Admin',
+      email: 'tickets-admin@example.com',
+      password: 'adminpw',
+      active: true,
+      roles: roles,
+      groups: groups,
+    )
+
+    # create agent
+    roles = Role.where(name: 'Agent')
+    @agent = User.create_or_update(
+      login: 'tickets-agent@example.com',
+      firstname: 'Tickets',
+      lastname: 'Agent',
+      email: 'tickets-agent@example.com',
+      password: 'agentpw',
+      active: true,
+      roles: roles,
+      groups: groups,
+    )
+
+    # create customer without org
+    roles = Role.where(name: 'Customer')
+    @customer_without_org = User.create_or_update(
+      login: 'tickets-customer1@example.com',
+      firstname: 'Tickets',
+      lastname: 'Customer1',
+      email: 'tickets-customer1@example.com',
+      password: 'customer1pw',
+      active: true,
+      roles: roles,
+    )
+
+  end
+
+  test '01.01 test attachment urls' do
+    ticket1 = Ticket.create(
+      title: 'attachment test 1',
+      group: Group.lookup(name: 'Users'),
+      customer_id: @customer_without_org.id,
+      state: Ticket::State.lookup(name: 'new'),
+      priority: Ticket::Priority.lookup(name: '2 normal'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    article1 = Ticket::Article.create(
+      ticket_id: ticket1.id,
+      from: 'some_customer_com-1@example.com',
+      to: 'some_zammad_com-1@example.com',
+      subject: 'attachment test 1-1',
+      message_id: 'some@id_com_1',
+      body: 'some message 123',
+      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,
+    )
+    store1 = Store.add(
+      object: 'Ticket::Article',
+      o_id: article1.id,
+      data: 'some content',
+      filename: 'some_file.txt',
+      preferences: {
+        'Content-Type' => 'text/plain',
+      },
+      created_by_id: 1,
+    )
+    article2 = Ticket::Article.create(
+      ticket_id: ticket1.id,
+      from: 'some_customer_com-1@example.com',
+      to: 'some_zammad_com-1@example.com',
+      subject: 'attachment test 1-2',
+      message_id: 'some@id_com_1',
+      body: 'some message 123',
+      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,
+    )
+
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket1.id}/#{article1.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(200)
+    assert_equal('some content', @response.body)
+
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket1.id}/#{article2.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(401)
+    assert_match(/401: Unauthorized/, @response.body)
+
+    ticket2 = Ticket.create(
+      title: 'attachment test 2',
+      group: Group.lookup(name: 'Users'),
+      customer_id: @customer_without_org.id,
+      state: Ticket::State.lookup(name: 'new'),
+      priority: Ticket::Priority.lookup(name: '2 normal'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    ticket1.merge_to(
+      ticket_id: ticket2.id,
+      user_id:   1,
+    )
+
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket2.id}/#{article1.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(200)
+    assert_equal('some content', @response.body)
+
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket2.id}/#{article2.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(401)
+    assert_match(/401: Unauthorized/, @response.body)
+
+    # allow access via merged ticket id also
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket1.id}/#{article1.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(200)
+    assert_equal('some content', @response.body)
+
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    get "/api/v1/ticket_attachment/#{ticket1.id}/#{article2.id}/#{store1.id}", {}, 'Authorization' => credentials
+    assert_response(401)
+    assert_match(/401: Unauthorized/, @response.body)
+
+  end
+
+end

test/controllers/ticket_articles_controller_test.rb

@@ -75,7 +75,7 @@ class TicketArticlesControllerTest < ActionDispatch::IntegrationTest
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_equal('some body', result['body'])
     assert_equal('text/plain', result['content_type'])
     assert_equal(@agent.id, result['updated_by_id'])
@@ -98,7 +98,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_no_match(/some body <img src="cid:.+?/, result['body'])
     assert_match(%r{some body <img src="/api/v1/ticket_attachment/.+?" alt="Red dot"}, result['body'])
     assert_equal('text/html', result['content_type'])
@@ -131,7 +131,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_equal('some body', result['body'])
     assert_equal('text/html', result['content_type'])
     assert_equal(@agent.id, result['updated_by_id'])
@@ -178,7 +178,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_equal('some body', result['body'])
     assert_equal('text/plain', result['content_type'])
     assert_equal(@customer_without_org.id, result['updated_by_id'])
@@ -201,7 +201,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_equal('some body', result['body'])
     assert_equal('text/plain', result['content_type'])
     assert_equal(@customer_without_org.id, result['updated_by_id'])
@@ -227,7 +227,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_response(201)
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
-    assert_equal(nil, result['subject'])
+    assert_nil(result['subject'])
     assert_equal('some body 2', result['body'])
     assert_equal('text/plain', result['content_type'])
     assert_equal(@customer_without_org.id, result['updated_by_id'])

test/controllers/user_organization_controller_test.rb

@@ -615,13 +615,13 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
-    assert_equal(result['name'], nil)
+    assert_nil(result['name'])
 
     get "/api/v1/organizations/#{@organization2.id}", {}, @headers.merge('Authorization' => credentials)
     assert_response(200)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
-    assert_equal(result['name'], nil)
+    assert_nil(result['name'])
 
     # search
     Scheduler.worker(true)
@@ -651,7 +651,7 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest
     assert_response(401)
     result = JSON.parse(@response.body)
     assert_equal(result.class, Hash)
-    assert_equal(result['name'], nil)
+    assert_nil(result['name'])
 
     # search
     Scheduler.worker(true)