diff --git a/docs/hook-event-brief.md b/docs/hook-event-brief.md new file mode 100644 index 0000000000..8021efa61c --- /dev/null +++ b/docs/hook-event-brief.md @@ -0,0 +1,149 @@ +# Hook and Event subsystem: scan and propose + +**Scan and propose only. Do not write implementation code. Do not open a PR.** + +Read `docs/refactor-facts.md` first; it outranks this file. + +--- + +## Why this is different from the phase work + +The hook system is the plugin ABI. Every bundled plugin and every third-party +plugin registers through `EventManager::register()` and is dispatched through +`HookManager::processEvent()`. ADR 0013 promises plugin authors that code +written against 1.6 keeps working for all of 1.6. + +If this goes out wrong, every plugin on every server breaks at once, and the +symptom is silent: a hook that fails to register logs a line and returns +normally. Nobody finds out until a feature quietly stops happening. + +So the bar here is higher than "the tests pass." The bar is: **no plugin, core +or third-party, needs editing.** Anything that would require a plugin author to +change a line is not a refactor, it is a breaking change, and it needs to be +identified as one and decided on separately. + +--- + +## Goals, in priority order + +1. **Correctness.** Several things below are wrong today, not merely ugly. +2. **The contract stays.** `register()`, `processEvent()`, `notify()`, + `hasListeners()` keep working for existing callers. +3. **Efficiency, measured not assumed.** I want this lean in memory and code, + but do not optimize on instinct. Measure what it costs now (how many hook + and event objects are constructed per request, how much of that a typical + request ever dispatches) and report it before proposing anything that trades + clarity for speed. If the current cost is already small, say so and drop + this goal. + +--- + +## What I have observed + +These are observations, not a specification of the fix. Verify each one +independently before relying on it; if any is wrong, say so. + +`packages/web/lib/fog/eventmanager.class.php`, +`hookmanager.class.php`, `event.class.php`, `hook.class.php`. + +**1. `load()` distinguishes the two managers with two sequential `instanceof` +checks** (`eventmanager.class.php:218-225`). `HookManager extends EventManager`, +so a HookManager satisfies both branches. The correct file extension and +directory are reached only because the second assignment overwrites the first. +Reordering the two blocks changes behaviour. + +**2. Whether a class is active is decided by a regular expression run over the +file's source text**, line by line (`eventmanager.class.php:248-252`), matching +`$active\s?=\s?true;`. The class is never loaded and the property is never read +at that point. `\s?` is zero-or-one, so spacing variations do not match, and the +scan does not distinguish code from comments. + +**3. `register()` branches on `self::shortName($this)`** with a case per +subclass and a `default` arm that throws +(`eventmanager.class.php:71-103`). The parent enumerates its children by name. +The throw is caught by the surrounding handler and written to the log +(`:105-122`), so a failed registration returns normally and the caller cannot +tell. The comment above the switch records that this shape already caused a +total registration failure once during the namespace migration. + +**4. `HookManager` inherits `notify()`**, which iterates listeners as objects +(`$element->active`, `$element->onEvent()`) while `HookManager` stores them as +`[$object, 'method']` arrays. Determine whether anything reaches it. + +**5. `notify()` treats "no listeners registered" as an exception** +(`eventmanager.class.php:175-177`) that is caught, logged, and returns false. +`processEvent()` handles the same condition with a bare `return` +(`hookmanager.class.php:89-91`). Same situation, two behaviours, one of which +writes a log line on what is probably the common case. + +**6. `processEvent()` force-activates any listener whose file path contains the +substring `plugins`** (`hookmanager.class.php:103-105`), regardless of its +`active` property. Note before concluding anything: `FOGBase::fileitems()` +builds `$regex_pgrep` from `self::$pluginsinstalled` +(`fogbase.class.php:3648-3654`), so plugin files appear to be filtered to +installed plugins before reaching this point. Establish whether that makes the +force-activation redundant, or whether there is a path where it is the only +thing making a listener run. + +**7. `Hook extends Event`.** Assess whether a hook is genuinely a kind of event, +or whether the two are peers with different dispatch contracts. `processEvent()` +merges arguments and calls a method that can mutate them through references +embedded in the payload; `notify()` calls `onEvent()` and discards the result. + +--- + +## Required before you propose anything + +**Inventory the contract.** Every call site of `register()`, `processEvent()`, +`notify()` and `hasListeners()` in `fogproject` AND in `fog-plugins`. What +shapes of listener are actually passed, what payloads are actually mutated. The +contract is what the callers do, not what the docblocks say. + +**Characterize current behaviour before changing it.** Write tests that pin what +happens today, including the parts that are wrong. Then a behaviour change shows +up as a test that has to be edited, deliberately, rather than as a silent +difference. + +**For each defect, determine whether anything currently depends on it.** This is +the part I care most about. Two concrete examples: + +- If activation stops being a source-text regex and starts reading the property, + a hook whose property is `false` but whose file contains the literal string + somewhere (a comment, an example) is active today and would go inactive. + Search `fog-plugins` and core for any file where the regex result and the real + property value disagree. +- If the path-substring force-activation is removed, any plugin hook whose + `$active` is not `true` becomes inactive. Search `fog-plugins` for hooks that + do not set it true. + +Report what you find. Do not assume the answer is zero. + +**Say what a third-party plugin cannot see.** `fog-plugins` is the plugins I +ship. Servers run plugins I have never read. Any change whose safety argument +depends on inspecting plugin source is weaker than one that does not, and I need +that distinction stated explicitly for each proposal. + +--- + +## What to bring back + +A plan in the same form as the phase plans: `VERIFIED` / `INFERRED` / `UNKNOWN` +throughout, commit-by-commit with the tree green after every commit, a shell +command per claim, alternatives considered and rejected, and the blast radius of +each step for plugin authors. + +Plus, and separately from the plan: + +1. **The defect list as you found it**, including anything above that turns out + to be wrong or that you found and I did not. +2. **The decisions that are mine, not yours.** Where fixing a defect would + change behaviour something might rely on, do not pick. Present the options + and what each costs. Bugs that are load-bearing for somebody are not + automatically bugs to fix. +3. **The measurement**, per goal 3 above. +4. **Whether this warrants an ADR.** It changes an ABI that ADR 0013 made + promises about, so my instinct is yes, but argue it. + +Ask me questions before planning if the code does not answer something. + +**End with: which claim in this plan, if false, would hurt most?** diff --git a/docs/refactor-brief.md b/docs/refactor-brief.md new file mode 100644 index 0000000000..b5016679be --- /dev/null +++ b/docs/refactor-brief.md @@ -0,0 +1,248 @@ +# FOG working-1.6: Modernization Planning Brief + +You are producing **plans, not implementations**. Read this whole file before +doing anything. + +There are four phases. They are ordered by dependency, not by importance. +**Plan one phase at a time and stop for my approval at each gate.** Do not +plan Phase 2 until I have approved Phase 1. If you find yourself planning +across phase boundaries, stop and tell me why the boundary is wrong. + +--- + +## Ground rules (apply to every phase) + +Before writing anything, read the actual code. Do not plan from assumption. +Every factual claim you make about how FOG currently works must cite +`file:line`, and you must have actually opened that file. + +Mark every statement as one of: + +- `VERIFIED` - I read this in the code, here is file:line +- `INFERRED` - this follows from what I read but I did not confirm it +- `UNKNOWN` - I could not determine this + +If the INFERRED and UNKNOWN sections are short, you have not looked hard +enough. I would rather have twenty honest unknowns than a confident plan +that is wrong in three places. + +Each phase plan must contain: + +1. A **commit-by-commit sequence** where the application boots and the test + suite passes after every single commit. No step may leave the tree broken + pending a later step. +2. For each commit, a **shell command** (grep, test, curl) I can run to verify + it did what it claims. If a claim cannot be reduced to a command, say so + explicitly rather than inventing one. +3. **Alternatives considered and rejected**, with reasons. Match the style of + `docs/adr/0009-plugins-become-installable-artifacts.md`. +4. Every **irreversible step** and every **data migration** called out + separately, each with its rollback story. +5. What breaks for a **third-party plugin author**, and the deprecation window. + +Ask me questions before planning if the codebase does not answer something. +Do not guess and proceed. + +At the end of each phase plan, answer this directly: **which claim in this +plan, if false, would hurt most?** I will verify that one by hand before +approving. + +--- + +## Why this order + +- Composer must exist before the auth work, because the entire argument for + provider-based auth is not hand-rolling JWT validation. +- The backslash-prefix pass must happen before anything else is written, or + new code needs the same treatment later. +- Site and RBAC must settle before auth, because auth maps IdP claims to + roles and those roles need to be stable first. +- Namespacing goes last because it touches every file. Anything in flight + during it becomes a merge conflict factory. The bridge autoloader from + Phase 0 is what allows it to be last without blocking the rest. + +--- + +## PHASE 0: Foundation + +Read `packages/web/commons/init.php` (the autoloader, `autoload()` around +line 256) and `packages/web/lib/fog/fogbase.class.php` `getClass()` around +line 531. + +Plan three independent changes, as separate PRs, in this order: + +**0.1 - Backslash-prefix every unqualified global class reference** across +`packages/web`. `\Exception`, `\DateTime`, `\DateTimeZone`, `\PharData`, +`\ReflectionClass`, `\PDO`, `\RecursiveIteratorIterator`, and anything else +you find. In the global namespace this is a no-op, which is the point: it +must be reviewable as a purely mechanical diff with zero behaviour change. +Give me the exact count per class before proposing the change. + +This exists because inside a namespace, `catch (Exception $e)` silently +becomes `catch (FOG\Exception $e)`, which fails only on the error path, only +at runtime. Defusing it now, in a boring standalone commit, is what turns +the scary part of Phase 3 into a non-event. + +**0.2 - Bridge autoloader.** Make the existing autoloader resolve a +namespaced request by falling back to the short name and `class_alias`ing +the result, so `FOG\User` works before any file is namespaced. Handle the +case where the included file does not declare the expected class name. +Note that `autoload()` builds a lowercased basename map, so a namespaced +lookup currently misses entirely and silently. + +**0.3 - composer.json** with PSR-4 `FOG\` mapped to a new, empty +`packages/web/src/`, registered alongside the existing autoloader rather +than replacing it. `spl_autoload_register` is a chain; both can coexist. + +For 0.3 the real question is operational, not code. FOG installs on +air-gapped and firewalled networks and currently vendors nothing. Lay out +committing `vendor/` to the repo versus running `composer install` at +install time: failure modes, repo size, update story, what happens when +Packagist is unreachable mid-install, and how `bin/fetch-plugins.sh` +interacts with either choice. Recommend one and argue for it. + +Also answer: does anything compare class names as strings, e.g. +`get_class($x) == 'Host'` or `::class` against a literal? Those do not +survive aliasing. Find them all and list them. + +**GATE. Stop here for approval.** + +--- + +## PHASE 1: Site into core, alongside RBAC + +Read `docs/adr/0006`, `docs/adr/0009`, +`packages/web/lib/fog/authorization.class.php`, and the `site` plugin at the +external plugin root. + +Plan moving site's scope enforcement into core alongside RBAC. + +**Design constraint I have already decided: no enable flag.** Sites are +unconditional. Zero sites defined means one implicit site containing +everything, which is today's behaviour and today's out-of-box experience. +Defining a site is what makes the boundary start to mean something. A +boundary that exists only when a boolean is true is still a boundary that +can be absent. Argue against this if you think I am wrong, but argue +*before* planning, not during. + +Answer these from the code specifically: + +- `objectInScope()`, `requirePageObjectScope()` and the Mass variant + currently depend on the site plugin's hook listeners. What exactly happens + to each when no listener is registered? Quote the code path. +- Do `registry()` or `purgePermissions($nodePrefix)` derive permission node + names from class names or plugin names? If yes, moving site into core + changes those strings, and that is a data migration that could silently + revoke access on live servers. **This is the thing I am most worried + about. Treat it as the headline question of this phase.** +- The site plugin owns tables with real customer data. Plan the migration + for three cases: servers with the plugin installed, servers that never had + it, and a server where the migration fails halfway through. + +The plugin must keep existing on disk for at least one release. Plan what it +degrades into: a shim, a no-op, or a hard error telling the admin to remove +it. + +Related, and cheap: there is no `'site'` string anywhere in +`plugin.class.php` or `pluginmanagement.page.php`, so nothing stops a future +commit adding `fog_max` to site's manifest and silently deleting the +security boundary via auto-deactivation. Add a CI test asserting site +declares no `fog_max`, regardless of what else this phase does. + +**GATE. Stop here for approval.** + +--- + +## PHASE 2: Authorization providers + +Read `user.class.php` `login()`, particularly the `USER_LOGGING_IN` hook +around line 127 and the `isExternal` guard at 144-153. Read +`authorization.class.php`. Read the `ldap` plugin's hooks. + +`USER_LOGGING_IN` already works for credential-presenting sources like LDAP +and AD. It cannot express OIDC, because there is no password: the browser +leaves for the IdP and returns with an authorization code. Plan the second +seam. + +**Scope: OIDC only, not SAML.** OIDC covers Google, Microsoft Entra, Okta, +Authentik, Keycloak, Auth0, and GitHub is close enough to share most of the +code. SAML means a large dependency tree for a project that currently +vendors nothing. Justify or challenge that scoping. + +Required: + +- **Split `login()`** into `authenticate()` and `establishSession(User)`. + Right now the only way to create a session is to know a password. Show me + the exact split against the real method body, including remember-me + cookies and `UserAuth` token handling. This refactor is worth doing on its + own merits regardless of SSO. +- **A route reachable without a session** for the OIDC callback. Read the + router and `resolvePagePermission()` and tell me whether the + unregistered-page fallback is safe to rely on for an auth endpoint, or + whether it is a compatibility stance that will be tightened later and + break silently. +- **Provisioning policy: default deny.** A successful IdP login for a user + with no existing FOG account fails with a clear message. JIT provisioning + is a later switch, not the default. Claims map to RBAC roles, never to the + legacy `type` field that `USER_TYPE_HOOK` rewrites. +- **Break-glass.** Interactive SSO can never be the only way in. The FOG + client, storage nodes and API consumers do not do browser redirects, and + an IdP outage must not lock every admin out of the imaging server during + the same incident that made them need it. Show how `effectiveAdminExists()` + style reasoning applies to auth sources. +- **Library choice** for JWT and JWKS. Hand-rolling this is how `alg: none` + bugs happen. Recommend a specific package, justify it on dependency + weight, and confirm it works under whatever vendoring decision came out of + Phase 0. + +This will be the first genuinely third-party-shaped plugin: it needs a +route, a login page contribution, a settings page, and a session. Where does +ADR 0009's plugin system fail to support it? That gap is more interesting to +me than the OIDC code itself. + +**GATE. Stop here for approval.** + +--- + +## PHASE 3: Namespacing + +Read `commons/init.php` `autoload()`, `fogbase.class.php` `getClass()`, and +the class hierarchy under `packages/web/lib/fog`. + +Phase 0 is merged by now: globals are backslash-prefixed, the bridge +autoloader resolves namespaced names via `class_alias`, and composer PSR-4 +maps `FOG\` to `packages/web/src/`. + +Plan migrating 227 classes with `class_alias` in **both directions**, so old +and new names work simultaneously and the app works after every commit. + +- **Sequence: leaves first**, `FOGBase` / `FOGController` / + `FOGManagerController` last, since everything descends from those. Derive + the actual dependency graph from the code. Do not guess at what is a leaf. +- **`getClass()` has 491 call sites and 136 distinct names.** Convert it to + a short-name-to-FQCN map so those call sites never need editing. This is + the whole reason the migration is tractable: one chokepoint controls the + overwhelming majority of class references. +- While in there, remove the `global $sub` that steers `Storage` to + `StorageNode` or `StorageGroup`. Tell me what that breaks. It is a + landmine independent of namespacing. +- **Namespace layout:** propose one and justify it. Flat `FOG\`, or + `FOG\Model`, `FOG\Manager`, `FOG\Page`, `FOG\Hook`? I lean toward + mirroring the existing directory structure but I want your argument, not + my assumption confirmed. +- **Case sensitivity:** the current map lowercases everything, which hides + any inconsistent casing across those 491 string references. PSR-4 is + case-sensitive and will not hide it. Find the inconsistencies before they + become runtime failures. +- **Plugins:** aliases stay through all of 1.6. Tell me what a plugin author + should write today, and draft the 1.7 deprecation note. + +Watch for permission nodes, API routes, or database columns derived from +class names. An alias does not change what is already stored in the +database. + +--- + +## Reminder + +Do not plan more than one phase per response. Stop at each gate. diff --git a/docs/route-listem-brief.md b/docs/route-listem-brief.md new file mode 100644 index 0000000000..3cbfb303dd --- /dev/null +++ b/docs/route-listem-brief.md @@ -0,0 +1,131 @@ +# `Route::listem()` and the route layer: scan and propose + +**Scan and propose only. Do not write implementation code. Do not open a PR.** + +Read `docs/refactor-facts.md` first; it outranks this file. + +--- + +## What this is + +`packages/web/lib/router/route.class.php` is 6,470 lines, the largest file in +the tree. `listem()` is 1,087 lines in one function. `deletemass()` is 326, +`getter()` 309, `edit()` 259, `joining()` 168. + +Nobody has reviewed `listem()`, because nobody can hold 1,087 lines in their +head at once. Whatever is wrong in there is wrong quietly and has been for +years. + +--- + +## Why this one is dangerous + +`listem()` is the read path for the whole API — all 55 entries in +`Route::$validClasses` flow through it — **and** it is where object scope gets +enforced. Everything Phase 1 built in `SiteScope` lands here. + +So this is not a readability exercise. A decomposition that moves a filter into +a branch which does not always execute does not throw, does not fail a type +check, and does not look wrong in review. It returns more rows. On a +site-scoped server that means one customer's users seeing another site's hosts, +and the only symptom is a list that looks fine. + +**Therefore, before proposing any decomposition:** map every line in `listem()` +that participates in access control — scope filtering, permission checks, field +redaction, anything that removes or masks a row or column. Report that map as +its own artifact. If you cannot say with confidence which lines those are, say +so and stop; a decomposition proposal built on an incomplete map is worse than +no proposal. + +--- + +## Hard constraints + +**The API response shape does not change.** Users have scripts, integrations and +bookmarks against these routes. `Route::$validClasses` strings appear in +customers' code. ADR 0011 governs result wrappers. Anything that alters what a +consumer receives is a breaking change, not a refactor, and must be identified +as one and decided separately. + +**`dev-branch` is out of scope.** This work is `working-1.6` only. If you find a +defect that also exists on `dev-branch`, do not propose a port and do not shape +the fix to make one easier. Flag it in a separate list at the end with a +severity, and let me decide. The one exception is a security defect with a +working exploit path, which I want told about immediately and separately from +this plan. + +**No plugin should need editing.** Plugins mutate the route surface through +`API_VALID_CLASSES` and friends (`route.class.php:451-467`). Establish what that +hook surface actually permits and treat it as frozen. + +--- + +## What I want established, in this order + +**1. The access-control map.** Above. This gates everything else. + +**2. What `listem()` actually does.** Not a summary — an enumeration of its +responsibilities. My expectation is that it is several functions wearing a +trenchcoat: request parsing, permission resolution, scope filtering, query +building, joining, pagination, sorting, search, field selection, result +shaping. Confirm or correct that, and say which responsibilities are entangled +such that they cannot be separated without changing behaviour. + +**3. Whether the existing tests are a net or a comfort.** +`route-filter-fields.test.php`, `route-result-wrappers.test.php`, +`routed-query-string.test.php` and `openapi-route-coverage.test.php` exist. Do +they actually pin the behaviour a refactor could break — scope filtering in +particular — or do they cover the edges and leave the middle open? Answer with +what they assert, not with a count. If they are not a sufficient net, then +building one is the first commit and the decomposition is the second. + +**4. The bugs.** A function this size has them. Report them whether or not they +relate to the refactor: early returns that skip later filtering, conditions that +can never be true, `$_REQUEST` reads that bypass validation elsewhere in the +function, silent failure paths, duplicated logic that has drifted between +copies. + +**5. Efficiency, measured.** `listem()` is the hot path for every list page and +every API read. Measure what it costs now — query count, peak memory, wall time +on a realistic host count — before proposing anything that trades clarity for +speed. If the current cost is fine, say so and drop this. I would rather have +correct and readable than fast, but I want to know the number before choosing. + +**6. `openapi.class.php` coupling.** It is 1,753 lines and derives the published +spec from this layer. Establish what in the route surface it reads, so a change +here cannot silently desynchronise the documented API from the real one. + +--- + +## What to bring back + +A plan in the established form: `VERIFIED` / `INFERRED` / `UNKNOWN` throughout, +commit-by-commit with the tree green after every commit, a shell command per +claim, alternatives considered and rejected, blast radius per step. + +Separately from the plan: + +1. **The access-control map**, as its own document. This outlives the refactor. +2. **The defect list**, with anything that would need a `dev-branch` decision + split into its own section, severity-rated, no port proposed. +3. **The decisions that are mine.** Where a fix changes observable behaviour, + present the options and their costs rather than picking. Some of what looks + wrong in a nine-year-old API is load-bearing for somebody. +4. **The measurement.** +5. **Whether this warrants an ADR.** My instinct is that a decomposition alone + does not, but any change to what the route layer guarantees about scope does. + Argue it. + +Ask me questions before planning if the code does not answer something. + +**End with: which claim in this plan, if false, would hurt most?** + +--- + +## One thing I am not asking for + +Do not propose splitting `route.class.php` into multiple files as a first step. +Moving 6,470 lines between files produces an enormous diff that hides behaviour +changes inside apparent relocations, and it is exactly the shape of change I +cannot review. If the end state wants several files, that is the last commit in +the sequence, not the first. diff --git a/packages/web/lib/fog/fogpage.class.php b/packages/web/lib/fog/fogpage.class.php index d4f554a7dc..9f219f73e9 100644 --- a/packages/web/lib/fog/fogpage.class.php +++ b/packages/web/lib/fog/fogpage.class.php @@ -304,19 +304,19 @@ public function __construct($name = '') parent::__construct(); self::$FOGCollapseBox = self::makeButton( '', - '', + '', 'btn btn-tool', 'data-lte-toggle="card-collapse"' ); self::$FOGExpandBox = self::makeButton( '', - '', + '', 'btn btn-tool', 'data-lte-toggle="card-maximize"' ); self::$FOGCloseBox = self::makeButton( '', - '', + '', 'btn btn-tool', 'data-lte-toggle="card-remove"' ); @@ -440,78 +440,78 @@ public static function buildMainMenuItems(&$main = '', &$hookMain = '') $menu = [ 'home' => [ self::$foglang['Dashboard'], - 'fa fa-dashboard' + 'fas fa-gauge-high' ], 'host' => [ self::$foglang['Hosts'], - 'fa fa-desktop' + 'fas fa-desktop' ], 'group' => [ self::$foglang['Groups'], - 'fa fa-sitemap' + 'fas fa-sitemap' ], 'image' => [ self::$foglang['Images'], - 'fa fa-hdd-o' + 'far fa-hard-drive' ], 'snapin' => [ self::$foglang['Snapins'], - 'fa fa-cube' + 'fas fa-cube' ], 'storagegroup' => [ self::$foglang['Storagegroups'], - 'fa fa-object-group' + 'far fa-object-group' ], 'storagenode' => [ self::$foglang['Storagenodes'], - 'fa fa-archive' + 'fas fa-server' ], 'printer' => [ self::$foglang['Printers'], - 'fa fa-print' + 'fas fa-print' ], 'module' => [ _('Modules'), - 'fa fa-cogs' + 'fas fa-sliders' ], 'task' => [ self::$foglang['Tasks'], - 'fa fa-tasks' + 'fas fa-bars-progress' ], 'user' => [ self::$foglang['Users'], - 'fa fa-users' + 'fas fa-users' ], 'usergroup' => [ _('User Groups'), - 'fa fa-address-book' + 'fas fa-people-group' ], 'role' => [ _('Roles'), - 'fa fa-key' + 'fas fa-user-shield' ], // Beside roles rather than beside hosts: a site says which // objects a user may reach, which is the same kind of answer a // role gives about which actions. 'site' => [ _('Sites'), - 'fa fa-map-marker' + 'fas fa-building' ], 'ipxe' => [ _('iPXE Menu'), - 'fa fa-bars' + 'fas fa-list-ol' ], 'about' => [ self::$foglang['FOG Configuration'], - 'fa fa-wrench' + 'fas fa-wrench' ], 'apidocs' => [ _('API Documentation'), - 'fa fa-code' + 'fas fa-code' ], 'report' => [ self::$foglang['Reports'], - 'fa fa-file-text' + 'fas fa-file-lines' ], // Beside Reports, because that is what people will look for it // under -- but a node of its own, not a report. The `report` @@ -521,7 +521,7 @@ public static function buildMainMenuItems(&$main = '', &$hookMain = '') // See docs/adr/0023. 'activity' => [ _('Activity'), - 'fa fa-history' + 'fas fa-clock-rotate-left' ], // Next to Activity, and a different page on purpose. Activity is // the operational narrative; this is the record of who was @@ -530,15 +530,15 @@ public static function buildMainMenuItems(&$main = '', &$hookMain = '') // from anyone not granted it. See docs/adr/0021. 'audit' => [ _('Audit Log'), - 'fa fa-shield' + 'fas fa-clipboard-list' ], 'service' => [ self::$foglang['ClientSettings'], - 'fa fa-cogs' + 'fas fa-gears' ], 'client' => [ _('FOG Client'), - 'fa fa-cloud-download' + 'fas fa-cloud-arrow-down' ] ]; $pluginSysOn = (bool)self::getSetting('FOG_PLUGINSYS_ENABLED'); @@ -549,7 +549,7 @@ public static function buildMainMenuItems(&$main = '', &$hookMain = '') 'plugin', [ self::$foglang['Plugins'], - 'fa fa-puzzle-piece' + 'fas fa-puzzle-piece' ] ); } @@ -711,7 +711,7 @@ private static function _menuGroups() return [ 'useradmin' => [ 'title' => _('User Administration'), - 'icon' => 'fa fa-shield', + 'icon' => 'fas fa-shield', 'children' => ['user', 'usergroup', 'role'], ], ]; @@ -745,7 +745,7 @@ function ($child) use ($menu) { echo ''; echo ''; echo '

