Commit 1c712b41 authored by Cédric Anne's avatar Cédric Anne Committed by Johan Cwiklinski
Browse files

Fix items ID fetching from email headers

- add a Message-Id on all notifications;
- keep old Message-Id format for Ticket notification (to ensure backward compatibility);
- parse In-Reply-To/References headers using the original Message-Id pattern;
- add tests
parent c6944271
......@@ -968,15 +968,13 @@ class MailCollector extends CommonDBTM {
$tkt['date'] = $headers['date'];
}
// Detect if it is a mail reply
$glpi_message_match = "/GLPI-([0-9]+)\.[0-9]+\.[0-9]+@\w*/";
// Check if email not send by GLPI : if yes -> blacklist
if (!isset($headers['message_id'])
|| preg_match($glpi_message_match, $headers['message_id'], $match)) {
if ($this->isMessageSentByGlpi($message)) {
// Message was sent by GLPI.
// Message is blacklisted to avoid infinite loop (where GLPI creates a ticket from its own notification).
$tkt['_blacklisted'] = true;
return $tkt;
}
// manage blacklist
$blacklisted_emails = Blacklist::getEmails();
// Add name of the mailcollector as blacklisted
......@@ -1071,23 +1069,10 @@ class MailCollector extends CommonDBTM {
$tkt['content'] = $body;
}
// prepare match to find ticket id in headers
// header is added in all notifications using pattern: GLPI-{itemtype}-{items_id}
$ref_match = "/GLPI-Ticket-([0-9]+)/";
// See In-Reply-To field
if (isset($headers['in_reply_to'])) {
if (preg_match($ref_match, $headers['in_reply_to'], $match)) {
$tkt['tickets_id'] = intval($match[1]);
}
}
// See in References
if (!isset($tkt['tickets_id'])
&& isset($headers['references'])) {
if (preg_match($ref_match, $headers['references'], $match)) {
$tkt['tickets_id'] = intval($match[1]);
}
// Search for referenced item in headers
$found_item = $this->getItemFromHeaders($message);
if ($found_item instanceof Ticket) {
$tkt['tickets_id'] = $found_item->fields['id'];
}
// See in title
......@@ -1960,6 +1945,93 @@ class MailCollector extends CommonDBTM {
return self::countCollectors(true);
}
/**
* Try to retrieve an existing item from references in message headers.
* References corresponds to original MessageId sent by GLPI.
*
* @param Message $message
*
* @since 9.5.4
*
* @return CommonDBTM|null
*/
public function getItemFromHeaders(Message $message): ?CommonDBTM {
$pattern = $this->getMessageIdExtractPattern();
foreach (['in_reply_to', 'references'] as $header_name) {
$matches = [];
if ($message->getHeaders()->has($header_name)
&& preg_match($pattern, $message->getHeader($header_name)->getFieldValue(), $matches)) {
$itemtype = $matches['itemtype'];
$items_id = $matches['items_id'];
// Handle old format MessageId where itemtype was not in header
if (empty($itemtype) && !empty($items_id)) {
$itemtype = Ticket::getType();
}
if (empty($itemtype) || !class_exists($itemtype) || !is_a($itemtype, CommonDBTM::class, true)) {
// itemtype not found or invalid
continue;
}
$item = new $itemtype();
if (!empty($items_id) && $item->getFromDB($items_id)) {
return $item;
}
}
}
return null;
}
/**
* Check if message was sent by current instance of GLPI.
* This can be verified by checking the MessageId header.
*
* @param Message $message
*
* @since 9.5.4
*
* @return bool
*/
public function isMessageSentByGlpi(Message $message): bool {
$pattern = $this->getMessageIdExtractPattern();
if (!$message->getHeaders()->has('message-id')) {
// Messages sent by GLPI now have always a message-id header.
return false;
}
$message_id = $message->getHeader('message_id')->getFieldValue();
$matches = [];
if (!preg_match($pattern, $message_id, $matches)) {
// message-id header does not match GLPI format.
return false;
}
return true;
}
/**
* Get pattern that can be used to extract informations from a GLPI MessageId (itemtype and items_id).
*
* @see NotificationTarget::getMessageID()
*
* @return string
*/
private function getMessageIdExtractPattern(): string {
// old format: GLPI-{$items_id}.{$time}.{$rand}@{$uname}
// without related item: GLPI.{$time}.{$rand}@{$uname}
// with related item: GLPI-{$itemtype}-{$items_id}.{$time}.{$rand}@{$uname}
return '/GLPI'
. '(-(?<itemtype>[a-z]+))?' // itemtype is not present if notification is not related to any object and was not present in old format
. '(-(?<items_id>[0-9]+))?' // items_id is not present if notification is not related to any object
. '\.[0-9]+' // time()
. '\.[0-9]+' // rand()
. '@\w*' // uname
. '/i'; // insensitive
}
/**
* @param $name
......
......@@ -234,12 +234,30 @@ class NotificationTarget extends CommonDBChild {
}
/**
* Get a unique message id.
*
* @since 0.84
*
* @return message id for notification
**/
* @return string
*/
function getMessageID() {
return '';
if ($this->obj instanceof CommonDBTM) {
return sprintf(
'GLPI-%s-%s.%s.%s@%s',
$this->obj->getType(),
$this->obj->getField('id'),
time(),
rand(),
php_uname('n')
);
}
return sprintf(
'GLPI.%s.%s@%s',
time(),
rand(),
php_uname('n')
);
}
......
......@@ -97,7 +97,7 @@ function loadDataset() {
// Unit test data definition
$data = [
// bump this version to force reload of the full dataset, when content change
'_version' => '4.5',
'_version' => '4.6',
// Type => array of entries
'Entity' => [
......@@ -325,7 +325,21 @@ function loadDataset() {
'content' => 'Content for ticket _ticket03',
'users_id_recipient' => TU_USER,
'entities_id' => '_test_child_1'
]
],
[
'id' => 100, // Force ID that will be used in imap test suite fixtures
'name' => '_ticket100',
'content' => 'Content for ticket _ticket100',
'users_id_recipient' => TU_USER,
'entities_id' => '_test_root_entity'
],
[
'id' => 101, // Force ID that will be used in imap test suite fixtures
'name' => '_ticket101',
'content' => 'Content for ticket _ticket101',
'users_id_recipient' => TU_USER,
'entities_id' => '_test_root_entity'
],
], 'TicketTask' => [
[
'tickets_id' => '_ticket01',
......
Return-Path: tech@glpi-project.org
Received: from 192.168.1.3 (LHLO mail.glpi-project.org) (192.168.1.3)
by mail.glpi-project.org with LMTP; Wed, 3 Apr 2019 11:48:35 +0200
(CEST)
Received: from mail.glpi-project.org (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id B74527E805E8
for <tech@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Received: from localhost (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id A9D7F7E805FF
for <tech@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Received: from mail.glpi-project.org ([127.0.0.1])
by localhost (mail.glpi-project.org [127.0.0.1]) (amavisd-new, port 10026)
with ESMTP id kjAq2bBt1e-c for <tech@glpi-project.org>;
Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Received: from [192.168.1.141] (ian34-1-78-198-202-105.fbx.proxad.net [78.198.202.105])
by mail.glpi-project.org (Postfix) with ESMTPSA id 7C7AF7E805E8
for <tech@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Subject: Re: [GLPI #0001155] New ticket database issue
References: <GLPI-1155.1554270788.1601670865@mycomputer>
From: Tech Ni Cian <tech@glpi-project.org>
To: tech@glpi-project.org
Message-ID: <961c2885-94e6-2343-b000-280c809fcdb1@glpi-project.org>
Date: Wed, 3 Apr 2019 11:48:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <GLPI-Ticket-1155.1554270788.1601670865@mycomputer>
Content-Type: text/plain; charset=utf-8
Content-Language: fr-FR
Content-Transfer-Encoding: quoted-printable
Le 03/04/2019 =C3=A0 10:33, admsys@mycomputer a =C3=A9crit=C2=A0:
> =3D-=3D-=3D-=3D To answer by email, write above this line =3D-=3D-=3D-=3D
>=20
> URL : http://localhost:8088/index.php?redirect=3Dticket_1155&noAUTO=3D1
>=20
> *Ticket: Description*
>=20
> Title=C2=A0:database issue
> Requesters=C2=A0: Nician
> Opening date=C2=A0:2019-04-03 06:51
> Closing date=C2=A0:
> Request source=C2=A0:Phone
>=20
> Associated item=C2=A0:
>=20
> Assigned to technicians=C2=A0: glpi
> Status =C2=A0: Processing (assigned)
>=20
> Urgency=C2=A0: Medium
> Impact=C2=A0: Medium
> Priority=C2=A0: Medium
>=20
> No defined category
> Description=C2=A0:
>=20
> It seems one field from the last migration has not been added in my
> database.
>=20
>=20
> Number of followups=C2=A0: 0
>=20
> Number of tasks=C2=A0: 0
>=20
>=20
>=20
> --=20
> SIGNATURE
> Automatically generated by GLPI
>=20
> =3D_=3D_=3D_=3D To answer by email, write under this line =3D_=3D_=3D_=3D
You can force the migration again using the --force flag from the CLI:
$ bin/console db:update --force
Cheers.
Return-Path: tech@glpi-project.org
Received: from mail.glpi-project.org (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id B74527E805E8
for <unittests@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Subject: Re: New database issue
From: Tech Ni Cian <tech@glpi-project.org>
To: GLPI debug <unittests@glpi-project.org>
Message-ID: <961c2885-94e6-2343-b000-280c809fcdb1@glpi-project.org>
Date: Wed, 3 Apr 2019 11:48:25 +0200
MIME-Version: 1.0
In-Reply-To: <GLPI-Ticket-100.1554270788.1601670865@mycomputer>
Content-Type: text/plain; charset=utf-8
Content-Language: fr-FR
Content-Transfer-Encoding: quoted-printable
This is a reply that references Ticket 100 in In-Reply-To header.
It should be added as followup.
Return-Path: tech@glpi-project.org
Received: from mail.glpi-project.org (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id B74527E805E8
for <unittests@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Subject: Re: New database issue
References: <GLPI-Ticket-100.1554270788.1601670865@mycomputer>
From: Tech Ni Cian <tech@glpi-project.org>
To: GLPI debug <unittests@glpi-project.org>
Message-ID: <961c2885-94e6-2343-b000-280c809fcdb1@glpi-project.org>
Date: Wed, 3 Apr 2019 11:48:25 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Language: fr-FR
Content-Transfer-Encoding: quoted-printable
This is a reply that references Ticket 100 in References header.
It should be added as followup.
Return-Path: tech@glpi-project.org
Received: from mail.glpi-project.org (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id B74527E805E8
for <unittests@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Subject: Re: [GLPI #0000101] New database issue
From: Tech Ni Cian <tech@glpi-project.org>
To: GLPI debug <unittests@glpi-project.org>
Message-ID: <961c2885-94e6-2343-b000-280c809fcdb1@glpi-project.org>
Date: Wed, 3 Apr 2019 11:48:25 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Language: fr-FR
Content-Transfer-Encoding: quoted-printable
This is a reply that references Ticket 101 in its subject.
It should be added as followup.
Return-Path: noreply@glpi-project.org
Received: from mail.glpi-project.org (localhost [127.0.0.1])
by mail.glpi-project.org (Postfix) with ESMTP id B74527E805E8
for <tech@glpi-project.org>; Wed, 3 Apr 2019 11:48:35 +0200 (CEST)
Subject: [GLPI #0000001] New ticket
From: noreply@glpi-project.org
To: tech@glpi-project.org
Message-ID: <GLPI-Ticket-1.1611674092.1756143577@localhost>
Date: Wed, 3 Apr 2019 11:48:25 +0200
MIME-Version: 1.0
In-Reply-To: <GLPI-Ticket-1>
Content-Type: text/plain; charset=utf-8
Content-Language: fr-FR
Content-Transfer-Encoding: quoted-printable
This is a notification for a new ticket.
It should be blacklisted to prevent infinite loop.
......@@ -33,6 +33,13 @@
namespace tests\units;
use \DbTestCase;
use ITILFollowup;
use Laminas\Mail\Storage\Message;
use NotificationTarget;
use NotificationTargetSoftwareLicense;
use NotificationTargetTicket;
use SoftwareLicense;
use Ticket;
class MailCollector extends DbTestCase {
private $collector;
......@@ -189,6 +196,198 @@ class MailCollector extends DbTestCase {
$this->integer($this->testedInstance->countCollectors())->isIdenticalTo(1);
}
protected function messageIdHeaderProvider() {
$root_ent_id = getItemByTypeName('Entity', '_test_root_entity', true);
$ticket_notif = new NotificationTargetTicket($root_ent_id, 'test_event', getItemByTypeName('Ticket', '_ticket01'));
$soft_notif = new NotificationTargetSoftwareLicense($root_ent_id, 'test_event', getItemByTypeName('SoftwareLicense', '_test_softlic_1'));
$base_notif = new NotificationTarget();
return [
[
'headers' => [],
'expected' => false,
],
[
'headers' => [
'message-id' => 'donotknow',
],
'expected' => false,
],
[
'headers' => [
'message-id' => $ticket_notif->getMessageID(), // ticket format
],
'expected' => true,
],
[
'headers' => [
'message-id' => $soft_notif->getMessageID(), // new format with object relation
],
'expected' => true,
],
[
'headers' => [
'message-id' => $base_notif->getMessageID(), // new format without object relation
],
'expected' => true,
],
];
}
/**
* @dataProvider messageIdHeaderProvider
*/
public function testIsMessageSentByGlpi(array $headers, bool $expected) {
$this->newTestedInstance();
$message = new Message(
[
'headers' => $headers,
'content' => 'Message contents...',
]
);
$this->boolean($this->testedInstance->isMessageSentByGlpi($message))->isEqualTo($expected);
}
protected function itemReferenceHeaderProvider() {
$root_ent_id = getItemByTypeName('Entity', '_test_root_entity', true);
$ticket_id = getItemByTypeName('Ticket', '_ticket01', true);
$ticket_notif = new NotificationTargetTicket($root_ent_id, 'test_event', getItemByTypeName('Ticket', '_ticket01'));
$soft_id = getItemByTypeName('SoftwareLicense', '_test_softlic_1', true);
$soft_notif = new NotificationTargetSoftwareLicense($root_ent_id, 'test_event', getItemByTypeName('SoftwareLicense', '_test_softlic_1'));
$time1 = time() - 548;
$time2 = $time1 - 1567;
$rand1 = rand();
$rand2 = rand();
$uname1 = 'localhost';
$uname2 = 'mail.glpi-project.org';
return [
// invalid header
[
'headers' => [
'in-reply-to' => 'notavalidvalue',
'references' => 'donotknow',
],
'expected_itemtype' => null,
'expected_items_id' => null,
'accepted' => true,
],
// ticket header format - found item
[
'headers' => [
'in-reply-to' => $ticket_notif->getMessageID(),
],
'expected_itemtype' => Ticket::class,
'expected_items_id' => $ticket_id,
'accepted' => true,
],
[
'headers' => [
'references' => $ticket_notif->getMessageID(),
],
'expected_itemtype' => Ticket::class,
'expected_items_id' => $ticket_id,
'accepted' => true,
],
// ticket header format - invalid items_id
[
'headers' => [
'in-reply-to' => "GLPI-9999999.{$time2}.{$rand2}@{$uname1}",
],
'expected_itemtype' => null,
'expected_items_id' => null,
'accepted' => true,
],
// other items header format - found item
[
'headers' => [
'in-reply-to' => $soft_notif->getMessageID(),
],
'expected_itemtype' => SoftwareLicense::class,
'expected_items_id' => $soft_id,
'accepted' => true,
],
[
'headers' => [
'references' => $soft_notif->getMessageID(),
],
'expected_itemtype' => SoftwareLicense::class,
'expected_items_id' => $soft_id,
'accepted' => true,
],
[
'headers' => [
'in-reply-to' => 'notavalidvalue',
'references' => $soft_notif->getMessageID(),
],
'expected_itemtype' => SoftwareLicense::class,
'expected_items_id' => $soft_id,
'accepted' => true,
],
[
'headers' => [
'in-reply-to' => $soft_notif->getMessageID(),
'references' => 'donotknow',
],
'expected_itemtype' => SoftwareLicense::class,
'expected_items_id' => $soft_id,
'accepted' => true,
],
// other items header format - invalid itemtype
[
'headers' => [
'references' => "GLPI-UnknownType-{$soft_id}.{$time2}.{$rand2}@{$uname1}",
],
'expected_itemtype' => null,
'expected_items_id' => null,
'accepted' => true,
],
// other items header format - invalid items_id
[
'headers' => [
'references' => "GLPI-SoftwareLicense-9999999.{$time1}.{$rand1}@{$uname2}",
],
'expected_itemtype' => null,
'expected_items_id' => null,
'accepted' => true,
],
];
}
/**
* @dataProvider itemReferenceHeaderProvider
*/
public function testGetItemFromHeader(
array $headers,
?string $expected_itemtype,
?int $expected_items_id,
bool $accepted
) {
$this->newTestedInstance();
$message = new Message(
[
'headers' => $headers,
'content' => 'Message contents...',
]
);
$item = $this->testedInstance->getItemFromHeaders($message);
if ($expected_itemtype === null) {
$this->variable($item)->isNull();
} else {
$this->object($item)->isInstanceOf($expected_itemtype);
$this->integer($item->getId())->isEqualTo($expected_items_id);
}
}
private function doConnect() {
if (null === $this->collector) {
$this->newTestedInstance();
......@@ -252,7 +451,7 @@ class MailCollector extends DbTestCase {
$total_count = count(glob(GLPI_ROOT . '/tests/emails-tests/*.eml'));
$expected_refused_count = 2;
$expected_error_count = 2;
$expected_blacklist_count = 0;
$expected_blacklist_count = 1;
$expected_expected_already_seen = 0;
$this->variable($msg)->isIdenticalTo(
......@@ -311,7 +510,6 @@ class MailCollector extends DbTestCase {
'actor_type' => \CommonITILActor::REQUESTER,
'tickets_names' => [
'PHP fatal error',
'Re: [GLPI #0001155] New ticket database issue',
'Ticket with observer',
'Re: [GLPI #0038927] Update - Issues with new Windows 10 machine',
'A message without to header',
......@@ -431,5 +629,28 @@ class MailCollector extends DbTestCase {
$this->array($filenames)->isIdenticalTo($expected_docs);
$this->integer(count($iterator))->isIdenticalTo(count($expected_docs));
// Check creation of expected followups
$expected_followups = [
[
'items_id' => 100,
'users_id' => $tuid,
'content' => 'This is a reply that references Ticket 100 in In-Reply-To header.&lt;br /&gt;It should be added as followup.',
],
[
'items_id' => 100,
'users_id' => $tuid,
'content' => 'This is a reply that references Ticket 100 in References header.&lt;br /&gt;It should be added as followup.',
],
[
'items_id' => 101,
'users_id' => $tuid,
'content' => 'This is a reply that references Ticket 101 in its subject.&lt;br /&gt;It should be added as followup.',
]
];
foreach ($expected_followups as $expected_followup) {
$this->integer(countElementsInTable(ITILFollowup::getTable(), $expected_followup))->isEqualTo(1);
}
}
}