From d394a5f1fe5a0750878b0f85e29c56d692dc73a4 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 4 Apr 2017 18:15:29 +0200 Subject: [PATCH] Allow accessing attachments of merged tickets by old ticket_id of merged ticket. --- app/controllers/ticket_articles_controller.rb | 11 +- ...ket_article_attachments_controller_test.rb | 144 ++++++++++++++++++ .../ticket_articles_controller_test.rb | 12 +- .../user_organization_controller_test.rb | 6 +- 4 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 test/controllers/ticket_article_attachments_controller_test.rb diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index 7c00282b4..9ef12fe24 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/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 || [] diff --git a/test/controllers/ticket_article_attachments_controller_test.rb b/test/controllers/ticket_article_attachments_controller_test.rb new file mode 100644 index 000000000..5183a6abe --- /dev/null +++ b/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 diff --git a/test/controllers/ticket_articles_controller_test.rb b/test/controllers/ticket_articles_controller_test.rb index e46de52a0..b32ed6120 100644 --- a/test/controllers/ticket_articles_controller_test.rb +++ b/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 Red dot 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)