diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php new file mode 100644 index 0000000000..e0bca428e7 --- /dev/null +++ b/lib/AuditLog/Operation.php @@ -0,0 +1,68 @@ + 'A table was created, from the web interface, the API, occ tables:add or a template', + self::TableArchived => 'A table was archived for all users it is shared with', + self::TableUnarchived => 'A table was unarchived', + self::TableDeleted => 'A table was deleted with its views, rows and shares', + self::TableTransferred => 'The ownership of a table was transferred to another user', + self::ViewCreated => 'A view was created on a table', + self::ViewUpdated => 'The filter, columns or settings of a view were changed, which changes the data people the view is shared with can see', + self::ViewDeleted => 'A view was deleted with its shares', + self::RowDeleted => 'A row was deleted', + self::ColumnDeleted => 'A column was deleted with its values in all rows', + self::ImportCompleted => 'Rows were imported into a table or view from a spreadsheet, possibly creating columns', + self::ContextCreated => 'An application was created', + self::ContextUpdated => 'An application was changed, including tables and views added to or removed from it and changes to their permissions in it, which exposes them to the people the application is shared with', + self::ContextDeleted => 'An application was deleted with its pages and shares', + self::ContextTransferred => 'The ownership of an application was transferred to another user', + self::ShareCreated => 'A table, view or application was shared with a user, group, team, remote user or by public link', + self::SharePermissionsUpdated => 'The permissions of a share of a table or view were changed', + self::ShareDeleted => 'A share was deleted, by someone managing the shared node or through the share review', + self::FederatedShareReceived => 'A table or view was shared with a local user from another server', + self::FederatedSharePermissionsUpdated => 'The server a table or view was shared from changed the permissions of the share with a local user', + self::LinkShareAuthenticationFailed => 'A wrong password was entered for a password protected public link share', + }; + } +} diff --git a/lib/Controller/PublicSharePageController.php b/lib/Controller/PublicSharePageController.php index 06ddf281cd..ef4b3fa860 100644 --- a/lib/Controller/PublicSharePageController.php +++ b/lib/Controller/PublicSharePageController.php @@ -9,10 +9,12 @@ namespace OCA\Tables\Controller; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Share; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Service\NodeService; use OCA\Tables\Service\ShareService; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\ShareToken; use OCA\Tables\Share\SharePageShareDataDecorator; use OCA\Text\Event\LoadEditor; @@ -44,6 +46,7 @@ public function __construct( private readonly NodeService $nodeService, private readonly IInitialState $initialState, private readonly IEventDispatcher $eventDispatcher, + private readonly AuditLogServiceInterface $auditLogService, ) { parent::__construct($appName, $request, $session, $urlGenerator); $token = $request->getParam('token'); @@ -103,6 +106,18 @@ public function showAuthenticate(): TemplateResponse { return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); } + protected function authFailed(): void { + if ($this->share === null) { + return; + } + + $this->auditLogService->log(Operation::LinkShareAuthenticationFailed, 'Wrong password entered for link share %s of %s %s', [ + 'shareId' => $this->share->getId(), + 'nodeType' => $this->share->getNodeType(), + 'nodeId' => $this->share->getNodeId(), + ]); + } + protected function showAuthFailed(): TemplateResponse { $templateParameters = ['share' => new SharePageShareDataDecorator($this->share), 'wrongpw' => true]; return new TemplateResponse('core', 'publicshareauth', $templateParameters, 'guest'); diff --git a/lib/Federation/FederationProvider.php b/lib/Federation/FederationProvider.php index 94fd4d29c3..06ebe7563b 100644 --- a/lib/Federation/FederationProvider.php +++ b/lib/Federation/FederationProvider.php @@ -9,6 +9,7 @@ namespace OCA\Tables\Federation; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Constants\ShareReceiverType; use OCA\Tables\Constants\ViewLayout; use OCA\Tables\Db\Share; @@ -20,6 +21,7 @@ use OCA\Tables\Errors\FederationDisabledError; use OCA\Tables\Errors\InternalError; use OCA\Tables\Service\FederationService; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\ShareToken; use OCP\AppFramework\Db\DoesNotExistException; use OCP\Federation\Exceptions\BadRequestException; @@ -42,6 +44,7 @@ public function __construct( private readonly ShareMapper $shareMapper, private readonly ICloudIdManager $cloudIdManager, private readonly FederationService $federationService, + private readonly AuditLogServiceInterface $auditLogService, ) { } @@ -89,10 +92,17 @@ public function shareReceived(ICloudFederationShare $share): string { $localShare = $this->buildShareForFederationNode($share, $nodeId, $nodeType, $localUser); try { - $this->shareMapper->insert($localShare); + $localShare = $this->shareMapper->insert($localShare); } catch (\Exception $e) { throw new ProviderCouldNotAddShareException('Could not create share for federated share: ' . $e->getMessage()); } + $this->auditLogService->log(Operation::FederatedShareReceived, 'Federated share %s of %s %s from "%s" was received for user "%s"', [ + 'shareId' => $localShare->getId(), + 'nodeType' => $nodeType, + 'nodeId' => $nodeId, + 'sharedBy' => $share->getSharedBy(), + 'receiver' => $localUser, + ]); return (string)$nodeId; } @@ -202,6 +212,18 @@ private function handlePermissionUpdate(array $notification): void { $share->setPermissionUpdate($notification['permissionUpdate']); $share->setPermissionDelete($notification['permissionDelete']); $this->shareMapper->update($share); + $this->auditLogService->log(Operation::FederatedSharePermissionsUpdated, 'Permissions of federated share %s of %s %s for "%s" were changed by the sharing server', [ + 'shareId' => $share->getId(), + 'nodeType' => $share->getNodeType(), + 'nodeId' => $share->getNodeId(), + 'receiver' => $share->getReceiver(), + 'permissions' => [ + 'read' => $share->getPermissionRead(), + 'create' => $share->getPermissionCreate(), + 'update' => $share->getPermissionUpdate(), + 'delete' => $share->getPermissionDelete(), + ], + ]); } private function handleNodeDelete(string $providerId, array $notification): void { diff --git a/lib/Listener/WhenRowDeletedAuditLogListener.php b/lib/Listener/WhenRowDeletedAuditLogListener.php index 356e3a6514..2cc5250efe 100644 --- a/lib/Listener/WhenRowDeletedAuditLogListener.php +++ b/lib/Listener/WhenRowDeletedAuditLogListener.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Event\RowDeletedEvent; use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\EventDispatcher\Event; @@ -28,10 +29,10 @@ public function handle(Event $event): void { } $row = $event->getRow(); - $rowId = $row->rowId; - $this->auditLogService->log("Row with ID: $rowId was deleted", [ - 'row' => $row->jsonSerialize(), + $this->auditLogService->log(Operation::RowDeleted, 'Row with ID: %s was deleted', [ + 'rowId' => $row->rowId, + 'tableId' => $row->tableId, ]); } } diff --git a/lib/Listener/WhenTableDeletedAuditLogListener.php b/lib/Listener/WhenTableDeletedAuditLogListener.php index 55e4a15662..76716345ca 100644 --- a/lib/Listener/WhenTableDeletedAuditLogListener.php +++ b/lib/Listener/WhenTableDeletedAuditLogListener.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Event\TableDeletedEvent; use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\EventDispatcher\Event; @@ -29,8 +30,9 @@ public function handle(Event $event): void { $table = $event->getTable(); - $this->auditLogService->log("Table with ID: $table->id was deleted", [ - 'table' => $table->jsonSerialize(), + $this->auditLogService->log(Operation::TableDeleted, 'Table with ID: %s was deleted', [ + 'tableId' => $table->getId(), + 'owner' => $table->getOwnership(), ]); } } diff --git a/lib/Listener/WhenTableTransferredAuditLogListener.php b/lib/Listener/WhenTableTransferredAuditLogListener.php index 8c78951e03..efabb5a99b 100644 --- a/lib/Listener/WhenTableTransferredAuditLogListener.php +++ b/lib/Listener/WhenTableTransferredAuditLogListener.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Event\TableOwnershipTransferredEvent; use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\EventDispatcher\Event; @@ -27,14 +28,10 @@ public function handle(Event $event): void { return; } - $table = $event->getTable(); - $fromUserId = $event->getFromUserId(); - $toUserId = $event->getToUserId(); - - $this->auditLogService->log("Table with ID: $table->id was transferred from user with ID: $fromUserId to user with ID: $toUserId", [ - 'table' => $table->jsonSerialize(), - 'fromUserId' => $fromUserId, - 'toUserId' => $toUserId, + $this->auditLogService->log(Operation::TableTransferred, 'Table with ID: %s was transferred from user with ID: %s to user with ID: %s', [ + 'tableId' => $event->getTable()->getId(), + 'fromUserId' => $event->getFromUserId(), + 'toUserId' => $event->getToUserId(), ]); } } diff --git a/lib/Listener/WhenViewDeletedAuditLogListener.php b/lib/Listener/WhenViewDeletedAuditLogListener.php index 0c2f816648..fb42cc8b4d 100644 --- a/lib/Listener/WhenViewDeletedAuditLogListener.php +++ b/lib/Listener/WhenViewDeletedAuditLogListener.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Event\ViewDeletedEvent; use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\EventDispatcher\Event; @@ -29,8 +30,9 @@ public function handle(Event $event): void { $view = $event->getView(); - $this->auditLogService->log("View with ID: $view->id was deleted", [ - 'view' => $view->jsonSerialize(), + $this->auditLogService->log(Operation::ViewDeleted, 'View with ID: %s was deleted', [ + 'viewId' => $view->getId(), + 'tableId' => $view->getTableId(), ]); } } diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 7bb5c79d59..201593c77d 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -10,6 +10,7 @@ use DateTime; use Exception; use OCA\Tables\Activity\ActivityManager; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Constants\ColumnType; use OCA\Tables\Db\Column; use OCA\Tables\Db\ColumnMapper; @@ -25,6 +26,7 @@ use OCA\Tables\Model\SelectionOptions; use OCA\Tables\Notification\NotificationHelper; use OCA\Tables\ResponseDefinitions; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\Title; use OCA\Tables\Service\ValueObject\ViewColumnInformation; use OCA\Tables\Validation\ColumnDtoValidator; @@ -54,6 +56,7 @@ public function __construct( private readonly IL10N $l, private readonly UserHelper $userHelper, private readonly ColumnDtoValidator $columnDtoValidator, + private readonly AuditLogServiceInterface $auditLogService, ) { parent::__construct($logger, $userId, $permissionsService); } @@ -614,6 +617,10 @@ public function delete(int $id, bool $skipRowCleanup = false, ?string $userId = if (!$skipRowCleanup) { $this->viewService->deleteColumnDataFromViews($id, $table); + $this->auditLogService->log(Operation::ColumnDeleted, 'Column with ID: %s of table with ID: %s was deleted', [ + 'columnId' => $id, + 'tableId' => $item->getTableId(), + ]); } return $this->enhanceColumn($item); diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index 468d3de83e..828897ad6f 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -10,6 +10,7 @@ use InvalidArgumentException; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; @@ -30,16 +31,15 @@ use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\ViewSettings; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IUserManager; -use OCP\Log\Audit\CriticalActionPerformedEvent; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; @@ -54,7 +54,7 @@ public function __construct( private LoggerInterface $logger, private PermissionsService $permissionsService, private IUserManager $userManager, - private IEventDispatcher $eventDispatcher, + private AuditLogServiceInterface $auditLogService, private IDBConnection $dbc, private ShareService $shareService, private bool $isCLI, @@ -150,6 +150,11 @@ public function create(string $name, string $iconName, string $description, arra } $this->insertPage($context); }, $this->dbc); + $this->auditLogService->log(Operation::ContextCreated, 'Tables application with ID %s was created for user %s', [ + 'contextId' => $context->getId(), + 'ownerId' => $ownerId, + 'nodes' => array_map(static fn (array $node): array => ['nodeType' => (int)$node['type'], 'nodeId' => (int)$node['id']], $nodes), + ]); return $context; } @@ -174,6 +179,9 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i } $hasUpdatedNodeInformation = false; + $addedNodes = []; + $removedNodes = []; + $updatedNodes = []; if ($nodes !== null) { $currentNodes = $context->getNodes(); $currentPages = $context->getPages(); @@ -197,6 +205,12 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $nodesBeingKept[$key] = $node; if ($node['permissions'] !== $currentNodes[$oldNodeResolvableIdMapper[$key]]['permissions']) { $nodeRel = $this->contextNodeRelMapper->findById($currentNodes[$oldNodeResolvableIdMapper[$key]]['id']); + $updatedNodes[] = [ + 'nodeType' => $nodeRel->getNodeType(), + 'nodeId' => $nodeRel->getNodeId(), + 'previousPermissions' => $nodeRel->getPermissions(), + 'permissions' => (int)$node['permissions'], + ]; $nodeRel->setPermissions($node['permissions']); $this->contextNodeRelMapper->update($nodeRel); $currentNodes[$oldNodeResolvableIdMapper[$key]]['permissions'] = $nodeRel->getPermissions(); @@ -216,6 +230,7 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $hasUpdatedNodeInformation = $hasUpdatedNodeInformation || !empty($nodesBeingAdded) || !empty($nodesBeingRemoved); foreach ($nodesBeingRemoved as $node) { + $removedNodes[] = ['nodeType' => (int)$node['node_type'], 'nodeId' => (int)$node['node_id']]; /** @var ContextNodeRelation $removedNode */ /** @var PageContent[] $removedContents */ [$removedNode, $removedContents] = $this->removeNodeFromContextAndPages($node['id']); @@ -232,6 +247,7 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i if (!$this->permissionsService->canManageNodeById($nodeType, $nodeId, $userId)) { throw new PermissionError(sprintf('Owner cannot manage node %d (type %d)', $nodeId, $nodeType)); } + $addedNodes[] = ['nodeType' => $nodeType, 'nodeId' => $nodeId]; /** @var ContextNodeRelation $addedNode */ /** @var PageContent $updatedContent */ @@ -304,6 +320,12 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $context->setNodes($currentNodes); $context->setPages($currentPages); } + $this->auditLogService->log(Operation::ContextUpdated, 'Tables application with ID %s was updated', [ + 'contextId' => $contextId, + 'addedNodes' => $addedNodes, + 'removedNodes' => $removedNodes, + 'updatedNodes' => $updatedNodes, + ]); return $context; } @@ -325,6 +347,10 @@ public function delete(int $contextId, string $userId): Context { } $this->contextMapper->delete($context); }, $this->dbc); + $this->auditLogService->log(Operation::ContextDeleted, 'Tables application with ID %s was deleted', [ + 'contextId' => $context->getId(), + 'ownerId' => $context->getOwnerId(), + ]); return $context; } @@ -374,15 +400,11 @@ public function transfer(int $contextId, string $newOwnerId, int $newOwnerType): throw new InternalError('Failed to transfer context ownership: ' . $e->getMessage()); } - $auditEvent = new CriticalActionPerformedEvent( - sprintf( - 'Tables application with ID %d was transferred to user %s', - $contextId, - $newOwnerId, - ) - ); - - $this->eventDispatcher->dispatchTyped($auditEvent); + $this->auditLogService->log(Operation::ContextTransferred, 'Tables application with ID %s was transferred to user %s', [ + 'contextId' => $contextId, + 'toUserId' => $newOwnerId, + 'fromUserId' => $oldOwnerId, + ]); return $context; } diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index 67fb529db1..fbc0054832 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -12,6 +12,7 @@ use LogicException; use OC\User\NoUserException; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\BackgroundJob\ImportTableJob; use OCA\Tables\Db\Column; use OCA\Tables\Dto\Column as ColumnDto; @@ -22,6 +23,7 @@ use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Model\ImportStats; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Vendor\PhpOffice\PhpSpreadsheet\Cell\Cell; use OCA\Tables\Vendor\PhpOffice\PhpSpreadsheet\Cell\Coordinate; use OCA\Tables\Vendor\PhpOffice\PhpSpreadsheet\Cell\DataType; @@ -98,6 +100,7 @@ public function __construct( private readonly ITempManager $tempManager, private readonly IFilenameValidator $filenameValidator, IAppDataFactory $appDataFactory, + private readonly AuditLogServiceInterface $auditLogService, ) { parent::__construct($logger, $userId, $permissionsService); $this->appData = $appDataFactory->get(Application::APP_ID); @@ -378,6 +381,7 @@ public function import(?int $tableId, ?int $viewId, string $path, bool $createMi $this->logger->warning('Storage for user could not be found', ['exception' => $e]); throw new NotFoundError('Storage for user could not be found'); } + $this->logImportCompleted(); return [ 'found_columns_count' => count(array_filter($this->columns, fn ($column): bool => $column instanceof Column)), @@ -531,6 +535,7 @@ public function importV2(string $userId, string $importType, string $path, ?int $this->logger->warning('Storage for user could not be found', ['exception' => $e]); throw new NotFoundError('Storage for user could not be found', 0, $e); } + $this->logImportCompleted(); return new ImportStats( count(array_filter($this->columns, fn ($column): bool => $column instanceof Column)), @@ -947,6 +952,22 @@ private function parseColumnDataType(Cell $cell): array { return $dataType; } + /** + * Only the counts are logged, never the imported values + */ + private function logImportCompleted(): void { + $this->auditLogService->log(Operation::ImportCompleted, 'Rows were imported into %s with ID: %s by user with ID: %s', [ + 'nodeType' => $this->viewId !== null ? 'view' : 'table', + 'nodeId' => $this->viewId ?? $this->tableId, + 'userId' => $this->userId, + 'insertedRows' => $this->countInsertedRows, + 'updatedRows' => $this->countUpdatedRows, + 'createdColumns' => $this->countCreatedColumns, + 'errors' => $this->countErrors, + 'parsingErrors' => $this->countParsingErrors, + ]); + } + /** * @return ISimpleFolder * @throws \OCP\Files\NotPermittedException diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php index cbbb649e05..1d5408c188 100644 --- a/lib/Service/ShareService.php +++ b/lib/Service/ShareService.php @@ -14,6 +14,7 @@ use OCA\Circles\Model\Circle; use OCA\Tables\Activity\ActivityManager; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Constants\ShareReceiverType; use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextNavigation; @@ -33,6 +34,7 @@ use OCA\Tables\Helper\UserHelper; use OCA\Tables\Model\Permissions; use OCA\Tables\ResponseDefinitions; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\ShareCreate; use OCA\Tables\Service\ValueObject\ShareToken; use OCP\AppFramework\Db\DoesNotExistException; @@ -71,6 +73,7 @@ public function __construct( private readonly IHasher $hasher, private readonly IShareManager $shareManager, private readonly FederationService $federationService, + private readonly AuditLogServiceInterface $auditLogService, ) { parent::__construct($logger, $userId, $permissionsService); } @@ -320,19 +323,21 @@ public function getSharedPermissionsIfSharedWithMe(int $elementId, string $eleme */ public function create(ShareCreate $dto): Share { if ($dto->getNodeType() === 'context') { - return $this->createContextShare( + $share = $this->createContextShare( $dto->getNodeId(), $dto->getReceiver(), $dto->getReceiverType(), $dto->getDisplayMode() ); + $this->logShareChange(Operation::ShareCreated, 'Share %s of %s %s was created for %s "%s"', $share); + return $share; } if ($dto->getReceiverType() === ShareReceiverType::REMOTE) { $dto->setShareToken($this->generateShareToken()); } - return $this->createNodeShare( + $share = $this->createNodeShare( $dto->getNodeId(), $dto->getNodeType(), $dto->getReceiver(), @@ -345,6 +350,8 @@ public function create(ShareCreate $dto): Share { $dto->getPassword(), $dto->getShareToken(), ); + $this->logShareChange(Operation::ShareCreated, 'Share %s of %s %s was created for %s "%s"', $share); + return $share; } /** @@ -651,6 +658,7 @@ public function updatePermission(int $id, array $permissions): Share { $share = $this->addReceiverDisplayName($share); $this->triggerShareActivity($share, ActivityManager::SUBJECT_SHARE_UPDATE); + $this->logShareChange(Operation::SharePermissionsUpdated, 'Permissions of share %s of %s %s for %s "%s" were changed', $share); return $share; } @@ -731,9 +739,30 @@ public function delete(int $id): Share { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } + $this->logShareChange(Operation::ShareDeleted, 'Share %s of %s %s for %s "%s" was deleted', $item); return $item; } + /** + * The share token and password are never logged + */ + private function logShareChange(Operation $operation, string $message, Share $share): void { + $this->auditLogService->log($operation, $message, [ + 'shareId' => $share->getId(), + 'nodeType' => $share->getNodeType(), + 'nodeId' => $share->getNodeId(), + 'receiverType' => $share->getReceiverType(), + 'receiver' => $share->getReceiver(), + 'permissions' => [ + 'read' => $share->getPermissionRead(), + 'create' => $share->getPermissionCreate(), + 'update' => $share->getPermissionUpdate(), + 'delete' => $share->getPermissionDelete(), + 'manage' => $share->getPermissionManage(), + ], + ]); + } + private function triggerShareActivity(Share $share, string $subject): void { if (!in_array($share->getNodeType(), ['table', 'view'], true)) { return; @@ -817,6 +846,7 @@ public function deleteForShareReview(int $id): void { if ($share->getNodeType() === 'context') { $this->contextNavigationMapper->deleteByShareId($share->getId()); } + $this->logShareChange(Operation::ShareDeleted, 'Share %s of %s %s for %s "%s" was deleted through the share review', $share); } public function deleteAllForTable(Table $table):void { diff --git a/lib/Service/Support/AuditLogServiceInterface.php b/lib/Service/Support/AuditLogServiceInterface.php index c93441011f..d8c9f5fb9a 100644 --- a/lib/Service/Support/AuditLogServiceInterface.php +++ b/lib/Service/Support/AuditLogServiceInterface.php @@ -8,6 +8,11 @@ namespace OCA\Tables\Service\Support; +use OCA\Tables\AuditLog\Operation; + interface AuditLogServiceInterface { - public function log(string $message, array $context): void; + /** + * @param array $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; } diff --git a/lib/Service/Support/DefaultAuditLogService.php b/lib/Service/Support/DefaultAuditLogService.php index 2d27230009..ea53d9afef 100644 --- a/lib/Service/Support/DefaultAuditLogService.php +++ b/lib/Service/Support/DefaultAuditLogService.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Service\Support; +use OCA\Tables\AuditLog\Operation; use OCP\EventDispatcher\IEventDispatcher; use OCP\Log\Audit\CriticalActionPerformedEvent; @@ -17,9 +18,16 @@ public function __construct( ) { } - public function log(string $message, array $context): void { - $auditEvent = new CriticalActionPerformedEvent($message, $context); + /** + * The operation is only passed on to servers whose CriticalActionPerformedEvent supports it. + * The check can be dropped once Nextcloud 36 is the lowest supported server version. + */ + 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($auditEvent); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(...$arguments)); } } diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index adf59034df..11d2ca1173 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -13,6 +13,7 @@ use OCA\Tables\Activity\ActivityManager; use OCA\Tables\Activity\ChangeSet; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Dto\Column as ColumnDto; @@ -31,6 +32,7 @@ use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\Title; use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; use OCP\App\IAppManager; @@ -72,6 +74,7 @@ public function __construct( protected Defaults $themingDefaults, private ActivityManager $activityManager, private FederationService $federationService, + private AuditLogServiceInterface $auditLogService, ) { parent::__construct($logger, $userId, $permissionsService); } @@ -348,6 +351,11 @@ public function create(string $title, string $template, ?string $emoji, ?string additionalParams: [], author: $userId ); + $this->auditLogService->log(Operation::TableCreated, 'Table with ID: %s was created by user with ID: %s', [ + 'tableId' => $table->getId(), + 'owner' => $userId, + 'template' => $template, + ]); return $table; } @@ -382,6 +390,7 @@ public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null throw new PermissionError('PermissionError: can not change table owner with table id ' . $id); } + $previousOwnerUserId = $table->getOwnership(); $table->setOwnership($newOwnerUserId); try { @@ -398,7 +407,7 @@ public function setOwner(int $id, string $newOwnerUserId, ?string $userId = null $event = new TableOwnershipTransferredEvent( table: $table, toUserId: $newOwnerUserId, - fromUserId: $userId + fromUserId: $previousOwnerUserId, ); $this->eventDispatcher->dispatchTyped($event); @@ -528,6 +537,7 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip } $changes = new ChangeSet($table); + $wasArchived = $table->getArchived(); $time = new DateTime(); if ($title !== null) { $title = (string)new Title($title); @@ -572,6 +582,13 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip changeSet: $changes, subject: ActivityManager::SUBJECT_TABLE_UPDATE ); + if ($archived !== null && $archived !== $wasArchived) { + $this->auditLogService->log( + $archived ? Operation::TableArchived : Operation::TableUnarchived, + $archived ? 'Table with ID: %s was archived' : 'Table with ID: %s was unarchived', + ['tableId' => $table->getId()], + ); + } return $table; } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index c50ae3aadd..f74009b549 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -16,6 +16,7 @@ use OCA\Tables\Activity\ActivityManager; use OCA\Tables\Activity\ChangeSet; use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Constants\ViewLayout; use OCA\Tables\Constants\ViewUpdatableParameters; use OCA\Tables\Db\Column; @@ -35,6 +36,7 @@ use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\ViewColumnInformation; use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; use OCP\AppFramework\Db\DoesNotExistException; @@ -61,6 +63,7 @@ public function __construct( protected IL10N $l, private readonly FederationService $federationService, private readonly ActivityManager $activityManager, + private readonly AuditLogServiceInterface $auditLogService, ) { parent::__construct($logger, $userId, $permissionsService); } @@ -244,6 +247,10 @@ public function create( additionalParams: [], author: $userId, ); + $this->auditLogService->log(Operation::ViewCreated, 'View with ID: %s was created on table with ID: %s', [ + 'viewId' => $newItem->getId(), + 'tableId' => $table->getId(), + ]); return $newItem; } @@ -270,7 +277,12 @@ public function update(int $id, ViewUpdateInput $data, ?string $userId = null, b // and it costs a query. $managedColumnIds = null; $columnSelectionChanged = false; + $changedDataScope = []; foreach ($data->updateDetail() as $parameter => $value) { + if (in_array($parameter, [ViewUpdatableParameters::FILTER, ViewUpdatableParameters::COLUMN_SETTINGS, ViewUpdatableParameters::VIEW_SETTINGS], true)) { + $changedDataScope[] = $parameter->value; + } + if ($parameter === ViewUpdatableParameters::COLUMN_SETTINGS && $value instanceof ColumnSettings ) { @@ -326,6 +338,13 @@ public function update(int $id, ViewUpdateInput $data, ?string $userId = null, b changeSet: $changes, subject: ActivityManager::SUBJECT_VIEW_UPDATE, ); + if ($changedDataScope !== []) { + $this->auditLogService->log(Operation::ViewUpdated, 'View with ID: %s of table with ID: %s was updated', [ + 'viewId' => $view->getId(), + 'tableId' => $view->getTableId(), + 'changedSettings' => $changedDataScope, + ]); + } return $view; } catch (BadRequestError|InvalidArgumentException $e) { throw $e; diff --git a/tests/unit/AuditLog/OperationTest.php b/tests/unit/AuditLog/OperationTest.php new file mode 100644 index 0000000000..e152710303 --- /dev/null +++ b/tests/unit/AuditLog/OperationTest.php @@ -0,0 +1,26 @@ + [$operation], Operation::cases()); + } + + /** + * @dataProvider dataOperations + */ + public function testOperationFormat(Operation $operation): void { + $this->assertMatchesRegularExpression('/^tables\.[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*$/', $operation->value); + $this->assertNotSame('', $operation->description()); + } +} diff --git a/tests/unit/Controller/PublicSharePageControllerTest.php b/tests/unit/Controller/PublicSharePageControllerTest.php new file mode 100644 index 0000000000..bacde0c20d --- /dev/null +++ b/tests/unit/Controller/PublicSharePageControllerTest.php @@ -0,0 +1,89 @@ +setId(12); + $share->setNodeType('table'); + $share->setNodeId(5); + $share->setToken('abcdefghijklmnop'); + + $request = $this->createMock(IRequest::class); + $request->method('getParam')->with('token')->willReturn('abcdefghijklmnop'); + $shareService = $this->createMock(ShareService::class); + $shareService->method('findByToken')->willReturn($share); + $shareService->method('isPublicShareAccessible')->willReturn(true); + $auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $controller = new PublicSharePageController( + 'tables', + $request, + $this->createMock(ISession::class), + $this->createMock(IURLGenerator::class), + $shareService, + $this->createMock(NodeService::class), + $this->createMock(IInitialState::class), + $this->createMock(IEventDispatcher::class), + $auditLogService, + ); + + $auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::LinkShareAuthenticationFailed, 'Wrong password entered for link share %s of %s %s', [ + 'shareId' => 12, + 'nodeType' => 'table', + 'nodeId' => 5, + ]); + + $authFailed = new \ReflectionMethod($controller, 'authFailed'); + $authFailed->invoke($controller); + } + + public function testAuthFailedDoesNotLogWithoutShare(): void { + $request = $this->createMock(IRequest::class); + $request->method('getParam')->with('token')->willReturn('abcdefghijklmnop'); + $shareService = $this->createMock(ShareService::class); + $shareService->method('findByToken')->willThrowException(new NotFoundError()); + $auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $controller = new PublicSharePageController( + 'tables', + $request, + $this->createMock(ISession::class), + $this->createMock(IURLGenerator::class), + $shareService, + $this->createMock(NodeService::class), + $this->createMock(IInitialState::class), + $this->createMock(IEventDispatcher::class), + $auditLogService, + ); + + $auditLogService->expects($this->never())->method('log'); + + $authFailed = new \ReflectionMethod($controller, 'authFailed'); + $authFailed->invoke($controller); + } +} diff --git a/tests/unit/Federation/FederationProviderTest.php b/tests/unit/Federation/FederationProviderTest.php index 92d23f8253..8fdc4a5ede 100644 --- a/tests/unit/Federation/FederationProviderTest.php +++ b/tests/unit/Federation/FederationProviderTest.php @@ -9,6 +9,7 @@ namespace OCA\Tables\Tests\Unit\Federation; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Constants\ShareReceiverType; use OCA\Tables\Db\Share; use OCA\Tables\Db\ShareMapper; @@ -19,9 +20,11 @@ use OCA\Tables\Errors\FederationDisabledError; use OCA\Tables\Federation\FederationProvider; use OCA\Tables\Service\FederationService; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\Federation\Exceptions\ProviderCouldNotAddShareException; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class FederationProviderTest extends TestCase { @@ -31,6 +34,7 @@ class FederationProviderTest extends TestCase { private ShareMapper $shareMapper; private FederationProvider $provider; private FederationService $federationService; + private AuditLogServiceInterface&MockObject $auditLogService; protected function setUp(): void { parent::setUp(); @@ -39,6 +43,7 @@ protected function setUp(): void { $this->shareMapper = $this->createMock(ShareMapper::class); $this->cloudIdManager = $this->createMock(ICloudIdManager::class); $this->federationService = $this->createMock(FederationService::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); $this->provider = new FederationProvider( $this->tableMapper, @@ -46,6 +51,7 @@ protected function setUp(): void { $this->shareMapper, $this->cloudIdManager, $this->federationService, + $this->auditLogService, ); } @@ -57,6 +63,7 @@ public function testShareReceivedThrowsWhenIncomingFederationDisabled(): void { $this->federationService->expects($this->once()) ->method('ensureIncomingFederationEnabled') ->willThrowException(new FederationDisabledError('Federation is disabled')); + $this->auditLogService->expects($this->never())->method('log'); $this->expectException(ProviderCouldNotAddShareException::class); $this->provider->shareReceived($share); @@ -89,8 +96,18 @@ public function testShareReceivedCreatesTable(): void { $this->assertEquals('table', $share->getNodeType()); $this->assertEquals(ShareReceiverType::USER, $share->getReceiverType()); $this->assertEquals('admin', $share->getReceiver()); + $share->setId(30); return $share; }); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::FederatedShareReceived, 'Federated share %s of %s %s from "%s" was received for user "%s"', [ + 'shareId' => 30, + 'nodeType' => 'table', + 'nodeId' => 1, + 'sharedBy' => 'admin@nextcloud.local', + 'receiver' => 'admin', + ]); $result = $this->provider->shareReceived($share); $this->assertEquals('1', $result); @@ -130,11 +147,28 @@ public function testShareReceivedCreatesView(): void { public function testNotificationReceivedPermissionUpdate(): void { $share = new Share(); + $share->setId(30); $share->setNodeId(1); + $share->setNodeType('table'); + $share->setReceiver('admin'); $share->setToken('abcdefghijklmnop'); $this->shareMapper->method('findByToken')->willReturn($share); $this->shareMapper->expects($this->once())->method('update'); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::FederatedSharePermissionsUpdated, 'Permissions of federated share %s of %s %s for "%s" were changed by the sharing server', [ + 'shareId' => 30, + 'nodeType' => 'table', + 'nodeId' => 1, + 'receiver' => 'admin', + 'permissions' => [ + 'read' => true, + 'create' => false, + 'update' => true, + 'delete' => false, + ], + ]); $this->provider->notificationReceived( FederationProvider::NOTIFICATION_UPDATE_PERMISSIONS, diff --git a/tests/unit/Listener/WhenRowDeletedAuditLogTest.php b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php index 59c262bb3d..701ca94c31 100644 --- a/tests/unit/Listener/WhenRowDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenRowDeletedAuditLogTest.php @@ -7,6 +7,7 @@ namespace unit\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Row2; use OCA\Tables\Event\RowDeletedEvent; use OCA\Tables\Listener\WhenRowDeletedAuditLogListener; @@ -35,10 +36,9 @@ public function testHandle(): void { ->expects($this->once()) ->method('log') ->with( - $this->equalTo("Row with ID: {$row->getId()} was deleted"), - $this->equalTo([ - 'row' => $row->toPublicRow()->jsonSerialize(), - ]) + Operation::RowDeleted, + 'Row with ID: %s was deleted', + ['rowId' => 1, 'tableId' => 161], ); $this->listener->handle($event); diff --git a/tests/unit/Listener/WhenTableDeletedAuditLogTest.php b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php index 045e58a99f..08ad1165af 100644 --- a/tests/unit/Listener/WhenTableDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenTableDeletedAuditLogTest.php @@ -7,6 +7,7 @@ namespace unit\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Table; use OCA\Tables\Event\TableDeletedEvent; use OCA\Tables\Listener\WhenTableDeletedAuditLogListener; @@ -27,6 +28,7 @@ protected function setUp(): void { public function testHandle(): void { $table = new Table(); $table->id = 1; + $table->setOwnership('alice'); $event = new TableDeletedEvent($table); @@ -34,10 +36,9 @@ public function testHandle(): void { ->expects($this->once()) ->method('log') ->with( - $this->equalTo("Table with ID: {$table->id} was deleted"), - $this->equalTo([ - 'table' => $table->jsonSerialize(), - ]) + Operation::TableDeleted, + 'Table with ID: %s was deleted', + ['tableId' => 1, 'owner' => 'alice'], ); $this->listener->handle($event); diff --git a/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php b/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php index 37b30f134c..b0fb2ca56b 100644 --- a/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php +++ b/tests/unit/Listener/WhenTableOwnerTransferredAuditLogTest.php @@ -7,6 +7,7 @@ namespace unit\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Table; use OCA\Tables\Event\TableOwnershipTransferredEvent; use OCA\Tables\Listener\WhenTableTransferredAuditLogListener; @@ -36,12 +37,9 @@ public function testHandle(): void { ->expects($this->once()) ->method('log') ->with( - $this->equalTo("Table with ID: {$table->id} was transferred from user with ID: $fromUserId to user with ID: $toUserId"), - $this->equalTo([ - 'table' => $table->jsonSerialize(), - 'fromUserId' => $fromUserId, - 'toUserId' => $toUserId, - ]) + Operation::TableTransferred, + 'Table with ID: %s was transferred from user with ID: %s to user with ID: %s', + ['tableId' => 1, 'fromUserId' => $fromUserId, 'toUserId' => $toUserId], ); $this->listener->handle($event); diff --git a/tests/unit/Listener/WhenViewDeletedAuditLogTest.php b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php index b1b2f51fdd..94ec79feef 100644 --- a/tests/unit/Listener/WhenViewDeletedAuditLogTest.php +++ b/tests/unit/Listener/WhenViewDeletedAuditLogTest.php @@ -7,6 +7,7 @@ namespace unit\Listener; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\View; use OCA\Tables\Event\ViewDeletedEvent; use OCA\Tables\Listener\WhenViewDeletedAuditLogListener; @@ -27,6 +28,7 @@ protected function setUp(): void { public function testHandle(): void { $view = new View(); $view->id = 1; + $view->setTableId(5); $event = new ViewDeletedEvent(view: $view); @@ -34,10 +36,9 @@ public function testHandle(): void { ->expects($this->once()) ->method('log') ->with( - $this->equalTo("View with ID: {$view->id} was deleted"), - $this->equalTo([ - 'view' => $view->jsonSerialize(), - ]) + Operation::ViewDeleted, + 'View with ID: %s was deleted', + ['viewId' => 1, 'tableId' => 5], ); $this->listener->handle($event); diff --git a/tests/unit/Service/ColumnServiceAuditLogTest.php b/tests/unit/Service/ColumnServiceAuditLogTest.php new file mode 100644 index 0000000000..83a91a1a4b --- /dev/null +++ b/tests/unit/Service/ColumnServiceAuditLogTest.php @@ -0,0 +1,87 @@ +createMock(PermissionsService::class); + $permissionsService->method('canDeleteColumnsByTableId')->willReturn(true); + $tableMapper = $this->createMock(TableMapper::class); + $tableMapper->method('find')->willReturn(new Table()); + $this->mapper = $this->createMock(ColumnMapper::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->service = new ColumnService( + $permissionsService, + $this->createMock(LoggerInterface::class), + 'alice', + $this->mapper, + $tableMapper, + $this->createMock(ViewService::class), + $this->createMock(RowService::class), + $this->createMock(ActivityManager::class), + $this->createMock(NotificationHelper::class), + $this->createMock(IL10N::class), + $this->createMock(UserHelper::class), + $this->createMock(ColumnDtoValidator::class), + $this->auditLogService, + ); + + $column = new Column(); + $column->setId(9); + $column->setTableId(5); + $column->setType('text'); + $column->setCreatedBy('alice'); + $column->setLastEditBy('alice'); + $this->mapper->method('find')->with(9)->willReturn($column); + } + + public function testDeleteLogsCriticalAction(): void { + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ColumnDeleted, 'Column with ID: %s of table with ID: %s was deleted', [ + 'columnId' => 9, + 'tableId' => 5, + ]); + + $this->service->delete(9, false, 'alice'); + } + + public function testDeleteTogetherWithTableDoesNotLog(): void { + $this->auditLogService->expects($this->never())->method('log'); + + $this->service->delete(9, true, 'alice'); + } +} diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 266827b7cd..67b94c8cca 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -9,8 +9,11 @@ namespace OCA\Tables\Service; +use OCA\Tables\AppInfo\Application; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextMapper; +use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; @@ -18,7 +21,7 @@ use OCA\Tables\Db\PageMapper; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; -use OCP\EventDispatcher\IEventDispatcher; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCP\IDBConnection; use OCP\INavigationManager; use OCP\IURLGenerator; @@ -33,6 +36,8 @@ final class ContextServiceTest extends TestCase { private MockObject $contextNodeRelMapper; private MockObject $pageMapper; private MockObject $pageContentMapper; + private MockObject $userManager; + private MockObject $auditLogService; protected function setUp(): void { parent::setUp(); @@ -43,8 +48,8 @@ protected function setUp(): void { $this->pageContentMapper = $this->createMock(PageContentMapper::class); $logger = $this->createMock(LoggerInterface::class); $permissionsService = $this->createMock(PermissionsService::class); - $userManager = $this->createMock(IUserManager::class); - $eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); $dbConnection = $this->createMock(IDBConnection::class); $shareService = $this->createMock(ShareService::class); $navigationManager = $this->createMock(INavigationManager::class); @@ -57,8 +62,8 @@ protected function setUp(): void { $this->pageContentMapper, $logger, $permissionsService, - $userManager, - $eventDispatcher, + $this->userManager, + $this->auditLogService, $dbConnection, $shareService, false, @@ -69,6 +74,26 @@ protected function setUp(): void { ); } + public function testTransferLogsPreviousAndNewOwner(): void { + $context = new Context(); + $context->setId(7); + $context->setOwnerId('alice'); + $context->setOwnerType(Application::OWNER_TYPE_USER); + $this->contextMapper->method('findById')->with(7)->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->userManager->method('userExists')->with('bob')->willReturn(true); + + $this->auditLogService->expects($this->once()) + ->method('log') + ->with( + Operation::ContextTransferred, + 'Tables application with ID %s was transferred to user %s', + ['contextId' => 7, 'toUserId' => 'bob', 'fromUserId' => 'alice'], + ); + + $this->service->transfer(7, 'bob', Application::OWNER_TYPE_USER); + } + public function testUpdateReordersStartPageContentsFromSubmittedNodes(): void { $context = new Context(); $context->id = 7; @@ -135,6 +160,14 @@ public function testUpdateReordersStartPageContentsFromSubmittedNodes(): void { ->method('update') ->with($context) ->willReturn($context); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ContextUpdated, 'Tables application with ID %s was updated', [ + 'contextId' => 7, + 'addedNodes' => [], + 'removedNodes' => [], + 'updatedNodes' => [], + ]); $updatedContext = $this->service->update(7, 'user-1', null, null, null, [ [ @@ -156,6 +189,100 @@ public function testUpdateReordersStartPageContentsFromSubmittedNodes(): void { self::assertSame(10, $updatedContext->getPages()[301]['content'][502]['order']); } + public function testUpdateLogsRemovedNodes(): void { + $context = new Context(); + $context->id = 7; + $context->setNodes([ + 101 => ['id' => 101, 'node_type' => 0, 'node_id' => 11, 'permissions' => 1], + ]); + $context->setPages([]); + $this->contextMapper->method('findById')->with(7, 'user-1')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $nodeRelation = new ContextNodeRelation(); + $nodeRelation->id = 101; + $this->contextNodeRelMapper->method('findById')->with(101)->willReturn($nodeRelation); + $this->contextNodeRelMapper->method('delete')->willReturnArgument(0); + $this->pageContentMapper->method('findByNodeRelation')->willReturn([]); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ContextUpdated, 'Tables application with ID %s was updated', [ + 'contextId' => 7, + 'addedNodes' => [], + 'removedNodes' => [['nodeType' => 0, 'nodeId' => 11]], + 'updatedNodes' => [], + ]); + + $this->service->update(7, 'user-1', null, null, null, []); + } + + public function testUpdateLogsNodePermissionChanges(): void { + $context = new Context(); + $context->id = 7; + $context->setNodes([ + 101 => ['id' => 101, 'node_type' => 0, 'node_id' => 11, 'permissions' => 1], + ]); + $context->setPages([]); + $this->contextMapper->method('findById')->with(7, 'user-1')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $nodeRelation = new ContextNodeRelation(); + $nodeRelation->id = 101; + $nodeRelation->setNodeType(0); + $nodeRelation->setNodeId(11); + $nodeRelation->setPermissions(1); + $this->contextNodeRelMapper->method('findById')->with(101)->willReturn($nodeRelation); + $this->contextNodeRelMapper->expects($this->once())->method('update')->with($nodeRelation)->willReturnArgument(0); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ContextUpdated, 'Tables application with ID %s was updated', [ + 'contextId' => 7, + 'addedNodes' => [], + 'removedNodes' => [], + 'updatedNodes' => [['nodeType' => 0, 'nodeId' => 11, 'previousPermissions' => 1, 'permissions' => 3]], + ]); + + $this->service->update(7, 'user-1', null, null, null, [ + ['id' => 11, 'type' => 0, 'permissions' => 3], + ]); + + self::assertSame(3, $nodeRelation->getPermissions()); + } + + public function testCreateLogsCriticalAction(): void { + $this->contextMapper->method('insert')->willReturnCallback(function (Context $context): Context { + $context->id = 9; + return $context; + }); + $this->pageMapper->method('insert')->willReturnCallback(function (Page $page): Page { + $page->id = 1; + return $page; + }); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ContextCreated, 'Tables application with ID %s was created for user %s', [ + 'contextId' => 9, + 'ownerId' => 'alice', + 'nodes' => [], + ]); + + $this->service->create('Projects', 'icon', '', [], 'alice', Application::OWNER_TYPE_USER); + } + + public function testDeleteLogsCriticalAction(): void { + $context = new Context(); + $context->id = 9; + $context->setOwnerId('alice'); + $this->contextMapper->method('findById')->with(9, 'alice')->willReturn($context); + $this->pageMapper->method('getPageIdsForContext')->willReturn([]); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ContextDeleted, 'Tables application with ID %s was deleted', [ + 'contextId' => 9, + 'ownerId' => 'alice', + ]); + + $this->service->delete(9, 'alice'); + } + private function buildPageContent(int $id, int $pageId, int $nodeRelId, int $order): PageContent { $pageContent = new PageContent(); $pageContent->id = $id; diff --git a/tests/unit/Service/ImportServiceAuditLogTest.php b/tests/unit/Service/ImportServiceAuditLogTest.php new file mode 100644 index 0000000000..561022843a --- /dev/null +++ b/tests/unit/Service/ImportServiceAuditLogTest.php @@ -0,0 +1,110 @@ +temporaryFile !== '' && file_exists($this->temporaryFile)) { + unlink($this->temporaryFile); + } + parent::tearDown(); + } + + private function createService(ITempManager $tempManager, IAppDataFactory $appDataFactory, AuditLogServiceInterface $auditLogService): ImportService { + return new ImportService( + $this->createMock(PermissionsService::class), + $this->createMock(LoggerInterface::class), + 'alice', + $this->createMock(IRootFolder::class), + $this->createMock(ColumnService::class), + $this->createMock(RowService::class), + $this->createMock(TableService::class), + $this->createMock(ViewService::class), + $this->createMock(IUserManager::class), + $this->createMock(ColumnsHelper::class), + $this->createMock(IJobList::class), + $tempManager, + $this->createMock(IFilenameValidator::class), + $appDataFactory, + $auditLogService, + ); + } + + public function testImportV2LogsCountsWithoutValues(): void { + $this->temporaryFile = tempnam(sys_get_temp_dir(), 'tables-import-') . '.csv'; + $file = $this->createMock(ISimpleFile::class); + $file->method('getExtension')->willReturn('csv'); + $file->method('getContent')->willReturn("Title\n"); + $folder = $this->createMock(ISimpleFolder::class); + $folder->method('getFile')->with('import.csv')->willReturn($file); + $appData = $this->createMock(IAppData::class); + $appData->method('getFolder')->with('import')->willReturn($folder); + $appDataFactory = $this->createMock(IAppDataFactory::class); + $appDataFactory->method('get')->willReturn($appData); + $tempManager = $this->createMock(ITempManager::class); + $tempManager->method('getTemporaryFile')->willReturn($this->temporaryFile); + $auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $service = $this->createService($tempManager, $appDataFactory, $auditLogService); + + $auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ImportCompleted, 'Rows were imported into %s with ID: %s by user with ID: %s', [ + 'nodeType' => 'table', + 'nodeId' => 5, + 'userId' => 'alice', + 'insertedRows' => 0, + 'updatedRows' => 0, + 'createdColumns' => 0, + 'errors' => 0, + 'parsingErrors' => 0, + ]); + + $service->importV2('alice', ImportService::IMPORT_TYPE_APP_FILE, 'import.csv', 5, null); + } + + public function testFailedImportDoesNotLog(): void { + $auditLogService = $this->createMock(AuditLogServiceInterface::class); + $auditLogService->expects($this->never())->method('log'); + $service = $this->createService( + $this->createMock(ITempManager::class), + $this->createMock(IAppDataFactory::class), + $auditLogService, + ); + + $this->expectException(NotFoundError::class); + + $service->importV2('alice', 'unknown', 'import.csv', 5, null); + } +} diff --git a/tests/unit/Service/ShareServiceTest.php b/tests/unit/Service/ShareServiceTest.php index ce37baf15f..1ab87968e5 100644 --- a/tests/unit/Service/ShareServiceTest.php +++ b/tests/unit/Service/ShareServiceTest.php @@ -9,6 +9,7 @@ namespace OCA\Tables\Tests\Unit\Service; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\ContextNavigationMapper; use OCA\Tables\Db\Share; use OCA\Tables\Db\ShareMapper; @@ -22,6 +23,7 @@ use OCA\Tables\Service\FederationService; use OCA\Tables\Service\PermissionsService; use OCA\Tables\Service\ShareService; +use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\ValueObject\ShareCreate; use OCA\Tables\Service\ValueObject\ShareToken; use OCP\AppFramework\Db\DoesNotExistException; @@ -32,6 +34,7 @@ use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Share\IManager as IShareManager; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -53,6 +56,7 @@ class ShareServiceTest extends TestCase { protected $shareService; private Share $publicShare; private FederationService $federationService; + private AuditLogServiceInterface&MockObject $auditLogService; protected function setUp(): void { parent::setUp(); @@ -71,6 +75,7 @@ protected function setUp(): void { $this->hasher = $this->createMock(IHasher::class); $this->shareManager = $this->createMock(IShareManager::class); $this->federationService = $this->createMock(FederationService::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); $this->shareService = new ShareService( $this->permissionsService, @@ -89,6 +94,7 @@ protected function setUp(): void { $this->hasher, $this->shareManager, $this->federationService, + $this->auditLogService, ); $this->publicShare = new Share(); @@ -116,6 +122,104 @@ public function testCreateContextShareSetsAllPermissionsFalse(): void { $this->assertFalse($share->getPermissionManage()); } + private function createTableShare(): Share { + $share = new Share(); + $share->setId(12); + $share->setNodeType('table'); + $share->setNodeId(5); + $share->setReceiverType('user'); + $share->setReceiver('bob'); + $share->setToken('secret-token'); + $share->setPermissionRead(true); + $share->setPermissionCreate(false); + $share->setPermissionUpdate(false); + $share->setPermissionDelete(false); + $share->setPermissionManage(false); + return $share; + } + + private function expectShareAuditLog(Operation $operation, string $message, array $permissions): void { + $this->auditLogService->expects($this->once()) + ->method('log') + ->with($operation, $message, [ + 'shareId' => 12, + 'nodeType' => 'table', + 'nodeId' => 5, + 'receiverType' => 'user', + 'receiver' => 'bob', + 'permissions' => $permissions, + ]); + } + + public function testCreateContextShareLogsCriticalAction(): void { + $this->mapper->method('insert')->willReturnCallback(function (Share $share) { + $share->setId(3); + return $share; + }); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ShareCreated, 'Share %s of %s %s was created for %s "%s"', [ + 'shareId' => 3, + 'nodeType' => 'context', + 'nodeId' => 1, + 'receiverType' => 'user', + 'receiver' => 'user1', + 'permissions' => ['read' => false, 'create' => false, 'update' => false, 'delete' => false, 'manage' => false], + ]); + + $this->shareService->create(new ShareCreate(1, 'context', 'user1', 'user', true, true, true, true, true, 0)); + } + + public function testUpdatePermissionLogsNewPermissions(): void { + $this->mapper->method('find')->with(12)->willReturn($this->createTableShare()); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canManageElementById')->willReturn(true); + $this->tableMapper->method('find')->willThrowException(new DoesNotExistException('')); + $this->expectShareAuditLog( + Operation::SharePermissionsUpdated, + 'Permissions of share %s of %s %s for %s "%s" were changed', + ['read' => true, 'create' => true, 'update' => true, 'delete' => false, 'manage' => false], + ); + + $this->shareService->updatePermission(12, ['create' => true, 'update' => true]); + } + + public function testDeleteLogsCriticalAction(): void { + $this->mapper->method('find')->with(12)->willReturn($this->createTableShare()); + $this->permissionsService->method('canManageElementById')->willReturn(true); + $this->tableMapper->method('find')->willThrowException(new DoesNotExistException('')); + $this->expectShareAuditLog( + Operation::ShareDeleted, + 'Share %s of %s %s for %s "%s" was deleted', + ['read' => true, 'create' => false, 'update' => false, 'delete' => false, 'manage' => false], + ); + + $this->shareService->delete(12); + } + + public function testDeleteWithoutPermissionDoesNotLog(): void { + $this->mapper->method('find')->with(12)->willReturn($this->createTableShare()); + $this->permissionsService->method('canManageElementById')->willReturn(false); + $this->mapper->expects($this->never())->method('delete'); + $this->auditLogService->expects($this->never())->method('log'); + + $this->expectException(PermissionError::class); + + $this->shareService->delete(12); + } + + public function testDeleteForShareReviewLogsCriticalAction(): void { + $this->mapper->method('find')->with(12)->willReturn($this->createTableShare()); + $this->tableMapper->method('find')->willThrowException(new DoesNotExistException('')); + $this->expectShareAuditLog( + Operation::ShareDeleted, + 'Share %s of %s %s for %s "%s" was deleted through the share review', + ['read' => true, 'create' => false, 'update' => false, 'delete' => false, 'manage' => false], + ); + + $this->shareService->deleteForShareReview(12); + } + public function testUpdatePermissionThrowsOnContextShare(): void { $share = new Share(); $share->setNodeType('context'); diff --git a/tests/unit/Service/Support/DefaultAuditLogServiceTest.php b/tests/unit/Service/Support/DefaultAuditLogServiceTest.php index 669890f7b9..0f36b07a45 100644 --- a/tests/unit/Service/Support/DefaultAuditLogServiceTest.php +++ b/tests/unit/Service/Support/DefaultAuditLogServiceTest.php @@ -7,6 +7,7 @@ namespace unit\Service\Support; +use OCA\Tables\AuditLog\Operation; use OCA\Tables\Service\Support\DefaultAuditLogService; use OCP\EventDispatcher\IEventDispatcher; use OCP\Log\Audit\CriticalActionPerformedEvent; @@ -24,14 +25,16 @@ protected function setUp(): void { } public function testLog(): void { - $message = 'Test message'; - $context = ['key' => 'value']; + $message = 'Table with ID: %s was deleted'; + $parameters = ['tableId' => 1]; $this->eventDispatcher ->expects($this->once()) ->method('dispatchTyped') - ->with($this->callback(fn (CriticalActionPerformedEvent $event) => $event->getLogMessage() === $message && $event->getParameters() === $context)); + ->with($this->callback(fn (CriticalActionPerformedEvent $event) => $event->getLogMessage() === $message + && $event->getParameters() === $parameters + && (!method_exists($event, 'getOperation') || $event->getOperation() === 'tables.table.deleted'))); - $this->service->log($message, $context); + $this->service->log(Operation::TableDeleted, $message, $parameters); } } diff --git a/tests/unit/Service/TableServiceAuditLogTest.php b/tests/unit/Service/TableServiceAuditLogTest.php new file mode 100644 index 0000000000..5445bbc6fa --- /dev/null +++ b/tests/unit/Service/TableServiceAuditLogTest.php @@ -0,0 +1,130 @@ +permissionsService = $this->createMock(PermissionsService::class); + $this->permissionsService->method('preCheckUserId')->willReturn('alice'); + $this->mapper = $this->createMock(TableMapper::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->service = new TableService( + $this->permissionsService, + $this->createMock(LoggerInterface::class), + 'alice', + $this->mapper, + $this->createMock(TableTemplateService::class), + $this->createMock(ColumnService::class), + $this->createMock(RowService::class), + $this->createMock(ViewService::class), + $this->createMock(ShareService::class), + $this->createMock(UserHelper::class), + $this->createMock(FavoritesService::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(ContextService::class), + $this->createMock(IAppManager::class), + $this->createMock(IDBConnection::class), + $this->createMock(IL10N::class), + $this->createMock(Defaults::class), + $this->createMock(ActivityManager::class), + $this->createMock(FederationService::class), + $this->auditLogService, + ); + } + + private function createTable(bool $archived): Table { + $table = new Table(); + $table->setId(5); + $table->setTitle('Budget'); + $table->setOwnership('alice'); + $table->setArchived($archived); + return $table; + } + + public function testCreateLogsCriticalAction(): void { + $this->mapper->method('insert')->willReturnCallback(function (Table $table): Table { + $table->setId(5); + return $table; + }); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::TableCreated, 'Table with ID: %s was created by user with ID: %s', [ + 'tableId' => 5, + 'owner' => 'alice', + 'template' => 'custom', + ]); + + $this->service->create('Budget', 'custom', null, '', 'alice'); + } + + public function testUpdateLogsArchiving(): void { + $this->mapper->method('find')->with(5)->willReturn($this->createTable(false)); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canUpdateTable')->willReturn(true); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::TableArchived, 'Table with ID: %s was archived', ['tableId' => 5]); + + $this->service->update(5, null, null, null, true, 'alice'); + } + + public function testUpdateLogsUnarchiving(): void { + $this->mapper->method('find')->with(5)->willReturn($this->createTable(true)); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canUpdateTable')->willReturn(true); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::TableUnarchived, 'Table with ID: %s was unarchived', ['tableId' => 5]); + + $this->service->update(5, null, null, null, false, 'alice'); + } + + public function testUpdateWithoutArchiveChangeDoesNotLog(): void { + $this->mapper->method('find')->with(5)->willReturn($this->createTable(false)); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canUpdateTable')->willReturn(true); + $this->auditLogService->expects($this->never())->method('log'); + + $this->service->update(5, 'New title', null, null, false, 'alice'); + } +} diff --git a/tests/unit/Service/ViewServiceAuditLogTest.php b/tests/unit/Service/ViewServiceAuditLogTest.php new file mode 100644 index 0000000000..4efe085810 --- /dev/null +++ b/tests/unit/Service/ViewServiceAuditLogTest.php @@ -0,0 +1,119 @@ +permissionsService = $this->createMock(PermissionsService::class); + $this->permissionsService->method('preCheckUserId')->willReturn('alice'); + $this->mapper = $this->createMock(ViewMapper::class); + $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); + + $this->service = new ViewService( + $this->permissionsService, + $this->createMock(LoggerInterface::class), + 'alice', + $this->mapper, + $this->createMock(ShareService::class), + $this->createMock(RowService::class), + $this->createMock(UserHelper::class), + $this->createMock(FavoritesService::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(ContextService::class), + $this->createMock(IL10N::class), + $this->createMock(FederationService::class), + $this->createMock(ActivityManager::class), + $this->auditLogService, + ); + } + + private function createView(): View { + $view = new View(); + $view->setId(8); + $view->setTableId(5); + $view->setTitle('Open items'); + $view->setTechnicalName('view_8'); + return $view; + } + + public function testCreateLogsCriticalAction(): void { + $table = new Table(); + $table->setId(5); + $table->setOwnership('alice'); + $this->permissionsService->method('canManageTable')->willReturn(true); + $this->mapper->method('insert')->willReturnCallback(function (View $view): View { + $view->setId(8); + $view->setTechnicalName('view_8'); + return $view; + }); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ViewCreated, 'View with ID: %s was created on table with ID: %s', [ + 'viewId' => 8, + 'tableId' => 5, + ]); + + $this->service->create('Open items', null, $table, 'alice'); + } + + public function testUpdateFilterLogsCriticalAction(): void { + $this->mapper->method('find')->with(8)->willReturn($this->createView()); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canManageView')->willReturn(true); + $this->auditLogService->expects($this->once()) + ->method('log') + ->with(Operation::ViewUpdated, 'View with ID: %s of table with ID: %s was updated', [ + 'viewId' => 8, + 'tableId' => 5, + 'changedSettings' => ['filter'], + ]); + + $this->service->update(8, new ViewUpdateInput(filterSet: FilterSet::createFromInputArray([])), 'alice', true); + } + + public function testUpdateTitleDoesNotLog(): void { + $this->mapper->method('find')->with(8)->willReturn($this->createView()); + $this->mapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canManageView')->willReturn(true); + $this->auditLogService->expects($this->never())->method('log'); + + $this->service->update(8, new ViewUpdateInput(title: new Title('Renamed')), 'alice', true); + } +}