Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

In this Discussion

osTicket v1.10 (stable) and Maintenance Release v1.9.15 are now available! Go get it now

Emailed Staff reply to ticket does not account for subject line fail safe

Take the following scenario:
I am a staff member - I want to reply to a ticket and know the number but I do not have a ticket thread handy.  So I just throw the ticket number into a new subject line and type my note.

Currently, in v1.9.7 - the fall back to subject line method does not account for the fact that it might be coming from a staff member - and so it will create a new ticket. It only checks if the sender is a user or a collaborator (which a staff member could be neither).

Posting original code for subject line below for reference - it seems to me this is a bug because when the message ID is include, it does check if the sender is a staff member.  

        // Search for ticket by the [#123456] in the subject line
        // This is the last resort -  emails must match to avoid message
        // injection by third-party.
        $subject = $mailinfo['subject'];
        $match = array();
        if ($subject
                && $mailinfo['email']
                // Required `#` followed by one or more of
                //      punctuation (-) then letters, numbers, and symbols
                // (Try not to match closing punctuation (`]`) in [#12345])
                && preg_match("/#((\p{P}*[^\p{C}\p{Z}\p{P}]+)+)/u", $subject, $match)
                //Lookup by ticket number
                && ($ticket = Ticket::lookupByNumber($match[1]))
                //Lookup the user using the email address
                && ($user = User::lookup(array('emails__address' => $mailinfo['email'])))) {
            //We have a valid ticket and user
            if ($ticket->getUserId() == $user->getId() //owner
                    ||  ($c = Collaborator::lookup( // check if collaborator
                            array('userId' => $user->getId(),
                                  'ticketId' => $ticket->getId())))) {

                $mailinfo['userId'] = $user->getId();
                return $ticket->getLastMessage();
            }
        }

        // Search for the message-id token in the body
        if (preg_match('`(?:data-mid="|Ref-Mid: )([^"\s]*)(?:$|")`',
                $mailinfo['message'], $match))
            if ($thread = ThreadEntry::lookupByRefMessageId($match[1],
                    $mailinfo['email']))
                return $thread;

        return null;
    }

Any ideas?

Comments

  • edited May 2015
    Fixed - but wont let me paste whole block - so here it is by section in class.thread.php
    Line 986: 
    && ($user = User::lookup(array('emails__address' => $mailinfo['email'])))) {
    To       
    && ($user = User::lookup(array('emails__address' => $mailinfo['email'])))
     && ($staff = Staff::getIdByEmail($mailinfo['email']))) {

    Line 989:
    $ticket->getUserId() == $user->getId() 
    to 
    !empty($staff) || $ticket->getUserId() == $user->getId()

    Line 993 (I didn't do a before, so will be around this line): 
    $mailinfo['userId'] = $user->getId(); 
    to
    if (empty($staff))
    $mailinfo['userId'] = $user->getId();
    else
    $mailinfo['staffId'] = $staff;

    Hope this helps somebody
  • Thanks for posting!
  • NP!  I found there is a bug in what I posted though...  A diff of the changes that need to be made can be found in the pull request thats currently out there - as far as I know, that version is bug free :)

Sign In or Register to comment.