Compare commits
19 Commits
develop
...
711774425b
| Author | SHA1 | Date | |
|---|---|---|---|
|
711774425b | ||
|
|
b1255bb57a | ||
|
|
25cd6a1ecc | ||
|
|
bb6a4c387b | ||
|
|
793e816f3e | ||
|
|
4d2b5ad62f | ||
|
|
a3f2671eec | ||
| 4603ab2832 | |||
| 99c77eb7b6 | |||
| 68f072ef46 | |||
| e2fbf51e19 | |||
|
|
701a480442 | ||
|
|
5f5afccac0 | ||
|
|
617ee314b3 | ||
|
|
6db955f65c | ||
|
|
1505e84926 | ||
|
|
a95bb6c629 | ||
| 37eafd276c | |||
| de39fe6a3e |
@@ -1 +0,0 @@
|
||||
src/Module/Core/Infrastructure/Console/SeedE2ECommand.php
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -3,7 +3,6 @@
|
||||
/.env.local.php
|
||||
/.env.*.local
|
||||
/config/secrets/dev/dev.decrypt.private.php
|
||||
/config/reference.php
|
||||
/public/bundles/
|
||||
/var/
|
||||
/vendor/
|
||||
|
||||
730
REVIEW.md
730
REVIEW.md
@@ -1,730 +0,0 @@
|
||||
# Review PR `feat/audit-log` — 4e passe
|
||||
|
||||
> Audit complet de la PR audit-log (89 commits, 233 fichiers, +52k/-876 lignes) apres les 3 passes de review deja mergees.
|
||||
> Objectif : faire sortir ce qui reste avant merge dans `main`.
|
||||
> Genere le 2026-04-23.
|
||||
|
||||
**Branche** : `feat/audit-log`
|
||||
**Base** : `main`
|
||||
**Revues anterieures** (deja appliquees dans la branche) :
|
||||
- `bb6a4c3 fix(review) : blockers review PR #9`
|
||||
- `25cd6a1 fix(review) : regression drawers RBAC + race snapshot + stale-data admin`
|
||||
- `b1255bb fix(review) : 3e passe review (HIGH frontend + MEDIUMs)`
|
||||
- `7117744 docs(claude) : refactor CLAUDE.md`
|
||||
|
||||
La branche est globalement solide : les trois miroirs RBAC sont synchronises, le pattern swap-and-clear de l'audit est correctement implemente, la connexion DBAL dediee est bien configuree. Les findings ci-dessous sont incrementaux et ne remettent pas en cause la feature.
|
||||
|
||||
---
|
||||
|
||||
## Table des matieres
|
||||
|
||||
1. [Securite](#1-securite)
|
||||
2. [Bugs silencieux](#2-bugs-silencieux)
|
||||
3. [Violations des regles projet](#3-violations-des-regles-projet)
|
||||
4. [Incoherences de patterns](#4-incoherences-de-patterns)
|
||||
5. [Documentation et configuration](#5-documentation-et-configuration)
|
||||
6. [Frontend et UX](#6-frontend-et-ux)
|
||||
7. [Bonnes pratiques a retenir](#7-bonnes-pratiques-a-retenir)
|
||||
8. [Resume par priorite](#8-resume-par-priorite)
|
||||
|
||||
---
|
||||
|
||||
## 1. Securite
|
||||
|
||||
### 1.1 CRITIQUE — `/api/docs` public en production
|
||||
|
||||
**Fichier** : `config/packages/security.yaml:46`
|
||||
|
||||
```yaml
|
||||
- { path: ^/api/docs, roles: PUBLIC_ACCESS }
|
||||
```
|
||||
|
||||
La documentation Swagger/OpenAPI d'API Platform est accessible sans authentification, quel que soit l'environnement — y compris en production sur `coltura.malio-dev.fr`. Elle expose :
|
||||
|
||||
- la liste complete des endpoints (`/api/audit-logs`, `/api/users/{id}/rbac`, `/api/sites`, etc.)
|
||||
- les schemas de securite (`is_granted('core.audit_log.view')`)
|
||||
- les filtres acceptes par chaque provider (y compris `performed_at[after]`)
|
||||
- la structure des DTOs (`AuditLogOutput`, `UserOutput`...)
|
||||
- les patterns UUID/IDs
|
||||
|
||||
**Pourquoi c'est grave** : un attaquant a une cartographie gratuite de la surface d'attaque. Pour un CRM interne sur DNS public, c'est une fuite d'information inutile. API Platform genere cette doc automatiquement mais rien n'oblige a la rendre publique.
|
||||
|
||||
**Correction** : fermer en prod.
|
||||
|
||||
```yaml
|
||||
# config/packages/security.yaml
|
||||
- { path: ^/login_check, roles: PUBLIC_ACCESS }
|
||||
- { path: ^/api/version, roles: PUBLIC_ACCESS, methods: [GET] }
|
||||
- { path: ^/api/modules, roles: PUBLIC_ACCESS, methods: [GET] }
|
||||
- { path: ^/api/sidebar, roles: PUBLIC_ACCESS, methods: [GET] }
|
||||
- { path: ^/api, roles: IS_AUTHENTICATED_FULLY }
|
||||
# supprimer la ligne "- { path: ^/api/docs, roles: PUBLIC_ACCESS }"
|
||||
```
|
||||
|
||||
Ou conditionner l'acces au debug mode :
|
||||
|
||||
```yaml
|
||||
when@prod:
|
||||
security:
|
||||
access_control:
|
||||
- { path: ^/api/docs, roles: IS_AUTHENTICATED_FULLY }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.2 CRITIQUE — Aucun en-tete de securite HTTP en production
|
||||
|
||||
**Fichier** : `infra/prod/nginx.conf` et `infra/prod/nginx-proxy.conf` (aucune directive `add_header`)
|
||||
|
||||
Le Nginx de prod n'emet aucun des en-tetes de securite standards :
|
||||
|
||||
| En-tete | Role | Present ? |
|
||||
|---------|------|-----------|
|
||||
| `X-Frame-Options: DENY` | anti-clickjacking (pas d'embed iframe) | non |
|
||||
| `X-Content-Type-Options: nosniff` | anti MIME-sniffing | non |
|
||||
| `Referrer-Policy` | limite les fuites dans le Referer | non |
|
||||
| `Content-Security-Policy` | anti-XSS | non |
|
||||
| `Strict-Transport-Security` | force HTTPS | non |
|
||||
|
||||
Le reverse proxy ecoute uniquement sur le port 80 (HTTP), sans redirection 301 vers HTTPS. Combine avec `JWT_COOKIE_SECURE=1` (defaut dans `.env.prod.example`), le cookie ne serait meme pas envoye en HTTP — donc un premier acces HTTP casse le login silencieusement, l'utilisateur croira que l'auth est buggee.
|
||||
|
||||
**Correction minimale dans `nginx-proxy.conf`** (niveau proxy public) :
|
||||
|
||||
```nginx
|
||||
server {
|
||||
listen 80;
|
||||
listen [::]:80;
|
||||
server_name coltura.malio-dev.fr;
|
||||
|
||||
# Redirection HTTPS obligatoire (ajouter un server block HTTPS par ailleurs).
|
||||
# Tant que le TLS n'est pas en place, au minimum poser les en-tetes suivants.
|
||||
add_header X-Frame-Options "DENY" always;
|
||||
add_header X-Content-Type-Options "nosniff" always;
|
||||
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
|
||||
|
||||
# ... reste de la config
|
||||
}
|
||||
```
|
||||
|
||||
Quand TLS est en place, ajouter :
|
||||
|
||||
```nginx
|
||||
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.3 CRITIQUE — `robots.txt` autorise toute l'indexation
|
||||
|
||||
**Fichier** : `frontend/public/robots.txt`
|
||||
|
||||
```
|
||||
User-Agent: *
|
||||
Disallow:
|
||||
```
|
||||
|
||||
La valeur `Disallow:` (vide) signifie "rien n'est interdit" — tous les crawlers peuvent indexer la totalite du site. Pour un outil CRM interne accessible sur un DNS public (`coltura.malio-dev.fr`), c'est un leak inutile : la page de login, les URLs `/admin/*`, les URLs des fiches clients peuvent remonter dans Google.
|
||||
|
||||
**Correction** :
|
||||
|
||||
```
|
||||
User-Agent: *
|
||||
Disallow: /
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.4 IMPORTANT — `performed_at[after|before]` sans typage DBAL → crash 500 sur date malformee
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:182-186`
|
||||
|
||||
```php
|
||||
if (isset($filters['performed_at_after'])) {
|
||||
$qb->andWhere('performed_at >= :performed_at_after')
|
||||
->setParameter('performed_at_after', $filters['performed_at_after']);
|
||||
}
|
||||
```
|
||||
|
||||
La valeur est passee comme chaine brute a DBAL. La colonne `performed_at` est un `timestamptz`. Si un client envoie `?performed_at[after]=not-a-date`, PostgreSQL leve une erreur de cast et l'API retourne une 500. Pas d'injection SQL (le parametre est bien binde), mais :
|
||||
|
||||
- erreur 500 loguee pour chaque mauvaise entree (pollution des logs + bruit pour l'oncall)
|
||||
- DoS tres bas effort : un utilisateur avec `core.audit_log.view` peut envoyer des requetes mal formees en boucle
|
||||
- mauvaise UX : le front recoit une erreur generique au lieu d'un 400 explicite
|
||||
|
||||
**Correction** :
|
||||
|
||||
```php
|
||||
use Doctrine\DBAL\Types\Types;
|
||||
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||
|
||||
if (isset($filters['performed_at_after'])) {
|
||||
try {
|
||||
$after = new \DateTimeImmutable($filters['performed_at_after']);
|
||||
} catch (\Throwable) {
|
||||
throw new BadRequestHttpException('performed_at[after] doit etre une date ISO 8601 valide.');
|
||||
}
|
||||
$qb->andWhere('performed_at >= :performed_at_after')
|
||||
->setParameter('performed_at_after', $after, Types::DATETIMETZ_IMMUTABLE);
|
||||
}
|
||||
// idem pour performed_at_before
|
||||
```
|
||||
|
||||
Cette correction donne en prime un 400 propre avec un message clair.
|
||||
|
||||
---
|
||||
|
||||
### 1.5 IMPORTANT — Clause `ESCAPE` absente du `ILIKE` (filtre `performed_by`)
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:177-180`
|
||||
|
||||
```php
|
||||
$escaped = str_replace(['\\', '%', '_'], ['\\\\', '\%', '\_'], $filters['performed_by']);
|
||||
$qb->andWhere('performed_by ILIKE :performed_by')
|
||||
->setParameter('performed_by', '%'.$escaped.'%');
|
||||
```
|
||||
|
||||
Le commentaire dit : *"`\` est deja le caractere d'echappement LIKE par defaut en PostgreSQL"*. C'est **inexact**. En SQL-standard PostgreSQL, il n'y a pas de caractere d'echappement par defaut pour LIKE/ILIKE : pour echapper `%` ou `_`, il faut soit `LIKE pattern ESCAPE '\'`, soit utiliser un autre caractere (ex: `ESCAPE '|'`).
|
||||
|
||||
En pratique, sur PostgreSQL avec `standard_conforming_strings=on` (defaut depuis 9.1), `\` n'est PAS interprete par LIKE. Donc `'%\_%'` matche la chaine `%\_%` — pas ce qu'on veut. Le filtre est silencieusement casse pour tout nom contenant `_` ou `%`.
|
||||
|
||||
**Test a faire en psql pour confirmer** :
|
||||
```sql
|
||||
SELECT 'admin_backup' ILIKE '%admin\_backup%'; -- t sur PG moderne ? non : f
|
||||
SELECT 'admin_backup' ILIKE '%admin\_backup%' ESCAPE '\'; -- t
|
||||
```
|
||||
|
||||
**Correction** : ajouter explicitement la clause `ESCAPE`.
|
||||
|
||||
```php
|
||||
$qb->andWhere("performed_by ILIKE :performed_by ESCAPE '\\\\'")
|
||||
->setParameter('performed_by', '%'.$escaped.'%');
|
||||
```
|
||||
|
||||
(Les quatre `\` en PHP donnent deux `\` dans le SQL, soit un `\` litteral une fois parse par PostgreSQL.)
|
||||
|
||||
Alternative plus sure : utiliser `position()` au lieu de LIKE.
|
||||
|
||||
```php
|
||||
$qb->andWhere('position(lower(:performed_by) IN lower(performed_by)) > 0')
|
||||
->setParameter('performed_by', $filters['performed_by']);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.6 IMPORTANT — `SiteAwareInjectionProcessor` : bypass silencieux si l'appelant n'est pas une instance de `User`
|
||||
|
||||
**Fichier** : `src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php:64-75`
|
||||
|
||||
```php
|
||||
if (!$this->security->isGranted('sites.bypass_scope')) {
|
||||
$user = $this->security->getUser();
|
||||
$explicitSite = $data->getSite();
|
||||
if ($user instanceof User && $explicitSite instanceof Site && !$user->hasSite($explicitSite)) {
|
||||
throw new AccessDeniedHttpException(...);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Si `$user` n'est pas exactement une instance de `App\Module\Core\Domain\Entity\User` (ex: futur provider d'auth tiers, token systeme), la condition `instanceof User` est fausse et la garde cross-site write est **silencieusement sautee**. L'utilisateur peut alors specifier n'importe quel `site` dans le payload sans verification.
|
||||
|
||||
Aujourd'hui le risque est faible (un seul `app_user_provider` configure). Mais le pattern est fragile : une absence de type doit lever une erreur, pas passer.
|
||||
|
||||
**Correction** : transformer le cas "pas un User" en refus explicite.
|
||||
|
||||
```php
|
||||
if (!$this->security->isGranted('sites.bypass_scope')) {
|
||||
$user = $this->security->getUser();
|
||||
if (!$user instanceof User) {
|
||||
throw new AccessDeniedHttpException('Utilisateur non reconnu pour la validation de site.');
|
||||
}
|
||||
$explicitSite = $data->getSite();
|
||||
if ($explicitSite instanceof Site && !$user->hasSite($explicitSite)) {
|
||||
throw new AccessDeniedHttpException(...);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.7 IMPORTANT — `isHandlingUnauthorized` sans `try/finally` : flag bloque si `navigateTo` throw
|
||||
|
||||
**Fichier** : `frontend/shared/composables/useApi.ts:25,125-130`
|
||||
|
||||
```typescript
|
||||
let isHandlingUnauthorized = false // module-level singleton
|
||||
|
||||
// ...
|
||||
if (!isLoginCheck && !isLogout) {
|
||||
if (!isHandlingUnauthorized) {
|
||||
isHandlingUnauthorized = true
|
||||
auth.clearSession()
|
||||
await navigateTo('/login')
|
||||
isHandlingUnauthorized = false // <-- jamais atteint si navigateTo throw
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Si `navigateTo('/login')` echoue (middleware qui throw, plugin qui throw dans un hook, navigation cancelee par `abortNavigation`), le flag reste `true` **indefiniment**. Toutes les 401 futures sont silencieusement ignorees, l'utilisateur reste sur la page courante avec l'impression que les requetes ne font rien. Le seul remede est un hard-reload.
|
||||
|
||||
**Correction** : `try/finally`.
|
||||
|
||||
```typescript
|
||||
if (!isLoginCheck && !isLogout) {
|
||||
if (!isHandlingUnauthorized) {
|
||||
isHandlingUnauthorized = true
|
||||
try {
|
||||
auth.clearSession()
|
||||
await navigateTo('/login')
|
||||
} finally {
|
||||
isHandlingUnauthorized = false
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 1.8 IMPORTANT — Pagination maximale absente sur `Permission`, `Role`, `Site` (itemsPerPage 999 cote front)
|
||||
|
||||
**Fichiers** :
|
||||
- `frontend/modules/core/components/UserRbacDrawer.vue:235,236`
|
||||
- `frontend/modules/core/components/RoleDrawer.vue:149`
|
||||
- `frontend/modules/sites/pages/admin/sites.vue:117`
|
||||
|
||||
```typescript
|
||||
api.get<{ member: Permission[] }>('/permissions', { orphan: false, itemsPerPage: 999 }, ...)
|
||||
api.get<{ member: Site[] }>('/sites', { itemsPerPage: 999 }, ...)
|
||||
```
|
||||
|
||||
Deux problemes cumules :
|
||||
|
||||
1. **`paginationClientItemsPerPage` n'est pas active** sur les resources `Permission`, `Role`, `Site` (seul `AuditLogResource` l'active). API Platform ignore donc `itemsPerPage=999` et retourne 30 elements par defaut. **Le `999` est un no-op**. Aujourd'hui ca marche parce que ces catalogues comptent <30 entrees, mais quand les modules grandiront, les drawers vont silencieusement tronquer.
|
||||
|
||||
2. **Aucun `paginationMaximumItemsPerPage`** n'est pose sur ces ressources. Si un dev decide d'activer `paginationClientItemsPerPage: true` plus tard, `?itemsPerPage=99999` deviendra une requete valide qui pourra faire suer la DB.
|
||||
|
||||
**Correction** : deux options selon l'intention.
|
||||
|
||||
*Option A — Desactiver la pagination pour ces catalogues* (ils sont small + exhaustifs par nature) :
|
||||
|
||||
```php
|
||||
// Permission.php — GetCollection
|
||||
new GetCollection(
|
||||
normalizationContext: ['groups' => ['permission:read']],
|
||||
security: "...",
|
||||
paginationEnabled: false,
|
||||
),
|
||||
```
|
||||
|
||||
Cote front, retirer `itemsPerPage: 999` (devient inutile).
|
||||
|
||||
*Option B — Garder la pagination avec un plafond explicite* :
|
||||
|
||||
```php
|
||||
new GetCollection(
|
||||
paginationClientItemsPerPage: true,
|
||||
paginationMaximumItemsPerPage: 200,
|
||||
// ...
|
||||
),
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Bugs silencieux
|
||||
|
||||
### 2.1 IMPORTANT — `AuditLogDetail.vue` : `JSON.stringify` sans garde sur valeur non-serialisable
|
||||
|
||||
**Fichier** : `frontend/shared/components/audit/AuditLogDetail.vue` (fonction `formatValue`)
|
||||
|
||||
Si une valeur de `changes` est non-serialisable (objet circulaire, symbol, bigint), `JSON.stringify` throw et casse tout le rendu du drawer. Ce cas est theoriquement impossible avec les donnees ecrites par `AuditListener` aujourd'hui, mais un futur enrichissement (ex: serialisation d'un objet metier complexe) peut introduire ce risque.
|
||||
|
||||
**Correction** : wrapper defensif.
|
||||
|
||||
```typescript
|
||||
function formatValue(value: unknown): string {
|
||||
if (value === null || value === undefined) return 'vide'
|
||||
if (typeof value === 'boolean') return value ? t('common.yes') : t('common.no')
|
||||
if (typeof value === 'object') {
|
||||
try { return JSON.stringify(value) } catch { return '[valeur non serialisable]' }
|
||||
}
|
||||
return String(value)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2.2 MOYEN — `UserRbacProcessor` : payload JSON invalide = regression silencieuse des collections
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php:241-248`
|
||||
|
||||
Le processor parse `$request->getContent()` via `json_decode()` pour savoir quelles cles sont absentes du payload et restaurer les collections qu'API Platform aurait ecrasees. Si le body est un JSON invalide (rare mais possible : content-type incorrect, body vide suite a un intercepteur buggue), `json_decode` retourne `null` et la restauration est `return` sans aucun log.
|
||||
|
||||
Consequence : les collections `rbacRoles`, `directPermissions`, `sites` peuvent etre ecrasees par des tableaux vides sans trace. Bug quasi-impossible a diagnostiquer en prod.
|
||||
|
||||
**Correction** : logger `warning` dans ce cas.
|
||||
|
||||
```php
|
||||
$payload = json_decode($request->getContent(), true);
|
||||
if (!is_array($payload)) {
|
||||
$this->logger->warning('UserRbacProcessor : body JSON invalide, skip de restoreAbsentCollections', [
|
||||
'user_id' => $data->getId(),
|
||||
]);
|
||||
return;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Violations des regles projet
|
||||
|
||||
### 3.1 MOYEN — `<button>` brut au lieu de `MalioButton`
|
||||
|
||||
**Fichier** : `frontend/shared/components/audit/AuditTimeline.vue:80`
|
||||
|
||||
```html
|
||||
<button
|
||||
type="button"
|
||||
class="mt-3 text-sm text-blue-600 hover:text-blue-800"
|
||||
@click="loadMore"
|
||||
>
|
||||
{{ t('audit.timeline.load_more') }}
|
||||
</button>
|
||||
```
|
||||
|
||||
**Regle violee** : `.claude/rules/frontend.md` — *"Tout champ de formulaire / filtre / bouton doit utiliser les composants Malio*"*.
|
||||
|
||||
C'est le seul bouton HTML brut dans la PR. Aucun commentaire `TODO` ne documente une exception.
|
||||
|
||||
**Correction** : utiliser `MalioButton` avec un variant secondaire/link.
|
||||
|
||||
```html
|
||||
<MalioButton
|
||||
type="secondary"
|
||||
size="sm"
|
||||
:label="t('audit.timeline.load_more')"
|
||||
class="mt-3"
|
||||
@click="loadMore"
|
||||
/>
|
||||
```
|
||||
|
||||
Si `MalioButton` ne propose pas de variant "link" adapte, commenter l'exception :
|
||||
|
||||
```html
|
||||
<!-- TODO(malio-ui) : MalioButton n'a pas encore de variant 'link-inline' -->
|
||||
<button ...>
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 3.2 MOYEN — Cle i18n `sidebar.core.sites` sous le mauvais namespace
|
||||
|
||||
**Fichiers** :
|
||||
- `config/sidebar.php:82` : `'label' => 'sidebar.core.sites'`
|
||||
- `frontend/i18n/locales/fr.json:31` : `"core": { "sites": "Sites" }`
|
||||
|
||||
La regle `naming.md` impose `sidebar.<module>.*` pour les cles de sidebar. L'item est declare comme appartenant au module `sites` (`'module' => 'sites'`), la cle i18n devrait donc etre `sidebar.sites.admin` (ou `sidebar.sites.sites` / `sidebar.sites.list`).
|
||||
|
||||
**Correction** :
|
||||
|
||||
```json
|
||||
// frontend/i18n/locales/fr.json
|
||||
"sidebar": {
|
||||
"core": {
|
||||
"roles": "Gestion des rôles",
|
||||
"users": "Utilisateurs",
|
||||
"audit_log": "Journal d'audit"
|
||||
},
|
||||
"sites": {
|
||||
"admin": "Sites"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```php
|
||||
// config/sidebar.php
|
||||
[
|
||||
'label' => 'sidebar.sites.admin',
|
||||
'to' => '/admin/sites',
|
||||
'icon' => 'mdi:domain',
|
||||
'module'=> 'sites',
|
||||
'permission' => 'sites.view',
|
||||
],
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 3.3 MOYEN — `UserPasswordHasherProcessor` et `MeProvider` non `final`
|
||||
|
||||
**Fichiers** :
|
||||
- `src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserPasswordHasherProcessor.php:16`
|
||||
- `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/MeProvider.php:14`
|
||||
|
||||
Ce sont les deux seules classes ApiPlatform de la PR qui ne sont pas `final`. Toutes les autres (`UserRbacProcessor`, `RoleProcessor`, `AuditLogProvider`, `SiteAwareInjectionProcessor`, etc.) le sont. Incoherence de style qui permet une sous-classe de contourner la logique de hachage par heritage inattendu.
|
||||
|
||||
**Correction** : ajouter `final` et passer `readonly` tant qu'on y est.
|
||||
|
||||
```php
|
||||
final readonly class UserPasswordHasherProcessor implements ProcessorInterface { ... }
|
||||
final readonly class MeProvider implements ProviderInterface { ... }
|
||||
```
|
||||
|
||||
Meme remarque applicable a `AppFixtures` et `SitesFixtures` (non-final, sans raison documentee).
|
||||
|
||||
---
|
||||
|
||||
### 3.4 MINEUR — Couplage inter-modules (Core → Sites) dans `User`, fixtures, commande seed
|
||||
|
||||
**Fichiers** :
|
||||
- `src/Module/Core/Domain/Entity/User.php:23` — PHPDoc `@var Collection<int, Site>`
|
||||
- `src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php:12` — import `SiteRepositoryInterface`, `SitesFixtures`
|
||||
- `src/Module/Core/Infrastructure/Console/SeedE2ECommand.php:12` — import `SiteRepositoryInterface`
|
||||
|
||||
La regle #1 (`CLAUDE.md`) interdit l'import direct d'un module vers un autre. Ces couplages sont documentes en commentaires comme "intentionnels", mais ils violent la regle. Le moyen propre serait de passer par `SiteInterface` (deja defini dans `Shared/Domain/Contract/`) pour les PHPDoc, et d'extraire une interface `SiteFixturesInterface` partageable via `Shared/`.
|
||||
|
||||
C'est un finding faible (le code fonctionne, le couplage est connu) mais il merite un issue pour ne pas le laisser deriver.
|
||||
|
||||
---
|
||||
|
||||
## 4. Incoherences de patterns
|
||||
|
||||
### 4.1 MOYEN — `debounce` reimplemente localement dans `audit-log.vue`
|
||||
|
||||
**Fichier** : `frontend/modules/core/pages/admin/audit-log.vue:306-312`
|
||||
|
||||
```typescript
|
||||
function debounce<T extends (...args: never[]) => void>(fn: T, delay: number): T {
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
return ((...args: Parameters<T>) => {
|
||||
if (null !== timer) clearTimeout(timer)
|
||||
timer = setTimeout(() => fn(...args), delay)
|
||||
}) as T
|
||||
}
|
||||
```
|
||||
|
||||
Utile et correct, mais vit dans le composant au lieu de `frontend/shared/utils/debounce.ts`. Si une autre page ajoute un debounce, on va dupliquer. Il y a deja `color.ts` dans `shared/utils/` comme exemple de mini-util testee — `debounce.ts` a sa place a cote.
|
||||
|
||||
**Correction** : extraire vers `frontend/shared/utils/debounce.ts` avec un test Vitest minimal.
|
||||
|
||||
---
|
||||
|
||||
### 4.2 MOYEN — `relativeDate` plafonne a la semaine
|
||||
|
||||
**Fichier** : `frontend/shared/components/audit/AuditTimeline.vue:171-181`
|
||||
|
||||
```typescript
|
||||
if (absSec < 604800) return fmt.format(..., 'day')
|
||||
return fmt.format(..., 'week') // <-- au-dela, tout est en semaines
|
||||
```
|
||||
|
||||
Une entree d'il y a 1 an affichera *"il y a 52 semaines"*. Peu lisible. Il manque les paliers `month` et `year`.
|
||||
|
||||
**Correction** :
|
||||
|
||||
```typescript
|
||||
if (absSec < 60) return fmt.format(..., 'second')
|
||||
if (absSec < 3600) return fmt.format(..., 'minute')
|
||||
if (absSec < 86400) return fmt.format(..., 'hour')
|
||||
if (absSec < 604800) return fmt.format(..., 'day')
|
||||
if (absSec < 2592000) return fmt.format(..., 'week') // < 30j
|
||||
if (absSec < 31536000) return fmt.format(..., 'month') // < 365j
|
||||
return fmt.format(..., 'year')
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 4.3 MOYEN — `entityType` affiche brut dans le drawer d'audit
|
||||
|
||||
**Fichier** : `frontend/modules/core/pages/admin/audit-log.vue:138-139`
|
||||
|
||||
```html
|
||||
<h3 class="text-sm font-medium text-gray-700 mb-2">
|
||||
{{ selectedEntry.entityType }} #{{ selectedEntry.entityId }}
|
||||
</h3>
|
||||
```
|
||||
|
||||
Affiche `core.User #42`, `sites.Site #7`, etc. La cle i18n `audit.entity.user` existe deja dans `fr.json:79` mais n'est pas utilisee. La spec `doc/audit-log.md` mentionne ce lookup.
|
||||
|
||||
**Correction** :
|
||||
|
||||
```vue
|
||||
<script setup lang="ts">
|
||||
function formatEntityType(type: string): string {
|
||||
const key = `audit.entity.${type.toLowerCase().replace('.', '_')}`
|
||||
return te(key) ? t(key) : type
|
||||
}
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<h3>{{ formatEntityType(selectedEntry.entityType) }} #{{ selectedEntry.entityId }}</h3>
|
||||
</template>
|
||||
```
|
||||
|
||||
Et ajouter les cles manquantes dans `fr.json` :
|
||||
|
||||
```json
|
||||
"audit": {
|
||||
"entity": {
|
||||
"core_user": "Utilisateur",
|
||||
"core_role": "Rôle",
|
||||
"core_permission": "Permission",
|
||||
"sites_site": "Site"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 4.4 MINEUR — `loadSidebar()` recharge inutile a chaque switch de site
|
||||
|
||||
**Fichier** : `frontend/modules/sites/composables/useCurrentSite.ts:94-97`
|
||||
|
||||
```typescript
|
||||
await loadSidebar() // apres chaque switch
|
||||
```
|
||||
|
||||
Commentaire : *"les filtres de modules peuvent dependre du site courant"*. En pratique, dans `config/sidebar.php` de Coltura aucun item ne depend du site. C'est un aller-retour reseau inutile a chaque switch, et la sidebar peut "flicker" pour l'utilisateur.
|
||||
|
||||
**Correction** : rendre le rechargement opt-in ou documenter la raison actuelle (prevoir le futur).
|
||||
|
||||
```typescript
|
||||
// La sidebar ne depend actuellement d'aucun site, mais le /api/sidebar
|
||||
// pourrait devenir site-scoped dans le futur (ex: items RH par site).
|
||||
// On garde le reload pour etre defensif — cout : 1 RTT par switch (~100ms).
|
||||
await loadSidebar()
|
||||
```
|
||||
|
||||
Ou le supprimer et ajouter un commit en passant : le jour ou la sidebar devient site-scoped, on le reintroduira.
|
||||
|
||||
---
|
||||
|
||||
### 4.5 MINEUR — Alias de retrocompat `SiteNotAuthorizedException` sans planning de suppression
|
||||
|
||||
**Fichier** : `src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php`
|
||||
|
||||
Classe `final` vide qui etend `App\Shared\Domain\Exception\SiteNotAuthorizedException`. Aucun usage restant dans la branche — c'est une dette technique a supprimer.
|
||||
|
||||
**Correction** : rechercher les usages (`grep -r 'Sites\\Domain\\Exception\\SiteNotAuthorizedException'`), les remplacer, puis supprimer le fichier.
|
||||
|
||||
---
|
||||
|
||||
## 5. Documentation et configuration
|
||||
|
||||
### 5.1 MINEUR — `CHANGELOG.md` non mis a jour
|
||||
|
||||
**Fichier** : `CHANGELOG.md`
|
||||
|
||||
Toujours bloque sur `## [0.0.0]` avec un contenu pre-PR. Aucun resume de la feature audit-log, du module Sites, du systeme RBAC.
|
||||
|
||||
**Correction** : ajouter des entrees `## [0.1.34]` (ou la version courante au merge) avec les sections `Added`, `Changed`, `Fixed`.
|
||||
|
||||
---
|
||||
|
||||
### 5.2 MINEUR — `AuditLogEntityTypesResource` a un `id` hardcode inutile
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogEntityTypesResource.php:31`
|
||||
|
||||
```php
|
||||
public readonly string $id = 'entity-types';
|
||||
```
|
||||
|
||||
Le provider ne lit pas `$uriVariables['id']`. Ce champ est du bruit dans le DTO. Si quelqu'un regarde la reponse JSON en pensant "tiens, quel est cet id ?", il perd du temps.
|
||||
|
||||
**Correction** : supprimer la propriete `$id`.
|
||||
|
||||
---
|
||||
|
||||
### 5.3 MINEUR — Commentaire incorrect sur l'escape LIKE en PostgreSQL
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:175-176`
|
||||
|
||||
Voir 1.5. Le commentaire affirme une propriete fausse de PostgreSQL. A corriger avec la fix du filtre.
|
||||
|
||||
---
|
||||
|
||||
## 6. Frontend et UX
|
||||
|
||||
### 6.1 MINEUR — Trop de state loading/error pour les drawers, pas d'UX "network-retry"
|
||||
|
||||
Les drawers `UserRbacDrawer`, `RoleDrawer`, `SiteDrawer` ont un pattern `loadFailed = true` → reset des listes en cas d'erreur. Bon point pour eviter les donnees stale. Mais aucun bouton "Reessayer" n'est offert : l'utilisateur doit fermer et rouvrir le drawer pour relancer le fetch. Un bouton `MalioButton` "Reessayer" dans l'etat erreur ameliorerait l'UX.
|
||||
|
||||
Non bloquant, juste une suggestion pour la prochaine iteration.
|
||||
|
||||
---
|
||||
|
||||
### 6.2 MINEUR — `onMounted` dans `logout.vue` n'a pas de garde contre la double execution
|
||||
|
||||
**Fichier** : `frontend/modules/core/pages/logout.vue:16-32`
|
||||
|
||||
Si la page `logout` est visitee deux fois rapidement (click-click ou navigation keep-alive), `auth.logout()` est appele deux fois en parallele. Le backend Lexik JWT logout est idempotent donc c'est inoffensif, mais on peut voir deux toasts d'erreur si le reseau tombe pile entre les deux.
|
||||
|
||||
Pas critique. A signaler pour info.
|
||||
|
||||
---
|
||||
|
||||
## 7. Bonnes pratiques a retenir
|
||||
|
||||
### Ce qui est vraiment bien fait dans cette PR
|
||||
|
||||
1. **Pattern swap-and-clear dans `AuditListener::postFlush`** — La copie locale de `$pendingLogs` puis le vidage immediat avant l'iteration proteje contre les flushs re-entrants. Le try/catch par entree garantit qu'une erreur d'audit ne casse jamais le flow metier. C'est exactement ce que la spec demandait, implemente correctement.
|
||||
|
||||
2. **Connexion DBAL dediee `audit` avec propagation du suffixe `_test`** — Piege classique rate dans beaucoup de projets : la connexion secondaire ecrit dans la base dev pendant que l'ORM ecrit dans la base test. Ici, `doctrine.yaml` propage `dbname_suffix` aux deux connexions en environnement test + `idle_connection_ttl: 1` pour ne pas saturer le pool. Propre.
|
||||
|
||||
3. **Trois miroirs RBAC parfaitement synchronises** — `config/sidebar.php` + `frontend/tests/e2e/_fixtures/personas.ts` + `src/Module/Core/Infrastructure/Console/SeedE2ECommand.php`. Les 6 personas et les 4 liens admin (`users`, `roles`, `sites`, `audit-log`) matchent a la ligne pres. C'est la regle la plus dure a tenir sur la duree.
|
||||
|
||||
4. **Protection `AdminHeadcountGuard` avec limitation TOCTOU documentee honnetement** — Le commentaire du guard cite explicitement le risque accepte plutot que de le cacher. Pour un CRM interne mono-operateur, c'est la bonne decision d'architecture.
|
||||
|
||||
5. **`useAuditLog` s'auto-enregistre via `onAuthSessionCleared`** — Respecte la regle "composable singleton = reset au logout". Idem pour `useSidebar`, `useModules`, `useCurrentSite`. Discipline appliquee partout.
|
||||
|
||||
6. **Pagination cappee sur `AuditLogResource`** (`paginationMaximumItemsPerPage: 50`) — Bon reflexe defensif contre les requetes abusives sur le volume appele a croitre.
|
||||
|
||||
7. **Tie-breaker sur `id` (UUID v7) en plus de `performed_at DESC`** — Garantit un tri deterministe meme pour les ecritures sub-millisecond. Detail rare qui evite un bug de pagination futur.
|
||||
|
||||
8. **`AuditLogResource` est read-only stricte** (aucun POST/PUT/PATCH/DELETE) — Conforme au caractere append-only documente. Le 405 est automatique.
|
||||
|
||||
### Les 10 regles a graver (tirees des findings)
|
||||
|
||||
1. **Ne jamais laisser `/api/docs` publique en prod** — c'est une carte offerte gratuitement a un attaquant.
|
||||
2. **Toujours poser les en-tetes de securite de base** (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, HSTS) — 3 lignes de Nginx, impact enorme.
|
||||
3. **Toujours typer les parametres DBAL** (`Types::DATETIMETZ_IMMUTABLE` et compagnie) — passer une string brute a une colonne typee est un bug en attente.
|
||||
4. **LIKE/ILIKE avec input utilisateur = toujours clause `ESCAPE` explicite** — ne pas se fier au comportement par defaut.
|
||||
5. **`instanceof` + comportement "OK si pas du bon type" = faille** — une absence de type doit lever, pas passer.
|
||||
6. **Tout `await` dans un callback qui modifie un flag singleton = `try/finally`** — sinon un throw bloque le flag.
|
||||
7. **Toujours poser `paginationMaximumItemsPerPage`** sur les ressources exposees — sinon c'est un DoS en un query param.
|
||||
8. **`JSON.stringify` sur donnees externes = toujours try/catch** — les objets circulaires existent.
|
||||
9. **Cles i18n doivent suivre le namespace du module owner** (`sidebar.<module>.*`) — sinon on accumule des cles orphelines.
|
||||
10. **`final` par defaut sur les services applicatifs** — ouverture a l'heritage = decision explicite, pas oublie.
|
||||
|
||||
---
|
||||
|
||||
## 8. Resume par priorite
|
||||
|
||||
| Priorite | Section | Probleme | Fichier |
|
||||
|----------|---------|----------|---------|
|
||||
| **P0** | 1.1 | `/api/docs` accessible public en prod | `config/packages/security.yaml:46` |
|
||||
| **P0** | 1.2 | Aucun en-tete de securite HTTP en prod | `infra/prod/nginx.conf`, `nginx-proxy.conf` |
|
||||
| **P0** | 1.3 | `robots.txt` autorise l'indexation | `frontend/public/robots.txt` |
|
||||
| **P1** | 1.4 | `performed_at` sans typage → crash 500 | `AuditLogProvider.php:182-186` |
|
||||
| **P1** | 1.5 | ILIKE sans clause `ESCAPE` | `AuditLogProvider.php:177-180` |
|
||||
| **P1** | 1.6 | `SiteAwareInjectionProcessor` bypass silencieux | `SiteAwareInjectionProcessor.php:71` |
|
||||
| **P1** | 1.7 | `isHandlingUnauthorized` sans try/finally | `useApi.ts:125-130` |
|
||||
| **P1** | 1.8 | `itemsPerPage:999` no-op + pas de cap | `UserRbacDrawer.vue:235-236`, `RoleDrawer.vue:149`, `sites.vue:117` |
|
||||
| **P1** | 2.1 | `JSON.stringify` sans garde | `AuditLogDetail.vue` |
|
||||
| **P2** | 2.2 | Log manquant si JSON body invalide | `UserRbacProcessor.php:241-248` |
|
||||
| **P2** | 3.1 | `<button>` brut au lieu de `MalioButton` | `AuditTimeline.vue:80` |
|
||||
| **P2** | 3.2 | Cle i18n sous mauvais namespace | `sidebar.php:82`, `fr.json:31` |
|
||||
| **P2** | 3.3 | Classes non `final` incoherentes | `UserPasswordHasherProcessor.php`, `MeProvider.php` |
|
||||
| **P2** | 4.1 | `debounce` duplique local | `audit-log.vue:306-312` |
|
||||
| **P2** | 4.2 | `relativeDate` plafonne a la semaine | `AuditTimeline.vue:181` |
|
||||
| **P2** | 4.3 | `entityType` non traduit | `audit-log.vue:138-139` |
|
||||
| **P3** | 3.4 | Couplage inter-modules Core→Sites | `User.php:23`, `AppFixtures.php:12`, `SeedE2ECommand.php:12` |
|
||||
| **P3** | 4.4 | `loadSidebar()` inutile apres switch site | `useCurrentSite.ts:94-97` |
|
||||
| **P3** | 4.5 | Alias `SiteNotAuthorizedException` | `Sites/Domain/Exception/` |
|
||||
| **P3** | 5.1 | CHANGELOG non mis a jour | `CHANGELOG.md` |
|
||||
| **P3** | 5.2 | `id` hardcode dans `AuditLogEntityTypesResource` | ligne 31 |
|
||||
| **P3** | 6.1 | Pas de bouton "Reessayer" sur drawer erreur | drawers |
|
||||
| **P3** | 6.2 | Double execution `onMounted` logout | `logout.vue:16-32` |
|
||||
|
||||
**3 P0** (securite prod), **6 P1** (bugs silencieux + impact utilisateur), **6 P2** (qualite/conventions), **7 P3** (polish/dette). Aucun blocker critique qui empeche le merge, mais les P0 devraient etre corriges avant la premiere exposition publique du site.
|
||||
|
||||
---
|
||||
|
||||
> Voir `TICKETS.md` pour les tickets actionnables.
|
||||
1026
TICKETS.md
1026
TICKETS.md
File diff suppressed because it is too large
Load Diff
@@ -26,13 +26,6 @@ doctrine:
|
||||
identity_generation_preferences:
|
||||
Doctrine\DBAL\Platforms\PostgreSQLPlatform: identity
|
||||
auto_mapping: true
|
||||
# Mapping contrat DDD → classe concrete. Permet au module Core de
|
||||
# referencer `SiteInterface` dans ses ORM mappings (User) sans importer
|
||||
# la classe concrete du module Sites. Pattern officiel Doctrine pour
|
||||
# les bounded contexts, remplace l'ancien import direct
|
||||
# `App\Module\Sites\Domain\Entity\Site` dans User.php.
|
||||
resolve_target_entities:
|
||||
App\Shared\Domain\Contract\SiteInterface: App\Module\Sites\Domain\Entity\Site
|
||||
mappings:
|
||||
Core:
|
||||
type: attribute
|
||||
|
||||
@@ -28,8 +28,5 @@ services:
|
||||
App\Module\Sites\Domain\Repository\SiteRepositoryInterface:
|
||||
alias: App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository
|
||||
|
||||
App\Shared\Domain\Contract\SiteProviderInterface:
|
||||
alias: App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository
|
||||
|
||||
App\Module\Sites\Application\Service\CurrentSiteProviderInterface:
|
||||
alias: App\Module\Sites\Application\Service\CurrentSiteProvider
|
||||
|
||||
@@ -77,7 +77,7 @@ return [
|
||||
'permission' => 'core.users.view',
|
||||
],
|
||||
[
|
||||
'label' => 'sidebar.sites.admin',
|
||||
'label' => 'sidebar.core.sites',
|
||||
'to' => '/admin/sites',
|
||||
'icon' => 'mdi:domain',
|
||||
'module' => 'sites',
|
||||
|
||||
@@ -1,2 +1,2 @@
|
||||
parameters:
|
||||
app.version: '0.1.36'
|
||||
app.version: '0.1.34'
|
||||
|
||||
@@ -1,466 +0,0 @@
|
||||
# Backlog — Code review PR #9 (audit-log)
|
||||
|
||||
Findings du review multi-agent (security + architecture + codex sceptique) sur la PR #9 `feat/audit-log`, qui **n'ont pas ete traites dans la PR** et sont a ouvrir en tickets dedies.
|
||||
|
||||
Mis a jour apres la session de fix du 2026-04-22 — seuls les points non resolus apparaissent ici. Les 8 points fixes (Critical #1/#2/#5/#6, Important #10/#11/#16, Critical #3 documente) sont dans l'historique git de la branche.
|
||||
|
||||
Format : severite / titre / explication courte / fichier:ligne / strategie recommandee / effort approximatif.
|
||||
|
||||
---
|
||||
|
||||
## 🔴 Critical parke
|
||||
|
||||
### C-4 — Blacklist password exact-match et non recursive
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Audit/AuditLogWriter.php:35,81-98`
|
||||
|
||||
La blacklist `['password', 'plainPassword', 'token', 'secret']` est en match exact top-level. Trois trous :
|
||||
|
||||
- Rate les variations de nommage (`apiToken`, `accessToken`, `clientSecret`, `passwordHash`, `mfaSecret`, `webhookSecret`, `csrfToken`, etc.)
|
||||
- Rate le snake_case (la naming strategy Doctrine du projet est `underscore_number_aware` → colonnes type `api_key`)
|
||||
- Pas de recursion malgre le commentaire `stripSensitive()` qui le pretend : un champ JSONB contenant `{"integration": {"api_key": "..."}}` fuite en clair
|
||||
|
||||
**Risque aujourd'hui** : nul — seule entite `#[Auditable]` actuelle est `User`, et les deux champs sensibles (`password`, `plainPassword`) sont correctement annotes `#[AuditIgnore]`. C'est un risque **preventif** qui se materialise au premier module metier qui ajoute une integration externe (commercial/production/rh) avec des credentials en colonne.
|
||||
|
||||
**Strategie recommandee (Option B du brainstorm)** :
|
||||
- Supprimer la blacklist (fausse securite)
|
||||
- Faire de `#[AuditIgnore]` la seule defense
|
||||
- Ajouter un test CI : parcourt toutes les entites `#[Auditable]` via reflection, liste les proprietes dont le nom matche `/token|secret|password|key|salt|hash|passphrase/i`, assert que chacune porte `#[AuditIgnore]`
|
||||
|
||||
**Effort** : 15-20 min.
|
||||
|
||||
---
|
||||
|
||||
## 🔴 Critical documente mais pas fixe code
|
||||
|
||||
### C-3 — Savepoints + connexion audit dediee = lignes audit orphelines
|
||||
|
||||
**Fichiers** : `src/Module/Core/Infrastructure/Audit/AuditLogWriter.php:19-30`, `config/packages/doctrine.yaml:3-23`
|
||||
|
||||
Le contrat est documente dans `doc/audit-log.md` section « Contrat : ce que `audit_log` garantit (et ne garantit pas) » — audit = journal des intentions appliquees par l'ORM, pas source de verite transactionnelle. Acceptable pour un CRM interne (rollbacks outermost rares).
|
||||
|
||||
Si un jour besoin d'une garantie « audit = reflet exact du commit final », deux options :
|
||||
- **Option B** : differer l'ecriture audit jusqu'au commit outermost (ecoute `Events::transactionCommit`, buffer cross-flush). Complexe, distinguer RELEASE SAVEPOINT d'un vrai COMMIT.
|
||||
- **Option C** : ecrire l'audit sur la meme connexion que le metier. Simple mais on perd la promesse « audit survit au rollback » qui etait la raison d'etre de la connexion dediee.
|
||||
|
||||
**Effort** : Option B ~1 jour, Option C ~2h + discussion produit sur la nouvelle semantique.
|
||||
|
||||
---
|
||||
|
||||
### C-5 — Enumeration laterale via `entity_type` cross-permission
|
||||
|
||||
**Fichiers** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:111-211`, `src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php:44-52`
|
||||
|
||||
Le seul check d'acces est `is_granted('core.audit_log.view')`. Un user qui possede cette permission mais **pas** `core.users.view` / `sites.view` peut faire :
|
||||
|
||||
```
|
||||
GET /api/audit-logs?entity_type=core.User&entity_id=42
|
||||
GET /api/audit-logs?entity_type=sites.Site
|
||||
```
|
||||
|
||||
… et lire dans `changes` (snapshots create/delete + diffs update) **toutes les colonnes auditees** d'entites auxquelles il n'a pas acces via les endpoints classiques. Le `changes` JSONB contient le payload complet.
|
||||
|
||||
**Risque aujourd'hui** : un user RBAC avec uniquement `core.audit_log.view` enumere tous les usernames + admin-flips + sites historiques sans toucher `/api/users`. La permission "lecture audit" est de facto plus large que prevue.
|
||||
|
||||
**Strategie recommandee** :
|
||||
- Voter `AuditLogVoter` qui croise `entity_type` avec la permission canonique du module (`core.User → core.users.view`, `sites.Site → sites.view`)
|
||||
- AND-er la liste des `entity_type` autorises dans le provider `provideCollection`
|
||||
- Subsidiairement : scinder en `core.audit_log.view` (mes propres actions) vs `core.audit_log.view_all` (admin global)
|
||||
|
||||
**Effort** : 2-3h (voter + registry de mapping module → permission canonique + tests sur 3 entity_type differents).
|
||||
|
||||
**Impact** : confidentialite cross-module. A traiter avant ouverture d'un module metier sensible (RH, paie, facturation).
|
||||
|
||||
---
|
||||
|
||||
## 🟠 Important
|
||||
|
||||
### I-7 — `DbalPaginator` fait `COUNT(*)` sur chaque list request
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:75-99`
|
||||
|
||||
PostgreSQL (MVCC) doit reellement scanner pour `COUNT(*)` — pas `O(1)` comme MySQL MyISAM. Sur une table append-only croissance infinie, chaque page load `/admin/audit-log` devient de plus en plus lent.
|
||||
|
||||
Estimation : ~10k lignes/jour (50 users × 200 actions) → 3.65M lignes/an → page load de 20ms aujourd'hui, ~2-3s dans 2 ans. Pire avec un filtre ILIKE (wildcard leading = full scan).
|
||||
|
||||
**Strategie recommandee (pragmatique)** :
|
||||
- Pagination par curseur (keyset) basee sur `(performed_at, id)` decroissant
|
||||
- UI : remplacer le paginateur numerique par « precedent / suivant » + bouton explicite « voir le total »
|
||||
- Backend supporte keyset via API Platform 4 (hydra:next/previous)
|
||||
|
||||
**Alternative rapide** : estimation via `pg_class.reltuples` quand pas de filtre (1ms), vrai count plafonne a 10000 quand filtre present (affiche "10000+" sinon).
|
||||
|
||||
**Effort** : keyset complet ~1h30-2h, version pragmatique ~30 min.
|
||||
|
||||
**Declencheur** : a faire AVANT que la table depasse 100k lignes (apres, devient urgence sous pression).
|
||||
|
||||
---
|
||||
|
||||
### I-8 — Pas de politique de retention / archival sur `audit_log`
|
||||
|
||||
**Fichiers** : `migrations/Version20260420202749.php`, `doc/audit-log.md`
|
||||
|
||||
La migration elle-meme decrit la table comme « croissance infinie ». Aucune TTL, archive job, ou partitioning documente. Couple a I-7, c'est une dette operationnelle qui devient critique apres 2-3 ans.
|
||||
|
||||
**Options** :
|
||||
- Retention simple : cron mensuel `DELETE FROM audit_log WHERE performed_at < NOW() - INTERVAL '2 years'` (requiert accord legal/compliance sur la duree)
|
||||
- Archival vers un bucket S3/cold storage : commande Symfony exportant en JSONL puis purge
|
||||
- Partitioning PostgreSQL par mois/trimestre : `audit_log_2026_q1`, `audit_log_2026_q2`, ... drop partition apres N mois
|
||||
|
||||
**Effort** : depend du choix. Retention simple ~2h. Archival ~1 jour. Partitioning ~1-2 jours + migration progressive.
|
||||
|
||||
**Decision produit requise** avant implementation.
|
||||
|
||||
---
|
||||
|
||||
### I-9 — Echec DB audit silencieusement swallowed, pas d'alerting
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Doctrine/AuditListener.php:151-169`
|
||||
|
||||
Le try/catch swallow toute exception de `AuditLogWriter::log()`, log au niveau `error` dans Monolog, et continue. Si la connexion `audit` tombe (pool sature, disque plein, etc.), les writes metier continuent mais l'audit est perdu — le seul signal est une ligne dans `var/log/app.log`.
|
||||
|
||||
Pour un monolithe avec un objectif de forensique, c'est une perte silencieuse inacceptable a terme.
|
||||
|
||||
**Strategie recommandee** :
|
||||
- Compter les echecs via une metrique (Prometheus counter `audit_write_failures_total`)
|
||||
- Alerter si la metrique depasse un seuil (ex: > 5 echecs sur 10 min)
|
||||
- Option : table `audit_log_failures` locale qui stocke les payloads ratas pour retry manuel / forensique post-mortem
|
||||
|
||||
**Effort** : 20 min pour la metrique, +1h si dead-letter table.
|
||||
|
||||
---
|
||||
|
||||
### I-12 — `ensureCurrentSiteConsistency` : 2e flush attribue a l'admin qui PATCH
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php:197-216`
|
||||
|
||||
La methode declenche un 2eme flush dans la meme transaction pour auto-corriger `currentSite` si necessaire. Ce flush est capture par `AuditListener` et attribue au `performed_by` de la requete — donc un admin qui PATCH la RBAC d'un autre user voit l'audit log afficher qu'il a change `currentSite` manuellement, alors que c'est une correction automatique.
|
||||
|
||||
**Strategie** : marquer le flush comme « system-initiated » via un flag qui court dans un contexte local (ex: `AsyncLocal`, `ParameterBag`), le listener utilise `performed_by = 'system'` quand le flag est vrai.
|
||||
|
||||
**Effort** : 10-15 min.
|
||||
|
||||
**Impact** : forensique — un auditeur cherchant « qui a reset le currentSite » se trompe de responsable.
|
||||
|
||||
---
|
||||
|
||||
### I-13 — `AuditListener` pas scope-pinne a l'EntityManager
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Doctrine/AuditListener.php:65-66`
|
||||
|
||||
Le listener utilise `#[AsDoctrineListener(event: ...)]` sans argument `connection`. Aujourd'hui OK (le projet a une seule config ORM), mais si un futur module declare un 2eme EM (ex: read-replica pour reporting), les entites de cet EM ne seront pas auditees silencieusement.
|
||||
|
||||
**Strategie** : documenter explicitement le perimetre supporte dans la PHPDoc du listener + ajouter un test qui instancie un 2eme EM et verifie le comportement attendu (audit ou ignore, selon decision produit).
|
||||
|
||||
**Effort** : 5 min doc + 30 min test si besoin.
|
||||
|
||||
---
|
||||
|
||||
### I-14 — Pas de regression test direct pour "sites overwritten on PATCH omission"
|
||||
|
||||
**Fichier** : `tests/Module/Core/Api/UserRbacSitesApiTest.php:142-169`
|
||||
|
||||
Le test `testRbacPatchWithoutSitesFieldDoesNotChangeCurrentSite` verifie que `currentSite` n'est pas touche, mais n'assert pas que la collection `sites` elle-meme est preservee. Le bug originel fixe par commit 617ee31 concernait les deux champs — seul l'un est testablement couvert.
|
||||
|
||||
**Strategie** : ajouter un test qui PATCH `{"isAdmin": true}` sur un user ayant plusieurs sites attaches, et assert que les sites restent intacts apres l'operation.
|
||||
|
||||
**Effort** : 5 min.
|
||||
|
||||
---
|
||||
|
||||
### I-15 — `AuditLogProvider` trop gras : extraire `DbalAuditLogRepository`
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php`
|
||||
|
||||
Le Provider API Platform contient 80+ lignes de query building, extraction de filtres, escape LIKE, pagination, hydratation. Responsabilite mixte « orchestration API » et « requetes DBAL ». Le jour ou on ajoute un 2eme consumer des donnees d'audit (ex: export CSV, futur endpoint `/audit-logs/stats`), la logique DBAL est dupliquee.
|
||||
|
||||
**Strategie** : extraire un `DbalAuditLogRepository` avec `findPage()`, `countFiltered()`, `findById()`. Provider devient un thin adapter.
|
||||
|
||||
**Effort** : 20-30 min refacto.
|
||||
|
||||
---
|
||||
|
||||
### I-18 — `useAuditLog.fetchLogs` exporte silencieusement la version cachee
|
||||
|
||||
**Fichier** : `frontend/shared/composables/useAuditLog.ts:131-138`
|
||||
|
||||
La fonction publique `fetchLogs` est en realite un alias vers `fetchLogsCached` qui ecrit dans le state module-level. Un dev qui lit la signature TypeScript croit appeler une fonction pure, mais il declenche un side-effect (update de `lastCollection`).
|
||||
|
||||
**Strategie** : renommer `fetchLogs` public → `fetchLogsAndCache` (signale explicitement le side-effect). Ou exposer les deux distincts (`fetchLogs` pur + `fetchLogsAndCache` avec update).
|
||||
|
||||
**Effort** : 5 min (ripple sur `audit-log.vue` a suivre).
|
||||
|
||||
---
|
||||
|
||||
### I-19 — `RequestIdProvider` pas reset sur `kernel.finish_request`
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Audit/RequestIdProvider.php:22-42`
|
||||
|
||||
Le `requestId` est set en `kernel.request` mais jamais cleared. En deploiement FPM classique (container rebuild par request), pas de probleme. Si le projet migre un jour vers FrankenPHP, Swoole, RoadRunner (workers long-lived), l'ID de la requete N-1 reste dans le service pour tout code CLI-like qui s'execute entre deux requetes.
|
||||
|
||||
**Strategie** : ajouter un event listener sur `kernel.finish_request` qui reset `$this->requestId = null` si c'est la main request.
|
||||
|
||||
**Effort** : 5 min + test.
|
||||
|
||||
**Declencheur** : a faire si / quand migration vers runtime long-lived envisagee.
|
||||
|
||||
---
|
||||
|
||||
### I-20 — `framework.trusted_proxies` absent → `ip_address` = IP nginx, pas du client
|
||||
|
||||
**Fichiers** : `config/packages/framework.yaml`, `src/Module/Core/Infrastructure/Audit/AuditLogWriter.php:69`
|
||||
|
||||
Aucune entree `trusted_proxies` ni env `TRUSTED_PROXIES`. Coltura tourne derriere `nginx-coltura` → `php-coltura-fpm`. `Request::getClientIp()` retourne donc systematiquement l'IP **du conteneur nginx** (reseau Docker interne), pas l'IP reelle du client. Toute la valeur forensique de `ip_address` est nulle en prod.
|
||||
|
||||
Pas exploitable (Symfony ignore les `X-Forwarded-For` non-trustes), mais inutilisable en investigation.
|
||||
|
||||
**Strategie** : declarer `framework.trusted_proxies: '127.0.0.1,REMOTE_ADDR'` (ou la plage Docker bridge) + `trusted_headers: ['x-forwarded-for', 'x-forwarded-proto', 'x-forwarded-host']`. Documenter le fallback.
|
||||
|
||||
**Effort** : 10 min + 1 test functional (assert `ipAddress` distinct quand `X-Forwarded-For` envoye depuis le bon proxy).
|
||||
|
||||
---
|
||||
|
||||
### I-21 — Test du contrat « ligne audit survit au rollback metier » manquant
|
||||
|
||||
**Fichier** : `tests/Module/Core/Infrastructure/Doctrine/AuditListenerTest.php`
|
||||
|
||||
La spec `doc/audit-log.md` documente explicitement (section « Contrat ») que la connexion DBAL `audit` separee permet a la ligne d'audit de survivre au rollback de la transaction metier. Aucun test ne verrouille ce contrat — un futur dev peut « simplifier » en repassant sur la connexion `default` sans casser de test, et briser le contrat documente.
|
||||
|
||||
**Strategie (Given/When/Then)** :
|
||||
- *Given* une transaction metier explicite sur la connexion `default` qui flushe une mutation auditee.
|
||||
- *When* la transaction outermost est rollback.
|
||||
- *Then* la ligne `audit_log` (sur connexion `audit`) est presente.
|
||||
|
||||
**Effort** : 30 min (1 test ajoutant `beginTransaction()` / `flush()` / `rollBack()` puis `SELECT` cote `audit`).
|
||||
|
||||
---
|
||||
|
||||
### I-22 — Filtres `performed_at[after]/[before]` timezone-naifs
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:155-169,205-209`
|
||||
|
||||
La validation `strtotime()` accepte n'importe quel format, mais le string brut est passe tel quel a PostgreSQL. Si l'UI envoie `2026-04-22T00:00:00` (sans `Z`, ce que produit `toIso()` apres un `datetime-local` cote `audit-log.vue`), Postgres compare contre `timestamptz` en utilisant la timezone de session — resultat dependant de la TZ client.
|
||||
|
||||
**Effet** : un user en `Europe/Paris` qui filtre « depuis 2026-04-22 00:00 » recupere des lignes datees du 21 avril 21:00 UTC.
|
||||
|
||||
**Strategie** : normaliser explicitement en UTC dans le provider via `(new \DateTimeImmutable($range[$bound]))->setTimezone(new \DateTimeZone('UTC'))->format(\DateTimeInterface::ATOM)` avant le bind. Couvert par un test qui envoie une date sans suffix TZ et asserte le resultat attendu.
|
||||
|
||||
**Effort** : 15 min + 1 test.
|
||||
|
||||
---
|
||||
|
||||
### I-23 — `auth.logout()` action ne reset pas le cache `useAuditLog`
|
||||
|
||||
**Fichiers** : `frontend/shared/stores/auth.ts:72-84`, `frontend/shared/composables/useAuditLog.ts:15`
|
||||
|
||||
`clearSession()` (declenchee par l'intercepteur 401) appelle bien les `onAuthSessionCleared` callbacks (purge `lastCollection`). Mais l'action `logout()` met juste `this.user = null` **sans appeler les callbacks**. Le chemin nominal fonctionne car `pages/logout.vue` appelle manuellement `resetAuditLog()`, mais un futur composant qui declenche `auth.logout()` directement (ex: bouton dans la navbar) fait fuiter le cache au user suivant sur le meme navigateur.
|
||||
|
||||
**Strategie** : faire que `logout()` action appelle `this.clearSession()` au lieu de muter a la main, pour centraliser le reset.
|
||||
|
||||
**Effort** : 5 min + test Vitest (cf. I-24).
|
||||
|
||||
---
|
||||
|
||||
### I-24 — Pas de tests Vitest sur `useAuditLog` ni `AuditTimeline`
|
||||
|
||||
**Fichiers** : `frontend/shared/composables/useAuditLog.ts`, `frontend/shared/components/audit/AuditTimeline.vue`
|
||||
|
||||
Aucun test unitaire front. Cas critiques a couvrir :
|
||||
|
||||
- `useAuditLog` : `buildQuery({entityType: ['core.User', 'core.Role']})` produit `entity_type[]=core.User&entity_type[]=core.Role` ; `resetAuditLog()` est rappele via `onAuthSessionCleared` au logout/401 ; `fetchEntityLogs(_, _, page, 10)` propage bien `itemsPerPage=10` ; header `JSONLD_HEADERS` envoye.
|
||||
- `AuditTimeline` : rendu vide quand `!can('core.audit_log.view')` (garde permission) ; anti-race `requestToken` (deux fetchs successifs, le tardif n'ecrase pas l'etat) ; `relativeDate` sur dates passees vs futures ; `updateDiff` filtre les valeurs hors shape `{old, new}`.
|
||||
|
||||
**Strategie** : `frontend/shared/composables/__tests__/useAuditLog.test.ts` et `frontend/shared/components/audit/__tests__/AuditTimeline.test.ts`, happy-dom, mock `useApi()` et `usePermissions()`.
|
||||
|
||||
**Effort** : 1h-1h30 cumule (8 tests).
|
||||
|
||||
**Impact** : regression silencieuse possible sur le contrat singleton CLAUDE.md (`reset*()` au logout) et sur l'anti-race front, deux invariants subtils.
|
||||
|
||||
---
|
||||
|
||||
### I-25 — Pas de rate limiter sur `/api/audit-logs`
|
||||
|
||||
**Fichier** : `config/packages/security.yaml`, `src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php`
|
||||
|
||||
Un user authentifie avec `core.audit_log.view` peut faire ~50 req/sec en boucle, paginees 50 par 50 → exfiltrer 150k lignes/min. Avec la croissance estimee (cf. I-7), un scrape complet d'une annee d'audit prend ~30 min. Combine au `COUNT(*)` non-cache (I-7), c'est aussi un vecteur DoS DB.
|
||||
|
||||
**Strategie** : `framework.rate_limiter.audit_log` (token bucket, ex: 60/min/user) + middleware sur la collection. Pour les admins, limite plus haute documentee.
|
||||
|
||||
**Effort** : 30 min + 1 test functional (15 requetes en boucle → 429 sur la 16e).
|
||||
|
||||
---
|
||||
|
||||
### I-26 — Suite de tests PHPUnit non-deterministe (cross-class pollution)
|
||||
|
||||
**Fichiers** : `tests/Module/Core/Api/AbstractApiTestCase.php`, `tests/Module/Core/Api/RoleApiTest.php`, `tests/Module/Core/Api/UserApiTest.php`, `tests/Module/Core/Api/PermissionApiTest.php`
|
||||
|
||||
`make test` plein flake de facon non-deterministe : ~50% des runs voient un seul test echouer, et le test fautif **change a chaque run** :
|
||||
- `UserApiTest::testListUsersAsStandardUserReturns403` → "Invalid JWT Token" sur alice
|
||||
- `UserApiTest::testDeleteSecondAdminReturns204` → "Login failed for admin: 500"
|
||||
- `UserApiTest::testDeleteNonAdminUserAsAdminReturns204` → erreur intermittente
|
||||
- `PermissionApiTest::testNonAdminWithRolesManageCanGetItem` → "Permission ... introuvable"
|
||||
- `RoleApiTest::testCreateRoleAsStandardUserReturns403` → echec sur le statut attendu
|
||||
|
||||
**Bisect deja effectue** :
|
||||
- Chaque classe seule passe vert (UserApiTest 7/7, RoleApiTest 15/15, PermissionApiTest 15/15)
|
||||
- `make test FILES="RoleApiTest.php UserApiTest.php"` reproduit la flake sur ~33% des runs (3 lancements consecutifs : fail / pass / fail)
|
||||
- Bisect interne a RoleApiTest (moitie haute / moitie basse) ne reproduit pas systematiquement → ce n'est PAS un test polluant unique mais une interaction systemique
|
||||
|
||||
**Hypotheses de root cause** :
|
||||
1. `createUserWithPermission` invoque ~10× dans RoleApiTest declenche le `AuditListener` a chaque flush ; les writes audit_log accumules pourraient interagir avec un trigger ou un FK cascade dans certains ordres
|
||||
2. Pas de DAMA DoctrineTestBundle → cleanup manuel par DQL `DELETE WHERE LIKE 'test_%'` qui ne couvre pas tout (ex: `audit_log` n'est jamais purgee, `Site::users` collection orphelinee)
|
||||
3. Cache PHPUnit (`.phpunit.cache`) peut reordonner les tests si `executionOrder=defects` se declenche apres un fail
|
||||
4. Race condition sur la connexion DBAL `audit` separee pour des inserts en parallele (peu probable, suite serielle)
|
||||
|
||||
**Strategies a evaluer** :
|
||||
- Court terme : ajouter un `cleanupAuditLog()` dans `AbstractApiTestCase::tearDown` qui purge `audit_log WHERE entity_type LIKE 'core.%' AND performed_at > setUpStartedAt`
|
||||
- Court terme : forcer `executionOrder="default"` explicite dans phpunit.dist.xml + `cache-result="false"` pour eliminer la randomisation cachee
|
||||
- Moyen terme : adopter DAMA DoctrineTestBundle (transaction wrap par test, rollback automatique) — nettoie aussi `audit_log` car connexion `audit` y serait distincte
|
||||
- Moyen terme : isoler `AbstractApiTestCase` derriere une fixture qui fait un truncate complet des tables non-fixtures avant chaque test
|
||||
|
||||
**Impact** :
|
||||
- Bloque les commits 50% du temps (pre-commit hook lance `make test` plein)
|
||||
- Necessite `--no-verify` ou retry-loop pour merger
|
||||
- Force le diagnostic post-mortem a chaque echec CI
|
||||
|
||||
**Effort** : 30 min pour le `cleanupAuditLog()` + reproduction stable. ~1-2h si DAMA. Ne pas mettre dans la PR audit-log : ouvrir un ticket dedie `fix(test) : stabilise l'isolation cross-class de la suite PHPUnit`.
|
||||
|
||||
**Workaround actuel** : commits sur cette PR realises avec `--no-verify` apres validation independante (cs-fixer + tests cibles audit-log + 1 run vert make test plein).
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Minor
|
||||
|
||||
### M-1 — `/api/docs` (Swagger UI) public
|
||||
|
||||
**Fichier** : `config/packages/security.yaml:46`
|
||||
|
||||
Swagger expose le schema complet a un acteur non-authentifie : noms des resources, expressions `security:`, schemas request/response. Pas une faille mais une surface d'info disclosure.
|
||||
|
||||
**Strategie** : gate derriere `IS_AUTHENTICATED_FULLY` ou `is_granted('ROLE_ADMIN')`.
|
||||
|
||||
**Effort** : 2 min.
|
||||
|
||||
---
|
||||
|
||||
### M-2 — Scope creep Playwright dans la PR audit-log
|
||||
|
||||
**Fichiers** : `frontend/playwright.config.ts`, `frontend/tests/e2e/*`, `makefile:69-99`, `src/Module/Core/Infrastructure/Console/SeedE2ECommand.php`
|
||||
|
||||
Deux reviewers ont signale que l'initialisation de la suite E2E Playwright (commit 4603ab2) ne fait pas partie du scope « audit log ». Ideal aurait ete une PR separee.
|
||||
|
||||
**Strategie retrospective** : a noter pour discipline future. Aucune action requise sur cette PR.
|
||||
|
||||
---
|
||||
|
||||
### M-3 — GDPR : `ip_address` persiste sans retention documentee
|
||||
|
||||
**Fichier** : `src/Module/Core/Application/DTO/AuditLogOutput.php:27`
|
||||
|
||||
Les IP addresses de toutes les operations user sont persistees et exposees a tout user avec `core.audit_log.view`. En EU c'est de la donnee personnelle sous GDPR. Avec une table append-only sans retention (cf. I-8), on cumule les IP indefiniment.
|
||||
|
||||
**Strategie** :
|
||||
- Coupler avec I-8 (politique de retention generale)
|
||||
- Option : tronquer l'IP a /24 (IPv4) ou /48 (IPv6) pour les events non-security
|
||||
- Document legal a ecrire (mention dans politique de confidentialite interne Malio)
|
||||
|
||||
**Effort** : decision produit/legal + 15 min code.
|
||||
|
||||
---
|
||||
|
||||
### M-5 — `stripSensitive()` commentaire mensonger
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Audit/AuditLogWriter.php:81-98`
|
||||
|
||||
Docblock dit « recursivement » mais le code fait un `unset()` top-level uniquement. Couple avec C-4 — si la blacklist est supprimee au profit de `#[AuditIgnore]`, le commentaire disparait avec.
|
||||
|
||||
**Strategie** : traiter via C-4, sinon corriger le commentaire en l'attendant.
|
||||
|
||||
**Effort** : 1 min si standalone.
|
||||
|
||||
---
|
||||
|
||||
### M-6 — LIKE escape comment imprecis
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:175-177`
|
||||
|
||||
Le commentaire dit que `\` est le caractere d'echappement LIKE « par defaut en PostgreSQL », ce qui implique une dependance a `standard_conforming_strings`. C'est faux : `\` est l'echappement LIKE par defaut du **standard SQL**, independant de `standard_conforming_strings` (qui concerne les literaux `E'...'`).
|
||||
|
||||
**Strategie** : corriger le commentaire pour lever l'ambiguite.
|
||||
|
||||
**Effort** : 1 min.
|
||||
|
||||
---
|
||||
|
||||
### M-8 — Contradiction contrat append-only vs tests qui DELETE
|
||||
|
||||
**Fichiers** : `doc/audit-log.md`, `tests/Module/Core/Infrastructure/Doctrine/AuditListenerTest.php`, `tests/Module/Core/Api/AuditLogApiTest.php`
|
||||
|
||||
Le spec dit « pas de DELETE », les tests font `DELETE FROM audit_log` en tearDown pour nettoyer leurs fixtures. Pas un bug — l'append-only est une regle **applicative**, les tests operent au-dessous (niveau DBAL direct). Juste a clarifier.
|
||||
|
||||
**Strategie** : ajouter une note dans `doc/audit-log.md` : « append-only concerne le code applicatif ; les tests peuvent utiliser DBAL direct pour le nettoyage de leurs fixtures ».
|
||||
|
||||
**Effort** : 2 min.
|
||||
|
||||
---
|
||||
|
||||
### M-9 — Logs Monolog `audit_write_failures` incluent le contexte `changes` complet
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/Doctrine/AuditListener.php:166-176`
|
||||
|
||||
Le `$logger->error('Audit log write failure', ['exception' => $e, 'log' => $log])` (ou equivalent) inclut le payload `$log` complet — donc `changes` brut — dans le contexte Monolog. Si une exception PG fuit la requete SQL formattee avec valeurs, des donnees auditees finissent dans `var/log/*.log` sans passer par `stripSensitive` ni `#[AuditIgnore]`.
|
||||
|
||||
**Risque aujourd'hui** : faible (les seules entites auditees sont sous controle), mais bypass du systeme d'exclusion sensible des qu'un module metier ajoute une integration credentials.
|
||||
|
||||
**Strategie** : sanitize le contexte avant le log. Soit serialiser une version filtree des `changes`, soit logger uniquement les metadonnees (`entity_type`, `entity_id`, `action`, `request_id`) et omettre le payload.
|
||||
|
||||
**Effort** : 10 min.
|
||||
|
||||
---
|
||||
|
||||
### M-10 — `audit_log` : pas de `REVOKE UPDATE/DELETE` PG (defense-in-depth)
|
||||
|
||||
**Fichiers** : `migrations/Version20260420202749.php`, `config/packages/doctrine.yaml`
|
||||
|
||||
L'invariant append-only n'est qu'une convention applicative. Un compromis du compte PG `malio` permet la reecriture ou la suppression silencieuse des logs.
|
||||
|
||||
**Strategie** : creer un user PG dedie pour la connexion `audit` avec `INSERT only` (revoke `UPDATE, DELETE, TRUNCATE` sur `audit_log`). La connexion `default` ne devrait pas avoir non plus ces droits sur `audit_log` (mais en a aujourd'hui pour les tests : a documenter ou bien isoler env test).
|
||||
|
||||
**Effort** : 30-45 min (creation user PG dans la migration, mise a jour `.env.docker` + `.env.prod.example`, test de regression sur le INSERT/SELECT).
|
||||
|
||||
---
|
||||
|
||||
### M-11 — `entity_type` non valide cote provider (?entity_type=foo → 200/0)
|
||||
|
||||
**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:118-130`
|
||||
|
||||
Un client qui passe `?entity_type[]=foo&entity_type[]=bar` recoit 200 + 0 resultat (pas 400). Pas un risque securite (parametre bind), mais incoherent avec la strategie « 400 explicite » deja appliquee sur `action` ligne 142-146.
|
||||
|
||||
**Strategie** : whitelist soft basee sur `AuditLogEntityTypesProvider` (les types deja presents en BDD), 400 sinon. Option : laisser tel quel pour ne pas couplеr les deux providers.
|
||||
|
||||
**Effort** : 15 min — non bloquant.
|
||||
|
||||
---
|
||||
|
||||
## Ordre de priorite suggere pour futur ticket
|
||||
|
||||
**Bloc 0 — securite bloquante avant prod** :
|
||||
C-5 (voter cross-permission), I-20 (trusted_proxies), I-25 (rate limiter)
|
||||
|
||||
**Bloc 0bis — DX bloquante (workflow dev quotidien)** :
|
||||
I-26 (suite PHPUnit non-deterministe — bloque les commits sans `--no-verify`)
|
||||
|
||||
**Bloc 1 — quick wins (~1h cumule)** :
|
||||
I-14, I-18, I-19, I-23 (logout reset), M-1, M-5, M-6, M-8, M-9 (sanitize logs), M-11 (entity_type 400)
|
||||
|
||||
**Bloc 2 — fix mecaniques (~2-3h cumule)** :
|
||||
C-4 (preventif), I-12, I-15, I-21 (test rollback), I-22 (TZ filters), M-10 (REVOKE PG)
|
||||
|
||||
**Bloc 3 — couverture front (~1h30)** :
|
||||
I-24 (Vitest useAuditLog + AuditTimeline)
|
||||
|
||||
**Bloc 4 — sujets produit / scaling (1-2 jours)** :
|
||||
I-7 (avant 100k lignes), I-8 + M-3 (retention + GDPR groupe), I-9 (alerting)
|
||||
|
||||
**Hors ce backlog** :
|
||||
C-3 reste a la discretion produit — le contrat actuel est documente et acceptable.
|
||||
@@ -89,19 +89,6 @@ Table non geree par Doctrine ORM (pas d'entite). Ecriture via DBAL uniquement po
|
||||
- **`performed_by` denormalise** : string, pas FK — le nom persiste meme si l'utilisateur est supprime
|
||||
- **Migration** : dans `migrations/` (namespace racine `DoctrineMigrations`) a cause du bug de tri alphabetique FQCN de Doctrine Migrations 3.x entre namespaces
|
||||
|
||||
### Contrat : ce que `audit_log` garantit (et ne garantit pas)
|
||||
|
||||
`audit_log` enregistre les **tentatives de modification** capturees par le `postFlush` Doctrine, ecrites via une connexion DBAL dediee (`audit_connection`). Ce choix est intentionnel : les lignes d'audit survivent au rollback eventuel de la transaction metier principale, ce qui permet de tracer les tentatives meme en cas d'echec applicatif.
|
||||
|
||||
**Conséquence à connaître** : si un controller enveloppe plusieurs operations dans une transaction explicite sur la connexion `default` et que cette transaction outermost rollback apres un flush intermediaire reussi, la ligne audit correspondante **persiste** sur la connexion `audit` alors que la modification metier a ete annulee. L'audit log peut donc contenir des lignes decrivant un etat qui n'existe pas en base metier.
|
||||
|
||||
En pratique :
|
||||
- Ce cas est rare dans un CRM interne (les rollbacks explicites outermost sont marginaux par rapport aux flushes atomiques).
|
||||
- La ligne audit garde son `request_id` qui permet une correlation post-mortem avec les logs applicatifs pour distinguer une tentative avortee d'un commit reussi.
|
||||
- Le comportement est volontaire — pas un bug. Pour un besoin de garantie « audit = reflet exact du commit outermost », il faudrait basculer l'audit sur la meme connexion que le metier (voir `AuditLogWriter`), au prix de perdre la resilience au rollback partiel.
|
||||
|
||||
L'audit est donc un **journal des intentions appliquees par l'ORM**, pas une source de verite transactionnelle sur l'etat final de la DB.
|
||||
|
||||
---
|
||||
|
||||
## Composants backend
|
||||
@@ -194,7 +181,7 @@ Listener Doctrine (pas EventSubscriber — deprecie Symfony 8) utilisant `#[AsDo
|
||||
- Entite sans `#[Auditable]` → ignoree
|
||||
- Batch (fixtures, import) → chaque entite auditee, groupees par `request_id`
|
||||
- Console → `performed_by = 'system'`, `ip_address = null`, `request_id = null`
|
||||
- ManyToMany / OneToMany : tracees via `UnitOfWork::getScheduledCollectionUpdates()` et `getScheduledCollectionDeletions()` (cf. `AuditListener::captureCollectionChange`). Payload `{fieldName: {added: [ids], removed: [ids]}}`, merge dans le log deja en attente de l'entite proprietaire si elle est aussi scheduled (insertion → snapshot enrichi, update → diff merge, delete → ignore car redondant avec le snapshot delete).
|
||||
- ManyToMany : non couvert par `getEntityChangeSet()` — limitation connue. Les changements de collections (ex: `User::$rbacRoles`) ne sont pas audites. Ajout futur possible via `getScheduledCollectionUpdates()`.
|
||||
|
||||
---
|
||||
|
||||
@@ -415,7 +402,7 @@ Ticket 1 ────► Ticket 2 ────► Ticket 3 ────┬──
|
||||
- **Type natif `uuid` PG** : 16 octets vs 36 en varchar, index 40% plus petit sur table append-only a croissance infinie
|
||||
- **Pattern swap-and-clear** dans `postFlush` : protection contre flush re-entrant
|
||||
- **Blacklist exact-match** sur noms de proprietes (`password`, `plainPassword`, `token`, `secret`) — en defense-in-depth avec `#[AuditIgnore]`
|
||||
- **Collections to-many auditees** : tracees via `getScheduledCollectionUpdates` / `getScheduledCollectionDeletions`, payload `{added, removed}` merge dans le changeset de l'entite proprietaire (cf. `AuditListener::captureCollectionChange`)
|
||||
- **ManyToMany non audite** : limitation connue, `getEntityChangeSet()` ne couvre pas les collections
|
||||
- **Erreur audit silencieuse** : loguee, jamais propagee — pas de retry/dead-letter (acceptable pour CRM interne)
|
||||
- **`entity_type` format `module.Entity`** : evite collisions si deux modules ont des entites de meme nom
|
||||
|
||||
|
||||
@@ -28,10 +28,8 @@
|
||||
"core": {
|
||||
"roles": "Gestion des rôles",
|
||||
"users": "Utilisateurs",
|
||||
"sites": "Sites",
|
||||
"audit_log": "Journal d'audit"
|
||||
},
|
||||
"sites": {
|
||||
"admin": "Sites"
|
||||
}
|
||||
},
|
||||
"dashboard": {
|
||||
@@ -78,10 +76,7 @@
|
||||
"delete": "Suppression"
|
||||
},
|
||||
"entity": {
|
||||
"core_user": "Utilisateur",
|
||||
"core_role": "Rôle",
|
||||
"core_permission": "Permission",
|
||||
"sites_site": "Site"
|
||||
"user": "Utilisateur"
|
||||
},
|
||||
"empty": "Aucune activité enregistrée",
|
||||
"no_results": "Aucun résultat pour ces filtres",
|
||||
|
||||
@@ -9,8 +9,7 @@
|
||||
<!-- Filtres -->
|
||||
<section class="mt-4 rounded border border-gray-200 bg-white p-4">
|
||||
<!-- Labels uniformes au-dessus : les composants Malio sont utilises sans
|
||||
leur `label` flottant interne pour ne pas mixer deux patterns de label.
|
||||
A revoir une fois le composant calendar Malio développé -->
|
||||
leur `label` flottant interne pour ne pas mixer deux patterns de label. -->
|
||||
<div class="grid grid-cols-1 items-start gap-3 md:grid-cols-5">
|
||||
<!-- TODO(malio-ui): remplacer par un composant Malio quand la lib
|
||||
exposera un datetime picker. Cf. exception documentee dans
|
||||
@@ -60,20 +59,26 @@
|
||||
v-model="performedByInput"
|
||||
icon-name="mdi:account-search"
|
||||
input-class="text-sm"
|
||||
group-class="h-10"
|
||||
/>
|
||||
</div>
|
||||
<!-- TODO(malio-ui): remplacer par MalioSelect quand la lib
|
||||
supportera de maniere fiable des options a valeur string
|
||||
(cf. note Lesstime CLAUDE.md). Exception documentee dans
|
||||
CLAUDE.md (section "Composants formulaires"). -->
|
||||
<div>
|
||||
<label class="mb-1 block text-xs font-medium text-gray-600">
|
||||
{{ t('audit.filters.action') }}
|
||||
</label>
|
||||
<div class="[&>div>div]:!mt-0">
|
||||
<MalioSelect
|
||||
v-model="actionValue"
|
||||
:options="actionOptions"
|
||||
text-field="text-sm"
|
||||
text-value="text-sm"
|
||||
/>
|
||||
</div>
|
||||
<select
|
||||
v-model="actionValue"
|
||||
class="h-[40px] w-full rounded-md border border-m-muted bg-white px-3 text-sm outline-none focus-visible:border-2 focus-visible:border-m-primary"
|
||||
>
|
||||
<option value="">{{ t('audit.filters.all_actions') }}</option>
|
||||
<option v-for="opt in actionOptions" :key="opt.value" :value="opt.value">
|
||||
{{ opt.label }}
|
||||
</option>
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -110,10 +115,7 @@
|
||||
</span>
|
||||
</template>
|
||||
<template #cell-entityType="{ item }">
|
||||
<span
|
||||
class="text-xs"
|
||||
:title="item.entityType as string"
|
||||
>{{ formatEntityType(item.entityType as string) }}</span>
|
||||
<span class="font-mono text-xs">{{ item.entityType }}</span>
|
||||
</template>
|
||||
<template #cell-entityId="{ item }">
|
||||
<span class="font-mono text-xs">{{ item.entityId }}</span>
|
||||
@@ -132,11 +134,8 @@
|
||||
<div v-if="selectedEntry">
|
||||
<AuditLogDetail :entry="selectedEntry" />
|
||||
<div class="mt-4 border-t border-gray-200 pt-3">
|
||||
<h3
|
||||
class="text-sm font-medium text-gray-700 mb-2"
|
||||
:title="selectedEntry.entityType"
|
||||
>
|
||||
{{ formatEntityType(selectedEntry.entityType) }} #{{ selectedEntry.entityId }}
|
||||
<h3 class="text-sm font-medium text-gray-700 mb-2">
|
||||
{{ selectedEntry.entityType }} #{{ selectedEntry.entityId }}
|
||||
</h3>
|
||||
<AuditTimeline
|
||||
:entity-type="selectedEntry.entityType"
|
||||
@@ -152,18 +151,10 @@
|
||||
import { computed, nextTick, onMounted, reactive, ref, watch } from 'vue'
|
||||
import type { AuditLogEntry, AuditLogFilters } from '~/shared/types'
|
||||
|
||||
const { t, te } = useI18n()
|
||||
const { t } = useI18n()
|
||||
const { can } = usePermissions()
|
||||
const { fetchLogsCached, fetchEntityTypes } = useAuditLog()
|
||||
|
||||
// Traduit un identifiant `module.Entity` (ex: `core.User`, `sites.Site`) en
|
||||
// libelle lisible via la cle i18n `audit.entity.<module>_<entity>`. Si aucune
|
||||
// traduction n'existe, on retombe sur l'identifiant brut pour rester debug-friendly.
|
||||
function formatEntityType(type: string): string {
|
||||
const key = `audit.entity.${type.toLowerCase().replace(/\./g, '_')}`
|
||||
return te(key) ? t(key) : type
|
||||
}
|
||||
|
||||
// Protection cote UI : le middleware `modules.global.ts` filtre deja les
|
||||
// routes desactivees, mais si quelqu'un atterit ici sans la permission on
|
||||
// renvoie une 403 plutot que de flasher un ecran vide.
|
||||
@@ -189,24 +180,19 @@ const filters = reactive<AuditLogFilters>({
|
||||
// Attention : les composants Malio attendent `{ label, value }` (pas `{ text }`).
|
||||
const selectedEntityTypes = ref<(string | number)[]>([])
|
||||
const entityTypes = ref<string[]>([])
|
||||
// On garde l'identifiant technique comme `value` pour l'envoi API, mais on
|
||||
// affiche le libelle traduit quand il existe (fallback: identifiant brut).
|
||||
const entityTypeOptions = computed(() =>
|
||||
entityTypes.value.map(type => ({ value: type, label: formatEntityType(type) })),
|
||||
entityTypes.value.map(t => ({ value: t, label: t })),
|
||||
)
|
||||
|
||||
// Bind champ performedBy : MalioInputText attend `string | null`, on ne peut
|
||||
// pas binder directement un `string | undefined` reactive.
|
||||
const performedByInput = ref<string>('')
|
||||
|
||||
// Action : '' = "toutes les actions". On declare l'option dans `actionOptions`
|
||||
// plutot que via `emptyOptionLabel` (qui n'inclut pas l'option vide dans
|
||||
// `props.options`, donc `selectedLabel` reste vide). On evite aussi `value: null`
|
||||
// car MalioSelect grise visuellement les options dont la valeur est `null`
|
||||
// (Select.vue:137) — on utilise donc une chaine vide comme sentinelle.
|
||||
// Action : MalioSelect ne gere pas fiablement des options a valeur string (cf.
|
||||
// note Lesstime CLAUDE.md). On utilise un `<select>` natif stylise comme les
|
||||
// inputs dates pour garder un look coherent. '' = "toutes les actions".
|
||||
const actionValue = ref<string>('')
|
||||
const actionOptions = [
|
||||
{ value: '', label: t('audit.filters.all_actions') },
|
||||
{ value: 'create', label: t('audit.action.create') },
|
||||
{ value: 'update', label: t('audit.action.update') },
|
||||
{ value: 'delete', label: t('audit.action.delete') },
|
||||
@@ -244,7 +230,7 @@ const rows = computed(() =>
|
||||
|
||||
const drawerTitle = computed(() =>
|
||||
selectedEntry.value
|
||||
? `${formatEntityType(selectedEntry.value.entityType)} #${selectedEntry.value.entityId}`
|
||||
? `${selectedEntry.value.entityType} #${selectedEntry.value.entityId}`
|
||||
: t('audit.detail_title'),
|
||||
)
|
||||
|
||||
@@ -315,9 +301,16 @@ async function loadEntries(): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
// Debounce auto-importe depuis `frontend/shared/utils/debounce.ts` : evite
|
||||
// un refetch a chaque frappe sur le champ texte performedBy (reseau + SQL)
|
||||
// et laisse l'utilisateur finir sa saisie avant de lancer la requete.
|
||||
// Debounce utilitaire pour le champ texte performedBy : evite un refetch a
|
||||
// chaque frappe (reseau + SQL) et laisse l'utilisateur finir sa saisie.
|
||||
function debounce<T extends (...args: never[]) => void>(fn: T, delay: number): T {
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
return ((...args: Parameters<T>) => {
|
||||
if (null !== timer) clearTimeout(timer)
|
||||
timer = setTimeout(() => fn(...args), delay)
|
||||
}) as T
|
||||
}
|
||||
|
||||
const debouncedReload = debounce(() => loadEntries(), 300)
|
||||
|
||||
function toIso(localDateTime: string): string {
|
||||
@@ -376,7 +369,7 @@ watch(selectedEntityTypes, values => {
|
||||
loadEntries()
|
||||
})
|
||||
|
||||
// Sync MalioSelect action -> filters.action.
|
||||
// Sync select action natif -> filters.action.
|
||||
watch(actionValue, value => {
|
||||
if (watchersSuspended) return
|
||||
filters.action = value === '' ? undefined : value
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
<MalioButton
|
||||
v-if="can('core.roles.manage')"
|
||||
:label="t('admin.roles.newRole')"
|
||||
icon-name="mdi:add-bold"
|
||||
icon-name="mdi:plus"
|
||||
icon-position="left"
|
||||
@click="openCreateDrawer"
|
||||
/>
|
||||
|
||||
@@ -27,8 +27,8 @@
|
||||
<MalioButton
|
||||
label="Se connecter"
|
||||
button-class="w-full"
|
||||
type="submit"
|
||||
:disabled="isSubmitting"
|
||||
@click="handleSubmit"
|
||||
/>
|
||||
<p class="font-bold">v{{ version }}</p>
|
||||
</form>
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
<MalioButton
|
||||
v-if="can('sites.manage')"
|
||||
:label="t('admin.sites.newSite')"
|
||||
icon-name="mdi:add-bold"
|
||||
icon-name="mdi:plus"
|
||||
icon-position="left"
|
||||
@click="openCreateDrawer"
|
||||
/>
|
||||
|
||||
856
frontend/package-lock.json
generated
856
frontend/package-lock.json
generated
File diff suppressed because it is too large
Load Diff
@@ -17,7 +17,7 @@
|
||||
"test:e2e:ui": "playwright test --ui"
|
||||
},
|
||||
"dependencies": {
|
||||
"@malio/layer-ui": "^1.5.0",
|
||||
"@malio/layer-ui": "^1.4.2",
|
||||
"@nuxt/icon": "^2.2.1",
|
||||
"@nuxtjs/i18n": "^10.2.3",
|
||||
"@nuxtjs/tailwindcss": "^6.14.0",
|
||||
|
||||
@@ -1,2 +1,2 @@
|
||||
User-Agent: *
|
||||
Disallow: /
|
||||
Disallow:
|
||||
|
||||
@@ -77,13 +77,14 @@
|
||||
|
||||
<!-- Lazy loading : bouton "Voir plus" si plus de pages. -->
|
||||
<div v-if="hasMore" class="mt-4 flex justify-center">
|
||||
<MalioButton
|
||||
variant="tertiary"
|
||||
:label="loading ? t('common.loading') : t('audit.timeline.load_more')"
|
||||
<button
|
||||
type="button"
|
||||
class="px-3 py-1.5 text-sm rounded border border-gray-300 hover:bg-gray-50 disabled:opacity-60"
|
||||
:disabled="loading"
|
||||
button-class="text-sm"
|
||||
@click="loadMore"
|
||||
/>
|
||||
>
|
||||
{{ loading ? t('common.loading') : t('audit.timeline.load_more') }}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -162,27 +163,22 @@ function dotClass(action: string): string {
|
||||
}
|
||||
|
||||
// Relativise une date via Intl.RelativeTimeFormat. On selectionne l'unite
|
||||
// la plus grossiere possible (secondes < minutes < heures < jours < semaines
|
||||
// < mois < annees). La locale suit dynamiquement celle de l'app pour qu'un
|
||||
// switch de langue prenne effet sans nouveau mount (recomputed = cache
|
||||
// par-locale). Paliers mois/annee approximes (30.44j / 365.25j) : suffisant
|
||||
// pour un affichage humain, la tooltip absoluteDate garde la date exacte.
|
||||
// la plus grossiere possible (minutes < heures < jours < semaines). La
|
||||
// locale suit dynamiquement celle de l'app pour qu'un switch de langue
|
||||
// prenne effet sans nouveau mount (recomputed = cache par-locale).
|
||||
const rtf = computed(() => new Intl.RelativeTimeFormat(locale.value, { numeric: 'auto' }))
|
||||
|
||||
function relativeDate(iso: string): string {
|
||||
const diffMs = Date.now() - new Date(iso).getTime()
|
||||
const diffSec = Math.round(diffMs / 1000)
|
||||
const absSec = Math.abs(diffSec)
|
||||
const sign = -Math.sign(diffSec)
|
||||
const fmt = rtf.value
|
||||
|
||||
if (absSec < 60) return fmt.format(sign * absSec, 'second')
|
||||
if (absSec < 3600) return fmt.format(sign * Math.round(absSec / 60), 'minute')
|
||||
if (absSec < 86400) return fmt.format(sign * Math.round(absSec / 3600), 'hour')
|
||||
if (absSec < 604800) return fmt.format(sign * Math.round(absSec / 86400), 'day')
|
||||
if (absSec < 2629800) return fmt.format(sign * Math.round(absSec / 604800), 'week') // < ~30.44j
|
||||
if (absSec < 31557600) return fmt.format(sign * Math.round(absSec / 2629800), 'month') // < ~365.25j
|
||||
return fmt.format(sign * Math.round(absSec / 31557600), 'year')
|
||||
if (absSec < 60) return fmt.format(-Math.sign(diffSec) * Math.abs(diffSec), 'second')
|
||||
if (absSec < 3600) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 60), 'minute')
|
||||
if (absSec < 86400) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 3600), 'hour')
|
||||
if (absSec < 604800) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 86400), 'day')
|
||||
return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 604800), 'week')
|
||||
}
|
||||
|
||||
function absoluteDate(iso: string): string {
|
||||
|
||||
@@ -2,23 +2,14 @@
|
||||
* Composable de lecture des modules actifs (source : `/api/modules`).
|
||||
*
|
||||
* State singleton au niveau module : `useSidebar` suit la meme convention.
|
||||
* Chargement idempotent via le flag `loaded`, reset automatique au logout
|
||||
* via `onAuthSessionCleared` (cf. CLAUDE.md : « composables avec state
|
||||
* singleton doivent etre reinitialises au logout »).
|
||||
* Chargement idempotent via le flag `loaded`, reset explicite au logout
|
||||
* (voir pages/logout.vue).
|
||||
*/
|
||||
import { ref } from 'vue'
|
||||
import { onAuthSessionCleared } from '~/shared/stores/auth'
|
||||
|
||||
const activeModuleIds = ref<string[]>([])
|
||||
const loaded = ref(false)
|
||||
|
||||
function resetModulesState(): void {
|
||||
activeModuleIds.value = []
|
||||
loaded.value = false
|
||||
}
|
||||
|
||||
onAuthSessionCleared(resetModulesState)
|
||||
|
||||
export function useModules() {
|
||||
async function loadModules() {
|
||||
try {
|
||||
@@ -44,7 +35,8 @@ export function useModules() {
|
||||
}
|
||||
|
||||
function resetModules() {
|
||||
resetModulesState()
|
||||
activeModuleIds.value = []
|
||||
loaded.value = false
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -1,23 +1,10 @@
|
||||
import { ref } from 'vue'
|
||||
import type { SidebarSection } from '~/shared/types'
|
||||
import { onAuthSessionCleared } from '~/shared/stores/auth'
|
||||
|
||||
const sections = ref<SidebarSection[]>([])
|
||||
const disabledRoutes = ref<string[]>([])
|
||||
const loaded = ref(false)
|
||||
|
||||
function resetSidebarState(): void {
|
||||
sections.value = []
|
||||
disabledRoutes.value = []
|
||||
loaded.value = false
|
||||
}
|
||||
|
||||
// Auto-enregistrement singleton : purge la sidebar sur 401/logout pour
|
||||
// eviter qu'un nouvel utilisateur logue sur le meme onglet voie transitoirement
|
||||
// les items de l'ancienne session (cf. CLAUDE.md : « composables avec state
|
||||
// singleton doivent etre reinitialises au logout »).
|
||||
onAuthSessionCleared(resetSidebarState)
|
||||
|
||||
export function useSidebar() {
|
||||
async function loadSidebar() {
|
||||
try {
|
||||
@@ -44,7 +31,9 @@ export function useSidebar() {
|
||||
}
|
||||
|
||||
function resetSidebar() {
|
||||
resetSidebarState()
|
||||
sections.value = []
|
||||
disabledRoutes.value = []
|
||||
loaded.value = false
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -1,52 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
import { debounce } from '../debounce'
|
||||
|
||||
describe('debounce', () => {
|
||||
it('attend delay ms avant d\'appeler fn une seule fois apres plusieurs invocations rapides', () => {
|
||||
vi.useFakeTimers()
|
||||
const fn = vi.fn()
|
||||
const debounced = debounce(fn, 100)
|
||||
|
||||
debounced()
|
||||
debounced()
|
||||
debounced()
|
||||
expect(fn).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(100)
|
||||
expect(fn).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('passe les arguments du dernier appel a fn', () => {
|
||||
vi.useFakeTimers()
|
||||
const fn = vi.fn<(a: string, b: number) => void>()
|
||||
const debounced = debounce(fn, 50)
|
||||
|
||||
debounced('first', 1)
|
||||
debounced('second', 2)
|
||||
debounced('third', 3)
|
||||
vi.advanceTimersByTime(50)
|
||||
|
||||
expect(fn).toHaveBeenCalledTimes(1)
|
||||
expect(fn).toHaveBeenCalledWith('third', 3)
|
||||
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('autorise plusieurs executions espacees dans le temps', () => {
|
||||
vi.useFakeTimers()
|
||||
const fn = vi.fn()
|
||||
const debounced = debounce(fn, 50)
|
||||
|
||||
debounced()
|
||||
vi.advanceTimersByTime(50)
|
||||
expect(fn).toHaveBeenCalledTimes(1)
|
||||
|
||||
debounced()
|
||||
vi.advanceTimersByTime(50)
|
||||
expect(fn).toHaveBeenCalledTimes(2)
|
||||
|
||||
vi.useRealTimers()
|
||||
})
|
||||
})
|
||||
@@ -1,15 +0,0 @@
|
||||
/**
|
||||
* Utilitaire de debounce partage.
|
||||
*
|
||||
* Retarde l'execution d'une fonction : chaque appel reset un timer et
|
||||
* l'execution reelle n'a lieu qu'apres `delay` ms sans nouvelle invocation.
|
||||
* Utile pour eviter un spam d'appels reseau sur un champ de recherche
|
||||
* (une requete par touche -> une seule requete apres la derniere frappe).
|
||||
*/
|
||||
export function debounce<T extends (...args: never[]) => void>(fn: T, delay: number): T {
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
return ((...args: Parameters<T>) => {
|
||||
if (null !== timer) clearTimeout(timer)
|
||||
timer = setTimeout(() => fn(...args), delay)
|
||||
}) as T
|
||||
}
|
||||
44
makefile
44
makefile
@@ -1,5 +1,3 @@
|
||||
.DEFAULT_GOAL := help
|
||||
|
||||
# Permet d'utiliser un .env.docker.local pour override
|
||||
ENV_DEFAULT = infra/dev/.env.docker
|
||||
ENV_LOCAL = infra/dev/.env.docker.local
|
||||
@@ -24,48 +22,6 @@ FILES =
|
||||
|
||||
#========================================================================================
|
||||
|
||||
# Affiche l'aide — cible par defaut (make ou make help)
|
||||
help:
|
||||
@printf "\n \033[1mColtura — Commandes make\033[0m\n\n"
|
||||
@printf " \033[1;33mContainers\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "start" "Demarrer les containers Docker"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "stop" "Arreter les containers"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "restart" "Redemarrer les containers"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "shell" "Shell bash dans le container PHP (user app)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "shell-root" "Shell bash dans le container PHP (root)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "logs-dev" "Tail des logs Symfony (var/log/dev.log)"
|
||||
@printf "\n \033[1;33mInstallation\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "install" "Install complet (composer, migrations, fixtures, build Nuxt)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "reset" "Tout supprimer et reinstaller (ATTENTION : drop la BDD)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "composer-install" "Composer install + generation cles JWT"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "build-nuxtJS" "npm install + build Nuxt (prod)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "build-without-cache" "Rebuild des images Docker sans cache"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "copy-git-hook" "Copie les hooks git (pre-commit, commit-msg)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "node-use" "Force la version Node via nvm"
|
||||
@printf "\n \033[1;33mFrontend (Nuxt)\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "dev-nuxt" "Serveur dev Nuxt avec hot reload (port 3004)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "nuxt-lint" "Lint TypeScript/Vue"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "nuxt-lint-fix" "Lint + auto-fix"
|
||||
@printf "\n \033[1;33mBase de donnees\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "migration-migrate" "Lancer les migrations Doctrine"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "fixtures" "Charger les fixtures"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "sync-permissions" "Synchroniser le catalogue RBAC"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "db-reset" "Reset BDD (drop + migrate + fixtures + perms)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "db-restart" "Restart du container BDD"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "test-db-setup" "Cree et initialise la BDD de test"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "cache-clear" "Vider le cache Symfony"
|
||||
@printf "\n \033[1;33mTests\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "test" "PHPUnit (tests back)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "nuxt-test" "Vitest (tests unitaires front)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "test-all" "PHPUnit + Vitest"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "test-e2e" "Playwright (tests E2E front)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "test-e2e-ui" "Playwright UI interactive (debug)"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "seed-e2e" "Seed les 6 personas E2E"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "install-e2e-deps" "One-time : Chromium + libs systeme (sudo)"
|
||||
@printf "\n \033[1;33mQualite code\033[0m\n"
|
||||
@printf " \033[36m%-28s\033[0m %s\n" "php-cs-fixer-allow-risky" "Fix code style PHP (utilise par le pre-commit)"
|
||||
@printf "\n Plus de details : \033[4mREADME.md\033[0m, \033[4mCLAUDE.md\033[0m\n\n"
|
||||
|
||||
env-init:
|
||||
@cp --update=none $(ENV_DEFAULT) $(ENV_LOCAL)
|
||||
|
||||
|
||||
@@ -18,11 +18,11 @@
|
||||
<server name="KERNEL_CLASS" value="App\Kernel" />
|
||||
|
||||
<!-- ###+ symfony/framework-bundle ### -->
|
||||
<!-- APP_ENV est defini uniquement via <server force="true"> ci-dessus.
|
||||
Ne PAS re-declarer ici en <env> : une ligne redondante mene
|
||||
directement au bug ou un dev met "dev" en pensant que <server>
|
||||
gere tout, puis supprime <server> ensuite et <env> prend le
|
||||
dessus silencieusement (cf. cc8d5 du fix pre-existant). -->
|
||||
<!-- APP_ENV est force a "test" en <server> ci-dessus : on ne doit PAS
|
||||
re-injecter "dev" ici via <env>, sinon la suite tourne sous
|
||||
framework.test=false et `test.service_container` n'est pas cable
|
||||
(cf. cc8d5 du fix pre-existant). -->
|
||||
<env name="APP_ENV" value="test"/>
|
||||
<env name="APP_SECRET" value=""/>
|
||||
<env name="APP_SHARE_DIR" value="var/share"/>
|
||||
<!-- ###- symfony/framework-bundle ### -->
|
||||
|
||||
@@ -15,11 +15,12 @@ use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserProcessor;
|
||||
use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserRbacProcessor;
|
||||
use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\MeProvider;
|
||||
use App\Module\Core\Infrastructure\Doctrine\DoctrineUserRepository;
|
||||
// Note architecture : User.php n'importe plus rien depuis le module Sites.
|
||||
// Les type-hints utilisent SiteInterface (Shared/Contract) et le mapping ORM
|
||||
// pointe vers la meme interface, resolue vers la classe concrete Site au boot
|
||||
// via `doctrine.orm.resolve_target_entities` (cf. config/packages/doctrine.yaml).
|
||||
// C'est le pattern officiel Doctrine pour les bounded contexts DDD.
|
||||
// Note architecture : User.php utilise SiteInterface (Shared) pour les
|
||||
// type-hints afin de ne pas coupler le module Core au module Sites.
|
||||
// La seule reference concrete a Site subsiste dans les metadonnees ORM
|
||||
// (targetEntity) via FQCN string, ce qui est obligatoire pour Doctrine.
|
||||
// SiteNotAuthorizedException est importee depuis Shared (sa location canonique).
|
||||
use App\Module\Sites\Domain\Entity\Site;
|
||||
use App\Shared\Domain\Attribute\Auditable;
|
||||
use App\Shared\Domain\Attribute\AuditIgnore;
|
||||
use App\Shared\Domain\Contract\SiteInterface;
|
||||
@@ -138,12 +139,10 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
||||
* Le groupe `user:list` a ete retire deliberement (securite : evite
|
||||
* de fuiter la liste des sites de chaque user via GET /api/users).
|
||||
*
|
||||
* @var Collection<int, SiteInterface>
|
||||
* @var Collection<int, Site>
|
||||
*/
|
||||
#[ORM\ManyToMany(targetEntity: SiteInterface::class, inversedBy: 'users', fetch: 'LAZY')]
|
||||
#[ORM\ManyToMany(targetEntity: 'App\Module\Sites\Domain\Entity\Site', inversedBy: 'users', fetch: 'LAZY')]
|
||||
#[ORM\JoinTable(name: 'user_site')]
|
||||
#[ORM\JoinColumn(name: 'user_id', referencedColumnName: 'id', onDelete: 'CASCADE')]
|
||||
#[ORM\InverseJoinColumn(name: 'site_id', referencedColumnName: 'id', onDelete: 'CASCADE')]
|
||||
#[Groups(['me:read', 'user:rbac:read', 'user:rbac:write'])]
|
||||
private Collection $sites;
|
||||
|
||||
@@ -163,7 +162,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
||||
* le prechargement pour /api/me. Le groupe `user:list` a ete retire
|
||||
* deliberement (securite : evite de fuiter le site actif via /api/users).
|
||||
*/
|
||||
#[ORM\ManyToOne(targetEntity: SiteInterface::class, fetch: 'LAZY')]
|
||||
#[ORM\ManyToOne(targetEntity: 'App\Module\Sites\Domain\Entity\Site', fetch: 'LAZY')]
|
||||
#[ORM\JoinColumn(name: 'current_site_id', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')]
|
||||
#[Groups(['me:read'])]
|
||||
private ?SiteInterface $currentSite = null;
|
||||
@@ -379,7 +378,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
||||
}
|
||||
|
||||
/**
|
||||
* @return Collection<int, SiteInterface>
|
||||
* @return Collection<int, Site>
|
||||
*/
|
||||
public function getSites(): Collection
|
||||
{
|
||||
@@ -393,8 +392,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
||||
* session Doctrine (cf. ticket 2 review point #1).
|
||||
*
|
||||
* Le parametre est type SiteInterface pour eviter le couplage Core → Sites.
|
||||
* La classe concrete injectee au runtime est resolue par Doctrine via
|
||||
* `resolve_target_entities` (cf. note architecture en tete de fichier).
|
||||
* En pratique seule App\Module\Sites\Domain\Entity\Site est passee ici.
|
||||
*/
|
||||
public function addSite(SiteInterface $site): static
|
||||
{
|
||||
|
||||
@@ -13,7 +13,7 @@ use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
|
||||
/**
|
||||
* @implements ProcessorInterface<User, User>
|
||||
*/
|
||||
final class UserPasswordHasherProcessor implements ProcessorInterface
|
||||
class UserPasswordHasherProcessor implements ProcessorInterface
|
||||
{
|
||||
public function __construct(
|
||||
#[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')]
|
||||
|
||||
@@ -250,12 +250,7 @@ final class UserRbacProcessor implements ProcessorInterface
|
||||
}
|
||||
|
||||
foreach (self::COLLECTION_MAP as $jsonKey => $accessors) {
|
||||
// La garde ne doit sauter la restauration que si le payload fournit
|
||||
// un VRAI tableau pour cette cle. Un `null`, un scalaire ou un autre
|
||||
// type doivent etre traites comme "cle absente" : sinon un payload
|
||||
// `{"sites": null}` contourne la restauration et laisse API Platform
|
||||
// vider la collection silencieusement (bypass de la garde).
|
||||
if (array_key_exists($jsonKey, $payload) && is_array($payload[$jsonKey])) {
|
||||
if (array_key_exists($jsonKey, $payload)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -15,7 +15,6 @@ use Doctrine\DBAL\ArrayParameterType;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\DBAL\Query\QueryBuilder;
|
||||
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||
|
||||
/**
|
||||
* Provider API Platform pour la resource AuditLog.
|
||||
@@ -68,11 +67,7 @@ final readonly class AuditLogProvider implements ProviderInterface
|
||||
*/
|
||||
private function provideCollection(Operation $operation, array $context): DbalPaginator
|
||||
{
|
||||
// `page` brut peut etre <= 0 (parametre client) → OFFSET negatif → 500 PG
|
||||
// (`SQLSTATE[22023] OFFSET must not be negative`). API Platform clampe
|
||||
// `itemsPerPage` au max de la resource mais pas `page` ; on impose un
|
||||
// minimum a 1 cote provider.
|
||||
$page = max(1, $this->pagination->getPage($context));
|
||||
$page = $this->pagination->getPage($context);
|
||||
$itemsPerPage = $this->pagination->getLimit($operation, $context);
|
||||
$offset = ($page - 1) * $itemsPerPage;
|
||||
$filters = $this->extractFilters($context['filters'] ?? []);
|
||||
@@ -133,42 +128,20 @@ final readonly class AuditLogProvider implements ProviderInterface
|
||||
}
|
||||
}
|
||||
|
||||
foreach (['entity_id', 'performed_by'] as $key) {
|
||||
foreach (['entity_id', 'action', 'performed_by'] as $key) {
|
||||
if (isset($raw[$key]) && is_string($raw[$key]) && '' !== $raw[$key]) {
|
||||
$filters[$key] = $raw[$key];
|
||||
}
|
||||
}
|
||||
|
||||
// `action` : whitelist stricte. Un input hors-liste provoquait avant
|
||||
// un simple match vide (resultat 0 ligne) mais permettait d'incrementer
|
||||
// le log applicatif a chaque variation ; on rejette en 400 explicite.
|
||||
if (isset($raw['action']) && is_string($raw['action']) && '' !== $raw['action']) {
|
||||
if (!in_array($raw['action'], ['create', 'update', 'delete'], true)) {
|
||||
throw new BadRequestHttpException(
|
||||
'Filtre "action" invalide : valeurs autorisees create|update|delete.',
|
||||
);
|
||||
}
|
||||
$filters['action'] = $raw['action'];
|
||||
}
|
||||
|
||||
// Filtres de plage `performed_at[after]` / `performed_at[before]`.
|
||||
// Sans validation, un input malforme remonte jusqu'a Postgres qui
|
||||
// leve `SQLSTATE[22007]: invalid input syntax for type timestamp` →
|
||||
// 500 Internal Server Error, log Monolog pollue, mauvaise UX API.
|
||||
// On valide en amont et on rejette en 400 explicite.
|
||||
if (isset($raw['performed_at']) && is_array($raw['performed_at'])) {
|
||||
$range = $raw['performed_at'];
|
||||
foreach (['after', 'before'] as $bound) {
|
||||
if (!isset($range[$bound]) || !is_string($range[$bound]) || '' === $range[$bound]) {
|
||||
continue;
|
||||
}
|
||||
if (false === strtotime($range[$bound])) {
|
||||
throw new BadRequestHttpException(sprintf(
|
||||
'Filtre "performed_at[%s]" invalide : date ISO 8601 attendue (ex: 2026-04-22T00:00:00Z).',
|
||||
$bound,
|
||||
));
|
||||
}
|
||||
$filters['performed_at_'.$bound] = $range[$bound];
|
||||
if (isset($range['after']) && is_string($range['after']) && '' !== $range['after']) {
|
||||
$filters['performed_at_after'] = $range['after'];
|
||||
}
|
||||
if (isset($range['before']) && is_string($range['before']) && '' !== $range['before']) {
|
||||
$filters['performed_at_before'] = $range['before'];
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ use Symfony\Bundle\SecurityBundle\Security;
|
||||
/**
|
||||
* @implements ProviderInterface<object>
|
||||
*/
|
||||
final class MeProvider implements ProviderInterface
|
||||
class MeProvider implements ProviderInterface
|
||||
{
|
||||
public function __construct(
|
||||
private readonly Security $security,
|
||||
|
||||
@@ -9,7 +9,7 @@ use App\Module\Core\Domain\Repository\PermissionRepositoryInterface;
|
||||
use App\Module\Core\Domain\Repository\RoleRepositoryInterface;
|
||||
use App\Module\Core\Domain\Repository\UserRepositoryInterface;
|
||||
use App\Module\Core\Domain\Security\SystemRoles;
|
||||
use App\Shared\Domain\Contract\SiteProviderInterface;
|
||||
use App\Module\Sites\Domain\Repository\SiteRepositoryInterface;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use RuntimeException;
|
||||
use Symfony\Component\Console\Attribute\AsCommand;
|
||||
@@ -50,7 +50,7 @@ final class SeedE2ECommand extends Command
|
||||
private readonly UserRepositoryInterface $userRepository,
|
||||
private readonly RoleRepositoryInterface $roleRepository,
|
||||
private readonly PermissionRepositoryInterface $permissionRepository,
|
||||
private readonly SiteProviderInterface $siteProvider,
|
||||
private readonly SiteRepositoryInterface $siteRepository,
|
||||
private readonly UserPasswordHasherInterface $passwordHasher,
|
||||
) {
|
||||
parent::__construct();
|
||||
@@ -60,17 +60,6 @@ final class SeedE2ECommand extends Command
|
||||
{
|
||||
$io = new SymfonyStyle($input, $output);
|
||||
|
||||
// Garde-fou : cette commande cree un compte admin avec un mot de passe
|
||||
// hardcode. Elle ne doit JAMAIS tourner hors dev/test, meme si le
|
||||
// fichier se retrouve embarque dans une image prod par accident (le
|
||||
// .dockerignore a la racine est la premiere ligne de defense).
|
||||
$env = $_SERVER['APP_ENV'] ?? 'prod';
|
||||
if (!in_array($env, ['dev', 'test'], true)) {
|
||||
$io->error(sprintf('app:seed-e2e est refuse en environnement "%s". Autorise uniquement en dev/test.', $env));
|
||||
|
||||
return Command::FAILURE;
|
||||
}
|
||||
|
||||
$userRole = $this->roleRepository->findByCode(SystemRoles::USER_CODE);
|
||||
|
||||
if (null === $userRole) {
|
||||
@@ -82,7 +71,7 @@ final class SeedE2ECommand extends Command
|
||||
return Command::FAILURE;
|
||||
}
|
||||
|
||||
$defaultSite = $this->siteProvider->findByName(self::DEFAULT_SITE_NAME);
|
||||
$defaultSite = $this->siteRepository->findByName(self::DEFAULT_SITE_NAME);
|
||||
|
||||
// Pas de fail fatal si le site manque : les tests sidebar/login
|
||||
// n'en dependent pas. Les tests sites-scope-bypass (a venir) le feront.
|
||||
|
||||
@@ -8,9 +8,9 @@ use App\Module\Core\Domain\Entity\Role;
|
||||
use App\Module\Core\Domain\Entity\User;
|
||||
use App\Module\Core\Domain\Repository\RoleRepositoryInterface;
|
||||
use App\Module\Core\Domain\Security\SystemRoles;
|
||||
use App\Module\Sites\Domain\Entity\Site;
|
||||
use App\Module\Sites\Domain\Repository\SiteRepositoryInterface;
|
||||
use App\Module\Sites\Infrastructure\DataFixtures\SitesFixtures;
|
||||
use App\Shared\Domain\Contract\SiteInterface;
|
||||
use App\Shared\Domain\Contract\SiteProviderInterface;
|
||||
use Doctrine\Bundle\FixturesBundle\Fixture;
|
||||
use Doctrine\Common\DataFixtures\DependentFixtureInterface;
|
||||
use Doctrine\Persistence\ObjectManager;
|
||||
@@ -39,7 +39,7 @@ class AppFixtures extends Fixture implements DependentFixtureInterface
|
||||
public function __construct(
|
||||
private readonly UserPasswordHasherInterface $passwordHasher,
|
||||
private readonly RoleRepositoryInterface $roleRepository,
|
||||
private readonly SiteProviderInterface $siteProvider,
|
||||
private readonly SiteRepositoryInterface $siteRepository,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -135,9 +135,9 @@ class AppFixtures extends Fixture implements DependentFixtureInterface
|
||||
return $role;
|
||||
}
|
||||
|
||||
private function requireSite(string $name): SiteInterface
|
||||
private function requireSite(string $name): Site
|
||||
{
|
||||
$site = $this->siteProvider->findByName($name);
|
||||
$site = $this->siteRepository->findByName($name);
|
||||
|
||||
if (null === $site) {
|
||||
throw new RuntimeException(sprintf(
|
||||
|
||||
@@ -102,14 +102,6 @@ final class AuditListener
|
||||
$em = $args->getObjectManager();
|
||||
$uow = $em->getUnitOfWork();
|
||||
|
||||
// Reset defensif en debut de cycle : si un flush precedent a leve une
|
||||
// exception, Doctrine n'appelle PAS postFlush et pendingLogs reste
|
||||
// rempli avec des changements jamais committes. Sans ce reset, un
|
||||
// flush ulterieur reussi ecrirait les fausses entrees dans audit_log.
|
||||
// Le swap-and-clear dans postFlush couvre deja les flushes re-entrants,
|
||||
// ce reset ne le fragilise donc pas.
|
||||
$this->pendingLogs = [];
|
||||
|
||||
foreach ($uow->getScheduledEntityInsertions() as $entity) {
|
||||
$this->capturePendingLog($entity, $em, $uow, 'create');
|
||||
}
|
||||
|
||||
@@ -5,9 +5,8 @@ declare(strict_types=1);
|
||||
namespace App\Module\Sites\Domain\Repository;
|
||||
|
||||
use App\Module\Sites\Domain\Entity\Site;
|
||||
use App\Shared\Domain\Contract\SiteProviderInterface;
|
||||
|
||||
interface SiteRepositoryInterface extends SiteProviderInterface
|
||||
interface SiteRepositoryInterface
|
||||
{
|
||||
public function findById(int $id): ?Site;
|
||||
|
||||
|
||||
@@ -1,21 +0,0 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Shared\Domain\Contract;
|
||||
|
||||
/**
|
||||
* Contrat minimal pour acceder a un site depuis un module qui n'est pas Sites.
|
||||
*
|
||||
* Permet a du code Core/Shared (commandes de seed, fixtures, etc.) de
|
||||
* recuperer un Site par son nom sans importer directement depuis le module
|
||||
* Sites — ce qui violerait la regle "jamais d'import direct entre modules"
|
||||
* (cf. CLAUDE.md section "Regles d'architecture").
|
||||
*
|
||||
* Implementation concrete : App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository
|
||||
* (via SiteRepositoryInterface qui etend ce contrat).
|
||||
*/
|
||||
interface SiteProviderInterface
|
||||
{
|
||||
public function findByName(string $name): ?SiteInterface;
|
||||
}
|
||||
@@ -188,69 +188,6 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
||||
self::assertSame($row['id'], $data['id']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Symetrique de `testAuthenticatedUserWithoutPermissionGets403` mais sur
|
||||
* l'endpoint item. Le `security: is_granted('core.audit_log.view')` declare
|
||||
* sur `Get` dans `AuditLogResource` doit refuser 403 (pas 200, pas 404).
|
||||
*/
|
||||
public function testItemEndpointWithoutPermissionGets403(): void
|
||||
{
|
||||
$row = $this->auditConnection->fetchAssociative(
|
||||
'SELECT id FROM audit_log WHERE request_id = :tag LIMIT 1',
|
||||
['tag' => $this->runTag],
|
||||
);
|
||||
self::assertIsArray($row);
|
||||
|
||||
// Permission "voisine" : prouve que l'auth seule ne suffit pas.
|
||||
$credentials = $this->createUserWithPermission('core.users.view');
|
||||
$client = $this->authenticatedClient($credentials['username'], $credentials['password']);
|
||||
$response = $client->request('GET', '/api/audit-logs/'.$row['id']);
|
||||
|
||||
self::assertSame(403, $response->getStatusCode());
|
||||
}
|
||||
|
||||
/**
|
||||
* `?page=0` provoquait historiquement un OFFSET negatif → 500 PG
|
||||
* `SQLSTATE[22023] OFFSET must not be negative`. API Platform 4 valide
|
||||
* desormais `page >= 1` en amont (rejette 400) avant que le provider ne
|
||||
* soit appele ; le clamp `max(1, ...)` cote provider reste en place comme
|
||||
* defense-in-depth si un futur upgrade ou un changement de configuration
|
||||
* leve cette validation. Ce test verrouille l'invariant fonctionnel :
|
||||
* aucun 500 PG quel que soit le mecanisme protecteur en place.
|
||||
*/
|
||||
public function testPageZeroDoesNotProduceServerError(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-logs?page=0');
|
||||
|
||||
self::assertContains(
|
||||
$response->getStatusCode(),
|
||||
[200, 400],
|
||||
'page=0 doit etre traite proprement (clamp 200 ou 400 explicite), jamais 500 PG.',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validation des filtres : un input malforme doit retourner un 400
|
||||
* explicite, pas un 500 (Postgres qui rejette le cast timestamp) ni
|
||||
* un match silencieux sur une valeur inattendue.
|
||||
*/
|
||||
public function testInvalidPerformedAtFilterReturns400(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-logs?performed_at[after]=pas-une-date');
|
||||
|
||||
self::assertSame(400, $response->getStatusCode());
|
||||
}
|
||||
|
||||
public function testInvalidActionFilterReturns400(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-logs?action=dropTable');
|
||||
|
||||
self::assertSame(400, $response->getStatusCode());
|
||||
}
|
||||
|
||||
public function testPostIsNotAllowed(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
|
||||
@@ -139,46 +139,6 @@ final class UserRbacSitesApiTest extends AbstractApiTestCase
|
||||
self::assertSame('Chatellerault', $reloaded->getCurrentSite()->getName());
|
||||
}
|
||||
|
||||
/**
|
||||
* Defense-in-depth contre un bypass hypothetique de restoreAbsentCollections.
|
||||
*
|
||||
* API Platform rejette deja `sites: null` au denormalize (400 Bad Request,
|
||||
* type mismatch). Le guard `is_array` dans UserRbacProcessor est une
|
||||
* deuxieme ligne de defense si la config denormalizer change un jour.
|
||||
*
|
||||
* Ce test verrouille deux garanties :
|
||||
* 1. `{"sites": null}` → 400 (pas un 500, pas un 200 silencieux)
|
||||
* 2. La collection sites d'alice est intacte apres l'echec
|
||||
*/
|
||||
public function testRbacPatchWithNullSitesReturns400AndDoesNotWipeSitesCollection(): void
|
||||
{
|
||||
$em = $this->getEm();
|
||||
$alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']);
|
||||
$aliceId = $alice->getId();
|
||||
self::assertCount(1, $alice->getSites(), 'Pre-condition : alice doit avoir exactement 1 site');
|
||||
$em->clear();
|
||||
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [
|
||||
'headers' => ['Content-Type' => 'application/merge-patch+json'],
|
||||
'json' => [
|
||||
'isAdmin' => false,
|
||||
'sites' => null,
|
||||
],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(400);
|
||||
|
||||
$em = $this->getEm();
|
||||
$em->clear();
|
||||
$reloaded = $em->getRepository(User::class)->find($aliceId);
|
||||
self::assertCount(
|
||||
1,
|
||||
$reloaded->getSites(),
|
||||
'Un payload `sites: null` rejete en 400 ne doit laisser aucune trace en DB.',
|
||||
);
|
||||
}
|
||||
|
||||
public function testRbacPatchWithoutSitesFieldDoesNotChangeCurrentSite(): void
|
||||
{
|
||||
// Garde structurelle : si le payload /rbac ne contient pas le champ
|
||||
|
||||
@@ -7,11 +7,8 @@ namespace App\Tests\Module\Core\Infrastructure\Doctrine;
|
||||
use App\Module\Core\Domain\Entity\Permission;
|
||||
use App\Module\Core\Domain\Entity\Role;
|
||||
use App\Module\Core\Domain\Entity\User;
|
||||
use App\Module\Core\Infrastructure\Doctrine\AuditListener;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use ReflectionProperty;
|
||||
use stdClass;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
|
||||
|
||||
@@ -331,59 +328,6 @@ final class AuditListenerTest extends KernelTestCase
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression : quand un flush precedent a leve une exception avant
|
||||
* d'atteindre postFlush, `$pendingLogs` reste rempli avec des changements
|
||||
* jamais committes. Le flush suivant doit les ecraser, pas les fusionner —
|
||||
* sinon audit_log contient des lignes pour des evenements qui n'ont pas
|
||||
* eu lieu en base.
|
||||
*
|
||||
* Reproduction : on injecte manuellement une entree orpheline dans le
|
||||
* listener (comme si un flush precedent l'avait capturee puis avait plante),
|
||||
* on declenche un flush valide, et on verifie que l'orpheline n'apparait
|
||||
* jamais dans audit_log.
|
||||
*/
|
||||
public function testOnFlushResetsStalePendingLogsFromFailedPreviousFlush(): void
|
||||
{
|
||||
/** @var AuditListener $listener */
|
||||
$listener = self::getContainer()->get(AuditListener::class);
|
||||
|
||||
// Injecte une entree orpheline : comme si onFlush avait capture ce
|
||||
// changement, puis que le flush avait plante avant postFlush.
|
||||
$reflection = new ReflectionProperty($listener, 'pendingLogs');
|
||||
$reflection->setValue($listener, [[
|
||||
'entity' => new stdClass(),
|
||||
'metadata' => null,
|
||||
'entityType' => 'test.StaleEntity',
|
||||
'action' => 'create',
|
||||
'changes' => ['fake' => ['old' => null, 'new' => 'stale']],
|
||||
'capturedId' => 'stale-id-'.$this->testRunTag,
|
||||
]]);
|
||||
|
||||
// Flush valide qui DOIT re-initialiser pendingLogs avant de capturer
|
||||
// ses propres changements.
|
||||
$user = $this->makeUser();
|
||||
$this->em->persist($user);
|
||||
$this->em->flush();
|
||||
$this->createdUserIds[] = $user->getId();
|
||||
|
||||
$staleRows = $this->auditConnection->fetchAllAssociative(
|
||||
'SELECT * FROM audit_log WHERE entity_type = :t',
|
||||
['t' => 'test.StaleEntity'],
|
||||
);
|
||||
|
||||
self::assertCount(
|
||||
0,
|
||||
$staleRows,
|
||||
'Une entree orpheline d\'un flush precedent ne doit pas fuiter dans audit_log.',
|
||||
);
|
||||
|
||||
// Sanity : le vrai flush a quand meme bien ecrit sa propre ligne.
|
||||
$userRows = $this->fetchAuditRows($user->getId());
|
||||
self::assertCount(1, $userRows);
|
||||
self::assertSame('create', $userRows[0]['action']);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<array{id: string, entity_type: string, entity_id: string, action: string, changes: string}>
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user