Raising Tickets for Others Developer Guide

Raising tickets for others — Developer Guide

How the requester picker works underneath: the request flow, where authorisation happens and why it happens twice, and the three bugs the feature fixed on the way through. The plain-language version is Raising a ticket for someone else.

Asked for in discussion #54. Shipped in 0a9ac8cb.

---

1. 📁 The files involved

🔵 Front end

File	Role
tickets/index.php	The combobox, the chosen-state chip, the inline new-requester panel, and the browse modal — all inside #newTicketModal
assets/js/inbox.js	initRequesterPicker() and the req* functions; createNewTicket() sends the result
assets/css/inbox.css	.req-* rules, including the [hidden] override described in §7
lang/en/tickets.php	tickets.new_ticket_modal.requester_*

🟠 Endpoints

File	Role
api/tickets/get_users.php	The search. Company-scoped (§4), ?search= and ?limit=
api/tickets/create_ticket.php	Accepts user_id, authorises it (§5)

🟢 Service

File	Role
includes/services/tickets.php	TicketsService::createTicket() — resolves the requester, creates one when needed (§6)
includes/tenancy.php	activeTenantFilter(), analystCanAccessUser(), resolveTenantForNewUser() — all pre-existing

⚪ Tests

File	Role
tests/security-findings/run.php	Four checks under Unscoped lists

No schema change. users.tenant_id and the search columns already existed.

---

2. 🔁 The flow

  analyst types "dan"
        │
        │  200ms debounce  (one request per pause, not per keystroke)
        ▼
  GET api/tickets/get_users.php?limit=8&search=dan
        │      └── activeTenantFilter() on the LIST, not only the count   §4
        ▼
  reqRenderResults()  →  matches + an "add someone new" row that is ALWAYS last   §3
        │
        ├── picks a person ─────────► reqChoose()   sets reqChosenUser = {id,…}
        │                                            swaps input for the chip
        │
        └── picks "add new" ───────► reqShowNew()   reveals name + email,
                                                    pre-filled from the term
        ▼
  createNewTicket()  posts EITHER  {user_id: 150}
                            OR     {from_name: "…", from_email: "…"}
        ▼
  create_ticket.php   analystCanAccessUser()  ← the boundary check              §5
        ▼
  TicketsService::createTicket()
        ├── user_id present → SELECT the row, read the address back FROM it
        └── otherwise       → match on email, or INSERT with a resolved company  §6

---

3. 🎯 Why the "add new" row is unconditional

reqRenderResults() appends the add row on every render, including when there are matches:

const addRow = `<div class="req-opt req-opt-add" role="option" data-add="1">…</div>`;
e.results.innerHTML = empty + rows + addRow;

The obvious design is to show it only on zero results. It is wrong for a specific and easily-missed reason: the case where you are adding somebody whose name resembles an existing person is exactly the case where a zero-results trigger never fires. Adding a second Daniel to a system that already has one would leave you with a list of Daniels and no way to say "not those, a new one".

It is also visually separated (border-top on .req-opt-add, a + avatar) because it is a different kind of action rather than the least-good match.

Preserving what was typed

const looksLikeEmail = term.indexOf('@') !== -1;

The term goes into the email box if it contains an @, the name box otherwise, and focus lands on the empty one. Retyping something you have already typed is what makes a fallback feel like a penalty for not finding the person.

---

4. 🔴 The list was not company-scoped

This was a live cross-company leak, found while building on top of the endpoint.

get_users.php called ticketTenantFilter() and applied it to the per-user ticket count subquery only:

list($ttSql, $ttParams) = ticketTenantFilter($conn, $analystId, 't');
// …
(SELECT COUNT(*) FROM tickets t WHERE t.user_id = u.id{$ttSql}) as ticket_count
FROM users u                              -- ← no filter here

The comment beside it explained that the count must not reveal a requester's activity in other companies — true, and the reason the file read as careful. Meanwhile FROM users u returned every requester on the install, with email addresses, to any analyst holding the tickets module. Scoping the number attached to a row is worth nothing when the row is there to be counted.

Same shape as the v1 /users list closed in the August security round; this is its analyst-facing twin. A searchable picker over an unscoped list would have made it materially worse, which is why it was fixed before the feature was built rather than after.

The fix, and the precedence trap

list($uSql, $uParams) = activeTenantFilter($conn, $analystId, 'u');
$sql .= " WHERE 1=1" . $uSql;
$params = array_merge($params, $uParams);

