From 71bf64c1e2c9b783a3603797071a82316e9e2f5f Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 12:17:15 -0400 Subject: [PATCH 1/2] Complete ErrorHandler rollout: wire into all endpoints, fix display_errors gaps, add styled 500 page (#38, #39, #105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README documented ErrorHandler.php as a "global error/exception handler", but ErrorHandler::init() had exactly one caller app-wide (api/get_template.php). 13 endpoints never called ini_set('display_errors', 0) at all, relying on the server's global php.ini default, and index.php never registered any handler — a genuine PHP fatal during a page render fell through to PHP's raw default handling with no app-level 500 response, styled or otherwise. Investigating the "13 endpoints" claim turned up that 9 of them (assign_ticket.php, audit_log.php, check_duplicates.php, get_comments.php, get_users.php, notifications.php, saved_filters.php, user_preferences.php, watch_ticket.php) already require api/bootstrap.php as their first statement, which itself calls ini_set('display_errors', 0) — so they were never actually exposed; the static grep just couldn't see through the require. The 3 that were genuinely unprotected (bulk_operation.php, download_attachment.php, health.php) are fixed here. ticket_dependencies.php already has its own complete hand-rolled equivalent (shutdown handler, error handler, exception handler, output-buffer aware) and was deliberately left alone rather than risk double-registering handlers. Rather than duplicate the fix 30+ times, wired ErrorHandler::init() directly into api/bootstrap.php (covering all 9 files above at once) and into each of the other endpoints' own ini_set/error_reporting pair, replacing it in place — additive only: existing try/catch blocks in every endpoint still handle what they already handled identically, this only adds a safety net for genuinely uncaught fatals that fell through everything else. Before doing this app-wide, removed ErrorHandler::init()'s override of PHP's 'error_log' ini setting: it redirected every error_log() call in the request to a fixed /tmp file, which would have silently diverted logs away from wherever the server is actually configured to send them the moment this got wired into more than one endpoint. That override only existed to support getRecentErrors(), which has zero callers app-wide. For index.php (page views, not JSON), added an 'html' response mode to ErrorHandler that renders a new views/error_500.php instead of a JSON body. That view is deliberately self-contained (no layout_header.php, no $GLOBALS/session/DB dependency) since a genuine fatal can happen before config.php finishes loading or mid-session-start. Verified: a real uncaught error with no prior output correctly produces a clean JSON 500 (API mode) or the styled HTML page (page mode) to the client while the full stack trace goes to error_log, not the response; normal (non-fatal) requests through both a bootstrap.php-based endpoint and index.php are byte-for-byte unaffected. Full project phpcs pass is clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- api/add_comment.php | 4 ++-- api/bootstrap.php | 4 ++-- api/bulk_operation.php | 3 +++ api/clone_ticket.php | 4 ++-- api/custom_fields.php | 4 ++-- api/delete_attachment.php | 4 ++-- api/delete_comment.php | 4 ++-- api/download_attachment.php | 3 +++ api/export_tickets.php | 4 ++-- api/generate_api_key.php | 4 ++-- api/health.php | 3 +++ api/manage_recurring.php | 4 ++-- api/manage_templates.php | 4 ++-- api/manage_workflows.php | 4 ++-- api/revoke_api_key.php | 4 ++-- api/ticket_comment_api.php | 4 ++-- api/ticket_status_api.php | 4 ++-- api/tickets_api.php | 4 ++-- api/update_comment.php | 4 ++-- api/update_ticket.php | 4 ++-- api/upload_attachment.php | 4 ++-- api/user_avatar.php | 4 ++-- create_ticket_api.php | 4 ++-- helpers/ErrorHandler.php | 31 +++++++++++++++++++++++++----- index.php | 7 +++++++ views/error_500.php | 38 +++++++++++++++++++++++++++++++++++++ 26 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 views/error_500.php diff --git a/api/add_comment.php b/api/add_comment.php index 6fecce9..0a17e8b 100644 --- a/api/add_comment.php +++ b/api/add_comment.php @@ -1,8 +1,8 @@ + + + + + + 500 — Something Went Wrong + + + + +
+ ╚╝ +
[ 500 ] SOMETHING WENT WRONG
+
+

+ An unexpected error occurred. It's been logged; please try again shortly. +

+ ← Dashboard +
+
+ + From 6b7e67eee48f8475d202f46b09922378abe5c682 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 12:17:27 -0400 Subject: [PATCH 2/2] Periodically re-sync session privileges from Authelia (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AuthMiddleware::authenticate() only re-read Remote-User/Remote-Groups (and thus is_admin, via UserModel::syncUserFromAuthelia) when $_SESSION['user'] didn't exist yet. Once a session existed, every subsequent request only checked the idle timer — never re-validating against current Authelia/LLDAP state. An admin's group membership revoked in LLDAP, or a logout at the Authelia proxy, left their already-open session with full access for up to SESSION_TIMEOUT (5h default), with no way to force early revocation short of clearing the server-side session store. Added PRIVILEGE_RESYNC_INTERVAL (default 5 min, matching UserModel's own cache TTL) and a resyncPrivileges() check on every already-authenticated request past that interval: re-reads the current request's forward-auth headers (enforcing the trusted-proxy check again, same as a fresh login), and either destroys the session and redirects to re-auth if the user no longer has any required group, or re-syncs is_admin/groups/display_name/email if they do. Best-effort if this particular request doesn't carry forward-auth headers at all (skips silently rather than force-logging out, retried next interval). UserModel::syncUserFromAuthelia() has its own 5-minute in-process cache keyed only by username (not by the groups being synced), so a naive re-call during a resync would have kept returning the pre-revocation cached result for up to 5 more minutes — invalidated that cache entry immediately beforehand to guarantee a real re-sync. Verified against real MariaDB across a fresh login, a same-interval request confirming no premature resync, an admin-privilege-revocation mid-session (is_admin flips to false in both session and DB, verified via a direct query), and a full group-membership revocation (session destroyed, redirected to re-auth, confirmed the request never reaches past that point). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- config/config.php | 5 +++ middleware/AuthMiddleware.php | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/config/config.php b/config/config.php index 80ab44e..c59b023 100644 --- a/config/config.php +++ b/config/config.php @@ -106,6 +106,11 @@ $GLOBALS['config'] = [ 'SESSION_TIMEOUT' => 18000, // 5 hours in seconds 'SESSION_REGENERATE_INTERVAL' => 300, // Regenerate session ID every 5 minutes + // How often an already-logged-in session re-validates Remote-User/ + // Remote-Groups against current Authelia/LLDAP state (AuthMiddleware). + // Without this, a revoked admin keeps full access for up to SESSION_TIMEOUT. + 'PRIVILEGE_RESYNC_INTERVAL' => 300, // 5 minutes + // CSRF settings 'CSRF_LIFETIME' => 3600, // 1 hour in seconds diff --git a/middleware/AuthMiddleware.php b/middleware/AuthMiddleware.php index 3201e29..27686fb 100644 --- a/middleware/AuthMiddleware.php +++ b/middleware/AuthMiddleware.php @@ -92,6 +92,19 @@ class AuthMiddleware } else { // Update last activity time $_SESSION['last_activity'] = time(); + + // Periodically re-validate Remote-User/Remote-Groups against + // current Authelia/LLDAP state, so a revoked admin (or anyone + // dropped from the required groups) loses access promptly + // instead of keeping it for up to SESSION_TIMEOUT. Only the + // idle timer was checked above; nothing previously re-read + // these headers once a session already existed. + $resyncInterval = $GLOBALS['config']['PRIVILEGE_RESYNC_INTERVAL'] ?? 300; + $lastSync = $_SESSION['last_privilege_sync'] ?? 0; + if (time() - $lastSync > $resyncInterval) { + $this->resyncPrivileges(); + } + return $_SESSION['user']; } } @@ -134,6 +147,7 @@ class AuthMiddleware // Store user in session $_SESSION['user'] = $user; $_SESSION['last_activity'] = time(); + $_SESSION['last_privilege_sync'] = time(); // Generate new CSRF token on login require_once __DIR__ . '/CsrfMiddleware.php'; @@ -142,6 +156,64 @@ class AuthMiddleware return $user; } + /** + * Re-validate the current session's Remote-User/Remote-Groups against + * this request's forward-auth headers, and re-sync or revoke access on + * mismatch. Called periodically (PRIVILEGE_RESYNC_INTERVAL) from an + * already-authenticated session — see authenticate(). + * + * Best-effort: if this particular request doesn't carry forward-auth + * headers at all (e.g. a proxy hiccup), the session is left as-is rather + * than force-logging the user out, and the check is simply retried on + * the next request past the interval. + */ + private function resyncPrivileges(): void + { + $username = $this->getHeader('HTTP_REMOTE_USER'); + $groups = $this->getHeader('HTTP_REMOTE_GROUPS'); + + if (empty($username)) { + return; + } + + $this->enforceTrustedProxy(); + + // A different Remote-User than the session's own means Authelia is + // now asserting a different identity entirely for this proxy path; + // don't silently relabel the session as that other user. + if ($username !== ($_SESSION['user']['username'] ?? null)) { + return; + } + + if (!$this->checkGroupAccess($groups)) { + $this->logSecurityEvent('privilege_resync_revoked', [ + 'username' => $username, + 'groups' => $groups ?: 'none', + ]); + session_unset(); + session_destroy(); + $this->redirectToAuth(); + exit; + } + + $displayName = $this->getHeader('HTTP_REMOTE_NAME'); + $email = $this->getHeader('HTTP_REMOTE_EMAIL'); + + // Bypass UserModel's 5-minute in-process cache — that cache key isn't + // group-aware, so a stale cached hit here would silently keep serving + // the pre-revocation is_admin value for the rest of the cache's TTL. + UserModel::invalidateCache(null, $username); + $user = $this->userModel->syncUserFromAuthelia($username, $displayName, $email, $groups); + + $wasAdmin = !empty($_SESSION['user']['is_admin']); + if ($wasAdmin && empty($user['is_admin'])) { + $this->logSecurityEvent('privilege_resync_admin_revoked', ['username' => $username]); + } + + $_SESSION['user'] = $user; + $_SESSION['last_privilege_sync'] = time(); + } + /** * Reject forward-auth headers that did not arrive via a trusted proxy. *