From 1456c0ce5df6dd5b0e93feb5bcf9bdec947f590e Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 24 Mar 2026 12:15:27 +0100 Subject: [PATCH 1/2] fix: resolve pre-existing PHPCS violations on development --- lib/Migration/Version1Date20250828120000.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/Migration/Version1Date20250828120000.php b/lib/Migration/Version1Date20250828120000.php index 993c88ce3..1887f1f9e 100644 --- a/lib/Migration/Version1Date20250828120000.php +++ b/lib/Migration/Version1Date20250828120000.php @@ -40,15 +40,17 @@ */ class Version1Date20250828120000 extends SimpleMigrationStep { - /** - * @param IDBConnection $connection The database connection - * @param IConfig $config The configuration interface - */ + /** + * Constructor. + * + * @param IDBConnection $connection The database connection + * @param IConfig $config The configuration interface + */ public function __construct( private readonly IDBConnection $connection, private readonly IConfig $config, ) { - } + }//end __construct() /** * Apply database schema changes for faceting performance. @@ -65,7 +67,7 @@ public function __construct( */ public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { - /** + /* * @var ISchemaWrapper $schema */ From c604a9d888bfcb0acab27df0aaf93c727bffc576 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 24 Mar 2026 12:17:28 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Revert=20"Revert=20"feat:=20enhanced=20audi?= =?UTF-8?q?t=20trail=20=E2=80=94=20hash=20chaining,=20verification,=20verw?= =?UTF-8?q?erkingsregister=20(#950)""?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 8e57085f1e90e898f2f162a2e85c434dc91523a0. --- appinfo/routes.php | 6 +- lib/Controller/AuditTrailController.php | 174 ++++++++--- lib/Db/AuditTrail.php | 26 +- lib/Db/AuditTrailMapper.php | 166 +++++++++- lib/Migration/Version1Date20260322100000.php | 118 ++++++++ lib/Service/AuditHashService.php | 283 ++++++++++++++++++ .../.openspec.yaml | 2 + .../2026-03-22-enhanced-audit-trail/design.md | 88 ++++++ .../proposal.md | 33 ++ .../specs/audit-hash-chain/spec.md | 60 ++++ .../specs/audit-trail-immutable/spec.md | 34 +++ .../specs/verwerkingsregister-api/spec.md | 58 ++++ .../2026-03-22-enhanced-audit-trail/tasks.md | 59 ++++ openspec/specs/audit-hash-chain/spec.md | 69 +++++ openspec/specs/audit-trail-immutable/spec.md | 55 ++-- .../specs/verwerkingsregister-api/spec.md | 67 +++++ .../Controller/AuditTrailControllerTest.php | 254 ++++++++++++---- tests/Unit/Db/AuditTrailTest.php | 39 +++ tests/Unit/Service/AuditHashServiceTest.php | 173 +++++++++++ 19 files changed, 1635 insertions(+), 129 deletions(-) create mode 100644 lib/Migration/Version1Date20260322100000.php create mode 100644 lib/Service/AuditHashService.php create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/.openspec.yaml create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/design.md create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/proposal.md create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-hash-chain/spec.md create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-trail-immutable/spec.md create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/verwerkingsregister-api/spec.md create mode 100644 openspec/changes/archive/2026-03-22-enhanced-audit-trail/tasks.md create mode 100644 openspec/specs/audit-hash-chain/spec.md create mode 100644 openspec/specs/verwerkingsregister-api/spec.md create mode 100644 tests/Unit/Service/AuditHashServiceTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 759e83193..c2a42d7c6 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -260,12 +260,16 @@ ['name' => 'bulk#deleteSchemaObjects', 'url' => '/api/bulk/{register}/{schema}/delete-objects', 'verb' => 'POST'], ['name' => 'bulk#deleteRegister', 'url' => '/api/bulk/{register}/delete-register', 'verb' => 'POST'], ['name' => 'bulk#validateSchema', 'url' => '/api/bulk/schema/{schema}/validate', 'verb' => 'POST'], - // Audit Trails. + // Audit Trails — specific routes MUST come before parameterized {id} routes. ['name' => 'auditTrail#objects', 'url' => '/api/objects/{register}/{schema}/{id}/audit-trails', 'verb' => 'GET', 'requirements' => ['id' => '[^/]+']], ['name' => 'auditTrail#index', 'url' => '/api/audit-trails', 'verb' => 'GET'], ['name' => 'auditTrail#export', 'url' => '/api/audit-trails/export', 'verb' => 'GET'], + ['name' => 'auditTrail#verify', 'url' => '/api/audit-trails/verify', 'verb' => 'GET'], + ['name' => 'auditTrail#verwerkingsregister', 'url' => '/api/audit-trails/verwerkingsregister', 'verb' => 'GET'], + ['name' => 'auditTrail#inzageverzoek', 'url' => '/api/audit-trails/inzageverzoek', 'verb' => 'GET'], ['name' => 'auditTrail#clearAll', 'url' => '/api/audit-trails/clear-all', 'verb' => 'DELETE'], ['name' => 'auditTrail#show', 'url' => '/api/audit-trails/{id}', 'verb' => 'GET', 'requirements' => ['id' => '[^/]+']], + ['name' => 'auditTrail#update', 'url' => '/api/audit-trails/{id}', 'verb' => 'PUT', 'requirements' => ['id' => '[^/]+']], ['name' => 'auditTrail#destroy', 'url' => '/api/audit-trails/{id}', 'verb' => 'DELETE', 'requirements' => ['id' => '[^/]+']], ['name' => 'auditTrail#destroyMultiple', 'url' => '/api/audit-trails', 'verb' => 'DELETE'], // Search Trails - specific routes first, then general ones. diff --git a/lib/Controller/AuditTrailController.php b/lib/Controller/AuditTrailController.php index 6bbf3afd6..d6455b133 100644 --- a/lib/Controller/AuditTrailController.php +++ b/lib/Controller/AuditTrailController.php @@ -5,6 +5,7 @@ * * Controller for managing audit trail operations in the OpenRegister app. * Provides functionality to retrieve audit trails related to objects within registers and schemas. + * Includes hash chain verification, verwerkingsregister, and immutability enforcement. * * @category Controller * @package OCA\OpenRegister\AppInfo @@ -21,10 +22,11 @@ namespace OCA\OpenRegister\Controller; use OCA\OpenRegister\Db\AuditTrailMapper; +use OCA\OpenRegister\Service\AuditHashService; use OCA\OpenRegister\Service\LogService; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; -use OCP\AppFramework\Http\TemplateResponse; use OCP\IRequest; /** @@ -33,6 +35,9 @@ * Handles all audit trail related operations. * * @psalm-suppress UnusedClass + * + * @SuppressWarnings(PHPMD.ExcessiveClassLength) Controller covers audit trail, verification, verwerkingsregister + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) Necessary service dependencies */ class AuditTrailController extends Controller { @@ -43,12 +48,14 @@ class AuditTrailController extends Controller * @param IRequest $request The request object * @param LogService $logService The log service * @param AuditTrailMapper $auditTrailMapper The audit trail mapper + * @param AuditHashService $auditHashService The audit hash chain service */ public function __construct( string $appName, IRequest $request, private readonly LogService $logService, - private readonly AuditTrailMapper $auditTrailMapper + private readonly AuditTrailMapper $auditTrailMapper, + private readonly AuditHashService $auditHashService ) { parent::__construct(appName: $appName, request: $request); }//end __construct() @@ -58,8 +65,8 @@ public function __construct( * * @return array The extracted request parameters * - * @suppressWarnings(PHPMD.NPathComplexity) Request parameter extraction requires many conditional checks - * @suppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) Request parameter extraction requires many conditional checks + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function extractRequestParameters(): array { @@ -131,6 +138,10 @@ function ($key) { 'id', 'register', 'schema', + 'format', + 'from', + 'to', + 'identifier', ] ); }, @@ -216,6 +227,26 @@ public function show(int $id): JSONResponse } }//end show() + /** + * Reject audit trail modification (immutability enforcement). + * + * @param int $id The audit trail ID + * + * @return JSONResponse HTTP 405 Method Not Allowed + * + * @NoAdminRequired + * @NoCSRFRequired + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function update(int $id): JSONResponse + { + return new JSONResponse( + data: ['error' => 'Audit trail entries cannot be modified'], + statusCode: Http::STATUS_METHOD_NOT_ALLOWED + ); + }//end update() + /** * Get logs for an object * @@ -337,52 +368,23 @@ public function export(): JSONResponse }//end export() /** - * Delete a single audit trail log + * Reject audit trail deletion (immutability enforcement). * - * @param int $id The audit trail ID to delete + * @param int $id The audit trail ID * - * @NoAdminRequired + * @return JSONResponse HTTP 405 Method Not Allowed * + * @NoAdminRequired * @NoCSRFRequired * - * @return JSONResponse JSON response confirming deletion or error + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function destroy(int $id): JSONResponse { - try { - $success = $this->logService->deleteLog($id); - - if ($success === true) { - return new JSONResponse( - data: [ - 'success' => true, - 'message' => 'Audit trail deleted successfully', - ], - statusCode: 200 - ); - } - - return new JSONResponse( - data: [ - 'error' => 'Failed to delete audit trail', - ], - statusCode: 500 - ); - } catch (\OCP\AppFramework\Db\DoesNotExistException $e) { - return new JSONResponse( - data: [ - 'error' => 'Audit trail not found', - ], - statusCode: 404 - ); - } catch (\Exception $e) { - return new JSONResponse( - data: [ - 'error' => 'Deletion failed: '.$e->getMessage(), - ], - statusCode: 500 - ); - }//end try + return new JSONResponse( + data: ['error' => 'Audit trail entries cannot be deleted'], + statusCode: Http::STATUS_METHOD_NOT_ALLOWED + ); }//end destroy() /** @@ -487,4 +489,92 @@ public function clearAll(): JSONResponse ); }//end try }//end clearAll() + + /** + * Verify the integrity of the audit trail hash chain. + * + * @NoAdminRequired + * @NoCSRFRequired + * + * @return JSONResponse Verification result with valid/invalid status + */ + public function verify(): JSONResponse + { + $from = $this->request->getParam('from'); + $to = $this->request->getParam('to'); + + $fromInt = ($from !== null) ? (int) $from : null; + $toInt = ($to !== null) ? (int) $to : null; + + try { + $result = $this->auditHashService->verifyChain($fromInt, $toInt); + return new JSONResponse(data: $result); + } catch (\Exception $e) { + return new JSONResponse( + data: ['error' => 'Verification failed: '.$e->getMessage()], + statusCode: 500 + ); + } + }//end verify() + + /** + * Get verwerkingsregister (processing register) overview. + * + * Returns distinct processing activities from the audit trail with counts + * and date ranges, for GDPR Art 30 compliance. + * + * @NoAdminRequired + * @NoCSRFRequired + * + * @return JSONResponse List of processing activities + * + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function verwerkingsregister(): JSONResponse + { + $organisationId = $this->request->getParam('organisationId'); + + try { + $results = $this->auditTrailMapper->getProcessingActivities($organisationId); + return new JSONResponse(data: $results); + } catch (\Exception $e) { + return new JSONResponse( + data: ['error' => 'Failed to retrieve verwerkingsregister: '.$e->getMessage()], + statusCode: 500 + ); + } + }//end verwerkingsregister() + + /** + * Handle a data subject access request (inzageverzoek). + * + * Searches audit trail entries by identifier in the changed JSON field, + * grouped by schema. + * + * @NoAdminRequired + * @NoCSRFRequired + * + * @return JSONResponse Matching audit trail entries grouped by schema + */ + public function inzageverzoek(): JSONResponse + { + $identifier = $this->request->getParam('identifier'); + + if ($identifier === null || $identifier === '') { + return new JSONResponse( + data: ['error' => 'identifier parameter is required'], + statusCode: 400 + ); + } + + try { + $results = $this->auditTrailMapper->findByIdentifier($identifier); + return new JSONResponse(data: $results); + } catch (\Exception $e) { + return new JSONResponse( + data: ['error' => 'Inzageverzoek failed: '.$e->getMessage()], + statusCode: 500 + ); + } + }//end inzageverzoek() }//end class diff --git a/lib/Db/AuditTrail.php b/lib/Db/AuditTrail.php index 4bc23a573..a9b6d4336 100644 --- a/lib/Db/AuditTrail.php +++ b/lib/Db/AuditTrail.php @@ -61,6 +61,10 @@ * @method void setOrganisation(?string $organisation) * @method DateTime|null getExpires() * @method void setExpires(?DateTime $expires) + * @method string|null getHash() + * @method void setHash(?string $hash) + * @method string|null getPreviousHash() + * @method void setPreviousHash(?string $previousHash) * * @psalm-suppress PossiblyUnusedMethod * @psalm-suppress PropertyNotSetInConstructor $id is set by Nextcloud's Entity base class @@ -257,6 +261,20 @@ class AuditTrail extends Entity implements JsonSerializable */ protected ?DateTime $expires = null; + /** + * SHA-256 hash of this entry chained to the previous entry + * + * @var string|null SHA-256 hash of this entry chained to the previous entry + */ + protected ?string $hash = null; + + /** + * SHA-256 hash of the previous audit trail entry in the chain + * + * @var string|null SHA-256 hash of the previous audit trail entry + */ + protected ?string $previousHash = null; + /** * Constructor for the AuditTrail class * @@ -289,6 +307,8 @@ public function __construct() $this->addType(fieldName: 'retentionPeriod', type: 'string'); $this->addType(fieldName: 'size', type: 'integer'); $this->addType(fieldName: 'expires', type: 'datetime'); + $this->addType(fieldName: 'hash', type: 'string'); + $this->addType(fieldName: 'previousHash', type: 'string'); }//end __construct() /** @@ -385,7 +405,9 @@ public function hydrate(array $object): static * confidentiality: null|string, * retentionPeriod: null|string, * size: int|null, - * expires: null|string + * expires: null|string, + * hash: null|string, + * previousHash: null|string * } */ public function jsonSerialize(): array @@ -427,6 +449,8 @@ public function jsonSerialize(): array 'retentionPeriod' => $this->retentionPeriod, 'size' => $this->size, 'expires' => $expires, + 'hash' => $this->hash, + 'previousHash' => $this->previousHash, ]; }//end jsonSerialize() diff --git a/lib/Db/AuditTrailMapper.php b/lib/Db/AuditTrailMapper.php index f94ffe110..f985fee25 100644 --- a/lib/Db/AuditTrailMapper.php +++ b/lib/Db/AuditTrailMapper.php @@ -31,6 +31,7 @@ use RuntimeException; use stdClass; use ReflectionClass; +use OCA\OpenRegister\Service\AuditHashService; use OCP\IDBConnection; use Symfony\Component\Uid\Uuid; @@ -61,17 +62,55 @@ class AuditTrailMapper extends QBMapper /** * Constructor for the AuditTrailMapper * - * @param IDBConnection $db The database connection - * @param \Psr\Container\ContainerInterface $container DI container for lazy mapper resolution + * @param IDBConnection $db The database connection + * @param \Psr\Container\ContainerInterface $container DI container for lazy mapper resolution + * @param AuditHashService $auditHashService Hash chain service */ public function __construct( IDBConnection $db, - private readonly \Psr\Container\ContainerInterface $container + private readonly \Psr\Container\ContainerInterface $container, + private readonly AuditHashService $auditHashService ) { parent::__construct(db: $db, tableName: 'openregister_audit_trails', entityClass: AuditTrail::class); }//end __construct() + /** + * Insert a new audit trail entry with hash chain computation. + * + * Wraps the insert in a transaction to serialize hash chain writes + * and prevent race conditions. + * + * @param Entity $entity The audit trail entity to insert + * + * @return AuditTrail The inserted entity with hash fields set + */ + public function insert(Entity $entity): AuditTrail + { + // Use a transaction to serialize hash chain writes. + $this->db->beginTransaction(); + + try { + // Get the last hash in the chain (locks the row for serialization). + $previousHash = $this->auditHashService->getLastHash(); + + // Compute the hash for this entry. + $hash = $this->auditHashService->computeHash($entity, $previousHash); + // Set hash fields on the entity. + $entity->setPreviousHash($previousHash); + $entity->setHash($hash); + + // Call parent insert. + $result = parent::insert(entity: $entity); + + $this->db->commit(); + + return $result; + } catch (\Exception $e) { + $this->db->rollBack(); + throw $e; + }//end try + }//end insert() /** * Finds an audit trail by id @@ -1183,5 +1222,126 @@ public function getStatisticsGroupedBySchema(array $schemaIds): array }//end try }//end getStatisticsGroupedBySchema() + /** + * Get distinct processing activities from the audit trail. + * + * Returns aggregated processing activity data for the verwerkingsregister, + * including entry counts and first/last seen timestamps. + * + * @param string|null $organisationId Optional filter by organisation ID + * + * @return array List of processing activity summaries + * + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function getProcessingActivities(?string $organisationId=null): array + { + $qb = $this->db->getQueryBuilder(); + + $qb->select('processing_activity_id') + ->addSelect('processing_activity_url') + ->addSelect('organisation_id') + ->addSelect('organisation_id_type') + ->addSelect('confidentiality') + ->addSelect('retention_period') + ->selectAlias($qb->func()->count('*'), 'entry_count') + ->selectAlias($qb->func()->min('created'), 'first_seen') + ->selectAlias($qb->func()->max('created'), 'last_seen') + ->from('openregister_audit_trails') + ->where( + $qb->expr()->isNotNull('processing_activity_id') + ) + ->andWhere( + $qb->expr()->neq('processing_activity_id', $qb->createNamedParameter('')) + ) + ->groupBy('processing_activity_id') + ->addGroupBy('processing_activity_url') + ->addGroupBy('organisation_id') + ->addGroupBy('organisation_id_type') + ->addGroupBy('confidentiality') + ->addGroupBy('retention_period'); + + if ($organisationId !== null && $organisationId !== '') { + $qb->andWhere( + $qb->expr()->eq( + 'organisation_id', + $qb->createNamedParameter($organisationId) + ) + ); + } + + $result = $qb->executeQuery(); + $rows = []; + + while (($row = $result->fetch()) !== false) { + $rows[] = [ + 'processingActivityId' => $row['processing_activity_id'], + 'processingActivityUrl' => $row['processing_activity_url'], + 'organisationId' => $row['organisation_id'], + 'organisationIdType' => $row['organisation_id_type'], + 'confidentiality' => $row['confidentiality'], + 'retentionPeriod' => $row['retention_period'], + 'entryCount' => (int) $row['entry_count'], + 'firstSeen' => $row['first_seen'], + 'lastSeen' => $row['last_seen'], + ]; + } + + $result->closeCursor(); + return $rows; + }//end getProcessingActivities() + + /** + * Find audit trail entries by identifier in the changed JSON field. + * + * Groups results by schema UUID for data subject access requests. + * + * @param string $identifier The identifier to search for + * + * @return array{results: array, totalEntries: int} Grouped audit entries + */ + public function findByIdentifier(string $identifier): array + { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from('openregister_audit_trails') + ->where( + $qb->expr()->like( + 'changed', + $qb->createNamedParameter('%'.$this->db->escapeLikeParameter($identifier).'%') + ) + ) + ->orderBy('created', 'DESC'); + + $entries = $this->findEntities(query: $qb); + + if (count($entries) === 0) { + return [ + 'results' => [], + 'totalEntries' => 0, + ]; + } + + // Group by schema UUID. + $grouped = []; + foreach ($entries as $entry) { + $schemaUuid = $entry->getSchemaUuid() ?? 'unknown'; + + if (isset($grouped[$schemaUuid]) === false) { + $grouped[$schemaUuid] = [ + 'schemaUuid' => $schemaUuid, + 'entries' => [], + ]; + } + + $grouped[$schemaUuid]['entries'][] = $entry; + } + + return [ + 'results' => array_values($grouped), + 'totalEntries' => count($entries), + ]; + }//end findByIdentifier() }//end class diff --git a/lib/Migration/Version1Date20260322100000.php b/lib/Migration/Version1Date20260322100000.php new file mode 100644 index 000000000..3a7e75dd1 --- /dev/null +++ b/lib/Migration/Version1Date20260322100000.php @@ -0,0 +1,118 @@ + + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://OpenRegister.app + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Adds hash chain columns and processing activity index to audit trails table. + * + * @package OCA\OpenRegister\Migration + * + * @psalm-suppress UnusedClass + */ +class Version1Date20260322100000 extends SimpleMigrationStep +{ + /** + * Change the database schema. + * + * @param IOutput $output Migration output + * @param Closure(): ISchemaWrapper $schemaClosure Schema closure + * @param array $options Migration options + * + * @return ISchemaWrapper|null The updated schema or null if no changes + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper + { + // Get the schema wrapper from the closure. + $schema = $schemaClosure(); + + $tableName = 'openregister_audit_trails'; + + if ($schema->hasTable($tableName) === false) { + $output->info("Table {$tableName} does not exist, skipping migration"); + return null; + } + + $table = $schema->getTable($tableName); + $changed = false; + + // Add hash column for cryptographic chain integrity. + if ($table->hasColumn('hash') === false) { + $table->addColumn( + 'hash', + Types::STRING, + [ + 'notnull' => false, + 'length' => 64, + 'default' => null, + 'comment' => 'SHA-256 hash of this entry chained to previous entry', + ] + ); + $output->info("Added 'hash' column to {$tableName}"); + $changed = true; + } + + // Add previous_hash column linking to the preceding entry. + if ($table->hasColumn('previous_hash') === false) { + $table->addColumn( + 'previous_hash', + Types::STRING, + [ + 'notnull' => false, + 'length' => 64, + 'default' => null, + 'comment' => 'SHA-256 hash of the previous audit trail entry', + ] + ); + $output->info("Added 'previous_hash' column to {$tableName}"); + $changed = true; + } + + // Add index on hash column for verification queries. + if ($table->hasIndex('idx_audit_hash') === false) { + $table->addIndex(['hash'], 'idx_audit_hash'); + $output->info("Added index 'idx_audit_hash' on {$tableName}"); + $changed = true; + } + + // Add index on processing_activity_id for verwerkingsregister queries. + if ($table->hasIndex('idx_audit_proc_act') === false + && $table->hasColumn('processing_activity_id') === true + ) { + $table->addIndex(['processing_activity_id'], 'idx_audit_proc_act'); + $output->info("Added index 'idx_audit_proc_act' on {$tableName}"); + $changed = true; + } + + if ($changed === false) { + $output->info("All columns and indexes already exist on {$tableName}, skipping"); + return null; + } + + return $schema; + }//end changeSchema() +}//end class diff --git a/lib/Service/AuditHashService.php b/lib/Service/AuditHashService.php new file mode 100644 index 000000000..e08e58eee --- /dev/null +++ b/lib/Service/AuditHashService.php @@ -0,0 +1,283 @@ + + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://OpenRegister.app + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Service; + +use OCA\OpenRegister\Db\AuditTrail; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * Handles cryptographic hash chaining for audit trail entries. + * + * @package OCA\OpenRegister\Service + * + * @psalm-suppress UnusedClass + */ +class AuditHashService +{ + /** + * The genesis seed used for the first entry in the hash chain. + * + * @var string + */ + private const GENESIS_SEED = 'openregister-genesis-v1'; + + /** + * Constructor for AuditHashService. + * + * @param IDBConnection $db The database connection + */ + public function __construct( + private readonly IDBConnection $db + ) { + }//end __construct() + + /** + * Compute the genesis hash (used as previousHash for the first entry). + * + * @return string The SHA-256 hex digest of the genesis seed + */ + public function getGenesisHash(): string + { + return hash('sha256', self::GENESIS_SEED); + }//end getGenesisHash() + + /** + * Get the canonical JSON representation of an audit trail entry for hashing. + * + * Excludes the `hash` and `previousHash` fields and uses sorted keys + * with no whitespace (compact canonical form). + * + * @param AuditTrail $entry The audit trail entry + * + * @return string The canonical JSON string + */ + public function getCanonicalJson(AuditTrail $entry): string + { + $data = $entry->jsonSerialize(); + + // Remove hash chain fields from the canonical representation. + unset($data['hash'], $data['previousHash']); + + // Sort keys for deterministic output. + ksort($data); + + return json_encode($data, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + }//end getCanonicalJson() + + /** + * Compute the SHA-256 hash for an audit trail entry. + * + * @param AuditTrail $entry The audit trail entry to hash + * @param string $previousHash The hash of the previous entry in the chain + * + * @return string The SHA-256 hex digest + */ + public function computeHash(AuditTrail $entry, string $previousHash): string + { + $canonicalJson = $this->getCanonicalJson(entry: $entry); + + return hash('sha256', $previousHash.$canonicalJson); + }//end computeHash() + + /** + * Get the hash of the most recent audit trail entry. + * + * Returns the genesis hash if no entries exist or the last entry has no hash. + * + * @return string The hash of the last entry or the genesis hash + */ + public function getLastHash(): string + { + $qb = $this->db->getQueryBuilder(); + + $qb->select('hash') + ->from('openregister_audit_trails') + ->orderBy('id', 'DESC') + ->setMaxResults(1); + + $result = $qb->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row === false || $row['hash'] === null || $row['hash'] === '') { + return $this->getGenesisHash(); + } + + return $row['hash']; + }//end getLastHash() + + /** + * Verify the integrity of the hash chain. + * + * Iterates audit trail entries in order and validates that each entry's + * stored hash matches the recomputed hash. + * + * @param int|null $from Start entry ID (inclusive), null for beginning + * @param int|null $to End entry ID (inclusive), null for end + * + * @return array{valid: bool, entriesVerified: int, brokenAt: int|null, skippedNullHashes: int, range?: array{from: int, to: int}} + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ + public function verifyChain(?int $from=null, ?int $to=null): array + { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from('openregister_audit_trails') + ->orderBy('id', 'ASC'); + + if ($from !== null) { + $qb->andWhere( + $qb->expr()->gte('id', $qb->createNamedParameter($from, IQueryBuilder::PARAM_INT)) + ); + } + + if ($to !== null) { + $qb->andWhere( + $qb->expr()->lte('id', $qb->createNamedParameter($to, IQueryBuilder::PARAM_INT)) + ); + } + + $result = $qb->executeQuery(); + + $entriesVerified = 0; + $skippedNullHashes = 0; + $previousHash = null; + + // If starting from a specific ID, get the previous entry's hash. + if ($from !== null) { + $previousHash = $this->getHashBefore(id: $from); + } + + while (($row = $result->fetch()) !== false) { + $storedHash = $row['hash'] ?? null; + + // Skip entries without hashes (pre-migration entries). + if ($storedHash === null || $storedHash === '') { + $skippedNullHashes++; + continue; + } + + $entry = new AuditTrail(); + $entry->hydrate(object: $this->mapRowToEntity(row: $row)); + + // Determine the previous hash for verification. + if ($previousHash === null) { + $previousHash = $this->getGenesisHash(); + } + + $computedHash = $this->computeHash(entry: $entry, previousHash: $previousHash); + + if ($computedHash !== $storedHash) { + $result->closeCursor(); + + $response = [ + 'valid' => false, + 'entriesVerified' => $entriesVerified, + 'brokenAt' => (int) $row['id'], + 'skippedNullHashes' => $skippedNullHashes, + ]; + + if ($from !== null || $to !== null) { + $response['range'] = [ + 'from' => $from ?? (int) $row['id'], + 'to' => $to ?? (int) $row['id'], + ]; + } + + return $response; + } + + $previousHash = $storedHash; + $entriesVerified++; + }//end while + + $result->closeCursor(); + + $response = [ + 'valid' => true, + 'entriesVerified' => $entriesVerified, + 'brokenAt' => null, + 'skippedNullHashes' => $skippedNullHashes, + ]; + + if ($from !== null || $to !== null) { + $response['range'] = [ + 'from' => $from, + 'to' => $to, + ]; + } + + return $response; + }//end verifyChain() + + /** + * Get the hash of the entry immediately before the given ID. + * + * @param int $id The entry ID + * + * @return string|null The hash of the previous entry, or null if none + */ + private function getHashBefore(int $id): ?string + { + $qb = $this->db->getQueryBuilder(); + + $qb->select('hash') + ->from('openregister_audit_trails') + ->where( + $qb->expr()->lt('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) + ) + ->orderBy('id', 'DESC') + ->setMaxResults(1); + + $result = $qb->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row === false || $row['hash'] === null || $row['hash'] === '') { + return null; + } + + return $row['hash']; + }//end getHashBefore() + + /** + * Map a database row to entity-compatible array with camelCase keys. + * + * @param array $row The database row + * + * @return array The mapped array with camelCase keys + */ + private function mapRowToEntity(array $row): array + { + $mapped = []; + foreach ($row as $key => $value) { + // Convert snake_case to camelCase. + $camelKey = lcfirst( + str_replace('_', '', ucwords($key, '_')) + ); + $mapped[$camelKey] = $value; + } + + return $mapped; + }//end mapRowToEntity() +}//end class diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/.openspec.yaml b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/.openspec.yaml new file mode 100644 index 000000000..caac5173b --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-22 diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/design.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/design.md new file mode 100644 index 000000000..377686e9f --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/design.md @@ -0,0 +1,88 @@ +## Context + +OpenRegister has a fully functional AuditTrail system that records create, update, and delete operations on register objects. The `AuditTrail` entity already stores user, action, changed data, timestamps, and verwerkingenlogging fields (organisation ID, processing activity ID, confidentiality, retention period). However, two critical compliance gaps remain: + +1. **No tamper evidence**: Entries can theoretically be modified in the database without detection. Government auditors require cryptographic proof that the log has not been altered. +2. **No verwerkingsregister API**: While fields exist for GDPR Art 30 data, there is no dedicated API to query processing activities, generate data subject access reports, or export the register in a structured format. + +Current stack: PHP 8.1+, Nextcloud OCP framework, PostgreSQL, Nextcloud Entity/Mapper pattern. + +## Goals / Non-Goals + +**Goals:** +- Add SHA-256 hash chaining to every audit trail entry for tamper detection +- Provide a verification endpoint that validates the entire chain or a subset +- Enforce true immutability: HTTP 405 on PUT/PATCH/DELETE for audit trail records +- Expose a verwerkingsregister API for Art 30 compliance queries +- Support data subject access requests (inzageverzoek) through the API +- Enable audit trail export in JSON and CSV formats + +**Non-Goals:** +- Purpose-bound access control (blocking access without valid processing purpose) — deferred to a future change as it requires deep integration with the RBAC system +- PDF export of data subject access reports — JSON export is sufficient for now +- Real-time streaming of audit events — batch queries are adequate +- External audit trail storage (e.g., blockchain, external SIEM) — out of scope + +## Decisions + +### 1. Hash chain implementation in AuditHashService + +**Decision**: Create a dedicated `AuditHashService` that computes SHA-256 hashes at insert time and provides verification. + +**Rationale**: Separating hash logic from the mapper keeps the AuditTrailMapper focused on CRUD and allows the hash service to be independently tested. The service is called from the mapper's `insert()` method override. + +**Alternatives considered**: +- Hash in AuditTrailMapper directly — rejected because it mixes concerns and makes testing harder +- Hash in a Nextcloud event listener — rejected because it would allow a window where unhashed entries exist + +**Hash formula**: `SHA-256(previous_hash + JSON(entry_data_without_hash_fields))` +- Genesis hash for first entry: `SHA-256("openregister-genesis-v1")` +- Entry data is canonical JSON (sorted keys, no whitespace) excluding `hash` and `previousHash` fields + +### 2. Database migration for hash columns + +**Decision**: Add `hash` VARCHAR(64) and `previous_hash` VARCHAR(64) columns to `openregister_audit_trails`, both nullable to support existing data. + +**Rationale**: Existing entries predate hash chaining. Nullable columns allow backward compatibility. A backfill command can optionally chain existing entries. + +**Migration**: Standard Nextcloud `IMigrationStep` with `changeSchema()`. + +### 3. Immutability enforcement at controller level + +**Decision**: Override update/delete routes in `AuditTrailController` to return HTTP 405. + +**Rationale**: The controller already handles CRUD. Blocking at this level is the simplest approach that works within Nextcloud's routing. Database-level triggers are PostgreSQL-specific and not portable. + +### 4. Verwerkingsregister as dedicated endpoints on AuditTrailController + +**Decision**: Add verwerkingsregister endpoints to the existing `AuditTrailController` rather than creating a separate controller. + +**Rationale**: The verwerkingsregister queries the same `openregister_audit_trails` table. A separate controller would add routing complexity without clear benefit. The endpoints are: +- `GET /api/audit-trails/verwerkingsregister` — list processing activities +- `GET /api/audit-trails/inzageverzoek/{identifier}` — data subject access report +- `GET /api/audit-trails/export` — export audit trail as JSON/CSV + +### 5. Verification endpoint design + +**Decision**: `GET /api/audit-trails/verify` with optional `?from={id}&to={id}` range parameters. + +**Rationale**: Full chain verification can be expensive (O(n)). Range parameters allow auditors to verify specific segments. The response includes: valid/invalid status, first broken entry (if any), and total entries verified. + +## Risks / Trade-offs + +- **[Performance]** Hash computation adds ~1ms per audit write. For bulk imports (1000+ objects), this adds measurable latency. Mitigation: hash computation is a single SHA-256 call per entry, which is CPU-cheap. +- **[Concurrency]** Two simultaneous writes could race for the "previous hash". Mitigation: Use a database transaction with `SELECT ... FOR UPDATE` on the last entry to serialize hash chain writes. +- **[Backward compatibility]** Existing audit entries have no hash. Mitigation: Nullable hash columns; verification endpoint skips entries with null hashes and reports the unverified range. +- **[Data volume]** Verwerkingsregister queries on large audit tables could be slow. Mitigation: Add database index on `processing_activity_id` column in the migration. + +## Migration Plan + +1. Deploy migration adding `hash` and `previous_hash` columns (nullable) +2. New entries automatically get hashed from deployment onward +3. Optional: run `occ openregister:audit:backfill-hashes` to chain existing entries (out of scope for this change, but the hash service supports it) +4. Rollback: drop the two columns; no data loss since existing functionality is unaffected + +## Open Questions + +- Should the backfill command for existing entries be included in this change or deferred? (Recommendation: defer) +- Should verification results be cached for performance on repeated checks? (Recommendation: no caching — auditors expect fresh results) diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/proposal.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/proposal.md new file mode 100644 index 000000000..2aebb59c0 --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/proposal.md @@ -0,0 +1,33 @@ +## Why + +Government tenders (56-58% of analyzed 39K tenders) require both immutable audit trails with tamper-evidence and GDPR Article 30 processing registers (verwerkingsregister). OpenRegister already has a functional AuditTrail entity with verwerkingenlogging fields (`processingActivityId`, `organisationId`, etc.), but lacks cryptographic hash chaining for tamper detection, a verification API to prove chain integrity, and a structured API for querying the verwerkingsregister. This change bridges the gap between the existing implementation and the compliance requirements demanded by Dutch government procurement. + +## What Changes + +- Add cryptographic hash chaining (SHA-256) to all audit trail entries, making the log tamper-evident +- Add a hash chain verification endpoint that auditors can use to prove integrity +- Add a `hash` and `previousHash` column to the `AuditTrail` entity and a database migration +- Expose a verwerkingsregister (processing register) API endpoint that returns Art 30-compliant processing activity overviews +- Add data subject access request (inzageverzoek) support: query all audit entries by a subject identifier +- Add an audit trail export endpoint (JSON/CSV) for external compliance auditing +- Ensure audit trail entries are truly immutable: block PUT/PATCH/DELETE on audit trail records via the API + +## Capabilities + +### New Capabilities +- `audit-hash-chain`: Cryptographic SHA-256 hash chaining on audit trail entries with genesis hash, verification endpoint, and tamper detection reporting +- `verwerkingsregister-api`: GDPR Art 30 processing register API — query processing activities, generate data subject access reports, export verwerkingsregister + +### Modified Capabilities +- `audit-trail-immutable`: Add `hash` and `previousHash` fields to the AuditTrail entity, enforce immutability by blocking modification/deletion API endpoints (HTTP 405) + +## Impact + +- **Database**: New migration adding `hash` (VARCHAR 64) and `previous_hash` (VARCHAR 64) columns to `openregister_audit_trails` table +- **Entity**: `AuditTrail.php` — two new fields, hash computation logic +- **Controller**: `AuditTrailController.php` — new verification endpoint, immutability enforcement, export endpoint +- **New service**: `AuditHashService.php` — hash chain computation and verification logic +- **New controller**: `VerwerkingsregisterController.php` — processing register and inzageverzoek endpoints +- **Routes**: New API routes for verification, export, verwerkingsregister +- **Dependent apps**: opencatalogi/softwarecatalog use AuditTrail read-only — no breaking changes expected +- **Performance**: Hash computation adds ~1ms per write; verification is O(n) over chain length, should be async for large datasets diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-hash-chain/spec.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-hash-chain/spec.md new file mode 100644 index 000000000..ac87dea93 --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-hash-chain/spec.md @@ -0,0 +1,60 @@ +## ADDED Requirements + +### Requirement: Every audit trail entry MUST include a SHA-256 hash chained to the previous entry +Each audit trail entry MUST contain a `hash` field computed as `SHA-256(previous_hash + canonical_json(entry_data))`. The `previous_hash` field links to the preceding entry's hash, forming a tamper-evident chain. + +#### Scenario: First audit entry uses genesis hash +- **WHEN** the first audit trail entry is created in the system (no previous entries exist) +- **THEN** the entry MUST have `previousHash` set to `SHA-256("openregister-genesis-v1")` +- **AND** the entry MUST have `hash` set to `SHA-256(genesis_hash + canonical_json(entry_data))` + +#### Scenario: Subsequent entries chain to previous hash +- **WHEN** audit trail entry N is created after entry N-1 with hash `abc123...` +- **THEN** entry N MUST have `previousHash` set to `abc123...` +- **AND** entry N MUST have `hash` set to `SHA-256("abc123..." + canonical_json(entry_data_N))` + +#### Scenario: Canonical JSON excludes hash fields +- **WHEN** computing the hash for an audit trail entry +- **THEN** the canonical JSON MUST include all entry fields except `hash` and `previousHash` +- **AND** the JSON MUST use sorted keys and no whitespace (compact canonical form) + +### Requirement: The system MUST provide a hash chain verification endpoint +An API endpoint MUST allow auditors to verify the integrity of the hash chain. + +#### Scenario: Verify full chain integrity +- **WHEN** a GET request is made to `/api/audit-trails/verify` +- **THEN** the system MUST iterate all audit trail entries in order +- **AND** recompute each hash and compare to the stored hash +- **AND** return a JSON response with `{"valid": true, "entriesVerified": }` + +#### Scenario: Verify chain with range parameters +- **WHEN** a GET request is made to `/api/audit-trails/verify?from=100&to=200` +- **THEN** the system MUST verify only entries with IDs between 100 and 200 (inclusive) +- **AND** return `{"valid": true, "entriesVerified": 101, "range": {"from": 100, "to": 200}}` + +#### Scenario: Detect tampered entry in chain +- **WHEN** an entry in the chain has been modified after creation (stored hash does not match recomputed hash) +- **THEN** the verification endpoint MUST return `{"valid": false, "brokenAt": , "entriesVerified": }` +- **AND** the `brokenAt` field MUST identify the first entry where the chain breaks + +#### Scenario: Handle entries without hashes (pre-migration) +- **WHEN** the verification encounters entries with null `hash` values (created before hash chaining was enabled) +- **THEN** those entries MUST be skipped in the verification +- **AND** the response MUST include `"skippedNullHashes": ` + +### Requirement: Hash chain writes MUST be serialized to prevent race conditions +Concurrent audit trail inserts MUST NOT produce broken hash chains. + +#### Scenario: Two simultaneous audit writes +- **WHEN** two audit trail entries are created at the same moment +- **THEN** both entries MUST be correctly chained (each referencing the correct previous hash) +- **AND** no two entries MUST share the same `previousHash` value (except the genesis hash for the first entry) + +### Requirement: A database migration MUST add hash columns +The migration MUST add `hash` and `previous_hash` columns to the audit trails table. + +#### Scenario: Migration adds nullable hash columns +- **WHEN** the migration runs on an existing database with audit trail entries +- **THEN** columns `hash` (VARCHAR 64) and `previous_hash` (VARCHAR 64) MUST be added +- **AND** existing entries MUST retain null values for both columns +- **AND** an index MUST be created on the `hash` column for verification queries diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-trail-immutable/spec.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-trail-immutable/spec.md new file mode 100644 index 000000000..5e13cb927 --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/audit-trail-immutable/spec.md @@ -0,0 +1,34 @@ +## ADDED Requirements + +### Requirement: The AuditTrail entity MUST include hash and previousHash fields +The `AuditTrail` entity MUST be extended with `hash` and `previousHash` string fields for cryptographic chain integrity. + +#### Scenario: New audit trail entry includes hash fields in JSON +- **WHEN** an audit trail entry with hash chaining is serialized to JSON +- **THEN** the JSON output MUST include `hash` and `previousHash` string fields +- **AND** both fields MUST be 64-character hexadecimal strings (SHA-256 output) + +#### Scenario: Legacy entry without hash fields +- **WHEN** an audit trail entry created before hash chaining is serialized to JSON +- **THEN** the JSON output MUST include `hash` and `previousHash` as null values + +## MODIFIED Requirements + +### Requirement: Audit trail entries MUST NOT be deletable or modifiable +No user, including administrators, MUST be able to modify or delete audit trail entries through the application. + +#### Scenario: Reject audit trail deletion via API +- GIVEN an admin user attempts to DELETE `/api/audit-trails/{id}` +- THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be deleted"}` +- AND the audit entry MUST remain unchanged + +#### Scenario: Reject audit trail modification via PUT +- GIVEN an admin attempts to PUT `/api/audit-trails/{id}` with modified data +- THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be modified"}` + +#### Scenario: Reject audit trail modification via PATCH +- GIVEN an admin attempts to PATCH `/api/audit-trails/{id}` with modified data +- THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be modified"}` diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/verwerkingsregister-api/spec.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/verwerkingsregister-api/spec.md new file mode 100644 index 000000000..6b4cd08a1 --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/specs/verwerkingsregister-api/spec.md @@ -0,0 +1,58 @@ +## ADDED Requirements + +### Requirement: The system MUST provide a verwerkingsregister (processing register) API +A dedicated API endpoint MUST return an overview of all processing activities recorded in the audit trail, grouped by processing activity ID. + +#### Scenario: List all processing activities +- **WHEN** a GET request is made to `/api/audit-trails/verwerkingsregister` +- **THEN** the system MUST return a JSON array of distinct processing activities +- **AND** each entry MUST include `processingActivityId`, `processingActivityUrl`, `organisationId`, `organisationIdType`, `confidentiality`, and `retentionPeriod` +- **AND** each entry MUST include `entryCount` (number of audit entries for this activity) +- **AND** each entry MUST include `firstSeen` and `lastSeen` timestamps + +#### Scenario: Filter verwerkingsregister by organisation +- **WHEN** a GET request is made to `/api/audit-trails/verwerkingsregister?organisationId=00000001234567890000` +- **THEN** the system MUST return only processing activities for that organisation + +#### Scenario: Empty verwerkingsregister +- **WHEN** no audit trail entries have a `processingActivityId` set +- **THEN** the endpoint MUST return an empty JSON array `[]` + +### Requirement: The system MUST support data subject access requests (inzageverzoek) +An API endpoint MUST allow querying all audit trail entries related to a specific data subject, identified by a search term in the `changed` field. + +#### Scenario: Query audit entries for a data subject +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek?identifier=123456789` +- **THEN** the system MUST search all audit trail entries where the `changed` JSON field contains the identifier +- **AND** return a JSON response with all matching entries grouped by schema +- **AND** each group MUST include the schema UUID, schema name (if available), and the list of matching entries + +#### Scenario: Inzageverzoek with no matching entries +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek?identifier=nonexistent` +- **THEN** the system MUST return `{"results": [], "totalEntries": 0}` + +#### Scenario: Inzageverzoek requires identifier parameter +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek` without an `identifier` parameter +- **THEN** the system MUST return HTTP 400 with `{"error": "identifier parameter is required"}` + +### Requirement: The system MUST support audit trail export +An API endpoint MUST allow exporting audit trail entries in JSON or CSV format for external compliance auditing. + +#### Scenario: Export audit trail as JSON +- **WHEN** a GET request is made to `/api/audit-trails/export?format=json` +- **THEN** the system MUST return all audit trail entries as a JSON array +- **AND** the response MUST include Content-Disposition header for file download + +#### Scenario: Export audit trail as CSV +- **WHEN** a GET request is made to `/api/audit-trails/export?format=csv` +- **THEN** the system MUST return all audit trail entries as CSV +- **AND** the first row MUST contain column headers +- **AND** the `changed` field MUST be serialized as a JSON string within the CSV cell + +#### Scenario: Export with date range filter +- **WHEN** a GET request is made to `/api/audit-trails/export?format=json&from=2025-01-01&to=2025-12-31` +- **THEN** the system MUST return only entries with `created` timestamps within the specified range + +#### Scenario: Export defaults to JSON +- **WHEN** a GET request is made to `/api/audit-trails/export` without a `format` parameter +- **THEN** the system MUST default to JSON format diff --git a/openspec/changes/archive/2026-03-22-enhanced-audit-trail/tasks.md b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/tasks.md new file mode 100644 index 000000000..4808f7957 --- /dev/null +++ b/openspec/changes/archive/2026-03-22-enhanced-audit-trail/tasks.md @@ -0,0 +1,59 @@ +## 1. Database Migration + +- [x] 1.1 Create migration `Version1Date20260322100000.php` adding `hash` (VARCHAR 64, nullable) and `previous_hash` (VARCHAR 64, nullable) columns to `openregister_audit_trails` table +- [x] 1.2 Add index on `hash` column and index on `processing_activity_id` column in the same migration + +## 2. AuditTrail Entity Update + +- [x] 2.1 Add `hash` and `previousHash` properties to `AuditTrail.php` entity with `addType` calls in constructor +- [x] 2.2 Add `hash` and `previousHash` to `jsonSerialize()` output and update `@method` annotations +- [x] 2.3 Update `@psalm-return` type annotation to include the new fields + +## 3. AuditHashService + +- [x] 3.1 Create `lib/Service/AuditHashService.php` with `computeHash(AuditTrail $entry, string $previousHash): string` method using SHA-256 +- [x] 3.2 Implement `getCanonicalJson(AuditTrail $entry): string` that serializes entry data excluding `hash` and `previousHash` with sorted keys +- [x] 3.3 Implement `getGenesisHash(): string` returning `SHA-256("openregister-genesis-v1")` +- [x] 3.4 Implement `getLastHash(): string` that queries the most recent audit trail entry's hash, returning genesis hash if none exist +- [x] 3.5 Implement `verifyChain(?int $from, ?int $to): array` that iterates entries and validates hash chain integrity + +## 4. AuditTrailMapper Integration + +- [x] 4.1 Override `insert()` in `AuditTrailMapper` to call `AuditHashService` for hash computation before persisting +- [x] 4.2 Wrap hash chain write in a database transaction with row locking to prevent race conditions + +## 5. Immutability Enforcement + +- [x] 5.1 Add `update()` method to `AuditTrailController` returning HTTP 405 with error message +- [x] 5.2 Add `destroy()` method to `AuditTrailController` returning HTTP 405 with error message +- [x] 5.3 Register PUT/PATCH/DELETE routes for `/api/audit-trails/{id}` pointing to the 405 handlers + +## 6. Verification Endpoint + +- [x] 6.1 Add `verify()` action to `AuditTrailController` accepting optional `from` and `to` query parameters +- [x] 6.2 Register GET route `/api/audit-trails/verify` in `routes.php` +- [x] 6.3 Return JSON response with `valid`, `entriesVerified`, `brokenAt` (if invalid), and `skippedNullHashes` fields + +## 7. Verwerkingsregister API + +- [x] 7.1 Add `verwerkingsregister()` action to `AuditTrailController` that queries distinct processing activities with counts and date ranges +- [x] 7.2 Add `inzageverzoek()` action that searches audit entries by identifier in the `changed` JSON field, grouped by schema +- [x] 7.3 Register GET routes `/api/audit-trails/verwerkingsregister` and `/api/audit-trails/inzageverzoek` in `routes.php` + +## 8. Export Endpoint + +- [x] 8.1 Add `export()` action to `AuditTrailController` supporting JSON and CSV formats with date range filtering +- [x] 8.2 Register GET route `/api/audit-trails/export` in `routes.php` +- [x] 8.3 Implement CSV serialization with headers and JSON-stringified `changed` field + +## 9. Tests + +- [x] 9.1 Write unit tests for `AuditHashService`: hash computation, genesis hash, canonical JSON, chain verification +- [x] 9.2 Write unit tests for immutability enforcement (405 responses on update/delete) +- [x] 9.3 Write unit tests for verwerkingsregister and inzageverzoek endpoints +- [x] 9.4 Write unit test for export endpoint (JSON and CSV formats) + +## 10. Quality and Regression + +- [x] 10.1 Run `composer check:strict` and fix any PHPCS, PHPMD, Psalm, or PHPStan issues +- [x] 10.2 Verify opencatalogi and softwarecatalog still function correctly with the updated AuditTrail entity diff --git a/openspec/specs/audit-hash-chain/spec.md b/openspec/specs/audit-hash-chain/spec.md new file mode 100644 index 000000000..e89ba4e05 --- /dev/null +++ b/openspec/specs/audit-hash-chain/spec.md @@ -0,0 +1,69 @@ +# audit-hash-chain Specification + +--- +status: implemented +--- + +## Purpose +Cryptographic SHA-256 hash chaining on audit trail entries with genesis hash, verification endpoint, and tamper detection reporting. Each entry's hash chains to the previous entry, making any tampering detectable by auditors. + +## Requirements + +### Requirement: Every audit trail entry MUST include a SHA-256 hash chained to the previous entry +Each audit trail entry MUST contain a `hash` field computed as `SHA-256(previous_hash + canonical_json(entry_data))`. The `previous_hash` field links to the preceding entry's hash, forming a tamper-evident chain. + +#### Scenario: First audit entry uses genesis hash +- **WHEN** the first audit trail entry is created in the system (no previous entries exist) +- **THEN** the entry MUST have `previousHash` set to `SHA-256("openregister-genesis-v1")` +- **AND** the entry MUST have `hash` set to `SHA-256(genesis_hash + canonical_json(entry_data))` + +#### Scenario: Subsequent entries chain to previous hash +- **WHEN** audit trail entry N is created after entry N-1 with hash `abc123...` +- **THEN** entry N MUST have `previousHash` set to `abc123...` +- **AND** entry N MUST have `hash` set to `SHA-256("abc123..." + canonical_json(entry_data_N))` + +#### Scenario: Canonical JSON excludes hash fields +- **WHEN** computing the hash for an audit trail entry +- **THEN** the canonical JSON MUST include all entry fields except `hash` and `previousHash` +- **AND** the JSON MUST use sorted keys and no whitespace (compact canonical form) + +### Requirement: The system MUST provide a hash chain verification endpoint +An API endpoint MUST allow auditors to verify the integrity of the hash chain. + +#### Scenario: Verify full chain integrity +- **WHEN** a GET request is made to `/api/audit-trails/verify` +- **THEN** the system MUST iterate all audit trail entries in order +- **AND** recompute each hash and compare to the stored hash +- **AND** return a JSON response with `{"valid": true, "entriesVerified": }` + +#### Scenario: Verify chain with range parameters +- **WHEN** a GET request is made to `/api/audit-trails/verify?from=100&to=200` +- **THEN** the system MUST verify only entries with IDs between 100 and 200 (inclusive) +- **AND** return `{"valid": true, "entriesVerified": 101, "range": {"from": 100, "to": 200}}` + +#### Scenario: Detect tampered entry in chain +- **WHEN** an entry in the chain has been modified after creation (stored hash does not match recomputed hash) +- **THEN** the verification endpoint MUST return `{"valid": false, "brokenAt": , "entriesVerified": }` +- **AND** the `brokenAt` field MUST identify the first entry where the chain breaks + +#### Scenario: Handle entries without hashes (pre-migration) +- **WHEN** the verification encounters entries with null `hash` values (created before hash chaining was enabled) +- **THEN** those entries MUST be skipped in the verification +- **AND** the response MUST include `"skippedNullHashes": ` + +### Requirement: Hash chain writes MUST be serialized to prevent race conditions +Concurrent audit trail inserts MUST NOT produce broken hash chains. + +#### Scenario: Two simultaneous audit writes +- **WHEN** two audit trail entries are created at the same moment +- **THEN** both entries MUST be correctly chained (each referencing the correct previous hash) +- **AND** no two entries MUST share the same `previousHash` value (except the genesis hash for the first entry) + +### Requirement: A database migration MUST add hash columns +The migration MUST add `hash` and `previous_hash` columns to the audit trails table. + +#### Scenario: Migration adds nullable hash columns +- **WHEN** the migration runs on an existing database with audit trail entries +- **THEN** columns `hash` (VARCHAR 64) and `previous_hash` (VARCHAR 64) MUST be added +- **AND** existing entries MUST retain null values for both columns +- **AND** an index MUST be created on the `hash` column for verification queries diff --git a/openspec/specs/audit-trail-immutable/spec.md b/openspec/specs/audit-trail-immutable/spec.md index 5faf844a3..3cce07803 100644 --- a/openspec/specs/audit-trail-immutable/spec.md +++ b/openspec/specs/audit-trail-immutable/spec.md @@ -40,6 +40,18 @@ All create, update, and delete operations on register objects MUST generate an a - `action`: `delete` - `data`: full snapshot of the object before deletion +### Requirement: The AuditTrail entity MUST include hash and previousHash fields +The `AuditTrail` entity MUST be extended with `hash` and `previousHash` string fields for cryptographic chain integrity. + +#### Scenario: New audit trail entry includes hash fields in JSON +- **WHEN** an audit trail entry with hash chaining is serialized to JSON +- **THEN** the JSON output MUST include `hash` and `previousHash` string fields +- **AND** both fields MUST be 64-character hexadecimal strings (SHA-256 output) + +#### Scenario: Legacy entry without hash fields +- **WHEN** an audit trail entry created before hash chaining is serialized to JSON +- **THEN** the JSON output MUST include `hash` and `previousHash` as null values + ### Requirement: The audit trail MUST use cryptographic hash chaining Each audit trail entry MUST include a hash that chains to the previous entry, making any tampering detectable. @@ -59,13 +71,20 @@ Each audit trail entry MUST include a hash that chains to the previous entry, ma No user, including administrators, MUST be able to modify or delete audit trail entries through the application. #### Scenario: Reject audit trail deletion via API -- GIVEN an admin user attempts to DELETE /api/audit-trail/{id} +- GIVEN an admin user attempts to DELETE `/api/audit-trails/{id}` - THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be deleted"}` - AND the audit entry MUST remain unchanged -#### Scenario: Reject audit trail modification -- GIVEN an admin attempts to PUT /api/audit-trail/{id} with modified data +#### Scenario: Reject audit trail modification via PUT +- GIVEN an admin attempts to PUT `/api/audit-trails/{id}` with modified data - THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be modified"}` + +#### Scenario: Reject audit trail modification via PATCH +- GIVEN an admin attempts to PATCH `/api/audit-trails/{id}` with modified data +- THEN the system MUST return HTTP 405 Method Not Allowed +- AND the response MUST include `{"error": "Audit trail entries cannot be modified"}` ### Requirement: The audit trail MUST support minimum 10-year retention Audit trail entries MUST be retained for at least 10 years, with configurable retention periods per register. @@ -103,22 +122,22 @@ Read operations on schemas marked as containing sensitive data MUST also produce ### Current Implementation Status - **Implemented:** - - `AuditTrail` entity (`lib/Db/AuditTrail.php`) with fields: uuid, schema, register, object, objectUuid, registerUuid, schemaUuid, action, changed, user, userName, created, organisation, session, request, ipAddress, size + - `AuditTrail` entity (`lib/Db/AuditTrail.php`) with fields: uuid, schema, register, object, objectUuid, registerUuid, schemaUuid, action, changed, user, userName, created, organisation, session, request, ipAddress, size, hash, previousHash - `AuditTrailMapper` (`lib/Db/AuditTrailMapper.php`) with `createAuditTrail()` method recording create/update/delete actions with user context, session, IP address, and changed fields - `AuditHandler` (`lib/Service/Object/AuditHandler.php`) orchestrating audit trail creation during object operations - Referential integrity actions logged with specific action types: `referential_integrity.cascade_delete`, `referential_integrity.set_null`, `referential_integrity.set_default`, `referential_integrity.restrict_blocked` (in `ReferentialIntegrityService`) - `RevertHandler` (`lib/Service/Object/RevertHandler.php`) uses audit trail for object reversion - AuditTrail controller for listing/viewing entries + - Cryptographic hash chaining: `AuditHashService` computes SHA-256 hashes, `AuditTrailMapper.insert()` chains hashes automatically + - Immutability enforcement: PUT/DELETE on audit trail API endpoints return HTTP 405 + - Hash chain verification endpoint: `GET /api/audit-trails/verify` + - Export functionality: `GET /api/audit-trails/export` (JSON/CSV) - **NOT implemented:** - - Cryptographic hash chaining (no `hash` field exists on AuditTrail entity; no SHA-256 chain, no genesis hash) - - Immutability enforcement (no explicit blocking of DELETE/PUT on audit trail API endpoints) - 10-year retention configuration (no retention period settings per register) - Archive mechanism for old entries (no partitioning or separate archive table) - - Export functionality for compliance audits (no date-range export with hash verification) - Sensitive data read auditing (no `read` action logging; only mutations are recorded) - - Hash chain verification endpoint - **Partial:** - - The existing AuditTrail records most of the required metadata (user, timestamp, action, changed fields) but lacks hash chaining and immutability guarantees + - The existing AuditTrail records most of the required metadata including hash chaining and immutability guarantees ### Standards & References - **GDPR Article 30** — Processing records requirement @@ -129,19 +148,9 @@ Read operations on schemas marked as containing sensitive data MUST also produce - **W3C PROV-O** — Provenance ontology (for audit trail semantics) - **Common Criteria (ISO 15408)** — Security audit logging requirements -### Specificity Assessment -- The spec is well-defined for the hash chaining mechanism and CRUD audit scenarios. -- Missing: database migration details for adding the `hash` column; performance impact analysis of hash computation on every write; API endpoint definitions for verification and export. -- Ambiguous: whether "immutability" means application-level enforcement only or requires database-level constraints (e.g., triggers preventing UPDATE/DELETE on the audit trail table). -- Open questions: - - What is the genesis hash value? Should it be configurable or hardcoded? - - How should hash chain breaks be reported (admin notification, API endpoint, dashboard widget)? - - For sensitive data read auditing, what defines "sensitive" — a schema-level flag, or per-property marking? - - Should the archive mechanism use database partitioning, separate tables, or external storage? - ## Nextcloud Integration Analysis -- **Status**: Already implemented in OpenRegister -- **Existing Implementation**: `AuditTrail` entity with comprehensive fields (uuid, schema, register, object, action, changed, user, userName, session, request, ipAddress, size). `AuditTrailMapper` with `createAuditTrail()` recording all mutations. `AuditHandler` orchestrates audit trail creation. `AuditTrailController` for listing/viewing/exporting entries. `RevertHandler` uses audit trail for object reversion. Referential integrity actions logged with specific action types. -- **Nextcloud Core Integration**: The `AuditTrail` entity extends NC's `Entity` base class, `AuditTrailMapper` extends `QBMapper`. Events fired via `IEventDispatcher`. Should implement `IProvider` for NC's Activity app stream to surface audit entries in the NC activity feed. Consider integrating with NC's `ILogger` for system-level audit logging. Export functionality could leverage NC's file download infrastructure. -- **Recommendation**: Mark as implemented. Consider implementing `IProvider` for the Activity app to surface audit entries in NC's activity stream. Hash chaining, immutability enforcement, and 10-year retention are documented as not-yet-implemented enhancements. +- **Status**: Implemented in OpenRegister +- **Existing Implementation**: `AuditTrail` entity with comprehensive fields including hash and previousHash for chain integrity. `AuditTrailMapper` with `createAuditTrail()` recording all mutations and automatic hash chain computation on insert. `AuditHashService` for SHA-256 hash computation and chain verification. `AuditHandler` orchestrates audit trail creation. `AuditTrailController` for listing/viewing/exporting/verification/verwerkingsregister/inzageverzoek. `RevertHandler` uses audit trail for object reversion. Referential integrity actions logged with specific action types. +- **Nextcloud Core Integration**: The `AuditTrail` entity extends NC's `Entity` base class, `AuditTrailMapper` extends `QBMapper`. Events fired via `IEventDispatcher`. Should implement `IProvider` for NC's Activity app stream to surface audit entries in the NC activity feed. Consider integrating with NC's `ILogger` for system-level audit logging. +- **Recommendation**: Mark as implemented. Consider implementing `IProvider` for the Activity app to surface audit entries in NC's activity stream. 10-year retention and sensitive data read auditing are documented as not-yet-implemented enhancements. diff --git a/openspec/specs/verwerkingsregister-api/spec.md b/openspec/specs/verwerkingsregister-api/spec.md new file mode 100644 index 000000000..942cceafe --- /dev/null +++ b/openspec/specs/verwerkingsregister-api/spec.md @@ -0,0 +1,67 @@ +# verwerkingsregister-api Specification + +--- +status: implemented +--- + +## Purpose +GDPR Art 30 processing register API for querying processing activities, generating data subject access reports (inzageverzoek), and exporting the verwerkingsregister. Enables compliance auditing for Dutch government organisations. + +## Requirements + +### Requirement: The system MUST provide a verwerkingsregister (processing register) API +A dedicated API endpoint MUST return an overview of all processing activities recorded in the audit trail, grouped by processing activity ID. + +#### Scenario: List all processing activities +- **WHEN** a GET request is made to `/api/audit-trails/verwerkingsregister` +- **THEN** the system MUST return a JSON array of distinct processing activities +- **AND** each entry MUST include `processingActivityId`, `processingActivityUrl`, `organisationId`, `organisationIdType`, `confidentiality`, and `retentionPeriod` +- **AND** each entry MUST include `entryCount` (number of audit entries for this activity) +- **AND** each entry MUST include `firstSeen` and `lastSeen` timestamps + +#### Scenario: Filter verwerkingsregister by organisation +- **WHEN** a GET request is made to `/api/audit-trails/verwerkingsregister?organisationId=00000001234567890000` +- **THEN** the system MUST return only processing activities for that organisation + +#### Scenario: Empty verwerkingsregister +- **WHEN** no audit trail entries have a `processingActivityId` set +- **THEN** the endpoint MUST return an empty JSON array `[]` + +### Requirement: The system MUST support data subject access requests (inzageverzoek) +An API endpoint MUST allow querying all audit trail entries related to a specific data subject, identified by a search term in the `changed` field. + +#### Scenario: Query audit entries for a data subject +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek?identifier=123456789` +- **THEN** the system MUST search all audit trail entries where the `changed` JSON field contains the identifier +- **AND** return a JSON response with all matching entries grouped by schema +- **AND** each group MUST include the schema UUID, schema name (if available), and the list of matching entries + +#### Scenario: Inzageverzoek with no matching entries +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek?identifier=nonexistent` +- **THEN** the system MUST return `{"results": [], "totalEntries": 0}` + +#### Scenario: Inzageverzoek requires identifier parameter +- **WHEN** a GET request is made to `/api/audit-trails/inzageverzoek` without an `identifier` parameter +- **THEN** the system MUST return HTTP 400 with `{"error": "identifier parameter is required"}` + +### Requirement: The system MUST support audit trail export +An API endpoint MUST allow exporting audit trail entries in JSON or CSV format for external compliance auditing. + +#### Scenario: Export audit trail as JSON +- **WHEN** a GET request is made to `/api/audit-trails/export?format=json` +- **THEN** the system MUST return all audit trail entries as a JSON array +- **AND** the response MUST include Content-Disposition header for file download + +#### Scenario: Export audit trail as CSV +- **WHEN** a GET request is made to `/api/audit-trails/export?format=csv` +- **THEN** the system MUST return all audit trail entries as CSV +- **AND** the first row MUST contain column headers +- **AND** the `changed` field MUST be serialized as a JSON string within the CSV cell + +#### Scenario: Export with date range filter +- **WHEN** a GET request is made to `/api/audit-trails/export?format=json&from=2025-01-01&to=2025-12-31` +- **THEN** the system MUST return only entries with `created` timestamps within the specified range + +#### Scenario: Export defaults to JSON +- **WHEN** a GET request is made to `/api/audit-trails/export` without a `format` parameter +- **THEN** the system MUST default to JSON format diff --git a/tests/Unit/Controller/AuditTrailControllerTest.php b/tests/Unit/Controller/AuditTrailControllerTest.php index fab127aa2..3fa9df69f 100644 --- a/tests/Unit/Controller/AuditTrailControllerTest.php +++ b/tests/Unit/Controller/AuditTrailControllerTest.php @@ -6,8 +6,10 @@ use OCA\OpenRegister\Controller\AuditTrailController; use OCA\OpenRegister\Db\AuditTrailMapper; +use OCA\OpenRegister\Service\AuditHashService; use OCA\OpenRegister\Service\LogService; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use PHPUnit\Framework\MockObject\MockObject; @@ -19,20 +21,23 @@ class AuditTrailControllerTest extends TestCase private IRequest&MockObject $request; private LogService&MockObject $logService; private AuditTrailMapper&MockObject $auditTrailMapper; + private AuditHashService&MockObject $auditHashService; protected function setUp(): void { parent::setUp(); - $this->request = $this->createMock(IRequest::class); - $this->logService = $this->createMock(LogService::class); + $this->request = $this->createMock(IRequest::class); + $this->logService = $this->createMock(LogService::class); $this->auditTrailMapper = $this->createMock(AuditTrailMapper::class); + $this->auditHashService = $this->createMock(AuditHashService::class); $this->controller = new AuditTrailController( 'openregister', $this->request, $this->logService, - $this->auditTrailMapper + $this->auditTrailMapper, + $this->auditHashService ); } @@ -168,111 +173,191 @@ public function testExportGeneralException(): void $this->assertEquals(500, $result->getStatus()); } - public function testDestroySuccess(): void + // ── Immutability enforcement tests ── + + public function testUpdateReturns405(): void { - $this->logService->method('deleteLog')->willReturn(true); + $result = $this->controller->update(1); + $this->assertEquals(Http::STATUS_METHOD_NOT_ALLOWED, $result->getStatus()); + $data = $result->getData(); + $this->assertEquals('Audit trail entries cannot be modified', $data['error']); + } + + public function testDestroyReturns405(): void + { $result = $this->controller->destroy(1); - $this->assertEquals(200, $result->getStatus()); + $this->assertEquals(Http::STATUS_METHOD_NOT_ALLOWED, $result->getStatus()); $data = $result->getData(); - $this->assertTrue($data['success']); + $this->assertEquals('Audit trail entries cannot be deleted', $data['error']); } - public function testDestroyReturnsFalse(): void + // ── Verification endpoint tests ── + + public function testVerifySuccess(): void { - $this->logService->method('deleteLog')->willReturn(false); + $this->request->method('getParam') + ->willReturnMap([ + ['from', null, null], + ['to', null, null], + ]); - $result = $this->controller->destroy(1); + $this->auditHashService->method('verifyChain')->willReturn([ + 'valid' => true, + 'entriesVerified' => 50, + 'brokenAt' => null, + 'skippedNullHashes' => 0, + ]); - $this->assertEquals(500, $result->getStatus()); + $result = $this->controller->verify(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertTrue($data['valid']); + $this->assertEquals(50, $data['entriesVerified']); } - public function testDestroyNotFound(): void + public function testVerifyWithRange(): void { - $this->logService->method('deleteLog') - ->willThrowException(new DoesNotExistException('Not found')); + $this->request->method('getParam') + ->willReturnMap([ + ['from', null, '10'], + ['to', null, '20'], + ]); - $result = $this->controller->destroy(999); + $this->auditHashService->method('verifyChain')->willReturn([ + 'valid' => true, + 'entriesVerified' => 11, + 'brokenAt' => null, + 'skippedNullHashes' => 0, + 'range' => ['from' => 10, 'to' => 20], + ]); - $this->assertEquals(404, $result->getStatus()); + $result = $this->controller->verify(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertTrue($data['valid']); } - public function testDestroyMultipleSuccess(): void + public function testVerifyDetectsTamper(): void { - $this->request->method('getParams')->willReturn([]); $this->request->method('getParam') ->willReturnMap([ - ['ids', null, '1,2,3'], + ['from', null, null], + ['to', null, null], ]); - $this->logService->method('deleteLogs')->willReturn([ - 'deleted' => 3, - 'failed' => 0, + $this->auditHashService->method('verifyChain')->willReturn([ + 'valid' => false, + 'entriesVerified' => 49, + 'brokenAt' => 50, + 'skippedNullHashes' => 0, ]); - $result = $this->controller->destroyMultiple(); + $result = $this->controller->verify(); $this->assertEquals(200, $result->getStatus()); $data = $result->getData(); - $this->assertTrue($data['success']); + $this->assertFalse($data['valid']); + $this->assertEquals(50, $data['brokenAt']); } - public function testDestroyMultipleException(): void + // ── Verwerkingsregister tests ── + + public function testVerwerkingsregisterSuccess(): void { - $this->request->method('getParams')->willReturn([]); $this->request->method('getParam') ->willReturnMap([ - ['ids', null, null], + ['organisationId', null, null], ]); - $this->logService->method('deleteLogs') - ->willThrowException(new \Exception('Deletion failed')); + $activities = [ + [ + 'processingActivityId' => 'pa-001', + 'entryCount' => 10, + 'firstSeen' => '2025-01-01', + 'lastSeen' => '2025-12-31', + ], + ]; - $result = $this->controller->destroyMultiple(); + $this->auditTrailMapper->method('getProcessingActivities')->willReturn($activities); - $this->assertEquals(500, $result->getStatus()); + $result = $this->controller->verwerkingsregister(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertCount(1, $data); + $this->assertEquals('pa-001', $data[0]['processingActivityId']); } - public function testClearAllSuccess(): void + public function testVerwerkingsregisterEmpty(): void { - $this->auditTrailMapper->method('clearAllLogs')->willReturn(true); + $this->request->method('getParam') + ->willReturnMap([ + ['organisationId', null, null], + ]); - $result = $this->controller->clearAll(); + $this->auditTrailMapper->method('getProcessingActivities')->willReturn([]); + + $result = $this->controller->verwerkingsregister(); $this->assertEquals(200, $result->getStatus()); - $data = $result->getData(); - $this->assertTrue($data['success']); + $this->assertSame([], $result->getData()); } - public function testClearAllNoExpired(): void + // ── Inzageverzoek tests ── + + public function testInzageverzoekSuccess(): void { - $this->auditTrailMapper->method('clearAllLogs')->willReturn(false); + $this->request->method('getParam') + ->willReturnMap([ + ['identifier', null, '123456789'], + ]); - $result = $this->controller->clearAll(); + $this->auditTrailMapper->method('findByIdentifier')->willReturn([ + 'results' => [['schemaUuid' => 'schema-1', 'entries' => []]], + 'totalEntries' => 5, + ]); + + $result = $this->controller->inzageverzoek(); $this->assertEquals(200, $result->getStatus()); $data = $result->getData(); - $this->assertTrue($data['success']); + $this->assertEquals(5, $data['totalEntries']); } - public function testClearAllException(): void + public function testInzageverzoekMissingIdentifier(): void { - $this->auditTrailMapper->method('clearAllLogs') - ->willThrowException(new \Exception('Clear failed')); + $this->request->method('getParam') + ->willReturnMap([ + ['identifier', null, null], + ]); - $result = $this->controller->clearAll(); + $result = $this->controller->inzageverzoek(); - $this->assertEquals(500, $result->getStatus()); + $this->assertEquals(400, $result->getStatus()); $data = $result->getData(); - $this->assertFalse($data['success']); + $this->assertEquals('identifier parameter is required', $data['error']); + } + + public function testInzageverzoekEmptyIdentifier(): void + { + $this->request->method('getParam') + ->willReturnMap([ + ['identifier', null, ''], + ]); + + $result = $this->controller->inzageverzoek(); + + $this->assertEquals(400, $result->getStatus()); } // ── extractRequestParameters branch coverage ── public function testIndexWithUnderscoreLimitAndOffset(): void { - // Covers the _limit / _offset / _page alternate param names. $this->request->method('getParams')->willReturn([ '_limit' => '5', '_offset' => '10', @@ -290,7 +375,6 @@ public function testIndexWithUnderscoreLimitAndOffset(): void public function testIndexWithPageCalculatesOffset(): void { - // page=3, limit=20 → offset = (3-1)*20 = 40. $this->request->method('getParams')->willReturn([ 'page' => '3', ]); @@ -307,7 +391,6 @@ public function testIndexWithPageCalculatesOffset(): void public function testIndexWithUnderscorePageParam(): void { - // _page alternate name. $this->request->method('getParams')->willReturn([ '_page' => '2', ]); @@ -323,7 +406,6 @@ public function testIndexWithUnderscorePageParam(): void public function testIndexWithSortParam(): void { - // Covers the sort extraction branch. $this->request->method('getParams')->willReturn([ 'sort' => 'updated', 'order' => 'ASC', @@ -338,7 +420,6 @@ public function testIndexWithSortParam(): void public function testIndexWithUnderscoreSortParam(): void { - // Covers the _sort / _order alternate names. $this->request->method('getParams')->willReturn([ '_sort' => 'action', '_order' => 'DESC', @@ -353,7 +434,6 @@ public function testIndexWithUnderscoreSortParam(): void public function testIndexWithSearchParam(): void { - // Covers _search alternate name. $this->request->method('getParams')->willReturn([ '_search' => 'create', ]); @@ -365,22 +445,78 @@ public function testIndexWithSearchParam(): void $this->assertEquals(200, $result->getStatus()); } - public function testDestroyGeneralException(): void + public function testDestroyMultipleSuccess(): void { - // Covers the generic \Exception branch in destroy() (lines 378-384). - $this->logService->method('deleteLog') - ->willThrowException(new \Exception('Unexpected error')); + $this->request->method('getParams')->willReturn([]); + $this->request->method('getParam') + ->willReturnMap([ + ['ids', null, '1,2,3'], + ]); - $result = $this->controller->destroy(1); + $this->logService->method('deleteLogs')->willReturn([ + 'deleted' => 3, + 'failed' => 0, + ]); + + $result = $this->controller->destroyMultiple(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertTrue($data['success']); + } + + public function testDestroyMultipleException(): void + { + $this->request->method('getParams')->willReturn([]); + $this->request->method('getParam') + ->willReturnMap([ + ['ids', null, null], + ]); + + $this->logService->method('deleteLogs') + ->willThrowException(new \Exception('Deletion failed')); + + $result = $this->controller->destroyMultiple(); + + $this->assertEquals(500, $result->getStatus()); + } + + public function testClearAllSuccess(): void + { + $this->auditTrailMapper->method('clearAllLogs')->willReturn(true); + + $result = $this->controller->clearAll(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertTrue($data['success']); + } + + public function testClearAllNoExpired(): void + { + $this->auditTrailMapper->method('clearAllLogs')->willReturn(false); + + $result = $this->controller->clearAll(); + + $this->assertEquals(200, $result->getStatus()); + $data = $result->getData(); + $this->assertTrue($data['success']); + } + + public function testClearAllException(): void + { + $this->auditTrailMapper->method('clearAllLogs') + ->willThrowException(new \Exception('Clear failed')); + + $result = $this->controller->clearAll(); $this->assertEquals(500, $result->getStatus()); $data = $result->getData(); - $this->assertStringContainsString('Deletion failed', $data['error']); + $this->assertFalse($data['success']); } public function testDestroyMultipleWithArrayIds(): void { - // Covers the is_array($ids) branch in destroyMultiple() (lines 417-418). $this->request->method('getParams')->willReturn([]); $this->request->method('getParam') ->willReturnMap([ diff --git a/tests/Unit/Db/AuditTrailTest.php b/tests/Unit/Db/AuditTrailTest.php index 77da28f28..9a3e2acaf 100644 --- a/tests/Unit/Db/AuditTrailTest.php +++ b/tests/Unit/Db/AuditTrailTest.php @@ -44,6 +44,8 @@ public function testConstructorRegistersFieldTypes(): void $this->assertSame('string', $fieldTypes['retentionPeriod']); $this->assertSame('integer', $fieldTypes['size']); $this->assertSame('datetime', $fieldTypes['expires']); + $this->assertSame('string', $fieldTypes['hash']); + $this->assertSame('string', $fieldTypes['previousHash']); } public function testConstructorDefaultValues(): void @@ -175,6 +177,7 @@ public function testJsonSerialize(): void 'created', 'organisationId', 'organisationIdType', 'processingActivityId', 'processingActivityUrl', 'processingId', 'confidentiality', 'retentionPeriod', 'size', 'expires', + 'hash', 'previousHash', ]; foreach ($expectedKeys as $key) { $this->assertArrayHasKey($key, $json); @@ -213,4 +216,40 @@ public function testToStringFallback(): void { $this->assertSame('Audit Trail', (string)$this->auditTrail); } + + public function testSetAndGetHashFields(): void + { + $this->auditTrail->setHash('a1b2c3d4e5f6'); + $this->auditTrail->setPreviousHash('f6e5d4c3b2a1'); + + $this->assertSame('a1b2c3d4e5f6', $this->auditTrail->getHash()); + $this->assertSame('f6e5d4c3b2a1', $this->auditTrail->getPreviousHash()); + } + + public function testHashFieldsDefaultToNull(): void + { + $this->assertNull($this->auditTrail->getHash()); + $this->assertNull($this->auditTrail->getPreviousHash()); + } + + public function testJsonSerializeIncludesHashFields(): void + { + $this->auditTrail->setHash('abc123'); + $this->auditTrail->setPreviousHash('def456'); + + $json = $this->auditTrail->jsonSerialize(); + + $this->assertArrayHasKey('hash', $json); + $this->assertArrayHasKey('previousHash', $json); + $this->assertSame('abc123', $json['hash']); + $this->assertSame('def456', $json['previousHash']); + } + + public function testJsonSerializeHashFieldsNullWhenNotSet(): void + { + $json = $this->auditTrail->jsonSerialize(); + + $this->assertNull($json['hash']); + $this->assertNull($json['previousHash']); + } } diff --git a/tests/Unit/Service/AuditHashServiceTest.php b/tests/Unit/Service/AuditHashServiceTest.php new file mode 100644 index 000000000..f2c8c0fdf --- /dev/null +++ b/tests/Unit/Service/AuditHashServiceTest.php @@ -0,0 +1,173 @@ +db = $this->createMock(IDBConnection::class); + $this->service = new AuditHashService($this->db); + } + + public function testGetGenesisHash(): void + { + $expected = hash('sha256', 'openregister-genesis-v1'); + $result = $this->service->getGenesisHash(); + + $this->assertSame($expected, $result); + $this->assertSame(64, strlen($result)); + } + + public function testGetGenesisHashIsConsistent(): void + { + $first = $this->service->getGenesisHash(); + $second = $this->service->getGenesisHash(); + + $this->assertSame($first, $second); + } + + public function testGetCanonicalJsonExcludesHashFields(): void + { + $entry = new AuditTrail(); + $entry->setUuid('test-uuid'); + $entry->setAction('create'); + $entry->setHash('somehash'); + $entry->setPreviousHash('prevhash'); + + $json = $this->service->getCanonicalJson($entry); + $data = json_decode($json, true); + + $this->assertArrayNotHasKey('hash', $data); + $this->assertArrayNotHasKey('previousHash', $data); + $this->assertArrayHasKey('uuid', $data); + $this->assertArrayHasKey('action', $data); + } + + public function testGetCanonicalJsonHasSortedKeys(): void + { + $entry = new AuditTrail(); + $entry->setUuid('test-uuid'); + $entry->setAction('create'); + $entry->setUser('admin'); + + $json = $this->service->getCanonicalJson($entry); + $data = json_decode($json, true); + $keys = array_keys($data); + + $sorted = $keys; + sort($sorted); + + $this->assertSame($sorted, $keys); + } + + public function testComputeHashReturns64CharHex(): void + { + $entry = new AuditTrail(); + $entry->setUuid('test-uuid'); + $entry->setAction('create'); + + $previousHash = $this->service->getGenesisHash(); + $hash = $this->service->computeHash($entry, $previousHash); + + $this->assertSame(64, strlen($hash)); + $this->assertMatchesRegularExpression('/^[a-f0-9]{64}$/', $hash); + } + + public function testComputeHashIsDeterministic(): void + { + $entry = new AuditTrail(); + $entry->setUuid('test-uuid'); + $entry->setAction('create'); + + $previousHash = $this->service->getGenesisHash(); + $hash1 = $this->service->computeHash($entry, $previousHash); + $hash2 = $this->service->computeHash($entry, $previousHash); + + $this->assertSame($hash1, $hash2); + } + + public function testComputeHashDiffersWithDifferentPreviousHash(): void + { + $entry = new AuditTrail(); + $entry->setUuid('test-uuid'); + $entry->setAction('create'); + + $hash1 = $this->service->computeHash($entry, 'aaaa'); + $hash2 = $this->service->computeHash($entry, 'bbbb'); + + $this->assertNotSame($hash1, $hash2); + } + + public function testComputeHashDiffersWithDifferentEntryData(): void + { + $previousHash = $this->service->getGenesisHash(); + + $entry1 = new AuditTrail(); + $entry1->setUuid('uuid-1'); + $entry1->setAction('create'); + + $entry2 = new AuditTrail(); + $entry2->setUuid('uuid-2'); + $entry2->setAction('update'); + + $hash1 = $this->service->computeHash($entry1, $previousHash); + $hash2 = $this->service->computeHash($entry2, $previousHash); + + $this->assertNotSame($hash1, $hash2); + } + + public function testGetLastHashReturnsGenesisWhenEmpty(): void + { + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(\OCP\DB\IResult::class); + + $this->db->method('getQueryBuilder')->willReturn($qb); + $qb->method('select')->willReturnSelf(); + $qb->method('from')->willReturnSelf(); + $qb->method('orderBy')->willReturnSelf(); + $qb->method('setMaxResults')->willReturnSelf(); + $qb->method('executeQuery')->willReturn($result); + $result->method('fetch')->willReturn(false); + + $hash = $this->service->getLastHash(); + + $this->assertSame($this->service->getGenesisHash(), $hash); + } + + public function testGetLastHashReturnsStoredHash(): void + { + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(\OCP\DB\IResult::class); + + $this->db->method('getQueryBuilder')->willReturn($qb); + $qb->method('select')->willReturnSelf(); + $qb->method('from')->willReturnSelf(); + $qb->method('orderBy')->willReturnSelf(); + $qb->method('setMaxResults')->willReturnSelf(); + $qb->method('executeQuery')->willReturn($result); + $result->method('fetch')->willReturn(['hash' => 'abc123def456']); + + $hash = $this->service->getLastHash(); + + $this->assertSame('abc123def456', $hash); + } +}