From f398a0f1ff29320d457330fd18c769da5f06b36c Mon Sep 17 00:00:00 2001 From: Pontoporeia Date: Sun, 31 May 2026 17:49:39 +0200 Subject: [PATCH] Fix non-constant-time credential comparisons - account.php: replace !== CSRF token check with hash_equals - ShareLink::setPassword(): also encrypt and store plain-text password alongside the hash, matching create() behavior so the decrypted_password decoration stays correct after password updates --- TODO.md | 287 +-------------------------- app/public/admin/actions/account.php | 2 +- app/src/ShareLink.php | 5 +- 3 files changed, 8 insertions(+), 286 deletions(-) diff --git a/TODO.md b/TODO.md index ac9a8bc..ded43e2 100644 --- a/TODO.md +++ b/TODO.md @@ -1,284 +1,5 @@ -# Test Coverage (docs/test-plan.md) +# TODO -## Phase 0 — Prerequisites -- [x] 0.1 Install PHPUnit (`composer require --dev phpunit/phpunit ^11`) -- [x] 0.2 Create `phpunit.xml` at project root -- [x] 0.3 Create `tests/bootstrap.php` (autoload classes, define constants) -- [x] 0.4 Create `tests/phpunit/` directory - -## Phase 1 — Pure Logic (no DB, no filesystem, no network) -- [x] 1.1 `CryptoTest.php` — encrypt/decrypt round-trip, isEncrypted, legacy fallback, edge cases -- [x] 1.2 `EmailObfuscatorTest.php` — encode, email, mailto, emailText, obfuscateHtml, edge cases -- [x] 1.3 `SystemControllerHelpersTest.php` — humanBytes, diskColor, logLineClass, nginxLineClass, statusLabel/statusClass -- [x] 1.4 `StudentEmailTest.php` — buildHtml: thesis fields, HTML escaping, missing optional fields -- [x] 1.5 `TfeControllerOgTest.php` — buildOgTags: required keys, image fallback, description truncation - -## Phase 2 — Integration (requires test database) -- [x] 2.0 Setup: `tests/fixtures/`, TestDatabase helper, `.env.test` -- [x] 2.1 `DatabaseExtendedTest.php` — escapeLikeString, buildSearchConditions, findDuplicateThesis, generateThesisIdentifier, getCoverPathsForTheses, findOrCreateAuthor, deduplicateLanguages/renameLanguage/mergeLanguage, renameTag/mergeTag -- [x] 2.2 `ShareLinkExtendedTest.php` — listActive, listArchived, findBySlug, setPassword+getDecryptedPassword round-trip, update -- [x] 2.3 `RateLimitExtendedTest.php` — checkKey per-key counts, getRemaining decrement, getClientIdentifier consistency - -## Phase 3 — Controller Validation -- [x] 3.1 `ThesisCreateValidationTest.php` — valid submission, missing required fields, invalid year, malformed URL, tag dedup, XSS escaping -- [x] 3.2 `ThesisEditValidationTest.php` — load known/404, collectJuryMembers, handleWebsiteUrl normalisation -- [x] 3.3 `AutofocusFieldForErrorTest.php` — correct field per error key, unknown key returns null/default, no CreateController name leak - -## Phase 4 — Cleanup -- [x] 4.1 Migrate 8 existing custom-runner tests to PHPUnit in `tests/phpunit/` -- [x] 4.2 Verify all pass under `vendor/bin/phpunit` -- [x] 4.3 Remove `run-tests.php` and old test files -- [x] 4.4 Add `vendor/bin/phpunit` to justfile/Makefile CI target -- [x] 4.5 Generate baseline coverage report (`--coverage-html coverage/`) — needs Xdebug/PCov -- [x] 4.6 Commit coverage baseline - ---- - -# Current tasks - -## De-librairisation — replace custom infrastructure with off-the-shelf libraries -- [x] Write docs/de-librairisation.md strategy document -- [x] Create composer.json with league/commonmark, guzzlehttp/guzzle, phpmailer/phpmailer -- [x] composer install (prod + dev deps) -- [x] Wire vendor/autoload.php into app/bootstrap.php -- [x] Update phpstan.neon (scanDirectories replaces manual Parsedown scan) -- [x] Write docs/system-setup.md (PHP extension requirements) -- [x] Phase 1: Replace Parsedown with league/commonmark (4 call sites) -- [x] Phase 2: Replace PeerTubeService HTTP client with Guzzle -- [x] Phase 3: Replace SmtpRelay SMTP socket with PHPMailer -- [-] Phase 4: Crypto → defuse/php-encryption — DEFERRED (current AES-256-GCM impl is correct; migration risk > reward) - -## justfile: combine phpstan + cs-check + cs-fix into lint-php -- [x] Merge phpstan, cs-check, cs-fix into single lint-php recipe with backward-compat aliases -- [x] Run lint-php + cs-fix, fix all fixable issues (4 real bugs + CS formatting + regenerated baseline) - -## Récapitulatif admin: fieldset + table fichiers -- [x] Convert all sections to fieldsets with legends -- [x] Convert files list to table (recap-files-table) -- [x] Add bottom margins on fieldsets (admin-main--recap class) -- [x] Remove image thumbnails from files table (emoji icon only) -- [x] Fix spurious beforeunload dialog on edit page (FilePond addfile on existing files) - -## Mots-clés fieldset in contenus -- [x] Create contenus-motscles-fragment.php (tags fragment mirroring langues) -- [x] Add Mots-clés fieldset to contenus template (search + table + bulk actions) -- [x] Keep button linking to dedicated tags.php page as backup -- [x] Add "Annuler" cancel button to both langues and mots-clés bulk action bars -- [x] Add max-height:50vh + overflow-y:auto to both table wraps -- [x] Deploy: just deploy -- [x] Fix: .env permission check — removed from generic file-perm loop (was expecting 660, but .env is 640) - -## Tmp file cleanup (stale filepond + _trash) -- [x] Session-based detection: check manifest session_id against PHP session files -- [x] DB-based detection for _trash: check thesis_files row still exists -- [x] Time-based fallback: >2h filepond, >30d trash -- [x] admin cleanup-stats.php: stale vs active breakdown with sizes -- [x] admin cleanup-tmp.php: smart cleanup with detailed JSON response -- [x] admin index: Nettoyer button + dialog with stats and cleanup trigger -- [x] .gitignore: exclude tmp/filepond/* and tmp/_trash/* -- [x] Deploy: just deploy - -## Index page improvements -- [x] Remove 'Mots-clés' button from toolbar -- [x] Move export to bulk selection bar (Exporter CSV + Exporter fichiers buttons) -- [x] Export dispatcher accepts ?ids= for per-selection filtering -- [x] All ExportController/Database export methods accept optional thesisIds -- [x] Graceful error when ZipArchive extension missing (php8.4-zip not installed) -- [x] Move DB export (SQLite download) to paramètres → Maintenance section -- [x] Sticky thead on index table (position: sticky, top: 0, z-index: 5) - -## Dialog & trash page margins -- [x] Add admin-dialog__body CSS rule with padding + margin resets -- [x] Add admin-dialog__stats + admin-dialog__hint classes -- [x] Fix admin-dialog__alert p margins (not-last-child gets bottom margin) -- [x] Add horizontal margins to .admin-main--list > direct children (trash page forms, tables, flash msgs) -- [x] Clean up tmp-cleanup dialog inline styles → CSS classes - -## Deploy exclusions -- [x] Exclude storage/tmp/ (not just filepond/*) to skip _trash dirs with bad perms -- [x] Exclude storage/documents/ and storage/theses/ from rsync deploy - -## Commit history cleanup -- [x] Squash 177→98 commits by merging similar/iterative fixes and immediate follow-ups -- [x] Resolve 206k lines of nested jj conflict markers in acces.php -- [x] Update commit descriptions for squashed groups - -## acces.php conflict marker cleanup -- [x] Remove 206k lines of nested jj conflict markers from acces.php (resolved from clean nzllwsxo base) -- [x] Restore missing features: create-result dialog, locked_year field, auto-generated password UI, file restrictions section, admin TOC wrapper - -## Save fixes (files disappearing on edit/terminer) -- [x] Fix: note_intention deleted on save — handleFilePondSingleFile treats existing DB id as new upload, deletes existing, then can't re-process (integer vs hex mismatch) -- [x] Fix: cover removal now uses trash, same hex-vs-integer guard as note_intention -- [x] Fix: all file deletions now route through deleteThesisFileToTrash (renames to tmp/_trash instead of unlinking) - -## Storage restructure -- [x] Move storage root from theses/ to documents/ (ThesisFileHandler, ThesisEditController, ThesisCreateController, MediaController) -- [x] MediaController: support both theses/ and documents/ prefixes for visibility gate -- [ ] Migration: rename existing theses/ directories to documents/ on disk and update DB paths - -## Relink feature -- [x] Backend: endpoint to browse documents/ directory (file-browser.php with HTMX tree) -- [x] Backend: endpoint to relink an existing file to a thesis (relink.php inserts thesis_files row) -- [x] Frontend: modal with folder browser, triggered by a "Relier" button next to each FilePond pool -- [x] JS: integrate relink button into FilePond UI (XamxamOpenFileBrowser + XamxamRelinkFile) -- [x] CSS: .relink-modal + .file-browser styles in form.css -- [x] Fix: relinked file not appearing in FilePond pool — add file metadata to addFile() options and extensive diag logging -- [x] Fix: addFile called with single object instead of (source, options) — FilePond API mismatch prevented files from loading -- [x] Fix: use type 'limbo' for relinked files so they go through DID_COMPLETE_ITEM_PROCESSING → onprocessfile → syncOrderInput + green checkmark visual -- [x] Fix: change .filepond--file default border from yellow to green (existing files never reach processing-complete state) -- [ ] Migration: rename existing theses/ directories to documents/ on disk and update DB paths - -## Trash policy -- [x] FilePond remove moves to tmp/_trash (already implemented in handleRemove) - -- [x] Fix: partage FilePond asks admin password — shared handler + separate partage endpoints with share_active session gate -- [x] Fix: mots-clé HTMX search — restored tag-search-fragment.php logic lost during fragment architecture refactor -- [x] Generalize pill-search: single fragment endpoint (type=tag|language|supervisor), deduplicate tag & language backends, add jury autocomplete (promoteur·ice interne/externe ULB, lecteur·ice interne/externe) -- [x] Deploy: just deploy (includes new partage/actions/filepond/ + FilepondHandler.php) -- [x] Fix: language pill-search showing mots-clé results — form field name collision; replaced hidden inputs with scoped hx-vals; fixed exclude logic per type -- [x] Add Créer button to jury supervisor autocomplete (removed guard in pill-search-fragment.php) -- [x] Fix: UNIQUE constraint on authors.email — findOrCreateAuthor now checks for existing author by email before inserting; prevents crash when two authors share an email - -# CSS Refactoring (css-methodology-spec.md) - -- [x] Split variables.css into colors.css + typography.css -- [x] Create reset.css (modern-normalize base — matches prior project reset; Tailwind Preflight caused regressions) -- [x] Create base.css (≤ 5 site-wide rules) -- [x] Create utilities.css (sr-only, skip-link, reduced-motion) -- [x] Create components/ (links, focus, forms, tables, dialog, details, media, buttons, badges, toasts, pagination, header, search) -- [x] Create style.css root @import file -- [x] Remove redundant @import url("./variables.css") from page files -- [x] Clean up duplicate status-badge / toast definitions from admin.css (now in components/) -- [x] Update head.php + partage pages to load style.css -- [x] Common.css → backward-compat wrapper importing style.css -- [x] Variables.css → backward-compat wrapper importing colors.css + typography.css -- [x] Update comment references from common.css → component files -- [x] Reverted CSS nesting (native CSS nesting breaks in browsers without support; no build step available) -- [x] Unnest header.css (was missed in original nesting revert — admin nav-left-links rendered vertically) -- [x] reset.css: modern-normalize base (not Tailwind Preflight) to avoid border/list/heading regressions -- [x] search.css: restored !important flags on input (overrides forms.css base selectors) -- [x] acces.php: copy password button now shows toast feedback -- [ ] Verify no visual regressions - -# Current tasks - -- [x] Add ZipArchive guard to legacy export-files.php -- [x] Refactor deploy recipe: split into deploy-code / deploy-deps / deploy-migrate; deploy-deps patches classmap path (app/src/ → src/) for flat server layout before running composer install; only runs install when lockfile checksum changed -- [x] Fix bootstrap.php autoload path: detect vendor/ location (same dir vs parent dir) to work on both flat server layout and nested local dev layout - -- [x] Cleanup modal: list files that will be removed (not just counts) -- [x] Storage restructure: documents/ → {objet}/ (tfe/theses/frart) -- [x] Migration script: move files + update DB paths -- [x] Fix: hardDeleteThesis doesn't prepend STORAGE_ROOT to file_path - -- [x] Sticky thead: fix with border-collapse:separate, CSS class, --sticky-top var, +min-height:50vh on wrappers, +bulk delete for mots-clés -- [x] Edit submit redirects to recapitulatif instead of staying on edit.php -- [x] Mandatory auto-generated passwords on share links (no custom passwords, regenerate-only in edit, rate limit on password gate) -- [x] .gitignore / .ignore: exclude *.db-wal and *.db-shm -- [x] CSS: FilePond pool file block border yellow → green on upload complete -- [x] Move shared fichiers-fragment.php from partage/ to templates/partials/form/ and update all links -- [x] Remove Écriture and Image format types (migration 035 + schema seed + query filter) -- [x] FilePond image previews: use site light colors (--bg-secondary, --text-secondary, --accent-green, --error) -- [x] Edit mode: remove custom file preview list above FilePond pools; use FilePond pools for preexisting files -- [x] Cover + note_intention: add data-existing-files to their FilePond inputs (per-queue-type JSON arrays) -- [x] Remove upload-progress bar at bottom (FilePond handles its own progress) -- [x] Remove upload-progress.js from edit/add/partage page extraJs arrays - -# FilePond Refactor — Merge video/audio into TFE pool - -- [x] A. `fichiers-fragment.php` — Remove separate video/audio pools, merge into TFE; include PeerTube in data-existing-files -- [x] B. `file-upload-filepond.js` — Remove peertube_video/peertube_audio/video/audio from QUEUE_CONFIG, remove acceptedFileTypesPeerTube, remove data-peertube-active logic -- [x] C. `process.php` — When queue_type=tfe and video/audio + PeerTube enabled, upload to PeerTube, return peertube:UUID -- [x] D. `load.php` — Handle peertube DB files: return placeholder SVG blob -- [x] E. `form.php` — Include PeerTube files in existingFilesJsonForTfe for edit mode -- [x] F. `ThesisEditController.php` — Remove separate video/audio/peertube_* handleFilePondQueueFiles calls; also legacy $_FILES path -- [x] G. `ThesisCreateController.php` — Same as F - -# HTMX Fragment Architecture Reorganization - -- [x] Create shared templates `_licence.php` and `_format-website.php` in `templates/partials/form/` -- [x] Create `src/FragmentRenderer.php` helper -- [x] Create `public/admin/fragments/` and `public/partage/fragments/` subdirectories -- [x] Create thin fragment endpoint files (auth + data prep + render shared template) -- [x] Update all hx-post references in templates to point to new `fragments/` paths -- [x] Update `partage/index.php` routing for new fragments -- [x] Keep old fragment files as thin delegates to new `fragments/` for backward compat -- [x] Update nginx config for partage fragment PHP handling - -# Maintenance mode + partage fragment fix - -- [x] `bootstrap.php`: add `/partage` as allowed path prefix in maintenance gate -- [x] `SystemController.php`: update maintenance detail message -- [x] `admin/parametres.php`: always-visible accessibility table (Normal vs Maintenance) -- [x] `admin.css`: `.param-access-table` styles (border-radius via overflow:hidden, green/secondary colours) -- [x] `partage/index.php`: fix fragment routing — `$slug` was `'fragments'` but check used `str_starts_with($slug, 'fragments/')`, causing HTMX fragments to redirect to / (main page) -- [x] Deploy: `just deploy` + `just deploy-nginx` - -## File browser fixes -- [x] Fix: top-folder navigation regex doesn't match bare `documents`/`theses` (requires trailing slash) -- [x] Replace emoji icons (📁📄) with proper SVG icons (folder, pdf, file-archive, text-file) -- [x] Fix relink endpoint: always return JSON (even on errors), guard finfo class, add diagnostic logging -- [x] Fix JS relink error handler to parse JSON error responses - -## Previous items - -- [x] Step 1 — Build 4 PHP endpoints (process.php, revert.php, load.php, remove.php) -- [x] Step 2 — Update ThesisFileHandler to accept file_ids instead of $_FILES -- [x] Step 3 — Update file-upload-filepond.js (async server model + all fixes) -- [x] Step 4 — Update templates (data-queue-type on all inputs, data-existing-files in edit) -- [x] Step 5 — Update upload-progress.js (new collectFileNames, pending-uploads guard) -- [ ] Step 6 — QA / integration testing -- [x] Logs accessible via Paramètres: app, admin, error, audit tabs (JSON parsed to readable lines), nginx tabs kept -- [x] Remove nginx config tab and PHP-FPM error log tab from UI -- [ ] Step 7 — Cleanup: remove transition flags, remove INPUT_ID_TO_TYPE - - - -# CSP & Deploy Fixes (May 2026) - -- [x] Track vendor JS files in jj (they were moved to vendor/ but never `jj file track`ed) -- [x] Add `script-src 'self' 'unsafe-inline'` to main CSP header (public pages use inline scripts + onclick handlers) -- [x] Add `storage/tmp/filepond/*` to .gitignore + rsync exclude, with .gitkeep -- [ ] Deploy: `just deploy` to sync vendor JS files + updated CSP + .gitkeep to server - -## Export: LINK.txt for PeerTube files -- [x] Add `getPeerTubeInstanceUrl()` helper to ExportController -- [x] In `createExportZip()`: collect PeerTube files, generate LINK.txt per thesis directory -- [x] Augment manifest.json with `peertube_links` array per thesis -- [x] Update docs/export.md with LINK.txt and peertube_links documentation - -# improvements_postlaunch — Année verrouillable dans partage + correction ID - -## Implémentation - -### 1. Schema: ajouter locked_year aux share_links -- [ ] `Database::runMigrations()`: ALTER TABLE share_links ADD COLUMN locked_year INTEGER -- [ ] `app/storage/schema.sql`: ajouter la colonne - -### 2. ShareLink model: lire/écrire locked_year -- [ ] `ShareLink::create()`: accepter et stocker locked_year -- [ ] `ShareLink::update()`: accepter et stocker locked_year -- [ ] `findBySlug()` retourne déjà SELECT *, donc locked_year remonte automatiquement - -### 3. Admin UI — Dialog de création de lien -- [ ] Ajouter champ "Année académique verrouillée" dans create-dialog (acces.php) -- [ ] Ajouter champ dans edit-dialog (acces.php) - -### 4. Admin UI — Liste des liens -- [ ] Afficher colonne "Année" dans le tableau des liens (acces.php) - -### 5. Admin actions (acces-etudiante.php) -- [ ] Lire locked_year depuis $_POST dans action 'create' et 'update' -- [ ] Passer au ShareLink model - -### 6. Partage — Formulaire -- [ ] `partage/index.php` (`renderShareLinkForm`): lire locked_year depuis le lien -- [ ] `fieldset-academic.php`: quand $lockedYear est défini → hidden input + span "Année académique verrouillée : YYYY" + explication; quand null → comportement actuel -- [ ] `ThesisCreateController::validateAndSanitise()`: respecter locked_year si présent dans POST (priorité sur $_POST['année']) - -### 7. Admin edit.php — Forcer l'identifiant -- [ ] Ajouter un champ "Identifiant" en lecture seule mais avec un bouton "Regénérer" -- [ ] `ThesisEditController`: ajouter méthode `regenerateIdentifier()` qui reconstruit YYYY-NNN avec MAX+1 sur la nouvelle année -- [ ] `Database`: méthode `regenerateThesisIdentifier(int $thesisId, int $year)` — met à jour identifier basé sur l'année dans un SELECT FOR UPDATE -- [ ] Attention: renommer les dossiers de fichiers sur disque si l'identifiant change +- [x] Fix `account.php`: replace `!==` CSRF token check with `hash_equals` (constant-time comparison) +- [x] Fix `ShareLink::setPassword()`: also encrypt and store plain-text password, matching `create()` behavior +- [x] Audit: confirm all remaining credential comparison sites use constant-time `hash_equals` or `password_verify` diff --git a/app/public/admin/actions/account.php b/app/public/admin/actions/account.php index 57f93de..07cd5ee 100644 --- a/app/public/admin/actions/account.php +++ b/app/public/admin/actions/account.php @@ -16,7 +16,7 @@ error_log('[account.php] ENTRY | method=' . $_SERVER['REQUEST_METHOD'] . ' | act AdminAuth::requireLogin(); // ── CSRF ────────────────────────────────────────────────────────────────────── -if (empty($_SESSION['csrf_token']) || ($_POST['csrf_token'] ?? '') !== $_SESSION['csrf_token']) { +if (empty($_SESSION['csrf_token']) || !hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'] ?? '')) { http_response_code(403); die('Invalid CSRF token.'); } diff --git a/app/src/ShareLink.php b/app/src/ShareLink.php index c2b6543..cdd2a2e 100644 --- a/app/src/ShareLink.php +++ b/app/src/ShareLink.php @@ -211,9 +211,10 @@ class ShareLink public function setPassword(int $id, ?string $password): void { $hash = $password !== null ? password_hash($password, PASSWORD_BCRYPT) : null; + $enc = $password !== null ? Crypto::encrypt($password) : null; $this->db->getConnection()->prepare( - 'UPDATE share_links SET password_hash = ? WHERE id = ?' - )->execute([$hash, $id]); + 'UPDATE share_links SET password_hash = ?, encrypted_password = ? WHERE id = ?' + )->execute([$hash, $enc, $id]); } /**