Multi Tenancy Isolation

Multi-tenancy: data isolation & the hardening audit

Part of the Multi-tenancy section. Built — on main. Covers how one company's data is kept separate from another's, and the audit that closed the gaps before it was trusted for real multi-company use.

The single biggest risk in any multi-tenant system is one client seeing another client's data. This page explains exactly how FreeITSM prevents that at the tickets layer, the class of bug we were guarding against, and the three-pass audit that found and fixed the holes.

The isolation model

The boundary between companies is not encryption — it's a tenant_id filter applied to every query that touches company-scoped data, routed through one shared choke-point (includes/tenancy.php) so there's a single place to get it right. Two primitives do the work:

1. ticketTenantFilter() — for list/aggregate queries

Returns a SQL fragment that scopes a tickets query to the analyst's active company:

• Multi-company: AND (t.tenant_id = ? OR t.tenant_id IS NULL) for the Default company (which also owns un-routed tickets), or AND t.tenant_id = ? for a client company.
• Single-company install (N=1): returns an empty string and no parameters — a complete no-op. This is the load-bearing guarantee: with one company, none of this machinery does anything, so single-company installs behave exactly as before.

Used by the ticket list, the folder counts, search, the scheduled-work calendar, the dashboard widgets, and the per-user ticket count.

2. analystCanAccessTicket() — for "by id" access

Some endpoints act on one specific ticket chosen by id (open a ticket, send a reply, add a note, delete it). A list filter doesn't help here — you need an access check. This helper answers "may this analyst touch this ticket?":

