diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php index 7d14fc834491e..3a1ebf427723c 100644 --- a/apps/workflowengine/lib/Manager.php +++ b/apps/workflowengine/lib/Manager.php @@ -1,4 +1,5 @@ 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.')); @@ -457,6 +454,16 @@ protected function validateEvents(string $entity, array $events, IOperation $ope * @throws \UnexpectedValueException */ public function validateOperation($class, $name, array $checks, $operation, ScopeContext $scope, string $entity, array $events) { + 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->query($class); @@ -464,10 +471,6 @@ public function validateOperation($class, $name, array $checks, $operation, Scop 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])); } @@ -489,6 +492,15 @@ public function validateOperation($class, $name, array $checks, $operation, Scop 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->query($check['class']); @@ -496,20 +508,12 @@ public function validateOperation($class, $name, array $checks, $operation, Scop 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 610a7c60f1e71..da6e373a3a024 100644 --- a/apps/workflowengine/tests/ManagerTest.php +++ b/apps/workflowengine/tests/ManagerTest.php @@ -11,6 +11,7 @@ use OCA\WorkflowEngine\Helper\ScopeContext; use OCA\WorkflowEngine\Manager; use OCP\AppFramework\QueryException; +use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\NodeCreatedEvent; use OCP\Files\IRootFolder; @@ -31,10 +32,41 @@ use OCP\WorkflowEngine\IEntityEvent; use OCP\WorkflowEngine\IManager; use OCP\WorkflowEngine\IOperation; +use OCP\WorkflowEngine\IRuleMatcher; use PHPUnit\Framework\MockObject\MockObject; 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 * @@ -405,19 +437,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']); @@ -680,11 +712,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'); @@ -732,7 +759,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);