From 81efab9cb051abf57f0e832d7d17b9b7e629cb5e Mon Sep 17 00:00:00 2001 From: twotalesanimation <80506065+twotalesanimation@users.noreply.github.com> Date: Sun, 9 Aug 2026 13:56:32 +0200 Subject: [PATCH] Security hardening, DB connection fix, and handover documentation Prepares the site for handover to the incoming committee. Three groups of change: security fixes, a production stability fix, and documentation. SECURITY - upload_blog_image.php accepted unauthenticated uploads with no extension or MIME validation, into a web-served directory. Anyone could upload and execute a .php file. Now requires a session, checks blog ownership, and runs through validateFileUpload() with a new blog_image type. - Added uploads/.htaccess and assets/uploads/.htaccess disabling the PHP engine, so an upload bypass still cannot execute. Force-added: .gitignore contains a bare .htaccess rule. - blog/autosave.php updated posts by blog_id alone, letting any logged-in member overwrite anyone's post. Now scoped to the author, admin override retained. It also silently reassigned authorship to the editing admin; the existing author is now preserved. - add_to_cart.php took item_price and user_id from POST, so drinks could be charged at any price to any member's tab, unauthenticated. Price and name are now looked up server-side from bar_items. Its require paths were also relative and broken. - Session cookies now httponly/secure/samesite with strict mode, set in both .htaccess and .user.ini since php_flag and .user.ini apply under different SAPIs. The 30 minute idle timeout that login set but never checked is now enforced. Google login now regenerates the session id. - Forced HTTPS. Required, because Secure cookies are never sent over plain HTTP and login would fail silently. LetsEncrypt acme-challenge is exempted above the redirect so certificate renewal keeps working. - display_errors off; .htaccess now blocks *.log, error_log, .git and dotfiles. Debug logs containing member IDs, raw webhook bodies and payment signatures were publicly fetchable. - Removed the IKHOKHA_BYPASS_SIGNATURE switch and the logging of raw webhook bodies, app IDs and signatures. Replaced a hardcoded personal Gmail recipient with the optional DEVELOPER_EMAIL variable. - Added auth to fetch_users, fetch_bar_tabs, fetch_drinks, create_bar_tab and submit_order. Converted get_tab_total to a prepared statement. - Tightened 11 mkdir(0777) calls to 0755. - Fixed the validate_login rewrite, which pointed at a nonexistent path and only worked by falling through to the generic .php rule. DATABASE STABILITY openDatabaseConnection() opened a new connection on every call across 73 call sites, and the server was hitting "Too many connections" in production. It now returns one shared handle per request, reusing the connection connection.php already opened. The 40 conn->close() calls in functions.php are removed, since closing the shared handle breaks every later caller; PHP releases it at end of request. A liveness check reconnects transparently if anything closes it. Verified no function was lost (99 before and after), every removed close was inside a braced block, and no transaction spans a helper that would open a nested one. DOCUMENTATION AND TOOLING - docs/HANDOVER.md: operator guide covering architecture, routing, roles, the membership lifecycle, payments, backups, POPIA obligations, environment variables, known issues and a handover checklist. - docs/FUNCTIONS.md: reference for the helpers in functions.php. - scripts/backup.sh: nightly database and uploads backup, written outside the web root, with rotation and verification. Dumps are written to a temp file and only promoted after passing gzip, CREATE TABLE and completion-marker checks, so a failed run can never overwrite a good backup. - .deployignore and .gitattributes added; .gitattributes pins LF on shell scripts and server config because core.autocrlf is true on Windows. - Removed leftover debris: src/api/test_log.php and .htaccess1. All 22 modified PHP files pass php -l. The backup script was tested against a stub mysqldump for healthy, truncated and failing dumps, and for rotation. Nothing was run against a live database or web server. Co-Authored-By: Claude Opus 5 --- .deployignore | 54 +++ .gitattributes | 28 ++ .htaccess | 51 ++- .htaccess1 | 215 ---------- .user.ini | 24 ++ assets/uploads/.htaccess | 19 + docs/FUNCTIONS.md | 300 ++++++++++++++ docs/HANDOVER.md | 473 ++++++++++++++++++++++ scripts/backup.sh | 218 ++++++++++ src/admin/add_campsite.php | 2 +- src/api/fetch_bar_tabs.php | 6 + src/api/fetch_drinks.php | 6 + src/api/fetch_users.php | 6 + src/api/get_tab_total.php | 41 +- src/api/ikhokha_webhook.php | 48 +-- src/api/test_log.php | 10 - src/config/functions.php | 101 ++--- src/config/session.php | 37 +- src/pages/shop/add_to_cart.php | 94 +++-- src/processors/blog/autosave.php | 41 +- src/processors/blog/submit_blog.php | 2 +- src/processors/blog/upload_blog_image.php | 63 ++- src/processors/create_bar_tab.php | 7 + src/processors/process_event.php | 4 +- src/processors/process_signature.php | 2 +- src/processors/process_trip.php | 2 +- src/processors/save_album.php | 2 +- src/processors/submit_order.php | 9 +- src/processors/submit_pop.php | 2 +- src/processors/update_album.php | 4 +- src/processors/upload_profile_picture.php | 2 +- uploads/.htaccess | 19 + validate_login.php | 6 + 33 files changed, 1505 insertions(+), 393 deletions(-) create mode 100644 .deployignore create mode 100644 .gitattributes delete mode 100644 .htaccess1 create mode 100644 assets/uploads/.htaccess create mode 100644 docs/FUNCTIONS.md create mode 100644 docs/HANDOVER.md create mode 100755 scripts/backup.sh delete mode 100644 src/api/test_log.php create mode 100644 uploads/.htaccess diff --git a/.deployignore b/.deployignore new file mode 100644 index 00000000..cd6ac771 --- /dev/null +++ b/.deployignore @@ -0,0 +1,54 @@ +# Files that must NOT be pushed to the live server. +# Used by: rsync -avz --delete --exclude-from=.deployignore ./ user@host:~/html/ +# +# ALWAYS dry-run first: +# rsync -avzn --delete --exclude-from=.deployignore ./ user@host:~/html/ +# +# See docs/HANDOVER.md section 12. + +# --- Version control and tooling ------------------------------------------- +.git/ +.gitignore +.deployignore +.github/ +.gitea/ + +# --- Secrets --------------------------------------------------------------- +# The server has its own .env. Overwriting it takes the site down. +.env +.env.* + +# --- Member-uploaded content ------------------------------------------------ +# These live on the server and are irreplaceable. Never sync over them. +uploads/ +assets/uploads/ + +# --- Backups ---------------------------------------------------------------- +backups/ + +# --- Orphaned dependency ---------------------------------------------------- +# ~231 MB, not referenced by Composer's autoloader, not used by any code. +# See docs/HANDOVER.md section 11. +vendor/google/ + +# --- Developer-only material ------------------------------------------------ +docs/ +scripts/backup.sh.bak +google-client/google-api-php-client--PHP8.0.zip +mailers/email_verify.zip + +# --- Logs, dumps and debris ------------------------------------------------- +*.log +error_log +progress.log +*.sql +*.bak +.htaccess1 +src/logs/ +src/api/_webhook_hit.txt +src/admin/_admin_tx_debug.log + +# --- OS noise --------------------------------------------------------------- +.DS_Store +Thumbs.db +desktop.ini diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..78013db6 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,28 @@ +# Line-ending rules. +# +# The server is Linux. Shell scripts and server config files MUST use LF: +# a shell script checked out with CRLF fails at runtime with +# /bin/bash^M: bad interpreter: No such file or directory +# which is a confusing way to discover your backups have not been running. + +* text=auto + +# Never let these gain CRLF, whatever the developer's platform +*.sh text eol=lf +.htaccess text eol=lf +.user.ini text eol=lf +.env* text eol=lf +*.php text eol=lf + +# Binary - leave completely alone +*.png binary +*.jpg binary +*.jpeg binary +*.gif binary +*.webp binary +*.pdf binary +*.zip binary +*.woff binary +*.woff2 binary +*.ttf binary +*.eot binary diff --git a/.htaccess b/.htaccess index 5f73e1ef..615f81bd 100644 --- a/.htaccess +++ b/.htaccess @@ -3,8 +3,16 @@ RewriteEngine On RewriteBase / -# Allow LetsEncrypt validation -# RewriteRule ^\.well-known/acme-challenge/ - [L] +# Allow LetsEncrypt validation (must stay above the HTTPS redirect) +RewriteRule ^\.well-known/acme-challenge/ - [L] + +# === FORCE HTTPS === +# Required: the session cookie is set with the Secure flag, so a plain-HTTP +# request would never receive it and login would silently fail. +# HTTP_X_FORWARDED_PROTO covers the case where a proxy terminates TLS. +RewriteCond %{HTTPS} !=on +RewriteCond %{HTTP:X-Forwarded-Proto} !=https +RewriteRule ^(.*)$ https://%{HTTP_HOST}/$1 [R=301,L] # Don't rewrite existing files or directories RewriteCond %{REQUEST_FILENAME} !-f @@ -123,7 +131,10 @@ RewriteRule ^google_validate_login$ src/api/google_validate_login.php [L] # === PROCESSORS === RewriteRule ^process_course$ /src/processors/process_course.php [L,QSA] RewriteRule ^delete_course$ /src/processors/delete_course.php [L,QSA] -RewriteRule ^validate_login$ src/processors/validate_login.php [L] +# validate_login.php lives at the repo root, not in src/processors/ - the +# restructure never moved it. This previously pointed at a nonexistent path +# and only worked by falling through to the generic .php rule above. +RewriteRule ^validate_login$ validate_login.php [L] RewriteRule ^register_user$ src/processors/register_user.php [L] RewriteRule ^process_application$ src/processors/process_application.php [L] RewriteRule ^process_booking$ src/processors/process_booking.php [L] @@ -171,12 +182,42 @@ RewriteRule ^autosave$ src/processors/blog/autosave.php [L] -php_flag display_errors On +# Never render PHP errors to visitors - they leak paths, SQL and credentials. +# Errors still go to the server error log. +php_flag display_errors Off +php_flag log_errors On # php_value error_reporting -1 # RedirectMatch 403 ^/\.well-known + +# Session hardening. Duplicated in .user.ini on purpose: php_flag/php_value +# only apply under mod_php, .user.ini only under CGI/FastCGI. Whichever SAPI +# this host uses, one of the two takes effect. Needed because ~25 files call +# session_start() directly instead of including src/config/session.php. +php_flag session.cookie_httponly On +php_flag session.use_strict_mode On +php_flag session.use_only_cookies On +php_flag session.cookie_secure On +php_value session.cookie_samesite Lax + Options -Indexes - +# Block source, dumps, archives and log files from being fetched directly. +# NOTE: "*.log" and the extensionless "error_log" matter most - PHP writes +# member IDs, raw webhook bodies and payment signatures into them. +# .txt is deliberately NOT blocked wholesale so robots.txt and domain +# verification files keep working; the debug .txt files are named below. + + Require all denied + + + + Require all denied + + +# Block dotfiles and VCS metadata (.git, .env, .gitignore, ...) +RedirectMatch 404 /\.(git|svn|hg|env|idea|vscode)(/|$) + + Require all denied diff --git a/.htaccess1 b/.htaccess1 deleted file mode 100644 index 46c835d3..00000000 --- a/.htaccess1 +++ /dev/null @@ -1,215 +0,0 @@ -# URL Rewrite Rules - Maps old URLs to new directory structure during migration - -RewriteEngine On -RewriteBase / - -# Don't rewrite existing files or directories -RewriteCond %{REQUEST_FILENAME} !-f -RewriteCond %{REQUEST_FILENAME} !-d - -# === STRIP .PHP EXTENSION === -# Redirect /page.php to /page (301 permanent redirect) -RewriteCond %{REQUEST_FILENAME} !-d -RewriteRule ^(.+)\.php$ /$1 [R=301,L] -# Internally rewrite /page to /page.php if page.php exists -RewriteCond %{REQUEST_FILENAME}\.php -f -RewriteRule ^(.+)$ $1.php [L] - -# === AUTH PAGES === -RewriteRule ^login$ src/pages/auth/login.php [L] -RewriteRule ^register$ src/pages/auth/register.php [L] -RewriteRule ^forgot_password$ src/pages/auth/forgot_password.php [L] -RewriteRule ^reset_password$ src/pages/auth/reset_password.php [L] -RewriteRule ^verify$ src/pages/auth/verify.php [L] -RewriteRule ^resend_verification$ src/pages/auth/resend_verification.php [L] -RewriteRule ^change_password$ src/pages/auth/change_password.php [L] -RewriteRule ^update_password$ src/pages/auth/update_password.php [L] - -# === MEMBERSHIP PAGES === -RewriteRule ^membership$ src/pages/memberships/membership.php [L] -RewriteRule ^membership_details$ src/pages/memberships/membership_details.php [L] -RewriteRule ^membership_application$ src/pages/memberships/membership_application.php [L] -RewriteRule ^membership_payment$ src/pages/memberships/membership_payment.php [L] -RewriteRule ^renew_membership$ src/pages/memberships/renew_membership.php [L] -RewriteRule ^member_info$ src/pages/memberships/member_info.php [L] - -# === BOOKING PAGES === -RewriteRule ^bookings$ src/pages/bookings/bookings.php [L] -RewriteRule ^campsites$ src/pages/bookings/campsites.php [L] -RewriteRule ^campsite_booking$ src/pages/bookings/campsite_booking.php [L] -RewriteRule ^add_campsite$ src/pages/add_campsite.php [L] -RewriteRule ^trips$ src/pages/bookings/trips.php [L] -RewriteRule ^trip-details$ src/pages/bookings/trip-details.php [L] -RewriteRule ^course_details$ src/pages/bookings/course_details.php [L] -RewriteRule ^driver_training$ src/pages/bookings/driver_training.php [L] - -# === SHOP PAGES === -RewriteRule ^view_cart$ src/pages/shop/view_cart.php [L] -RewriteRule ^add_to_cart$ src/pages/shop/add_to_cart.php [L] -RewriteRule ^bar_tabs$ src/pages/shop/bar_tabs.php [L] -RewriteRule ^payment_confirmation$ src/pages/shop/payment_confirmation.php [L] -RewriteRule ^confirm$ src/pages/shop/confirm.php [L] -RewriteRule ^confirm2$ src/pages/shop/confirm2.php [L] - -# === GALLERY PAGES === -RewriteRule ^gallery$ src/pages/gallery/gallery.php [L] -RewriteRule ^create_album$ src/pages/gallery/create_album.php [L] -RewriteRule ^edit_album$ src/pages/gallery/create_album.php [L] -RewriteRule ^view_album$ src/pages/gallery/view_album.php [L] - -# === EVENTS & BLOG PAGES === -RewriteRule ^events$ src/pages/events/events.php [L] -RewriteRule ^blog$ src/pages/blog/blog.php [L] -RewriteRule ^blog_details$ src/pages/blog/blog_details.php [L] -RewriteRule ^best_of_the_eastern_cape_2024$ src/pages/events/best_of_the_eastern_cape_2024.php [L] -RewriteRule ^2025_agm_minutes$ src/pages/events/2025_agm_minutes.php [L] -RewriteRule ^agm_content$ src/pages/events/agm_content.php [L] -RewriteRule ^instapage$ src/pages/events/instapage.php [L] - -# === OTHER PAGES === -RewriteRule ^about$ src/pages/other/about.php [L] -RewriteRule ^contact$ src/pages/other/contact.php [L] -RewriteRule ^privacy_policy$ src/pages/other/privacy_policy.php [L] -RewriteRule ^track-map$ src/pages/track-map.php [L] -RewriteRule ^404$ src/pages/other/404.php [L] -RewriteRule ^account_settings$ src/pages/other/account_settings.php [L] -RewriteRule ^rescue_recovery$ src/pages/other/rescue_recovery.php [L] -RewriteRule ^bush_mechanics$ src/pages/other/bush_mechanics.php [L] -RewriteRule ^indemnity$ src/pages/other/indemnity.php [L] -RewriteRule ^indemnity_waiver$ src/pages/other/indemnity_waiver.php [L] -RewriteRule ^basic_indemnity$ src/pages/other/basic_indemnity.php [L] -RewriteRule ^view_indemnity$ src/pages/other/view_indemnity.php [L] - -# === ADMIN PAGES === -RewriteRule ^admin_members$ src/admin/admin_members.php [L] -RewriteRule ^admin_payments$ src/admin/admin_payments.php [L] -RewriteRule ^admin_web_users$ src/admin/admin_web_users.php [L] -RewriteRule ^admin_events$ src/admin/admin_events.php [L] -RewriteRule ^admin_course_bookings$ src/admin/admin_course_bookings.php [L] -RewriteRule ^admin_camp_bookings$ src/admin/admin_camp_bookings.php [L] -RewriteRule ^admin_trip_bookings$ src/admin/admin_trip_bookings.php [L] -RewriteRule ^admin_visitors$ src/admin/admin_visitors.php [L] -RewriteRule ^admin_efts$ src/admin/admin_efts.php [L] -RewriteRule ^admin_trips$ src/admin/admin_trips.php [L] -RewriteRule ^manage_events$ src/admin/manage_events.php [L] -RewriteRule ^manage_trips$ src/admin/manage_trips.php [L] - -# === API/AJAX ENDPOINTS === -RewriteRule ^fetch_users$ src/api/fetch_users.php [L] -RewriteRule ^fetch_drinks$ src/api/fetch_drinks.php [L] -RewriteRule ^fetch_bar_tabs$ src/api/fetch_bar_tabs.php [L] -RewriteRule ^get_campsites$ src/api/get_campsites.php [L] -RewriteRule ^get_tab_total$ src/api/get_tab_total.php [L] -RewriteRule ^google_validate_login$ src/api/google_validate_login.php [L] - -# === PROCESSORS === -RewriteRule ^validate_login$ src/processors/validate_login.php [L] -RewriteRule ^register_user$ src/processors/register_user.php [L] -RewriteRule ^process_application$ src/processors/process_application.php [L] -RewriteRule ^process_booking$ src/processors/process_booking.php [L] -RewriteRule ^process_camp_booking$ src/processors/process_camp_booking.php [L] -RewriteRule ^process_course_booking$ src/processors/process_course_booking.php [L] -RewriteRule ^process_trip_booking$ src/processors/process_trip_booking.php [L] -RewriteRule ^process_membership_payment$ src/processors/process_membership_payment.php [L] -RewriteRule ^process_payments$ src/processors/process_payments.php [L] -RewriteRule ^process_eft$ src/processors/process_eft.php [L] -RewriteRule ^submit_order$ src/processors/submit_order.php [L] -RewriteRule ^submit_pop$ src/processors/submit_pop.php [L] -RewriteRule ^process_signature$ src/processors/process_signature.php [L] -RewriteRule ^create_bar_tab$ src/processors/create_bar_tab.php [L] -RewriteRule ^update_application$ src/processors/update_application.php [L] -RewriteRule ^update_user$ src/processors/update_user.php [L] -RewriteRule ^upload_profile_picture$ src/processors/upload_profile_picture.php [L] -RewriteRule ^send_reset_link$ src/processors/send_reset_link.php [L] -RewriteRule ^logout$ src/processors/logout.php [L] -RewriteRule ^process_trip$ src/processors/process_trip.php [L] -RewriteRule ^process_event$ src/processors/process_event.php [L] -RewriteRule ^toggle_trip_published$ src/processors/toggle_trip_published.php [L] -RewriteRule ^toggle_event_published$ src/processors/toggle_event_published.php [L] -RewriteRule ^delete_trip$ src/processors/delete_trip.php [L] -RewriteRule ^delete_event$ src/processors/delete_event.php [L] -RewriteRule ^save_album$ src/processors/save_album.php [L] -RewriteRule ^update_album$ src/processors/update_album.php [L] -RewriteRule ^delete_album$ src/processors/delete_album.php [L] -RewriteRule ^delete_photo$ src/processors/delete_photo.php [L] -RewriteRule ^get_album_photos$ src/processors/get_album_photos.php [L] -RewriteRule ^link_membership_user$ src/processors/link_membership_user.php [L] -RewriteRule ^unlink_membership_user$ src/processors/unlink_membership_user.php [L] - -# Blog routes -RewriteRule ^admin_blogs$ src/pages/blog/admin_blogs.php [L] -RewriteRule ^user_blogs$ src/pages/blog/user_blogs.php [L] -RewriteRule ^blog_read$ src/pages/blog/blog_read.php [L] -RewriteRule ^blog_edit$ src/pages/blog/blog_edit.php [L] -RewriteRule ^blog_create$ src/processors/blog/blog_create.php [L] -RewriteRule ^blog_delete$ src/processors/blog/blog_delete.php [L] -RewriteRule ^publish_blog$ src/processors/blog/publish_blog.php [L] -RewriteRule ^blog_unpublish$ src/processors/blog/blog_unpublish.php [L] -RewriteRule ^submit_blog$ src/processors/blog/submit_blog.php [L] -RewriteRule ^upload_blog_image$ src/processors/blog/upload_blog_image.php [L] -RewriteRule ^autosave$ src/processors/blog/autosave.php [L] - - - -php_flag display_errors On -# php_value error_reporting -1 -RedirectMatch 403 ^/\.well-known -Options -Indexes - - - Require all denied - - -ErrorDocument 404 /404.php - - - Require all granted - Require not ip 4.222.252.98 - Require not ip 4.222.252.97 - - - - Order allow,deny - Deny from all - - - -# ALL CUSTOM ENTRIES SHOULD GO ABOVE THIS LINE -# BEGIN IWORX header -# This file was created by InterWorx-CP -# You may modify this file, but any changes made between -# BEGIN IWORX and END IWORX tags may be lost on future -# updates. Additionally, changes NOT made between these -# tags will not be recognized in the SiteWorx interface. -# END IWORX header - -# BEGIN IWORX accesscontrol -# END IWORX accesscontrol - -# BEGIN IWORX errordocs -# END IWORX errordocs - -# BEGIN IWORX mimetypes -# END IWORX mimetypes - -# BEGIN IWORX handlers -# END IWORX handlers - -# BEGIN IWORX charset -# END IWORX charset - -# BEGIN IWORX redirects -# END IWORX redirects - -# BEGIN IWORX phpvars -# END IWORX phpvars - -# BEGIN IWORX dirindex -# END IWORX dirindex - -# BEGIN IWORX hotlink -# END IWORX hotlink - -# BEGIN IWORX passwordprotection -# END IWORX passwordprotection - diff --git a/.user.ini b/.user.ini index af4bd56b..2ed2fb95 100644 --- a/.user.ini +++ b/.user.ini @@ -2,3 +2,27 @@ upload_max_filesize = 64M post_max_size = 64M max_execution_time = 120 + +; --------------------------------------------------------------------------- +; Session hardening. +; Set here rather than only in src/config/session.php because ~25 files call +; session_start() directly without including that bootstrap. Setting it at the +; PHP level means the cookie is hardened no matter which entry point starts +; the session. +; --------------------------------------------------------------------------- +session.cookie_httponly = 1 +session.cookie_samesite = "Lax" +session.use_strict_mode = 1 +session.use_only_cookies = 1 + +; The live site is HTTPS-only. If you need to test over plain HTTP locally, +; comment this line out locally - do not remove it from the deployed copy. +session.cookie_secure = 1 + +; --------------------------------------------------------------------------- +; Never render errors to visitors. They still go to the server error log. +; --------------------------------------------------------------------------- +display_errors = Off +display_startup_errors = Off +log_errors = On +expose_php = Off diff --git a/assets/uploads/.htaccess b/assets/uploads/.htaccess new file mode 100644 index 00000000..5b4b3306 --- /dev/null +++ b/assets/uploads/.htaccess @@ -0,0 +1,19 @@ +# User-uploaded content. Never execute anything in here. +# Defence-in-depth: even if upload validation is bypassed, the file cannot run. + +php_flag engine off + + + php_flag engine off + + +# Strip any handler that could execute these +RemoveHandler .php .phtml .php3 .php4 .php5 .php7 .php8 .phar .pl .py .cgi .sh +RemoveType .php .phtml .php3 .php4 .php5 .php7 .php8 .phar + +# Serve everything here as inert content + + Require all denied + + +Options -Indexes -ExecCGI diff --git a/docs/FUNCTIONS.md b/docs/FUNCTIONS.md new file mode 100644 index 00000000..4d411c11 --- /dev/null +++ b/docs/FUNCTIONS.md @@ -0,0 +1,300 @@ +# Function Reference + +Developer reference for the helpers in `src/config/functions.php` (~99 +functions). This covers the ones you will actually reach for. For how the site +fits together, see [HANDOVER.md](HANDOVER.md). + +Everything here is available after: + +```php +require_once($rootPath . "/src/config/functions.php"); +``` + +--- + +## Database + +### `openDatabaseConnection()` +Returns a **shared** `mysqli` connection for the current request, or `null` if +the database is unreachable. + +```php +$conn = openDatabaseConnection(); +if ($conn === null) { /* handle it */ } +``` + +> **Do not call `$conn->close()` on the result.** The handle is reused across +> every caller in the request; closing it breaks everything that runs after. +> PHP releases it automatically when the request ends. +> +> This used to open a fresh connection per call, across 73 call sites, which +> was exhausting the server's connection limit in production. + +Most helpers below call this internally, so you rarely need it directly. + +### `classes/DatabaseService.php` +A prepared-statement wrapper available as `$db` after including +`connection.php`. Cleaner than raw mysqli, but only lightly used. Prefer it for +new code. + +Methods: `select()`, `selectOne()`, `insert()`, `update()`, `delete()`, +`execute()`, `count()`, `exists()`, `beginTransaction()`, `commit()`, +`rollback()`, `getConnection()`, plus `getLastError()` / `getLastQuery()` for +debugging. + +```php +$rows = $db->select("SELECT * FROM trips WHERE published = ?", [1], "i"); +``` + +--- + +## Authentication and access control + +| Function | Returns | Notes | +|---|---|---| +| `getUserRole()` | `'user'` / `'admin'` / `'superadmin'` / `null` | `null` when logged out | +| `checkAdmin()` | — | **Redirects** to `index.php` unless admin/superadmin | +| `checkSuperAdmin()` | — | **Redirects** unless superadmin | +| `checkUserSession()` | — | **Redirects** to login if not logged in | +| `get_user_info($field)` | mixed | Reads one column from `users` for the logged-in user | + +`checkAdmin()` and friends `exit()` after redirecting — call them at the very +top of a page, before any output. + +```php +require_once(...functions.php); +checkAdmin(); // page below is admin-only +``` + +> `get_user_info()` interpolates `$field` straight into the SQL. Only ever pass +> a hardcoded column name — never user input. + +### Login throttling + +| Function | Purpose | +|---|---| +| `recordLoginAttempt($email, $success = false)` | Log an attempt | +| `countRecentFailedAttempts($email, $minutesBack = 15)` | Recent failures | +| `checkAccountLockout($email)` | `['is_locked' => bool, 'minutes_remaining' => int, ...]` | +| `lockAccount($email, $minutes = 15)` | Lock after repeated failures | +| `unlockAccount($email)` | Manual unlock — use this when a member phones up | + +Five failures in 15 minutes triggers a 15-minute lockout. + +--- + +## CSRF protection + +```php +// In the form + + +// In the processor, before doing anything else +if (!isset($_POST['csrf_token']) || !validateCSRFToken($_POST['csrf_token'])) { + http_response_code(403); + exit(json_encode(['status' => 'error', 'message' => 'Security token validation failed.'])); +} +``` + +- `generateCSRFToken($duration = 3600)` — random, expiring token +- `validateCSRFToken($token)` — **single-use**; validating consumes it + +Tokens are single-use, so a page with several independent forms needs a token +per form. If a form is submitted twice (double-click, back button) the second +attempt fails validation — that is intended. + +**Apply this to every state-changing endpoint.** Roughly 40% currently do. + +--- + +## Input validation + +All return the cleaned value, or `false` if invalid. + +| Function | Checks | +|---|---| +| `validateEmail($email, $maxLength = 254)` | Format and length | +| `validatePhoneNumber($phone)` | 7–20 digits | +| `validateName($name, $min = 2, $max = 100)` | Length and characters | +| `validateDate($date, $format = 'Y-m-d')` | Real date in the given format | +| `validateAmount($amount, $min = 0, $max = 999999.99)` | Money | +| `validateInteger($int, $min = 0, $max = 2147483647)` | Bounded integer | +| `validateSAIDNumber($idNumber)` | SA ID number incl. checksum | +| `sanitizeTextInput($text, $maxLength = 1000)` | Strips dangerous content | + +### `validateFileUpload($file, $fileType = 'document')` + +The **only** correct way to accept an upload. Returns +`['filename' => ..., 'extension' => ..., 'mimeType' => ...]` or `false`. + +```php +$validated = validateFileUpload($_FILES['file'], 'blog_image'); +if ($validated === false) { /* reject */ } +move_uploaded_file($_FILES['file']['tmp_name'], $dir . $validated['filename']); +``` + +Types: `profile_picture`, `blog_image`, `proof_of_payment`, `document`. + +It checks the upload error code, size, an extension **whitelist**, double +extensions (`shell.php.jpg`), MIME type, that images really are images, and +`is_uploaded_file()`. It returns a **random** filename — always use that rather +than the user's original name. + +> An endpoint that skipped this was the most serious vulnerability found in the +> August 2026 audit: it allowed anyone to upload and execute a `.php` file. +> Do not hand-roll upload handling. + +--- + +## Membership + +| Function | Returns | +|---|---| +| `getUserMemberStatus($user_id)` | `true` if member benefits apply — the one to use for gating | +| `getUserMemberStatusPending($user_id)` | `true` if awaiting payment | +| `hasAcceptedIndemnity($user_id)` | Indemnity signed? | +| `getMembershipType($user_id)` | Type from the latest application | +| `getMembershipEndDate($user_id)` | Expiry date | +| `isMembershipExpiringSoon($user_id)` | `true` within 90 days of expiry | +| `validateHonoraryMemberName($first, $last)` | Fuzzy match against a hardcoded list | + +`getUserMemberStatus()` also returns `true` for people **linked** to somebody +else's active membership — that is usually what you want. See HANDOVER.md §6 +for the full rules. + +### Linked memberships (spouse / family) + +| Function | Purpose | +|---|---| +| `linkSecondaryUserToMembership($primary_id, $secondary_id, $relationship = 'spouse')` | Create a link + default permissions. Transactional | +| `getUserMembershipLink($user_id)` | `['has_access' => bool, ...]` for a secondary user | +| `getLinkedSecondaryUsers($primary_id)` | Everyone linked to a primary | +| `unlinkSecondaryUser($link_id, $primary_id)` | Remove a link. Verifies ownership | + +--- + +## Pricing and payments + +| Function | Purpose | +|---|---| +| `calculateProrata()` | Subscription fee for right now (full Mar–Oct, pro-rata Nov–Feb) | +| `getPriceByDescription($description)` | One row from the `prices` table | +| `getAvailableSpaces($trip_id)` | Vehicle capacity minus bookings | +| `generatePaymentRef(string $type, ?int $course_trip_id, int $user_id): string` | Unique reference. `$type` is `SUBS`, `COURSE`, `TRIP`, … | +| `getPaymentAmount($payment_id)` | Amount for a payment | +| `getPaymentLinkByPaymentId($payment_id)` | Stored gateway payment link | + +### iKhokha + +| Function | Purpose | +|---|---| +| `createIkhokhaPayment($payment_id, $amount, $description, $publicRef)` | Creates a payment link and records provider metadata | +| `getIkhokhaTransactionHistory($startDate, $endDate)` | Raw history, used by Admin → Transactions | +| `createPayloadToSign($urlPath, $body)` / `generateSignature($payload, $secret)` | HMAC signing. Used by the webhook — **do not change without testing against live iKhokha callbacks** | + +`$amount` is in **rands**; the function converts to cents internally. + +### EFT + +- `addEFT($eft_id, $booking_id, $user_id, $status, $amount, $description)` +- `addSubsEFT($eft_id, $user_id, $status, $amount, $description)` — subscriptions +- `getEFTDetails($eft_id)`, `getUserIdFromEFT($eft_id)`, `getEftDescription($eft_id)` + +### Dead code + +`processPayment()`, `processMembershipPayment()`, `processPaymentTest()` are +PayFast and are **not called from anywhere live**. `processZeroPayment()` *is* +live — it handles free bookings. + +--- + +## Email + +All go through Mailjet and mostly use stored templates. + +| Function | Goes to | +|---|---| +| `sendVerificationEmail($email, $name, $token)` | New registrant | +| `sendPaymentConfirmation($email, $name, $description)` | Paying member | +| `sendInvoice($email, $name, $eft_id, $amount, $description)` | Member, after booking | +| `sendPOP($fullname, $eft_id, $amount, $description)` | `POP_NOTIFICATION_EMAILS` | +| `sendAdminNotification($subject, $message)` | `NOTIFICATION_ADDR` | +| `sendFinanceNotification($subject, $message)` | `FINANCE_EMAIL` | +| `sendEmail($email, $subject, $message)` | Plain text, no template | + +Template IDs are hardcoded. If a template is edited in Mailjet the ID stays the +same; if it is recreated, the ID changes and the code must be updated. + +--- + +## Tokens and URLs + +### `encryptData($input, $salt)` / `decryptData($encrypted, $salt)` +Used to put record IDs in URLs without exposing raw database IDs: + +```php +$token = encryptData($blog_id, $salt); // link out +$blog_id = intval(decryptData($token, $salt)); // read back +``` + +`$salt` comes from `.env` via `connection.php`. **Changing `SALT` invalidates +every existing link.** + +> This obscures IDs; it is not access control. Always still check ownership. + +### `url($page)` +Maps a page name to a path. See HANDOVER.md §4 for why this and `.htaccess` +must be kept in step. + +--- + +## Audit and logging + +### `auditLog($user_id, $action, $resource_type = null, $resource_id = null, $details = null)` +Writes to the audit table. Use for security-relevant events: + +```php +auditLog($user_id, 'BLOG_AUTOSAVE_DENIED', 'blogs', $blog_id); +``` + +`$user_id` may be `null` for pre-authentication events. Do not put secrets, +passwords or signature values in `$details` — admins can read this table. + +### `progress_log($message, $context = null)` +Appends to `progress.log` in the site root. Used heavily by the payment +webhook. Never throws, so it is safe in error paths. Blocked from web access by +`.htaccess` — keep it that way, it contains payment data. + +### Visitors +`logVisitor()`, `getUserIP()`, `getClientIPAddress()`, `guessCountry()`, +`blockBlacklistedIP()`. `visitor_logs` holds IP addresses tied to user IDs — +personal data under POPIA (HANDOVER.md §9). + +--- + +## Formatting helpers + +| Function | Does | +|---|---| +| `convertDate($dateString)` | `2026-03-15` → `Sun, 15 Mar 2026` | +| `calculateDaysAndNights($start, $end)` | `"3 days 2 nights"` | +| `formatCurrency($amount)` | Rand formatting | +| `getFullName($user_id)` | `"First Last"` | +| `getInitialSurname($user_id)` | `"F.Last"` | +| `getEmail($user_id)` / `getProfilePic($user_id)` | Single lookups | +| `normalizeName($name)` | Lowercase, punctuation stripped — for fuzzy matching | +| `optimizeImage($filePath, $maxWidth = 1920, $maxHeight = 1080)` | Resize in place | + +--- + +## Conventions for new code + +1. **Always use prepared statements.** No string interpolation into SQL. +2. **Check permissions on every endpoint** — logged in *and* allowed to touch + *this* record. Several bugs have come from checking only the first. +3. **CSRF-validate every state-changing POST.** +4. **Route uploads through `validateFileUpload()`.** +5. **Do not close the shared database connection.** +6. **Add new pages to both routing tables** (`.htaccess` and `url()`). +7. **Never echo raw user input** — `htmlspecialchars()` on output. +8. **Keep secrets in `.env`**, never in code. diff --git a/docs/HANDOVER.md b/docs/HANDOVER.md new file mode 100644 index 00000000..c839cbb4 --- /dev/null +++ b/docs/HANDOVER.md @@ -0,0 +1,473 @@ +# 4WDCSA Website — Handover Guide + +**Audience:** the incoming committee, and whoever maintains this site next. +**Last updated:** 2026-08-09 + +This document describes what the site does, how it is put together, how to run +it day to day, and what is known to be wrong with it. It is deliberately blunt +about the rough edges — you are inheriting this, so you should know. + +--- + +## 1. What this is + +The membership site for the 4x4 Club of South Africa. It handles: + +- **Membership** — applications, indemnity signing, payment, annual renewal +- **Bookings** — club trips, driver-training courses, campsites +- **Payments** — card payments via iKhokha, plus EFT with proof-of-payment upload +- **Content** — a member blog, photo galleries, event pages +- **Bar tabs** — an in-clubhouse till system for drinks +- **Track map** — an interactive Leaflet map of the Base 4 track + +It stores real personal data on members and their families. See **§9 Data +protection** before you touch anything. + +--- + +## 2. Technology and hosting + +| Thing | What it is | +|---|---| +| Language | PHP 8 (no framework — plain PHP with includes) | +| Database | MySQL / MariaDB, accessed via `mysqli` | +| Web server | Apache with `mod_rewrite` | +| Hosting | InterWorx / SiteWorx shared hosting | +| Front end | Bootstrap 5, jQuery 3.6, AOS, Slick — no build step | +| Dependencies | Composer (`mailjet`, `phpdotenv`) + a hand-vendored Google API client | +| Email | Mailjet (transactional templates) | +| Card payments | iKhokha | +| Source control | Git | + +There is **no build step and no test suite**. What is in the files is what runs. + +--- + +## 3. Directory layout + +``` +/ Web root +├── index.php Home page +├── header.php Shared header + nav (included by nearly every page) +├── validate_login.php Login handler (email/password AND Google OAuth) +├── .env Secrets. NEVER commit. NEVER deploy over the live one. +├── .htaccess Routing + security rules +├── .user.ini PHP settings (session hardening, error display) +│ +├── src/ +│ ├── config/ +│ │ ├── env.php Loads .env +│ │ ├── connection.php Opens the DB connection ($conn, $db) +│ │ ├── session.php Starts + hardens the session, enforces timeout +│ │ └── functions.php ~99 helper functions. The heart of the app. +│ ├── pages/ Views, grouped by area (auth, memberships, bookings, +│ │ shop, gallery, events, blog, payment, other) +│ ├── processors/ Form POST handlers ("do a thing, then redirect") +│ ├── api/ JSON/AJAX endpoints +│ ├── admin/ Admin-only pages +│ ├── helpers/ notification_helper.php +│ └── logs/ Runtime logs (not web-readable) +│ +├── classes/ +│ └── DatabaseService.php Prepared-statement wrapper (only lightly used) +├── components/ banner.php, insta_footer.php +├── assets/ css / js / images / sass / uploads +├── uploads/ Member uploads: blogs, proofs of payment, signatures +├── mailers/ Email templates +├── google-client/ Vendored Google API client (see §10) +├── scripts/ +│ └── backup.sh Nightly backup — see §8 +└── docs/ This file, FUNCTIONS.md, migrations, feature notes +``` + +--- + +## 4. How a request is routed + +There are **two** routing systems and they do not fully agree. Know this before +you debug a 404. + +1. **`.htaccess`** — ~120 `RewriteRule` lines mapping clean URLs + (`/membership`) onto files (`src/pages/memberships/membership.php`). +2. **The `url()` helper** in `functions.php` — a PHP array mapping page names + to *file paths*. Used by links inside templates. + +Because `url()` returns file paths like `/src/pages/other/about.php`, and +`.htaccess` 301-redirects any `.php` URL to its extensionless form, links +generated by `url()` cost an extra redirect and expose the internal folder +structure in the address bar. + +**If you add a page, you must add it to BOTH.** Forgetting the `.htaccess` entry +gives a 404; forgetting the `url()` entry gives a broken link. + +Long term this should collapse to one table. See §11. + +--- + +## 5. Roles and access control + +Roles live in `users.role` and there are three: + +| Role | Can do | +|---|---| +| `user` | Normal member/visitor | +| `admin` | All admin pages except superadmin-only ones | +| `superadmin` | Everything, including editing the track map | + +Enforced by these helpers (from `functions.php`): + +- `checkAdmin()` — redirects to `index.php` unless admin or superadmin +- `checkSuperAdmin()` — redirects unless superadmin +- `getUserRole()` — returns the role string, or `null` if not logged in +- `checkUserSession()` — redirects to login if not logged in + +**To grant someone admin** there is no UI. Update the database directly: + +```sql +UPDATE users SET role = 'admin' WHERE email = 'person@example.com'; +``` + +> **Do this as part of every handover:** list everyone who currently holds +> elevated access and remove people who have left the committee. +> ```sql +> SELECT user_id, first_name, last_name, email, role +> FROM users WHERE role IN ('admin','superadmin'); +> ``` + +--- + +## 6. Membership lifecycle + +This is the part most likely to generate "why can't I book?" questions. + +### The tables + +| Table | Holds | +|---|---| +| `users` | Login account, name, email, role | +| `membership_application` | The application form: ID numbers, address, family, vehicles, indemnity acceptance and signature | +| `membership_fees` | What was paid, when, and when membership expires | +| `membership_links` | Links a second person (spouse/family) to a primary membership | +| `membership_permissions` | What each linked person is allowed to do | + +### When is someone "a member"? + +`getUserMemberStatus($user_id)` returns `true` if **any** of these hold: + +1. They accepted the indemnity **and** `membership_fees.payment_status = 'PAID'` + **and** today is on or before `membership_end_date`; or +2. `payment_status = 'PENDING RENEWAL'` and today is on or before + `renewal_period_end` (a grace period while they renew); or +3. They are **linked** to somebody else's active membership. + +If a member says they have paid but the site treats them as a non-member, check +those three in order. The usual cause is `payment_status` never moving to +`'PAID'` because a payment webhook did not arrive (see §7). + +### Payment status values + +`PENDING` (default) → `AWAITING PAYMENT` → `PAID`, with `PENDING RENEWAL` used +during the renewal window. + +### Subscription pricing + +`calculateProrata()` decides the fee: + +- **March–October** — full rate +- **November–February** — pro-rata, charged per remaining month + +Both rates come from the `prices` table (`membership_fees` and `pro_rata` rows), +editable via **Admin → Prices**. The function has hardcoded fallbacks +(R2600 full, R220/month) used only if those rows are missing. + +### Honorary members + +`validateHonoraryMemberName()` holds a **hardcoded list of names** in +`functions.php` and matches with 80% fuzzy similarity. To add or remove an +honorary member you must edit that array in the code. Be aware the fuzzy match +can produce false positives. + +--- + +## 7. Payments + +### iKhokha — this is the live card gateway + +1. A booking or subscription creates a row in `payments` with status + `AWAITING PAYMENT`. +2. `createIkhokhaPayment()` asks iKhokha for a payment link and stores it. +3. The member pays on iKhokha's site. +4. iKhokha calls back to **`src/api/ikhokha_webhook.php`**. +5. The webhook verifies an HMAC signature, then marks `payments`, `bookings` or + `membership_fees` as `PAID` and emails confirmations. + +**If a payment shows as paid at iKhokha but not on the site, the webhook did not +arrive or did not verify.** Check `progress.log` in the site root. Admin → +Transactions can pull history directly from iKhokha for reconciliation. + +> The webhook does **not** currently check that the amount paid matches the +> amount owed. It trusts the status. Worth fixing — see §11. + +### EFT + +Members choose EFT, get a reference (`addEFT()` / `addSubsEFT()`), and upload a +proof of payment (`submit_pop.php`, stored in `uploads/pop/`). Notification goes +to the addresses in `POP_NOTIFICATION_EMAILS`. **A human must then confirm the +payment in Admin → EFTs.** This step is manual by design. + +### Zero-amount bookings + +`processZeroPayment()` handles free bookings (e.g. members' free campsite +nights) without going near a payment gateway. + +### PayFast — dead code + +`processPayment()` and `processMembershipPayment()` implement PayFast, but +**every call site is commented out**. PayFast is not in use. The functions and +the `PAYFAST_*` environment variables can be removed once someone confirms +there is no intention to switch back. + +--- + +## 8. Backups + +**This is the single most important operational task.** Before August 2026 there +were no automated backups at all. + +`scripts/backup.sh` dumps the database and archives member uploads. + +### Setup + +```bash +chmod +x scripts/backup.sh +./scripts/backup.sh # run once by hand, confirm it prints SUCCESS +``` + +Then add a nightly cron job (SiteWorx → Cron Jobs). The script deploys with the +site, so it sits inside the web root — adjust `html` if your document root is +named differently: + +``` +30 2 * * * /home/YOURUSER/html/scripts/backup.sh >> /home/YOURUSER/backups/cron.log 2>&1 +``` + +Backups are written to `~/backups/` — a **sibling** of the web root, so they are +not reachable over the web. The script itself is inside the web root but +`.htaccess` blocks `.sh` files from being served. + +### What it does + +- Dumps the full database (`--single-transaction`, so the site is not locked) +- Archives `uploads/` and `assets/uploads/` +- Writes to a `backups/` directory **outside the web root**, so backups can + never be downloaded by a visitor +- **Verifies every dump** — gzip integrity, presence of `CREATE TABLE` + statements, and the `Dump completed` marker. A dump that fails any check is + discarded rather than kept, so a broken backup can never masquerade as a good + one, and a failed run can never overwrite yesterday's good backup +- Keeps 14 days and deletes older ones (`KEEP_DAYS` to change) +- Exits non-zero on failure so cron reports it + +### Two things you must still do + +1. **Copy backups off this server.** A backup that only lives on the machine + it is backing up is not a backup. Download them, or sync them to cloud + storage. +2. **Test a restore at least once.** An untested backup is a guess. + +### Restoring + +```bash +# Database +gunzip < backups/db/DBNAME-YYYY-MM-DD_HHMM.sql.gz | mysql -u USER -p DBNAME + +# Uploaded files +tar -xzf backups/files/uploads-YYYY-MM-DD_HHMM.tar.gz -C /path/to/site/ +``` + +--- + +## 9. Data protection (POPIA) + +**Read this before granting anyone database access.** + +The site stores, for members and in some cases their families: + +- South African ID numbers (member **and** spouse) +- Dates of birth (member, spouse, **and up to three children**) +- Physical and postal addresses, phone numbers, occupations +- Vehicle registrations +- Digital indemnity signatures (images in `uploads/signatures/`) +- Payment records +- IP addresses and page visits (`visitor_logs`) + +Children's personal information carries heightened obligations under POPIA. The +committee becomes the responsible party for all of it. + +**Two known gaps:** + +1. **There is no way to delete a member's data.** No account deletion, no + anonymisation. The privacy policy states data is kept only for "the period of + time in which users have an account with us" — the software cannot currently + honour that. Either build an erasure path or amend the policy. +2. **No Information Officer is named.** POPIA expects one. Designate a + committee role and record it. + +This is a flag, not legal advice. Take it to whoever handles club governance. + +--- + +## 10. Environment variables + +All secrets live in `.env` at the web root. It is **not** in git and must never +be committed. Loaded by `src/config/env.php` via phpdotenv. + +### Required — the site breaks without these + +| Variable | Purpose | +|---|---| +| `DB_HOST` `DB_USER` `DB_PASS` `DB_NAME` | Database connection | +| `SALT` | Key for `encryptData()` / `decryptData()`, used for URL tokens | +| `HOST` | Base URL, used for Google OAuth redirect | +| `MAILJET_API_KEY` `MAILJET_API_SECRET` | Sending all email | +| `MAILJET_FROM_EMAIL` `MAILJET_FROM_NAME` | From address on all email | +| `GOOGLE_CLIENT_ID` `GOOGLE_CLIENT_SECRET` | "Sign in with Google" | + +### Required for payments + +| Variable | Purpose | +|---|---| +| `IKHOKHA_APP_ID` `IKHOKHA_APP_SECRET` | iKhokha API credentials and webhook signature | +| `IKHOKHA_ENDPOINT` | iKhokha payment-link API URL | +| `IKHOKHA_CALLBACK_URL` | Where iKhokha posts payment results. **Must exactly match** the URL used to compute the signature, or every webhook fails | +| `IKHOKHA_SUCCESS_URL` `IKHOKHA_FAILURE_URL` `IKHOKHA_CANCEL_URL` | Where the member is sent afterwards | +| `IKHOKHA_MODE` | `live` or test | +| `IKHOKHA_REQUESTER_URL` | Site URL sent to iKhokha | + +### Notifications + +| Variable | Purpose | +|---|---| +| `ADMIN_EMAIL` | New-registration notifications | +| `NOTIFICATION_ADDR` | General admin notifications | +| `FINANCE_EMAIL` | Payment notifications | +| `POP_NOTIFICATION_EMAILS` | Comma-separated list for proof-of-payment alerts | +| `DEVELOPER_EMAIL` | *Optional.* Extra copy of payment notifications. Leave unset in normal operation | + +> **Handover action:** these should point at **role addresses** +> (`chair@`, `treasurer@`) that the club controls, not at individuals' +> personal addresses. Committees rotate; code should not need editing when +> they do. Note `sendAdminNotification()` in `functions.php` also has a +> person's name hardcoded in it. + +### Unused / legacy + +`PAYFAST_*` (dead — see §7), `ENFORCE_HTTPS` (never read by any code; HTTPS is +now forced in `.htaccess` instead), `APP_ENV`, `APP_DEBUG`, `APP_URL`, +`TIMEZONE` (falls back to `Africa/Johannesburg`). + +--- + +## 11. Known issues + +Ordered roughly by how much they matter. + +### Will bite you eventually + +- **Two route tables** (§4) that drift apart. Adding a page means editing both. +- **The webhook does not verify payment amounts** (§7) — it trusts the status + field from iKhokha. +- **CSRF protection covers roughly 40% of state-changing endpoints.** The + framework is there (`generateCSRFToken()` / `validateCSRFToken()`, correctly + implemented) — it just is not applied everywhere. +- **No null-checking on database failures.** Many helpers do + `$conn = openDatabaseConnection();` then use `$conn` immediately. If the + database is down this produces a fatal error rather than a friendly message. + +### Annoying but stable + +- **The Google API client is installed twice** — once hand-vendored in + `google-client/` (~145 MB, committed to git) and once as an orphan in + `vendor/google/` (~231 MB, not tracked, not referenced by Composer's + autoloader, apparently dead). Both autoloaders load on the login page. + Consolidating onto one Composer install would remove ~376 MB and a class of + future dependency conflicts. +- **`docs/DB_existing schema.sql` is out of date.** It is missing columns that + later migrations added (for example `membership_fees.renewal_period_end`, + which the code queries). Treat the live database as the source of truth. +- **Password reset** has no rate limiting, no password-strength requirement, and + reveals whether an email address is registered. +- **The half-finished restructure.** `validate_login.php` still sits in the web + root rather than `src/processors/` where the routing table expected it. It + works; it is just inconsistent. + +### Deliberate, not bugs + +- `.htaccess` blocks `.log`, `.env`, `.sql`, archives and dotfiles. Do not + "tidy" those rules away. +- `uploads/.htaccess` and `assets/uploads/.htaccess` disable PHP execution. + These are a safety net against upload bypasses — **never remove them**. + Note `.gitignore` contains a bare `.htaccess` rule, so these files were + force-added to git; if you recreate them, use `git add -f`. +- Sessions expire after 30 minutes idle. This is intentional. + +--- + +## 12. Deploying changes + +There is no CI. Deployment is manual. + +**If you have SSH** (check SiteWorx), use rsync — it transfers only what +changed: + +```bash +rsync -avz --delete --exclude-from=.deployignore ./ user@4wdcsa.co.za:~/html/ +``` + +Always dry-run first (`--dry-run`). Excludes must cover at minimum: `.git/`, +`.env`, `uploads/`, `assets/uploads/`, `docs/`, `*.log`, `vendor/google/`. + +**If you only have FTP**, use `lftp mirror` rather than dragging files: + +```bash +lftp -c "open ftp.4wdcsa.co.za; mirror -R --only-newer --delete --parallel=4 ./ /html" +``` + +**Never deploy `.env` or `uploads/`.** The server copies are the real ones. + +After any deploy, load the site once and check the login page works — the +`.htaccess` and `.user.ini` files are the two things most likely to take the +whole site down if a directive is not supported by the host. + +--- + +## 13. When something breaks + +| Symptom | Look at | +|---|---| +| Whole site 500s after a deploy | `.htaccess` — a directive the host rejects. Restore the previous version first, debug second | +| "Too many connections" | Should be fixed (connections are now shared per request), but check `src/logs/db_errors.log` | +| Member paid but shows unpaid | iKhokha webhook — check `progress.log`, then Admin → Transactions | +| Nobody can log in | Session config. Check the site is on HTTPS — cookies are `Secure` and will not be sent over plain HTTP | +| Emails not arriving | Mailjet dashboard first, then `MAILJET_*` values in `.env` | +| Uploads failing | Directory permissions on `uploads/`, and `upload_max_filesize` in `.user.ini` | + +**Logs:** `src/logs/db_errors.log`, `progress.log` (payments), and the Apache +error log via SiteWorx. None are web-readable, by design. + +--- + +## 14. Handover checklist + +Technical work is only part of this. The rest: + +- [ ] **Transfer account ownership**: hosting/InterWorx, domain registrar, + Mailjet, Google Cloud (OAuth), iKhokha merchant, Instagram +- [ ] **Migrate the Git repository.** It currently lives on a private server + belonging to the outgoing developer. Without this the committee has no + source code and no way to deploy +- [ ] **Set up backups** (§8) and verify a restore works +- [ ] **Audit admin/superadmin accounts** (§5) and remove departed members +- [ ] **Repoint notification emails** to role addresses (§10) +- [ ] **Designate an Information Officer** and resolve the deletion gap (§9) +- [ ] **Record who to call** when the site breaks diff --git a/scripts/backup.sh b/scripts/backup.sh new file mode 100755 index 00000000..90b0e73b --- /dev/null +++ b/scripts/backup.sh @@ -0,0 +1,218 @@ +#!/bin/bash +# +# 4WDCSA - Nightly backup +# ============================================================================= +# Dumps the MySQL database and archives member-uploaded files. +# +# WHY THIS EXISTS +# Before this script the only backup was a manual .sql dump left sitting in +# the web root. If the host lost a disk, the club lost its entire membership +# register, payment history and indemnity signatures. +# +# WHAT IT BACKS UP +# 1. The full MySQL database (schema + data) +# 2. uploads/ - proofs of payment, blog images, indemnity signatures +# 3. assets/uploads/ - campsite and gallery images +# +# WHERE BACKUPS GO +# OUTSIDE the web root, so they can never be downloaded by a visitor. +# Default: /backups (i.e. a sibling of the web root) +# +# SETUP +# 1. chmod +x scripts/backup.sh +# 2. Run it once by hand and confirm it reports SUCCESS: +# ./scripts/backup.sh +# 3. Add a nightly cron job (SiteWorx > Cron Jobs), 02:30 daily: +# 30 2 * * * /home/YOURUSER/scripts/backup.sh >> /home/YOURUSER/backups/cron.log 2>&1 +# 4. IMPORTANT: copy the backups off this server periodically. A backup that +# lives only on the machine it is backing up is not a backup. +# +# RESTORING (see docs/HANDOVER.md for the full procedure) +# gunzip < backups/db/4wdcsa-YYYY-MM-DD.sql.gz | mysql -u USER -p DBNAME +# +# ============================================================================= + +set -uo pipefail + +# --- Configuration ---------------------------------------------------------- + +# Web root = the directory containing .env (this script lives in scripts/) +SITE_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +# Backups are written outside the web root by default. Override with BACKUP_DIR. +BACKUP_DIR="${BACKUP_DIR:-$(dirname "$SITE_ROOT")/backups}" + +# How many daily backups to keep before deleting the oldest. +KEEP_DAYS="${KEEP_DAYS:-14}" + +STAMP="$(date +%Y-%m-%d_%H%M)" +DATE_ONLY="$(date +%Y-%m-%d)" + +DB_DIR="$BACKUP_DIR/db" +FILES_DIR="$BACKUP_DIR/files" + +# --- Helpers ---------------------------------------------------------------- + +log() { echo "[$(date '+%Y-%m-%d %H:%M:%S')] $*"; } +fail() { log "ERROR: $*"; cleanup; exit 1; } + +MYSQL_CNF="" +DUMP_ERR="" +DB_PART="" +cleanup() { + # Always remove the temporary credentials file, even on failure + [ -n "$MYSQL_CNF" ] && [ -f "$MYSQL_CNF" ] && rm -f "$MYSQL_CNF" + [ -n "$DUMP_ERR" ] && [ -f "$DUMP_ERR" ] && rm -f "$DUMP_ERR" + # Never leave a half-written or unverified dump lying around - someone + # could later restore from it believing it to be good. + [ -n "$DB_PART" ] && [ -f "$DB_PART" ] && rm -f "$DB_PART" +} +trap cleanup EXIT INT TERM + +# Read a single key out of the .env file. +# Handles optional quotes and ignores comments. +# NOTE: this runs inside $( ), i.e. a subshell - it must not call fail(), +# because exiting a subshell would not stop the script. The caller validates +# the returned value instead. +read_env() { + local key="$1" file="$SITE_ROOT/.env" value + [ -f "$file" ] || return 1 + value="$(grep -E "^[[:space:]]*${key}[[:space:]]*=" "$file" | head -n1 | cut -d= -f2-)" + # strip surrounding whitespace, then surrounding quotes + value="$(echo "$value" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" + value="$(echo "$value" | sed -e 's/^"\(.*\)"$/\1/' -e "s/^'\(.*\)'$/\1/")" + echo "$value" +} + +# --- Preflight -------------------------------------------------------------- + +log "=== 4WDCSA backup starting ===" +log "Site root: $SITE_ROOT" +log "Backup dir: $BACKUP_DIR" + +[ -f "$SITE_ROOT/.env" ] || fail ".env not found at $SITE_ROOT/.env - cannot read database credentials" + +command -v mysqldump >/dev/null 2>&1 || fail "mysqldump not found in PATH. On shared hosting it is sometimes at /usr/bin/mysqldump - adjust PATH in the cron job." + +DB_HOST="$(read_env DB_HOST)" +DB_USER="$(read_env DB_USER)" +DB_PASS="$(read_env DB_PASS)" +DB_NAME="$(read_env DB_NAME)" + +[ -n "$DB_NAME" ] || fail "DB_NAME is empty - could not parse .env" +[ -n "$DB_USER" ] || fail "DB_USER is empty - could not parse .env" + +mkdir -p "$DB_DIR" "$FILES_DIR" || fail "Cannot create $BACKUP_DIR" + +# Backups contain the full member register. Keep them private. +chmod 700 "$BACKUP_DIR" "$DB_DIR" "$FILES_DIR" 2>/dev/null + +# --- Database --------------------------------------------------------------- +# The password is passed via a temporary defaults-file rather than on the +# command line, because command lines are visible to every user on the box +# via `ps`. + +MYSQL_CNF="$(mktemp)" +chmod 600 "$MYSQL_CNF" +cat > "$MYSQL_CNF" <"$DUMP_ERR" | gzip -9 > "$DB_PART" + +DUMP_STATUS="${PIPESTATUS[0]}" + +if [ "$DUMP_STATUS" -ne 0 ]; then + log "mysqldump stderr: $(cat "$DUMP_ERR" 2>/dev/null)" + fail "mysqldump failed with status $DUMP_STATUS" +fi + +# --- Verify the dump is actually usable ------------------------------------- +# A truncated dump is worse than no dump, because it looks like a backup +# succeeded. Verify by content, not by file size: a well-compressing dump can +# legitimately be small, so byte count tells us very little. +# +# Three checks, in order of strength: +# 1. gzip integrity - the archive is not corrupt +# 2. CREATE TABLE - it actually contains schema +# 3. "Dump completed" - mysqldump reached the end and was not killed midway + +gzip -t "$DB_PART" 2>/dev/null || fail "Dump failed gzip integrity check" + +# Single decompression pass, reused for both content checks +DUMP_TEXT="$(gunzip -c "$DB_PART")" || fail "Could not decompress the dump" + +TABLE_COUNT="$(printf '%s' "$DUMP_TEXT" | grep -c '^CREATE TABLE')" +[ "$TABLE_COUNT" -ge 1 ] || fail "Dump contains no CREATE TABLE statements - the database may be empty or the dump failed" + +printf '%s' "$DUMP_TEXT" | tail -5 | grep -q "Dump completed" \ + || fail "Dump is missing the 'Dump completed' marker - it is truncated" + +unset DUMP_TEXT + +# Verified - promote it to the real filename +mv -f "$DB_PART" "$DB_FILE" || fail "Could not move verified dump into place" +DB_PART="" # so the cleanup trap does not try to delete it + +log "Database OK: $(du -h "$DB_FILE" | cut -f1), ${TABLE_COUNT} tables" + +# --- Uploaded files --------------------------------------------------------- +# Member uploads are not in git, so if they are lost they are gone for good. + +FILES_FILE="$FILES_DIR/uploads-${STAMP}.tar.gz" +TAR_TARGETS="" + +[ -d "$SITE_ROOT/uploads" ] && TAR_TARGETS="$TAR_TARGETS uploads" +[ -d "$SITE_ROOT/assets/uploads" ] && TAR_TARGETS="$TAR_TARGETS assets/uploads" + +if [ -n "$TAR_TARGETS" ]; then + log "Archiving uploads:$TAR_TARGETS" + # shellcheck disable=SC2086 + if tar -czf "$FILES_FILE" -C "$SITE_ROOT" $TAR_TARGETS 2>/dev/null; then + gzip -t "$FILES_FILE" 2>/dev/null || fail "Uploads archive failed integrity check" + log "Uploads OK: $(du -h "$FILES_FILE" | cut -f1)" + else + fail "Failed to archive uploads" + fi +else + log "WARNING: no upload directories found - skipping file backup" +fi + +# --- Rotation --------------------------------------------------------------- + +log "Removing backups older than ${KEEP_DAYS} days..." +DELETED_DB="$(find "$DB_DIR" -name '*.sql.gz' -type f -mtime +"$KEEP_DAYS" -print -delete 2>/dev/null | wc -l | tr -d ' ')" +DELETED_FL="$(find "$FILES_DIR" -name '*.tar.gz' -type f -mtime +"$KEEP_DAYS" -print -delete 2>/dev/null | wc -l | tr -d ' ')" +log "Rotated out ${DELETED_DB} database and ${DELETED_FL} upload backups" + +# --- Summary ---------------------------------------------------------------- + +CURRENT_COUNT="$(find "$DB_DIR" -name '*.sql.gz' -type f | wc -l | tr -d ' ')" +TOTAL_SIZE="$(du -sh "$BACKUP_DIR" 2>/dev/null | cut -f1)" + +log "Backups on disk: ${CURRENT_COUNT}, total size ${TOTAL_SIZE}" +log "=== SUCCESS - backup completed for ${DATE_ONLY} ===" + +exit 0 diff --git a/src/admin/add_campsite.php b/src/admin/add_campsite.php index c1ccc35a..5477c3f2 100644 --- a/src/admin/add_campsite.php +++ b/src/admin/add_campsite.php @@ -39,7 +39,7 @@ if (isset($_FILES['thumbnail']) && $_FILES['thumbnail']['error'] !== UPLOAD_ERR_ $uploadDir = "assets/uploads/campsites/"; if (!file_exists($uploadDir)) { - mkdir($uploadDir, 0777, true); + mkdir($uploadDir, 0755, true); } $randomFilename = $validationResult['filename']; diff --git a/src/api/fetch_bar_tabs.php b/src/api/fetch_bar_tabs.php index f803c5c6..28b8c497 100644 --- a/src/api/fetch_bar_tabs.php +++ b/src/api/fetch_bar_tabs.php @@ -4,6 +4,12 @@ require_once($rootPath . "/src/config/session.php"); require_once($rootPath . "/src/config/connection.php"); require_once($rootPath . "/src/config/functions.php"); +// Returns member names and IDs - must not be readable by anonymous visitors. +if (!isset($_SESSION['user_id'])) { + http_response_code(401); + exit(json_encode(['error' => 'Not authorized'])); +} + // Prepare the SQL query to fetch bar tabs along with user details, including user_id $sql = " SELECT bt.tab_id, u.user_id, u.first_name, u.last_name, u.profile_pic diff --git a/src/api/fetch_drinks.php b/src/api/fetch_drinks.php index 5d2d2eab..82a6feda 100644 --- a/src/api/fetch_drinks.php +++ b/src/api/fetch_drinks.php @@ -2,8 +2,14 @@ 'error', 'message' => 'Not authorized.'])); +} + if (isset($_GET['tab_id'])) { $tab_id = (int) $_GET['tab_id']; // Convert to integer diff --git a/src/api/fetch_users.php b/src/api/fetch_users.php index 60f8de70..03e79a5e 100644 --- a/src/api/fetch_users.php +++ b/src/api/fetch_users.php @@ -5,6 +5,12 @@ require_once($rootPath . "/src/config/session.php"); require_once($rootPath . "/src/config/connection.php"); require_once($rootPath . "/src/config/functions.php"); +// Member directory - do not expose the full member list to anonymous visitors. +if (!isset($_SESSION['user_id'])) { + http_response_code(401); + exit(json_encode(['error' => 'Not authorized'])); +} + if ($conn->connect_error) { die(json_encode([])); // Return empty JSON on failure } diff --git a/src/api/get_tab_total.php b/src/api/get_tab_total.php index cae023ad..6f85445b 100644 --- a/src/api/get_tab_total.php +++ b/src/api/get_tab_total.php @@ -1,24 +1,33 @@ 'success', 'total' => $total]); - } else { - echo json_encode(['status' => 'error', 'message' => 'Failed to fetch total.']); - } -} else { - echo json_encode(['status' => 'error', 'message' => 'Missing tab ID.']); +if (!isset($_SESSION['user_id'])) { + http_response_code(401); + exit(json_encode(['status' => 'error', 'message' => 'Not authorized.'])); } -?> +if (!isset($_POST['tab_id'])) { + exit(json_encode(['status' => 'error', 'message' => 'Missing tab ID.'])); +} + +$tab_id = (int) $_POST['tab_id']; + +$stmt = $conn->prepare("SELECT SUM(item_price) AS total FROM bar_transactions WHERE tab_id = ?"); +if (!$stmt) { + exit(json_encode(['status' => 'error', 'message' => 'Failed to fetch total.'])); +} + +$stmt->bind_param("i", $tab_id); +$stmt->execute(); +$result = $stmt->get_result(); +$row = $result->fetch_assoc(); +$stmt->close(); + +$total = $row['total'] ?? 0; + +echo json_encode(['status' => 'success', 'total' => $total]); diff --git a/src/api/ikhokha_webhook.php b/src/api/ikhokha_webhook.php index 61bc27e8..17b569a2 100644 --- a/src/api/ikhokha_webhook.php +++ b/src/api/ikhokha_webhook.php @@ -1,6 +1,4 @@ $expected, - 'received' => $ikSign - ]); - } - exit('Invalid signature'); +// Signature verification is mandatory - there is deliberately no bypass switch. +// Without it, anyone who knows a payment_id can mark an order PAID. +if (!hash_equals($expected, $ikSign)) { + http_response_code(403); + progress_log('iKhokha webhook: signature mismatch'); + if (function_exists('auditLog')) { + // Do not log the signatures themselves - the audit table is readable + // by admins and the expected value is derived from the app secret. + auditLog(null, 'IKHOKHA_SIGNATURE_MISMATCH', 'webhook', null, [ + 'remote_addr' => $_SERVER['REMOTE_ADDR'] ?? null + ]); } -} else { - progress_log('⚠️ IKHOKHA SIGNATURE CHECK BYPASSED'); + exit('Invalid signature'); } /** @@ -289,11 +278,12 @@ if (in_array($normalized, ['PAID', 'SUCCESS', 'COMPLETED', 'SETTLED'], true)) { $subject = "4WDCSA.co.za Payment Confirmation for Payment ID: " . $localPaymentId; progress_log('Payment confirmation sent for payment ID: ' . $localPaymentId); - sendEmail( - 'chrispintoza@gmail.com', - $subject, - nl2br($message) - ); + // Optional extra recipient for payment notifications (was a hardcoded + // personal address). Set DEVELOPER_EMAIL in .env to re-enable. + $devEmail = $_ENV['DEVELOPER_EMAIL'] ?? null; + if (!empty($devEmail)) { + sendEmail($devEmail, $subject, nl2br($message)); + } sendFinanceNotification($subject, nl2br($message)); sendAdminNotification($subject, nl2br($message)); diff --git a/src/api/test_log.php b/src/api/test_log.php deleted file mode 100644 index 3d333f26..00000000 --- a/src/api/test_log.php +++ /dev/null @@ -1,10 +0,0 @@ -thread_id > 0; + } catch (Throwable $e) { + return false; + } + }; + + if ($isUsable($sharedConn)) { + return $sharedConn; + } + + // Prefer the connection connection.php already opened for this request, + // so a page ends up using one connection in total rather than two. + if (isset($GLOBALS['conn']) && $isUsable($GLOBALS['conn'])) { + $sharedConn = $GLOBALS['conn']; + return $sharedConn; + } + + $sharedConn = null; + + // Database connection parameters + $dbhost = $_ENV['DB_HOST'] ?? ''; + $dbuser = $_ENV['DB_USER'] ?? ''; + $dbpass = $_ENV['DB_PASS'] ?? ''; + $dbname = $_ENV['DB_NAME'] ?? ''; // Disable mysqli exceptions mysqli_report(MYSQLI_REPORT_OFF); @@ -26,7 +63,9 @@ function openDatabaseConnection() return null; } - return $conn; + $sharedConn = $conn; + + return $sharedConn; } //function to determine whether membership_end_date is within 3 months from current date where user_id = ?, if so return true, else false @@ -39,7 +78,6 @@ function isMembershipExpiringSoon($user_id) $stmt = $conn->prepare("SELECT membership_end_date FROM membership_fees WHERE user_id = ? LIMIT 1"); if (!$stmt) { - $conn->close(); return false; } @@ -49,7 +87,6 @@ function isMembershipExpiringSoon($user_id) if ($result->num_rows === 0) { $stmt->close(); - $conn->close(); return false; } @@ -59,7 +96,6 @@ function isMembershipExpiringSoon($user_id) $interval = $current_date->diff($membership_end_date); $stmt->close(); - $conn->close(); return ($interval->days <= 90 && $membership_end_date > $current_date); } @@ -110,7 +146,6 @@ function getMembershipType($user_id) $stmt = $conn->prepare("SELECT membership_type FROM membership_applications WHERE user_id = ? ORDER BY id DESC LIMIT 1"); if (!$stmt) { - $conn->close(); return null; } @@ -119,11 +154,9 @@ function getMembershipType($user_id) $stmt->bind_result($membership_type); if ($stmt->fetch()) { $stmt->close(); - $conn->close(); return $membership_type; } else { $stmt->close(); - $conn->close(); return null; } } @@ -203,7 +236,6 @@ function getTripCount() } // Close connection - $conn->close(); // Return the number of rows return $totalTrips; @@ -645,7 +677,6 @@ function getUserMemberStatus($user_id) error_log("No membership application found for user_id: $user_id - checking if linked to another membership"); // Check if user is linked to another user's membership $linkedStatus = getUserMembershipLink($user_id); - $conn->close(); return $linkedStatus['has_access']; } @@ -657,7 +688,6 @@ function getUserMemberStatus($user_id) error_log("User has not accepted indemnity for user_id: $user_id - checking if linked to another membership"); // User hasn't accepted indemnity directly, but check if they're linked to an active membership $linkedStatus = getUserMembershipLink($user_id); - $conn->close(); return $linkedStatus['has_access']; } @@ -678,7 +708,6 @@ function getUserMemberStatus($user_id) error_log("Membership fees not found for user_id: $user_id - checking if linked to another membership"); // No direct membership fees, check if linked $linkedStatus = getUserMembershipLink($user_id); - $conn->close(); return $linkedStatus['has_access']; } @@ -692,19 +721,16 @@ function getUserMemberStatus($user_id) $membership_end_date_obj = DateTime::createFromFormat('Y-m-d', $membership_end_date); if ($payment_status === "PAID" && $current_date <= $membership_end_date_obj) { - $conn->close(); return true; // Direct membership is active }elseif ($payment_status === "PENDING RENEWAL") { $renewal_period_end_obj = DateTime::createFromFormat('Y-m-d', $renewal_period_end); if ($current_date <= $renewal_period_end_obj) { - $conn->close(); return true; // Direct membership is in renewal period } } else { // Direct membership is not active, check if user is linked to another active membership error_log("Direct membership not active for user_id: $user_id - checking linked memberships"); $linkedStatus = getUserMembershipLink($user_id); - $conn->close(); return $linkedStatus['has_access']; } } @@ -998,7 +1024,6 @@ function createIkhokhaPayment($payment_id, $amount, $description, $publicRef) } } - $conn->close(); return $resp; } @@ -1233,7 +1258,6 @@ function processPaymentTest($payment_id, $amount, $description) $stmt->close(); $updateStmt->close(); - $conn->close(); // Redirect to bookings.php with the booking_id parameter header("Location: bookings.php"); @@ -1271,7 +1295,6 @@ function processZeroPayment($payment_id, $amount, $description) ]); $updateStmt->close(); - $conn->close(); // Redirect to bookings.php with the booking_id parameter header("Location: bookings.php"); @@ -1477,7 +1500,6 @@ function checkAdmin() // Close the statement and connection $stmt->close(); - $conn->close(); } function checkSuperAdmin() @@ -1518,7 +1540,6 @@ function checkSuperAdmin() // Close the statement and connection $stmt->close(); - $conn->close(); } function calculateProrata() @@ -1528,7 +1549,6 @@ function calculateProrata() $stmt->execute(); $rows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC); $stmt->close(); - $conn->close(); $monthlyRate = 220.00; $fullRate = 2600.00; @@ -1824,7 +1844,6 @@ function addEFT($eft_id, $booking_id, $user_id, $status, $amount, $description) // Close the statement and connection $stmt->close(); - $conn->close(); } function addSubsEFT($eft_id, $user_id, $status, $amount, $description) @@ -1851,7 +1870,6 @@ function addSubsEFT($eft_id, $user_id, $status, $amount, $description) // Close the statement and connection $stmt->close(); - $conn->close(); } function encryptData($input, $salt) @@ -1917,7 +1935,6 @@ function checkMembershipApplication($user_id) // Close the prepared statement and connection $stmt->close(); - $conn->close(); // Check if the record exists and redirect if ($count > 0) { @@ -1947,7 +1964,6 @@ function checkMembershipApplication2($user_id) // Close the prepared statement and connection $stmt->close(); - $conn->close(); // Check if the record exists and redirect if ($count < 1) { @@ -2562,7 +2578,6 @@ function getCommentCount($page_id) { // Close connections $stmt->close(); - $conn->close(); return $count; } @@ -2859,6 +2874,12 @@ function validateFileUpload($file, $fileType = 'document') { 'maxSize' => 5242880, // 5MB 'description' => 'Profile Picture' ], + 'blog_image' => [ + 'extensions' => ['jpg', 'jpeg', 'png', 'gif', 'webp'], + 'mimeTypes' => ['image/jpeg', 'image/png', 'image/gif', 'image/webp'], + 'maxSize' => 10485760, // 10MB + 'description' => 'Blog Image' + ], 'proof_of_payment' => [ 'extensions' => ['pdf'], 'mimeTypes' => ['application/pdf'], @@ -2943,7 +2964,7 @@ function validateFileUpload($file, $fileType = 'document') { } // ===== CHECK 6: Additional Image Validation (for images) ===== - if (in_array($fileType, ['profile_picture'])) { + if (in_array($fileType, ['profile_picture', 'blog_image'])) { $imageInfo = @getimagesize($file['tmp_name']); if ($imageInfo === false) { error_log("File is not a valid image for type: $fileType"); @@ -3441,7 +3462,6 @@ function linkSecondaryUserToMembership($primary_user_id, $secondary_user_id, $re error_log("linkSecondaryUserToMembership: Primary user member status = " . ($memberStatus ? 'true' : 'false')); if (!$memberStatus) { - $conn->close(); error_log("linkSecondaryUserToMembership: Primary user does not have active membership"); return ['success' => false, 'message' => 'Primary user does not have active membership']; } @@ -3454,7 +3474,6 @@ function linkSecondaryUserToMembership($primary_user_id, $secondary_user_id, $re $userCheck->close(); if ($userResult->num_rows === 0) { - $conn->close(); error_log("linkSecondaryUserToMembership: Secondary user does not exist"); return ['success' => false, 'message' => 'Secondary user does not exist']; } @@ -3467,7 +3486,6 @@ function linkSecondaryUserToMembership($primary_user_id, $secondary_user_id, $re $existingLink->close(); if ($existingResult->num_rows > 0) { - $conn->close(); error_log("linkSecondaryUserToMembership: Users are already linked"); return ['success' => false, 'message' => 'Users are already linked']; } @@ -3519,14 +3537,12 @@ function linkSecondaryUserToMembership($primary_user_id, $secondary_user_id, $re // Commit transaction $conn->commit(); error_log("linkSecondaryUserToMembership: Transaction committed successfully"); - $conn->close(); return ['success' => true, 'message' => 'User successfully linked to membership', 'link_id' => $linkId]; } catch (Exception $e) { error_log("linkSecondaryUserToMembership: Exception - " . $e->getMessage()); $conn->rollback(); - $conn->close(); return ['success' => false, 'message' => 'Failed to create link: ' . $e->getMessage()]; } } @@ -3565,7 +3581,6 @@ function getUserMembershipLink($user_id) if ($result->num_rows > 0) { $link = $result->fetch_assoc(); - $conn->close(); return [ 'has_access' => true, 'primary_user_id' => $link['primary_user_id'], @@ -3573,7 +3588,6 @@ function getUserMembershipLink($user_id) ]; } - $conn->close(); return ['has_access' => false, 'primary_user_id' => null, 'relationship' => null]; } @@ -3616,7 +3630,6 @@ function getLinkedSecondaryUsers($primary_user_id) $linkedUsers[] = $row; } - $conn->close(); return $linkedUsers; } @@ -3642,13 +3655,11 @@ function unlinkSecondaryUser($link_id, $primary_user_id) $linkCheck->close(); if ($linkResult->num_rows === 0) { - $conn->close(); return ['success' => false, 'message' => 'Link not found']; } $linkData = $linkResult->fetch_assoc(); if ($linkData['primary_user_id'] !== $primary_user_id) { - $conn->close(); return ['success' => false, 'message' => 'Unauthorized: you do not have permission to remove this link']; } @@ -3670,13 +3681,11 @@ function unlinkSecondaryUser($link_id, $primary_user_id) // Commit transaction $conn->commit(); - $conn->close(); return ['success' => true, 'message' => 'User successfully unlinked from membership']; } catch (Exception $e) { $conn->rollback(); - $conn->close(); return ['success' => false, 'message' => 'Failed to remove link: ' . $e->getMessage()]; } } @@ -3698,7 +3707,6 @@ function getPaymentLinkByPaymentId($payment_id) $stmt = $conn->prepare("SELECT payment_link FROM payments WHERE payment_id = ? LIMIT 1"); if (!$stmt) { - $conn->close(); return null; } @@ -3707,7 +3715,6 @@ function getPaymentLinkByPaymentId($payment_id) $stmt->bind_result($payment_link); $found = $stmt->fetch(); $stmt->close(); - $conn->close(); if ($found) { return $payment_link; @@ -3733,7 +3740,6 @@ function getMembershipEndDate($user_id) $stmt = $conn->prepare("SELECT membership_end_date FROM membership_fees WHERE user_id = ? LIMIT 1"); if (!$stmt) { - $conn->close(); return null; } @@ -3742,7 +3748,6 @@ function getMembershipEndDate($user_id) $stmt->bind_result($membership_end_date); $found = $stmt->fetch(); $stmt->close(); - $conn->close(); if ($found) { return $membership_end_date; diff --git a/src/config/session.php b/src/config/session.php index 4e39f67e..bce9043d 100644 --- a/src/config/session.php +++ b/src/config/session.php @@ -1,6 +1,39 @@ 0, // expires when the browser closes + 'path' => '/', + 'httponly' => true, // not readable from JavaScript (blocks XSS session theft) + 'secure' => $isHttps, // only sent over HTTPS when available + 'samesite' => 'Lax', // blocks cross-site POST CSRF while keeping normal links working + ]); + + session_start(); +} + +// Enforce the idle timeout that login sets but nothing previously checked. +if (isset($_SESSION['user_id'], $_SESSION['login_time'])) { + $timeout = $_SESSION['session_timeout'] ?? 1800; // 30 minutes + $lastSeen = $_SESSION['last_activity'] ?? $_SESSION['login_time']; + + if ((time() - $lastSeen) > $timeout) { + $_SESSION = []; + session_destroy(); + session_start(); + $_SESSION['message'] = 'Your session expired. Please log in again.'; + } else { + $_SESSION['last_activity'] = time(); + } +} function Message(){ if(isset($_SESSION["message"])){ @@ -12,4 +45,4 @@ function Message(){ } } -?> \ No newline at end of file +?> diff --git a/src/pages/shop/add_to_cart.php b/src/pages/shop/add_to_cart.php index be1334c4..92d8f2e1 100644 --- a/src/pages/shop/add_to_cart.php +++ b/src/pages/shop/add_to_cart.php @@ -1,36 +1,66 @@ - $item_id, - 'item_name' => $item_name, - 'item_price' => $item_price, - 'user_id' => $user_id - ]; - - echo json_encode(['status' => 'success', 'cart' => $_SESSION['cart']]); -} else { - echo json_encode(['status' => 'error', 'message' => 'Missing required parameters.']); +// Bar tabs are staff/member operated - never anonymous. +if (!isset($_SESSION['user_id'])) { + http_response_code(401); + exit(json_encode(['status' => 'error', 'message' => 'Not authorized.'])); } -?> + +if (!isset($_POST['tab_id'], $_POST['item_id'], $_POST['user_id'])) { + exit(json_encode(['status' => 'error', 'message' => 'Missing required parameters.'])); +} + +$tab_id = (int) $_POST['tab_id']; +$item_id = (int) $_POST['item_id']; +$user_id = (int) $_POST['user_id']; + +if ($tab_id <= 0 || $item_id <= 0 || $user_id <= 0) { + exit(json_encode(['status' => 'error', 'message' => 'Invalid parameters.'])); +} + +// Look the price and name up server-side. They were previously taken straight +// from POST, which let a caller set any price (including negative) and charge +// the drink to any member's tab. +$stmt = $conn->prepare("SELECT description, price FROM bar_items WHERE item_id = ? LIMIT 1"); +$stmt->bind_param("i", $item_id); +$stmt->execute(); +$item = $stmt->get_result()->fetch_assoc(); +$stmt->close(); + +if (!$item) { + exit(json_encode(['status' => 'error', 'message' => 'Unknown item.'])); +} + +// Confirm the tab exists before charging anything to it +$stmt = $conn->prepare("SELECT tab_id FROM bar_tabs WHERE tab_id = ? LIMIT 1"); +$stmt->bind_param("i", $tab_id); +$stmt->execute(); +$tabExists = $stmt->get_result()->num_rows > 0; +$stmt->close(); + +if (!$tabExists) { + exit(json_encode(['status' => 'error', 'message' => 'Unknown tab.'])); +} + +// Initialize cart session if not set +if (!isset($_SESSION['cart'])) { + $_SESSION['cart'] = []; +} +if (!isset($_SESSION['cart'][$tab_id])) { + $_SESSION['cart'][$tab_id] = []; +} + +$_SESSION['cart'][$tab_id][] = [ + 'item_id' => $item_id, + 'item_name' => $item['description'], + 'item_price' => (float) $item['price'], + 'user_id' => $user_id +]; + +echo json_encode(['status' => 'success', 'cart' => $_SESSION['cart']]); diff --git a/src/processors/blog/autosave.php b/src/processors/blog/autosave.php index 3d8e2c8d..ad03bf48 100644 --- a/src/processors/blog/autosave.php +++ b/src/processors/blog/autosave.php @@ -23,16 +23,41 @@ $description = $_POST['subtitle'] ?? ''; $category = $_POST['category'] ?? ''; $user_id = $_SESSION['user_id']; - -// Default to current user -$author_id = $_SESSION['user_id']; - -// Allow override if admin $role = getUserRole(); -if (($role === 'admin' || $role === 'superadmin') && isset($_POST['author'])) { +$isAdmin = in_array($role, ['admin', 'superadmin'], true); + +if ($article_id <= 0) { + http_response_code(400); + echo "Invalid blog ID"; + exit; +} + +// Check permissions: user must be the author, or an admin. +// Without this any logged-in user could overwrite any post by passing its id. +$stmt = $conn->prepare("SELECT author FROM blogs WHERE blog_id = ?"); +$stmt->bind_param("i", $article_id); +$stmt->execute(); +$blog = $stmt->get_result()->fetch_assoc(); +$stmt->close(); + +if (!$blog) { + http_response_code(404); + echo "Blog not found"; + exit; +} + +if ($blog['author'] != $user_id && !$isAdmin) { + auditLog($user_id, 'BLOG_AUTOSAVE_DENIED', 'blogs', $article_id); + http_response_code(403); + echo "Not authorized to edit this blog"; + exit; +} + +// Default to the existing author; only an admin may reassign it +$author_id = $blog['author']; +if ($isAdmin && isset($_POST['author'])) { $author_id = (int)$_POST['author']; } -echo $author_id; $cover_image_path = null; @@ -42,7 +67,7 @@ if (!empty($_FILES['cover_image']['name']) && $_FILES['cover_image']['error'] == // Create directory if it doesn't exist (match working pattern) if (!file_exists($uploadDir)) { - mkdir($uploadDir, 0777, true); + mkdir($uploadDir, 0755, true); } // Simple validation - check extension diff --git a/src/processors/blog/submit_blog.php b/src/processors/blog/submit_blog.php index a0872b6a..3264a57a 100644 --- a/src/processors/blog/submit_blog.php +++ b/src/processors/blog/submit_blog.php @@ -27,7 +27,7 @@ if (isset($_FILES['cover_image']) && $_FILES['cover_image']['error'] === UPLOAD_ // Create directory if it doesn't exist if (!file_exists($upload_dir)) { - mkdir($upload_dir, 0777, true); + mkdir($upload_dir, 0755, true); } // Validate and process the file diff --git a/src/processors/blog/upload_blog_image.php b/src/processors/blog/upload_blog_image.php index bad24ad7..06dba655 100644 --- a/src/processors/blog/upload_blog_image.php +++ b/src/processors/blog/upload_blog_image.php @@ -1,40 +1,71 @@ 'Not authorized'])); +} + if (!isset($_FILES['file'])) { - echo json_encode(['error' => 'No file uploaded']); http_response_code(400); - exit; + exit(json_encode(['error' => 'No file uploaded'])); } // Get blog_id from query parameter -$blog_id = isset($_GET['blog_id']) ? intval($_GET['blog_id']) : null; -if (!$blog_id) { - echo json_encode(['error' => 'Blog ID required']); +$blog_id = isset($_GET['blog_id']) ? intval($_GET['blog_id']) : 0; +if ($blog_id <= 0) { http_response_code(400); - exit; + exit(json_encode(['error' => 'Blog ID required'])); +} + +// Only the author (or an admin) may attach images to a post +$user_id = $_SESSION['user_id']; +$role = getUserRole(); + +$stmt = $conn->prepare("SELECT author FROM blogs WHERE blog_id = ?"); +$stmt->bind_param("i", $blog_id); +$stmt->execute(); +$blog = $stmt->get_result()->fetch_assoc(); +$stmt->close(); + +if (!$blog) { + http_response_code(404); + exit(json_encode(['error' => 'Blog not found'])); +} + +if ($blog['author'] != $user_id && !in_array($role, ['admin', 'superadmin'], true)) { + auditLog($user_id, 'BLOG_IMAGE_UPLOAD_DENIED', 'blogs', $blog_id); + http_response_code(403); + exit(json_encode(['error' => 'Not authorized to upload to this blog'])); +} + +// Validate extension, MIME type, size, and that it is a real image +$validated = validateFileUpload($_FILES['file'], 'blog_image'); +if ($validated === false) { + http_response_code(400); + exit(json_encode(['error' => 'Invalid image file'])); } $targetDir = $rootPath . "/uploads/blogs/" . $blog_id . "/"; -if (!file_exists($targetDir)) { - mkdir($targetDir, 0777, true); +if (!file_exists($targetDir) && !mkdir($targetDir, 0755, true)) { + http_response_code(500); + exit(json_encode(['error' => 'Failed to create upload directory'])); } -$tmp = $_FILES['file']['tmp_name']; -$name = basename($_FILES['file']['name']); -$targetFile = $targetDir . uniqid() . "-" . $name; +// validateFileUpload() returns a random, extension-checked filename +$targetFile = $targetDir . $validated['filename']; -if (move_uploaded_file($tmp, $targetFile)) { - // Return a relative path for the image - $relativePath = "/uploads/blogs/" . $blog_id . "/" . basename($targetFile); +if (move_uploaded_file($_FILES['file']['tmp_name'], $targetFile)) { + $relativePath = "/uploads/blogs/" . $blog_id . "/" . $validated['filename']; echo json_encode(['location' => $relativePath]); } else { - echo json_encode(['error' => 'Failed to move uploaded file']); http_response_code(500); + echo json_encode(['error' => 'Failed to move uploaded file']); } diff --git a/src/processors/create_bar_tab.php b/src/processors/create_bar_tab.php index 35ffbad6..9ab280ff 100644 --- a/src/processors/create_bar_tab.php +++ b/src/processors/create_bar_tab.php @@ -4,6 +4,13 @@ require_once($rootPath . "/src/config/session.php"); require_once($rootPath . "/src/config/connection.php"); require_once($rootPath . "/src/config/functions.php"); +// Must be logged in - this creates a billable tab against a member's account. +if (!isset($_SESSION['user_id'])) { + http_response_code(401); + echo json_encode(['status' => 'error', 'message' => 'Not authorized.']); + exit(); +} + // CSRF Token Validation if (!isset($_POST['csrf_token']) || !validateCSRFToken($_POST['csrf_token'])) { http_response_code(403); diff --git a/src/processors/process_event.php b/src/processors/process_event.php index cc352ab4..fc8b401b 100644 --- a/src/processors/process_event.php +++ b/src/processors/process_event.php @@ -79,7 +79,7 @@ $image_path = null; if (!empty($_FILES['image']['name'])) { $upload_dir = $rootPath . '/assets/images/events/'; if (!file_exists($upload_dir)) { - mkdir($upload_dir, 0777, true); + mkdir($upload_dir, 0755, true); } $file_name = uniqid() . '_' . basename($_FILES['image']['name']); @@ -109,7 +109,7 @@ $promo_path = null; if (!empty($_FILES['promo']['name'])) { $upload_dir = $rootPath . '/assets/images/events/'; if (!file_exists($upload_dir)) { - mkdir($upload_dir, 0777, true); + mkdir($upload_dir, 0755, true); } $file_name = uniqid() . '_promo_' . basename($_FILES['promo']['name']); diff --git a/src/processors/process_signature.php b/src/processors/process_signature.php index f0c6694a..0a2ea8da 100644 --- a/src/processors/process_signature.php +++ b/src/processors/process_signature.php @@ -37,7 +37,7 @@ if (isset($_POST['signature'])) { // Ensure the directory exists if (!file_exists($rootPath . '/uploads/signatures')) { - mkdir($rootPath . '/uploads/signatures', 0777, true); + mkdir($rootPath . '/uploads/signatures', 0755, true); } // Save the image file diff --git a/src/processors/process_trip.php b/src/processors/process_trip.php index 7f4a2901..7a99a75e 100644 --- a/src/processors/process_trip.php +++ b/src/processors/process_trip.php @@ -137,7 +137,7 @@ try { // Create directory if it doesn't exist if (!file_exists($upload_dir)) { - mkdir($upload_dir, 0777, true); + mkdir($upload_dir, 0755, true); } $allowed_extensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; diff --git a/src/processors/save_album.php b/src/processors/save_album.php index 2c8a031e..224e17fc 100644 --- a/src/processors/save_album.php +++ b/src/processors/save_album.php @@ -53,7 +53,7 @@ try { // Create album directory $albumDir = $rootPath . '/assets/uploads/gallery/' . $album_id; if (!file_exists($albumDir)) { - mkdir($albumDir, 0777, true); + mkdir($albumDir, 0755, true); } // Handle cover image upload diff --git a/src/processors/submit_order.php b/src/processors/submit_order.php index dc7ea87d..b282ab26 100644 --- a/src/processors/submit_order.php +++ b/src/processors/submit_order.php @@ -1,9 +1,16 @@ 'error', 'message' => 'Not authorized.']); + exit(); +} + // CSRF Token Validation if (!isset($_POST['csrf_token']) || !validateCSRFToken($_POST['csrf_token'])) { http_response_code(403); diff --git a/src/processors/submit_pop.php b/src/processors/submit_pop.php index 4bc4415b..d894c27f 100644 --- a/src/processors/submit_pop.php +++ b/src/processors/submit_pop.php @@ -45,7 +45,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { // Make sure target directory exists if (!file_exists($target_dir)) { - mkdir($target_dir, 0777, true); + mkdir($target_dir, 0755, true); } if (move_uploaded_file($_FILES['pop_file']['tmp_name'], $target_file)) { diff --git a/src/processors/update_album.php b/src/processors/update_album.php index 0da0c73c..c53f055c 100644 --- a/src/processors/update_album.php +++ b/src/processors/update_album.php @@ -97,7 +97,7 @@ try { // Create directory if it doesn't exist (match working pattern) if (!file_exists($albumDir)) { - mkdir($albumDir, 0777, true); + mkdir($albumDir, 0755, true); } // Delete old cover if it exists @@ -139,7 +139,7 @@ try { // Create directory if it doesn't exist (match working pattern) if (!file_exists($albumDir)) { - mkdir($albumDir, 0777, true); + mkdir($albumDir, 0755, true); } // Get current max display order diff --git a/src/processors/upload_profile_picture.php b/src/processors/upload_profile_picture.php index f3868fff..c4de3660 100644 --- a/src/processors/upload_profile_picture.php +++ b/src/processors/upload_profile_picture.php @@ -45,7 +45,7 @@ if (isset($_FILES['profile_picture']) && $_FILES['profile_picture']['error'] != // Ensure upload directory exists if (!file_exists($target_dir)) { - mkdir($target_dir, 0777, true); + mkdir($target_dir, 0755, true); } // Move the uploaded file diff --git a/uploads/.htaccess b/uploads/.htaccess new file mode 100644 index 00000000..5b4b3306 --- /dev/null +++ b/uploads/.htaccess @@ -0,0 +1,19 @@ +# User-uploaded content. Never execute anything in here. +# Defence-in-depth: even if upload validation is bypassed, the file cannot run. + +php_flag engine off + + + php_flag engine off + + +# Strip any handler that could execute these +RemoveHandler .php .phtml .php3 .php4 .php5 .php7 .php8 .phar .pl .py .cgi .sh +RemoveType .php .phtml .php3 .php4 .php5 .php7 .php8 .phar + +# Serve everything here as inert content + + Require all denied + + +Options -Indexes -ExecCGI diff --git a/validate_login.php b/validate_login.php index e23a6ad3..f6d7d8a0 100644 --- a/validate_login.php +++ b/validate_login.php @@ -57,9 +57,12 @@ if (isset($_GET['code'])) { $stmt->bind_param("sssssi", $email, $first_name, $last_name, $picture, $password, $is_verified); if ($stmt->execute()) { // User successfully registered, set session and redirect + session_regenerate_id(true); $_SESSION['user_id'] = $conn->insert_id; $_SESSION['first_name'] = $first_name; $_SESSION['profile_pic'] = $picture; + $_SESSION['login_time'] = time(); + $_SESSION['session_timeout'] = 1800; // 30 minutes // Send Notification $event = 'user_login'; @@ -81,9 +84,12 @@ if (isset($_GET['code'])) { } else { // User exists, set session and redirect $row = $result->fetch_assoc(); + session_regenerate_id(true); $_SESSION['user_id'] = $row['user_id']; $_SESSION['first_name'] = $row['first_name']; $_SESSION['profile_pic'] = $row['profile_pic']; + $_SESSION['login_time'] = time(); + $_SESSION['session_timeout'] = 1800; // 30 minutes // Send Notification $event = 'user_login';