Skip to content

Commit

Permalink
Merge pull request #36820 from nextcloud/backport/36814/stable20
Browse files Browse the repository at this point in the history 
[stable20] Validate the scope when validating operations
  • Loading branch information
@skjnldsv
skjnldsv authored Feb 23, 2023
2 parents 50f7433 + b24a18a commit fbc5007
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 8 deletions.
36 changes: 33 additions & 3 deletions apps/workflowengine/lib/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ public function getAllConfiguredScopesForOperation(string $operationClass): arra
return $scopesByOperation[$operationClass];
}

try {
/** @var IOperation $operation */
$operation = $this->container->query($operationClass);
} catch (QueryException $e) {
return [];
}

$query = $this->connection->getQueryBuilder();

$query->selectDistinct('s.type')
Expand All @@ -195,6 +202,11 @@ public function getAllConfiguredScopesForOperation(string $operationClass): arra
$scopesByOperation[$operationClass] = [];
while ($row = $result->fetch()) {
$scope = new ScopeContext($row['type'], $row['value']);

if (!$operation->isAvailableForScope((int) $row['type'])) {
continue;
}

$scopesByOperation[$operationClass][$scope->getHash()] = $scope;
}

Expand Down Expand Up @@ -224,6 +236,17 @@ public function getAllOperations(ScopeContext $scopeContext): array {

$this->operations[$scopeContext->getHash()] = [];
while ($row = $result->fetch()) {
try {
/** @var IOperation $operation */
$operation = $this->container->query($row['class']);
} catch (QueryException $e) {
continue;
}

if (!$operation->isAvailableForScope((int) $row['scope_type'])) {
continue;
}

if (!isset($this->operations[$scopeContext->getHash()][$row['class']])) {
$this->operations[$scopeContext->getHash()][$row['class']] = [];
}
Expand Down Expand Up @@ -302,7 +325,7 @@ public function addOperation(
string $entity,
array $events
) {
$this->validateOperation($class, $name, $checks, $operation, $entity, $events);
$this->validateOperation($class, $name, $checks, $operation, $scope, $entity, $events);

$this->connection->beginTransaction();

Expand Down Expand Up @@ -374,7 +397,7 @@ public function updateOperation(
throw new \DomainException('Target operation not within scope');
};
$row = $this->getOperation($id);
$this->validateOperation($row['class'], $name, $checks, $operation, $entity, $events);
$this->validateOperation($row['class'], $name, $checks, $operation, $scopeContext, $entity, $events);

$checkIds = [];
try {
Expand Down Expand Up @@ -474,9 +497,12 @@ protected function validateEvents(string $entity, array $events, IOperation $ope
* @param string $name
* @param array[] $checks
* @param string $operation
* @param ScopeContext $scope
* @param string $entity
* @param array $events
* @throws \UnexpectedValueException
*/
public function validateOperation($class, $name, array $checks, $operation, string $entity, array $events) {
public function validateOperation($class, $name, array $checks, $operation, ScopeContext $scope, string $entity, array $events) {
try {
/** @var IOperation $instance */
$instance = $this->container->query($class);
Expand All @@ -488,6 +514,10 @@ public function validateOperation($class, $name, array $checks, $operation, stri
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]));
}

$this->validateEvents($entity, $events, $instance);

if (count($checks) === 0) {
Expand Down
166 changes: 161 additions & 5 deletions apps/workflowengine/tests/ManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use OCA\WorkflowEngine\Entity\File;
use OCA\WorkflowEngine\Helper\ScopeContext;
use OCA\WorkflowEngine\Manager;
use OCP\AppFramework\QueryException;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\IRootFolder;
use OCP\IConfig;
Expand Down Expand Up @@ -194,6 +195,32 @@ public function testGetAllOperations() {
$userScope = $this->buildScope('jackie');
$entity = File::class;

$adminOperation = $this->createMock(IOperation::class);
$adminOperation->expects($this->any())
->method('isAvailableForScope')
->willReturnMap([
[IManager::SCOPE_ADMIN, true],
[IManager::SCOPE_USER, false],
]);
$userOperation = $this->createMock(IOperation::class);
$userOperation->expects($this->any())
->method('isAvailableForScope')
->willReturnMap([
[IManager::SCOPE_ADMIN, false],
[IManager::SCOPE_USER, true],
]);

$this->container->expects($this->any())
->method('query')
->willReturnCallback(function ($className) use ($adminOperation, $userOperation) {
switch ($className) {
case 'OCA\WFE\TestAdminOp':
return $adminOperation;
case 'OCA\WFE\TestUserOp':
return $userOperation;
}
});

$opId1 = $this->invokePrivate(
$this->manager,
'insertOperation',
Expand All @@ -214,6 +241,13 @@ public function testGetAllOperations() {
);
$this->invokePrivate($this->manager, 'addScope', [$opId3, $userScope]);

$opId4 = $this->invokePrivate(
$this->manager,
'insertOperation',
['OCA\WFE\TestAdminOp', 'Test04', [41, 10, 4], 'NoBar', $entity, []]
);
$this->invokePrivate($this->manager, 'addScope', [$opId4, $userScope]);

$adminOps = $this->manager->getAllOperations($adminScope);
$userOps = $this->manager->getAllOperations($userScope);

Expand Down Expand Up @@ -264,6 +298,25 @@ public function testGetOperations() {
);
$this->invokePrivate($this->manager, 'addScope', [$opId5, $userScope]);

$operation = $this->createMock(IOperation::class);
$operation->expects($this->any())
->method('isAvailableForScope')
->willReturnMap([
[IManager::SCOPE_ADMIN, true],
[IManager::SCOPE_USER, true],
]);

$this->container->expects($this->any())
->method('query')
->willReturnCallback(function ($className) use ($operation) {
switch ($className) {
case 'OCA\WFE\TestOp':
return $operation;
case 'OCA\WFE\OtherTestOp':
throw new QueryException();
}
});

$adminOps = $this->manager->getOperations('OCA\WFE\TestOp', $adminScope);
$userOps = $this->manager->getOperations('OCA\WFE\TestOp', $userScope);

Expand All @@ -283,11 +336,20 @@ public function testUpdateOperation() {
$userScope = $this->buildScope('jackie');
$entity = File::class;

$operationMock = $this->createMock(IOperation::class);
$operationMock->expects($this->any())
->method('isAvailableForScope')
->withConsecutive(
[IManager::SCOPE_ADMIN],
[IManager::SCOPE_USER]
)
->willReturn(true);

$this->container->expects($this->any())
->method('query')
->willReturnCallback(function ($class) {
->willReturnCallback(function ($class) use ($operationMock) {
if (substr($class, -2) === 'Op') {
return $this->createMock(IOperation::class);
return $operationMock;
} elseif ($class === File::class) {
return $this->getMockBuilder(File::class)
->setConstructorArgs([
Expand Down Expand Up @@ -448,6 +510,16 @@ public function testValidateOperationOK() {
$entityMock = $this->createMock(IEntity::class);
$eventEntityMock = $this->createMock(IEntityEvent::class);
$checkMock = $this->createMock(ICheck::class);
$scopeMock = $this->createMock(ScopeContext::class);

$scopeMock->expects($this->any())
->method('getScope')
->willReturn(IManager::SCOPE_ADMIN);

$operationMock->expects($this->once())
->method('isAvailableForScope')
->with(IManager::SCOPE_ADMIN)
->willReturn(true);

$operationMock->expects($this->once())
->method('validateOperation')
Expand Down Expand Up @@ -484,7 +556,7 @@ public function testValidateOperationOK() {
}
});

$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', IEntity::class, ['MyEvent']);
$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', $scopeMock, IEntity::class, ['MyEvent']);
}

public function testValidateOperationCheckInputLengthError() {
Expand All @@ -498,6 +570,16 @@ public function testValidateOperationCheckInputLengthError() {
$entityMock = $this->createMock(IEntity::class);
$eventEntityMock = $this->createMock(IEntityEvent::class);
$checkMock = $this->createMock(ICheck::class);
$scopeMock = $this->createMock(ScopeContext::class);

$scopeMock->expects($this->any())
->method('getScope')
->willReturn(IManager::SCOPE_ADMIN);

$operationMock->expects($this->once())
->method('isAvailableForScope')
->with(IManager::SCOPE_ADMIN)
->willReturn(true);

$operationMock->expects($this->once())
->method('validateOperation')
Expand Down Expand Up @@ -535,7 +617,7 @@ public function testValidateOperationCheckInputLengthError() {
});

try {
$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', IEntity::class, ['MyEvent']);
$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', $scopeMock, IEntity::class, ['MyEvent']);
} catch (\UnexpectedValueException $e) {
$this->assertSame('The provided check value is too long', $e->getMessage());
}
Expand All @@ -553,6 +635,16 @@ public function testValidateOperationDataLengthError() {
$entityMock = $this->createMock(IEntity::class);
$eventEntityMock = $this->createMock(IEntityEvent::class);
$checkMock = $this->createMock(ICheck::class);
$scopeMock = $this->createMock(ScopeContext::class);

$scopeMock->expects($this->any())
->method('getScope')
->willReturn(IManager::SCOPE_ADMIN);

$operationMock->expects($this->once())
->method('isAvailableForScope')
->with(IManager::SCOPE_ADMIN)
->willReturn(true);

$operationMock->expects($this->never())
->method('validateOperation');
Expand Down Expand Up @@ -589,9 +681,73 @@ public function testValidateOperationDataLengthError() {
});

try {
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, IEntity::class, ['MyEvent']);
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, $scopeMock, IEntity::class, ['MyEvent']);
} catch (\UnexpectedValueException $e) {
$this->assertSame('The provided operation data is too long', $e->getMessage());
}
}

public function testValidateOperationScopeNotAvailable() {
$check = [
'class' => ICheck::class,
'operator' => 'is',
'value' => 'barfoo',
];
$operationData = str_pad('', IManager::MAX_OPERATION_VALUE_BYTES + 1, 'FooBar');

$operationMock = $this->createMock(IOperation::class);
$entityMock = $this->createMock(IEntity::class);
$eventEntityMock = $this->createMock(IEntityEvent::class);
$checkMock = $this->createMock(ICheck::class);
$scopeMock = $this->createMock(ScopeContext::class);

$scopeMock->expects($this->any())
->method('getScope')
->willReturn(IManager::SCOPE_ADMIN);

$operationMock->expects($this->once())
->method('isAvailableForScope')
->with(IManager::SCOPE_ADMIN)
->willReturn(false);

$operationMock->expects($this->never())
->method('validateOperation');

$entityMock->expects($this->any())
->method('getEvents')
->willReturn([$eventEntityMock]);

$eventEntityMock->expects($this->any())
->method('getEventName')
->willReturn('MyEvent');

$checkMock->expects($this->any())
->method('supportedEntities')
->willReturn([IEntity::class]);
$checkMock->expects($this->never())
->method('validateCheck');

$this->container->expects($this->any())
->method('query')
->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) {
switch ($className) {
case IOperation::class:
return $operationMock;
case IEntity::class:
return $entityMock;
case IEntityEvent::class:
return $eventEntityMock;
case ICheck::class:
return $checkMock;
default:
return $this->createMock($className);
}
});

try {
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, $scopeMock, IEntity::class, ['MyEvent']);
} catch (\UnexpectedValueException $e) {
$this->assertSame('Operation OCP\WorkflowEngine\IOperation is invalid', $e->getMessage());
}
}
}

0 comments on commit fbc5007

Please sign in to comment.