diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php index 44d4c71b4f0f0..bbc76d9894f32 100644 --- a/apps/workflowengine/lib/Manager.php +++ b/apps/workflowengine/lib/Manager.php @@ -422,16 +422,18 @@ public function deleteOperation(int $id, ScopeContext $scopeContext): bool { * @param array $events */ protected function validateEvents(string $entity, array $events, IOperation $operation): void { + /** @psalm-suppress TaintedCallable newInstance is not called */ + $reflection = new \ReflectionClass($entity); + if ($entity !== IEntity::class && !in_array(IEntity::class, $reflection->getInterfaceNames())) { + throw new \UnexpectedValueException($this->l->t('Entity %s is invalid', [$entity])); + } + try { $instance = $this->container->get($entity); } catch (ContainerExceptionInterface $e) { throw new \UnexpectedValueException($this->l->t('Entity %s does not exist', [$entity])); } - if (!$instance instanceof IEntity) { - throw new \UnexpectedValueException($this->l->t('Entity %s is invalid', [$entity])); - } - if (empty($events)) { if (!$operation instanceof IComplexOperation) { throw new \UnexpectedValueException($this->l->t('No events are chosen.')); @@ -458,6 +460,16 @@ protected function validateEvents(string $entity, array $events, IOperation $ope * @throws \UnexpectedValueException */ public function validateOperation(string $class, string $name, array $checks, string $operation, ScopeContext $scope, string $entity, array $events): void { + if (strlen($operation) > IManager::MAX_OPERATION_VALUE_BYTES) { + throw new \UnexpectedValueException($this->l->t('The provided operation data is too long')); + } + + /** @psalm-suppress TaintedCallable newInstance is not called */ + $reflection = new \ReflectionClass($class); + if ($class !== IOperation::class && !in_array(IOperation::class, $reflection->getInterfaceNames())) { + throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class]) . join(', ', $reflection->getInterfaceNames())); + } + try { /** @var IOperation $instance */ $instance = $this->container->get($class); @@ -465,10 +477,6 @@ public function validateOperation(string $class, string $name, array $checks, st throw new \UnexpectedValueException($this->l->t('Operation %s does not exist', [$class])); } - if (!($instance instanceof IOperation)) { - throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class])); - } - if (!$instance->isAvailableForScope($scope->getScope())) { throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class])); } @@ -479,10 +487,6 @@ public function validateOperation(string $class, string $name, array $checks, st throw new \UnexpectedValueException($this->l->t('At least one check needs to be provided')); } - if (strlen($operation) > IManager::MAX_OPERATION_VALUE_BYTES) { - throw new \UnexpectedValueException($this->l->t('The provided operation data is too long')); - } - $instance->validateOperation($name, $checks, $operation); foreach ($checks as $check) { @@ -490,6 +494,15 @@ public function validateOperation(string $class, string $name, array $checks, st throw new \UnexpectedValueException($this->l->t('Invalid check provided')); } + if (strlen((string)$check['value']) > IManager::MAX_CHECK_VALUE_BYTES) { + throw new \UnexpectedValueException($this->l->t('The provided check value is too long')); + } + + $reflection = new \ReflectionClass($check['class']); + if ($check['class'] !== ICheck::class && !in_array(ICheck::class, $reflection->getInterfaceNames())) { + throw new \UnexpectedValueException($this->l->t('Check %s is invalid', [$class])); + } + try { /** @var ICheck $instance */ $instance = $this->container->get($check['class']); @@ -497,20 +510,12 @@ public function validateOperation(string $class, string $name, array $checks, st throw new \UnexpectedValueException($this->l->t('Check %s does not exist', [$class])); } - if (!($instance instanceof ICheck)) { - throw new \UnexpectedValueException($this->l->t('Check %s is invalid', [$class])); - } - if (!empty($instance->supportedEntities()) && !in_array($entity, $instance->supportedEntities()) ) { throw new \UnexpectedValueException($this->l->t('Check %s is not allowed with this entity', [$class])); } - if (strlen((string)$check['value']) > IManager::MAX_CHECK_VALUE_BYTES) { - throw new \UnexpectedValueException($this->l->t('The provided check value is too long')); - } - $instance->validateCheck($check['operator'], $check['value']); } } diff --git a/apps/workflowengine/tests/ManagerTest.php b/apps/workflowengine/tests/ManagerTest.php index 3f2e5064fbe32..c32367738e047 100644 --- a/apps/workflowengine/tests/ManagerTest.php +++ b/apps/workflowengine/tests/ManagerTest.php @@ -12,6 +12,7 @@ use OCA\WorkflowEngine\Manager; use OCP\AppFramework\QueryException; use OCP\AppFramework\Services\IAppConfig; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\NodeCreatedEvent; use OCP\Files\IRootFolder; @@ -31,11 +32,42 @@ use OCP\WorkflowEngine\IEntityEvent; use OCP\WorkflowEngine\IManager; use OCP\WorkflowEngine\IOperation; +use OCP\WorkflowEngine\IRuleMatcher; use PHPUnit\Framework\MockObject\MockObject; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; +class TestAdminOp implements IOperation { + public function getDisplayName(): string { + return 'Admin'; + } + + public function getDescription(): string { + return ''; + } + + public function getIcon(): string { + return ''; + } + + public function isAvailableForScope(int $scope): bool { + return true; + } + + public function validateOperation(string $name, array $checks, string $operation): void { + } + + public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatcher): void { + } +} + +class TestUserOp extends TestAdminOp { + public function getDisplayName(): string { + return 'User'; + } +} + /** * Class ManagerTest * @@ -393,19 +425,19 @@ public function testUpdateOperation(): void { $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo', $entity, []] + [TestAdminOp::class, 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar', $entity, []] + [TestUserOp::class, 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]); - $check1 = ['class' => 'OCA\WFE\C22', 'operator' => 'eq', 'value' => 'asdf']; - $check2 = ['class' => 'OCA\WFE\C33', 'operator' => 'eq', 'value' => 23456]; + $check1 = ['class' => ICheck::class, 'operator' => 'eq', 'value' => 'asdf']; + $check2 = ['class' => ICheck::class, 'operator' => 'eq', 'value' => 23456]; /** @noinspection PhpUnhandledExceptionInspection */ $op = $this->manager->updateOperation($opId1, 'Test01a', [$check1, $check2], 'foohur', $adminScope, $entity, ['\OCP\Files::postDelete']); @@ -667,11 +699,6 @@ public function testValidateOperationDataLengthError(): void { ->method('getScope') ->willReturn(IManager::SCOPE_ADMIN); - $operationMock->expects($this->once()) - ->method('isAvailableForScope') - ->with(IManager::SCOPE_ADMIN) - ->willReturn(true); - $operationMock->expects($this->never()) ->method('validateOperation'); @@ -719,7 +746,7 @@ public function testValidateOperationScopeNotAvailable(): void { 'operator' => 'is', 'value' => 'barfoo', ]; - $operationData = str_pad('', IManager::MAX_OPERATION_VALUE_BYTES + 1, 'FooBar'); + $operationData = str_pad('', IManager::MAX_OPERATION_VALUE_BYTES - 1, 'FooBar'); $operationMock = $this->createMock(IOperation::class); $entityMock = $this->createMock(IEntity::class);