Extending Directory Sync - edmozley/freeitsm GitHub Wiki

Extending directory sync

Recipes for changing the feature, and the checklists that stop a change being half-applied. How it works is Directory sync โ€” Developer Guide.

Every list here was produced by tracing an existing field or flavour through the code, not from memory. If you add something and the list looks wrong, trace it again and fix the page โ€” a stale checklist here is worse than none, because it will be trusted.

1. Adding a field that imports from the directory

The canonical example is office, which appears in fourteen places. Miss one and the failure is usually silent: the column exists, the mapping box exists, and nothing ever writes to it.

๐Ÿ”ต Storage โ€” 4 places

File What to add
database/freeitsm.sql users.<field>, and auth_providers.ldap_attr_<field>
includes/db_verify_schema.php The same two columns, in the users and auth_providers blocks

โš ๏ธ Both files, always. freeitsm.sql is the fresh install; the $schema array is the upgrade. Dev only ever exercises the upgrade path, so a missing freeitsm.sql entry is invisible until somebody installs from scratch.

๐ŸŸข The engine โ€” includes/directory_sync.php, 5 places

Function What to add
dsyncAttrDefaults() The default attribute name for both flavours โ€” the ad and the openldap arrays
dsyncFetchPeople() dsyncAttr($provider, '<field>') in the $attrs list, or the attribute is never requested
dsyncMapPerson() Read it out of the entry
dsyncApplyToExisting() Add to the $map array, so a change is detected and logged
dsyncCreate() The column and value in the INSERT

Optionally dsyncFieldLabel() (so the run log says "Office" and not office) and dsyncNewPersonSummary() (so it shows on a "will be added" row).

โšช The person record โ€” includes/users.php, 2 places

Constant Meaning
USER_PERSON_FIELDS It is a person field, so api/tickets/save_user.php will accept it
USER_DIRECTORY_OWNED The directory owns it, so it is refused on a managed record rather than accepted and then silently overwritten by the next sync

userPersonFieldValue() if the value needs normalising.

๐ŸŸ  The admin screens โ€” 3 places

File What to add
api/system/save_sso_provider.php The $ldap array entry, the $cols list and the $vals list โ€” these are positional, so they must stay in step. Name the column after its request key, with an ldap_ or sync_ prefix: on an update, a setting the screen does not send keeps its stored value, and that rule finds LDAP settings by prefix and matches them to the request by name (#1723)
system/sso/provider.php A row in $mapRows, and the field in payload()
api/system/test_directory_mapping.php The form-value passthrough list and the $fields array, or Test will not show it

๐ŸŸฃ Strings and tests โ€” 2 places

File What to add
lang/en/system.php map_field_<field> and map_hint_<field>. An empty hint is fine; a missing key renders the key itself
tests/people-fields.sh An assertion, so the next person to add a field finds out when they miss one

๐Ÿ“Œ A quick way to check you got them all: grep -rn "office" includes/ api/ system/ lang/en/ database/ tests/ and compare the file list against yours.

2. Supporting another kind of directory

dsyncFlavour() decides between ad and openldap, and it does so by inference, not configuration:

// Whatever ldap_attr_username / ldap_attr_guid were set to already tells us
// which directory this is. Asking again would let the two answers disagree.

dsyncAttrDefaults($flavour) then supplies the attribute names, and dsyncAttr() prefers a configured value over the default.

To add a third flavour: extend both, and be honest about what does not carry across:

Assumption Where Applies to
userAccountControl bit 2 = disabled DSYNC_UAC_DISABLED, dsyncMapPerson() AD only. A directory without the attribute reports 0, read as enabled โ€” the right default, since we should never invent a reason to deactivate somebody
objectGUID is binary, entryUUID is text ldapStringifyId() in ldap.php Handled, but a third flavour needs checking
LDAP_CONTROL_PAGEDRESULTS is supported dsyncFetchPeople() Not universal. A server that ignores it returns its own cap and no error
manager holds a DN dsyncResolveManagers() Some directories hold a name or a uid instead โ€” that would need a different second pass, not a different attribute name

The fixture exists precisely so a third flavour cannot be added blind: docker/ldap-test/ runs Samba AD and OpenLDAP side by side.

3. Adding an action to the run log

dsyncLogEntry($conn, $runId, $action, $userId, $person, $detail).

A new action value needs four things, or it is recorded and never seen:

  1. The display order โ€” get_directory_sync_log.php sorts by an explicit FIELD(action, โ€ฆ) list. An unlisted action sorts first, which is probably not what you want.
  2. The filter chips โ€” RUN_ACTIONS in system/sso/provider.php, including whether it starts selected.
  3. Both tenses โ€” act_will_<action> and act_did_<action> in lang/en/system.php. A preview describes the future and an import describes the past; one label for both states as fact something nobody has agreed to yet.
  4. A colour โ€” .act.<action> in the page CSS. Green arrives, amber changes, grey leaves, red needs you.

detail is English prose written at run time, not a translation key. The log is written once and read later, and the run has no user whose locale could render a key.

4. Adding a write to the engine

๐Ÿ”ด It must sit inside a $preview guard. There are seven. A write that escapes one makes preview lie โ€” and preview is trusted because it is supposed to change nothing, so a lying preview is worse than no preview at all.

if (!$preview) {
    $conn->prepare("UPDATE โ€ฆ")->execute($args);
}

Log the entry outside the guard. What somebody was shown before they pressed the button is worth being able to check afterwards, so a preview records everything it would have done.

Then add an assertion to tests/directory-sync.sh โ€” the existing one is "a preview changes no user at all", comparing three counts before and after.

5. Adding a safety rule

The four existing ones (see the developer guide ยง4) are independent on purpose: each covers a failure the others cannot see. Before adding a fifth, work out which failure is invisible to all four, because that is the only kind worth adding.

The most recent one came from exactly that question. The brake watches for a sudden drop, so a renamed carve-out โ€” which causes people to be added โ€” was invisible to every existing rule.

Two rules for anything in this area:

  • Refuse rather than half-do, when the wrong outcome is destructive. A missing include branch fails the whole run, because importing the branches that do still exist is precisely how everybody in the missing one ends up marked as left.
  • Pair the check with a positive control in the tests. Without one, "no warning was produced" is indistinguishable from "the check never ran". tests/directory-sync.sh proves the brake stops a run and that the same run proceeds with the brake off.

6. Things that look like extension points and are not

Tempting Why not
Expressing a carve-out as an LDAP filter "Not under this subtree" is about position in the tree; a filter tests attributes. It cannot be written
Comparing DNs without the trailing comma OU=Sales then matches OU=WholesaleSales. Use dsyncDnIsUnder()
Storing '' for a person with no email users.email is UNIQUE. MySQL tolerates many NULLs and exactly one '', so the second mailbox-less person is rejected as a duplicate
Matching people by email across companies Two companies can legitimately share an address. dsyncFindExisting() scopes it, and that scoping is a tenancy boundary, not a nicety
A second preview implementation Preview runs the same code path with writes suppressed. Two implementations drift, and the one that drifts is the one nobody runs for real
Enforcing "no manager cycles" in the database A self-referencing FK cannot express it. userManagerIsSafe() walks the chain, bounded

See also

FreeITSM โ€” an open-source IT Service Management platform ยท github.com/edmozley/freeitsm ยท MIT licence

โš ๏ธ **GitHub.com Fallback** โš ๏ธ