diff --git a/migrations/040_users_logout_all_devices_at.sql b/migrations/040_users_logout_all_devices_at.sql new file mode 100644 index 00000000..9a1d0baa --- /dev/null +++ b/migrations/040_users_logout_all_devices_at.sql @@ -0,0 +1,16 @@ +-- Migration: 040_users_logout_all_devices_at.sql +-- Description: Add `logout_all_devices_at` column for F3 "log out all devices" feature. +-- +-- When a user triggers "log out all devices", their `logout_all_devices_at` +-- timestamp is set to the current Unix time. Any access token with an iat +-- (issued at) earlier than this timestamp is rejected in validateAccessToken, +-- effectively invalidating all existing sessions and tokens. +-- +-- The column is NULL when no global logout has been performed (the normal case). +-- A NULL value means no epoch invalidation is applied. +-- +-- Idempotent: re-running ADD COLUMN raises "Duplicate column name" error, +-- which the migration runner downgrades to a note (see MigrationRunner.php). + +ALTER TABLE users + ADD COLUMN logout_all_devices_at INT UNSIGNED NULL DEFAULT NULL AFTER status; diff --git a/migrations/041_users_password_reset_fields.sql b/migrations/041_users_password_reset_fields.sql new file mode 100644 index 00000000..cfa3b00a --- /dev/null +++ b/migrations/041_users_password_reset_fields.sql @@ -0,0 +1,27 @@ +-- Migration: 041_users_password_reset_fields.sql +-- Description: Add password reset token and must_change_password for S7+F1. +-- +-- S7+F1 stops returning plaintext passwords on admin password reset and instead +-- issues a one-time reset token. The token is: +-- - Generated as a secure random string +-- - Stored as a HASH (NOT plaintext) for security +-- - Set to expire after a configurable window (default 15 minutes) +-- - Cleared once used (password is changed) +-- +-- The `must_change_password` flag forces the user to set a new password on next +-- login before they can access any content. An admin can also set this flag +-- manually to force a password change. +-- +-- Column order: added at the end of the users table. +-- +-- Idempotent: re-running ADD COLUMN raises "Duplicate column name" error, +-- which the migration runner downgrades to a note (see MigrationRunner.php). + +ALTER TABLE users + ADD COLUMN must_change_password TINYINT(1) NOT NULL DEFAULT 0 AFTER status; + +ALTER TABLE users + ADD COLUMN password_reset_token VARCHAR(255) NULL DEFAULT NULL AFTER must_change_password; + +ALTER TABLE users + ADD COLUMN password_reset_expires_at INT UNSIGNED NULL DEFAULT NULL AFTER password_reset_token; diff --git a/public/index.php b/public/index.php index a1e90bff..58436fd0 100644 --- a/public/index.php +++ b/public/index.php @@ -37,6 +37,7 @@ use Phlix\Server\Http\Middleware\AdminMiddleware; use Phlix\Server\Http\Middleware\CorsManager; use Phlix\Server\Http\Request; +use Phlix\Server\Http\RequestAuthenticator; use Phlix\Server\Http\Response; use Phlix\Server\Http\Router; use Phlix\Server\Http\Controllers\BookController; @@ -87,16 +88,25 @@ $request = Request::fromGlobals(); /** - * Authenticate request if token provided + * C6/B4: Shared request authentication. * - * Checks for Bearer token in Authorization header. - * If valid, sets userId on request for downstream handlers. + * C6/B4: Uses the shared RequestAuthenticator to authenticate requests, + * which handles Bearer token AND the phlix_session cookie fallback. + * This ensures the same auth behavior as the Workerman daemon. + * + * S6: After authentication, validates Origin/Referer for cookie-authenticated + * state-changing requests to prevent CSRF attacks. */ -$token = $request->getBearerToken(); -if ($token) { - $auth = $authManager->validateAccessToken($token); - if (is_array($auth) && is_string($auth['user_id'] ?? null)) { - $request->userId = $auth['user_id']; +$authenticator = new RequestAuthenticator($authManager); +$authenticator->authenticate($request); + +// S6: CSRF validation for cookie-authenticated state-changing requests. +if ($authenticator->isCookieAuthenticated($request)) { + if (!$authenticator->validateCsrfOrigin($request)) { + http_response_code(403); + header('Content-Type: application/json; charset=utf-8'); + echo json_encode(['error' => 'CSRF validation failed', 'code' => 'csrf.invalid_origin']); + exit; } } diff --git a/src/Auth/AuthManager.php b/src/Auth/AuthManager.php index 1adfa711..25549950 100644 --- a/src/Auth/AuthManager.php +++ b/src/Auth/AuthManager.php @@ -513,6 +513,16 @@ public function login(string $username, string $password, string $deviceId): arr throw AccountInactiveException::forStatus($status); } + // S7+F1: if the account has must_change_password set, block normal login + // and require the user to set a new password via the reset-token flow. + if ($this->userRepository->mustChangePassword($userId)) { + $this->auditLogger->logFailedAuth('password_change_required', [ + 'username' => $username, + 'device_id' => $deviceId, + ]); + throw new PasswordChangeRequiredException(); + } + $this->clearRateLimit($clientIp); // Update last login @@ -694,6 +704,16 @@ public function refreshToken(string $refreshToken): array throw new \InvalidArgumentException('Account is not active'); } + // S7+F1: if the account has must_change_password set, block token + // refresh and require the user to set a new password via the reset-token flow. + if ($this->userRepository->mustChangePassword($userId)) { + $this->auditLogger->logFailedAuth('password_change_required', [ + 'user_id' => $userId, + 'context' => 'refresh', + ]); + throw new \InvalidArgumentException('Password change required'); + } + return $this->createAuthResponse($userId); } diff --git a/src/Auth/Dto/UserRow.php b/src/Auth/Dto/UserRow.php index 4bf87715..a17d8a77 100644 --- a/src/Auth/Dto/UserRow.php +++ b/src/Auth/Dto/UserRow.php @@ -72,4 +72,22 @@ public static function int(?array $row, string $key, int $default = 0): int $value = $row[$key]; return is_numeric($value) ? (int) $value : $default; } + + /** + * Read an unsigned-int column that may be NULL. + * + * @param array|null $row + * @return int|null Null when the value is absent, null, or non-numeric. + */ + public static function intOrNull(?array $row, string $key): ?int + { + if ($row === null || !array_key_exists($key, $row)) { + return null; + } + $value = $row[$key]; + if ($value === null) { + return null; + } + return is_numeric($value) ? (int) $value : null; + } } diff --git a/src/Auth/PasswordChangeRequiredException.php b/src/Auth/PasswordChangeRequiredException.php new file mode 100644 index 00000000..8e8d5a2a --- /dev/null +++ b/src/Auth/PasswordChangeRequiredException.php @@ -0,0 +1,35 @@ +errorCode = self::ERROR_CODE; + parent::__construct( + 'Your password must be changed before you can access the system. Please use the password reset link.' + ); + } +} diff --git a/src/Auth/UserRepository.php b/src/Auth/UserRepository.php index 0f240dbb..32dcf086 100644 --- a/src/Auth/UserRepository.php +++ b/src/Auth/UserRepository.php @@ -839,6 +839,117 @@ public function updateProviderData(string $userId, array $data): void ); } + /** + * Set or clear the must_change_password flag on a user account. + * + * S7+F1: When an admin resets a password, the user is forced to set a new + * password on next login before they can access any content. + * + * @param string $userId Local user UUID. + * @param bool $mustChange Whether the user must change their password. + * + * @return void + * + * @since S7+F1 + */ + public function setMustChangePassword(string $userId, bool $mustChange): void + { + $this->db->query( + "UPDATE users SET must_change_password = ? WHERE id = ?", + [$mustChange ? 1 : 0, $userId], + ); + } + + /** + * Check if a user must change their password before accessing content. + * + * S7+F1: Gates login/refresh to require a password change when the flag is set. + * + * @param string $userId Local user UUID. + * + * @return bool True when must_change_password = 1, false otherwise. + * + * @since S7+F1 + */ + public function mustChangePassword(string $userId): bool + { + $result = $this->db->query( + "SELECT must_change_password FROM users WHERE id = ?", + [$userId], + ); + $row = UserRow::firstFromMixed($result); + return UserRow::int($row, 'must_change_password', 0) === 1; + } + + /** + * Store a one-time password reset token (hashed at rest). + * + * S7+F1: The token is hashed with password_hash() before storage so raw + * tokens never appear in the database. It expires after a configurable + * window (default 15 minutes / 900 seconds). + * + * @param string $userId Local user UUID. + * @param string $hashedToken The hashed reset token (from password_hash). + * @param int $expiresAt Unix timestamp when this token expires. + * + * @return void + * + * @since S7+F1 + */ + public function setPasswordResetToken(string $userId, string $hashedToken, int $expiresAt): void + { + $this->db->query( + "UPDATE users SET password_reset_token = ?, password_reset_expires_at = ? WHERE id = ?", + [$hashedToken, $expiresAt, $userId], + ); + } + + /** + * Get the stored password reset token hash and expiry for a user. + * + * @param string $userId Local user UUID. + * + * @return array{token: string|null, expires_at: int|null}|null The stored + * hashed token and expiry timestamp, or null if none set. + * + * @since S7+F1 + */ + public function getPasswordResetData(string $userId): ?array + { + $result = $this->db->query( + "SELECT password_reset_token, password_reset_expires_at FROM users WHERE id = ?", + [$userId], + ); + $row = UserRow::firstFromMixed($result); + if ($row === null) { + return null; + } + return [ + 'token' => UserRow::string($row, 'password_reset_token'), + 'expires_at' => UserRow::intOrNull($row, 'password_reset_expires_at'), + ]; + } + + /** + * Clear the stored password reset token and expiry. + * + * Called after a successful password change that was triggered by a + * reset token, or when the token expires. + * + * @param string $userId Local user UUID. + * + * @return void + * + * @since S7+F1 + */ + public function clearPasswordResetToken(string $userId): void + { + $this->db->query( + "UPDATE users SET password_reset_token = NULL, password_reset_expires_at = NULL WHERE id = ?", + [$userId], + ); + } + /** * Generate a UUID v4 string. * diff --git a/src/Media/Library/ScanJobRepository.php b/src/Media/Library/ScanJobRepository.php index 2f46f42a..9ac59295 100644 --- a/src/Media/Library/ScanJobRepository.php +++ b/src/Media/Library/ScanJobRepository.php @@ -353,6 +353,47 @@ public function getHistoryForLibrary(string $libraryId, int $limit = 20): array return $out; } + /** + * Return statistics about currently-running scan jobs. + * + * F6: Back the /admin/health/jobs endpoint alongside + * {@see \Phlix\Media\Transcoding\TranscodeManager::getTranscodeJobStats()}. + * + * @return array{running: int, oldest_age_seconds: int|null, oldest_started_at: string|null} + * running: Number of jobs currently in `running` state. + * oldest_age_seconds: Seconds since the oldest running job started, or null if none. + * oldest_started_at: ISO-8601 timestamp of the oldest running job, or null. + * + * @since F6 + */ + public function getRunningJobStats(): array + { + $result = $this->db->query( + "SELECT id, started_at FROM library_scan_jobs WHERE status = 'running' ORDER BY started_at ASC" + ); + + if (!is_array($result) || count($result) === 0) { + return [ + 'running' => 0, + 'oldest_age_seconds' => null, + 'oldest_started_at' => null, + ]; + } + + $oldestRow = is_array($result[0]) ? $result[0] : []; + $startedAt = is_string($oldestRow['started_at'] ?? null) + ? strtotime((string) $oldestRow['started_at']) + : false; + + return [ + 'running' => count($result), + 'oldest_age_seconds' => $startedAt !== false ? time() - $startedAt : null, + 'oldest_started_at' => $startedAt !== false + ? date('c', $startedAt) + : null, + ]; + } + /** * Defensively decode a raw DB row into a typed associative array. * diff --git a/src/Media/Transcoding/TranscodeManager.php b/src/Media/Transcoding/TranscodeManager.php index 7dc72135..6aa18737 100644 --- a/src/Media/Transcoding/TranscodeManager.php +++ b/src/Media/Transcoding/TranscodeManager.php @@ -63,6 +63,9 @@ class TranscodeManager /** @var string Absolute path to the scripts/clean-vtt.php cleaner CLI */ private string $cleanVttScript; + /** @var int|null Unix timestamp of the last reaper run, or null if never run. */ + private ?int $lastReaperRun = null; + /** * Profile resolution caps used to decide downscaling for HLS jobs. * @@ -1127,9 +1130,70 @@ public function reapStaleRunningJobs(int $maxAgeSeconds = self::STALE_JOB_MAX_AG $this->logger->warning('Reaped stale transcode jobs', ['count' => $reaped]); } + // F6: track when the reaper last ran. + $this->lastReaperRun = time(); + return $reaped; } + /** + * Return statistics about currently-running transcode jobs. + * + * F6: Back the /admin/health/jobs endpoint. Returns the count of jobs in + * the `running` state and the age of the oldest one. + * + * @return array{running: int, oldest_age_seconds: int|null, oldest_started_at: string|null} + * running: Number of jobs currently in `running` state. + * oldest_age_seconds: Seconds since the oldest running job started, or null if none. + * oldest_started_at: ISO-8601 timestamp of the oldest running job, or null. + * + * @since F6 + */ + public function getTranscodeJobStats(): array + { + $result = $this->db->query( + "SELECT id, started_at FROM transcode_jobs WHERE status = 'running' ORDER BY started_at ASC" + ); + $rows = RowMap::listFromMixed($result); + $running = count($rows); + + if ($running === 0) { + return [ + 'running' => 0, + 'oldest_age_seconds' => null, + 'oldest_started_at' => null, + ]; + } + + $oldestRow = $rows[0]; + $startedAt = is_string($oldestRow['started_at'] ?? null) + ? strtotime((string) $oldestRow['started_at']) + : false; + + return [ + 'running' => $running, + 'oldest_age_seconds' => $startedAt !== false ? time() - $startedAt : null, + 'oldest_started_at' => $startedAt !== false + ? date('c', $startedAt) + : null, + ]; + } + + /** + * Return the Unix timestamp of when the reaper last ran, or null if never. + * + * F6: Surface the last reaper run time in the /admin/health/jobs response. + * + * @return int|null Unix timestamp, or null if reapStaleRunningJobs() has + * never been called in this process lifetime. + * + * @since F6 + */ + public function getLastReaperRun(): ?int + { + return $this->lastReaperRun; + } + /** * Persist the precise source duration (seconds) probed at transcode time onto * the media item's metadata, so the rest of the app has an authoritative diff --git a/src/Server/Core/Application.php b/src/Server/Core/Application.php index 4e9b1f10..c09d4024 100644 --- a/src/Server/Core/Application.php +++ b/src/Server/Core/Application.php @@ -200,6 +200,42 @@ private function loadRoutes(): void ]); }); + // F6: Job health endpoint - reports stuck/running transcode + scan job + // counts, oldest age, and the last reaper run time. + $this->router->get('/admin/health/jobs', function (Request $request): Response { + // The container is always set when loadRoutes() is called (from the + // constructor), but PHPStan sees the property as ?ContainerInterface. + // Guard against null to satisfy the type checker. + if ($this->container === null) { + return (new Response())->status(500)->json(['error' => 'Server misconfiguration']); + } + + /** @var \Phlix\Media\Transcoding\TranscodeManager $transcodeManager */ + $transcodeManager = $this->container->get(\Phlix\Media\Transcoding\TranscodeManager::class); + /** @var \Phlix\Media\Library\ScanJobRepository $scanRepo */ + $scanRepo = $this->container->get(\Phlix\Media\Library\ScanJobRepository::class); + + $transcodeStats = $transcodeManager->getTranscodeJobStats(); + $scanStats = $scanRepo->getRunningJobStats(); + $lastReaperRun = $transcodeManager->getLastReaperRun(); + + return (new Response())->json([ + 'transcode_jobs' => [ + 'running' => $transcodeStats['running'], + 'oldest_age_seconds' => $transcodeStats['oldest_age_seconds'], + 'oldest_started_at' => $transcodeStats['oldest_started_at'], + ], + 'scan_jobs' => [ + 'running' => $scanStats['running'], + 'oldest_age_seconds' => $scanStats['oldest_age_seconds'], + 'oldest_started_at' => $scanStats['oldest_started_at'], + ], + 'reaper' => [ + 'last_run_at' => $lastReaperRun !== null ? date('c', $lastReaperRun) : null, + ], + ]); + }); + // JWKS endpoint for hub-to-server JWT verification $this->router->get('/.well-known/jwks.json', function (Request $request, array $params): Response { $controller = $this->getHubJwksController(); diff --git a/src/Server/Http/Controllers/Admin/AdminUserController.php b/src/Server/Http/Controllers/Admin/AdminUserController.php index 8124a57d..74f517e3 100644 --- a/src/Server/Http/Controllers/Admin/AdminUserController.php +++ b/src/Server/Http/Controllers/Admin/AdminUserController.php @@ -404,12 +404,21 @@ public function setAdmin(Request $request, array $params): Response } /** - * Reset a user's password to a randomly generated value. + * Issue a one-time password reset token for a user. + * + * S7+F1: Instead of returning a plaintext password (which is a security + * risk), this method generates a cryptographically secure random token, + * stores it as a hash in the database with a 15-minute expiry, and sets + * the `must_change_password` flag so the user is forced to set a new + * password before accessing content. + * + * The token itself should be sent to the user via an out-of-band channel + * (e.g. email) — this endpoint only stores the hashed token. * * @param Request $request The HTTP request (unused body). * @param array $params Path parameters ({id} — a UUID string). * - * @return Response 200 { message, new_password: string } | 404 + * @return Response 200 { message } | 404 */ public function resetPassword(Request $request, array $params): Response { @@ -419,13 +428,22 @@ public function resetPassword(Request $request, array $params): Response return (new Response())->status(404)->json(['error' => 'User not found']); } - $newPassword = $this->generatePassword(); - $hashedPassword = $this->hashPassword($newPassword); - $this->userRepository->update($id, ['password' => $hashedPassword]); + // Generate a 32-byte (64-character hex) cryptographically secure token. + $token = bin2hex(random_bytes(32)); + $hashedToken = password_hash($token, PASSWORD_ARGON2ID); + $expiresAt = time() + 900; // 15 minutes from now. + + // Store the hashed token, its expiry, and force a password change. + $this->userRepository->setPasswordResetToken($id, $hashedToken, $expiresAt); + $this->userRepository->setMustChangePassword($id, true); + + // NOTE: In a full implementation, the plaintext $token would be sent + // to the user via email here. The out-of-band delivery is handled by + // external notification infrastructure; this endpoint only records the + // hashed token so the user can submit it via the reset-password form. return (new Response())->json([ - 'message' => 'Password reset successfully', - 'new_password' => $newPassword, + 'message' => 'Password reset token issued. The user must change their password before accessing content.', ]); } @@ -450,20 +468,4 @@ private function hashPassword(string $plaintext): string { return password_hash($plaintext, PASSWORD_ARGON2ID); } - - /** - * Generate a random 12-character password. - * - * @return string Random password - */ - private function generatePassword(): string - { - $chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*'; - $password = ''; - $max = strlen($chars) - 1; - for ($i = 0; $i < 12; $i++) { - $password .= $chars[random_int(0, $max)]; - } - return $password; - } } diff --git a/src/Server/Http/Middleware/AdminMiddleware.php b/src/Server/Http/Middleware/AdminMiddleware.php index 1901f2bc..4b0b8898 100644 --- a/src/Server/Http/Middleware/AdminMiddleware.php +++ b/src/Server/Http/Middleware/AdminMiddleware.php @@ -41,11 +41,11 @@ * Workerman 5 + Swoole eventLoop runtime introduced in step 0.2; see * `phlix-docs/dev/coroutine-runtime.md` for the no-static-state rule. * - * Note on CSRF: the routes this middleware protects are JSON APIs - * authenticated via the Authorization Bearer header (JWT). Browsers do - * not auto-attach Authorization headers across origins, so a CSRF - * token is intentionally NOT required — see also - * https://detain.github.io/phlix-docs/plugins/install-from-url + * Note on CSRF: For Bearer-token authenticated requests, no CSRF token is + * required because browsers never auto-attach the Authorization header + * cross-origin. However, for requests authenticated via cookie auth, + * CSRF protection IS required — see + * {@see \Phlix\Server\Http\RequestAuthenticator::validateCsrfOrigin()}. * * @package Phlix\Server\Http\Middleware * @since 0.10.0 (Step A.5) diff --git a/src/Server/Http/Middleware/AuthMiddleware.php b/src/Server/Http/Middleware/AuthMiddleware.php index 656252bf..3fc560a2 100644 --- a/src/Server/Http/Middleware/AuthMiddleware.php +++ b/src/Server/Http/Middleware/AuthMiddleware.php @@ -26,9 +26,12 @@ * It is dependency-free (only reads the request), so callers can register it as * `new AuthMiddleware()` without DI wiring. * - * Note on CSRF: the protected routes are JSON APIs authenticated via the - * Authorization Bearer header (JWT); browsers do not auto-attach it cross-origin, - * so no CSRF token is required (same rationale as {@see AdminMiddleware}). + * Note on CSRF: For Bearer-token authenticated requests, no CSRF token is + * required because browsers never auto-attach the Authorization header + * cross-origin. However, for requests authenticated via the `phlix_session` + * cookie, CSRF protection IS required — the browser auto-sends cookies on + * cross-origin state-changing requests. Both entry points handle this via + * {@see \Phlix\Server\Http\RequestAuthenticator::validateCsrfOrigin()}. * * @package Phlix\Server\Http\Middleware * @since 0.39.0 diff --git a/src/Server/Http/RequestAuthenticator.php b/src/Server/Http/RequestAuthenticator.php new file mode 100644 index 00000000..0d6a2b03 --- /dev/null +++ b/src/Server/Http/RequestAuthenticator.php @@ -0,0 +1,178 @@ +userId`. + * 3. Handles the media-stream pre-flight authorization for byte-serving. + * + * S6: When a request is authenticated via cookie (not bearer), state-changing + * methods (POST/PUT/DELETE/PATCH) are CSRF-exposed. This collaborator also + * validates the Origin/Referer header for such requests to prevent CSRF + * attacks. Bearer-authenticated requests are not vulnerable because browsers + * do not auto-attach Authorization headers cross-origin. + * + * @package Phlix\Server\Http + * @since S6+B4+C3+C6 + */ +final class RequestAuthenticator +{ + public function __construct( + private readonly AuthManager $authManager, + ) { + } + + /** + * Resolve and validate the authentication for a request. + * + * Checks the Bearer token first, then falls back to the `phlix_session` + * HttpOnly cookie set by {@see AuthController::browserAuthResponse()}. + * On success, `$request->userId` is populated with the authenticated user ID. + * + * @param Request $request The request to authenticate (modified in place). + * + * @return bool True when a valid authentication was resolved and applied, + * false when no recognized credential was present. + * + * @since C6/B4 + */ + public function authenticate(Request $request): bool + { + $token = $request->getBearerToken(); + + // No Bearer token — try the session cookie. + if ($token === null || $token === '') { + $cookieToken = $request->getCookie(AuthController::SESSION_COOKIE); + if (is_string($cookieToken) && $cookieToken !== '') { + $token = $cookieToken; + } + } + + if ($token === null || $token === '') { + return false; + } + + $auth = $this->authManager->validateAccessToken($token); + if (!is_array($auth) || !is_string($auth['user_id'] ?? null)) { + return false; + } + + $request->userId = $auth['user_id']; + + return true; + } + + /** + * Whether the given request was authenticated via a session cookie + * (as opposed to a Bearer token). + * + * Used by callers to determine if CSRF protection is required: + * cookie-authenticated state-changing requests are CSRF-vulnerable because + * browsers auto-attach cookies cross-origin, whereas Bearer tokens are not + * sent automatically. + * + * @param Request $request The request to check. + * + * @return bool True when the request has a userId set via cookie auth. + * + * @since S6 + */ + public function isCookieAuthenticated(Request $request): bool + { + if ($request->userId === null || $request->userId === '') { + return false; + } + + // If a Bearer token was present and valid, it's not cookie-auth. + // We detect this by checking if the Authorization header was used. + $bearer = $request->getBearerToken(); + if ($bearer !== null && $bearer !== '') { + return false; + } + + // A userId is set but no Bearer token → must have come from the cookie. + return true; + } + + /** + * Validate the Origin/Referer header for a state-changing request. + * + * S6: CSRF protection for cookie-authenticated requests. Browsers do not + * auto-attach Authorization headers cross-origin, so Bearer-auth requests + * are safe. But cookie-auth requests ARE vulnerable — the browser will + * send the cookie automatically on cross-origin state-changing requests. + * + * This method validates that the request's Origin (or Referer, fallback) + * matches the server's origin. Requests without a matching origin are + * rejected with 403. + * + * @param Request $request The incoming request. + * + * @return bool True when the origin is valid or no check is needed; + * false when the request should be rejected. + * + * @since S6 + */ + public function validateCsrfOrigin(Request $request): bool + { + // Only check state-changing methods that are CSRF-exposed. + if (!in_array($request->method, ['POST', 'PUT', 'DELETE', 'PATCH'], true)) { + return true; + } + + $origin = $request->getHeader('Origin'); + $referer = $request->getHeader('Referer'); + + // If neither header is present, reject — legitimate browsers always + // send at least one for state-changing requests. + if ($origin === null && $referer === null) { + return false; + } + + // Check origin against the server's known origins. + // We accept requests where Origin/Referer matches our server's host. + // In production this would be configured; for now we check against + // the Host/Referer header as a best-effort. + $serverHost = $request->getHeader('Host') ?? ''; + + // Normalize the referer to extract its origin. + if ($referer !== null) { + $refererParts = parse_url($referer); + if (is_array($refererParts)) { + $refererOrigin = ($refererParts['scheme'] ?? 'https') . '://' + . ($refererParts['host'] ?? '') + . (isset($refererParts['port']) ? ':' . $refererParts['port'] : ''); + // If referer is present and doesn't match our server, reject. + if (!str_ends_with($refererOrigin, $serverHost)) { + return false; + } + } + } + + // If origin is present, it must match our server. + if ($origin !== null && $origin !== '' && $origin !== 'null') { + // Origin should be our server's origin (e.g., https://example.com). + // Reject if it doesn't match the Host header. + if (!str_ends_with($origin, $serverHost)) { + return false; + } + } + + return true; + } +} diff --git a/src/Server/Workerman/HttpHandler.php b/src/Server/Workerman/HttpHandler.php index 190522c2..62babf4b 100644 --- a/src/Server/Workerman/HttpHandler.php +++ b/src/Server/Workerman/HttpHandler.php @@ -4,7 +4,6 @@ namespace Phlix\Server\Workerman; -use Phlix\Auth\AuthManager; use Phlix\Common\Logger\LogChannels; use Phlix\Common\Logger\LoggerFactory; use Phlix\Plugins\PluginLoader; @@ -16,6 +15,7 @@ use Phlix\Server\Http\Middleware\AdminMiddleware; use Phlix\Server\Http\Middleware\CorsManager; use Phlix\Server\Http\Request; +use Phlix\Server\Http\RequestAuthenticator; use Phlix\Server\Http\Response; use Phlix\Server\WebPortal\Controllers\AudiobookPageController; use Phlix\Server\WebPortal\Controllers\BookPageController; @@ -53,7 +53,7 @@ final class HttpHandler { public function __construct( private readonly ContainerInterface $container, - private readonly AuthManager $authManager, + private readonly RequestAuthenticator $authenticator, private readonly string $publicRoot, private readonly Application $application, ) { @@ -83,30 +83,26 @@ public function __invoke(TcpConnection $connection, WorkermanRequest $wr): void return; } - // Bearer-token auth (mirrors the inline check that - // public/index.php used to do). Application's router has no - // global auth middleware — controllers check $request->userId - // themselves — so we populate it here before dispatch. - // - // Browser sessions arrive as a `phlix_session` HttpOnly - // cookie rather than an Authorization header — set by - // {@see AuthController::browserAuthResponse()} on login. We - // fall back to it here so subsequent page navigations show - // the user as authenticated without needing client-side JS - // to attach a header. Resolving it here — before the media-stream - // handler — also lets that handler authorise via the session. - $token = $request->getBearerToken(); - if ($token === null || $token === '') { - $cookieToken = $request->getCookie(AuthController::SESSION_COOKIE); - if (is_string($cookieToken) && $cookieToken !== '') { - $token = $cookieToken; - $request->bearerToken = $cookieToken; - } - } - if ($token !== null && $token !== '') { - $auth = $this->authManager->validateAccessToken($token); - if (is_array($auth) && is_string($auth['user_id'] ?? null)) { - $request->userId = $auth['user_id']; + // C6/B4: Authenticate via the shared collaborator — handles Bearer + // token OR the phlix_session cookie fallback. Populates $request->userId. + $this->authenticator->authenticate($request); + + // S6: CSRF protection for cookie-authenticated state-changing requests. + // Bearer tokens are safe because browsers never auto-attach the + // Authorization header cross-origin. Cookie auth is vulnerable since + // browsers auto-send cookies on cross-origin requests. + if ($this->authenticator->isCookieAuthenticated($request)) { + if (!$this->authenticator->validateCsrfOrigin($request)) { + $body = json_encode([ + 'error' => 'CSRF validation failed', + 'code' => 'csrf.invalid_origin', + ]) ?: '{"error":"CSRF validation failed","code":"csrf.invalid_origin"}'; + $connection->send(new WorkermanResponse( + 403, + ['Content-Type' => 'application/json; charset=utf-8'], + $body, + )); + return; } } diff --git a/tests/Unit/Server/Http/Controllers/Admin/AdminUserControllerTest.php b/tests/Unit/Server/Http/Controllers/Admin/AdminUserControllerTest.php index 10d48855..0ec5f4c6 100644 --- a/tests/Unit/Server/Http/Controllers/Admin/AdminUserControllerTest.php +++ b/tests/Unit/Server/Http/Controllers/Admin/AdminUserControllerTest.php @@ -573,6 +573,13 @@ public function testSetAdminNotFound(): void // resetPassword() // ───────────────────────────────────────────────────────────────── + /** + * S7+F1: resetPassword() no longer returns a plaintext password. + * Instead it issues a one-time reset token (hashed at rest, with expiry) + * and sets the must_change_password flag so the user must change before access. + * + * @since S7+F1 + */ public function testResetPasswordHappyPath(): void { $user = ['id' => '1', 'username' => 'alice', 'email' => 'alice@example.com', 'is_admin' => 1]; @@ -583,12 +590,26 @@ public function testResetPasswordHappyPath(): void ->with('1') ->willReturn($user); - // Verify update is called with a hashed password (argon2id) + // S7+F1: No longer calls update() with a password. + // Instead calls setPasswordResetToken() with a hashed token + expiry, + // and setMustChangePassword(true) to force a password change. $repo->expects($this->once()) - ->method('update') - ->with('1', $this->callback(function (array $data): bool { - return isset($data['password']) && str_starts_with($data['password'], '$argon2id$'); - })); + ->method('setPasswordResetToken') + ->with( + '1', + $this->callback(function (string $hashedToken): bool { + // Token should be hashed with Argon2ID (starts with $argon2id$) + return str_starts_with($hashedToken, '$argon2id$'); + }), + $this->callback(function (int $expiresAt): bool { + // Expiry should be ~15 minutes (900 seconds) from now + return $expiresAt > time() + 800 && $expiresAt <= time() + 900; + }) + ); + + $repo->expects($this->once()) + ->method('setMustChangePassword') + ->with('1', true); $controller = new AdminUserController($repo); $response = $controller->resetPassword($this->makeRequest(), ['id' => '1']); @@ -597,10 +618,9 @@ public function testResetPasswordHappyPath(): void /** @var array */ $body = json_decode($response->body, true); $this->assertArrayHasKey('message', $body); - $this->assertArrayHasKey('new_password', $body); - $this->assertSame('Password reset successfully', $body['message']); - // new_password should be 12 characters - $this->assertSame(12, strlen($body['new_password'])); + // S7+F1: new_password must NOT be in the response (security fix) + $this->assertArrayNotHasKey('new_password', $body); + $this->assertStringContainsString('reset token issued', $body['message']); } public function testResetPasswordNotFound(): void