• true on a single-company install (nothing to isolate).
• otherwise true only if the ticket's company is in the analyst's accessible set (an all-access analyst sees every company; others only those granted to them).
• unknown ticket id → false (so a probe can't even confirm a ticket exists in another company).

A ticket with tenant_id NULL (an un-routed ticket) is treated as Default-owned, so access follows the Default company.

The rule of thumb

List view → ticketTenantFilter (active company). Act on one record by id → analystCanAccessTicket (accessible set). Anything reached by a child id (an email, an attachment, a note, a time entry) is resolved back to its parent ticket first, then access-checked.

The class of bug we were closing: cross-company IDOR

The feature was rolled out in slices: the ticket list and counts were scoped first. But a service desk reaches a ticket's data through many endpoints — the email thread, attachments, notes, time entries, the audit trail, SLA state, CSAT, plus every action (assign, reply, schedule, delete…). Any one of those that took a ticket id (or a child id) without an access check was an IDOR — Insecure Direct Object Reference: an analyst in Company A could, by changing an id in a request, read or modify Company B's data.

Crucially, none of these are visible or exploitable on a single-company install (there's only one company, so nothing to leak). They only matter the moment a second company exists — which is precisely why they had to be found and closed before anyone runs a real multi-company setup, not after.

The three-sweep audit

Rather than assume the rollout was complete, the change was audited in independent passes before merging to main. Each pass deliberately cast a wider net than the last — and each one found real gaps the previous sweep had missed. That progression is the interesting part:

Pass 1 — pre-merge audit (the categories)

Two independent reviews of the whole branch confirmed two things: (a) single-company (N=1) behaviour was byte-for-byte unchanged, and (b) the already-scoped paths (list, counts, triage) had no holes — including the trickiest query, the folder-count aggregation, where the tenant fragment lands in a LEFT JOIN ... ON clause and the bound parameters must be ordered ON-clause-before-WHERE. It also surfaced the category of remaining work: older ticket endpoints that predated the feature and weren't yet scoped.

Pass 2 — verifying the first fix (found 5 more writes)

After scoping the obvious list reads and the first batch of by-id endpoints, a verification pass swept all of api/tickets and found five more mutation-by-id endpoints with no guard — most seriously delete_ticket, which would let an analyst delete another company's ticket (and cascade-delete its emails, attachments, notes and audit). Also unguarded: schedule_ticket, save_note, save_time_entry, log_ticket_audit.

Pass 3 — the non-obvious parameters (found 5 more)

The id-based sweeps keyed on ticket_id. A third pass looked instead at endpoints reached by a different identifier, and found a fresh batch:

• get_attachment (High severity) — served an email attachment's file content by attachment id, with no check. An analyst could fetch another company's attachments by enumerating ids. Fixed by resolving attachment → email → ticket and then access-checking.
• debug_get_attachment — same leak, plus it dumped SQL, server paths and directory listings (a debug artifact that shouldn't be in production).
• get_ticket_sla / get_tickets_sla_batch — returned SLA timing/priority for any ticket id (the batch one looped over a caller-supplied list).
• request_csat — could trigger a satisfaction-survey email to another company's requester.

Pass 4 — the coverage matrix (clean)

A final pass enumerated every endpoint in api/tickets and classified each: does it touch a specific ticket or a child row (emails, attachments, notes, time entries, audit, CMDB links, recordings, SLA, CSAT), and is it guarded? It also chased indirection (any child id must resolve to its parent ticket before the check) and the usual suspects (recordings, mailbox lookup, reply-body builder, by-ticket_number lookups). Verdict: fully isolated — no remaining ticket-data-by-id leak. That's when it merged.

How each gap was fixed

Kind of endpoint	Fix
List / aggregate reads (search, user tickets, scheduled, dashboard widgets, per-user count)	Append ticketTenantFilter() to the query; merge its parameters in the correct positional order
Detail & child-data reads by id (email detail, thread, notes, attachments list, audit, CMDB links, time entries)	analystCanAccessTicket() guard right after the DB connection; child ids resolved to their ticket first
Mutations by id (assign, owner, send, AI cleanup, schedule, delete, note, time, audit, CMDB link/unlink)	Same analystCanAccessTicket() guard before acting; forbidden/unknown → "not found"
Attachment serving	Resolve attachment → email → ticket, then guard
Per-ticket SLA / CSAT	Guard the ticket id (the batch endpoint silently skips ids the analyst can't access)
Triage filing (write-auth)	analystCanAccessTenant() — can't file a triaged ticket into a company you can't access

A subtlety worth recording: the dashboard-widget endpoint builds one shared WHERE reused by ~15 aggregate queries, so the tenant condition is injected once where that WHERE is assembled — every widget inherits it, and the parameter order stays correct across the categorical, time-series and created-vs-closed query shapes.

What's deliberately not covered yet

Isolation was completed for the tickets module. Other modules that read the tickets table are Phase 3 work and are tracked, not forgotten — they only matter once a second company exists:

• Self-service portal (requester-facing — needs its own tenancy model)
• CMDB → tickets cross-reads, Tasks ↔ ticket links
• Reporting / cron machinery: Watchtower queries, the SLA breach cron, the workflow engine, CSAT and templated-email senders

There's also a small non-tenancy cleanup outstanding: the debug_get_attachment diagnostic still discloses server paths and should simply be removed.

Rolling isolation to the other modules (Phase 3) — and why the service layer made it nearly free

Tickets was the hard, hand-audited proving ground. Every module after it follows the same two-part mechanism (list filter + by-id guard), and by the time Phase 3 started the groundwork from two other projects — the service-layer refactor and the REST-API rollout — had turned each module into a largely copy-paste job. Change Management was the first (after tickets and problems), and it's the template for the rest.

The per-module recipe (generalised from the tickets work into reusable helpers):

1. Schema — add tenant_id INT NULL to the module's main table (+ index + FK to tenants). NULL normalises to the Default company on every read, so existing rows need no data migration and single-company installs are unaffected.
2. The write path — the module's service (includes/services/<module>.php) gets the company gate in one place: its by-id loader checks ctx->companyScope and 404s anything out of scope; its create takes an adapter-resolved company id and stamps it. Because the UI and the API both call that one service, every write (update, delete, comments, sub-objects…) inherits isolation from a single ~10-line gate.
3. The read perimeter — a generic analystCanAccessChange()-style guard (twin of analystCanAccessTicket) on each direct-read endpoint that sits outside the service, plus activeTenantFilter() (the generic ticketTenantFilter) on the list/aggregate queries.
4. The REST API — the resource gains a ?company_id= filter, key-scope enforcement (apiKeyTenantFilter on the list, apiKeyCanAccessTenantRow in its apiLoadX), a company field in the response, and company_id/default resolution on create. About five mechanical edits, because the API framework was built tenancy-aware from day one (keys already carry their company scope).

Why the earlier architecture work paid off here (this is the important bit):

• One write path, not two. Pre-refactor, "create/update/delete a change" existed twice — once in the UI endpoint, once in the REST resource. Isolating a module would have meant adding the same tenant_id stamp + scope gate in both copies and keeping them in lockstep forever (drift being the exact bug class the refactor killed). Post-refactor it goes in the service once, and both transports inherit it.
• ActorContext.companyScope already existed and was proven. The service layer had already introduced the normalised "who is acting + what companies may they see" object, and Problems had already been scoped through it. So there was nothing to design — the new module's loader is Problems' loader, transplanted. The adapters (ActorContext::fromSession / fromApiKey) already compute the right scope from the session or the API key.
• The API was tenancy-first. apiKeyCanAccessTenantRow, apiKeyTenantFilter, apiKeyDefaultTenantId already existed and were in use by tickets/problems, so the API side really was a five-line transplant.

The honest caveat. The service layer only unified each module's core CRUD. A module's perimeter — audit/comments/roster/attachment endpoints that were left as UI-only direct queries during the refactor — gets no automatic protection from the service gate and must be guarded by hand, one endpoint at a time (exactly the completeness discipline the tickets audit taught). That perimeter sweep is the bulk of the file count in a Phase-3 diff. So the refactor didn't make everything free — it made the risky, easy-to-get-wrong part (the shared business rules) a copy-paste, and left only the mechanical perimeter to enumerate.

Rough magnitude: scoping Change Management was on the order of 60–70% less work than it would have been pre-refactor, and — more importantly — lower risk, because the isolation logic lives in one audited place per module rather than being duplicated across two drift-prone copies. The same dividend applies to every remaining Phase-3 module (CMDB, Calendar, Forms, Network Mapper, Assets, Knowledge). It's a concrete payoff of paying down the duplication earlier.

The takeaway

The mechanism is simple — one filter for lists, one access check for by-id — but completeness is everything: a single missed endpoint is a leak. The value of auditing in widening passes (rather than trusting the first sweep) is concrete here: passes 2 and 3 each caught real, exploitable gaps — including a cross-company delete and an attachment-content read — that a single look would have shipped. And the value of the service-layer + tenancy-first-API groundwork is equally concrete: it turned "isolate a module" from a bespoke, error-prone, two-places-to-keep-in-sync job into transplant-a-template-and-sweep-the-perimeter.