Demo Data Deleted Real Accounts

The demo data import deleted real accounts

Importing the Core demo data on a working installation deleted every analyst except one, every self-service requester, and the departments, teams and permission roles alongside them. The person it happened to then found he could not sign back in, permanently, and that re-creating his account by hand did not help.

Reported by Kristian Madsen - the same person as issue #117, immediately after that one was settled.

Fixed in f3fe850e, a99f630b, f935d8c8 and ace526af, released as updates #1296, #1297 and #1298.

This is really three faults wearing one coat, and the second one is the interesting one, because it only became visible because the first one was fixed badly a long time ago.

---

1. What you saw

"I had an account that was made with OIDC and the account was working fine. After I imported the demo data and users, it removed my account both on service and analyst. It gives me this error when trying to sign in - 'Your account is no longer available. Contact your service desk.' - it doesn't exist anywhere in the admin interface."

Two symptoms, and it matters that they are separate:

1. The account is gone from the admin interface. That is the importer.
2. Signing in is impossible, and stays impossible even after an administrator re-creates the account with the same email address. That is the sign-in code, and it would have happened to anyone whose account was deleted by any means that skipped the database's own housekeeping.

---

2. Fault one: the importer had no idea which rows were its own

Before inserting anything, the importer cleared each table it was about to fill. It had to - otherwise importing twice would give you two of everything. But nothing distinguished a sample row from a real one, so "clear the table" meant the whole table:

DELETE FROM analysts WHERE NOT (username = 'admin')   -- one hardcoded survivor
DELETE FROM users                                      -- no exemption at all

The single exemption for admin comes from a _skip_insert marker in database/demo-data/core.json, which exists so the importer can reference the admin account rather than create one. It was never a safety mechanism, and it protected exactly one row.

Importing Core therefore removed:

Table	What that is
analysts	every analyst except admin
users	every self-service requester, with no exemption
departments, teams	the whole organisational structure
rbac_roles, rbac_role_capabilities	the permission model - 80 capability rows
ticket_types, ticket_origins, ticket_prefixes	lookup values that existing tickets point at

What the screen said about that

"Designed for fresh installations only. Importing demo data into a system that already contains real data may cause conflicts."

It does not cause conflicts. It deletes. And the confirmation dialog - which does exist - was only shown when the button had already been used in that page session, so the very first import, the one that takes an installation from having no demo data to having it, was the only one that never asked.

---

3. Fault two: a sign-in link that outlived its account

This is the half that turned a bad afternoon into a permanent lock-out.

Both identity tables declare the correct constraint:

CONSTRAINT `fk_user_sso_identity_user` FOREIGN KEY (`user_id`)
    REFERENCES `users` (`id`) ON DELETE CASCADE

Delete a requester and their sign-in link goes with them. That is why nobody had ever hit this using the admin interface.

But the importer ran its deletes like this:

$conn->exec("SET FOREIGN_KEY_CHECKS = 0");

Turning foreign key checks off does not only skip the checks. It skips the cascades. The accounts were deleted; the links survived, pointing at ids that no longer existed.

Why a leftover row locked him out

Every sign-in path resolves a person in the same three steps:

1. an existing link, by (provider, subject),
2. failing that, match an existing account by verified email,
3. failing that, create one just in time.

Step 1 runs first, and the branch was chosen on "is there a link row", not "is there an account":

$userId = $stmt->fetchColumn();   // finds the orphan