' . $group['title']; - echo ''; + echo ''; echo '

'; echo ''; echo ' @@ -169,7 +169,7 @@
  • - +
    • @@ -180,10 +180,10 @@
      • -

      • -

      • -

      • -

      • +

      • +

      • +

      • +

    diff --git a/packages/web/management/webfonts/LICENSE-fontawesome.txt b/packages/web/management/webfonts/LICENSE-fontawesome.txt new file mode 100644 index 0000000000..69403b25cd --- /dev/null +++ b/packages/web/management/webfonts/LICENSE-fontawesome.txt @@ -0,0 +1,165 @@ +Fonticons, Inc. (https://fontawesome.com) + +-------------------------------------------------------------------------------- + +Font Awesome Free License + +Font Awesome Free is free, open source, and GPL friendly. You can use it for +commercial projects, open source projects, or really almost whatever you want. +Full Font Awesome Free license: https://fontawesome.com/license/free. + +-------------------------------------------------------------------------------- + +# Icons: CC BY 4.0 License (https://creativecommons.org/licenses/by/4.0/) + +The Font Awesome Free download is licensed under a Creative Commons +Attribution 4.0 International License and applies to all icons packaged +as SVG and JS file types. + +-------------------------------------------------------------------------------- + +# Fonts: SIL OFL 1.1 License + +In the Font Awesome Free download, the SIL OFL license applies to all icons +packaged as web and desktop font files. + +Copyright (c) 2026 Fonticons, Inc. (https://fontawesome.com) +with Reserved Font Name: "Font Awesome". + +This Font Software is licensed under the SIL Open Font License, Version 1.1. +This license is copied below, and is also available with a FAQ at: +http://scripts.sil.org/OFL + +SIL OPEN FONT LICENSE +Version 1.1 - 26 February 2007 + +PREAMBLE +The goals of the Open Font License (OFL) are to stimulate worldwide +development of collaborative font projects, to support the font creation +efforts of academic and linguistic communities, and to provide a free and +open framework in which fonts may be shared and improved in partnership +with others. + +The OFL allows the licensed fonts to be used, studied, modified and +redistributed freely as long as they are not sold by themselves. The +fonts, including any derivative works, can be bundled, embedded, +redistributed and/or sold with any software provided that any reserved +names are not used by derivative works. The fonts and derivatives, +however, cannot be released under any other type of license. The +requirement for fonts to remain under this license does not apply +to any document created using the fonts or their derivatives. + +DEFINITIONS +"Font Software" refers to the set of files released by the Copyright +Holder(s) under this license and clearly marked as such. This may +include source files, build scripts and documentation. + +"Reserved Font Name" refers to any names specified as such after the +copyright statement(s). + +"Original Version" refers to the collection of Font Software components as +distributed by the Copyright Holder(s). + +"Modified Version" refers to any derivative made by adding to, deleting, +or substituting — in part or in whole — any of the components of the +Original Version, by changing formats or by porting the Font Software to a +new environment. + +"Author" refers to any designer, engineer, programmer, technical +writer or other person who contributed to the Font Software. + +PERMISSION & CONDITIONS +Permission is hereby granted, free of charge, to any person obtaining +a copy of the Font Software, to use, study, copy, merge, embed, modify, +redistribute, and sell modified and unmodified copies of the Font +Software, subject to the following conditions: + +1) Neither the Font Software nor any of its individual components, +in Original or Modified Versions, may be sold by itself. + +2) Original or Modified Versions of the Font Software may be bundled, +redistributed and/or sold with any software, provided that each copy +contains the above copyright notice and this license. These can be +included either as stand-alone text files, human-readable headers or +in the appropriate machine-readable metadata fields within text or +binary files as long as those fields can be easily viewed by the user. + +3) No Modified Version of the Font Software may use the Reserved Font +Name(s) unless explicit written permission is granted by the corresponding +Copyright Holder. This restriction only applies to the primary font name as +presented to the users. + +4) The name(s) of the Copyright Holder(s) or the Author(s) of the Font +Software shall not be used to promote, endorse or advertise any +Modified Version, except to acknowledge the contribution(s) of the +Copyright Holder(s) and the Author(s) or with their explicit written +permission. + +5) The Font Software, modified or unmodified, in part or in whole, +must be distributed entirely under this license, and must not be +distributed under any other license. The requirement for fonts to +remain under this license does not apply to any document created +using the Font Software. + +TERMINATION +This license becomes null and void if any of the above conditions are +not met. + +DISCLAIMER +THE FONT SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO ANY WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT +OF COPYRIGHT, PATENT, TRADEMARK, OR OTHER RIGHT. IN NO EVENT SHALL THE +COPYRIGHT HOLDER BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +INCLUDING ANY GENERAL, SPECIAL, INDIRECT, INCIDENTAL, OR CONSEQUENTIAL +DAMAGES, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF THE USE OR INABILITY TO USE THE FONT SOFTWARE OR FROM +OTHER DEALINGS IN THE FONT SOFTWARE. + +-------------------------------------------------------------------------------- + +# Code: MIT License (https://opensource.org/licenses/MIT) + +In the Font Awesome Free download, the MIT license applies to all non-font and +non-icon files. + +Copyright 2026 Fonticons, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in the +Software without restriction, including without limitation the rights to use, copy, +modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, +and to permit persons to whom the Software is furnished to do so, subject to the +following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +-------------------------------------------------------------------------------- + +# Attribution + +Attribution is required by MIT, SIL OFL, and CC BY licenses. Downloaded Font +Awesome Free files already contain embedded comments with sufficient +attribution, so you shouldn't need to do anything additional when using these +files normally. + +We've kept attribution comments terse, so we ask that you do not actively work +to remove them from files, especially code. They're a great way for folks to +learn about Font Awesome. + +-------------------------------------------------------------------------------- + +# Brand Icons + +All brand icons are trademarks of their respective owners. The use of these +trademarks does not indicate endorsement of the trademark holder by Font +Awesome, nor vice versa. **Please do not use brand logos for any purpose except +to represent the company, product, or service to which they refer.** diff --git a/packages/web/management/webfonts/fa-brands-400.woff2 b/packages/web/management/webfonts/fa-brands-400.woff2 new file mode 100644 index 0000000000..e5b3ce0812 Binary files /dev/null and b/packages/web/management/webfonts/fa-brands-400.woff2 differ diff --git a/packages/web/management/webfonts/fa-regular-400.woff2 b/packages/web/management/webfonts/fa-regular-400.woff2 new file mode 100644 index 0000000000..d470bd1758 Binary files /dev/null and b/packages/web/management/webfonts/fa-regular-400.woff2 differ diff --git a/packages/web/management/webfonts/fa-solid-900.woff2 b/packages/web/management/webfonts/fa-solid-900.woff2 new file mode 100644 index 0000000000..3ffbb19eb1 Binary files /dev/null and b/packages/web/management/webfonts/fa-solid-900.woff2 differ diff --git a/packages/web/management/webfonts/fa-v4compatibility.woff2 b/packages/web/management/webfonts/fa-v4compatibility.woff2 new file mode 100644 index 0000000000..971a2b3273 Binary files /dev/null and b/packages/web/management/webfonts/fa-v4compatibility.woff2 differ diff --git a/tests/fontawesome7-icon-names.test.php b/tests/fontawesome7-icon-names.test.php new file mode 100644 index 0000000000..ea5a15eb66 --- /dev/null +++ b/tests/fontawesome7-icon-names.test.php @@ -0,0 +1,231 @@ +check('the Font Awesome stylesheet is vendored', is_file($css)); + +$cssSrc = is_file($css) ? file_get_contents($css) : ''; +$t->check( + 'it is Font Awesome 7', + (bool)preg_match('/Font Awesome Free 7\./', $cssSrc) +); +// Shims are the thing whose absence every other check here depends on. +$t->check( + 'no v4 shim stylesheet is vendored', + [] === glob($web . '/management/css/*v4-shims*') +); +$t->check( + 'and nothing loads one', + false === strpos( + file_get_contents($web . '/lib/fog/page.class.php'), + 'v4-shims' + ) +); +// The FA4 webfont set is gone; its replacements are present. +$t->check( + 'the FA4 webfonts are no longer shipped', + [] === glob($web . '/management/fonts/fontawesome-webfont.*') + && !is_file($web . '/management/fonts/FontAwesome.otf') +); +foreach (['fa-solid-900', 'fa-regular-400', 'fa-brands-400'] as $font) { + $t->check( + sprintf('webfont %s.woff2 is shipped', $font), + is_file($web . '/management/webfonts/' . $font . '.woff2') + ); +} +// Every font the stylesheet asks for must exist, or those glyphs are blank. +preg_match_all('#url\(\.\./webfonts/([a-z0-9.-]+)\)#i', $cssSrc, $m); +$missing = []; +foreach (array_unique($m[1]) as $file) { + if (!is_file($web . '/management/webfonts/' . $file)) { + $missing[] = $file; + } +} +$t->check( + sprintf( + 'every webfont the stylesheet references exists (%d referenced)%s', + count(array_unique($m[1])), + [] === $missing ? '' : ' -- MISSING: ' . implode(', ', $missing) + ), + [] === $missing +); + +// --------------------------------------------------------------------------- +// 2. The call sites. +// +// lib/plugins is excluded deliberately: it is gitignored staging for the +// separate fog-plugins repo, which carries its own copy of this check. Core +// cannot fix a name that lives there, and failing on it would make this file +// unfixable from inside this repo. +// --------------------------------------------------------------------------- +$files = []; +$it = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($web, \FilesystemIterator::SKIP_DOTS) +); +foreach ($it as $f) { + $path = $f->getPathname(); + if (false !== strpos($path, '/vendor/') + || false !== strpos($path, '/lib/plugins/') + ) { + continue; + } + // Vendored bundles carry their own icon names and we do not get to rename + // them -- pnotify.min.js names FA4 icons in its styling presets. Ours live + // in management/js/fog/; everything directly under management/js/ is a + // third-party bundle. Where a vendored preset would render blank, FOG + // overrides it at its own call site (see the PNotify styling block in + // fog.common.js, pinned below) rather than editing the bundle, because + // editing it means the next upgrade silently reverts the fix. + if (preg_match('#/management/js/[^/]+\.js$#', $path)) { + continue; + } + if (preg_match('/\.(php|js|css|scss)$/', $path)) { + $files[] = $path; + } +} +$t->check('source files were found to scan', count($files) > 100); + +$bare = []; +$used = []; +foreach ($files as $path) { + $src = file_get_contents($path); + if (preg_match_all('/\bfa fa-([a-z0-9-]+)/', $src, $mm)) { + foreach ($mm[1] as $name) { + $bare[$name][] = str_replace($root . '/', '', $path); + } + } + if (preg_match_all('/\bfa[bsrl] fa-([a-z0-9-]+)/', $src, $mm)) { + foreach ($mm[1] as $name) { + $used[$name] = true; + } + } +} +$t->check( + sprintf( + 'no call site uses the bare "fa fa-*" form%s', + [] === $bare ? '' : ' -- FOUND: ' . implode(', ', array_keys($bare)) + ), + [] === $bare +); + +// --------------------------------------------------------------------------- +// 3. Every emitted name resolves in the shipped stylesheet. +// +// This is the check that catches a hand-written icon nobody ran the migration +// over, and a pro-only name -- which is absent from the free build and renders +// exactly as blank as an FA4 one. +// --------------------------------------------------------------------------- +$modifiers = [ + 'fw', 'lg', 'sm', 'xs', 'spin', 'pulse', 'border', 'li', 'inverse', + 'stack', 'stack-1x', 'stack-2x', 'rotate-90', 'rotate-180', 'rotate-270', + 'flip-horizontal', 'flip-vertical', 'flip-both', 'beat', 'fade', 'bounce', + 'shake', 'spin-pulse', 'spin-reverse', 'pull-left', 'pull-right', + '2x', '3x', '4x', '5x', '6x', '7x', '8x', '9x', '10x', 'solid', 'regular', + 'brands', 'classic', 'sharp' +]; +$unresolved = []; +foreach (array_keys($used) as $name) { + if (in_array($name, $modifiers, true)) { + continue; + } + // FA emits one rule per icon; an alias gets its own selector too, so a + // name that resolves at all appears here. + if (!preg_match('/\.fa-' . preg_quote($name, '/') . '[,:{ ]/', $cssSrc)) { + $unresolved[] = $name; + } +} +$t->check( + sprintf( + 'every emitted icon name resolves in the shipped CSS (%d distinct)%s', + count($used), + [] === $unresolved ? '' : ' -- UNRESOLVED: ' . implode(', ', $unresolved) + ), + [] === $unresolved +); + +// --------------------------------------------------------------------------- +// 4. Vendored presets that would otherwise render blank. +// +// PNotify's own styling presets are FA4. FOG used its "bootstrap3" preset, +// which emits glyphicon classes -- dropped by Bootstrap 4 -- so from the BS3 +// to BS5 move until this migration every toast rendered its icon slot as an +// empty box. Swapping to PNotify's "fontawesome" preset alone would have +// traded glyphicons for FA4 names, which are equally blank here. +// --------------------------------------------------------------------------- +$common = file_get_contents($web . '/management/js/fog/fog.common.js'); +$t->check( + 'PNotify is not left on the glyphicon-emitting bootstrap3 preset', + false === strpos($common, 'options.styling = "bootstrap3"') +); +$t->check( + 'and its FA4 preset names are overridden with FA7 ones', + (bool)preg_match( + '/PNotify\.styling\.fontawesome,\s*\{/', + $common + ) + && (bool)preg_match("/error_icon: 'fas fa-triangle-exclamation'/", $common) + && (bool)preg_match("/closer: 'fas fa-xmark'/", $common) +); + +$t->finish();