root/JobcardSystem
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: harden sessions and domain validation

Browse Source
This commit is contained in:
Marco0300
2026-09-01 18:58:18 +02:00
parent 480494c5ed
commit 9f5142b071
9 changed files with 40 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+1 -1
View File
@@ -33,4 +33,4 @@ find app config database public -type f -name '*.php' -print0 | xargs -0 -n1 php
- Do not commit `.env` or production credentials. - Do not commit `.env` or production credentials.
- Set `APP_KEY` to a long random value and keep it in a secrets manager in production. - Set `APP_KEY` to a long random value and keep it in a secrets manager in production.
- Credential vault encryption and the remaining domain modules are scheduled in later phases. - Credential vault encryption and the remaining domain modules are scheduled in later phases.
- The initial schema is intentionally migration-ready but is delivered as an idempotent bootstrap SQL file for the first Docker increment. - The initial schema is delivered as a Docker bootstrap SQL file. Apply it once to a new database; later releases should use versioned migrations.
app/Domain/Jobcard/TimeAggregator.php
+1 -1
View File
@@ -22,7 +22,7 @@ final class TimeAggregator
{ {
return $this->total(array_values(array_filter( return $this->total(array_values(array_filter(
$entries, $entries,
static fn ($entry): bool => is_array($entry) && (($entry['counts_toward_sla'] ?? true) === true) static fn ($entry): bool => is_array($entry) && in_array($entry['counts_toward_sla'] ?? true, [true, 1, '1'], true)
))); )));
} }
} }
app/Domain/Jobcard/TimeCalculator.php
+1 -1
View File
@@ -4,7 +4,7 @@ declare(strict_types=1);
function calculate_duration_hours(?string $start, ?string $end, ?float $manualHours = null): ?float function calculate_duration_hours(?string $start, ?string $end, ?float $manualHours = null): ?float
{ {
if ($manualHours !== null) { if ($manualHours !== null) {
return $manualHours >= 0 ? round($manualHours, 2) : null; return $manualHours > 0 ? round($manualHours, 2) : null;
} }
if ($start === null || $end === null || !preg_match('/^\d{2}:\d{2}$/', $start) || !preg_match('/^\d{2}:\d{2}$/', $end)) { if ($start === null || $end === null || !preg_match('/^\d{2}:\d{2}$/', $start) || !preg_match('/^\d{2}:\d{2}$/', $end)) {
return null; return null;
app/Domain/SLA/SlaCalculator.php
+3
View File
@@ -3,6 +3,9 @@ declare(strict_types=1);
function calculate_sla_usage(float $allocatedHours, array $hours): array function calculate_sla_usage(float $allocatedHours, array $hours): array
{ {
if ($allocatedHours < 0) {
throw new InvalidArgumentException('Allocated hours must not be negative.');
}
$used = round(array_sum(array_map(static fn ($value): float => max(0.0, (float)$value), $hours)), 2); $used = round(array_sum(array_map(static fn ($value): float => max(0.0, (float)$value), $hours)), 2);
$remaining = round(max(0.0, $allocatedHours - $used), 2); $remaining = round(max(0.0, $allocatedHours - $used), 2);
$percentage = $allocatedHours > 0 ? round(($used / $allocatedHours) * 100, 2) : ($used > 0 ? 100.0 : 0.0); $percentage = $allocatedHours > 0 ? round(($used / $allocatedHours) * 100, 2) : ($used > 0 ? 100.0 : 0.0);
config/bootstrap.php
+6 -1
View File
@@ -35,9 +35,14 @@ function csrf_token(): string
return $_SESSION['csrf']; return $_SESSION['csrf'];
} }
function scalar_input(mixed $value, string $default = ''): string
{
return is_scalar($value) ? (string)$value : $default;
}
function verify_csrf(): void function verify_csrf(): void
{ {
$provided = (string)($_POST['_csrf'] ?? ''); $provided = scalar_input($_POST['_csrf'] ?? null);
if (!hash_equals((string)($_SESSION['csrf'] ?? ''), $provided)) { if (!hash_equals((string)($_SESSION['csrf'] ?? ''), $provided)) {
http_response_code(419); http_response_code(419);
exit('Invalid CSRF token'); exit('Invalid CSRF token');
public/index.php
+19 -11
View File
@@ -3,8 +3,11 @@ declare(strict_types=1);
require_once __DIR__ . '/../config/bootstrap.php'; require_once __DIR__ . '/../config/bootstrap.php';
require_once __DIR__ . '/../app/Domain/Client/ClientValidator.php'; require_once __DIR__ . '/../app/Domain/Client/ClientValidator.php';
require_once __DIR__ . '/../app/Domain/Jobcard/JobcardReference.php';
session_set_cookie_params(['httponly' => true, 'secure' => !empty($_SERVER['HTTPS']), 'samesite' => 'Lax']); ini_set('session.use_strict_mode', '1');
$forwardedHttps = getenv('TRUST_PROXY') === '1' && scalar_input($_SERVER['HTTP_X_FORWARDED_PROTO'] ?? '') === 'https';
session_set_cookie_params(['httponly' => true, 'secure' => !empty($_SERVER['HTTPS']) || $forwardedHttps, 'samesite' => 'Lax', 'path' => '/']);
session_start(); session_start();
function render_header(string $title): void function render_header(string $title): void
@@ -12,7 +15,7 @@ function render_header(string $title): void
$user = current_user(); $user = current_user();
echo '<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1"><title>' . e($title) . ' · JOBcard</title><link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css" rel="stylesheet"><link href="/assets/app.css" rel="stylesheet"></head><body>'; echo '<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1"><title>' . e($title) . ' · JOBcard</title><link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css" rel="stylesheet"><link href="/assets/app.css" rel="stylesheet"></head><body>';
if ($user) { if ($user) {
echo '<nav class="navbar navbar-dark bg-primary"><div class="container-fluid"><a class="navbar-brand fw-bold" href="/?route=dashboard">JOBcard</a><span class="text-white small">' . e($user['name']) . ' · ' . e($user['role_name']) . ' <a class="btn btn-sm btn-light ms-2" href="/?route=logout">Sign out</a></span></div></nav><div class="container-fluid"><div class="row"><aside class="col-md-2 col-lg-2 border-end bg-white min-vh-100 p-3"><nav class="nav flex-column gap-1"><a class="nav-link sidebar-link" href="/?route=dashboard">Dashboard</a>'; echo '<nav class="navbar navbar-dark bg-primary"><div class="container-fluid"><a class="navbar-brand fw-bold" href="/?route=dashboard">JOBcard</a><span class="text-white small">' . e($user['name']) . ' · ' . e($user['role_name']) . ' <form method="post" action="/?route=logout" class="d-inline"><input type="hidden" name="_csrf" value="' . e(csrf_token()) . '"><button class="btn btn-sm btn-light ms-2">Sign out</button></form></span></div></nav><div class="container-fluid"><div class="row"><aside class="col-md-2 col-lg-2 border-end bg-white min-vh-100 p-3"><nav class="nav flex-column gap-1"><a class="nav-link sidebar-link" href="/?route=dashboard">Dashboard</a>';
if (can('clients.view')) echo '<a class="nav-link sidebar-link" href="/?route=clients">Clients</a>'; if (can('clients.view')) echo '<a class="nav-link sidebar-link" href="/?route=clients">Clients</a>';
if (can('jobcards.view')) echo '<a class="nav-link sidebar-link" href="/?route=jobcards">Jobcards</a>'; if (can('jobcards.view')) echo '<a class="nav-link sidebar-link" href="/?route=jobcards">Jobcards</a>';
if (can('reports.view')) echo '<a class="nav-link sidebar-link" href="/?route=reports">Reports</a>'; if (can('reports.view')) echo '<a class="nav-link sidebar-link" href="/?route=reports">Reports</a>';
@@ -29,9 +32,11 @@ function render_footer(): void
echo '</main>' . ($user ? '</div></div>' : '') . '<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/js/bootstrap.bundle.min.js"></script></body></html>'; echo '</main>' . ($user ? '</div></div>' : '') . '<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/js/bootstrap.bundle.min.js"></script></body></html>';
} }
$route = $_GET['route'] ?? (current_user() ? 'dashboard' : 'login'); $route = scalar_input($_GET['route'] ?? null, current_user() ? 'dashboard' : 'login');
if ($route === 'logout') { if ($route === 'logout') {
if ($_SERVER['REQUEST_METHOD'] !== 'POST') { http_response_code(405); exit('Logout requires POST'); }
verify_csrf();
if (current_user()) audit('logout', 'user', (int)current_user()['id']); if (current_user()) audit('logout', 'user', (int)current_user()['id']);
$_SESSION = []; $_SESSION = [];
session_destroy(); session_destroy();
@@ -45,9 +50,9 @@ if ($route === 'login') {
if ($_SERVER['REQUEST_METHOD'] === 'POST') { if ($_SERVER['REQUEST_METHOD'] === 'POST') {
verify_csrf(); verify_csrf();
$stmt = db()->prepare('SELECT u.*, r.name AS role_name FROM users u JOIN roles r ON r.id = u.role_id WHERE u.email = :email LIMIT 1'); $stmt = db()->prepare('SELECT u.*, r.name AS role_name FROM users u JOIN roles r ON r.id = u.role_id WHERE u.email = :email LIMIT 1');
$stmt->execute(['email' => strtolower(trim((string)($_POST['email'] ?? ''))) ]); $stmt->execute(['email' => strtolower(trim(scalar_input($_POST['email'] ?? null))) ]);
$user = $stmt->fetch(); $user = $stmt->fetch();
if (!$user || !$user['is_active'] || !password_verify((string)($_POST['password'] ?? ''), $user['password_hash'])) { if (!$user || !$user['is_active'] || !password_verify(scalar_input($_POST['password'] ?? null), $user['password_hash'])) {
$error = 'The email or password is incorrect.'; $error = 'The email or password is incorrect.';
} else { } else {
session_regenerate_id(true); session_regenerate_id(true);
@@ -74,9 +79,9 @@ if ($route === 'jobcards') {
if ($_SERVER['REQUEST_METHOD'] === 'POST') { if ($_SERVER['REQUEST_METHOD'] === 'POST') {
require_permission('jobcards.manage'); require_permission('jobcards.manage');
verify_csrf(); verify_csrf();
$clientId = filter_var($_POST['client_id'] ?? null, FILTER_VALIDATE_INT); $clientId = filter_var(scalar_input($_POST['client_id'] ?? null), FILTER_VALIDATE_INT);
$workRequested = trim((string)($_POST['work_requested'] ?? '')); $workRequested = trim(scalar_input($_POST['work_requested'] ?? null));
$priority = (string)($_POST['priority'] ?? 'normal'); $priority = scalar_input($_POST['priority'] ?? null, 'normal');
if (!$clientId || $workRequested === '' || mb_strlen($workRequested) > 10000 || !in_array($priority, ['low', 'normal', 'high', 'critical'], true)) { if (!$clientId || $workRequested === '' || mb_strlen($workRequested) > 10000 || !in_array($priority, ['low', 'normal', 'high', 'critical'], true)) {
$errors[] = 'Select a client, enter the requested work, and choose a valid priority.'; $errors[] = 'Select a client, enter the requested work, and choose a valid priority.';
} else { } else {
@@ -85,7 +90,10 @@ if ($route === 'jobcards') {
if (!$clientCheck->fetchColumn()) $errors[] = 'The selected client is not active or does not exist.'; if (!$clientCheck->fetchColumn()) $errors[] = 'The selected client is not active or does not exist.';
} }
if (!$errors) { if (!$errors) {
$reference = 'JC-' . date('Ymd') . '-' . strtoupper(bin2hex(random_bytes(3))); $year = (int)date('Y');
$sequenceStmt = db()->prepare('SELECT COALESCE(MAX(CAST(SUBSTRING(reference_no, 9) AS UNSIGNED)), 0) + 1 FROM jobcards WHERE reference_no LIKE :prefix');
$sequenceStmt->execute(['prefix' => 'JC-' . $year . '-%']);
$reference = \App\Domain\Jobcard\JobcardReference::generate((int)$sequenceStmt->fetchColumn(), $year);
$stmt = db()->prepare('INSERT INTO jobcards (reference_no, client_id, created_by, priority, status, work_requested) VALUES (:reference, :client, :created_by, :priority, \'new\', :requested)'); $stmt = db()->prepare('INSERT INTO jobcards (reference_no, client_id, created_by, priority, status, work_requested) VALUES (:reference, :client, :created_by, :priority, \'new\', :requested)');
$stmt->execute(['reference' => $reference, 'client' => $clientId, 'created_by' => $user['id'], 'priority' => $priority, 'requested' => $workRequested]); $stmt->execute(['reference' => $reference, 'client' => $clientId, 'created_by' => $user['id'], 'priority' => $priority, 'requested' => $workRequested]);
$jobcardId = (int)db()->lastInsertId(); $jobcardId = (int)db()->lastInsertId();
@@ -111,7 +119,7 @@ if ($route === 'jobcards') {
if ($route === 'client') { if ($route === 'client') {
require_permission('clients.view'); require_permission('clients.view');
$clientId = filter_input(INPUT_GET, 'id', FILTER_VALIDATE_INT); $clientId = filter_var(scalar_input($_GET['id'] ?? null), FILTER_VALIDATE_INT);
if (!$clientId) { http_response_code(400); exit('Invalid client'); } if (!$clientId) { http_response_code(400); exit('Invalid client'); }
$stmt = db()->prepare('SELECT * FROM clients WHERE id = :id'); $stmt = db()->prepare('SELECT * FROM clients WHERE id = :id');
$stmt->execute(['id' => $clientId]); $stmt->execute(['id' => $clientId]);
@@ -148,7 +156,7 @@ if ($route === 'clients') {
exit; exit;
} }
} }
$search = trim((string)($_GET['q'] ?? '')); $search = trim(scalar_input($_GET['q'] ?? null));
$stmt = db()->prepare('SELECT id, name, status, support_email, support_phone, created_at FROM clients WHERE (:search = \'\' OR name LIKE :like_name OR support_email LIKE :like_email) ORDER BY name LIMIT 100'); $stmt = db()->prepare('SELECT id, name, status, support_email, support_phone, created_at FROM clients WHERE (:search = \'\' OR name LIKE :like_name OR support_email LIKE :like_email) ORDER BY name LIMIT 100');
$stmt->execute(['search' => $search, 'like_name' => "%{$search}%", 'like_email' => "%{$search}%"]); $stmt->execute(['search' => $search, 'like_name' => "%{$search}%", 'like_email' => "%{$search}%"]);
$clients = $stmt->fetchAll(); $clients = $stmt->fetchAll();
tests/JobcardCalculationsTest.php
+3
View File
@@ -13,6 +13,9 @@ if (calculate_duration_hours(null, null, 1.25) !== 1.25) {
if (calculate_duration_hours('11:30', '09:00') !== null) { if (calculate_duration_hours('11:30', '09:00') !== null) {
throw new RuntimeException('Expected invalid reverse time to be rejected'); throw new RuntimeException('Expected invalid reverse time to be rejected');
} }
if (calculate_duration_hours(null, null, 0.0) !== null) {
throw new RuntimeException('Expected zero manual hours to be rejected');
}
$sla = calculate_sla_usage(20.0, [2.0, 1.5, 1.0]); $sla = calculate_sla_usage(20.0, [2.0, 1.5, 1.0]);
if ($sla !== ['used' => 4.5, 'remaining' => 15.5, 'percentage' => 22.5, 'status' => 'within_limit']) { if ($sla !== ['used' => 4.5, 'remaining' => 15.5, 'percentage' => 22.5, 'status' => 'within_limit']) {
throw new RuntimeException('Unexpected SLA calculation: ' . json_encode($sla)); throw new RuntimeException('Unexpected SLA calculation: ' . json_encode($sla));
tests/JobcardDomainTest.php
+1 -1
View File
@@ -29,5 +29,5 @@ if ($nonNumericEntry['valid'] || !isset($nonNumericEntry['errors']['hours'])) th
$aggregator = new TimeAggregator(); $aggregator = new TimeAggregator();
if ($aggregator->total([['hours' => 2.25], ['hours' => 1.5], ['hours' => -4]]) !== 3.75) throw new RuntimeException('Expected positive time aggregation.'); if ($aggregator->total([['hours' => 2.25], ['hours' => 1.5], ['hours' => -4]]) !== 3.75) throw new RuntimeException('Expected positive time aggregation.');
if ($aggregator->slaTotal([['hours' => 2, 'counts_toward_sla' => true], ['hours' => 3, 'counts_toward_sla' => false]]) !== 2.0) throw new RuntimeException('Expected SLA-filtered aggregation.'); if ($aggregator->slaTotal([['hours' => 2, 'counts_toward_sla' => true], ['hours' => 3, 'counts_toward_sla' => false], ['hours' => 1, 'counts_toward_sla' => 1]]) !== 3.0) throw new RuntimeException('Expected SLA-filtered aggregation.');
printf("Jobcard domain tests: 12 passed\n"); printf("Jobcard domain tests: 12 passed\n");
tests/SlaDomainTest.php
+5
View File
@@ -2,9 +2,14 @@
declare(strict_types=1); declare(strict_types=1);
require_once __DIR__ . '/../app/Domain/SLA/SlaThresholdClassifier.php'; require_once __DIR__ . '/../app/Domain/SLA/SlaThresholdClassifier.php';
require_once __DIR__ . '/../app/Domain/SLA/SlaCalculator.php';
use App\Domain\SLA\SlaThresholdClassifier; use App\Domain\SLA\SlaThresholdClassifier;
$negativeRejected = false;
try { calculate_sla_usage(-1.0, [1.0]); } catch (InvalidArgumentException $exception) { $negativeRejected = true; }
if (!$negativeRejected) throw new RuntimeException('Negative SLA allocation must be rejected');
$classifier = new SlaThresholdClassifier(); $classifier = new SlaThresholdClassifier();
if ($classifier->classify(7.5, 10.0) !== 'warning') throw new RuntimeException('75% should be warning.'); if ($classifier->classify(7.5, 10.0) !== 'warning') throw new RuntimeException('75% should be warning.');
if ($classifier->classify(9.0, 10.0) !== 'critical') throw new RuntimeException('90% should be critical.'); if ($classifier->classify(9.0, 10.0) !== 'critical') throw new RuntimeException('90% should be critical.');