diff --git a/lib/AuditLog/AuditLogger.php b/lib/AuditLog/AuditLogger.php new file mode 100644 index 000000000..a540305b1 --- /dev/null +++ b/lib/AuditLog/AuditLogger.php @@ -0,0 +1,54 @@ + $parameters named values for the placeholders of $message, in order, optionally followed by more context + */ + public function log(Operation $operation, string $message, array $parameters = []): void { + $arguments = [$message, $parameters, false]; + if (method_exists(CriticalActionPerformedEvent::class, 'getOperation')) { + $arguments[] = $operation->value; + } + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(...$arguments)); + } + + /** + * Logs a share with its share ID and form ID as the first parameters. + * The token of a link share is stored as its shareWith and is never logged. + * + * @param array $extraParameters + */ + public function logShare(Operation $operation, string $message, Share $share, array $extraParameters = []): void { + $this->log($operation, $message, [ + 'shareId' => $share->getId(), + 'formId' => $share->getFormId(), + 'shareType' => $share->getShareType(), + 'shareWith' => $share->getShareType() === IShare::TYPE_LINK ? null : $share->getShareWith(), + 'permissions' => $share->getPermissions(), + ] + $extraParameters); + } +} diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php new file mode 100644 index 000000000..d791f42f8 --- /dev/null +++ b/lib/AuditLog/Operation.php @@ -0,0 +1,58 @@ + 'A form was created, either empty or as a copy of another form', + self::FormDeleted => 'A form was deleted with its questions, shares, submissions and uploaded files', + self::FormOwnerTransferred => 'The ownership of a form was transferred to another user', + self::FormAccessUpdated => 'A form was opened to or closed for all users of the instance', + self::FormStateChanged => 'A form was closed, archived or reopened', + self::FormSettingsUpdated => 'The response settings of a form (anonymity, multiple or editable submissions, expiration, submission limit) were changed', + self::ResultsFileLinked => 'The results of a form were linked to a spreadsheet file, which is then kept in sync with the submissions', + self::ShareCreated => 'A form was shared with a user, group or team or by public link, with its permissions', + self::ShareUpdated => 'The permissions or the custom public link token of a form share were changed', + self::ShareDeleted => 'A form share was deleted, by someone managing the form or through the share review', + self::ResultsDownloaded => 'All submissions of a form were downloaded as a CSV, ODS or XLSX file', + self::ResultsExported => 'All submissions of a form were exported to a file in the cloud', + self::SubmissionUpdated => 'The answers of a submission were replaced', + self::SubmissionDeleted => 'A submission was deleted', + self::SubmissionsDeletedAll => 'All submissions of a form were deleted', + self::ConfigUpdated => 'An admin setting of Forms (sharing, form creation, confirmation emails, comments) was changed', + }; + } +} diff --git a/lib/Constants.php b/lib/Constants.php index 3a8f2e5fa..21bed641a 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -71,6 +71,15 @@ class Constants { public const FORM_STATE_CLOSED = 1; public const FORM_STATE_ARCHIVED = 2; + /** + * Names of the form states, used in the audit log + */ + public const FORM_STATE_NAMES = [ + self::FORM_STATE_ACTIVE => 'active', + self::FORM_STATE_CLOSED => 'closed', + self::FORM_STATE_ARCHIVED => 'archived', + ]; + /** * Access flags of a form */ diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index db27cc45b..85485d2b1 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -7,6 +7,8 @@ namespace OCA\Forms\Controller; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\BackgroundJob\DeleteQuestionFoldersJob; use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob; use OCA\Forms\Constants; @@ -92,6 +94,7 @@ public function __construct( private readonly IMimeTypeDetector $mimeTypeDetector, private readonly IJobList $jobList, private readonly ISecureRandom $secureRandom, + private readonly AuditLogger $auditLogger, ) { parent::__construct($appName, $request); $this->currentUser = $userSession->getUser(); @@ -178,6 +181,11 @@ public function newForm(?int $fromId = null): DataResponse { $form->setIsAnonymous(false); $this->formMapper->insert($form); + $this->auditLogger->log(Operation::FormCreated, 'Form %s was created by "%s"', [ + 'formId' => $form->getId(), + 'ownerId' => $form->getOwnerId(), + 'sourceFormId' => null, + ]); } else { $oldForm = $this->formsService->getFormIfAllowed($fromId, Constants::PERMISSION_EDIT); @@ -238,6 +246,11 @@ public function newForm(?int $fromId = null): DataResponse { } $this->formMapper->update($form); + $this->auditLogger->log(Operation::FormCreated, 'Form %s was created by "%s" as a copy of form %s', [ + 'formId' => $form->getId(), + 'ownerId' => $form->getOwnerId(), + 'sourceFormId' => $oldForm->getId(), + ]); } return new DataResponse($this->formsService->getForm($form), Http::STATUS_CREATED); @@ -323,11 +336,17 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse { // Do not allow changing showToAllUsers or permitAllUsers if disabled $this->checkAccessUpdate($keyValuePairs); + $previousAccess = $form->getAccess(); + $previousState = $form->getState(); + $previousSettings = $this->getResponseSettings($form); + // Process file linking + $fileLinked = false; if (isset($keyValuePairs['path']) && isset($keyValuePairs['fileFormat'])) { $file = $this->submissionService->writeFileToCloud($form, $keyValuePairs['path'], $keyValuePairs['fileFormat']); $form->setFileId($file->getId()); $form->setFileFormat($keyValuePairs['fileFormat']); + $fileLinked = true; } // Process file unlinking @@ -365,6 +384,8 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse { // Update changed Columns in Db. $this->formMapper->update($form); + $this->logFormUpdate($form, $previousAccess, $previousState, $previousSettings, $fileLinked); + return new DataResponse($form->getId()); } @@ -389,6 +410,10 @@ public function deleteForm(int $formId): DataResponse { $form = $this->formsService->getFormIfAllowed($formId); $this->formMapper->deleteForm($form); + $this->auditLogger->log(Operation::FormDeleted, 'Form %s of "%s" was deleted', [ + 'formId' => $form->getId(), + 'ownerId' => $form->getOwnerId(), + ]); return new DataResponse($formId); } @@ -1239,6 +1264,10 @@ public function getSubmissions(int $formId, ?string $query = null, ?int $limit = $submissionsData = $this->submissionService->getSubmissionsData($form, $fileFormat); $fileName = $this->formsService->getFileName($form, $fileFormat); + $this->auditLogger->log(Operation::ResultsDownloaded, 'Results of form %s were downloaded as %s', [ + 'formId' => $form->getId(), + 'fileFormat' => $fileFormat, + ]); return new DataDownloadResponse($submissionsData, $fileName, Constants::SUPPORTED_EXPORT_FORMATS[$fileFormat]); } @@ -1377,6 +1406,9 @@ public function deleteAllSubmissions(int $formId): DataResponse { // Delete all submissions (incl. Answers) $this->submissionMapper->deleteByForm($formId); $this->formMapper->update($form); + $this->auditLogger->log(Operation::SubmissionsDeletedAll, 'All submissions of form %s were deleted', [ + 'formId' => $form->getId(), + ]); $this->queueLinkedFileSync($form); @@ -1545,6 +1577,12 @@ public function updateSubmission(int $formId, int $submissionId, array $answers) $this->storeAnswersForQuestion($form, $submission->getId(), $question, $answerArray); } + $this->auditLogger->log(Operation::SubmissionUpdated, 'Submission %s of form %s by "%s" was updated', [ + 'submissionId' => $submissionId, + 'formId' => $form->getId(), + 'submissionUserId' => $submission->getUserId(), + ]); + //Create Activity $this->formsService->notifyNewSubmission($form, $submission); @@ -1597,6 +1635,11 @@ public function deleteSubmission(int $formId, int $submissionId): DataResponse { // Delete submission (incl. Answers) $this->submissionMapper->deleteById($form, $submissionId); $this->formMapper->update($form); + $this->auditLogger->log(Operation::SubmissionDeleted, 'Submission %s of form %s by "%s" was deleted', [ + 'submissionId' => $submissionId, + 'formId' => $form->getId(), + 'submissionUserId' => $submission->getUserId(), + ]); $this->queueLinkedFileSync($form); @@ -1630,6 +1673,12 @@ public function exportSubmissionsToCloud(int $formId, string $path, string $file } $file = $this->submissionService->writeFileToCloud($form, $path, $fileFormat); + $this->auditLogger->log(Operation::ResultsExported, 'Results of form %s were exported to file %s', [ + 'formId' => $form->getId(), + 'fileId' => $file->getId(), + 'fileName' => $file->getName(), + 'fileFormat' => $fileFormat, + ]); return new DataResponse($file->getName()); } @@ -2019,13 +2068,79 @@ private function handleOwnerTransfer(Form $form, int $formId, string $currentUse $this->logger->debug('Could not find new form owner'); throw new OCSBadRequestException('Could not find new form owner'); } + $previousOwnerId = $form->getOwnerId(); $form->setOwnerId($keyValuePairs['ownerId']); $form->setLockedBy(null); $form->setLockedUntil(null); $this->formMapper->update($form); + $this->auditLogger->log(Operation::FormOwnerTransferred, 'Ownership of form %s was transferred from "%s" to "%s"', [ + 'formId' => $form->getId(), + 'oldOwnerId' => $previousOwnerId, + 'newOwnerId' => $form->getOwnerId(), + ]); return new DataResponse($form->getOwnerId()); } + /** + * Settings that decide how responses are collected and handled + * + * @return array + */ + private function getResponseSettings(Form $form): array { + return [ + 'isAnonymous' => $form->getIsAnonymous(), + 'submitMultiple' => $form->getSubmitMultiple(), + 'allowEditSubmissions' => $form->getAllowEditSubmissions(), + 'expires' => $form->getExpires(), + 'maxSubmissions' => $form->getMaxSubmissions(), + ]; + } + + /** + * Only the names of changed settings are logged, never texts like the title, + * description or confirmation email + * + * @param array $previousSettings + */ + private function logFormUpdate(Form $form, array $previousAccess, ?int $previousState, array $previousSettings, bool $fileLinked): void { + $access = $form->getAccess(); + if ($access !== $previousAccess) { + $this->auditLogger->log(Operation::FormAccessUpdated, 'Access to form %s was changed', [ + 'formId' => $form->getId(), + 'permitAllUsers' => $access['permitAllUsers'], + 'showToAllUsers' => $access['showToAllUsers'], + ]); + } + + if ($form->getState() !== $previousState) { + $this->auditLogger->log(Operation::FormStateChanged, 'State of form %s was changed to %s', [ + 'formId' => $form->getId(), + 'state' => Constants::FORM_STATE_NAMES[$form->getState()] ?? $form->getState(), + 'previousState' => Constants::FORM_STATE_NAMES[$previousState] ?? $previousState, + ]); + } + + $changedKeys = array_keys(array_filter( + $this->getResponseSettings($form), + static fn (mixed $value, string $key): bool => $value !== $previousSettings[$key], + ARRAY_FILTER_USE_BOTH, + )); + if ($changedKeys !== []) { + $this->auditLogger->log(Operation::FormSettingsUpdated, 'Response settings of form %s were changed', [ + 'formId' => $form->getId(), + 'changedKeys' => $changedKeys, + ]); + } + + if ($fileLinked) { + $this->auditLogger->log(Operation::ResultsFileLinked, 'Results of form %s were linked to file %s', [ + 'formId' => $form->getId(), + 'fileId' => $form->getFileId(), + 'fileFormat' => $form->getFileFormat(), + ]); + } + } + /** * Shift existing question orders to make room for an insertion at $position. * Normalizes the given $position to the valid range and updates all questions diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 7eee3e403..e314db2d3 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -8,6 +8,8 @@ namespace OCA\Forms\Controller; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Constants; use OCA\Forms\Service\ConfigService; use OCP\AppFramework\ApiController; @@ -27,6 +29,7 @@ public function __construct( private readonly IAppConfig $appConfig, private readonly LoggerInterface $logger, IRequest $request, + private readonly AuditLogger $auditLogger, ) { parent::__construct($appName, $request); } @@ -83,6 +86,12 @@ public function updateAppConfig(string $configKey, mixed $configValue): DataResp return new DataResponse('Invalid value for ' . $configKey, Http::STATUS_BAD_REQUEST); } + // All settings are booleans, integers or group IDs + $this->auditLogger->log(Operation::ConfigUpdated, 'Forms setting %s was changed', [ + 'configKey' => $configKey, + 'configValue' => $configValue, + ]); + return new DataResponse(); } } diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index fde50da43..b1771c3fa 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -9,6 +9,8 @@ namespace OCA\Forms\Controller; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Constants; use OCA\Forms\Db\Form; use OCA\Forms\Db\FormMapper; @@ -64,6 +66,7 @@ public function __construct( private readonly FilePathHelper $filePathHelper, private readonly IManager $shareManager, private readonly UploadedFilesShareService $uploadedFilesShareService, + private readonly AuditLogger $auditLogger, ) { parent::__construct($appName, $request); } @@ -194,6 +197,7 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar /** @var Share */ $share = $this->shareMapper->insert($share); $this->formMapper->update($form); + $this->auditLogger->logShare(Operation::ShareCreated, 'Share %s of form %s was created', $share); // Create share-notifications (activity) $this->formsService->notifyNewShares($form, $share); @@ -270,6 +274,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da throw new OCSBadRequestException('Invalid permission given'); } + $previousShareWith = $formShare->getShareWith(); if (array_key_exists('token', $keyValuePairs)) { if (!$this->configService->getAllowCustomPublicToken()) { $this->logger->debug('Custom public share tokens are not allowed.'); @@ -341,6 +346,9 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da } $this->formMapper->update($form); + $this->auditLogger->logShare(Operation::ShareUpdated, 'Share %s of form %s was updated', $formShare, [ + 'tokenChanged' => $formShare->getShareType() === IShare::TYPE_LINK && $formShare->getShareWith() !== $previousShareWith, + ]); return new DataResponse($formShare->getId()); } @@ -393,6 +401,7 @@ public function deleteShare(int $formId, int $shareId): DataResponse { $this->shareMapper->delete($share); $this->formMapper->update($form); + $this->auditLogger->logShare(Operation::ShareDeleted, 'Share %s of form %s was deleted', $share); return new DataResponse($shareId); } diff --git a/lib/ShareReview/ShareReviewSource.php b/lib/ShareReview/ShareReviewSource.php index 40b1b8b4f..d4c9b91b7 100644 --- a/lib/ShareReview/ShareReviewSource.php +++ b/lib/ShareReview/ShareReviewSource.php @@ -9,6 +9,8 @@ namespace OCA\Forms\ShareReview; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Constants; use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\ShareMapper; @@ -43,6 +45,7 @@ public function __construct( private readonly IEventDispatcher $eventDispatcher, private readonly LoggerInterface $logger, private readonly IL10N $l10n, + private readonly AuditLogger $auditLogger, ) { } @@ -96,11 +99,13 @@ public function deleteShare(string $shareId): bool { $this->shareMapper->delete($share); // Bump the form's last_updated timestamp, matching the regular deletion flow $this->formMapper->update($form); - return true; } catch (\Exception $e) { $this->logger->error('Forms ShareReview: failed to delete share {id}: {message}', ['id' => $shareId, 'message' => $e->getMessage()]); return false; } + + $this->auditLogger->logShare(Operation::ShareDeleted, 'Share %s of form %s was deleted through the share review', $share); + return true; } /** @param array $share */ diff --git a/tests/Unit/AuditLog/AuditEvents.php b/tests/Unit/AuditLog/AuditEvents.php new file mode 100644 index 000000000..0a7faf12c --- /dev/null +++ b/tests/Unit/AuditLog/AuditEvents.php @@ -0,0 +1,20 @@ +getOperation() === $operation; + } +} diff --git a/tests/Unit/AuditLog/AuditLoggerTest.php b/tests/Unit/AuditLog/AuditLoggerTest.php new file mode 100644 index 000000000..00b5c39a6 --- /dev/null +++ b/tests/Unit/AuditLog/AuditLoggerTest.php @@ -0,0 +1,86 @@ +eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->auditLogger = new AuditLogger($this->eventDispatcher); + } + + private function expectAuditEvent(string $operation, string $message, array $parameters): void { + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event) use ($operation, $message, $parameters): bool { + $this->assertTrue(AuditEvents::hasOperation($event, $operation)); + $this->assertSame($message, $event->getLogMessage()); + $this->assertSame($parameters, $event->getParameters()); + $this->assertFalse($event->getObfuscateParameters()); + return true; + })); + } + + public function testLog(): void { + $this->expectAuditEvent('forms.form.deleted', 'Form %s of "%s" was deleted', ['formId' => 1, 'ownerId' => 'alice']); + + $this->auditLogger->log(Operation::FormDeleted, 'Form %s of "%s" was deleted', ['formId' => 1, 'ownerId' => 'alice']); + } + + public function testLogShare(): void { + $share = new Share(); + $share->setId(8); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_GROUP); + $share->setShareWith('staff'); + $share->setPermissions(['submit', 'results']); + + $this->expectAuditEvent('forms.share.updated', 'Share %s of form %s was updated', [ + 'shareId' => 8, + 'formId' => 5, + 'shareType' => IShare::TYPE_GROUP, + 'shareWith' => 'staff', + 'permissions' => ['submit', 'results'], + 'tokenChanged' => false, + ]); + + $this->auditLogger->logShare(Operation::ShareUpdated, 'Share %s of form %s was updated', $share, ['tokenChanged' => false]); + } + + public function testLogShareNeverLogsLinkShareToken(): void { + $share = new Share(); + $share->setId(9); + $share->setFormId(5); + $share->setShareType(IShare::TYPE_LINK); + $share->setShareWith('secretToken123'); + $share->setPermissions(['submit']); + + $this->expectAuditEvent('forms.share.created', 'Share %s of form %s was created', [ + 'shareId' => 9, + 'formId' => 5, + 'shareType' => IShare::TYPE_LINK, + 'shareWith' => null, + 'permissions' => ['submit'], + ]); + + $this->auditLogger->logShare(Operation::ShareCreated, 'Share %s of form %s was created', $share); + } +} diff --git a/tests/Unit/AuditLog/OperationTest.php b/tests/Unit/AuditLog/OperationTest.php new file mode 100644 index 000000000..1e0e474b8 --- /dev/null +++ b/tests/Unit/AuditLog/OperationTest.php @@ -0,0 +1,25 @@ + [$operation], Operation::cases()); + } + + #[DataProvider('dataOperations')] + public function testOperationFormat(Operation $operation): void { + $this->assertMatchesRegularExpression('/^forms\.[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*$/', $operation->value); + $this->assertNotSame('', $operation->description()); + } +} diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index c3c0bbde1..b67d6bdbf 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -29,6 +29,8 @@ function is_uploaded_file(string|bool|null $filename) { namespace OCA\Forms\Tests\Unit\Controller; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob; use OCA\Forms\Constants; use OCA\Forms\Controller\ApiController; @@ -47,6 +49,7 @@ function is_uploaded_file(string|bool|null $filename) { use OCA\Forms\Service\ConfirmationEmailService; use OCA\Forms\Service\FormsService; use OCA\Forms\Service\SubmissionService; +use OCA\Forms\Tests\Unit\AuditLog\AuditEvents; use OCA\Forms\Tests\Unit\UserFolderMockTrait; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; @@ -56,6 +59,7 @@ function is_uploaded_file(string|bool|null $filename) { use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\BackgroundJob\IJobList; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IMimeTypeDetector; @@ -66,6 +70,7 @@ function is_uploaded_file(string|bool|null $filename) { use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -93,6 +98,9 @@ class ApiControllerTest extends TestCase { private IMimeTypeDetector|MockObject $mimeTypeDetector; private IJobList|MockObject $jobList; private ISecureRandom|MockObject $secureRandom; + private IEventDispatcher|MockObject $eventDispatcher; + /** @var list */ + private array $dispatchedEvents = []; public function setUp(): void { $this->answerMapper = $this->createMock(AnswerMapper::class); @@ -117,6 +125,12 @@ public function setUp(): void { $this->jobList = $this->createMock(IJobList::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->secureRandom->method('generate')->willReturn('abcdefghijklmnopqrstuvwxyz012345'); + $this->dispatchedEvents = []; + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->eventDispatcher->method('dispatchTyped') + ->willReturnCallback(function (object $event): void { + $this->dispatchedEvents[] = $event; + }); $this->apiController = new ApiController( 'forms', @@ -139,7 +153,26 @@ public function setUp(): void { $this->mimeTypeDetector, $this->jobList, $this->secureRandom, + new AuditLogger($this->eventDispatcher), + ); + } + + private function expectNoAuditLog(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + } + + /** + * @param list}> $expected operation, message and parameters of each audit log entry, in order + */ + private function assertAuditLogged(array $expected): void { + $events = array_values(array_filter($this->dispatchedEvents, static fn (object $event): bool => $event instanceof CriticalActionPerformedEvent)); + $this->assertSame( + array_map(static fn (array $entry): array => [$entry[1], $entry[2]], $expected), + array_map(static fn (CriticalActionPerformedEvent $event): array => [$event->getLogMessage(), $event->getParameters()], $events), ); + foreach ($events as $i => $event) { + $this->assertTrue(AuditEvents::hasOperation($event, $expected[$i][0]->value)); + } } /** @@ -425,6 +458,7 @@ public function testExportSubmissions_noExportPermissions() { ->with($form) ->willReturn([Constants::PERMISSION_SUBMIT]); + $this->expectNoAuditLog(); $this->expectException(NoSuchFormException::class); $this->apiController->getSubmissions(1, fileFormat: 'csv'); } @@ -457,6 +491,9 @@ public function testExportSubmissions() { ->willReturn($fileName); $this->assertEquals(new DataDownloadResponse($csv, $fileName, 'text/csv'), $this->apiController->getSubmissions(1, fileFormat: 'csv')); + $this->assertAuditLogged([ + [Operation::ResultsDownloaded, 'Results of form %s were downloaded as %s', ['formId' => 1, 'fileFormat' => 'csv']], + ]); } public function testExportSubmissionsToCloud_invalidForm() { @@ -485,11 +522,45 @@ public function testExportSubmissionsToCloud_noExportPermissions() { $this->submissionService->expects($this->never()) ->method('writeFileToCloud'); + $this->expectNoAuditLog(); $this->expectException(OCSForbiddenException::class); $this->apiController->exportSubmissionsToCloud(1, '/', 'csv'); } + public function testExportSubmissionsToCloud(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('currentUser'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1, Constants::PERMISSION_RESULTS) + ->willReturn($form); + $this->formsService->expects($this->once()) + ->method('getPermissions') + ->with($form) + ->willReturn([Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS]); + + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(42); + $file->method('getName')->willReturn('Form 1 (responses).xlsx'); + $this->submissionService->expects($this->once()) + ->method('writeFileToCloud') + ->with($form, '/Exports', 'xlsx') + ->willReturn($file); + + $this->assertEquals(new DataResponse('Form 1 (responses).xlsx'), $this->apiController->exportSubmissionsToCloud(1, '/Exports', 'xlsx')); + $this->assertAuditLogged([ + [Operation::ResultsExported, 'Results of form %s were exported to file %s', [ + 'formId' => 1, + 'fileId' => 42, + 'fileName' => 'Form 1 (responses).xlsx', + 'fileFormat' => 'xlsx', + ]], + ]); + } + public function testCreateNewForm_notAllowed() { $this->configService->expects($this->once()) ->method('canCreateForms') @@ -556,6 +627,9 @@ public function testCreateNewForm($expectedForm) { return $form; }); $this->assertEquals(new DataResponse([], Http::STATUS_CREATED), $this->apiController->newForm()); + $this->assertAuditLogged([ + [Operation::FormCreated, 'Form %s was created by "%s"', ['formId' => 7, 'ownerId' => 'currentUser', 'sourceFormId' => null]], + ]); } public static function dataCloneForm_exceptions() { @@ -589,6 +663,7 @@ public function testCloneForm_exceptions(bool $canCreate, $callback, string $exc ->method('getFormIfAllowed') ->with(7, Constants::PERMISSION_EDIT) ->willReturnCallback($callback); + $this->expectNoAuditLog(); $this->expectException($exception); $this->apiController->newForm(7); } @@ -667,6 +742,9 @@ public function testCloneForm($old, $new) { ->willReturn([]); $this->assertEquals(new DataResponse([], Http::STATUS_CREATED), $this->apiController->newForm(7)); + $this->assertAuditLogged([ + [Operation::FormCreated, 'Form %s was created by "%s" as a copy of form %s', ['formId' => 14, 'ownerId' => 'currentUser', 'sourceFormId' => 7]], + ]); } private function formAccess(bool $hasUserAccess = true, bool $hasFormExpired = false, bool $canSubmit = true) { @@ -990,6 +1068,10 @@ public function testDeleteSubmissionNoPermission($submissionData, $formData) { ->with(1, Constants::PERMISSION_RESULTS_DELETE) ->willReturn($form); + $this->submissionMapper->expects($this->never()) + ->method('deleteById'); + $this->expectNoAuditLog(); + $this->expectException(OCSForbiddenException::class); $this->apiController->deleteSubmission(1, 42); } @@ -1027,13 +1109,18 @@ public function testDeleteSubmission($submissionData, $formData) { ->with(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => 1]); $this->assertEquals(new DataResponse(42), $this->apiController->deleteSubmission(1, 42)); + $this->assertAuditLogged([ + [Operation::SubmissionDeleted, 'Submission %s of form %s by "%s" was deleted', ['submissionId' => 42, 'formId' => 1, 'submissionUserId' => 'respondent']], + ]); } public static function dataTestDeletePermission() { return [ [ [ + 'id' => 42, 'formId' => 1, + 'userId' => 'respondent', ], [ 'id' => 1, @@ -1092,6 +1179,7 @@ public function testDeleteAllSubmissionsNoPermission(): void { $this->submissionMapper ->expects($this->never()) ->method('deleteByForm'); + $this->expectNoAuditLog(); $this->expectException(OCSForbiddenException::class); $this->apiController->deleteAllSubmissions(1); @@ -1138,6 +1226,9 @@ public function testDeleteAllSubmissionsAsOwner(): void { ->with(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => 1]); $this->assertEquals(new DataResponse(1), $this->apiController->deleteAllSubmissions(1)); + $this->assertAuditLogged([ + [Operation::SubmissionsDeletedAll, 'All submissions of form %s were deleted', ['formId' => 1]], + ]); } /** @@ -1180,6 +1271,9 @@ public function testDeleteAllSubmissionsWithResultsDeletePermission(): void { ->method('add'); $this->assertEquals(new DataResponse(1), $this->apiController->deleteAllSubmissions(1)); + $this->assertAuditLogged([ + [Operation::SubmissionsDeletedAll, 'All submissions of form %s were deleted', ['formId' => 1]], + ]); } public function testUpdateFormAllowsValidConfirmationEmailQuestionId(): void { @@ -1202,6 +1296,7 @@ public function testUpdateFormAllowsValidConfirmationEmailQuestionId(): void { $this->assertEquals(new DataResponse(1), $this->apiController->updateForm(1, ['confirmationEmailQuestionId' => 7])); $this->assertSame(7, $form->getConfirmationEmailQuestionId()); + $this->assertAuditLogged([]); } public static function dataUpdateFormRejectsForbiddenKeys(): array { @@ -1231,6 +1326,7 @@ public function testUpdateFormRejectsForbiddenKeys(array $keyValuePairs): void { ->willReturn($form); $this->formMapper->expects($this->never())->method('update'); + $this->expectNoAuditLog(); $this->expectException(OCSForbiddenException::class); $this->apiController->updateForm(1, $keyValuePairs); @@ -1403,6 +1499,7 @@ public function testTransferOwnerNotOwner() { ->with(1, Constants::PERMISSION_EDIT) ->willReturn($form); + $this->expectNoAuditLog(); $this->expectException(OCSForbiddenException::class); $this->apiController->updateForm(1, ['ownerId' => 'newOwner']); } @@ -1428,6 +1525,7 @@ public function testTransferNewOwnerNotFound() { ->with('newOwner') ->willReturn(null); + $this->expectNoAuditLog(); $this->expectException(OCSBadRequestException::class); $this->apiController->updateForm(1, ['ownerId' => 'newOwner']); } @@ -1456,6 +1554,143 @@ public function testTransferOwner() { $this->assertEquals(new DataResponse('newOwner'), $this->apiController->updateForm(1, ['ownerId' => 'newOwner'])); $this->assertEquals('newOwner', $form->getOwnerId()); + $this->assertAuditLogged([ + [Operation::FormOwnerTransferred, 'Ownership of form %s was transferred from "%s" to "%s"', ['formId' => 1, 'oldOwnerId' => 'currentUser', 'newOwnerId' => 'newOwner']], + ]); + } + + public function testDeleteForm(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('formOwner'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1) + ->willReturn($form); + $this->formMapper->expects($this->once()) + ->method('deleteForm') + ->with($form); + + $this->assertEquals(new DataResponse(1), $this->apiController->deleteForm(1)); + $this->assertAuditLogged([ + [Operation::FormDeleted, 'Form %s of "%s" was deleted', ['formId' => 1, 'ownerId' => 'formOwner']], + ]); + } + + public function testDeleteFormNotAllowed(): void { + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1) + ->willThrowException(new NoSuchFormException('This form is not owned by the current user', Http::STATUS_FORBIDDEN)); + $this->formMapper->expects($this->never()) + ->method('deleteForm'); + $this->expectNoAuditLog(); + + $this->expectException(NoSuchFormException::class); + $this->apiController->deleteForm(1); + } + + private function createUpdatableForm(): Form { + $form = new Form(); + $form->setId(1); + $form->setHash('hash'); + $form->setOwnerId('currentUser'); + $form->setState(Constants::FORM_STATE_ACTIVE); + $form->setAccess(['permitAllUsers' => false, 'showToAllUsers' => false]); + $form->setIsAnonymous(false); + $form->setSubmitMultiple(false); + $form->setAllowEditSubmissions(false); + $form->setExpires(0); + + $this->formsService + ->method('getFormIfAllowed') + ->with(1, Constants::PERMISSION_EDIT) + ->willReturn($form); + + return $form; + } + + public function testUpdateFormLogsAccessChange(): void { + $this->createUpdatableForm(); + $this->configService->method('getAllowPermitAll')->willReturn(true); + + $this->apiController->updateForm(1, ['access' => ['permitAllUsers' => true, 'showToAllUsers' => false]]); + + $this->assertAuditLogged([ + [Operation::FormAccessUpdated, 'Access to form %s was changed', ['formId' => 1, 'permitAllUsers' => true, 'showToAllUsers' => false]], + ]); + } + + public function testUpdateFormDoesNotLogUnchangedAccess(): void { + $this->createUpdatableForm(); + + $this->apiController->updateForm(1, ['access' => ['permitAllUsers' => false, 'showToAllUsers' => false]]); + + $this->assertAuditLogged([]); + } + + public function testUpdateFormNotAllowedAccessChangeIsNotLogged(): void { + $this->createUpdatableForm(); + $this->configService->method('getAllowShowToAll')->willReturn(false); + $this->formMapper->expects($this->never())->method('update'); + $this->expectNoAuditLog(); + + $this->expectException(OCSForbiddenException::class); + $this->apiController->updateForm(1, ['access' => ['permitAllUsers' => true, 'showToAllUsers' => true]]); + } + + public function testUpdateFormLogsStateChange(): void { + $this->createUpdatableForm(); + $this->formsService->method('isFormArchived')->willReturn(false); + + $this->apiController->updateForm(1, ['state' => Constants::FORM_STATE_ARCHIVED]); + + $this->assertAuditLogged([ + [Operation::FormStateChanged, 'State of form %s was changed to %s', ['formId' => 1, 'state' => 'archived', 'previousState' => 'active']], + ]); + } + + public function testUpdateFormLogsOnlyNamesOfChangedResponseSettings(): void { + $this->createUpdatableForm(); + + $this->apiController->updateForm(1, [ + 'title' => 'New title', + 'isAnonymous' => true, + 'expires' => 0, + 'maxSubmissions' => 5, + ]); + + $this->assertAuditLogged([ + [Operation::FormSettingsUpdated, 'Response settings of form %s were changed', ['formId' => 1, 'changedKeys' => ['isAnonymous', 'maxSubmissions']]], + ]); + } + + public function testUpdateFormLogsLinkedResultsFile(): void { + $form = $this->createUpdatableForm(); + $file = $this->createMock(File::class); + $file->method('getId')->willReturn(42); + $this->submissionService->expects($this->once()) + ->method('writeFileToCloud') + ->with($form, '/Results', 'csv') + ->willReturn($file); + + $this->apiController->updateForm(1, ['path' => '/Results', 'fileFormat' => 'csv']); + + $this->assertAuditLogged([ + [Operation::ResultsFileLinked, 'Results of form %s were linked to file %s', ['formId' => 1, 'fileId' => 42, 'fileFormat' => 'csv']], + ]); + } + + public function testUpdateFormDoesNotLogUnlinkedResultsFile(): void { + $form = $this->createUpdatableForm(); + $form->setFileId(42); + $form->setFileFormat('csv'); + + $this->apiController->updateForm(1, ['fileId' => null, 'fileFormat' => null]); + + $this->assertNull($form->getFileId()); + $this->assertAuditLogged([]); } public function testGetSubmission_invalidForm() { @@ -1708,6 +1943,9 @@ public function testUpdateSubmission_success() { $response = $this->apiController->updateSubmission($formId, $submissionId, $answers); $this->assertEquals(new DataResponse($submissionId), $response); + $this->assertAuditLogged([ + [Operation::SubmissionUpdated, 'Submission %s of form %s by "%s" was updated', ['submissionId' => 42, 'formId' => 1, 'submissionUserId' => 'currentUser']], + ]); } public function testUpdateSubmission_noPermission() { @@ -1766,6 +2004,7 @@ public function testUpdateSubmission_invalidAnswers() { ->method('validateSubmission') ->willThrowException(new \InvalidArgumentException('Invalid answers')); + $this->expectNoAuditLog(); $this->expectException(OCSBadRequestException::class); $this->expectExceptionMessage('Invalid answers'); $this->apiController->updateSubmission($formId, $submissionId, $answers); diff --git a/tests/Unit/Controller/ConfigControllerTest.php b/tests/Unit/Controller/ConfigControllerTest.php index 128e35049..f05f7d18a 100644 --- a/tests/Unit/Controller/ConfigControllerTest.php +++ b/tests/Unit/Controller/ConfigControllerTest.php @@ -8,11 +8,16 @@ namespace OCA\Forms\Tests\Unit\Controller; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Controller\ConfigController; use OCA\Forms\Service\ConfigService; +use OCA\Forms\Tests\Unit\AuditLog\AuditEvents; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Services\IAppConfig; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; +use OCP\Log\Audit\CriticalActionPerformedEvent; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -24,6 +29,7 @@ class ConfigControllerTest extends TestCase { private IAppConfig|MockObject $config; private LoggerInterface|MockObject $logger; private IRequest|MockObject $request; + private IEventDispatcher|MockObject $eventDispatcher; public function setUp(): void { parent::setUp(); @@ -32,13 +38,15 @@ public function setUp(): void { $this->config = $this->createMock(IAppConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->request = $this->createMock(IRequest::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->configController = new ConfigController( 'forms', $this->configService, $this->config, $this->logger, - $this->request + $this->request, + new AuditLogger($this->eventDispatcher), ); } @@ -99,9 +107,31 @@ public function testUpdateAppConfig(string $configKey, $configValue) { ->with($configKey, $configValue); } + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event) use ($configKey, $configValue): bool { + $this->assertTrue(AuditEvents::hasOperation($event, Operation::ConfigUpdated->value)); + $this->assertSame('Forms setting %s was changed', $event->getLogMessage()); + $this->assertSame(['configKey' => $configKey, 'configValue' => $configValue], $event->getParameters()); + return true; + })); + $this->assertEquals(new DataResponse(), $this->configController->updateAppConfig($configKey, $configValue)); } + public function testUpdateAppConfigInvalidValueIsNotLogged(): void { + $this->config->expects($this->once()) + ->method('setAppValueInt') + ->willThrowException(new \InvalidArgumentException()); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); + + $this->assertEquals( + new DataResponse('Invalid value for confirmationEmailRateLimit', 400), + $this->configController->updateAppConfig('confirmationEmailRateLimit', -1) + ); + } + public function testUpdateAppConfig_unknownKey() { $this->logger->expects($this->once()) ->method('debug'); @@ -110,6 +140,8 @@ public function testUpdateAppConfig_unknownKey() { ->method('setAppValueBool'); $this->config->expects($this->never()) ->method('setAppValueArray'); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); $this->assertEquals(new DataResponse('Unknown appConfig key: someUnknownKey', 400), $this->configController->updateAppConfig('someUnknownKey', 'storeThisValue!')); } diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 42099d651..636bb3bd7 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -9,6 +9,8 @@ namespace OCA\Forms\Tests\Unit\Controller; use OCA\Circles\Model\Circle; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Constants; use OCA\Forms\Controller\ShareApiController; use OCA\Forms\Db\Form; @@ -21,6 +23,7 @@ use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; use OCA\Forms\Service\UploadedFilesShareService; +use OCA\Forms\Tests\Unit\AuditLog\AuditEvents; use OCA\Forms\Tests\Unit\UserFolderMockTrait; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; @@ -29,6 +32,7 @@ use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IFilenameValidator; @@ -38,6 +42,7 @@ use OCP\IRequest; use OCP\IUser; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Security\ISecureRandom; use OCP\Share\IManager; use OCP\Share\IShare; @@ -68,6 +73,9 @@ class ShareApiControllerTest extends TestCase { private IFilenameValidator|MockObject $filenameValidator; private IManager|MockObject $shareManager; private UploadedFilesShareService $uploadedFilesShareService; + private IEventDispatcher|MockObject $eventDispatcher; + /** @var list */ + private array $dispatchedEvents = []; public function setUp(): void { $this->formMapper = $this->createMock(FormMapper::class); @@ -95,6 +103,13 @@ public function setUp(): void { $this->logger, ); + $this->dispatchedEvents = []; + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->eventDispatcher->method('dispatchTyped') + ->willReturnCallback(function (object $event): void { + $this->dispatchedEvents[] = $event; + }); + $this->shareApiController = new ShareApiController( 'forms', $this->request, @@ -111,9 +126,28 @@ public function setUp(): void { $this->filePathHelper, $this->shareManager, $this->uploadedFilesShareService, + new AuditLogger($this->eventDispatcher), ); } + private function expectNoAuditLog(): void { + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + } + + /** + * @param list}> $expected operation, message and parameters of each audit log entry, in order + */ + private function assertAuditLogged(array $expected): void { + $events = array_values(array_filter($this->dispatchedEvents, static fn (object $event): bool => $event instanceof CriticalActionPerformedEvent)); + $this->assertSame( + array_map(static fn (array $entry): array => [$entry[1], $entry[2]], $expected), + array_map(static fn (CriticalActionPerformedEvent $event): array => [$event->getLogMessage(), $event->getParameters()], $events), + ); + foreach ($events as $i => $event) { + $this->assertTrue(AuditEvents::hasOperation($event, $expected[$i][0]->value)); + } + } + public static function dataValidNewShare() { return [ 'newUserShare' => [ @@ -213,6 +247,15 @@ public function testValidNewShare(int $shareType, string $shareWith, array $perm // Share Form '5' to 'user1' of share-type 'user=0' $expectedResponse = new DataResponse($expected, Http::STATUS_CREATED); $this->assertEquals($expectedResponse, $this->shareApiController->newShare(5, $shareType, $shareWith, $permissions)); + $this->assertAuditLogged([ + [Operation::ShareCreated, 'Share %s of form %s was created', [ + 'shareId' => 13, + 'formId' => 5, + 'shareType' => $shareType, + 'shareWith' => $shareWith, + 'permissions' => $permissions, + ]], + ]); } public static function dataNewLinkShare() { @@ -296,7 +339,17 @@ public function testNewLinkShare(int $shareType, string $shareWith, array $permi // Share the form. $expectedResponse = new DataResponse($expected, Http::STATUS_CREATED); $this->assertEquals($expectedResponse, $this->shareApiController->newShare(5, $shareType, $shareWith, $permissions)); + $this->assertAuditLogged([ + [Operation::ShareCreated, 'Share %s of form %s was created', [ + 'shareId' => 13, + 'formId' => 5, + 'shareType' => IShare::TYPE_LINK, + 'shareWith' => null, + 'permissions' => $permissions, + ]], + ]); } else { + $this->expectNoAuditLog(); $this->expectException($exception); $this->shareApiController->newShare(5, $shareType, $shareWith, $permissions); } @@ -519,6 +572,15 @@ public function testDeleteShare() { $response = new DataResponse(8); $this->assertEquals($response, $this->shareApiController->deleteShare(5, 8)); + $this->assertAuditLogged([ + [Operation::ShareDeleted, 'Share %s of form %s was deleted', [ + 'shareId' => 8, + 'formId' => 5, + 'shareType' => null, + 'shareWith' => null, + 'permissions' => [Constants::PERMISSION_SUBMIT], + ]], + ]); } /** @@ -644,6 +706,7 @@ public function testDeleteForeignShare() { ->with('5') ->willThrowException(new NoSuchFormException('This form is not owned by the current user')); + $this->expectNoAuditLog(); $this->expectException(NoSuchFormException::class); $this->shareApiController->deleteShare(5, 8); } @@ -887,7 +950,18 @@ public function testUpdateShare(array $share, string $formOwner, array $keyValue if ($exception === null) { $expectedResponse = new DataResponse($expected); $this->assertEquals($expectedResponse, $this->shareApiController->updateShare($share['formId'], $share['id'], $keyValuePairs)); + $this->assertAuditLogged([ + [Operation::ShareUpdated, 'Share %s of form %s was updated', [ + 'shareId' => $share['id'], + 'formId' => $share['formId'], + 'shareType' => $share['shareType'], + 'shareWith' => $share['shareType'] === IShare::TYPE_LINK ? null : $share['shareWith'], + 'permissions' => $keyValuePairs['permissions'], + 'tokenChanged' => false, + ]], + ]); } else { + $this->expectNoAuditLog(); $this->expectException($exception); $this->shareApiController->updateShare($share['formId'], $share['id'], $keyValuePairs); } @@ -970,6 +1044,16 @@ public function testUpdateShareToken() { }); $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'tokenabcd'])); + $this->assertAuditLogged([ + [Operation::ShareUpdated, 'Share %s of form %s was updated', [ + 'shareId' => 8, + 'formId' => 5, + 'shareType' => IShare::TYPE_LINK, + 'shareWith' => null, + 'permissions' => [Constants::PERMISSION_SUBMIT], + 'tokenChanged' => true, + ]], + ]); } public function testUpdateShareToken_CustomTokensDisabled() { @@ -1197,6 +1281,7 @@ public function testUpdateShareToken_SameTokenReturnsEarly() { ->method('update'); $this->formsService->expects($this->never()) ->method('obtainFormLock'); + $this->expectNoAuditLog(); $this->assertEquals(new DataResponse(8), $this->shareApiController->updateShare(5, 8, ['token' => 'sameToken123'])); } diff --git a/tests/Unit/ShareReview/ShareReviewSourceTest.php b/tests/Unit/ShareReview/ShareReviewSourceTest.php index f262b1d2d..2189a57fd 100644 --- a/tests/Unit/ShareReview/ShareReviewSourceTest.php +++ b/tests/Unit/ShareReview/ShareReviewSourceTest.php @@ -9,16 +9,20 @@ namespace OCA\Forms\Tests\Unit\ShareReview; +use OCA\Forms\AuditLog\AuditLogger; +use OCA\Forms\AuditLog\Operation; use OCA\Forms\Db\Form; use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\Share; use OCA\Forms\Db\ShareMapper; use OCA\Forms\Service\UploadedFilesShareService; use OCA\Forms\ShareReview\ShareReviewSource; +use OCA\Forms\Tests\Unit\AuditLog\AuditEvents; use OCP\AppFramework\Db\DoesNotExistException; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; use OCP\IL10N; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Share\IShare; use OCP\Share\ShareReview\Events\ShareReviewAccessCheckEvent; use OCP\Share\ShareReview\ShareReviewEntry; @@ -34,6 +38,7 @@ final class ShareReviewSourceTest extends TestCase { private IEventDispatcher|MockObject $eventDispatcher; private LoggerInterface|MockObject $logger; private IL10N|MockObject $l10n; + private IEventDispatcher|MockObject $auditEventDispatcher; private ShareReviewSource $source; protected function setUp(): void { @@ -43,6 +48,7 @@ protected function setUp(): void { $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->logger = $this->createMock(LoggerInterface::class); $this->l10n = $this->createMock(IL10N::class); + $this->auditEventDispatcher = $this->createMock(IEventDispatcher::class); $this->l10n->method('t')->willReturnCallback( fn (string $text, array $params = []): string => empty($params) ? $text : vsprintf($text, $params) ); @@ -53,6 +59,7 @@ protected function setUp(): void { $this->eventDispatcher, $this->logger, $this->l10n, + new AuditLogger($this->auditEventDispatcher), ); } @@ -339,6 +346,7 @@ public function testDeleteShareEventDeniedReturnsFalse(): void { $event->denyAccess('not in group'); }); $this->shareMapper->expects($this->never())->method('delete'); + $this->auditEventDispatcher->expects($this->never())->method('dispatchTyped'); $this->assertFalse($this->source->deleteShare('7')); } @@ -369,6 +377,20 @@ public function testDeleteShareGrantedDeletesShareAndBumpsForm(): void { $this->formMapper->expects($this->once())->method('update')->with($form); // No results permission — the uploaded-files share must not be touched $this->uploadedFilesShareService->expects($this->never())->method('removeForCollaborator'); + $this->auditEventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with($this->callback(function (CriticalActionPerformedEvent $event): bool { + $this->assertTrue(AuditEvents::hasOperation($event, Operation::ShareDeleted->value)); + $this->assertSame('Share %s of form %s was deleted through the share review', $event->getLogMessage()); + $this->assertSame([ + 'shareId' => 7, + 'formId' => 10, + 'shareType' => IShare::TYPE_USER, + 'shareWith' => 'bob', + 'permissions' => ['submit'], + ], $event->getParameters()); + return true; + })); $this->assertTrue($this->source->deleteShare('7')); } @@ -401,6 +423,7 @@ public function testDeleteShareDbErrorReturnsFalse(): void { $this->formMapper->method('findById')->willReturn($form); $this->shareMapper->method('delete')->willThrowException($this->createMock(Exception::class)); $this->logger->expects($this->once())->method('error'); + $this->auditEventDispatcher->expects($this->never())->method('dispatchTyped'); $this->assertFalse($this->source->deleteShare('7')); }