fix(review) : resout findings 3e passe review (HIGH frontend + MEDIUMs backend/frontend/E2E)
Backend :
- AuditLogWriter::stripSensitive rendu reellement recursif (matche doc).
- Tests GET /api/permissions/{id} non-admin pour chaque branche OR (gap Codex).
- Gardes non-regression UserRbacProcessor : PATCH /rbac sans clef sites ne
doit ni auto-selectionner currentSite ni exiger sites.manage.
Frontend :
- useAuditLog : renomme export trompeur fetchLogs -> fetchLogsCached, le
nom reflete desormais le comportement (cache pollue sinon).
- RoleDrawer / UserRbacDrawer : catch explicite + message d'erreur +
bouton save disabled si le chargement des referentiels a echoue (evite
un ecrasement silencieux des droits).
- AuditTimeline / AuditLogDetail : `oui`/`non` passent par common.yes/no.
- AuditTimeline : Intl.RelativeTimeFormat et toLocaleString suivent la
locale i18n courante (plus de hardcode 'fr').
E2E :
- sidebar-visibility.spec : remplace waitForLoadState('networkidle')
fragile par attente semantique sur accountDashboardLink (stable en CI).
Tests : 237/237 green, eslint clean, php-cs-fixer clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,7 +136,8 @@
|
|||||||
},
|
},
|
||||||
"permissions": {
|
"permissions": {
|
||||||
"selectAll": "Tout selectionner",
|
"selectAll": "Tout selectionner",
|
||||||
"noPermissions": "Aucune permission disponible"
|
"noPermissions": "Aucune permission disponible",
|
||||||
|
"loadFailed": "Impossible de charger le catalogue de permissions. L'enregistrement est désactivé pour éviter tout écrasement accidentel."
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"users": {
|
"users": {
|
||||||
@@ -160,7 +161,8 @@
|
|||||||
"noEffectivePermissions": "Aucune permission effective",
|
"noEffectivePermissions": "Aucune permission effective",
|
||||||
"sourceRole": "via {role}",
|
"sourceRole": "via {role}",
|
||||||
"sourceDirect": "Direct",
|
"sourceDirect": "Direct",
|
||||||
"lastAdminWarning": "Impossible de retirer le statut administrateur du dernier admin"
|
"lastAdminWarning": "Impossible de retirer le statut administrateur du dernier admin",
|
||||||
|
"loadFailed": "Impossible de charger les droits de cet utilisateur. L'enregistrement est désactivé pour éviter tout écrasement accidentel."
|
||||||
},
|
},
|
||||||
"toast": {
|
"toast": {
|
||||||
"updated": "Permissions mises à jour avec succès"
|
"updated": "Permissions mises à jour avec succès"
|
||||||
|
|||||||
@@ -33,7 +33,15 @@
|
|||||||
<h4 class="mb-3 text-sm font-semibold text-neutral-700">
|
<h4 class="mb-3 text-sm font-semibold text-neutral-700">
|
||||||
{{ t('admin.roles.form.permissions') }}
|
{{ t('admin.roles.form.permissions') }}
|
||||||
</h4>
|
</h4>
|
||||||
<div v-if="permissionsByModule.length === 0" class="text-sm text-neutral-400">
|
<!-- Etat d'erreur explicite : sans ce message, un drawer vide
|
||||||
|
ressemblerait a un role legitimement sans permissions. -->
|
||||||
|
<div
|
||||||
|
v-if="permissionsLoadFailed"
|
||||||
|
class="rounded border border-red-200 bg-red-50 p-3 text-sm text-red-700"
|
||||||
|
>
|
||||||
|
{{ t('admin.roles.permissions.loadFailed') }}
|
||||||
|
</div>
|
||||||
|
<div v-else-if="permissionsByModule.length === 0" class="text-sm text-neutral-400">
|
||||||
{{ t('admin.roles.permissions.noPermissions') }}
|
{{ t('admin.roles.permissions.noPermissions') }}
|
||||||
</div>
|
</div>
|
||||||
<div class="flex flex-col gap-4">
|
<div class="flex flex-col gap-4">
|
||||||
@@ -70,7 +78,7 @@
|
|||||||
<MalioButton
|
<MalioButton
|
||||||
:label="t('common.save')"
|
:label="t('common.save')"
|
||||||
variant="primary"
|
variant="primary"
|
||||||
:disabled="saving"
|
:disabled="saving || permissionsLoadFailed"
|
||||||
@click="handleSave"
|
@click="handleSave"
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
@@ -102,6 +110,11 @@ const emit = defineEmits<{
|
|||||||
|
|
||||||
const saving = ref(false)
|
const saving = ref(false)
|
||||||
const allPermissions = ref<Permission[]>([])
|
const allPermissions = ref<Permission[]>([])
|
||||||
|
// Signale un echec de chargement du catalogue de permissions : on bloque
|
||||||
|
// alors la sauvegarde pour eviter qu'un drawer ouvert avec zero permission
|
||||||
|
// visible (cas d'un 403 ou d'une panne reseau) n'ecrase silencieusement
|
||||||
|
// toutes les permissions du role.
|
||||||
|
const permissionsLoadFailed = ref(false)
|
||||||
|
|
||||||
const form = ref({
|
const form = ref({
|
||||||
label: '',
|
label: '',
|
||||||
@@ -129,12 +142,21 @@ const permissionsByModule = computed<PermissionModule[]>(() => {
|
|||||||
|
|
||||||
// Charger les permissions au montage
|
// Charger les permissions au montage
|
||||||
async function loadPermissions() {
|
async function loadPermissions() {
|
||||||
const data = await api.get<{ member: Permission[] }>(
|
permissionsLoadFailed.value = false
|
||||||
'/permissions',
|
try {
|
||||||
{ 'orphan': false, itemsPerPage: 999 },
|
const data = await api.get<{ member: Permission[] }>(
|
||||||
{ toast: false },
|
'/permissions',
|
||||||
)
|
{ 'orphan': false, itemsPerPage: 999 },
|
||||||
allPermissions.value = data.member
|
// `toast: true` : en cas d'echec (403, reseau, 500), l'utilisateur
|
||||||
|
// voit l'erreur remonter. Sans ce feedback, un catalogue vide
|
||||||
|
// ressemblerait a un role sans permissions disponibles.
|
||||||
|
{ toast: true },
|
||||||
|
)
|
||||||
|
allPermissions.value = data.member
|
||||||
|
} catch {
|
||||||
|
allPermissions.value = []
|
||||||
|
permissionsLoadFailed.value = true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remplir le formulaire quand le role change
|
// Remplir le formulaire quand le role change
|
||||||
|
|||||||
@@ -6,6 +6,16 @@
|
|||||||
@update:model-value="emit('update:modelValue', $event)"
|
@update:model-value="emit('update:modelValue', $event)"
|
||||||
>
|
>
|
||||||
<div class="flex flex-col gap-6 p-4">
|
<div class="flex flex-col gap-6 p-4">
|
||||||
|
<!-- Etat d'erreur de chargement des referentiels : bloque la
|
||||||
|
sauvegarde pour empecher un ecrasement silencieux des droits. -->
|
||||||
|
<div
|
||||||
|
v-if="loadFailed"
|
||||||
|
class="flex items-center gap-2 rounded-lg border border-red-300 bg-red-50 px-4 py-3 text-sm text-red-800"
|
||||||
|
>
|
||||||
|
<Icon name="mdi:alert-circle-outline" class="size-5 shrink-0" />
|
||||||
|
{{ t('admin.users.drawer.loadFailed') }}
|
||||||
|
</div>
|
||||||
|
|
||||||
<!-- Avertissement auto-edition -->
|
<!-- Avertissement auto-edition -->
|
||||||
<div
|
<div
|
||||||
v-if="isSelfEdit"
|
v-if="isSelfEdit"
|
||||||
@@ -103,7 +113,7 @@
|
|||||||
<MalioButton
|
<MalioButton
|
||||||
:label="t('common.save')"
|
:label="t('common.save')"
|
||||||
variant="primary"
|
variant="primary"
|
||||||
:disabled="saving"
|
:disabled="saving || loadFailed"
|
||||||
@click="handleSave"
|
@click="handleSave"
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
@@ -138,6 +148,10 @@ const saving = ref(false)
|
|||||||
const allRoles = ref<Role[]>([])
|
const allRoles = ref<Role[]>([])
|
||||||
const allPermissions = ref<Permission[]>([])
|
const allPermissions = ref<Permission[]>([])
|
||||||
const allSites = ref<Site[]>([])
|
const allSites = ref<Site[]>([])
|
||||||
|
// Signale un echec de chargement des referentiels : on bloque alors la
|
||||||
|
// sauvegarde pour eviter qu'un drawer ouvert sans donnees (403, reseau)
|
||||||
|
// n'ecrase silencieusement l'etat RBAC du user (vidage roles/permissions/sites).
|
||||||
|
const loadFailed = ref(false)
|
||||||
|
|
||||||
const form = ref({ isAdmin: false })
|
const form = ref({ isAdmin: false })
|
||||||
const selectedRoleIds = ref(new Set<number>())
|
const selectedRoleIds = ref(new Set<number>())
|
||||||
@@ -211,20 +225,32 @@ const effectivePermissions = computed<EffectivePermission[]>(() => {
|
|||||||
// Le detail RBAC est la seule source de verite pour l'etat initial du formulaire :
|
// Le detail RBAC est la seule source de verite pour l'etat initial du formulaire :
|
||||||
// props.user vient de la liste /api/users qui n'expose pas les sites (groupe leger).
|
// props.user vient de la liste /api/users qui n'expose pas les sites (groupe leger).
|
||||||
async function loadData(userId: number) {
|
async function loadData(userId: number) {
|
||||||
const [rolesData, permsData, sitesData, userRbac] = await Promise.all([
|
loadFailed.value = false
|
||||||
api.get<{ member: Role[] }>('/roles', {}, { toast: false }),
|
try {
|
||||||
api.get<{ member: Permission[] }>('/permissions', { orphan: false, itemsPerPage: 999 }, { toast: false }),
|
const [rolesData, permsData, sitesData, userRbac] = await Promise.all([
|
||||||
api.get<{ member: Site[] }>('/sites', { itemsPerPage: 999 }, { toast: false }),
|
// `toast: true` : en cas d'echec, l'utilisateur voit un toast
|
||||||
api.get<UserRbacDetail>(`/users/${userId}/rbac`, {}, { toast: false }),
|
// d'erreur. Sans ce feedback, le drawer s'afficherait vide et la
|
||||||
])
|
// sauvegarde ecraserait silencieusement l'etat RBAC du user.
|
||||||
allRoles.value = rolesData.member
|
api.get<{ member: Role[] }>('/roles', {}, { toast: true }),
|
||||||
allPermissions.value = permsData.member
|
api.get<{ member: Permission[] }>('/permissions', { orphan: false, itemsPerPage: 999 }, { toast: true }),
|
||||||
allSites.value = sitesData.member
|
api.get<{ member: Site[] }>('/sites', { itemsPerPage: 999 }, { toast: true }),
|
||||||
|
api.get<UserRbacDetail>(`/users/${userId}/rbac`, {}, { toast: true }),
|
||||||
|
])
|
||||||
|
allRoles.value = rolesData.member
|
||||||
|
allPermissions.value = permsData.member
|
||||||
|
allSites.value = sitesData.member
|
||||||
|
|
||||||
form.value.isAdmin = userRbac.isAdmin
|
form.value.isAdmin = userRbac.isAdmin
|
||||||
selectedRoleIds.value = new Set((userRbac.roles ?? []).map(iriToId))
|
selectedRoleIds.value = new Set((userRbac.roles ?? []).map(iriToId))
|
||||||
selectedDirectPermissionIds.value = new Set((userRbac.directPermissions ?? []).map(iriToId))
|
selectedDirectPermissionIds.value = new Set((userRbac.directPermissions ?? []).map(iriToId))
|
||||||
selectedSiteIds.value = new Set((userRbac.sites ?? []).map(iriToId))
|
selectedSiteIds.value = new Set((userRbac.sites ?? []).map(iriToId))
|
||||||
|
} catch {
|
||||||
|
loadFailed.value = true
|
||||||
|
allRoles.value = []
|
||||||
|
allPermissions.value = []
|
||||||
|
allSites.value = []
|
||||||
|
resetForm()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function resetForm() {
|
function resetForm() {
|
||||||
|
|||||||
@@ -153,7 +153,7 @@ import type { AuditLogEntry, AuditLogFilters } from '~/shared/types'
|
|||||||
|
|
||||||
const { t } = useI18n()
|
const { t } = useI18n()
|
||||||
const { can } = usePermissions()
|
const { can } = usePermissions()
|
||||||
const { fetchLogs, fetchEntityTypes } = useAuditLog()
|
const { fetchLogsCached, fetchEntityTypes } = useAuditLog()
|
||||||
|
|
||||||
// Protection cote UI : le middleware `modules.global.ts` filtre deja les
|
// Protection cote UI : le middleware `modules.global.ts` filtre deja les
|
||||||
// routes desactivees, mais si quelqu'un atterit ici sans la permission on
|
// routes desactivees, mais si quelqu'un atterit ici sans la permission on
|
||||||
@@ -275,7 +275,7 @@ async function loadEntries(): Promise<void> {
|
|||||||
const token = ++requestToken
|
const token = ++requestToken
|
||||||
loading.value = true
|
loading.value = true
|
||||||
try {
|
try {
|
||||||
const data = await fetchLogs({
|
const data = await fetchLogsCached({
|
||||||
...filters,
|
...filters,
|
||||||
// Convertit datetime-local (YYYY-MM-DDTHH:MM) en ISO pour l'API.
|
// Convertit datetime-local (YYYY-MM-DDTHH:MM) en ISO pour l'API.
|
||||||
performedAtAfter: filters.performedAtAfter ? toIso(filters.performedAtAfter) : undefined,
|
performedAtAfter: filters.performedAtAfter ? toIso(filters.performedAtAfter) : undefined,
|
||||||
|
|||||||
@@ -91,7 +91,9 @@ const collectionDiff = computed<Record<string, { added: unknown[]; removed: unkn
|
|||||||
|
|
||||||
function formatValue(value: unknown): string {
|
function formatValue(value: unknown): string {
|
||||||
if (value === null || value === undefined) return '∅'
|
if (value === null || value === undefined) return '∅'
|
||||||
if (typeof value === 'boolean') return value ? 'oui' : 'non'
|
// Passe par i18n plutot qu'un hardcode FR : si une autre locale est
|
||||||
|
// ajoutee, le rendu s'adapte sans nouvelle passe sur ce composant.
|
||||||
|
if (typeof value === 'boolean') return value ? t('common.yes') : t('common.no')
|
||||||
if (typeof value === 'object') return JSON.stringify(value)
|
if (typeof value === 'object') return JSON.stringify(value)
|
||||||
return String(value)
|
return String(value)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -100,7 +100,7 @@ const props = defineProps<{
|
|||||||
|
|
||||||
const { entityType, entityId } = toRefs(props)
|
const { entityType, entityId } = toRefs(props)
|
||||||
|
|
||||||
const { t } = useI18n()
|
const { t, locale } = useI18n()
|
||||||
const { can } = usePermissions()
|
const { can } = usePermissions()
|
||||||
const { fetchEntityLogs } = useAuditLog()
|
const { fetchEntityLogs } = useAuditLog()
|
||||||
|
|
||||||
@@ -162,24 +162,28 @@ function dotClass(action: string): string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Relativise une date en francais via Intl.RelativeTimeFormat. On selectionne
|
// Relativise une date via Intl.RelativeTimeFormat. On selectionne l'unite
|
||||||
// l'unite la plus grossiere possible (minutes < heures < jours < semaines).
|
// la plus grossiere possible (minutes < heures < jours < semaines). La
|
||||||
const rtf = new Intl.RelativeTimeFormat('fr', { numeric: 'auto' })
|
// 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 {
|
function relativeDate(iso: string): string {
|
||||||
const diffMs = Date.now() - new Date(iso).getTime()
|
const diffMs = Date.now() - new Date(iso).getTime()
|
||||||
const diffSec = Math.round(diffMs / 1000)
|
const diffSec = Math.round(diffMs / 1000)
|
||||||
const absSec = Math.abs(diffSec)
|
const absSec = Math.abs(diffSec)
|
||||||
|
const fmt = rtf.value
|
||||||
|
|
||||||
if (absSec < 60) return rtf.format(-Math.sign(diffSec) * Math.abs(diffSec), 'second')
|
if (absSec < 60) return fmt.format(-Math.sign(diffSec) * Math.abs(diffSec), 'second')
|
||||||
if (absSec < 3600) return rtf.format(-Math.sign(diffSec) * Math.round(absSec / 60), 'minute')
|
if (absSec < 3600) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 60), 'minute')
|
||||||
if (absSec < 86400) return rtf.format(-Math.sign(diffSec) * Math.round(absSec / 3600), 'hour')
|
if (absSec < 86400) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 3600), 'hour')
|
||||||
if (absSec < 604800) return rtf.format(-Math.sign(diffSec) * Math.round(absSec / 86400), 'day')
|
if (absSec < 604800) return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 86400), 'day')
|
||||||
return rtf.format(-Math.sign(diffSec) * Math.round(absSec / 604800), 'week')
|
return fmt.format(-Math.sign(diffSec) * Math.round(absSec / 604800), 'week')
|
||||||
}
|
}
|
||||||
|
|
||||||
function absoluteDate(iso: string): string {
|
function absoluteDate(iso: string): string {
|
||||||
return new Date(iso).toLocaleString('fr-FR', {
|
// Meme logique : la locale de formatage suit celle de l'app.
|
||||||
|
return new Date(iso).toLocaleString(locale.value, {
|
||||||
dateStyle: 'medium',
|
dateStyle: 'medium',
|
||||||
timeStyle: 'short',
|
timeStyle: 'short',
|
||||||
})
|
})
|
||||||
@@ -223,7 +227,9 @@ function snapshotSummary(entry: AuditLogEntry): string {
|
|||||||
|
|
||||||
function formatValue(value: unknown): string {
|
function formatValue(value: unknown): string {
|
||||||
if (value === null || value === undefined) return '∅'
|
if (value === null || value === undefined) return '∅'
|
||||||
if (typeof value === 'boolean') return value ? 'oui' : 'non'
|
// Passe par i18n plutot qu'un hardcode FR : si une autre locale est
|
||||||
|
// ajoutee, le rendu s'adapte sans nouvelle passe sur ce composant.
|
||||||
|
if (typeof value === 'boolean') return value ? t('common.yes') : t('common.no')
|
||||||
if (typeof value === 'object') return JSON.stringify(value)
|
if (typeof value === 'object') return JSON.stringify(value)
|
||||||
return String(value)
|
return String(value)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -128,9 +128,14 @@ export function useAuditLog() {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// API publique : on expose volontairement deux noms distincts pour les
|
||||||
|
// deux contrats (cache/no-cache). Aliaser `fetchLogs` vers la version
|
||||||
|
// cachee trompait les appelants : un consommateur qui destructurait
|
||||||
|
// `{ fetchLogs }` en pensant faire un appel neutre polluait en realite
|
||||||
|
// `lastCollection`, effet indetectable sans lire l'impl.
|
||||||
return {
|
return {
|
||||||
lastCollection,
|
lastCollection,
|
||||||
fetchLogs: fetchLogsCached,
|
fetchLogsCached,
|
||||||
fetchLogById,
|
fetchLogById,
|
||||||
fetchEntityLogs,
|
fetchEntityLogs,
|
||||||
fetchEntityTypes,
|
fetchEntityTypes,
|
||||||
|
|||||||
@@ -30,13 +30,14 @@ test.describe('Sidebar visibility', () => {
|
|||||||
await loginAs(context, persona.key)
|
await loginAs(context, persona.key)
|
||||||
await page.goto('/')
|
await page.goto('/')
|
||||||
|
|
||||||
// Attendre que la sidebar soit chargee (le middleware auth fetch /api/sidebar
|
|
||||||
// apres login). Les liens presents apparaissent alors ; les absents ne
|
|
||||||
// seront jamais attaches au DOM.
|
|
||||||
await page.waitForLoadState('networkidle')
|
|
||||||
|
|
||||||
const sidebar = new SidebarComponent(page)
|
const sidebar = new SidebarComponent(page)
|
||||||
|
|
||||||
|
// Attente semantique : on ancre sur un lien toujours present pour
|
||||||
|
// tout user authentifie (Mon compte > Tableau de bord). Remplace
|
||||||
|
// `networkidle` qui est reconnu instable en CI (SPAs avec polling
|
||||||
|
// ou HMR peuvent ne jamais quitter cet etat).
|
||||||
|
await expect(sidebar.accountDashboardLink()).toBeVisible({ timeout: 10000 })
|
||||||
|
|
||||||
for (const link of ALL_ADMIN_LINKS) {
|
for (const link of ALL_ADMIN_LINKS) {
|
||||||
const locator = sidebar.adminLink(link)
|
const locator = sidebar.adminLink(link)
|
||||||
const shouldBeVisible = persona.expectedAdminLinks.includes(link)
|
const shouldBeVisible = persona.expectedAdminLinks.includes(link)
|
||||||
@@ -66,10 +67,11 @@ test.describe('Sidebar visibility', () => {
|
|||||||
// dessus — ca bloquerait le logout de users sans permissions.
|
// dessus — ca bloquerait le logout de users sans permissions.
|
||||||
await loginAs(context, 'user-nothing')
|
await loginAs(context, 'user-nothing')
|
||||||
await page.goto('/')
|
await page.goto('/')
|
||||||
await page.waitForLoadState('networkidle')
|
|
||||||
|
|
||||||
const sidebar = new SidebarComponent(page)
|
const sidebar = new SidebarComponent(page)
|
||||||
await expect(sidebar.accountDashboardLink()).toBeVisible()
|
// Meme strategie que ci-dessus : ancrage semantique plutot que
|
||||||
|
// `networkidle` pour eviter les faux timeouts en CI.
|
||||||
|
await expect(sidebar.accountDashboardLink()).toBeVisible({ timeout: 10000 })
|
||||||
await expect(sidebar.logoutLink()).toBeVisible()
|
await expect(sidebar.logoutLink()).toBeVisible()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -85,14 +85,25 @@ final class AuditLogWriter
|
|||||||
* d'update : le listener prefiltre deja mais on garde cette garde
|
* d'update : le listener prefiltre deja mais on garde cette garde
|
||||||
* en defense-in-depth si un appelant direct oublie `#[AuditIgnore]`.
|
* en defense-in-depth si un appelant direct oublie `#[AuditIgnore]`.
|
||||||
*
|
*
|
||||||
|
* Recursion : parcourt les sous-tableaux (ex: changes structures
|
||||||
|
* `{field: {old, new}}`, snapshots avec relations imbriquees, ou
|
||||||
|
* payload arbitraire pousse par un appelant direct).
|
||||||
|
*
|
||||||
* @param array<string, mixed> $data
|
* @param array<string, mixed> $data
|
||||||
*
|
*
|
||||||
* @return array<string, mixed>
|
* @return array<string, mixed>
|
||||||
*/
|
*/
|
||||||
private function stripSensitive(array $data): array
|
private function stripSensitive(array $data): array
|
||||||
{
|
{
|
||||||
foreach (self::SENSITIVE_KEYS as $sensitiveKey) {
|
foreach ($data as $key => $value) {
|
||||||
unset($data[$sensitiveKey]);
|
if (in_array($key, self::SENSITIVE_KEYS, true)) {
|
||||||
|
unset($data[$key]);
|
||||||
|
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (is_array($value)) {
|
||||||
|
$data[$key] = $this->stripSensitive($value);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return $data;
|
return $data;
|
||||||
|
|||||||
@@ -226,6 +226,51 @@ final class PermissionApiTest extends AbstractApiTestCase
|
|||||||
self::assertResponseIsSuccessful();
|
self::assertResponseIsSuccessful();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testNonAdminWithPermissionViewCanGetItem(): void
|
||||||
|
{
|
||||||
|
// Miroir item de testNonAdminWithPermissionViewCanListPermissions :
|
||||||
|
// la security expression OR est repliquee sur `new Get(...)` et doit
|
||||||
|
// donc aussi s'appliquer ici.
|
||||||
|
$permission = $this->getEm()->getRepository(Permission::class)
|
||||||
|
->findOneBy(['code' => 'test.core.users.view'])
|
||||||
|
;
|
||||||
|
self::assertNotNull($permission);
|
||||||
|
|
||||||
|
$creds = $this->createUserWithPermission('core.permissions.view');
|
||||||
|
$client = $this->authenticatedClient($creds['username'], $creds['password']);
|
||||||
|
$client->request('GET', '/api/permissions/'.$permission->getId());
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testNonAdminWithUsersManageCanGetItem(): void
|
||||||
|
{
|
||||||
|
$permission = $this->getEm()->getRepository(Permission::class)
|
||||||
|
->findOneBy(['code' => 'test.core.users.view'])
|
||||||
|
;
|
||||||
|
self::assertNotNull($permission);
|
||||||
|
|
||||||
|
$creds = $this->createUserWithPermission('core.users.manage');
|
||||||
|
$client = $this->authenticatedClient($creds['username'], $creds['password']);
|
||||||
|
$client->request('GET', '/api/permissions/'.$permission->getId());
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testNonAdminWithRolesManageCanGetItem(): void
|
||||||
|
{
|
||||||
|
$permission = $this->getEm()->getRepository(Permission::class)
|
||||||
|
->findOneBy(['code' => 'test.core.users.view'])
|
||||||
|
;
|
||||||
|
self::assertNotNull($permission);
|
||||||
|
|
||||||
|
$creds = $this->createUserWithPermission('core.roles.manage');
|
||||||
|
$client = $this->authenticatedClient($creds['username'], $creds['password']);
|
||||||
|
$client->request('GET', '/api/permissions/'.$permission->getId());
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
}
|
||||||
|
|
||||||
private function cleanupTestPermissions(): void
|
private function cleanupTestPermissions(): void
|
||||||
{
|
{
|
||||||
$em = $this->getEm();
|
$em = $this->getEm();
|
||||||
|
|||||||
@@ -174,6 +174,81 @@ final class UserRbacSitesApiTest extends AbstractApiTestCase
|
|||||||
self::assertSame('Chatellerault', $reloaded->getSites()->first()->getName());
|
self::assertSame('Chatellerault', $reloaded->getSites()->first()->getName());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testRbacPatchWithoutSitesKeyDoesNotAutoSwitchCurrentSiteWhenNull(): void
|
||||||
|
{
|
||||||
|
// Scenario : alice a des sites mais currentSite=null. Un PATCH /rbac
|
||||||
|
// qui ne touche PAS a la clef `sites` ne doit PAS auto-selectionner
|
||||||
|
// silencieusement un site via ensureCurrentSiteConsistency.
|
||||||
|
//
|
||||||
|
// Sans ce garde, un payload { "isAdmin": false } pourrait, si la
|
||||||
|
// denormalisation marque la collection `sites` dirty a tort (ou si
|
||||||
|
// la logique de detection se base sur PersistentCollection::isDirty()
|
||||||
|
// avant restauration), declencher ensureCurrentSiteConsistency et
|
||||||
|
// recaler currentSite sur sites->first() — ce qui est un effet de
|
||||||
|
// bord indetectable par le client.
|
||||||
|
$em = $this->getEm();
|
||||||
|
$alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']);
|
||||||
|
$aliceId = $alice->getId();
|
||||||
|
$alice->setCurrentSite(null);
|
||||||
|
$em->flush();
|
||||||
|
$em->clear();
|
||||||
|
|
||||||
|
$client = $this->authenticatedClient('admin', 'admin');
|
||||||
|
$client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [
|
||||||
|
'headers' => ['Content-Type' => 'application/merge-patch+json'],
|
||||||
|
'json' => ['isAdmin' => false],
|
||||||
|
]);
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
|
||||||
|
$em = $this->getEm();
|
||||||
|
$em->clear();
|
||||||
|
$reloaded = $em->getRepository(User::class)->find($aliceId);
|
||||||
|
self::assertNotNull($reloaded);
|
||||||
|
// currentSite doit rester null : PATCH /rbac sans clef `sites` ne doit
|
||||||
|
// pas muter la selection de site courant de l'user.
|
||||||
|
self::assertNull(
|
||||||
|
$reloaded->getCurrentSite(),
|
||||||
|
'PATCH /rbac sans clef `sites` ne doit pas auto-selectionner un site.',
|
||||||
|
);
|
||||||
|
// Les sites eux-memes doivent etre preserves.
|
||||||
|
self::assertCount(1, $reloaded->getSites());
|
||||||
|
|
||||||
|
$this->restoreAliceSites();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRbacPatchWithoutSitesKeyDoesNotRequireSitesManagePermission(): void
|
||||||
|
{
|
||||||
|
// Scenario Codex : un user non-admin qui porte `core.users.manage`
|
||||||
|
// mais PAS `sites.manage` doit pouvoir PATCHer /rbac sans clef `sites`
|
||||||
|
// sans se faire refuser l'acces.
|
||||||
|
//
|
||||||
|
// Si sitesWereMutated se base uniquement sur PersistentCollection::isDirty()
|
||||||
|
// calcule avant restoreAbsentCollections, une denormalisation qui
|
||||||
|
// marque a tort la collection dirty lorsque la clef est absente du
|
||||||
|
// payload lancerait un 403 parasite. La source de verite doit etre
|
||||||
|
// la presence de la clef dans le payload JSON.
|
||||||
|
$this->skipIfSitesModuleDisabled();
|
||||||
|
|
||||||
|
$em = $this->getEm();
|
||||||
|
$alice = $em->getRepository(User::class)->findOneBy(['username' => 'alice']);
|
||||||
|
$aliceId = $alice->getId();
|
||||||
|
$em->clear();
|
||||||
|
|
||||||
|
// User jetable : core.users.manage uniquement (pas sites.manage).
|
||||||
|
$creds = $this->createUserWithPermission('core.users.manage');
|
||||||
|
$client = $this->authenticatedClient($creds['username'], $creds['password']);
|
||||||
|
|
||||||
|
$client->request('PATCH', '/api/users/'.$aliceId.'/rbac', [
|
||||||
|
'headers' => ['Content-Type' => 'application/merge-patch+json'],
|
||||||
|
'json' => ['isAdmin' => false],
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Pas de 403 : la requete ne touche pas aux sites, sites.manage n'est
|
||||||
|
// pas requis.
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Remet alice dans l'etat des fixtures : un seul site Chatellerault,
|
* Remet alice dans l'etat des fixtures : un seul site Chatellerault,
|
||||||
* currentSite Chatellerault. Evite la pollution inter-tests.
|
* currentSite Chatellerault. Evite la pollution inter-tests.
|
||||||
|
|||||||
@@ -111,6 +111,33 @@ final class AuditLogWriterTest extends TestCase
|
|||||||
$this->assertSame('bob@example.com', $changes['email']);
|
$this->assertSame('bob@example.com', $changes['email']);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testStripsSensitiveKeysRecursively(): void
|
||||||
|
{
|
||||||
|
// Defense-in-depth : un appelant direct peut passer un payload
|
||||||
|
// imbrique (ex: relations embarquees). Les cles sensibles doivent
|
||||||
|
// etre supprimees a tous les niveaux de profondeur.
|
||||||
|
$security = $this->buildSecurityWithUser('alice');
|
||||||
|
$writer = new AuditLogWriter($this->connection, $security, $this->requestStack, $this->requestIdProvider);
|
||||||
|
|
||||||
|
$writer->log('core.User', '1', 'create', [
|
||||||
|
'username' => 'bob',
|
||||||
|
'profile' => [
|
||||||
|
'email' => 'bob@example.com',
|
||||||
|
'password' => 'leaked_in_nested',
|
||||||
|
'nested' => [
|
||||||
|
'token' => 'should_be_stripped',
|
||||||
|
'harmless' => 'kept',
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
|
||||||
|
$changes = $this->capturedInsert[1]['changes'];
|
||||||
|
$this->assertArrayNotHasKey('password', $changes['profile']);
|
||||||
|
$this->assertArrayNotHasKey('token', $changes['profile']['nested']);
|
||||||
|
$this->assertSame('kept', $changes['profile']['nested']['harmless']);
|
||||||
|
$this->assertSame('bob@example.com', $changes['profile']['email']);
|
||||||
|
}
|
||||||
|
|
||||||
public function testCapturesIpAddressWhenRequestPresent(): void
|
public function testCapturesIpAddressWhenRequestPresent(): void
|
||||||
{
|
{
|
||||||
$request = Request::create('/api/users', 'POST');
|
$request = Request::create('/api/users', 'POST');
|
||||||
|
|||||||
Reference in New Issue
Block a user