forked from extern/django-helpdesk
Prevent adding Users or Queue email to the CC list
This commit is contained in:
Garret Wassermann
parent
220c0460e1
commit
8963fa694a
@ -372,7 +372,16 @@ def ticket_from_message(message, queue, logger):
|
||||
if cc:
|
||||
# get list of currently CC'd emails
|
||||
current_cc = TicketCC.objects.filter(ticket=ticket)
|
||||
current_cc = set([x.email for x in current_cc])
|
||||
current_cc_emails = [x.email for x in current_cc]
|
||||
# get emails of any Users CC'd to email
|
||||
current_cc_users = [x.user.email for x in current_cc]
|
||||
# ensure submitter, assigned user, queue email not added
|
||||
other_emails = [queue.email_address]
|
||||
if t.submitter_email:
|
||||
other_emails.append(t.submitter_email)
|
||||
if t.assigned_to:
|
||||
other_emails.append(t.assigned_to.email)
|
||||
current_cc = set(current_cc_emails + current_cc_users + other_emails)
|
||||
# add any email in cc that's not already in current_cc (set difference)
|
||||
new_cc = cc.difference(current_cc)
|
||||
# add emails alphabetically, makes testing easy
|
||||
|
||||
@ -7,6 +7,8 @@ from django.test import TestCase
|
||||
from django.core.management import call_command
|
||||
from django.utils import six
|
||||
from django.shortcuts import get_object_or_404
|
||||
from django.contrib.auth.models import User
|
||||
from django.contrib.auth.hashers import make_password
|
||||
import itertools
|
||||
from shutil import rmtree
|
||||
import sys
|
||||
@ -44,13 +46,13 @@ class GetEmailCommonTests(TestCase):
|
||||
|
||||
|
||||
class GetEmailParametricTemplate(object):
|
||||
"""TestCase that checks email functionality across methods and socks configs."""
|
||||
"""TestCase that checks basic email functionality across methods and socks configs."""
|
||||
|
||||
def setUp(self):
|
||||
|
||||
self.temp_logdir = mkdtemp()
|
||||
kwargs = {
|
||||
"title": 'Queue 1',
|
||||
"title": 'Basic Queue',
|
||||
"slug": 'QQ',
|
||||
"allow_public_submission": True,
|
||||
"allow_email_submission": True,
|
||||
@ -221,100 +223,6 @@ class GetEmailParametricTemplate(object):
|
||||
self.assertEqual(ticket2.title, test_email_subject)
|
||||
self.assertEqual(ticket2.description, test_email_body)
|
||||
|
||||
def test_read_email_cc(self):
|
||||
"""Tests reading plain text emails from a queue and creating tickets,
|
||||
particularly to test appropriate handling of CC'd emails.
|
||||
For each email source supported, we mock the backend to provide
|
||||
authentically formatted responses containing our test data."""
|
||||
|
||||
# example email text from Django docs: https://docs.djangoproject.com/en/1.10/ref/unicode/
|
||||
test_email_from = "Arnbjörg Ráðormsdóttir <arnbjorg@example.com>"
|
||||
# NOTE: CC emails are in alphabetical order and must be tested as such!
|
||||
# implementation uses sets, so only way to ensure tickets created
|
||||
# in right order is to change set to list and sort it
|
||||
test_email_cc_one = "Alice Ráðormsdóttir <alice@example.com>"
|
||||
test_email_cc_two = "nobody@example.com"
|
||||
test_email_cc_three = "other@example.com"
|
||||
test_email_cc_four = "someone@example.com"
|
||||
test_email_subject = "My visit to Sør-Trøndelag"
|
||||
test_email_body = "Unicode helpdesk comment with an s-hat (ŝ) via email."
|
||||
test_email = "To: helpdesk@example.com\nCc: " + test_email_cc_one + ", " + test_email_cc_one + ", " + test_email_cc_two + ", " + test_email_cc_three + "\nCC: " + test_email_cc_one + ", " + test_email_cc_three + ", " + test_email_cc_four + "\nFrom: " + test_email_from + "\nSubject: " + test_email_subject + "\n\n" + test_email_body
|
||||
test_mail_len = len(test_email)
|
||||
|
||||
if self.socks:
|
||||
from socks import ProxyConnectionError
|
||||
with self.assertRaisesRegexp(ProxyConnectionError, '%s:%s' % (unrouted_socks_server, unused_port)):
|
||||
call_command('get_email')
|
||||
|
||||
else:
|
||||
# Test local email reading
|
||||
if self.method == 'local':
|
||||
with mock.patch('helpdesk.management.commands.get_email.listdir') as mocked_listdir, \
|
||||
mock.patch('helpdesk.management.commands.get_email.isfile') as mocked_isfile, \
|
||||
mock.patch('builtins.open' if six.PY3 else '__builtin__.open', mock.mock_open(read_data=test_email)):
|
||||
mocked_isfile.return_value = True
|
||||
mocked_listdir.return_value = ['filename1', 'filename2']
|
||||
|
||||
call_command('get_email')
|
||||
|
||||
mocked_listdir.assert_called_with('/var/lib/mail/helpdesk/')
|
||||
mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename1')
|
||||
mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename2')
|
||||
|
||||
elif self.method == 'pop3':
|
||||
# mock poplib.POP3's list and retr methods to provide responses as per RFC 1939
|
||||
pop3_emails = {
|
||||
'1': ("+OK", test_email.split('\n')),
|
||||
'2': ("+OK", test_email.split('\n')),
|
||||
}
|
||||
pop3_mail_list = ("+OK 2 messages", ("1 %d" % test_mail_len, "2 %d" % test_mail_len))
|
||||
mocked_poplib_server = mock.Mock()
|
||||
mocked_poplib_server.list = mock.Mock(return_value=pop3_mail_list)
|
||||
mocked_poplib_server.retr = mock.Mock(side_effect=lambda x: pop3_emails[x])
|
||||
with mock.patch('helpdesk.management.commands.get_email.poplib', autospec=True) as mocked_poplib:
|
||||
mocked_poplib.POP3 = mock.Mock(return_value=mocked_poplib_server)
|
||||
call_command('get_email')
|
||||
|
||||
elif self.method == 'imap':
|
||||
# mock imaplib.IMAP4's search and fetch methods with responses from RFC 3501
|
||||
imap_emails = {
|
||||
"1": ("OK", (("1", test_email),)),
|
||||
"2": ("OK", (("2", test_email),)),
|
||||
}
|
||||
imap_mail_list = ("OK", ("1 2",))
|
||||
mocked_imaplib_server = mock.Mock()
|
||||
mocked_imaplib_server.search = mock.Mock(return_value=imap_mail_list)
|
||||
|
||||
# we ignore the second arg as the data item/mime-part is constant (RFC822)
|
||||
mocked_imaplib_server.fetch = mock.Mock(side_effect=lambda x, _: imap_emails[x])
|
||||
with mock.patch('helpdesk.management.commands.get_email.imaplib', autospec=True) as mocked_imaplib:
|
||||
mocked_imaplib.IMAP4 = mock.Mock(return_value=mocked_imaplib_server)
|
||||
call_command('get_email')
|
||||
|
||||
ticket1 = get_object_or_404(Ticket, pk=1)
|
||||
self.assertEqual(ticket1.ticket_for_url, "QQ-%s" % ticket1.id)
|
||||
self.assertEqual(ticket1.title, test_email_subject)
|
||||
self.assertEqual(ticket1.description, test_email_body)
|
||||
cc1 = get_object_or_404(TicketCC, pk=1)
|
||||
self.assertEqual(cc1.email, test_email_cc_one)
|
||||
cc2 = get_object_or_404(TicketCC, pk=2)
|
||||
self.assertEqual(cc2.email, test_email_cc_two)
|
||||
cc3 = get_object_or_404(TicketCC, pk=3)
|
||||
self.assertEqual(cc3.email, test_email_cc_three)
|
||||
cc4 = get_object_or_404(TicketCC, pk=4)
|
||||
# the second CC has an extra test_email_cc_one and three that
|
||||
# should not be saved again, so the 4th email should be
|
||||
# test_email_cc_four
|
||||
self.assertEqual(cc4.email, test_email_cc_four)
|
||||
# ensure these 4 CCs are the only ones created
|
||||
# (otherwise, there were duplicates)
|
||||
self.assertEqual(len(TicketCC.objects.filter(ticket=1)), 4)
|
||||
|
||||
ticket2 = get_object_or_404(Ticket, pk=2)
|
||||
self.assertEqual(ticket2.ticket_for_url, "QQ-%s" % ticket2.id)
|
||||
self.assertEqual(ticket2.title, test_email_subject)
|
||||
self.assertEqual(ticket2.description, test_email_body)
|
||||
|
||||
def test_read_html_multipart_email(self):
|
||||
"""Tests reading multipart MIME (HTML body and plain text alternative)
|
||||
emails from a queue and creating tickets.
|
||||
@ -447,6 +355,121 @@ class GetEmailParametricTemplate(object):
|
||||
self.assertEqual(attach2.followup.id, 2)
|
||||
self.assertEqual(attach2.filename, 'email_html_body.html')
|
||||
|
||||
class GetEmailCCHandling(TestCase):
|
||||
"""TestCase that checks CC handling in email. Needs its own test harness."""
|
||||
|
||||
def setUp(self):
|
||||
|
||||
self.temp_logdir = mkdtemp()
|
||||
|
||||
kwargs = {
|
||||
"title": 'CC Queue',
|
||||
"slug": 'CC',
|
||||
"allow_public_submission": True,
|
||||
"allow_email_submission": True,
|
||||
"email_box_type": 'local',
|
||||
"email_box_local_dir": '/var/lib/mail/helpdesk/',
|
||||
"logging_dir": self.temp_logdir,
|
||||
"logging_type": 'none'
|
||||
}
|
||||
self.queue_public = Queue.objects.create(**kwargs)
|
||||
|
||||
user1_kwargs = {
|
||||
'username': 'staff',
|
||||
'email': 'staff@example.com',
|
||||
'password': make_password('Test1234'),
|
||||
'is_staff': True,
|
||||
'is_superuser': False,
|
||||
'is_active': True
|
||||
}
|
||||
self.staff_user = User.objects.create(**user1_kwargs)
|
||||
|
||||
user2_kwargs = {
|
||||
'username': 'assigned',
|
||||
'email': 'assigned@example.com',
|
||||
'password': make_password('Test1234'),
|
||||
'is_staff': True,
|
||||
'is_superuser': False,
|
||||
'is_active': True
|
||||
}
|
||||
self.assigned_user = User.objects.create(**user2_kwargs)
|
||||
|
||||
ticket_kwargs = {
|
||||
'title': 'Original Ticket',
|
||||
'queue': self.queue_public,
|
||||
'submitter_email': 'submitter@example.com',
|
||||
'assigned_to': self.assigned_user,
|
||||
'status': 1
|
||||
}
|
||||
self.original_ticket = Ticket.objects.create(**ticket_kwargs)
|
||||
|
||||
cc_kwargs = {
|
||||
'ticket': self.original_ticket,
|
||||
'user': self.staff_user,
|
||||
'can_view': True,
|
||||
'can_update': True
|
||||
}
|
||||
self.original_cc = TicketCC.objects.create(**cc_kwargs)
|
||||
|
||||
|
||||
def tearDown(self):
|
||||
|
||||
rmtree(self.temp_logdir)
|
||||
|
||||
def test_read_email_cc(self):
|
||||
"""Tests reading plain text emails from a queue and adding to a ticket,
|
||||
particularly to test appropriate handling of CC'd emails."""
|
||||
|
||||
# first, check that test ticket exists
|
||||
ticket1 = get_object_or_404(Ticket, pk=1)
|
||||
self.assertEqual(ticket1.ticket_for_url, "CC-1")
|
||||
self.assertEqual(ticket1.title, "Original Ticket")
|
||||
# only the staff_user is CC'd for now
|
||||
self.assertEqual(len(TicketCC.objects.filter(ticket=1)), 1)
|
||||
|
||||
# example email text from Django docs: https://docs.djangoproject.com/en/1.10/ref/unicode/
|
||||
test_email_from = "Arnbjörg Ráðormsdóttir <arnbjorg@example.com>"
|
||||
# NOTE: CC emails are in alphabetical order and must be tested as such!
|
||||
# implementation uses sets, so only way to ensure tickets created
|
||||
# in right order is to change set to list and sort it
|
||||
test_email_cc_one = "Alice Ráðormsdóttir <alice@example.com>"
|
||||
test_email_cc_two = "nobody@example.com"
|
||||
test_email_cc_three = "other@example.com"
|
||||
test_email_cc_four = "someone@example.com"
|
||||
ticket_user_emails = "assigned@example.com, staff@example.com, submitter@example.com"
|
||||
test_email_subject = "[CC-1] My visit to Sør-Trøndelag"
|
||||
test_email_body = "Unicode helpdesk comment with an s-hat (ŝ) via email."
|
||||
test_email = "To: helpdesk@example.com\nCc: " + test_email_cc_one + ", " + test_email_cc_one + ", " + test_email_cc_two + ", " + test_email_cc_three + "\nCC: " + test_email_cc_one + ", " + test_email_cc_three + ", " + test_email_cc_four + ", " + ticket_user_emails + "\nFrom: " + test_email_from + "\nSubject: " + test_email_subject + "\n\n" + test_email_body
|
||||
test_mail_len = len(test_email)
|
||||
|
||||
with mock.patch('helpdesk.management.commands.get_email.listdir') as mocked_listdir, \
|
||||
mock.patch('helpdesk.management.commands.get_email.isfile') as mocked_isfile, \
|
||||
mock.patch('builtins.open' if six.PY3 else '__builtin__.open', mock.mock_open(read_data=test_email)):
|
||||
mocked_isfile.return_value = True
|
||||
mocked_listdir.return_value = ['filename1']
|
||||
|
||||
call_command('get_email')
|
||||
|
||||
mocked_listdir.assert_called_with('/var/lib/mail/helpdesk/')
|
||||
mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename1')
|
||||
|
||||
# next we make sure no duplicates were added, and the
|
||||
# staff users nor submitter were not re-added as email TicketCCs
|
||||
cc1 = get_object_or_404(TicketCC, pk=2)
|
||||
self.assertEqual(cc1.email, test_email_cc_one)
|
||||
cc2 = get_object_or_404(TicketCC, pk=3)
|
||||
self.assertEqual(cc2.email, test_email_cc_two)
|
||||
cc3 = get_object_or_404(TicketCC, pk=4)
|
||||
self.assertEqual(cc3.email, test_email_cc_three)
|
||||
cc4 = get_object_or_404(TicketCC, pk=5)
|
||||
self.assertEqual(cc4.email, test_email_cc_four)
|
||||
# ensure these 4 CCs (test_email_cc one thru four) are the only ones
|
||||
# created and added to the existing staff_user that was CC'd,
|
||||
# and that submitter and assignee are not added as CC either
|
||||
# (in other words, even though everyone was CC'd to this email,
|
||||
# we should come out with only 5 CCs after filtering)
|
||||
self.assertEqual(len(TicketCC.objects.filter(ticket=1)), 5)
|
||||
|
||||
|
||||
# build matrix of test cases
|
||||
case_methods = [c[0] for c in Queue._meta.get_field('email_box_type').choices]
|
||||
@ -466,7 +489,8 @@ for method, socks in case_matrix:
|
||||
test_name = str(
|
||||
"TestGetEmail%s%s" % (method.capitalize(), socks_str))
|
||||
|
||||
cl = type(test_name, (GetEmailParametricTemplate, TestCase,), {
|
||||
"method": method,
|
||||
"socks": socks})
|
||||
cl = type(test_name,
|
||||
(GetEmailParametricTemplate,TestCase,),
|
||||
{ "method": method, "socks": socks}
|
||||
)
|
||||
setattr(thismodule, test_name, cl)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user