if (!empty($search)) {
    $sql .= " AND (u.display_name LIKE ? OR u.email LIKE ? OR u.username LIKE ?)";

⚠️ The brackets are half the fix. The original search clause was a bare WHERE a LIKE ? OR b LIKE ? OR c LIKE ?. Appending AND tenant_id = ? to that binds the company condition to the last OR branch only — every row matching display_name or email would still come back from any company. AND binds tighter than OR, so a tenancy clause added to an unbracketed OR list is decorative.

activeTenantFilter() rather than a hand-rolled clause because it already encodes that the Default company owns tenant_id IS NULL rows — so requesters who have never been assigned a company stay visible from Default instead of vanishing from every queue.

Measured: an analyst scoped to one company saw all 34 requesters before, and 1 after. An unscoped admin still sees theirs, which is the control proving it is a filter rather than a break.

?limit= was added at the same time (clamped 1–200, interpolated as an integer, never a bound parameter — MySQL will not take a placeholder in LIMIT). The picker asks for 8, the browse modal for 100.

---

5. 🔒 Why the id is checked at the boundary

if ($userId !== null && !analystCanAccessUser($conn, $analystId, $userId)) {
    echo json_encode(['success' => false, 'error' => 'That requester was not found']);
    exit;
}

A scoped list is not a check. get_users.php governs what is easy to choose; user_id arrives in a JSON body and can be any integer. Without this line an analyst scoped to one company could file a ticket against another company's contact by editing one number — the same shape as the S1/S2 findings, arriving fresh with a new feature rather than inherited from an old one.

The refusal text matches a non-existent id on purpose: "not yours" and "not there" must stay indistinguishable, or the refusal confirms the record.

The service takes user_id on trust, exactly as it takes $tenantId on trust, and says so in a comment at the top of createTicket(). That is the existing division of labour in this codebase — boundaries authorise, services execute — and breaking it for one field would be worse than following it.

---

6. 🧩 Requester resolution in the service

$requesterUserId = isset($in['user_id']) && (int)$in['user_id'] > 0 ? (int)$in['user_id'] : null;

Optional and additive — every existing caller (the v1 API, inbound mail, the portal) still sends an address and is unaffected. When present it wins, because "this exact person" is a stronger statement than an address that has to be matched.

Two consequences worth noting:

The address requirement is now conditional. It is required only when there is no user_id, because picking somebody already identifies them — and a directory-backed requester may legitimately have no mailbox at all (GitHub #47).

The address is read back FROM the record, not from the request:

$pStmt = $conn->prepare("SELECT id, email, display_name FROM users WHERE id = ?");

So the ticket and its initial email row describe the person that was actually picked, and a stale id fails loudly rather than creating something unexpected.

🟠 The company that was never set

On the create path, the old code was:

$conn->prepare("INSERT INTO users (email, display_name, created_at) VALUES (?, ?, UTC_TIMESTAMP())")

No tenant_id. Meanwhile api/tickets/save_user.php, includes/ldap.php and api/auth/oidc_callback.php all call resolveTenantForNewUser() to map the email domain to a company. Three paths did it properly and the fourth quietly did not — so a requester born from a manual ticket had no company at all on a multi-company install, their tickets sat in triage indefinitely, and a scoped analyst could not see them.

$newTenantId = function_exists('resolveTenantForNewUser')
    ? resolveTenantForNewUser($conn, $requesterEmail)
    : null;

null is still the right answer when the domain maps to nothing — "unknown, send to triage" rather than a guess. Verified both ways, which matters because NULL is also what a broken fix would produce: an unmapped domain still yields NULL, and @bateswells.co.uk (present in tenant_domains) now yields company 4.

---

7. 🎨 Front-end details worth keeping

The [hidden] override

.req-chosen[hidden], .req-picker[hidden], .req-new[hidden],
.req-results[hidden], .req-chosen-company[hidden] { display: none !important; }

⚠️ The hidden attribute works only through the user-agent rule [hidden] { display: none }, which any author rule setting display beats on specificity. .req-chosen { display: flex } therefore rendered an empty chip above the search box permanently, with correct markup. Found in a screenshot; invisible to reading the HTML, because the HTML was right.

mousedown, not click

e.results.addEventListener('mousedown', (ev) => { … });

The input's blur closes the list. On click the list is gone before the event lands, so the row never activates.

Motion

The dropdown uses transform-origin: top center and a 150ms cubic-bezier(0.23, 1, 0.32, 1) entry from scale(0.97) — it belongs to the field it hangs off, not to the middle of itself, and nothing in the real world appears from nothing. The browse button and the clear button take scale(0.92) on :active. All of it is disabled under prefers-reduced-motion.

The picker itself is not a modal, and that is the main interaction decision on the page: raising a ticket for somebody is a tens-of-times-a-day action, and a second surface to open and close is friction paid on every one of them to help a rare case. The modal exists behind the magnifier and you only meet it by asking.

---

8. 🧪 Verification

Four checks in tests/security-findings/run.php under Unscoped lists:

Check	Guards against
get_users.php scopes the user LIST	the leak returning
…and brackets the search terms	the OR-precedence trap
create_ticket.php authorises a chosen id	the picker becoming a cross-company write
a manual-ticket requester gets a company	the fourth path drifting again

Sabotage-verified: removing the filter fails the first check.

Live, with controls:

• cross-tenant user_id → refused; the same analyst's own company → succeeds
• the old from_name + from_email payload → still works, so the v1 API and inbound mail are unaffected
• unmapped domain → NULL; mapped domain → company 4

⚠️ A measurement method that was wrong. --window-size=360 does not produce a 360px viewport in headless Chrome on Windows — the minimum window width forces about 504, so a "360px" screenshot is a 360-wide crop of a 504-wide page. Both this feature and the row-display chips are now checked inside a 360px iframe, which does evaluate media queries at 360, and both report document.body.scrollWidth === 360.

---

9. 🔮 What this does not do

• Changing the requester on an existing ticket. The picker is a component in one modal; nothing consumes it elsewhere yet. It is the obvious next use and would need its own audit trail entry.
• Merging duplicate requesters. Two records for the same person created before this change stay two records.
• The portal. Requesters raising their own tickets are already identified by their session; there is nothing to pick.

---

Related pages

• Raising a ticket for someone else — the plain-language version
• Tickets · Multi-Tenancy — Users and Self-Service
• Security hardening 2026-08 — Round three Developer Guide — the v1 twin of §4
• Multi-Tenancy — Pitfalls