Attempt to merge main into our semester work#52
Conversation
GBSA-8329: Replace User Static pages hook up to live Feed
Admin live data
…e db GBSA-8331 connecting pages to the db
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to the CyberCafe system, including a new database seeding script, support ticket schema, and comprehensive backend logic for user, admin, and owner dashboards. However, several critical issues must be addressed before merging. Most notably, a security vulnerability was introduced where all new registrants are assigned the 'owner' role by default. Additionally, multiple files contain unresolved merge conflict markers, and several scripts include debugging code (var_dump), active error display settings, or incorrect redirection paths that will cause runtime failures. Improvements are also needed in error handling to prevent sensitive information exposure in database functions.
| $phone_number = trim($_POST['phone_number'] ?? ''); | ||
| $access_code = trim($_POST['access_code'] ?? ''); | ||
| $user_role = 'user'; | ||
| $user_role = 'owner'; //All New Users are set as owner changed if needed |
There was a problem hiding this comment.
Assigning the 'owner' role to all new registrants is a critical security vulnerability. This allows any user who registers to gain full administrative control over the system. The default role should be 'user'.
$user_role = 'user';| <<<<<<< HEAD | ||
| declare(strict_types=1); | ||
| $pageTitle = "A - Dashboard"; | ||
| ======= | ||
| require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php'; | ||
| require_roles(['admin']); | ||
| // Set the page title dynamically | ||
| $pageTitle = "A - Dashboard"; | ||
| >>>>>>> main |
There was a problem hiding this comment.
Unresolved merge conflict markers found in the code. This will cause a syntax error and prevent the script from executing.
declare(strict_types=1);
require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php';
require_roles(['admin']);
$pageTitle = "A - Dashboard";| <<<<<<< HEAD | ||
| declare(strict_types=1); | ||
|
|
||
| ======= | ||
| require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php'; | ||
| require_roles(['admin']); | ||
| >>>>>>> main |
There was a problem hiding this comment.
Unresolved merge conflict markers found in the code.
declare(strict_types=1);
require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php';
require_roles(['admin']);| <<<<<<< HEAD | ||
| declare(strict_types=1); | ||
|
|
||
| ======= | ||
| require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php'; | ||
| require_roles(['admin']); | ||
| >>>>>>> main |
There was a problem hiding this comment.
Unresolved merge conflict markers found in the code.
declare(strict_types=1);
require_once $_SERVER['DOCUMENT_ROOT'] . '/Website/config/auth.php';
require_roles(['admin']);| echo "<pre style='background:red;color:white;padding:20px;'>"; | ||
| echo "BLOCK URL ERROR: " . $e->getMessage(); | ||
| echo "</pre>"; |
There was a problem hiding this comment.
Directly echoing exception messages to the browser is a security risk as it can reveal sensitive information about the database structure or server environment. Use error_log() for debugging and show a generic error message to the user.
error_log("BLOCK URL ERROR: " . $e->getMessage());| echo '<pre>'; | ||
| var_dump($activeUsers); | ||
| var_dump($bandwidthUsage); | ||
| var_dump($allUser); | ||
| echo '</pre>'; |
There was a problem hiding this comment.
Debugging code (var_dump) should be removed before merging into production to avoid leaking internal data structures to the end user.
| echo '<pre>'; | ||
| var_dump($blockedUrls); | ||
| var_dump($restrictedevices); | ||
| echo '</pre>'; |
There was a problem hiding this comment.
Debugging code (var_dump) should be removed.
|
|
||
| $result = getUserSessionSummary(); | ||
|
|
||
| var_dump($result); |
There was a problem hiding this comment.
Debugging code (var_dump) should be removed.
| ini_set('display_errors', 1); | ||
| error_reporting(E_ALL); |
There was a problem hiding this comment.
Enabling display_errors in production code is a security risk. Error reporting should be handled via logs.
| if ($_SERVER['REQUEST_METHOD'] === 'POST' && !empty($_POST['add_url'])) { | ||
| $url = trim($_POST['add_url']); | ||
| if (!str_starts_with($url, 'http')) $url = 'https://' . $url; | ||
|
|
||
| blockUrl($url, $_SESSION['user_id'] ?? 'ADMIN'); | ||
| $_SESSION['flash'] = [ | ||
| 'type' => 'success', | ||
| 'msg' => 'URL has been blocked: <strong>' . htmlspecialchars($url) . '</strong>' | ||
| ]; | ||
| echo "<script> | ||
| setTimeout(() => { | ||
| window.location.href = window.location.pathname; | ||
| }, 100); | ||
| </script>"; | ||
| exit; | ||
| } |
There was a problem hiding this comment.
POST request handling should be moved to the top of the file, before any HTML output (including the header file). This allows for proper HTTP redirects using header('Location: ...') and exit;, which is more robust and standard than using JavaScript setTimeout for redirection.
4 participants
No description provided.