diff --git a/app/Config/Filters.php b/app/Config/Filters.php index 823a703..c1cd695 100644 --- a/app/Config/Filters.php +++ b/app/Config/Filters.php @@ -24,6 +24,8 @@ class Filters extends BaseConfig 'honeypot' => Honeypot::class, 'invalidchars' => InvalidChars::class, 'secureheaders' => SecureHeaders::class, + 'sanitizeinput' => \App\Filters\SanitizeInputFilter::class, + 'apiratelimit' => \App\Filters\ApiRateLimitFilter::class, 'auth' => \App\Filters\AuthFilter::class, // Define the alias for your auth filter 'apiAuth' => \App\Filters\ApiAuthFilter::class, // JWT-based API authentication 'cleanupScheduler' => \App\Filters\CleanupScheduler::class, @@ -42,6 +44,8 @@ class Filters extends BaseConfig public array $globals = [ 'before' => [ 'timezone', + 'sanitizeinput', + 'invalidchars', 'csrf' => ['except' => [ // Webhooks / integrations 'api/paypal-webhook', @@ -80,6 +84,7 @@ class Filters extends BaseConfig ]], ], 'after' => [ + 'secureheaders', 'toolbar', 'cleanupScheduler' => ['except' => ['cleanup/*']], ], @@ -112,7 +117,7 @@ class Filters extends BaseConfig * @var array>> */ public array $filters = [ - + 'apiratelimit' => ['before' => ['api/*', 'index.php/api/*']], ]; } diff --git a/app/Controllers/AuthController.php b/app/Controllers/AuthController.php index df2a879..0f49ca3 100644 --- a/app/Controllers/AuthController.php +++ b/app/Controllers/AuthController.php @@ -5,7 +5,6 @@ namespace App\Controllers; use App\Models\LoginActivityModel; use App\Models\UserModel; use App\Models\UserRoleModel; -use CodeIgniter\Controller; use CodeIgniter\Events\Events; use App\Models\IpAttemptModel; use App\Models\PasswordResetModel; @@ -19,7 +18,7 @@ require_once APPPATH . 'Helpers/pbkdf2_helper.php'; require_once APPPATH . 'Helpers/jwt_helper.php'; -class AuthController extends Controller +class AuthController extends BaseController { protected $configModel; protected $userModel; @@ -83,9 +82,23 @@ class AuthController extends Controller log_message('info', 'Processing login form submission.'); $redirectTo = $this->sanitizeRedirectTarget((string) ($this->request->getPost('redirect_to') ?? $this->request->getGet('redirect_to') ?? '')); + $validator = \Config\Services::validation(); + $requestData = sanitize_request_value($this->request->getPost(['email', 'password'])); + $validator->setRules([ + 'email' => 'required|valid_email|max_length[254]', + 'password' => 'required|max_length[255]', + ]); + + if (! $validator->run($requestData)) { + return redirect() + ->back() + ->with('error', 'Please enter a valid email and password.') + ->withInput(); + } + // Step 1: Get email, password, and IP from the request - $email = $this->request->getPost('email'); - $password = $this->request->getPost('password'); + $email = strtolower((string) $requestData['email']); + $password = (string) $requestData['password']; $ip = $this->request->getIPAddress(); log_message('info', 'Login attempt from IP: ' . $ip . ' for email: ' . $email); @@ -139,26 +152,29 @@ class AuthController extends Controller // JSON API: POST /api/login public function apiLogin() { - $requestData = $this->request->getJSON(true); - if (!$requestData) { - // fallback to form vars - $requestData = [ - 'email' => $this->request->getPost('email'), - 'password' => $this->request->getPost('password'), - ]; - } + $requestData = sanitize_request_value($this->request->getJSON(true) ?: [ + 'email' => $this->request->getPost('email'), + 'password' => $this->request->getPost('password'), + ]); - $email = $requestData['email'] ?? ''; - $password = $requestData['password'] ?? ''; - $ip = $this->request->getIPAddress(); + $validation = \Config\Services::validation(); + $validation->setRules([ + 'email' => 'required|valid_email|max_length[254]', + 'password' => 'required|max_length[255]', + ]); - if (!$email || !$password) { - return $this->response->setStatusCode(400)->setJSON([ + if (! $validation->run($requestData)) { + return $this->response->setStatusCode(422)->setJSON([ 'status' => false, - 'message' => 'Email and password are required.' + 'message' => 'Validation failed.', + 'errors' => $validation->getErrors(), ]); } + $email = strtolower((string) ($requestData['email'] ?? '')); + $password = (string) ($requestData['password'] ?? ''); + $ip = $this->request->getIPAddress(); + if ($this->isIpBlocked($ip)) { return $this->response->setStatusCode(429)->setJSON([ 'status' => false, @@ -220,7 +236,15 @@ class AuthController extends Controller 'exp' => $exp, ]; - $secret = env('JWT_SECRET', 'change-me-in-env'); + try { + $secret = require_env('JWT_SECRET'); + } catch (\RuntimeException $e) { + return $this->response->setStatusCode(500)->setJSON([ + 'status' => false, + 'message' => 'JWT configuration is missing.', + ]); + } + $token = jwt_encode($payload, $secret, 'HS256'); return $this->response->setJSON([ @@ -240,18 +264,14 @@ class AuthController extends Controller */ public function apiRegister() { - $requestData = $this->request->getJSON(true); - if (!$requestData) { - // fallback to form vars - $requestData = $this->request->getPost(); - } + $requestData = sanitize_request_value($this->request->getJSON(true) ?: $this->request->getPost()); // Basic validation $rules = [ - 'firstname' => 'required|min_length[2]|max_length[30]', - 'lastname' => 'required|min_length[2]|max_length[30]', - 'email' => 'required|valid_email|is_unique[users.email]', - 'password' => 'required|min_length[8]', + 'firstname' => "required|regex_match[/^[\\p{L}\\s'\\-]+$/u]|min_length[2]|max_length[30]", + 'lastname' => "required|regex_match[/^[\\p{L}\\s'\\-]+$/u]|min_length[2]|max_length[30]", + 'email' => 'required|valid_email|max_length[254]|is_unique[users.email]', + 'password' => 'required|min_length[8]|max_length[255]', 'cellphone' => 'required|min_length[10]|max_length[20]', ]; @@ -332,7 +352,7 @@ class AuthController extends Controller 'exp' => $exp, ]; - $secret = env('JWT_SECRET', 'change-me-in-env'); + $secret = require_env('JWT_SECRET'); $token = jwt_encode($payload, $secret, 'HS256'); return $this->response->setStatusCode(201)->setJSON([ diff --git a/app/Controllers/BaseController.php b/app/Controllers/BaseController.php index 143ba73..9289198 100644 --- a/app/Controllers/BaseController.php +++ b/app/Controllers/BaseController.php @@ -36,7 +36,7 @@ abstract class BaseController extends Controller * * @var list */ - protected $helpers = []; + protected $helpers = ['security']; /** @var ApiClient */ protected ApiClient $api; @@ -73,6 +73,21 @@ abstract class BaseController extends Controller // Assuming the user role is stored in the session return session()->get('role'); } + + protected function validated(array $rules, ?array $data = null): array + { + $validation = service('validation'); + $payload = $data ?? ($this->request->getJSON(true) ?: $this->request->getPost()); + $payload = sanitize_request_value($payload); + + $validation->setRules($rules); + + if (! $validation->run($payload)) { + throw new \InvalidArgumentException(json_encode($validation->getErrors(), JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES) ?: 'Validation failed'); + } + + return $payload; + } } -?> \ No newline at end of file +?> diff --git a/app/Controllers/ProofreadController.php b/app/Controllers/ProofreadController.php index 3c8a897..d8e64a4 100644 --- a/app/Controllers/ProofreadController.php +++ b/app/Controllers/ProofreadController.php @@ -20,15 +20,22 @@ class ProofreadController extends ResourceController } // Accept form-urlencoded payload to play nicely with CSRF protection - $text = (string) ($this->request->getPost('text') ?? ''); - if ($text === '' || mb_strlen($text) > 20000) { + $validation = service('validation'); + $payload = sanitize_request_value($this->request->getPost(['text'])); + $validation->setRules([ + 'text' => 'required|min_length[1]|max_length[20000]', + ]); + + if (! $validation->run($payload)) { return $this->respond([ 'ok' => false, - 'error' => 'Invalid text (empty or too long).', + 'error' => 'Invalid text payload.', 'csrfHash' => csrf_hash(), ], 422); } + $text = (string) $payload['text']; + $client = \Config\Services::curlrequest(['timeout' => 10]); try { diff --git a/app/Controllers/View/ContactController.php b/app/Controllers/View/ContactController.php index 1f0a109..bdd597e 100644 --- a/app/Controllers/View/ContactController.php +++ b/app/Controllers/View/ContactController.php @@ -24,10 +24,10 @@ class ContactController extends BaseController // Define validation rules $validation->setRules([ - 'name' => 'required|min_length[3]', - 'email' => 'required|valid_email', - 'subject' => 'required|min_length[3]', - 'message' => 'required|min_length[10]' + 'name' => "required|regex_match[/^[\\p{L}\\s'\\-]+$/u]|min_length[3]|max_length[100]", + 'email' => 'required|valid_email|max_length[254]', + 'subject' => 'required|min_length[3]|max_length[150]', + 'message' => 'required|min_length[10]|max_length[5000]' ]); if (!$validation->withRequest($this->request)->run()) { @@ -37,10 +37,10 @@ class ContactController extends BaseController } // Process form data - $name = $this->request->getPost('name'); - $email = strtolower($this->request->getPost('email')); - $subject = $this->request->getPost('subject'); - $message = $this->request->getPost('message'); + $name = esc((string) $this->request->getPost('name')); + $email = esc(strtolower((string) $this->request->getPost('email'))); + $subject = esc((string) $this->request->getPost('subject')); + $message = nl2br(esc((string) $this->request->getPost('message'))); // Initialize the EmailController $emailController = new \App\Controllers\View\EmailController(); @@ -63,4 +63,4 @@ class ContactController extends BaseController return redirect()->to('/parent/contact'); } -} \ No newline at end of file +} diff --git a/app/Controllers/View/EventController.php b/app/Controllers/View/EventController.php index 5e3caf1..d44b2a6 100644 --- a/app/Controllers/View/EventController.php +++ b/app/Controllers/View/EventController.php @@ -486,7 +486,7 @@ class EventController extends ResourceController $chargesBuilder = $this->eventChargesModel ->select('event_charges.*, - users.firstname AS parent_firstname, users.lastname AS parent_lastname, + users.firstname AS parent_firstname, users.lastname AS parent_lastname, users.cellphone AS parent_cellphone, students.firstname AS student_firstname, students.lastname AS student_lastname, events.event_name, events.description AS event_description, events.amount AS event_amount') ->join('users', 'users.id = event_charges.parent_id', 'left') diff --git a/app/Filters/ApiAuthFilter.php b/app/Filters/ApiAuthFilter.php index a91c3f9..a687b44 100644 --- a/app/Filters/ApiAuthFilter.php +++ b/app/Filters/ApiAuthFilter.php @@ -11,6 +11,8 @@ class ApiAuthFilter implements FilterInterface { public function before(RequestInterface $request, $arguments = null) { + helper('security'); + $authorization = $request->getHeaderLine('Authorization'); if (!$authorization || stripos($authorization, 'Bearer ') !== 0) { return $this->unauthorized('Missing or invalid Authorization header'); @@ -21,7 +23,12 @@ class ApiAuthFilter implements FilterInterface return $this->unauthorized('Bearer token is required'); } - $secret = env('JWT_SECRET', 'change-me-in-env'); + try { + $secret = require_env('JWT_SECRET'); + } catch (\RuntimeException $e) { + return $this->unauthorized('JWT configuration is missing'); + } + $payload = jwt_decode($token, $secret); if (!$payload || empty($payload['sub'])) { diff --git a/app/Filters/ApiDocsAuthFilter.php b/app/Filters/ApiDocsAuthFilter.php index 4938b9e..0791808 100644 --- a/app/Filters/ApiDocsAuthFilter.php +++ b/app/Filters/ApiDocsAuthFilter.php @@ -13,13 +13,15 @@ class ApiDocsAuthFilter implements FilterInterface { public function before(RequestInterface $request, $arguments = null) { + helper('security'); + $authHeader = $request->getHeaderLine('Authorization'); if ($authHeader && str_starts_with($authHeader, 'Bearer ')) { $token = trim(substr($authHeader, 7)); try { - $key = getenv('JWT_SECRET') ?: 'your_default_secret'; + $key = require_env('JWT_SECRET'); $decoded = JWT::decode($token, new Key($key, 'HS256')); if (!isset($decoded->roles) || empty($decoded->roles->admin)) { diff --git a/app/Filters/ApiRateLimitFilter.php b/app/Filters/ApiRateLimitFilter.php new file mode 100644 index 0000000..751a462 --- /dev/null +++ b/app/Filters/ApiRateLimitFilter.php @@ -0,0 +1,42 @@ +get('user_id'); + $identifier = $userId ? 'user:' . $userId : 'ip:' . $request->getIPAddress(); + $route = trim($request->getUri()->getPath(), '/'); + $key = 'api-rate-' . sha1($request->getMethod() . '|' . $route . '|' . $identifier); + + if (! $throttler->check($key, $maxRequests, $windowSeconds)) { + return $response + ->setStatusCode(429) + ->setHeader('Retry-After', (string) $windowSeconds) + ->setJSON([ + 'status' => false, + 'message' => 'Too many requests. Please try again later.', + ]); + } + + return null; + } + + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null) + { + return $response; + } +} diff --git a/app/Filters/SanitizeInputFilter.php b/app/Filters/SanitizeInputFilter.php new file mode 100644 index 0000000..6b87b52 --- /dev/null +++ b/app/Filters/SanitizeInputFilter.php @@ -0,0 +1,48 @@ +getGet() ?? []); + $sanitizedPost = sanitize_request_value($request->getPost() ?? []); + $request->setGlobal('get', $sanitizedGet); + $request->setGlobal('post', $sanitizedPost); + $request->setGlobal('request', array_merge(is_array($sanitizedGet) ? $sanitizedGet : [], is_array($sanitizedPost) ? $sanitizedPost : [])); + $request->setGlobal('cookie', sanitize_request_value($request->getCookie() ?? [])); + + $contentType = strtolower($request->getHeaderLine('Content-Type')); + $body = $request->getBody(); + + if (str_contains($contentType, 'application/json') && trim($body) !== '') { + $decoded = json_decode($body, true); + + if (json_last_error() !== JSON_ERROR_NONE || ! is_array($decoded)) { + return Services::response() + ->setStatusCode(400) + ->setJSON([ + 'status' => false, + 'message' => 'Malformed JSON payload.', + ]); + } + + $request->setBody((string) json_encode(sanitize_request_value($decoded), JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES)); + } + + return null; + } + + public function after(RequestInterface $request, ResponseInterface $response, $arguments = null) + { + return $response; + } +} diff --git a/app/Helpers/security_helper.php b/app/Helpers/security_helper.php new file mode 100644 index 0000000..e1b9124 --- /dev/null +++ b/app/Helpers/security_helper.php @@ -0,0 +1,41 @@ + $item) { + $value[$key] = sanitize_request_value($item); + } + + return $value; + } + + if (! is_string($value)) { + return $value; + } + + $value = preg_replace('/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/u', '', $value) ?? $value; + + return trim($value); + } +} + +if (! function_exists('require_env')) { + /** + * Fetch an environment variable and fail closed when it is missing. + */ + function require_env(string $key): string + { + $value = env($key); + + if ($value === null || $value === '') { + throw new RuntimeException(sprintf('Missing required environment variable: %s', $key)); + } + + return (string) $value; + } +} diff --git a/app/Libraries/StaffTimeOffLinkService.php b/app/Libraries/StaffTimeOffLinkService.php index 4e9cb81..83a548a 100644 --- a/app/Libraries/StaffTimeOffLinkService.php +++ b/app/Libraries/StaffTimeOffLinkService.php @@ -16,8 +16,9 @@ class StaffTimeOffLinkService public function __construct(?string $secret = null, ?int $ttlSeconds = null) { helper('jwt'); + helper('security'); - $this->secret = $secret ?: (string)env('JWT_SECRET', 'change-me-in-env'); + $this->secret = $secret ?: require_env('JWT_SECRET'); $this->ttl = ($ttlSeconds !== null && $ttlSeconds > 0) ? $ttlSeconds : self::DEFAULT_TTL; } diff --git a/app/Views/administrator/events/event_charges.php b/app/Views/administrator/events/event_charges.php index fcf2652..cdbc817 100644 --- a/app/Views/administrator/events/event_charges.php +++ b/app/Views/administrator/events/event_charges.php @@ -290,11 +290,15 @@ ); $externalParentPhone = $charge['external_parent_phone'] ?? ''; $externalParentEmail = $charge['external_parent_email'] ?? ''; + $parentPhone = $charge['parent_cellphone'] ?? ''; $standardParentName = trim($charge['parent_firstname'] . ' ' . $charge['parent_lastname']); $parentColumnName = $externalParentLabel ?: ($standardParentName ?: '—'); + $isExternalParticipant = $externalName !== ''; + $infoPhone = $isExternalParticipant ? $externalParentPhone : $parentPhone; + $infoEmail = $isExternalParticipant ? $externalParentEmail : ''; $externalInfoValue = trim(implode(' ', array_filter([ - $externalParentPhone, - $externalParentEmail, + $infoPhone, + $infoEmail, $externalNote, ]))); ?> @@ -306,13 +310,13 @@ - -
+ +
- -
+ +
- + — diff --git a/app/Views/user/login.php b/app/Views/user/login.php index 5818426..3815030 100644 --- a/app/Views/user/login.php +++ b/app/Views/user/login.php @@ -18,7 +18,8 @@
+ placeholder="Enter your email" maxlength="254" autocomplete="username" + inputmode="email" value="" required>
@@ -30,7 +31,8 @@ id="password" name="password" placeholder="Enter your password" - maxlength="30" + maxlength="255" + autocomplete="current-password" required> connect_error) { diff --git a/tests/app/Filters/ApiRateLimitFilterTest.php b/tests/app/Filters/ApiRateLimitFilterTest.php new file mode 100644 index 0000000..da1ad2d --- /dev/null +++ b/tests/app/Filters/ApiRateLimitFilterTest.php @@ -0,0 +1,35 @@ +before($request, [1, 60]); + $second = $filter->before($request, [1, 60]); + + $this->assertNull($first); + $this->assertInstanceOf(Response::class, $second); + $this->assertSame(429, $second->getStatusCode()); + } +} diff --git a/tests/app/Filters/SanitizeInputFilterTest.php b/tests/app/Filters/SanitizeInputFilterTest.php new file mode 100644 index 0000000..3b0e879 --- /dev/null +++ b/tests/app/Filters/SanitizeInputFilterTest.php @@ -0,0 +1,48 @@ +setHeader('Content-Type', 'application/json'); + $request->setGlobal('get', ['q' => " hello\x00 "]); + $request->setGlobal('post', ['email' => " user@example.com \n"]); + $request->setGlobal('request', ['email' => " user@example.com \n"]); + $request->setGlobal('cookie', ['token' => " abc\t"]); + $request->setBody("{\"email\":\" user@example.com \",\"name\":\" Bob\\u0000 \"}"); + + $filter = new SanitizeInputFilter(); + $result = $filter->before($request); + + $this->assertNull($result); + $this->assertSame('hello', $request->getGet('q')); + $this->assertSame('user@example.com', $request->getPost('email')); + $this->assertSame('abc', $request->getCookie('token')); + $this->assertSame(['email' => 'user@example.com', 'name' => 'Bob'], $request->getJSON(true)); + } + + public function testRejectsMalformedJsonPayload(): void + { + $request = new IncomingRequest(config(App::class), new URI('https://example.test/api/login'), 'php://input', new UserAgent()); + $request->setHeader('Content-Type', 'application/json'); + $request->setBody('{invalid'); + + $filter = new SanitizeInputFilter(); + $result = $filter->before($request); + + $this->assertInstanceOf(Response::class, $result); + $this->assertSame(400, $result->getStatusCode()); + } +}