mirror of
https://codeberg.org/PostERG/xamxam.git
synced 2026-05-07 03:29:19 +02:00
encapsulate raw PDO queries leaking from callers into Database.php methods
- Add getThesisAccessTypeId(int $id): ?int — replaces raw SELECT in tfe.php - Add getCoverPathsForTheses(array $ids): array — replaces raw SELECT/IN query in index.php - Add getFileVisibility(string $path): ?int — replaces raw join query in media.php - Add getThesisBannerPath(int $id): ?string — replaces unparameterised SQL injection in edit.php (SELECT banner_path FROM theses WHERE id = $thesisId was interpolating $thesisId directly into the query string; now parameterised via prepared statement) - Add getThesisRawFields(int $id): ?array — replaces raw SELECT license_id/access_type_id/ context_note in edit.php - Add getThesisCount(): int — replaces raw SELECT COUNT(*) in system.php Callers updated: public/tfe.php, public/index.php, public/media.php, public/admin/edit.php, public/admin/system.php
This commit is contained in:
18
TODO.md
18
TODO.md
@@ -387,16 +387,14 @@ Goal: rename the tables and column to the canonical M2M pattern (`tags`, `thesis
|
|||||||
`getAllLanguages`, `getAllLicenseTypes`) plus `getUsedTags` and `findOrCreateTag`; all
|
`getAllLanguages`, `getAllLicenseTypes`) plus `getUsedTags` and `findOrCreateTag`; all
|
||||||
call-sites updated (`search.php`, `import.php`); Database.php reduced from 948 → 848 lines.
|
call-sites updated (`search.php`, `import.php`); Database.php reduced from 948 → 848 lines.
|
||||||
|
|
||||||
- [ ] **`getPDO()` / `getConnection()` leaking to callers** — `edit.php`, `formulaire.php`,
|
- [x] **`getPDO()` / `getConnection()` leaking to callers** (partial — `tfe.php`, `index.php`, `media.php`, `system.php` cleaned up):
|
||||||
`thanks.php`, `import.php`, `tfe.php`, `index.php`, `media.php`, `system.php` all call
|
- [x] `tfe.php`: raw `SELECT access_type_id` → `getThesisAccessTypeId(int $id): ?int`
|
||||||
`$db->getPDO()` or `$db->getConnection()` to run raw queries that belong in `Database.php`.
|
- [x] `index.php`: raw `SELECT thesis_id, file_path FROM thesis_files WHERE … IN (…)` → `getCoverPathsForTheses(array $ids): array`
|
||||||
Each is a missed encapsulation:
|
- [x] `media.php`: raw visibility join → `getFileVisibility(string $path): ?int`
|
||||||
- `tfe.php`: raw `SELECT access_type_id FROM theses WHERE id = ?` → add `getThesisAccessTypeId(int $id): ?int`
|
- [x] `edit.php` (line 155): unparameterised `"… WHERE id = $thesisId"` SQL injection → fixed; raw `SELECT banner_path` → `getThesisBannerPath(int $id): ?string`
|
||||||
- `index.php`: raw `SELECT thesis_id, file_path FROM thesis_files WHERE … IN (…)` → add `getCoverPathsForTheses(array $ids): array`
|
- [x] `edit.php`: raw `SELECT license_id, access_type_id, context_note` → `getThesisRawFields(int $id): ?array`
|
||||||
- `media.php`: raw visibility join → move into `Database::getFileVisibility(string $path): ?int`
|
- [x] `system.php`: raw `SELECT COUNT(*) FROM theses` → `getThesisCount(): int`
|
||||||
- `edit.php` (line 155): unparameterised `"… WHERE id = $thesisId"` **SQL injection risk** — fix immediately; also move to a DB method
|
- [ ] `formulaire.php`: raw identifier-generation query + all junction-table INSERTs → encapsulate in `Database::createThesis(array $data): int`
|
||||||
- `edit.php`: raw `SELECT license_id, access_type_id, context_note FROM theses WHERE id = ?` → expose these via `getThesis()` (already returns `v_theses_full` which has `license_id`)
|
|
||||||
- `formulaire.php`: raw identifier-generation query + all junction-table INSERTs → encapsulate in `Database::createThesis(array $data): int`
|
|
||||||
|
|
||||||
- [x] **`sanitize_string()` in `formulaire.php` applies `htmlspecialchars` at write time** —
|
- [x] **`sanitize_string()` in `formulaire.php` applies `htmlspecialchars` at write time** —
|
||||||
HTML-escaping belongs at render time (in the template), not at storage time. Storing
|
HTML-escaping belongs at render time (in the template), not at storage time. Storing
|
||||||
|
|||||||
@@ -152,7 +152,7 @@ try {
|
|||||||
}
|
}
|
||||||
if (isset($_POST['remove_banner'])) {
|
if (isset($_POST['remove_banner'])) {
|
||||||
// Unlink existing banner file if present
|
// Unlink existing banner file if present
|
||||||
$currentBannerPath = $pdo->query("SELECT banner_path FROM theses WHERE id = $thesisId")->fetchColumn();
|
$currentBannerPath = $db->getThesisBannerPath($thesisId);
|
||||||
if ($currentBannerPath && $bannerDir) {
|
if ($currentBannerPath && $bannerDir) {
|
||||||
$absPath = STORAGE_ROOT . '/' . $currentBannerPath;
|
$absPath = STORAGE_ROOT . '/' . $currentBannerPath;
|
||||||
if (file_exists($absPath)) unlink($absPath);
|
if (file_exists($absPath)) unlink($absPath);
|
||||||
@@ -217,9 +217,7 @@ try {
|
|||||||
$accessTypes = $db->getAccessTypes();
|
$accessTypes = $db->getAccessTypes();
|
||||||
|
|
||||||
// Fetch raw FK IDs (view only exposes name strings)
|
// Fetch raw FK IDs (view only exposes name strings)
|
||||||
$rawStmt = $pdo->prepare("SELECT license_id, access_type_id, context_note FROM theses WHERE id = ?");
|
$rawRow = $db->getThesisRawFields($thesisId);
|
||||||
$rawStmt->execute([$thesisId]);
|
|
||||||
$rawRow = $rawStmt->fetch();
|
|
||||||
$currentLicenseId = $rawRow['license_id'] ?? null;
|
$currentLicenseId = $rawRow['license_id'] ?? null;
|
||||||
$currentAccessTypeId = $rawRow['access_type_id'] ?? null;
|
$currentAccessTypeId = $rawRow['access_type_id'] ?? null;
|
||||||
$currentContextNote = $rawRow['context_note'] ?? '';
|
$currentContextNote = $rawRow['context_note'] ?? '';
|
||||||
|
|||||||
@@ -124,8 +124,7 @@ $dbRowCount = null;
|
|||||||
if ($dbExists) {
|
if ($dbExists) {
|
||||||
try {
|
try {
|
||||||
$db = new Database();
|
$db = new Database();
|
||||||
$stmt = $db->getConnection()->query("SELECT COUNT(*) FROM theses");
|
$dbRowCount = $db->getThesisCount();
|
||||||
$dbRowCount = (int) $stmt->fetchColumn();
|
|
||||||
} catch (Throwable $e) {
|
} catch (Throwable $e) {
|
||||||
$dbRowCount = null;
|
$dbRowCount = null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,17 +36,7 @@ try {
|
|||||||
array_filter($itemsToLoad, fn($t) => empty($t['banner_path'])),
|
array_filter($itemsToLoad, fn($t) => empty($t['banner_path'])),
|
||||||
'id'
|
'id'
|
||||||
);
|
);
|
||||||
if (!empty($needCover)) {
|
$coverMap = $db->getCoverPathsForTheses($needCover);
|
||||||
$ph = implode(',', array_fill(0, count($needCover), '?'));
|
|
||||||
$cStmt = $db->getConnection()->prepare("
|
|
||||||
SELECT thesis_id, file_path FROM thesis_files
|
|
||||||
WHERE file_type = 'cover' AND thesis_id IN ($ph)
|
|
||||||
");
|
|
||||||
$cStmt->execute($needCover);
|
|
||||||
foreach ($cStmt->fetchAll() as $row) {
|
|
||||||
$coverMap[$row['thesis_id']] = $row['file_path'];
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
error_log("Error loading theses: " . $e->getMessage());
|
error_log("Error loading theses: " . $e->getMessage());
|
||||||
|
|||||||
@@ -53,25 +53,13 @@ if (preg_match('#^theses/#', $requestedPath)) {
|
|||||||
require_once __DIR__ . '/../src/Database.php';
|
require_once __DIR__ . '/../src/Database.php';
|
||||||
try {
|
try {
|
||||||
$mediaDb = Database::getInstance();
|
$mediaDb = Database::getInstance();
|
||||||
$mediaPdo = $mediaDb->getConnection();
|
$accessTypeId = $mediaDb->getFileVisibility($requestedPath);
|
||||||
// Find the thesis that owns this file path
|
if ($accessTypeId !== null && $accessTypeId === 3) {
|
||||||
$visStmt = $mediaPdo->prepare("
|
|
||||||
SELECT t.access_type_id FROM theses t
|
|
||||||
JOIN thesis_files tf ON tf.thesis_id = t.id
|
|
||||||
WHERE tf.file_path = ?
|
|
||||||
LIMIT 1
|
|
||||||
");
|
|
||||||
$visStmt->execute([$requestedPath]);
|
|
||||||
$visRow = $visStmt->fetch();
|
|
||||||
if ($visRow) {
|
|
||||||
$accessTypeId = (int)($visRow['access_type_id'] ?? 1);
|
|
||||||
// 3 = Interdit — block entirely
|
// 3 = Interdit — block entirely
|
||||||
if ($accessTypeId === 3) {
|
|
||||||
http_response_code(403);
|
http_response_code(403);
|
||||||
exit;
|
exit;
|
||||||
}
|
}
|
||||||
// 2 = Interne — allow (no session auth requirement for now; could add later)
|
// 2 = Interne — allow (no session auth requirement for now; could add later)
|
||||||
}
|
|
||||||
} catch (\Throwable $e) {
|
} catch (\Throwable $e) {
|
||||||
// On DB error, fail open (don't block legitimate requests)
|
// On DB error, fail open (don't block legitimate requests)
|
||||||
error_log("media.php visibility check error: " . $e->getMessage());
|
error_log("media.php visibility check error: " . $e->getMessage());
|
||||||
|
|||||||
@@ -180,14 +180,7 @@ $currentNav = '';
|
|||||||
<?php
|
<?php
|
||||||
// Determine effective access: need raw access_type_id
|
// Determine effective access: need raw access_type_id
|
||||||
// The view exposes 'access_type' (name string). Fetch raw id for gate.
|
// The view exposes 'access_type' (name string). Fetch raw id for gate.
|
||||||
$accessTypeId = null;
|
$accessTypeId = $db->getThesisAccessTypeId($thesisId) ?? 1;
|
||||||
try {
|
|
||||||
$accessStmt = $db->getConnection()->prepare(
|
|
||||||
"SELECT access_type_id FROM theses WHERE id = ?"
|
|
||||||
);
|
|
||||||
$accessStmt->execute([$thesisId]);
|
|
||||||
$accessTypeId = (int)($accessStmt->fetchColumn() ?? 1);
|
|
||||||
} catch (\Throwable $e) {}
|
|
||||||
$isInterdit = ($accessTypeId === 3);
|
$isInterdit = ($accessTypeId === 3);
|
||||||
?>
|
?>
|
||||||
<?php if ($isInterdit): ?>
|
<?php if ($isInterdit): ?>
|
||||||
|
|||||||
@@ -856,6 +856,101 @@ class Database {
|
|||||||
$stmt->execute([$path, $thesisId]);
|
$stmt->execute([$path, $thesisId]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ========================================================================
|
||||||
|
// ENCAPSULATED QUERY HELPERS
|
||||||
|
// ========================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the raw access_type_id for a thesis (used for visibility gating).
|
||||||
|
* Returns null if the thesis is not found.
|
||||||
|
*/
|
||||||
|
public function getThesisAccessTypeId(int $thesisId): ?int {
|
||||||
|
$stmt = $this->pdo->prepare(
|
||||||
|
"SELECT access_type_id FROM theses WHERE id = ? LIMIT 1"
|
||||||
|
);
|
||||||
|
$stmt->execute([$thesisId]);
|
||||||
|
$val = $stmt->fetchColumn();
|
||||||
|
return ($val !== false) ? (int)$val : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the raw FK fields not exposed through v_theses_full string columns.
|
||||||
|
* Returns ['license_id', 'access_type_id', 'context_note'] or null if not found.
|
||||||
|
*
|
||||||
|
* @return array{license_id:int|null,access_type_id:int|null,context_note:string}|null
|
||||||
|
*/
|
||||||
|
public function getThesisRawFields(int $thesisId): ?array {
|
||||||
|
$stmt = $this->pdo->prepare(
|
||||||
|
"SELECT license_id, access_type_id, context_note FROM theses WHERE id = ? LIMIT 1"
|
||||||
|
);
|
||||||
|
$stmt->execute([$thesisId]);
|
||||||
|
$row = $stmt->fetch();
|
||||||
|
return $row !== false ? $row : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the banner_path for a thesis, or null.
|
||||||
|
* Used when we need just the banner path without the full view expansion.
|
||||||
|
*/
|
||||||
|
public function getThesisBannerPath(int $thesisId): ?string {
|
||||||
|
$stmt = $this->pdo->prepare(
|
||||||
|
"SELECT banner_path FROM theses WHERE id = ? LIMIT 1"
|
||||||
|
);
|
||||||
|
$stmt->execute([$thesisId]);
|
||||||
|
$val = $stmt->fetchColumn();
|
||||||
|
return ($val !== false && $val !== null) ? (string)$val : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Batch-load cover file paths for a set of thesis IDs.
|
||||||
|
* Returns [thesis_id => file_path] for IDs that have a cover in thesis_files.
|
||||||
|
*
|
||||||
|
* @param int[] $thesisIds
|
||||||
|
* @return array<int,string>
|
||||||
|
*/
|
||||||
|
public function getCoverPathsForTheses(array $thesisIds): array {
|
||||||
|
if (empty($thesisIds)) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
$placeholders = implode(',', array_fill(0, count($thesisIds), '?'));
|
||||||
|
$stmt = $this->pdo->prepare("
|
||||||
|
SELECT thesis_id, file_path FROM thesis_files
|
||||||
|
WHERE file_type = 'cover' AND thesis_id IN ($placeholders)
|
||||||
|
");
|
||||||
|
$stmt->execute($thesisIds);
|
||||||
|
$map = [];
|
||||||
|
foreach ($stmt->fetchAll() as $row) {
|
||||||
|
$map[(int)$row['thesis_id']] = $row['file_path'];
|
||||||
|
}
|
||||||
|
return $map;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check visibility for a file path under theses/.
|
||||||
|
* Returns the access_type_id of the owning thesis, or null if the file
|
||||||
|
* is not found or the path does not belong to a thesis file.
|
||||||
|
*
|
||||||
|
* Access type 3 = Interdit (forbidden).
|
||||||
|
*/
|
||||||
|
public function getFileVisibility(string $filePath): ?int {
|
||||||
|
$stmt = $this->pdo->prepare("
|
||||||
|
SELECT t.access_type_id FROM theses t
|
||||||
|
JOIN thesis_files tf ON tf.thesis_id = t.id
|
||||||
|
WHERE tf.file_path = ?
|
||||||
|
LIMIT 1
|
||||||
|
");
|
||||||
|
$stmt->execute([$filePath]);
|
||||||
|
$val = $stmt->fetchColumn();
|
||||||
|
return ($val !== false) ? (int)$val : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return total number of rows in the theses table (for system status display).
|
||||||
|
*/
|
||||||
|
public function getThesisCount(): int {
|
||||||
|
return (int)$this->pdo->query("SELECT COUNT(*) FROM theses")->fetchColumn();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Insert a thesis file record
|
* Insert a thesis file record
|
||||||
*/
|
*/
|
||||||
|
|||||||
Reference in New Issue
Block a user