auth.php ended in a 130-line block at file scope, so merely including the file
authenticated the current request: it read $_REQUEST, opened a session, queried
the database, and on a login could rewrite the user's stored password hash and
populate $_SESSION. Any caller that wanted one of the functions in the file got
all of that as a side effect, and the order of includes decided when it ran.
HostController requires auth.php twice purely to reach generateAuthHash() and
validateToken().
Move the block into zm_authenticate_request() and call it explicitly from the
two places that want it, web/index.php and AppController::beforeFilter(). The
function returns the ZM\User or null and still sets the global $user, so the
views, ajax handlers and API controllers that read that global are unaffected.
HostController now gets only the function definitions from its requires, which
is all it ever wanted.
Inside a function the five `unset($user)` calls would drop the local binding
and leave the global set, so they become `$user = null` - the idiom the rest of
the file already uses for this, and one that keeps isset($user) false for the
gate at index.php:255. The block's other locals ($ret, $username, $password,
$sql) no longer leak into the caller's scope, which in beforeFilter() means they
can no longer collide with the variables of the same name it assigns just after.
The body is otherwise unchanged; `git diff -w` shows only the wrapper, those
five assignments and the return.
Tests: tests/php/test_auth_no_include_side_effects.php tokenises auth.php and
asserts nothing executes at file scope, with a fixture check so a broken
detector cannot pass vacuously. 4 assertions, all pass. Verified it reports the
pre-refactor file's file-scope block, so it would have caught this.
Not covered by tests: the login, logout, auth-hash and API token flows this
touches. auth.php cannot be included without a database (User.php pulls in
database.php, which connects at include time), so the check is structural.
Needs manual testing on an installed tree before merging.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01477mR97vfnK6zczbHgzq6T
ZM_AUTH_HASH_IPS binds the auth hash to the client address. When that address
changes mid-session - a phone moving between wifi and cellular is the common
case - the hash the browser is still holding no longer matches the address we
now see, and the user is bounced to the login page. The usual workaround is to
turn ZM_AUTH_HASH_IPS off entirely.
Accept the address the request arrives from plus the one it arrived from
immediately before, so an in-flight hash validates once and generateAuthHash()
then reissues against the new address. Addresses are matched exactly. A netmask
was considered and rejected: accepting a whole subnet would let any other host
on the client's network replay a stolen hash, which on a home LAN includes the
cameras themselves.
The previous address is only accepted for as long as a hash issued to it would
itself still be valid (ZM_AUTH_HASH_TTL), so this widens which address is
accepted without extending how long any hash lives. A login clears it, since
nothing from before a privilege boundary should stay acceptable, and only one
previous address is ever retained.
userFromSession() needed the same treatment: it looks the cached hash up by the
live address, so after a change the slot does not exist yet and the user was
reported as not logged in regardless of what getAuthUser() would have accepted.
Also centralises the X-Forwarded-For/REMOTE_ADDR handling in getRemoteAddr(),
replacing four duplicated copies across session.php and auth.php. Those copies
sat on both the generation and validation sides, so any drift between them broke
authentication outright behind a reverse proxy. Network.php holds only that
address parsing; which addresses an auth hash is accepted from is auth policy
and lives in auth.php.
This is web-side only; zms has no session, so a stream request still fails once
on an address change and recovers through the existing auth-refresh path in
MonitorStream.js.
Tests: tests/php/test_remote_addr.php covers getRemoteAddr() parsing and the
session address rotation, and needs no config or database - 18 assertions, all
pass. tests/php/test_auth_hash_candidate_addrs.php covers the acceptance window
including both sides of the TTL boundary; it bootstraps config.php as the other
tests in that directory do and so needs an installed tree to run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01477mR97vfnK6zczbHgzq6T
validateToken() returns array(false, $errorMessage) when a token is
invalid or its signature fails. The token branch in auth.php assigned
$user = $ret[0] unconditionally, leaving $user as boolean false on
failure. Because isset($false) is true in PHP, the ZM_OPT_USE_AUTH gate
in index.php (!isset($user)) was skipped, allowing unauthenticated
access via any malformed ?token= value. It also permitted an
unauthenticated DoS: downstream $user->Username() on a bool fatals.
Check !$ret[0] and unset($user) on failure, mirroring the existing
validateUser branch, so $user stays undefined and the auth gate blocks
the request. The API call sites already throw UnauthorizedException on
!$user and are unaffected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
generateAuthHash() cached every hash under the IP-keyed slot
'AuthHash'.$remoteAddr, but the value was IP-bound only when $useRemoteAddr was
set. getZmuCommand() calls generateAuthHash(false, true), which wrote an
IP-less hash into the IP-bound slot and reset AuthHashGeneratedAt. Because the
status poll runs getZmuCommand (web/ajax/status.php) right before emitting the
auth hash, the poll then served the IP-less hash to the browser; the next
IP-bound request was rejected by the validator, redirecting the user to login
roughly every poll. This happens with a completely stable client IP, so it is
distinct from the IP-rotation case.
Key the cache slot by the address actually baked into the value: only use the
session address when this caller asked for it (and AUTH_HASH_IPS is on). IP-less
and IP-bound hashes now occupy separate slots and can no longer clobber each
other.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On successful login auth.php called zm_session_clear() followed by
zm_session_regenerate_id(), which together emitted three Set-Cookie
ZMSESSID headers: a deletion, a throwaway intermediate id, and the final
authenticated id. This is the multiple-cookie behaviour reported in #2471.
Add zm_session_regenerate_id_login(), which clears the pre-auth session
data and calls session_regenerate_id(true) to issue one new id while
deleting the old session server-side. Same anti-session-fixation
guarantee in a single Set-Cookie. Logout (zm_session_clear) and the
periodic mid-session regeneration are left unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The auth hash is stateless (HMAC over secret+username+password+IP+date)
and intentionally independent of the PHP session. Stream/image URLs
minted while user A was logged in keep working for user A's hash even
after the active session has rotated to B (re-login, session timeout +
new login, another tab, etc.), which is the design.
The eager Warning was firing on these legitimate cross-session requests
and producing log noise. A genuinely tampered request without a valid
hash still falls through to the existing failure-path Info log, which
now also reports sessionUser for diagnostics.
- AppController.php: stop overwriting $_SESSION['remoteAddr'] with bare
REMOTE_ADDR right after zm_session_start() already populated it from
HTTP_X_FORWARDED_FOR. The clobber bound generated hashes to the proxy
IP, but getAuthUser() validates against XFF, so any hash produced
inside the legacy stateful API path was DOA behind a reverse proxy.
- getAuthUser(): prefer the URL user= parameter over
\$_SESSION['username'] for filtering, matching what zms's
zmLoadAuthUser does, and honor ZM_CASE_INSENSITIVE_USERNAMES on the
primary filter. Warn when the URL user= disagrees with the session
username (stale hash, cross-tab contamination, or tampered request).
- Add a Debug input dump on entry and an Info-level failure line that
reports filterUser, XFF, REMOTE_ADDR, rowsTried and the TTL window so
the next 401 surfaces which input is wrong.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When AUTH_HASH_IPS is enabled and ZoneMinder is behind a reverse proxy
(e.g. Nginx in front of Apache), the hash is generated using
HTTP_X_FORWARDED_FOR (the real client IP) but was validated using only
REMOTE_ADDR (the proxy's IP), causing all authentication to fail.
Fix by consistently using HTTP_X_FORWARDED_FOR (first IP only, to guard
against spoofed multi-value headers) with REMOTE_ADDR as fallback in
all three places:
- web/includes/session.php: where remoteAddr is stored for hash generation
- web/includes/auth.php: getAuthUser() validation (PHP, also used by zms CGI)
- src/zm_user.cpp: zmLoadAuthUser() validation (C++ zms binary)
refs #4758
Agent-Logs-Url: https://github.com/ZoneMinder/zoneminder/sessions/959dfe9d-edea-4de5-a3a0-f90b758e5628
Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com>
- Add user= parameter to get_auth_relay() so zms can use the indexed
Username column instead of iterating all users to validate the hash
- Apply the same fix to Event.php getStreamSrc() and getThumbnailSrc()
- Tighten Monitor.php from isset() to !empty() for consistency
- In MonitorStream.js start(), check if the auth hash in the img src
matches the current auth_hash before resuming via CMD_PLAY. If stale,
fall through to rebuild the URL with fresh auth_relay. This prevents
long-running montage pages from spawning zms with expired credentials.
- Downgrade zms auth failure from Error to Warning
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add ZM_OPT_USE_REMEMBER_ME config option (auth section, requires
ZM_OPT_USE_AUTH) that controls whether a Remember Me checkbox appears
on the login form. When enabled and unchecked, the session cookie
lifetime is set to 0 so the browser discards it on close, logging the
user out. When checked, the session persists for ZM_COOKIE_LIFETIME.
When the option is disabled, behavior is unchanged.
- ConfigData.pm.in: new ZM_OPT_USE_REMEMBER_ME boolean option
- login.php: checkbox between password field and reCAPTCHA/submit
- session.php: use lifetime=0 when remember me is off
- actions/login.php: set/clear ZM_REMEMBER_ME cookie on login, also
update $_COOKIE so zm_session_start sees it in the same request
- auth.php: clear ZM_REMEMBER_ME cookie on logout
- en_gb.php: add RememberMe translation string
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a new AUDIT logging level (-5) between PANIC (-4) and NOLOG (shifted
to -6) across C++, PHP, and Perl loggers. AUDIT entries use code 'AUD'
and syslog priority LOG_NOTICE. They record who changed what, from where,
for monitors, filters, users, config, roles, groups, zones, states,
servers, storage, events, snapshots, control caps, and login/logout.
AUDIT entries have their own retention period (ZM_LOG_AUDIT_DATABASE_LIMIT,
default 1 year) separate from regular log pruning. The log pruning in
zmstats.pl and zmaudit.pl now excludes AUDIT rows from regular pruning
and prunes them independently.
Critical safety: the C++ termination logic is changed from
'if (level <= FATAL)' to 'if (level == FATAL || level == PANIC)' to
prevent AUDIT-level log calls from killing the process.
Includes db migration zm_update-1.39.1.sql to shift any stored NOLOG
config values from -5 to -6.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix SQL injection vulnerability in migrateHash() by using prepared statements
- Add null/empty check in password_type() to prevent array access error
- Remove dead code branch in generateAuthHash() (unreachable $_SESSION check)
- Fix PHP version in error message (5.3 -> 5.5 for password_hash)
- Prevent username enumeration by using consistent error messages
- Fix spacing inconsistency in substr() call
- Add TODO comment about MD5 hash weakness
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a User Roles system where roles define reusable permission templates.
When a user has a role assigned, the role provides fallback permissions
(user's direct permissions take precedence; role is used when user has 'None').
Database changes:
- Add User_Roles table with same permission fields as Users
- Add Role_Groups_Permissions table for per-role group overrides
- Add Role_Monitors_Permissions table for per-role monitor overrides
- Add RoleId foreign key to Users table
Permission resolution order:
1. User's direct Monitor/Group permissions (if not 'Inherit')
2. Role's Monitor/Group permissions (if user has role)
3. Role's base permission (if user's is 'None')
4. User's base permission (fallback)
Includes:
- PHP models: User_Role, Role_Group_Permission, Role_Monitor_Permission
- Role management UI in Options > Roles tab
- Role selector in user edit form
- REST API endpoints for roles CRUD
- Translation strings for en_gb
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
