diff --git a/fusion_helpdesk/controllers/main.py b/fusion_helpdesk/controllers/main.py
index 3fb22ab5..58612b59 100644
--- a/fusion_helpdesk/controllers/main.py
+++ b/fusion_helpdesk/controllers/main.py
@@ -22,12 +22,14 @@ from urllib.parse import urljoin, urlparse
from odoo import _, http
from odoo.exceptions import UserError
from odoo.http import request
+from odoo.tools import email_normalize
from odoo.addons.fusion_helpdesk.utils import (
build_ticket_vals,
build_scope_domain,
is_public_message,
compute_unread_count,
+ escape_like,
)
_logger = logging.getLogger(__name__)
@@ -86,7 +88,10 @@ class FusionHelpdeskController(http.Controller):
# the scoped "My Tickets" inbox. reply_email is the (editable) value the
# user confirmed in the dialog; fall back to their Odoo email/login.
user = request.env.user
- reporter_email = (reply_email or user.email or user.login or '').strip()
+ # Normalise the confirmed email (and fall back to the user's own).
+ # Normalising rejects garbage / wildcard-bearing values so the stored
+ # partner_email — which is also the inbox scope key — stays clean.
+ reporter_email = _norm_email(reply_email, user.email, user.login)
ticket_vals = build_ticket_vals(
kind=kind, subject=subject, body_html='\n'.join(body_parts),
team_id=cfg['team_id'], client_label=cfg['client_label'],
@@ -143,7 +148,12 @@ class FusionHelpdeskController(http.Controller):
return _network_error_response(cfg['url'], e)
# ---- Push attachments -----------------------------------------
+ # The ticket already exists; an attachment failure must NOT bubble up
+ # as a 500 (the user would think the whole submission failed and file
+ # a duplicate). Catch network errors too, count failures, and report
+ # them back so the dialog can tell the user which files didn't make it.
attached = 0
+ failed = 0
for att in attachments or []:
data_b64 = (att or {}).get('data_b64')
name = (att or {}).get('name') or 'attachment.bin'
@@ -162,10 +172,12 @@ class FusionHelpdeskController(http.Controller):
}],
)
attached += 1
- except xmlrpc.client.Fault as e:
+ except (xmlrpc.client.Fault, xmlrpc.client.ProtocolError,
+ socket.timeout, OSError, ssl.SSLError) as e:
+ failed += 1
_logger.warning(
'fusion_helpdesk: attachment "%s" upload failed: %s',
- name, e.faultString,
+ name, e,
)
ticket_url = urljoin(
@@ -173,9 +185,9 @@ class FusionHelpdeskController(http.Controller):
'odoo/helpdesk/%s' % ticket_id,
)
_logger.info(
- 'fusion_helpdesk: created remote ticket #%s (%s attachments) '
+ 'fusion_helpdesk: created remote ticket #%s (%s attached, %s failed) '
'on %s for user %s',
- ticket_id, attached, cfg['url'],
+ ticket_id, attached, failed, cfg['url'],
request.env.user.login,
)
return {
@@ -183,6 +195,7 @@ class FusionHelpdeskController(http.Controller):
'ticket_id': ticket_id,
'ticket_url': ticket_url,
'attached': attached,
+ 'failed': failed,
}
# ------------------------------------------------------------------
@@ -374,7 +387,8 @@ class FusionHelpdeskController(http.Controller):
cfg = self._read_config()
return {
'cfg': cfg,
- 'email': (user.email or user.login or '').strip(),
+ # Normalised so a self-set wildcard email ('%') can't widen scope.
+ 'email': _norm_email(user.email, user.login),
'label': cfg['client_label'],
'is_admin': user.has_group('fusion_helpdesk.group_reporter_admin'),
'name': user.name,
@@ -384,11 +398,25 @@ class FusionHelpdeskController(http.Controller):
return all([cfg['url'], cfg['db'], cfg['login'], cfg['password']])
def _rpc(self, cfg, model, method, args, kw=None):
- """Authenticate + execute_kw against the central Odoo as the bot."""
+ """Authenticate + execute_kw against the central Odoo as the bot.
+
+ A ProtocolError on the execute_kw leg (e.g. a 502/503/429 from the
+ central reverse proxy) is NOT an OSError subclass, so we convert it to
+ a _RemoteError here — otherwise it would escape every endpoint's
+ except-tuple and surface as a raw 500 (mislabelled "Network error")."""
uid, proxy = self._authenticate(cfg)
- return proxy.execute_kw(
- cfg['db'], uid, cfg['password'], model, method, args, kw or {},
- )
+ try:
+ return proxy.execute_kw(
+ cfg['db'], uid, cfg['password'], model, method, args, kw or {},
+ )
+ except xmlrpc.client.ProtocolError as e:
+ _logger.warning('fusion_helpdesk: HTTP %s on %s.%s: %s',
+ e.errcode, model, method, e.errmsg)
+ raise _RemoteError(
+ 'remote_http_error',
+ _('The central Helpdesk returned HTTP %(code)s. Please try '
+ 'again in a moment.') % {'code': e.errcode},
+ )
def _internal_subtype_map(self, cfg, subtype_ids):
"""{subtype_id: internal_bool} so internal notes can be hidden."""
@@ -465,22 +493,45 @@ class FusionHelpdeskController(http.Controller):
return out
def _resolve_author(self, cfg, ident, ticket):
- """Find-or-create the replier's partner on central so their reply is
- correctly attributed. Falls back to the ticket's customer if partner
- creation isn't permitted for the bot."""
+ """Find-or-create the replier's OWN partner on central so their reply
+ is correctly attributed.
+
+ `ident['email']` is already normalised (no wildcards); we escape it for
+ the =ilike search as belt-and-suspenders. On any failure we log and
+ return False — message_post then attributes the reply to the service
+ account, which is honest. We deliberately do NOT fall back to the
+ ticket's customer: for an admin replying to a colleague's ticket that
+ would silently impersonate the customer."""
email = ident['email']
- ticket_partner = ticket['partner_id'][0] if ticket.get('partner_id') else False
if not email:
- return ticket_partner
+ return False
try:
pids = self._rpc(cfg, 'res.partner', 'search',
- [[('email', '=ilike', email)]], {'limit': 1})
+ [[('email', '=ilike', escape_like(email))]], {'limit': 1})
if pids:
return pids[0]
return self._rpc(cfg, 'res.partner', 'create',
[{'name': ident['name'], 'email': email}])
- except (xmlrpc.client.Fault, _RemoteError):
- return ticket_partner
+ except (xmlrpc.client.Fault, _RemoteError) as e:
+ _logger.warning(
+ 'fusion_helpdesk: could not resolve reply author for %s on '
+ 'ticket %s (%s); posting as the service account.',
+ email, ticket.get('id'), e)
+ return False
+
+ def _mark_ticket_seen(self, ticket_id, messages):
+ """Best-effort read-tracking. Runs AFTER the remote read/post, so it
+ must never raise — otherwise a local DB hiccup here would turn an
+ already-successful reply into a reported failure, and the user would
+ resubmit (posting a duplicate). Bookkeeping only; log and swallow."""
+ if not messages:
+ return
+ try:
+ request.env['fusion.helpdesk.ticket.seen']._mark_seen(
+ ticket_id, max(m['id'] for m in messages))
+ except Exception: # noqa: BLE001 — non-critical bookkeeping
+ _logger.exception(
+ 'fusion_helpdesk: mark-seen failed for ticket %s', ticket_id)
def _remote_failure(self, cfg, err):
"""Map a mid-RPC failure to the dialog's response shape."""
@@ -559,9 +610,7 @@ class FusionHelpdeskController(http.Controller):
messages = self._public_messages(cfg, ticket_id)
except (_RemoteError, xmlrpc.client.Fault, OSError, ssl.SSLError) as e:
return self._remote_failure(cfg, e)
- if messages:
- request.env['fusion.helpdesk.ticket.seen']._mark_seen(
- ticket_id, max(m['id'] for m in messages))
+ self._mark_ticket_seen(ticket_id, messages)
t = found[0]
# Magic link: the customer's own access-token URL on central, so they
# can open the full ticket (incl. attachments) in the portal if needed.
@@ -611,9 +660,7 @@ class FusionHelpdeskController(http.Controller):
messages = self._public_messages(cfg, ticket_id)
except (_RemoteError, xmlrpc.client.Fault, OSError, ssl.SSLError) as e:
return self._remote_failure(cfg, e)
- if messages:
- request.env['fusion.helpdesk.ticket.seen']._mark_seen(
- ticket_id, max(m['id'] for m in messages))
+ self._mark_ticket_seen(ticket_id, messages)
return {'ok': True, 'messages': messages}
@http.route('/fusion_helpdesk/unread_count',
diff --git a/fusion_helpdesk/static/src/js/fusion_helpdesk_dialog.js b/fusion_helpdesk/static/src/js/fusion_helpdesk_dialog.js
index b4a89ad2..fecd5cc1 100644
--- a/fusion_helpdesk/static/src/js/fusion_helpdesk_dialog.js
+++ b/fusion_helpdesk/static/src/js/fusion_helpdesk_dialog.js
@@ -46,6 +46,7 @@ export class FusionHelpdeskDialog extends Component {
ticketId: null,
ticketUrl: "",
attached: 0,
+ failed: 0,
// ---- My Tickets ----
isAdmin: false,
scope: "mine", // 'mine' | 'all'
@@ -110,6 +111,7 @@ export class FusionHelpdeskDialog extends Component {
this.state.isAdmin = !!res.is_admin;
}
} catch (err) {
+ console.error("fusion_helpdesk: my_tickets failed", err);
this.state.listError = (err && err.message) || _t("Network error.");
} finally {
this.state.loadingList = false;
@@ -143,6 +145,7 @@ export class FusionHelpdeskDialog extends Component {
row.has_unread = false;
}
} catch (err) {
+ console.error("fusion_helpdesk: open ticket failed", err);
this.state.threadError = (err && err.message) || _t("Network error.");
} finally {
this.state.loadingThread = false;
@@ -173,6 +176,7 @@ export class FusionHelpdeskDialog extends Component {
this.notification.add(_t("Reply sent."), { type: "success" });
}
} catch (err) {
+ console.error("fusion_helpdesk: send reply failed", err);
this.state.threadError = (err && err.message) || _t("Network error.");
} finally {
this.state.sendingReply = false;
@@ -358,12 +362,14 @@ export class FusionHelpdeskDialog extends Component {
this.state.ticketId = res.ticket_id;
this.state.ticketUrl = res.ticket_url;
this.state.attached = res.attached || 0;
+ this.state.failed = res.failed || 0;
this.state.subject = "";
this.state.description = "";
this.state.errorCode = "";
this.state.attachments = [];
}
} catch (err) {
+ console.error("fusion_helpdesk: submit failed", err);
this.state.error = (err && err.message) || _t("Network error.");
} finally {
this.state.submitting = false;
diff --git a/fusion_helpdesk/static/src/xml/fusion_helpdesk_dialog.xml b/fusion_helpdesk/static/src/xml/fusion_helpdesk_dialog.xml
index 6aa83afb..5a45fec2 100644
--- a/fusion_helpdesk/static/src/xml/fusion_helpdesk_dialog.xml
+++ b/fusion_helpdesk/static/src/xml/fusion_helpdesk_dialog.xml
@@ -110,6 +110,11 @@
created with attachment(s).
You'll get replies by email, and can follow up under My Tickets.
+
+
+ attachment(s) could not be uploaded.
+ Open the ticket from My Tickets and add them there.
+
diff --git a/fusion_helpdesk/tests/test_utils.py b/fusion_helpdesk/tests/test_utils.py
index aa47adcd..30fa6d3b 100644
--- a/fusion_helpdesk/tests/test_utils.py
+++ b/fusion_helpdesk/tests/test_utils.py
@@ -68,6 +68,20 @@ class TestScopeDomain(TransactionCase):
self.assertEqual(len(label_terms), 1)
self.assertNotEqual(label_terms[0][2], '')
+ def test_wildcard_email_cannot_widen_scope(self):
+ # IDOR guard: a self-set email of '%' must NOT become a match-all
+ # =ilike term — the wildcard has to be escaped to a literal.
+ dom = build_scope_domain(label='ENTECH', email='%', is_admin=False)
+ email_terms = [t for t in dom if t[0] == 'partner_email']
+ self.assertEqual(len(email_terms), 1)
+ self.assertEqual(email_terms[0][2], '\\%',
+ "'%' must be escaped so ILIKE matches it literally")
+
+ def test_underscore_in_real_email_is_escaped_but_preserved(self):
+ dom = build_scope_domain(label='ENTECH', email='john_doe@x.com', is_admin=False)
+ email_terms = [t for t in dom if t[0] == 'partner_email']
+ self.assertEqual(email_terms[0][2], 'john\\_doe@x.com')
+
@tagged('post_install', '-at_install', 'fusion_helpdesk')
class TestMessageFilterAndUnread(TransactionCase):
diff --git a/fusion_helpdesk/utils.py b/fusion_helpdesk/utils.py
index 56a06f9a..28b81310 100644
--- a/fusion_helpdesk/utils.py
+++ b/fusion_helpdesk/utils.py
@@ -15,6 +15,20 @@ filter and the unread maths without a live central Odoo to talk to.
_NO_MATCH = '__none__'
+def escape_like(value):
+ """Escape SQL LIKE/ILIKE wildcards so a user-supplied value can never
+ widen an `=ilike` match to other rows.
+
+ `res.users.email` is self-writeable and unvalidated, so without this a
+ user could set their email to ``%`` and have ``partner_email =ilike '%'``
+ match EVERY ticket in their deployment (a cross-user IDOR). Escaping the
+ backslash first, then ``%`` and ``_``, makes those characters match
+ literally. Real emails containing ``_`` (e.g. ``john_doe@x.com``) keep
+ working — the underscore is matched as a literal, which is what we want.
+ """
+ return (value or '').replace('\\', '\\\\').replace('%', '\\%').replace('_', '\\_')
+
+
def build_ticket_vals(kind, subject, body_html, team_id, client_label,
reporter_name, reporter_email, company_name):
"""Construct the `helpdesk.ticket` create vals for a forwarded report.
@@ -53,7 +67,8 @@ def build_scope_domain(label, email, is_admin):
"""
domain = [('x_fc_client_label', '=', label or _NO_MATCH)]
if not is_admin:
- domain.append(('partner_email', '=ilike', email or _NO_MATCH))
+ safe_email = escape_like(email)
+ domain.append(('partner_email', '=ilike', safe_email or _NO_MATCH))
return domain