From ecd2e6a9c78ad67ce5b522e87b2705a95dac8a4f Mon Sep 17 00:00:00 2001 From: Jared Hancock <jared@osticket.com> Date: Sun, 14 Dec 2014 22:03:48 -0600 Subject: [PATCH] email: Message-Id header with user and thread ID This patch suggests a change to the message-id creation process that includes stamping the receiving user-id (staff or client) along with the thread-id of the originating notice. This allows detection of threading if the clinically brain-dead mail client drops all the other header detection mechanisms, including the tag placed in the email body, on response. This patch works for both client and agent communication. --- include/class.client.php | 11 ++- include/class.mailer.php | 150 +++++++++++++++++++++++++++++++++++++-- include/class.staff.php | 3 +- include/class.thread.php | 23 +++++- include/class.ticket.php | 14 ++-- 5 files changed, 184 insertions(+), 17 deletions(-) diff --git a/include/class.client.php b/include/class.client.php index bfcffeaca..d65b0eea7 100644 --- a/include/class.client.php +++ b/include/class.client.php @@ -15,7 +15,8 @@ **********************************************************************/ require_once INCLUDE_DIR.'class.user.php'; -abstract class TicketUser { +abstract class TicketUser +implements EmailContact { static private $token_regex = '/^(?P<type>\w{1})(?P<algo>\d+)x(?P<hash>.*)$/i'; @@ -50,6 +51,9 @@ abstract class TicketUser { } + function getId() { return $this->user->getId(); } + function getEmail() { return $this->user->getEmail(); } + function sendAccessLink() { global $ost; @@ -417,4 +421,9 @@ class ClientAccount extends UserAccount { } } +// Used by the email system +interface EmailContact { + function getId(); + function getEmail(); +} ?> diff --git a/include/class.mailer.php b/include/class.mailer.php index a3093c203..3db5e0a93 100644 --- a/include/class.mailer.php +++ b/include/class.mailer.php @@ -90,6 +90,142 @@ class Mailer { $this->addAttachment($a); } + /** + * getMessageId + * + * Generates a unique message ID for an outbound message. Optionally, + * the recipient can be used to create a tag for the message ID where + * the user-id and thread-entry-id are encoded in the message-id so + * the message can be threaded if it is replied to without any other + * indicator of the thread to which it belongs. This tag is signed with + * the secret-salt of the installation to guard against false positives. + * + * Parameters: + * $recipient - (EmailContact|null) recipient of the message. The ID of + * the recipient is placed in the message id TAG section so it can + * be recovered if the email replied to directly by the end user. + * $options - (array) - options passed to ::send(). If it includes a + * 'thread' element, the threadId will be recorded in the TAG + * + * Returns: + * (string) - email message id, with leading and trailing <> chars. See + * the format below for the structure. + * + * Format: + * VA-B-C-D, with dash separators and A-D explained below: + * + * V: Version code of the generated Message-Id + * A: Predictable random code — used for loop detection + * B: Random data for unique identifier + * Version Code: A (at char position 10) + * C: TAG: Base64(Pack(userid, ticketid, type)), = chars discarded + * D: Signature: + * '@' + Signed Tag value, last 10 chars from + * HMAC(sha1, tag+rand, SECRET_SALT) + * -or- Original From email address + */ + function getMessageId($recipient, $options=array(), $version='A') { + $tag = ''; + $rand = str_replace('-','_', Misc::randCode(9)); + $sig = $this->getEmail()?$this->getEmail()->getEmail():'@osTicketMailer'; + if ($recipient instanceof EmailContact) { + // Create a tag for the outbound email + $tag = pack('VVa', + $recipient->getId(), + (isset($options['thread']) && $options['thread'] instanceof ThreadEntry) + ? $options['thread']->getId() : 0, + ($recipient instanceof Staff ? 'S' + : ($recipient instanceof TicketOwner ? 'U' + : ($recipient instanceof Collaborator ? 'C' + : '?'))) + ); + $tag = str_replace('=','',base64_encode($tag)); + // Sign the tag with the system secret salt + $sig = '@' . substr(hash_hmac('sha1', $tag.$rand, SECRET_SALT), -10); + } + return sprintf('<A%s-%s-%s-%s>', + static::getSystemMessageIdCode(), + $rand, $tag, $sig); + } + + /** + * decodeMessageId + * + * Decodes a message-id generated by osTicket using the ::getMessageId() + * method of this class. This will digest the received message-id token + * and return an array with some information about it. + * + * Parameters: + * $mid - (string) message-id from an email Message-Id, In-Reply-To, and + * References header. + * + * Returns: + * (array) of information containing all or some of the following keys + * 'loopback' - (bool) true or false if the message originated by + * this osTicket installation. + * 'version' - (string|FALSE) version code of the message id + * 'code' - (string) unique but predictable help desk message-id + * 'id' - (string) random characters serving as the unique id + * 'threadId' - (int) thread-id from which the message originated + * 'staffId' - (int|null) staff the email was originally sent to + * 'userId' - (int|null) user the email was originally sent to + * 'userClass' - (string) class of user the email was sent to + * 'U' - TicketOwner + * 'S' - Staff + * 'C' - Collborator + * '?' - Something else + */ + static function decodeMessageId($mid) { + // Drop <> tokens + $mid = trim($mid, '<> '); + // Drop email domain on rhs + list($lhs, $sig) = explode('@', $mid, 2); + // LHS should be tokenized by '-' + $parts = explode('-', $lhs); + + $rv = array('loopback' => false, 'version' => false); + + // There should be at least two tokens if the message was sent by + // this system. Otherwise, there's nothing to be detected + if (count($parts) < 2) + return $rv; + + // Detect the MessageId version, which should be the tenth char of + // the second segment + $rv['version'] = @$parts[0][0]; + switch ($rv['version']) { + case 'A': + default: + list($rv['code'], $rv['id'], $tag) = $parts; + // Drop the leading version code + $rv['code'] = substr($rv['code'], 1); + // Verify tag signature + $chksig = substr(hash_hmac('sha1', $tag.$rv['id'], SECRET_SALT), -10); + if ($tag && $sig == $chksig && ($tag = base64_decode($tag))) { + // Find user and ticket id + $rv += unpack('Vuid/VthreadId/auserClass', $tag); + // Attempt to make the user-id more specific + $classes = array( + 'S' => 'staffId', 'U' => 'userId' + ); + if (isset($classes[$rv['userClass']])) + $rv[$classes[$rv['userClass']]] = $rv['uid']; + } + // Round-trip detection - the first section is the local + // system's message-id code + $rv['loopback'] = (0 === strcasecmp($rv['code'], + static::getSystemMessageIdCode())); + break; + } + return $rv; + } + + static function getSystemMessageIdCode() { + return substr(str_replace('+', '=', + base64_encode(md5('mail'.SECRET_SALT, true))), + 0, 6); + } + function send($to, $subject, $message, $options=null) { global $ost, $cfg; @@ -97,16 +233,18 @@ class Mailer { require_once (PEAR_DIR.'Mail.php'); // PEAR Mail package require_once (PEAR_DIR.'Mail/mime.php'); // PEAR Mail_Mime packge + $messageId = $this->getMessageId($to, $options); + + if ($to instanceof EmailContact + || (is_object($to) && is_callable(array($to, 'getEmail'))) + ) { + $to = $to->getEmail(); + } + //do some cleanup $to = preg_replace("/(\r\n|\r|\n)/s",'', trim($to)); $subject = preg_replace("/(\r\n|\r|\n)/s",'', trim($subject)); - /* Message ID - generated for each outgoing email */ - $messageId = sprintf('<%s-%s-%s>', - substr(md5('mail'.SECRET_SALT), -9), - Misc::randCode(9), - ($this->getEmail()?$this->getEmail()->getEmail():'@osTicketMailer')); - $headers = array ( 'From' => $this->getFromAddress(), 'To' => $to, diff --git a/include/class.staff.php b/include/class.staff.php index da2473c0e..4779da709 100644 --- a/include/class.staff.php +++ b/include/class.staff.php @@ -22,7 +22,8 @@ include_once(INCLUDE_DIR.'class.passwd.php'); include_once(INCLUDE_DIR.'class.user.php'); include_once(INCLUDE_DIR.'class.auth.php'); -class Staff extends AuthenticatedUser { +class Staff extends AuthenticatedUser +implements EmailContact { var $ht; var $id; diff --git a/include/class.thread.php b/include/class.thread.php index aa1404d26..eb7af30b4 100644 --- a/include/class.thread.php +++ b/include/class.thread.php @@ -665,8 +665,8 @@ Class ThreadEntry { // where code is a predictable string based on the SECRET_SALT of // this osTicket installation. If this incoming mail matches the // code, then it very likely originated from this system and looped - @list($code) = explode('-', $mailinfo['mid'], 2); - if (0 === strcasecmp(ltrim($code, '<'), substr(md5('mail'.SECRET_SALT), -9))) { + $msgId_info = Mailer::decodeMessageId($mailinfo['mid']); + if ($msgId_info['loopback']) { // This mail was sent by this system. It was received due to // some kind of mail delivery loop. It should not be considered // a response to an existing thread entry @@ -896,6 +896,25 @@ Class ThreadEntry { return $t; } } + // Attempt to detect the ticket and user ids from the + // message-id header. If the message originated from + // osTicket, the Mailer class can break it apart. If it came + // from this help desk, the 'loopback' property will be set + // to true. + $mid_info = Mailer::decodeMessageId($mid); + if ($mid_info['loopback'] && isset($mid_info['uid']) + && @$mid_info['threadId'] + && ($t = ThreadEntry::lookup($mid_info['threadId'])) + ) { + if (@$mid_info['userId']) { + $mailinfo['userId'] = $mid_info['userId']; + } + elseif (@$mid_info['staffId']) { + $mailinfo['staffId'] = $mid_info['staffId']; + } + // ThreadEntry was positively identified + return $t; + } } // Second best case — found a thread but couldn't identify the // user from the header. Return the first thread entry matched diff --git a/include/class.ticket.php b/include/class.ticket.php index 3215f10ff..97f6c25b4 100644 --- a/include/class.ticket.php +++ b/include/class.ticket.php @@ -1032,7 +1032,7 @@ class Ticket { foreach( $recipients as $k=>$staff) { if(!is_object($staff) || !$staff->isAvailable() || in_array($staff->getEmail(), $sentlist)) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null, $options); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null, $options); $sentlist[] = $staff->getEmail(); } } @@ -1253,7 +1253,7 @@ class Ticket { foreach( $recipients as $k=>$staff) { if(!is_object($staff) || !$staff->isAvailable() || in_array($staff->getEmail(), $sentlist)) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null, $options); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null, $options); $sentlist[] = $staff->getEmail(); } } @@ -1302,7 +1302,7 @@ class Ticket { foreach( $recipients as $k=>$staff) { if(!is_object($staff) || !$staff->isAvailable() || in_array($staff->getEmail(), $sentlist)) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null); $sentlist[] = $staff->getEmail(); } @@ -1505,7 +1505,7 @@ class Ticket { foreach( $recipients as $k=>$staff) { if(!is_object($staff) || !$staff->isAvailable() || in_array($staff->getEmail(), $sentlist)) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null, $options); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null, $options); $sentlist[] = $staff->getEmail(); } } @@ -1736,7 +1736,7 @@ class Ticket { foreach( $recipients as $k=>$staff) { if(!$staff || !$staff->getEmail() || !$staff->isAvailable() || in_array($staff->getEmail(), $sentlist)) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null, $options); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null, $options); $sentlist[] = $staff->getEmail(); } } @@ -1859,7 +1859,7 @@ class Ticket { $variables + array('recipient' => $this->getOwner())); $attachments = $cfg->emailAttachments()?$response->getAttachments():array(); - $email->send($this->getEmail(), $msg['subj'], $msg['body'], $attachments, + $email->send($this->getOwner(), $msg['subj'], $msg['body'], $attachments, $options); } @@ -2006,7 +2006,7 @@ class Ticket { ) continue; $alert = $this->replaceVars($msg, array('recipient' => $staff)); - $email->sendAlert($staff->getEmail(), $alert['subj'], $alert['body'], null, $options); + $email->sendAlert($staff, $alert['subj'], $alert['body'], null, $options); $sentlist[$staff->getEmail()] = 1; } } -- GitLab