Directory Accounts Could Be Matched To Someone Else - edmozley/freeitsm GitHub Wiki

Security: a directory account could be matched to someone else

Fixed in 4.9.0 · Reported privately by vsc55, with tests (GHSA-gwgh-rphf-wjcq) · Present in every release up to 4.8.0 that uses LDAP sign-in or a directory import (CardDAV from 1.8.0) · Modules: Sign-in, People, System → Directory sync

The short version

Organisations reuse email addresses and sign-in names. When somebody leaves, their address is often given to the next person in the role.

FreeITSM recognised a returning person by name - their email address or directory sign-in name - whenever the directory account itself was new to it. So a new person carrying a reused address was taken to be the old person:

LDAP sign-in. The new person's first sign-in could land in the old person's record. For a requester, that meant seeing the old person's tickets. For an analyst, it meant that analyst's access.
The LDAP, CardDAV and Entra imports could update the old person's record with the new person's details, keeping the old record's tickets and assets attached. CardDAV could then write changes back to the wrong contact.

It needed an account in your own directory carrying the reused address, so it was not open to the internet. If you use LDAP sign-in or a directory import, upgrade.

What was wrong

Every directory account has a permanent identity of its own - a GUID in LDAP and Entra, a card UID in CardDAV - which never changes and is never reused. FreeITSM records it against the person the first time it links them.

The trouble was the order of the checks. On sign-in and on import:

Look for the identity. Found - that is the person. Correct.
Not found - look for the name: the same email address or sign-in name. Found - treat that record as this person, and link the new identity to it.

Step 2 is what lets an existing person sign in with the directory for the first time, so it has to exist. But it never asked whether the record it found was already linked to a different identity from the same directory. A record that is linked elsewhere is not a person signing in for the first time. It is somebody else whose name has been reused.

On sign-in, the refusal was swallowed

On LDAP sign-in, linking the new identity to the old record failed - the database allows one link per person per directory, and the old person still had theirs. But that failure was caught and ignored, with a comment that the person had been "resolved either way". So sign-in went ahead into the old person's record.

It was three importers, not one

The LDAP and CardDAV imports share one matcher, and Entra has its own. All three matched by name without asking whether the record was already somebody else's, and CardDAV went one step further: it re-pointed the old record's link at the new card, so its write-back reached the wrong contact.

The fix

A name match is now only a candidate. Before it is used, FreeITSM asks the question it was missing:

Sign-in (ldapResolveAnalyst(), ldapResolveUser()): an analyst or person already linked to a different identity in the same directory is refused - "This email address belongs to a different directory account. Ask an administrator." The same check now runs where the swallowed failure used to be, so a refused link can never be signed in to anyway.
The imports (dsyncNameMatchRefused() for LDAP and CardDAV, the same rule in Entra's matcher): a record that is already linked to a different identity, or that belongs to a different sign-in source altogether, is left untouched. The person is reported as a conflict in the run's History, whatever the conflict setting, for an administrator to decide who the record belongs to.

Nothing is merged automatically, in either direction. Deciding that two directory accounts are really one person - a hybrid setup, say - is an administrator's call, never an import's.

📁 Files changed

File	Change
🔵 `includes/ldap.php`	`ldapLinkedToOther()`; sign-in refuses an analyst or person linked to a different identity, before and after linking
🔵 `includes/directory_sync.php`	`dsyncNameMatchRefused()` and `dsyncRefusedMessage()`; the LDAP import reports a refused match as a conflict
🔵 `includes/carddav_sync.php`	the CardDAV import does the same, so nothing is re-pointed at the wrong card
🔵 `includes/entra_sync.php`	the Entra import does the same
🟢 `tests/directory-identity-reuse.php`	the proof - see below

Tested

tests/directory-identity-reuse.php runs the cases from the report through the real functions on a test database: a reused email at LDAP sign-in, for an analyst and for a portal user; a name match in the LDAP and CardDAV import matcher on a record already linked to a different identity, or owned by a different sign-in source; and the same case in the Entra import. 14 checks pass on 4.9.0. On 4.8.0, 7 of them fail - the control that shows the test catches the bug rather than passing whatever the code does. Controls inside the test also prove the ordinary cases still work: the same identity signs in again, a genuinely new person is created, and a person signing in for the first time with an unlinked record is still matched by email.

FREEITSM_TEST_DATABASE=<a throwaway database> php tests/directory-identity-reuse.php

Do I need to do anything?

Upgrade to 4.9.0, then run System → Database Verification as usual.

After that, look at the History of your next directory import run. Any person reported as a conflict is a record two directory accounts were claiming. Decide who it belongs to - usually the old person, who has left, and a new record for the new starter.

If someone reports "This email address belongs to a different directory account" at sign-in, that is the fix working: their address used to belong to someone else here. Give them their own record, or unlink the old one if it really is the same person.

For developers

The rule, for any new way of recognising a person: a match on a name - email, username, display name - is a candidate, never an identity. Before using it, check that the record is not already linked to a different permanent identity from the same source, and not owned by a different source. Use ldapLinkedToOther() for sign-in and dsyncNameMatchRefused() for imports rather than writing the query again.

Never catch and ignore a failed identity link. A unique-key failure on a link table is the database telling you the record belongs to somebody else. Treat it as a refusal.