diff --git a/CHANGELOG.md b/CHANGELOG.md index dd7fa85..26d86e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed +- **Inbound replies come only from the ticket's requester.** With an inbound secret set, only the signed Reply-To address links mail to a ticket. A matched email is a reply only when `From` is the ticket's guest email or requester email, and it is posted as the requester; any other sender gets a new ticket, and only an accepted reply reopens a resolved or closed ticket. + ## [1.5.2] - 2026-09-13 ### Fixed diff --git a/README.md b/README.md index 0151dbd..b3776cd 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,15 @@ Supported adapters: - `postmark` - `ses` +How an inbound email is matched to a ticket: + +- **Thread matching**: when an inbound secret is set (the `escalated_email_inbound_secret` option or the `ESCALATED_EMAIL_INBOUND_SECRET` constant), outbound mail carries a signed Reply-To (`reply+{id}.{hmac8}@domain`) and only that address links an inbound email to a ticket. Without a secret, the `In-Reply-To` / `References` Message-IDs and the subject reference (e.g. `[ESC-00001]`) are used instead. +- **Sender check**: a matched email becomes a reply only when the `From` address is the ticket's requester (the guest email, or the requester's WordPress email, case-insensitive). It is posted as that requester. Staff identity is never taken from the `From` header, so agents reply in the admin, not by email. +- **Anything else** (no match, or a sender who is not the requester) creates a new ticket, so mail is never dropped. +- **Reopen**: an accepted reply reopens a resolved or closed ticket. + +Set a secret in production; Message-IDs and ticket references can be guessed. + ## Scheduled Tasks (WP-Cron) On activation, Escalated schedules: diff --git a/includes/Services/InboundEmailService.php b/includes/Services/InboundEmailService.php index 5b23c41..116df9d 100644 --- a/includes/Services/InboundEmailService.php +++ b/includes/Services/InboundEmailService.php @@ -19,6 +19,11 @@ class InboundEmailService * email, finds the ticket it replies to, and either adds a reply to that * ticket or creates a new one. * + * A thread match alone is not enough to post on a ticket: the sender must + * also be the ticket's requester, and the reply is posted as that + * requester, never as a user named by the unauthenticated From header. + * Anything else becomes a new ticket, so no mail is lost. + * * @param Inbound_Message $message The email, as parsed by an inbound adapter. * @param string $adapter The inbound email adapter name (mailgun, postmark or ses). * @return object|null The ticket that was created or replied to, or null for a @@ -64,14 +69,12 @@ public function process(Inbound_Message $message, string $adapter): ?object try { // Try to find an existing ticket this email belongs to. $ticket = $this->find_ticket_by_email($message); - - // Find the WordPress user by email. - $user = $this->find_user_by_email(sanitize_email($message->fromEmail)); + $author = $ticket ? $this->resolve_reply_author($ticket, $message) : false; $reply = null; - if ($ticket) { - // Add reply to existing ticket. - $reply = $this->add_reply_to_ticket($ticket, $message, $user); + if ($ticket && $author !== false) { + // Add reply to existing ticket as its requester. + $reply = $this->add_reply_to_ticket($ticket, $message, $author); $wpdb->update( $inbound_table, @@ -84,7 +87,8 @@ public function process(Inbound_Message $message, string $adapter): ?object ['id' => $inbound_id] ); } else { - // Create a new ticket. + // Create a new ticket, as the sender if they are a registered user. + $user = $this->find_user_by_email(sanitize_email($message->fromEmail)); $ticket = $this->create_new_ticket($message, $user); $wpdb->update( @@ -131,16 +135,24 @@ public function process(Inbound_Message $message, string $adapter): ?object * Find the ticket an inbound email replies to. * * Follows the inbound resolution chain from the email-threading domain - * model. The first match wins: + * model. Message-IDs and ticket references are guessable, so once an + * inbound secret is configured (and outbound mail therefore carries the + * signed Reply-To) only path 3 is used: + * + * 3. The recipient is a signed reply+{id}.{hmac8}@domain address. + * + * Without a secret, the first match wins: * * 1. In-Reply-To holds a Message-ID we issued (). * 2. References holds one. - * 3. The recipient is a signed reply+{id}.{hmac8}@domain address. * 4. The subject holds a ticket reference such as [ESC-00001]. * 5. In-Reply-To or References matches the Message-ID of an earlier * inbound email, which covers Message-IDs from before the canonical * format. * + * Either way, process() only accepts the email as a reply when it comes + * from the ticket's requester. + * * @param Inbound_Message $message Inbound email. * @return object|null The matching ticket, or null if not found. */ @@ -148,6 +160,12 @@ public function find_ticket_by_email(Inbound_Message $message): ?object { $wpdb = \Escalated\Escalated::db(); + // 3: with a secret, only a signed Reply-To address identifies a ticket. + $secret = Email_Threading::get_inbound_secret(); + if ($secret !== '') { + return $this->find_ticket(Message_Id_Util::verify_reply_to(trim($message->toEmail), $secret)); + } + // In-Reply-To first, then References, for priorities 1, 2 and 5. $thread_message_ids = array_merge( $this->message_ids_in($message->inReplyTo), @@ -162,15 +180,6 @@ public function find_ticket_by_email(Inbound_Message $message): ?object } } - // 3: a signed Reply-To address. - $secret = Email_Threading::get_inbound_secret(); - if ($secret !== '') { - $ticket = $this->find_ticket(Message_Id_Util::verify_reply_to($message->toEmail, $secret)); - if ($ticket) { - return $ticket; - } - } - // 4: a ticket reference in the subject, like [ESC-00001]. if (preg_match('/\[([A-Z]+-\d{5,})\]/', $message->subject, $matches)) { $ticket = Ticket::find_by_reference($matches[1]); @@ -199,6 +208,45 @@ public function find_ticket_by_email(Inbound_Message $message): ?object return null; } + /** + * Decide who a threaded inbound email may post as. + * + * Returns null (a guest reply) when the From address is the ticket's guest + * email, the requester's WP_User when it is the requester's email, and + * false for anyone else. Staff identity is never derived from the From + * header: an agent replying by email is not the requester, so the message + * becomes a new ticket instead. + * + * @param object $ticket The matched ticket. + * @param Inbound_Message $message Inbound email. + * @return \WP_User|null|false + */ + public function resolve_reply_author(object $ticket, Inbound_Message $message) + { + $sender = $this->normalize_email($message->fromEmail); + if ($sender === '') { + return false; + } + + if (! empty($ticket->guest_email) && $this->normalize_email($ticket->guest_email) === $sender) { + return null; + } + + if (! empty($ticket->requester_id)) { + $requester = get_user_by('id', (int) $ticket->requester_id); + if ($requester instanceof \WP_User && $this->normalize_email($requester->user_email) === $sender) { + return $requester; + } + } + + return false; + } + + private function normalize_email(?string $email): string + { + return strtolower(trim((string) $email)); + } + /** * Find a WordPress user by email address. * @@ -223,7 +271,8 @@ public function find_user_by_email(string $email): ?\WP_User * * @param object $ticket The existing ticket object. * @param Inbound_Message $message Inbound email. - * @param \WP_User|null $user The WordPress user who sent the email, or null for guest. + * @param \WP_User|null $user The ticket's requester, or null for a guest reply. Callers + * must pass the requester, never a user found by From. * @return object|null The created reply, or null on failure. */ public function add_reply_to_ticket(object $ticket, Inbound_Message $message, ?\WP_User $user): ?object diff --git a/readme.txt b/readme.txt index f610e3b..c34a321 100644 --- a/readme.txt +++ b/readme.txt @@ -70,7 +70,7 @@ Yes. Generate API tokens from the Escalated admin panel. Authenticate requests u = How do I set up inbound email? = -Configure your email provider (Mailgun, Postmark, or Amazon SES) in the Escalated settings. Point the provider's webhook URL to your site's webhook endpoint. Incoming emails will automatically create new tickets or be appended as replies to existing ones. +Configure your email provider (Mailgun, Postmark, or Amazon SES) in the Escalated settings. Point the provider's webhook URL to your site's webhook endpoint. Incoming emails will automatically create new tickets or be appended as replies to existing ones. A reply is accepted only from the ticket's requester and is posted as that requester; mail from anyone else opens a new ticket. Set an inbound secret so replies are matched by the signed Reply-To address rather than guessable Message-IDs or subject references. = Can I customize the frontend portal styles? = diff --git a/screenshots/seed.php b/screenshots/seed.php index 2a2dbf0..dad4e6f 100644 --- a/screenshots/seed.php +++ b/screenshots/seed.php @@ -1,4 +1,5 @@ prefix . 'escalated_'; +$p = $wpdb->prefix.'escalated_'; $now = current_time('timestamp'); /** Format a timestamp N hours ago as MySQL datetime. */ @@ -58,7 +59,7 @@ 'Onboarding', 'Enterprise', ] as $name) { - $wpdb->insert($p . 'departments', [ + $wpdb->insert($p.'departments', [ 'name' => $name, // slug is NOT NULL with no default; leaving it out only works while // MySQL is not in strict mode. @@ -108,7 +109,7 @@ ['performance', '#1abc9c'], ['security', '#c0392b'], ] as [$name, $colour]) { - $wpdb->insert($p . 'tags', [ + $wpdb->insert($p.'tags', [ 'name' => $name, 'slug' => sanitize_title($name), 'color' => $colour, @@ -133,7 +134,7 @@ ['Omar Haddad', 'omar@logistics.net'], ['Grace Mwangi', 'grace@fintech.africa'], ] as [$name, $email]) { - $wpdb->insert($p . 'contacts', [ + $wpdb->insert($p.'contacts', [ 'email' => $email, 'name' => $name, 'created_at' => $ago(24 * 60), @@ -189,10 +190,10 @@ ? null : $all_agents[$i % count($all_agents)]; - $wpdb->insert($p . 'tickets', [ + $wpdb->insert($p.'tickets', [ 'reference' => sprintf('ESC-%05d', $i + 1), 'subject' => $subject, - 'description' => $subject . '. Reported by ' . $contact['name'] . '.', + 'description' => $subject.'. Reported by '.$contact['name'].'.', 'status' => $status, 'priority' => $priority, 'requester_id' => $admin, @@ -216,7 +217,7 @@ foreach ($row_tags as $t) { if (isset($tags[$t])) { - $wpdb->insert($p . 'ticket_tag', ['ticket_id' => $tid, 'tag_id' => $tags[$t]]); + $wpdb->insert($p.'ticket_tag', ['ticket_id' => $tid, 'tag_id' => $tags[$t]]); } } @@ -233,7 +234,7 @@ } foreach ($thread as [$author, $body, $internal, $type, $at]) { - $wpdb->insert($p . 'replies', [ + $wpdb->insert($p.'replies', [ 'ticket_id' => $tid, 'author_id' => $author, 'body' => $body, diff --git a/tests/Test_Inbound_Email_Routes.php b/tests/Test_Inbound_Email_Routes.php index 06eaf4f..846b832 100644 --- a/tests/Test_Inbound_Email_Routes.php +++ b/tests/Test_Inbound_Email_Routes.php @@ -7,17 +7,19 @@ * public REST route, has to create a ticket, or add a reply when the email * belongs to an existing ticket. * - * Replies are matched to tickets by the resolution chain in - * escalated-developer-context/domain-model/email-threading.md. The first - * match wins: + * Replies are matched to tickets as described in + * escalated-developer-context/domain-model/email-threading.md. With an + * inbound secret set, only the signed reply+{id}.{hmac8}@domain address + * identifies a ticket. Without one, the first match wins: * * 1. In-Reply-To holds a Message-ID we issued (). * 2. References holds one. - * 3. The recipient is a signed reply+{id}.{hmac8}@domain address. * 4. The subject holds a [PREFIX-00001] ticket reference. * 5. In-Reply-To or References matches an earlier inbound email. * - * Anything else opens a new ticket. + * A matched email is a reply only when it comes from the ticket's + * requester, and it is posted as that requester. Anything else opens a + * new ticket. */ use Escalated\Escalated; @@ -200,6 +202,7 @@ public function test_postmark_attachment_is_stored_under_its_file_name(): void public function test_reply_is_threaded_by_in_reply_to(): void { + $this->without_reply_secret(); $ticket = $this->existing_ticket(); $response = $this->mailgun([ @@ -212,6 +215,7 @@ public function test_reply_is_threaded_by_in_reply_to(): void public function test_reply_is_threaded_by_references(): void { + $this->without_reply_secret(); $ticket = $this->existing_ticket(); $response = $this->mailgun([ @@ -250,6 +254,7 @@ public function test_reply_to_address_with_a_bad_signature_opens_a_new_ticket(): public function test_reply_is_threaded_by_subject_reference(): void { + $this->without_reply_secret(); $ticket = $this->existing_ticket(); $response = $this->mailgun([ @@ -261,6 +266,7 @@ public function test_reply_is_threaded_by_subject_reference(): void public function test_reply_is_threaded_by_an_earlier_inbound_message_id(): void { + $this->without_reply_secret(); $first = $this->mailgun(['Message-Id' => '']); $this->assertSame(200, $first->get_status(), wp_json_encode($first->get_data())); $ticket = $this->latest_ticket(); @@ -276,6 +282,7 @@ public function test_reply_is_threaded_by_an_earlier_inbound_message_id(): void public function test_in_reply_to_outranks_a_subject_reference(): void { + $this->without_reply_secret(); $replied_to = $this->existing_ticket(); $mentioned = $this->existing_ticket(); @@ -288,6 +295,108 @@ public function test_in_reply_to_outranks_a_subject_reference(): void $this->assertSame(0, $this->reply_count($mentioned)); } + // ========================================================================= + // Only the requester may reply + // ========================================================================= + + public function test_stranger_quoting_a_subject_reference_opens_their_own_ticket(): void + { + $this->without_reply_secret(); + $ticket = $this->existing_ticket(); + $before = $this->ticket_count(); + + $response = $this->mailgun([ + 'from' => 'Stranger ', + 'subject' => 'Re: ['.$ticket->reference.'] Printer is on fire', + ]); + + $this->assertSame(200, $response->get_status(), wp_json_encode($response->get_data())); + $this->assertSame($before + 1, $this->ticket_count()); + $this->assertSame(0, $this->reply_count($ticket)); + $this->assertSame('stranger@elsewhere.example', $this->latest_ticket()->guest_email); + } + + public function test_stranger_threading_onto_a_closed_ticket_does_not_reopen_it(): void + { + $this->without_reply_secret(); + $ticket = $this->existing_ticket(); + $this->set_status($ticket, 'closed'); + + $response = $this->mailgun([ + 'from' => 'Stranger ', + 'subject' => 'Re: ['.$ticket->reference.'] Printer is on fire', + 'In-Reply-To' => Message_Id_Util::build_message_id((int) $ticket->id, null, self::DOMAIN), + ]); + + $this->assertSame(200, $response->get_status(), wp_json_encode($response->get_data())); + $this->assertSame(0, $this->reply_count($ticket)); + $this->assertSame('closed', Ticket::find((int) $ticket->id)->status); + $this->assertNotSame((int) $ticket->id, (int) $response->get_data()['ticket_id']); + } + + public function test_email_naming_an_agent_is_not_posted_as_that_agent(): void + { + $agent_id = $this->factory->user->create(['user_email' => 'agent@support.example.org', 'role' => 'administrator']); + $ticket = $this->existing_ticket(); + $before = $this->ticket_count(); + + $response = $this->mailgun([ + 'from' => 'Agent ', + 'recipient' => Message_Id_Util::build_reply_to((int) $ticket->id, self::REPLY_SECRET, self::DOMAIN), + 'subject' => 'Re: ['.$ticket->reference.'] Printer is on fire', + ]); + + $this->assertSame(200, $response->get_status(), wp_json_encode($response->get_data())); + $this->assertSame($before + 1, $this->ticket_count()); + $this->assertSame(0, $this->reply_count($ticket)); + $this->assertSame(0, (int) Escalated::db()->get_var(Escalated::db()->prepare( + 'SELECT COUNT(*) FROM '.Reply::table().' WHERE author_id = %d', + $agent_id + ))); + } + + public function test_with_a_reply_secret_only_the_signed_address_threads(): void + { + $ticket = $this->existing_ticket(); + $before = $this->ticket_count(); + + $response = $this->mailgun([ + 'subject' => 'Re: ['.$ticket->reference.'] Printer is on fire', + 'In-Reply-To' => Message_Id_Util::build_message_id((int) $ticket->id, null, self::DOMAIN), + ]); + + $this->assertSame(200, $response->get_status(), wp_json_encode($response->get_data())); + $this->assertSame($before + 1, $this->ticket_count()); + $this->assertSame(0, $this->reply_count($ticket)); + } + + public function test_requester_reply_is_posted_as_the_requester_and_reopens_the_ticket(): void + { + $requester_id = $this->factory->user->create(['user_email' => 'member@customer.example']); + $ticket = (new TicketService)->create($requester_id, [ + 'subject' => 'Printer is on fire', + 'description' => 'Smoke everywhere.', + 'channel' => 'web', + ]); + $this->set_status($ticket, 'resolved'); + + $response = $this->mailgun([ + 'from' => 'Member ', + 'recipient' => Message_Id_Util::build_reply_to((int) $ticket->id, self::REPLY_SECRET, self::DOMAIN), + 'subject' => 'Re: Printer is on fire', + ]); + + $this->assertSame(200, $response->get_status(), wp_json_encode($response->get_data())); + $this->assertSame((int) $ticket->id, (int) $response->get_data()['ticket_id']); + $this->assertSame(1, $this->reply_count($ticket)); + $reply = Escalated::db()->get_row(Escalated::db()->prepare( + 'SELECT * FROM '.Reply::table().' WHERE ticket_id = %d', + $ticket->id + )); + $this->assertSame($requester_id, (int) $reply->author_id); + $this->assertSame('reopened', Ticket::find((int) $ticket->id)->status); + } + public function test_redelivered_email_opens_only_one_ticket(): void { $before = $this->ticket_count(); @@ -459,17 +568,33 @@ private function sns_post(array $payload): WP_REST_Response // Helpers // ========================================================================= + /** + * A guest ticket opened by the sender, so the sender's replies are accepted. + */ private function existing_ticket(): object { - $requester_id = $this->factory->user->create(); - - return (new TicketService)->create($requester_id, [ + return (new TicketService)->create_guest([ 'subject' => 'Printer is on fire', 'description' => 'Smoke everywhere.', 'channel' => 'web', + 'guest_name' => 'Jane Customer', + 'guest_email' => self::SENDER, ]); } + /** + * Without a secret, the unsigned Message-ID and subject paths are used. + */ + private function without_reply_secret(): void + { + delete_option('escalated_email_inbound_secret'); + } + + private function set_status(object $ticket, string $status): void + { + Escalated::db()->update(Ticket::table(), ['status' => $status], ['id' => (int) $ticket->id]); + } + private function ticket_count(): int { return (int) Escalated::db()->get_var('SELECT COUNT(*) FROM '.Ticket::table());