fa/breadcrumb-the-shire
1
0
Fork 1
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(audit): harden API audit log redaction and schema

Browse Source 
- Change query_json column from TEXT to JSON, ip/user_agent to CHAR(64)
  for PII redaction compliance
- Update repository, service, and views for hardened schema
- Add architecture contract test for security logging redaction
- Add search resource provider test coverage
- Include DB migration script for existing installs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Felix Schneider
2026-04-05 23:20:55 +02:00
parent c2a7d6b789
commit c6c5d06936
14 changed files with 255 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
db/init/init.sql
View File

@@ -511,7 +511,7 @@ CREATE TABLE IF NOT EXISTS `api_audit_log` (
`request_id` CHAR(36) NOT NULL,
`method` VARCHAR(8) NOT NULL,
`path` VARCHAR(255) NOT NULL,
`query_json` TEXT NULL,
`query_json` JSON NULL,
`status_code` SMALLINT UNSIGNED NOT NULL,
`duration_ms` INT UNSIGNED NULL,
`error_code` VARCHAR(100) NULL,
@@ -519,8 +519,8 @@ CREATE TABLE IF NOT EXISTS `api_audit_log` (
`tenant_id` INT UNSIGNED NULL,
`api_token_id` INT UNSIGNED NULL,
`token_tenant_id` INT UNSIGNED NULL,
`ip` VARCHAR(45) NULL,
`user_agent` VARCHAR(255) NULL,
`ip` CHAR(64) NULL,
`user_agent` CHAR(64) NULL,
`created_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
PRIMARY KEY (`id`),
UNIQUE KEY `uniq_api_audit_request_id_created` (`request_id`, `created_at`),

1
i18n/default_de.json
View File

@@ -1010,6 +1010,7 @@
"Request details": "Request-Details",
"Scope": "Geltungsbereich",
"Query": "Query",
"Query metadata (keys)": "Query-Metadaten (Schlüssel)",
"User agent": "User-Agent",
"All methods": "Alle Methoden",
"Tenant ID": "Mandanten-ID",

1
i18n/default_en.json
View File

@@ -1010,6 +1010,7 @@
"Request details": "Request details",
"Scope": "Scope",
"Query": "Query",
"Query metadata (keys)": "Query metadata (keys)",
"User agent": "User agent",
"All methods": "All methods",
"Tenant ID": "Tenant ID",

9
modules/audit/db/updates/2026-04-05-api-audit-redaction-hardening.sql Normal file
View File

@@ -0,0 +1,9 @@
-- API audit redaction hardening:
-- 1) Store full SHA-256 hashes (64 chars) for IP and user-agent fields.
-- 2) Remove historical API audit rows that may contain plaintext query values or identifiers.
ALTER TABLE `api_audit_log`
MODIFY COLUMN `ip` CHAR(64) NULL,
MODIFY COLUMN `user_agent` CHAR(64) NULL;
DELETE FROM `api_audit_log`;

6
modules/audit/lib/Module/Audit/Providers/AuditSearchResourceProvider.php
View File

@@ -15,9 +15,9 @@ final class AuditSearchResourceProvider implements SearchResourceProvider
'api-audit' => [
'label' => t('API audit'),
'permission' => 'audit.api.view',
'count_sql' => "select count(*) from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\')",
'preview_sql' => "select id, request_id, method, path, status_code, created_at from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\') order by created_at desc limit ?",
'result_sql' => "select id, request_id, method, path, status_code, created_at from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\') order by created_at desc",
'count_sql' => "select count(*) from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\' or user_agent like ? escape '\\\\')",
'preview_sql' => "select id, request_id, method, path, status_code, created_at from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\' or user_agent like ? escape '\\\\') order by created_at desc limit ?",
'result_sql' => "select id, request_id, method, path, status_code, created_at from api_audit_log where (request_id like ? escape '\\\\' or path like ? escape '\\\\' or error_code like ? escape '\\\\' or ip like ? escape '\\\\' or user_agent like ? escape '\\\\') order by created_at desc",
],
'system-audit' => [
'label' => t('System audit'),

41
modules/audit/lib/Module/Audit/Repository/ApiAuditLogRepository.php
View File

@@ -68,7 +68,13 @@ class ApiAuditLogRepository
RepoQuery::addLikeFilter(
$where,
$params,
['api_audit_log.path', 'api_audit_log.request_id', 'api_audit_log.ip', 'api_audit_log.error_code'],
[
'api_audit_log.path',
'api_audit_log.request_id',
'api_audit_log.error_code',
'api_audit_log.ip',
'api_audit_log.user_agent',
],
$search
);
@@ -331,8 +337,41 @@ class ApiAuditLogRepository
$log['user_email'] = (string) ($user['email'] ?? '');
$log['tenant_uuid'] = (string) ($tenant['uuid'] ?? '');
$log['tenant_description'] = (string) ($tenant['description'] ?? '');
$log['ip_hash'] = trim((string) ($log['ip'] ?? ''));
$log['user_agent_hash'] = trim((string) ($log['user_agent'] ?? ''));
$log['query_keys'] = $this->extractQueryKeys($log['query_json'] ?? null);
return $log;
}
/**
* @return list<string>
*/
private function extractQueryKeys(mixed $queryJson): array
{
if (!is_string($queryJson) || trim($queryJson) === '') {
return [];
}
$decoded = json_decode($queryJson, true);
if (!is_array($decoded)) {
return [];
}
$keysRaw = $decoded['keys'] ?? null;
if (!is_array($keysRaw)) {
return [];
}
$keys = [];
foreach ($keysRaw as $value) {
$key = trim((string) $value);
if ($key === '' || in_array($key, $keys, true)) {
continue;
}
$keys[] = $key;
}
return $keys;
}
}

128
modules/audit/lib/Module/Audit/Service/ApiAuditService.php
View File

@@ -10,9 +10,33 @@ use MintyPHP\Module\Audit\Repository\ApiAuditLogRepository;
class ApiAuditService
{
private const RETENTION_DAYS = 90;
private const MAX_USER_AGENT_LENGTH = 255;
private const MAX_STRING_LENGTH = 500;
private const MAX_ARRAY_ITEMS = 30;
private const MAX_QUERY_KEYS = 30;
private const ALLOWED_QUERY_KEYS = [
'active',
'status',
'method',
'order',
'dir',
'sort',
'page',
'per_page',
'limit',
'offset',
'cursor',
'from',
'to',
'created_from',
'created_to',
'tenant_id',
'tenant_ids',
'user_id',
'user_ids',
'id',
'ids',
'request_id',
'error_code',
'search',
];
private ?array $context = null;
@@ -51,7 +75,7 @@ class ApiAuditService
'request_id' => (string) ($requestContext['request_id'] ?? RequestContext::id()),
'method' => (string) ($requestContext['method'] ?? substr($method !== '' ? $method : 'GET', 0, 8)),
'path' => (string) ($requestContext['path'] ?? substr($path, 0, 255)),
'query_json' => $this->buildRedactedQueryJson($this->requestRuntime->queryParams()),
'query_json' => $this->buildQueryMetadataJson($this->requestRuntime->queryParams()),
];
}
@@ -98,18 +122,10 @@ class ApiAuditService
$requestContext = RequestContext::context();
$ip = trim((string) ($requestContext['ip'] ?? $this->requestRuntime->ip()));
if ($ip === '') {
$ip = null;
} elseif (strlen($ip) > 45) {
$ip = substr($ip, 0, 45);
}
$ipHash = $ip !== '' ? RequestContext::hashValue($ip) : null;
$userAgent = trim((string) ($requestContext['user_agent'] ?? $this->requestRuntime->userAgent()));
if ($userAgent === '') {
$userAgent = null;
} elseif (strlen($userAgent) > self::MAX_USER_AGENT_LENGTH) {
$userAgent = substr($userAgent, 0, self::MAX_USER_AGENT_LENGTH);
}
$userAgentHash = $userAgent !== '' ? RequestContext::hashValue($userAgent) : null;
$userId = ApiAuth::isAuthenticated() ? ApiAuth::userId() : 0;
$tenantId = ApiAuth::isAuthenticated() ? (int) (ApiAuth::tenantId() ?? 0) : 0;
@@ -128,8 +144,8 @@ class ApiAuditService
'tenant_id' => $tenantId > 0 ? $tenantId : null,
'api_token_id' => $apiTokenId > 0 ? $apiTokenId : null,
'token_tenant_id' => $tokenTenantId > 0 ? $tokenTenantId : null,
'ip' => $ip,
'user_agent' => $userAgent,
'ip' => $ipHash,
'user_agent' => $userAgentHash,
];
try {
@@ -159,60 +175,73 @@ class ApiAuditService
return $this->apiAuditLogRepository->purgeOlderThanDays(self::RETENTION_DAYS);
}
private function buildRedactedQueryJson(array $query): ?string
private function buildQueryMetadataJson(array $query): ?string
{
if (!$query) {
return null;
}
$redacted = $this->redactArray($query);
if ($redacted === []) {
$keys = $this->extractAllowedQueryKeys($query);
if ($keys === []) {
return null;
}
$encoded = json_encode($redacted, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
$encoded = json_encode(
['keys' => $keys],
JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES
);
return is_string($encoded) ? $encoded : null;
}
private function redactArray(array $data): array
/**
* @param array<string, mixed> $query
* @return list<string>
*/
private function extractAllowedQueryKeys(array $query): array
{
$result = [];
$i = 0;
foreach ($data as $rawKey => $value) {
if ($i >= self::MAX_ARRAY_ITEMS) {
$keys = [];
foreach ($query as $rawKey => $_value) {
if (count($keys) >= self::MAX_QUERY_KEYS) {
break;
}
$key = trim((string) $rawKey);
if ($key === '') {
$key = $this->normalizeQueryKey((string) $rawKey);
if ($key === '' || $this->isSensitiveKey($key) || !$this->isAllowedQueryKey($key)) {
continue;
}
if ($this->isSensitiveKey($key)) {
$result[$key] = '[REDACTED]';
} else {
$result[$key] = $this->normalizeValue($value);
if (!in_array($key, $keys, true)) {
$keys[] = $key;
}
$i++;
}
return $result;
return $keys;
}
private function normalizeValue(mixed $value): mixed
private function normalizeQueryKey(string $rawKey): string
{
if (is_array($value)) {
return $this->redactArray($value);
}
if (is_bool($value) || is_int($value) || is_float($value) || $value === null) {
return $value;
}
$string = trim((string) $value);
if ($string === '') {
$key = strtolower(trim($rawKey));
if ($key === '') {
return '';
}
if (strlen($string) > self::MAX_STRING_LENGTH) {
return substr($string, 0, self::MAX_STRING_LENGTH);
$bracketPos = strpos($key, '[');
if ($bracketPos !== false) {
$key = substr($key, 0, $bracketPos);
}
return $string;
$dotPos = strpos($key, '.');
if ($dotPos !== false) {
$key = substr($key, 0, $dotPos);
}
return trim($key);
}
private function isAllowedQueryKey(string $key): bool
{
return in_array($key, self::ALLOWED_QUERY_KEYS, true);
}
private function isSensitiveKey(string $key): bool
@@ -233,6 +262,9 @@ class ApiAuditService
'api_key',
'client_secret',
'key',
'email',
'to_email',
'uuid',
], true)) {
return true;
}
@@ -241,6 +273,8 @@ class ApiAuditService
|| str_contains($key, 'secret')
|| str_contains($key, 'password')
|| str_contains($key, 'authorization')
|| str_contains($key, 'email')
|| str_contains($key, 'uuid')
|| str_ends_with($key, '_key');
}
}

2
modules/audit/pages/audit/api-audit/data().php
View File

@@ -42,7 +42,7 @@ foreach ($result['rows'] as $row) {
'user_id' => (int) ($row['user_id'] ?? 0),
'user_label' => $userLabel,
'user_uuid' => (string) ($row['user_uuid'] ?? ''),
'ip' => (string) ($row['ip'] ?? ''),
'ip_hash' => (string) ($row['ip_hash'] ?? ''),
];
}

2
modules/audit/pages/audit/api-audit/index(default).phtml
View File

@@ -53,7 +53,7 @@ $pageConfig = [
'user' => t('User'),
'tenant' => t('Tenant'),
'errorCode' => t('Error code'),
'ip' => t('IP'),
'ip' => t('IP hash'),
],
];
?>

43
modules/audit/pages/audit/api-audit/view(default).phtml
View File

@@ -15,17 +15,24 @@ if ($statusCode >= 200 && $statusCode < 300) {
$statusVariant = 'danger';
}
$queryKeys = is_array($auditLog['query_keys'] ?? null) ? $auditLog['query_keys'] : [];
$queryJson = trim((string) ($auditLog['query_json'] ?? ''));
$queryPretty = '-';
if ($queryJson !== '') {
if ($queryKeys === [] && $queryJson !== '') {
$decoded = json_decode($queryJson, true);
if (is_array($decoded)) {
$pretty = json_encode($decoded, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
$queryPretty = is_string($pretty) ? $pretty : $queryJson;
} else {
$queryPretty = $queryJson;
$decodedKeys = is_array($decoded['keys'] ?? null) ? $decoded['keys'] : [];
foreach ($decodedKeys as $value) {
$key = trim((string) $value);
if ($key === '' || in_array($key, $queryKeys, true)) {
continue;
}
$queryKeys[] = $key;
}
}
$queryMetadataPretty = '-';
if ($queryKeys !== []) {
$pretty = json_encode(['keys' => $queryKeys], JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
$queryMetadataPretty = is_string($pretty) ? $pretty : '-';
}
$userLabel = trim((string) ($auditLog['user_display_name'] ?? ''));
$userEmail = trim((string) ($auditLog['user_email'] ?? ''));
@@ -41,8 +48,8 @@ $requestId = trim((string) ($auditLog['request_id'] ?? ''));
$method = strtoupper(trim((string) ($auditLog['method'] ?? '')));
$path = trim((string) ($auditLog['path'] ?? ''));
$errorCode = trim((string) ($auditLog['error_code'] ?? ''));
$ip = trim((string) ($auditLog['ip'] ?? ''));
$userAgent = trim((string) ($auditLog['user_agent'] ?? ''));
$ipHash = trim((string) ($auditLog['ip_hash'] ?? $auditLog['ip'] ?? ''));
$userAgentHash = trim((string) ($auditLog['user_agent_hash'] ?? $auditLog['user_agent'] ?? ''));
$durationMs = (int) ($auditLog['duration_ms'] ?? 0);
$tokenId = (int) ($auditLog['api_token_id'] ?? 0);
$tokenTenantId = (int) ($auditLog['token_tenant_id'] ?? 0);
@@ -73,22 +80,22 @@ $tokenTenantId = (int) ($auditLog['token_tenant_id'] ?? 0);
<p><code><?php e($path !== '' ? $path : '-'); ?></code></p>
</div>
</div>
<?php if ($errorCode !== '' || $ip !== ''): ?>
<?php if ($errorCode !== '' || $ipHash !== ''): ?>
<div class="grid">
<div>
<small><?php e(t('Error code')); ?></small>
<p><?php e($errorCode !== '' ? $errorCode : '-'); ?></p>
</div>
<div>
<small><?php e(t('IP')); ?></small>
<p><?php e($ip !== '' ? $ip : '-'); ?></p>
<small><?php e(t('IP hash')); ?></small>
<p><code><?php e($ipHash !== '' ? substr($ipHash, 0, 16) . '...' : '-'); ?></code></p>
</div>
</div>
<?php endif; ?>
<?php if ($userAgent !== ''): ?>
<?php if ($userAgentHash !== ''): ?>
<div>
<small><?php e(t('User agent')); ?></small>
<textarea readonly rows="3"><?php e($userAgent); ?></textarea>
<small><?php e(t('User agent hash')); ?></small>
<p><code><?php e(substr($userAgentHash, 0, 16) . '...'); ?></code></p>
</div>
<?php endif; ?>
</details>
@@ -133,12 +140,12 @@ $tokenTenantId = (int) ($auditLog['token_tenant_id'] ?? 0);
<?php endif; ?>
</details>
<?php if ($queryPretty !== '-'): ?>
<?php if ($queryMetadataPretty !== '-'): ?>
<hr>
<details>
<summary><?php e(t('Query')); ?></summary>
<summary><?php e(t('Query metadata (keys)')); ?></summary>
<hr>
<textarea readonly rows="12"><?php e($queryPretty); ?></textarea>
<textarea readonly rows="8"><?php e($queryMetadataPretty); ?></textarea>
</details>
<?php endif; ?>
</div>

10
modules/audit/tests/Module/Audit/Providers/AuditSearchResourceProviderTest.php
View File

@@ -42,6 +42,16 @@ class AuditSearchResourceProviderTest extends TestCase
$this->assertSame('audit.system.view', $resources['system-audit']['permission']);
}
public function testApiAuditSearchSqlIncludesUserAgentHashField(): void
{
$resources = $this->provider->resources();
$apiAudit = $resources['api-audit'];
$this->assertStringContainsString('user_agent like ?', $apiAudit['count_sql']);
$this->assertStringContainsString('user_agent like ?', $apiAudit['preview_sql']);
$this->assertStringContainsString('user_agent like ?', $apiAudit['result_sql']);
}
public function testMapApiAuditItem(): void
{
$row = [

66
modules/audit/tests/Module/Audit/Service/ApiAuditServiceTest.php
View File

@@ -2,6 +2,7 @@
namespace MintyPHP\Tests\Module\Audit\Service;
use MintyPHP\Http\ApiAuth;
use MintyPHP\Http\RequestContext;
use MintyPHP\Http\RequestRuntime;
use MintyPHP\Module\Audit\Repository\ApiAuditLogRepository;
@@ -18,6 +19,7 @@ class ApiAuditServiceTest extends TestCase
$this->serverBackup = $_SERVER;
$this->getBackup = $_GET;
RequestContext::resetForTests();
ApiAuth::authenticate();
}
protected function tearDown(): void
@@ -25,6 +27,7 @@ class ApiAuditServiceTest extends TestCase
$_SERVER = $this->serverBackup;
$_GET = $this->getBackup;
RequestContext::resetForTests();
ApiAuth::authenticate();
}
public function testCurrentRequestIdIsNullBeforeStart(): void
@@ -73,4 +76,67 @@ class ApiAuditServiceTest extends TestCase
$this->assertNull($service->currentRequestId());
}
public function testFinishStoresOnlyAllowlistedQueryKeysAndHashedNetworkMetadata(): void
{
$_SERVER['REQUEST_METHOD'] = 'GET';
$_SERVER['REQUEST_URI'] = '/api/v1/users?active=1&email=user@example.com&token=secret&search=max';
$_SERVER['REMOTE_ADDR'] = '203.0.113.10';
$_SERVER['HTTP_USER_AGENT'] = 'TestAgent/1.0';
$_GET = [
'active' => '1',
'email' => 'user@example.com',
'token' => 'secret',
'search' => 'max',
];
$captured = null;
$repository = $this->createMock(ApiAuditLogRepository::class);
$repository->expects($this->once())
->method('create')
->willReturnCallback(static function (array $payload) use (&$captured): int {
$captured = $payload;
return 1;
});
$service = new ApiAuditService($repository, new RequestRuntime());
$service->startRequestContext();
$service->finish(204);
$this->assertIsArray($captured);
$decodedQuery = json_decode((string) ($captured['query_json'] ?? ''), true);
$this->assertSame(['keys' => ['active', 'search']], $decodedQuery);
$this->assertSame(RequestContext::hashValue('203.0.113.10'), $captured['ip']);
$this->assertSame(RequestContext::hashValue('TestAgent/1.0'), $captured['user_agent']);
$this->assertNotSame('203.0.113.10', $captured['ip']);
$this->assertNotSame('TestAgent/1.0', $captured['user_agent']);
}
public function testFinishSkipsQueryMetadataWhenNoAllowlistedKeyExists(): void
{
$_SERVER['REQUEST_METHOD'] = 'GET';
$_SERVER['REQUEST_URI'] = '/api/v1/users?email=user@example.com&token=secret';
$_SERVER['REMOTE_ADDR'] = '203.0.113.20';
$_SERVER['HTTP_USER_AGENT'] = 'NoAllowlistAgent/1.0';
$_GET = [
'email' => 'user@example.com',
'token' => 'secret',
];
$captured = null;
$repository = $this->createMock(ApiAuditLogRepository::class);
$repository->expects($this->once())
->method('create')
->willReturnCallback(static function (array $payload) use (&$captured): int {
$captured = $payload;
return 1;
});
$service = new ApiAuditService($repository, new RequestRuntime());
$service->startRequestContext();
$service->finish(200);
$this->assertIsArray($captured);
$this->assertNull($captured['query_json'] ?? null);
}
}

4
modules/audit/web/js/pages/admin-api-audit-index.js
View File

@@ -85,7 +85,7 @@ if (config) {
formatter: (cell) => (cell ? gridjs.html(`<code>${String(cell)}</code>`) : gridjs.html('-')),
},
{
name: labels.ip || 'IP',
name: labels.ip || 'IP hash',
sort: false,
},
{
@@ -114,7 +114,7 @@ if (config) {
label: row.tenant_label || '-',
},
row.error_code || '',
row.ip || '',
row.ip_hash || '',
row.id,
]),
rowInteraction: { linkColumn: 3 },

13
tests/Architecture/SecurityLoggingRedactionContractTest.php
View File

@@ -35,7 +35,7 @@ final class SecurityLoggingRedactionContractTest extends TestCase
self::assertStringContainsString("|| str_ends_with(\$key, '_key');", $content);
}
public function testApiAuditRedactionCoversCredentialsAndAuthorizationFields(): void
public function testApiAuditRedactionCoversSensitiveKeysAndPersistsOnlyMetadataHashes(): void
{
$content = $this->readProjectFile('modules/audit/lib/Module/Audit/Service/ApiAuditService.php');
@@ -50,14 +50,25 @@ final class SecurityLoggingRedactionContractTest extends TestCase
"'api_key'",
"'client_secret'",
"'key'",
"'email'",
"'to_email'",
"'uuid'",
] as $needle) {
self::assertStringContainsString($needle, $content);
}
self::assertStringContainsString("'query_json' => \$this->buildQueryMetadataJson(", $content);
self::assertStringContainsString("['keys' => \$keys]", $content);
self::assertStringContainsString('private const ALLOWED_QUERY_KEYS = [', $content);
self::assertStringContainsString("RequestContext::hashValue(\$ip)", $content);
self::assertStringContainsString("RequestContext::hashValue(\$userAgent)", $content);
self::assertStringNotContainsString('normalizeValue(', $content);
self::assertStringContainsString("return str_contains(\$key, 'token')", $content);
self::assertStringContainsString("|| str_contains(\$key, 'secret')", $content);
self::assertStringContainsString("|| str_contains(\$key, 'password')", $content);
self::assertStringContainsString("|| str_contains(\$key, 'authorization')", $content);
self::assertStringContainsString("|| str_contains(\$key, 'email')", $content);
self::assertStringContainsString("|| str_contains(\$key, 'uuid')", $content);
self::assertStringContainsString("|| str_ends_with(\$key, '_key');", $content);
}
}