if ($userId) {                    // true - so we are committed to "returning user"
    $user = ssLoadUser($conn, $userId);
    if (!$user) {
        ssoBail('Your account is no longer available. Contact your service desk.');
    }

Steps 2 and 3 live in the else. He could never reach them. And that is why re-creating his account did not help - the link still pointed at the old id, so the new account was never even looked for.

The irony worth keeping

Had the cascade been allowed to fire, nobody would ever have noticed any of this. The link would have gone with the account, the next sign-in would have fallen through to just-in-time provisioning, and the account would have quietly reappeared. The bug that made the damage permanent is the same line that made it silent.

---

4. It was four branches, not two

The convention here is to sweep for other instances before writing anything up, and this one paid for itself. The same resolve-by-link-first shape exists in the LDAP and Active Directory path:

File	Function	What it said when the account was missing
api/auth/oidc_callback.php	analyst branch	"Your account is inactive" - for an account that does not exist at all
api/auth/oidc_callback.php	completeSelfServiceSso()	"Your account is no longer available"
includes/ldap.php	ldapResolveAnalyst()	"Your account is inactive"
includes/ldap.php	ldapResolveUser()	"Your account could not be found"

Four branches, four different messages, one fault. Two of the four messages actively name the wrong cause: an account that has been deleted is not an account that is inactive, and an administrator sent looking for a disabled account will not find one.

The handling now lives in one place, includes/sso_identity.php, required by both includes/oidc.php and includes/ldap.php.

The LDAP one had a second layer

ldapResolveAnalyst() re-links the identity at the end, inside a try/catch that deliberately swallows a unique-key failure - a reasonable guard against a concurrent sign-in. But it means that if you fix only the fall-through and not the stale row, the person signs in and the link goes on pointing at the deleted id for ever, with nothing anywhere reporting it. The first version of the test for this passed with the fix disconnected, for exactly that reason. It only became a real test once it asserted where the link ended up, not just that sign-in succeeded.

---

5. The fix

Demo rows are marked

Every row the importer creates now carries is_demo = 1, on all 73 tables it writes to, and only marked rows are ever removed:

DELETE FROM `$tableName` WHERE is_demo = 1

The mark is set in the importer rather than in the JSON, so it cannot be forgotten when a new demo file is written.

Foreign key checks stay on

SET FOREIGN_KEY_CHECKS = 0 is gone. With checks on, a demo row that something else still refers to cannot be silently orphaned - the delete fails, the transaction rolls back, and the import reports what is in the way:

"The Tickets demo data still refers to the Core demo data, so it cannot be replaced while that is in place. Remove or re-import Tickets first. Nothing has been changed."

This is a deliberate loss of capability. Re-importing Core while the Tickets demo data exists used to "work". It worked by stranding rows.

A dangling link heals itself

A link whose account is gone is now cleared, and the sign-in continues down the ordinary first-time path - the same verified-email and provider-assignment checks a new user faces. Nothing becomes reachable that a first-time user could not already reach: deleting an account through the interface already cascades the link away and already permits just-in-time re-creation.

The screen tells the truth

• The confirmation is asked before every import, not only a repeat.
• The warning says what actually happens, and that your own records are not touched.
• Installations holding demo data from before the mark existed are told so. There is no way to tag it retrospectively - nothing ever recorded which rows came from where - so the honest move is to say it rather than quietly produce a second copy.
• The duplicate-key failure that such an installation hits is now explained, rather than reported as Duplicate entry 'jsmith' for key 'analysts.uq_analysts_username' - a message naming a record the administrator never created.

And one thing the detection had been getting wrong all along

The page worked out whether a module had been imported by asking "does this table have any rows in it?" for most modules. So an installation with real tickets, or real assets, was told its demo data was already imported. Same root gap - nothing distinguished demo from real - showing a second face on the same screen. It now counts marked rows, and covers all 19 modules instead of five.

---

6. Files

🗄️ schema · 📖 read · ✏️ write · 🖥️ UI · 🌍 strings · 🧪 test

🎨	File	What you do there
🗄️	database/freeitsm.sql	is_demo on 73 tables
🗄️	includes/db_verify_schema.php	the same 73, so existing installations gain the column
✏️	api/system/import_demo_data.php	marked-rows-only clearing, checks left on, explained failures
📖	includes/demo_data.php	new. The one home for module → tables, demo row counts, blocking-module lookup, untagged detection
📖	api/system/check_demo_core.php	counts marked rows instead of asking whether a table has any
✏️	includes/sso_identity.php	new. ssoClearDanglingLink(), shared by both sign-in paths
✏️	api/auth/oidc_callback.php	both branches fall through instead of dead-ending
✏️	includes/ldap.php	both resolvers, the same
🖥️	system/demo-data/index.php	confirm on every import, untagged banner, per-module detection
🌍	lang/*/system.php	rewritten warning; new confirmations in the 7 locales carrying this namespace
🧪	tests/sso-dangling-link.php	12 assertions across both sign-in paths

---

7. How it was verified

Two kinds of proof, because neither is sufficient alone.

The test suite covers the sign-in half: that the cascade works when checks are on, that the orphan appears when they are off, that clearing it restores the fall-through, that a healthy link on the same provider is untouched, and that the LDAP resolver ends up pointing at the re-created account. Proved load-bearing by disconnecting the fix - 12 passing becomes 11 passing and 1 failing.

A live run covers the importer, on a throwaway database built from database/freeitsm.sql: all 19 modules import; a real analyst and a real requester survive an import and a re-import with their ids unchanged; a re-import replaces rather than duplicates; and the cross-module refusal rolls back leaving nothing changed. It was the live run, not the tests, that found the re-import refusal - the tests were written against the design and would have gone on agreeing with it.

Then the same thing on a real development database with 108 tickets, 68 requesters and 7 analysts in it, checked with a row-count snapshot of all 264 tables before and after. The only difference across the whole database was the SLA cron ticking over during the test.

The mistake in the middle, which is the useful part

The first attempt at the throwaway run appeared to fail: Duplicate entry 'jsmith' on a database that had just been created empty. It had not failed. PHP's opcache was still serving the previous database configuration file, so the import had been running against the real development database all along.

Nothing was damaged, and why nothing was damaged is the point: the delete phase ran, matched no rows because nothing was marked, and the insert then collided and rolled back. Under the old code that same slip would have emptied analysts and users - which is to say the accident reproduced the original bug's conditions exactly and demonstrated the fix at the same time.

Every subsequent run went through a gate: a probe page that calls opcache_reset(), connects, and returns SELECT DATABASE(), with the harness refusing to continue unless the answer is the throwaway. If you swap a database configuration under a running web server, do not trust that it took effect. Ask the web server.

---

8. What this means for you

• If you have never imported demo data, nothing here affects you, and importing is now safe on a populated installation: only rows the importer created are ever removed.
• If you imported demo data before update #1297, those rows are not marked and cannot be recognised. The page will tell you. Re-importing would add a second copy, so remove the old sample records by hand first if you want a clean set.
• If you have lost accounts to this, the recovery is two statements, and the local admin analyst is the way back in because it is the one row the importer spared:

DELETE FROM user_sso_identities    WHERE user_id    NOT IN (SELECT id FROM users);
DELETE FROM analyst_sso_identities WHERE analyst_id NOT IN (SELECT id FROM analysts);

Then sign in again. The account is re-created if your provider allows it, or matched against one an administrator re-creates with the same email address. On any installation running #1296 or later this happens by itself and the statements are unnecessary.

---

9. What is not fixed

• There is no "remove demo data" action. Only import. So when a module blocks a re-import, the way out is to re-import the blocking module rather than clear it, which is a blunt instrument.
• Untagged demo data cannot be recognised, and no amount of cleverness will change that - the information was never recorded. The detection probe looks for an analyst named jsmith, which is a guess, not a fact: a real person with that username would be reported as untagged demo data.
• Whether a cross-module re-import should cascade - clearing Core's demo rows also clearing the demo rows that depend on them - is undecided. It would be safe, since only marked rows are ever touched, but it deletes things the administrator did not name.

---

10. Related

• Sign-in redirected to the wrong address - the same reporter, the week before
• Single sign-on · LDAP and Active Directory
• Database verification - how the is_demo column reaches an existing installation