diff --git a/CHANGELOG.md b/CHANGELOG.md index 426ff3f7c..e4b9d3283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,37 @@ This file documents all notable changes made to ITFlow. +## [26.08.1] Maint Release + +### Upgrading to 26.08.1 + +Update the files from Settings > Update as normal. This release moves the database to 2.6.7 and the web updater completes it for you — the command line step that 26.08 required is not needed again. + +### Breaking Changes and Notes + +- Client access: agents with restricted client access now see records that have no client assigned. Previously this varied by page — unassigned tickets and projects were visible, unassigned expenses and credentials were not. It is now consistent everywhere. + +### Bug Fixes +- Setup: fixed the wizard closing itself after the first user, which left new 26.08 installs stuck in a redirect loop between `/setup` and `/login.php`. +- API: tightened client scoping on the expense read and record update endpoints. +- Income: revenue rows now respect restricted client access. +- Client PDF Export: fixed the export producing a CSV file, and each section is now gated on the module that owns it. +- AI: fixed model creation, per-use-case model selection, configurable temperature, and error reporting. +- Ticket: system-generated replies no longer record time worked that was never worked. +- Ticket: fixed an error when scheduling a ticket. +- Ticket: cancelling a schedule now cancels the calendar event on the recipient's calendar. +- Ticket: history no longer records a status change when the status did not change. +- Recurring Ticket: bulk priority changes no longer deny access to agents who are not administrators. +- Contact: deleting a contact now removes the linked portal user, and anonymizing now redacts the phone number. +- Calendar: fixed event deletion. + +### New Features & Updates +- Performance: queries now select only the columns they use instead of `SELECT *`, cutting memory use and query time across the app and especially in the crons. +- Performance: removed client joins that were only there for scoping — side nav badge counts are significantly faster. +- Client scoping: added a `clientScopeSql()` helper so list queries scope on the owning column instead of a joined `clients.client_id`. +- Contributing: documented the column-selection and client-scoping conventions. + + ## [26.08] ### Upgrading to 26.08 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3e076677d..7ebf94f02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,6 +74,10 @@ if (isset($_POST['edit_ticket_priority'])) { ### The `_model.php` pattern Files named `agent/post/*_model.php` hold shared field collection/sanitization logic used by both the create and edit blocks of a module (e.g. `asset_model.php` is included by both `add_asset` and `edit_asset`). If create and edit share more than a couple of fields, use this pattern rather than duplicating. Model files carry the same `FROM_POST_HANDLER` guard and are excluded from the dispatcher's auto-load. + +**`_model.php` is a reserved suffix.** The exclusion is a filename match, so a *handler* named `*_model.php` is silently never loaded — its form posts, nothing claims the request, and the user gets a blank page with no error anywhere. This is what happened to `admin/post/ai_model.php`, which is why the AI Models handler is now `admin/post/ai_models.php`. Name entity handlers around the suffix (`ai_models.php`, `users.php`, `api_keys.php`). + +A POST that reaches the end of `admin/post.php` or `agent/post.php` without a handler claiming it is logged to App Logs as a `Request` warning, which is the fastest way to spot this class of mistake. --- @@ -171,7 +175,25 @@ Everywhere else — anything under `agent/post/` — the call belongs in the blo ### 4. Client scoping is enforced, not assumed. -After loading a record, call `enforceClientAccess()` (optionally with the record's client ID) so technicians restricted to specific clients cannot touch other clients' data by editing an ID in the URL. Look at how `resolve_ticket` does it — including the "skip if the record has no client" case. +A user can be restricted to a subset of clients through `user_client_permissions`. Enforcing that has two halves, and a page usually needs both. + +**One record — `enforceClientAccess()`.** After loading a record, call it (optionally with the record's client ID) so technicians restricted to specific clients cannot touch other clients' data by editing an ID in the URL. Look at how `resolve_ticket` does it. + +**A list — `clientScopeSql()`.** Any query returning more than one row appends the fragment for that resource's own client column: + +```php +$sql = mysqli_query($mysqli, "SELECT expense_id, expense_date, expense_amount, expense_description + FROM expenses + WHERE expense_archived_at IS NULL + " . clientScopeSql('expense_client_id') . " + ORDER BY expense_date DESC"); +``` + +It returns `" AND ..."` or `""`, so it needs a `WHERE` to hang off — add `WHERE 1=1` if the query has no other condition. It is column-aware and takes an alias fine (`clientScopeSql('t.ticket_client_id')`). The API calls the same helper through the `apiClientScopeSql()` wrapper. + +Scope on the resource's **own** column, not on a joined `clients.client_id`. Joining `clients` just to scope makes the filter depend on the join: with a `LEFT JOIN`, a row whose client column is `0` produces `NULL`, and `NULL IN (...)` is neither true nor false, so the row silently vanishes. If the query joins `clients` for `client_name`, keep the join for that — but still scope on the owning column. + +**Records with no client (`0`) stay visible to restricted users.** `clientScopeSql()` emits `IN (0,...)` deliberately. Client restrictions partition *client* data, and a record belonging to no client is not any client's data to withhold. Do not hand-roll a variant that drops the `0` — the tree had accumulated several before this helper existed, disagreeing with each other, and reconciling them is what surfaced the inconsistency. ### 5. Escape on output. @@ -206,8 +228,29 @@ Per [SECURITY.md](SECURITY.md) — never in a public issue. --- ## Conventions - -**Database naming.** Every column is prefixed with the singular name of the entity it belongs to: `tickets.ticket_id`, `tickets.ticket_subject`, `clients.client_name`. This makes JOIN results unambiguous and is why queries can `SELECT *` across joins safely. New tables must follow it. + +**Only technician-entered time is time worked.** `ticket_replies.ticket_reply_time_worked` is billable labour and feeds ticket totals, the technician and client time reports, project totals, invoicing and the API. A reply the *system* writes — assignment, priority change, merge, close, invoice/quote created, schedule edited, task completed or reopened — is an audit trail, not work, and records `'00:00:00'`. Only a value the technician actually typed goes in that column. Task completion estimates are planning information and stay on the task; they are never converted into time worked. `agent/ticket.php` hides the clock badge on a reply whose time is exactly `00:00:00`, so a zero renders as no time rather than as "0m". + +**Database naming.** Every column is prefixed with the singular name of the entity it belongs to: `tickets.ticket_id`, `tickets.ticket_subject`, `clients.client_name`. This makes JOIN results unambiguous, so a `SELECT *` across joins is never *wrong*. New tables must follow it. + +**Select the columns you use, not `*`.** Unambiguous is not the same as cheap. `SELECT *` across three joined tables fetches every column of all three, including the `*_notes` and `*_details` TEXT columns, and throws away whatever the page never renders. A search result list that shows five fields was pulling sixty. List the columns instead: + +```php +$sql = mysqli_query($mysqli, "SELECT ticket_id, ticket_prefix, ticket_number, ticket_subject, client_name + FROM tickets + LEFT JOIN clients ON ticket_client_id = client_id + WHERE ticket_archived_at IS NULL + " . clientScopeSql('ticket_client_id') . ""); +``` + +Two things follow from that: + +- A query whose result only feeds `mysqli_num_rows()` needs no columns at all — write `SELECT 1`. Do not select a primary key "just in case": if the query joins two tables that both carry that column name, an unqualified `SELECT ticket_template_id` is an ambiguous-column error. +- Keep the join even when no column of the joined table survives into the `SELECT`, if the join is doing work — supplying a `WHERE` term, an `ORDER BY`, or the client column you scope on. Dropping a join is a separate decision from trimming the column list. + +The trade is real and worth stating: `SELECT *` picks up new columns for free, an explicit list does not. Add a column to a table and every query that needs it must be updated by hand, and the failure mode is a blank field or a PHP 8 undefined-key warning rather than an error. That is the price of not fetching data nobody reads, and the project has decided to pay it on anything that loops or touches a TEXT column. + +The exception is `api/v1/*/read.php`. Those endpoints hand the whole row to `read_output.php`, which serialises it straight into the JSON response — there the row *is* the output contract, so `SELECT *` is correct and trimming it would silently drop fields from every consumer. The prefix is the entity name, which is usually but not always the singular of the table name. Where a table is named for its container rather than its row, the prefix follows the row: `calendar_events` → `event_*`, `asset_interfaces` → `interface_*`, `invoice_items` / `quote_items` → `item_*`, `rack_units` → `unit_*`, `user_roles` → `role_*`, `product_stock` → `stock_*`. Pick the prefix your columns will read best as and use it for every column in the table. @@ -224,6 +267,8 @@ A single update run applies every pending migration in order, stopping at the fi **After acting, log and notify.** State changes call `logAudit($type, $action, $description, $client_id, $entity_id)` for the audit trail. User-facing events may also call `appNotify()`. Fire `triggerCustomAction()` where a site might reasonably want a hook. Then call `flashAlert($message, $type)` and `redirect()` (defaults to the referer) rather than setting session keys or `header()` manually. **Function names (post-rename).** Helpers were renamed for clarity in 2026; the old names **no longer exist** — code calling them fatals. If you're rebasing an old PR or following an old tutorial, translate: `sanitizeInput` → `escapeSql`, `nullable_htmlentities` → `escapeHtml`, `logAction` → `logAudit`, `flash_alert` → `flashAlert`, `customAction` → `triggerCustomAction`, `encryptLoginEntry`/`decryptLoginEntry` → `encryptCredentialEntry`/`decryptCredentialEntry`, `strtoAZaz09` → `toAlphanumeric`, `fetchUpdates` → `checkForUpdates`, `sanitize_url` → `escapeUrl`. + +One removed **variable** deserves its own warning: the old `$access_permission_query` global is gone, replaced by `clientScopeSql()` (security rule 4). Unlike a removed function, it does not fatal — an undefined variable interpolates as an empty string, so a rebased query keeps running with **no client scoping at all**. Grep for it before rebasing anything that touches a list query. **Helpers that fetch data return it raw.** If you add a `getXById()`-style helper, return the column value untouched and let callers escape it (security rule 1). Validating what the helper interpolates into its *own* query — table and column names, the id — is still the helper's job; that is query construction, not output escaping, and the two are not the same thing. diff --git a/admin/ai_models.php b/admin/ai_models.php index e30dc6db3..db8f96d81 100644 --- a/admin/ai_models.php +++ b/admin/ai_models.php @@ -6,7 +6,8 @@ $order = "ASC"; require_once "includes/inc_all_admin.php"; -$sql = mysqli_query($mysqli, "SELECT * FROM ai_models LEFT JOIN ai_providers ON ai_model_ai_provider_id = ai_provider_id ORDER BY $sort $order"); +$sql = mysqli_query($mysqli, "SELECT ai_model_id, ai_model_name, ai_model_prompt, ai_model_use_case, ai_provider_id, + ai_provider_name FROM ai_models LEFT JOIN ai_providers ON ai_model_ai_provider_id = ai_provider_id ORDER BY $sort $order"); $num_rows = mysqli_num_rows($sql); diff --git a/admin/ai_providers.php b/admin/ai_providers.php index 3bb6b5729..b9b728314 100644 --- a/admin/ai_providers.php +++ b/admin/ai_providers.php @@ -6,7 +6,7 @@ $order = "ASC"; require_once "includes/inc_all_admin.php"; -$sql = mysqli_query($mysqli, "SELECT * FROM ai_providers ORDER BY $sort $order"); +$sql = mysqli_query($mysqli, "SELECT ai_provider_api_key, ai_provider_api_url, ai_provider_id, ai_provider_name FROM ai_providers ORDER BY $sort $order"); $num_rows = mysqli_num_rows($sql); diff --git a/admin/api_keys.php b/admin/api_keys.php index 1d6173b81..c0ed6cae9 100644 --- a/admin/api_keys.php +++ b/admin/api_keys.php @@ -8,7 +8,7 @@ require_once "includes/inc_all_admin.php"; $sql = mysqli_query( $mysqli, - "SELECT SQL_CALC_FOUND_ROWS * FROM api_keys + "SELECT SQL_CALC_FOUND_ROWS api_key_created_at, api_key_expire, api_key_id, api_key_name, api_key_secret, user_name FROM api_keys LEFT JOIN users on api_key_user_id = user_id WHERE (api_key_name LIKE '%$q%') ORDER BY $sort $order LIMIT $record_from, $record_to" diff --git a/admin/app_logs.php b/admin/app_logs.php index cf95b9478..2a1f7dce6 100644 --- a/admin/app_logs.php +++ b/admin/app_logs.php @@ -28,7 +28,7 @@ if (isset($_GET['category']) & !empty($_GET['catergory'])) { $sql = mysqli_query( $mysqli, - "SELECT SQL_CALC_FOUND_ROWS * FROM app_logs + "SELECT SQL_CALC_FOUND_ROWS app_log_category, app_log_created_at, app_log_details, app_log_id, app_log_type FROM app_logs WHERE (app_log_type LIKE '%$q%' OR app_log_category LIKE '%$q%' OR app_log_details LIKE '%$q%') AND DATE(app_log_created_at) BETWEEN '$dtf' AND '$dtt' $log_type_query diff --git a/admin/audit_logs.php b/admin/audit_logs.php index fd91cd8f6..0ce0d9709 100644 --- a/admin/audit_logs.php +++ b/admin/audit_logs.php @@ -48,7 +48,8 @@ if (isset($_GET['action']) & !empty($_GET['action'])) { $sql = mysqli_query( $mysqli, - "SELECT SQL_CALC_FOUND_ROWS * FROM logs + "SELECT SQL_CALC_FOUND_ROWS client_id, client_name, log_action, log_created_at, log_description, log_entity_id, log_id, + log_ip, log_type, log_user_agent, user_id, user_name FROM logs LEFT JOIN users ON log_user_id = user_id LEFT JOIN clients ON log_client_id = client_id WHERE (log_type LIKE '%$q%' OR log_action LIKE '%$q%' OR log_description LIKE '%$q%' OR log_ip LIKE '%$q%' OR log_user_agent LIKE '%$q%' OR user_name LIKE '%$q%' OR client_name LIKE '%$q%') @@ -87,7 +88,7 @@ $num_rows = mysqli_fetch_row(mysqli_query($mysqli, "SELECT FOUND_ROWS()")); - All Users -