Skip to content

Commit 4f7ae31

Browse files
skjnldsvskjnldsv
authored
Merge pull request #36817 from nextcloud/backport/36814/stable23
[stable23] Validate the scope when validating operations
2 parents 17c1183 + 769ca9c commit 4f7ae31

File tree

2 files changed

+194
-8
lines changed

2 files changed

+194
-8
lines changed

‎apps/workflowengine/lib/Manager.php‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ public function getAllConfiguredScopesForOperation(string $operationClass): arra
189189
return $scopesByOperation[$operationClass];
190190
}
191191

192+
try {
193+
/** @var IOperation $operation */
194+
$operation = $this->container->query($operationClass);
195+
} catch (QueryException $e) {
196+
return [];
197+
}
198+
192199
$query = $this->connection->getQueryBuilder();
193200

194201
$query->selectDistinct('s.type')
@@ -203,6 +210,11 @@ public function getAllConfiguredScopesForOperation(string $operationClass): arra
203210
$scopesByOperation[$operationClass] = [];
204211
while ($row = $result->fetch()) {
205212
$scope = new ScopeContext($row['type'], $row['value']);
213+
214+
if (!$operation->isAvailableForScope((int) $row['type'])) {
215+
continue;
216+
}
217+
206218
$scopesByOperation[$operationClass][$scope->getHash()] = $scope;
207219
}
208220

@@ -232,6 +244,17 @@ public function getAllOperations(ScopeContext $scopeContext): array {
232244

233245
$this->operations[$scopeContext->getHash()] = [];
234246
while ($row = $result->fetch()) {
247+
try {
248+
/** @var IOperation $operation */
249+
$operation = $this->container->query($row['class']);
250+
} catch (QueryException $e) {
251+
continue;
252+
}
253+
254+
if (!$operation->isAvailableForScope((int) $row['scope_type'])) {
255+
continue;
256+
}
257+
235258
if (!isset($this->operations[$scopeContext->getHash()][$row['class']])) {
236259
$this->operations[$scopeContext->getHash()][$row['class']] = [];
237260
}
@@ -310,7 +333,7 @@ public function addOperation(
310333
string $entity,
311334
array $events
312335
) {
313-
$this->validateOperation($class, $name, $checks, $operation, $entity, $events);
336+
$this->validateOperation($class, $name, $checks, $operation, $scope, $entity, $events);
314337

315338
$this->connection->beginTransaction();
316339

@@ -382,7 +405,7 @@ public function updateOperation(
382405
throw new \DomainException('Target operation not within scope');
383406
};
384407
$row = $this->getOperation($id);
385-
$this->validateOperation($row['class'], $name, $checks, $operation, $entity, $events);
408+
$this->validateOperation($row['class'], $name, $checks, $operation, $scopeContext, $entity, $events);
386409

387410
$checkIds = [];
388411
try {
@@ -482,9 +505,12 @@ protected function validateEvents(string $entity, array $events, IOperation $ope
482505
* @param string $name
483506
* @param array[] $checks
484507
* @param string $operation
508+
* @param ScopeContext $scope
509+
* @param string $entity
510+
* @param array $events
485511
* @throws \UnexpectedValueException
486512
*/
487-
public function validateOperation($class, $name, array $checks, $operation, string $entity, array $events) {
513+
public function validateOperation($class, $name, array $checks, $operation, ScopeContext $scope, string $entity, array $events) {
488514
try {
489515
/** @var IOperation $instance */
490516
$instance = $this->container->query($class);
@@ -496,6 +522,10 @@ public function validateOperation($class, $name, array $checks, $operation, stri
496522
throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class]));
497523
}
498524

525+
if (!$instance->isAvailableForScope($scope->getScope())) {
526+
throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class]));
527+
}
528+
499529
$this->validateEvents($entity, $events, $instance);
500530

501531
if (count($checks) === 0) {

‎apps/workflowengine/tests/ManagerTest.php‎

Lines changed: 161 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use OCA\WorkflowEngine\Entity\File;
3131
use OCA\WorkflowEngine\Helper\ScopeContext;
3232
use OCA\WorkflowEngine\Manager;
33+
use OCP\AppFramework\QueryException;
3334
use OCP\EventDispatcher\IEventDispatcher;
3435
use OCP\Files\IRootFolder;
3536
use OCP\IConfig;
@@ -199,6 +200,32 @@ public function testGetAllOperations() {
199200
$userScope = $this->buildScope('jackie');
200201
$entity = File::class;
201202

203+
$adminOperation = $this->createMock(IOperation::class);
204+
$adminOperation->expects($this->any())
205+
->method('isAvailableForScope')
206+
->willReturnMap([
207+
[IManager::SCOPE_ADMIN, true],
208+
[IManager::SCOPE_USER, false],
209+
]);
210+
$userOperation = $this->createMock(IOperation::class);
211+
$userOperation->expects($this->any())
212+
->method('isAvailableForScope')
213+
->willReturnMap([
214+
[IManager::SCOPE_ADMIN, false],
215+
[IManager::SCOPE_USER, true],
216+
]);
217+
218+
$this->container->expects($this->any())
219+
->method('query')
220+
->willReturnCallback(function ($className) use ($adminOperation, $userOperation) {
221+
switch ($className) {
222+
case 'OCA\WFE\TestAdminOp':
223+
return $adminOperation;
224+
case 'OCA\WFE\TestUserOp':
225+
return $userOperation;
226+
}
227+
});
228+
202229
$opId1 = $this->invokePrivate(
203230
$this->manager,
204231
'insertOperation',
@@ -219,6 +246,13 @@ public function testGetAllOperations() {
219246
);
220247
$this->invokePrivate($this->manager, 'addScope', [$opId3, $userScope]);
221248

249+
$opId4 = $this->invokePrivate(
250+
$this->manager,
251+
'insertOperation',
252+
['OCA\WFE\TestAdminOp', 'Test04', [41, 10, 4], 'NoBar', $entity, []]
253+
);
254+
$this->invokePrivate($this->manager, 'addScope', [$opId4, $userScope]);
255+
222256
$adminOps = $this->manager->getAllOperations($adminScope);
223257
$userOps = $this->manager->getAllOperations($userScope);
224258

@@ -269,6 +303,25 @@ public function testGetOperations() {
269303
);
270304
$this->invokePrivate($this->manager, 'addScope', [$opId5, $userScope]);
271305

306+
$operation = $this->createMock(IOperation::class);
307+
$operation->expects($this->any())
308+
->method('isAvailableForScope')
309+
->willReturnMap([
310+
[IManager::SCOPE_ADMIN, true],
311+
[IManager::SCOPE_USER, true],
312+
]);
313+
314+
$this->container->expects($this->any())
315+
->method('query')
316+
->willReturnCallback(function ($className) use ($operation) {
317+
switch ($className) {
318+
case 'OCA\WFE\TestOp':
319+
return $operation;
320+
case 'OCA\WFE\OtherTestOp':
321+
throw new QueryException();
322+
}
323+
});
324+
272325
$adminOps = $this->manager->getOperations('OCA\WFE\TestOp', $adminScope);
273326
$userOps = $this->manager->getOperations('OCA\WFE\TestOp', $userScope);
274327

@@ -288,11 +341,20 @@ public function testUpdateOperation() {
288341
$userScope = $this->buildScope('jackie');
289342
$entity = File::class;
290343

344+
$operationMock = $this->createMock(IOperation::class);
345+
$operationMock->expects($this->any())
346+
->method('isAvailableForScope')
347+
->withConsecutive(
348+
[IManager::SCOPE_ADMIN],
349+
[IManager::SCOPE_USER]
350+
)
351+
->willReturn(true);
352+
291353
$this->container->expects($this->any())
292354
->method('query')
293-
->willReturnCallback(function ($class) {
355+
->willReturnCallback(function ($class) use ($operationMock) {
294356
if (substr($class, -2) === 'Op') {
295-
return $this->createMock(IOperation::class);
357+
return $operationMock;
296358
} elseif ($class === File::class) {
297359
return $this->getMockBuilder(File::class)
298360
->setConstructorArgs([
@@ -453,6 +515,16 @@ public function testValidateOperationOK() {
453515
$entityMock = $this->createMock(IEntity::class);
454516
$eventEntityMock = $this->createMock(IEntityEvent::class);
455517
$checkMock = $this->createMock(ICheck::class);
518+
$scopeMock = $this->createMock(ScopeContext::class);
519+
520+
$scopeMock->expects($this->any())
521+
->method('getScope')
522+
->willReturn(IManager::SCOPE_ADMIN);
523+
524+
$operationMock->expects($this->once())
525+
->method('isAvailableForScope')
526+
->with(IManager::SCOPE_ADMIN)
527+
->willReturn(true);
456528

457529
$operationMock->expects($this->once())
458530
->method('validateOperation')
@@ -489,7 +561,7 @@ public function testValidateOperationOK() {
489561
}
490562
});
491563

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

495567
public function testValidateOperationCheckInputLengthError() {
@@ -503,6 +575,16 @@ public function testValidateOperationCheckInputLengthError() {
503575
$entityMock = $this->createMock(IEntity::class);
504576
$eventEntityMock = $this->createMock(IEntityEvent::class);
505577
$checkMock = $this->createMock(ICheck::class);
578+
$scopeMock = $this->createMock(ScopeContext::class);
579+
580+
$scopeMock->expects($this->any())
581+
->method('getScope')
582+
->willReturn(IManager::SCOPE_ADMIN);
583+
584+
$operationMock->expects($this->once())
585+
->method('isAvailableForScope')
586+
->with(IManager::SCOPE_ADMIN)
587+
->willReturn(true);
506588

507589
$operationMock->expects($this->once())
508590
->method('validateOperation')
@@ -540,7 +622,7 @@ public function testValidateOperationCheckInputLengthError() {
540622
});
541623

542624
try {
543-
$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', IEntity::class, ['MyEvent']);
625+
$this->manager->validateOperation(IOperation::class, 'test', [$check], 'operationData', $scopeMock, IEntity::class, ['MyEvent']);
544626
} catch (\UnexpectedValueException $e) {
545627
$this->assertSame('The provided check value is too long', $e->getMessage());
546628
}
@@ -558,6 +640,16 @@ public function testValidateOperationDataLengthError() {
558640
$entityMock = $this->createMock(IEntity::class);
559641
$eventEntityMock = $this->createMock(IEntityEvent::class);
560642
$checkMock = $this->createMock(ICheck::class);
643+
$scopeMock = $this->createMock(ScopeContext::class);
644+
645+
$scopeMock->expects($this->any())
646+
->method('getScope')
647+
->willReturn(IManager::SCOPE_ADMIN);
648+
649+
$operationMock->expects($this->once())
650+
->method('isAvailableForScope')
651+
->with(IManager::SCOPE_ADMIN)
652+
->willReturn(true);
561653

562654
$operationMock->expects($this->never())
563655
->method('validateOperation');
@@ -594,9 +686,73 @@ public function testValidateOperationDataLengthError() {
594686
});
595687

596688
try {
597-
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, IEntity::class, ['MyEvent']);
689+
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, $scopeMock, IEntity::class, ['MyEvent']);
598690
} catch (\UnexpectedValueException $e) {
599691
$this->assertSame('The provided operation data is too long', $e->getMessage());
600692
}
601693
}
694+
695+
public function testValidateOperationScopeNotAvailable() {
696+
$check = [
697+
'class' => ICheck::class,
698+
'operator' => 'is',
699+
'value' => 'barfoo',
700+
];
701+
$operationData = str_pad('', IManager::MAX_OPERATION_VALUE_BYTES + 1, 'FooBar');
702+
703+
$operationMock = $this->createMock(IOperation::class);
704+
$entityMock = $this->createMock(IEntity::class);
705+
$eventEntityMock = $this->createMock(IEntityEvent::class);
706+
$checkMock = $this->createMock(ICheck::class);
707+
$scopeMock = $this->createMock(ScopeContext::class);
708+
709+
$scopeMock->expects($this->any())
710+
->method('getScope')
711+
->willReturn(IManager::SCOPE_ADMIN);
712+
713+
$operationMock->expects($this->once())
714+
->method('isAvailableForScope')
715+
->with(IManager::SCOPE_ADMIN)
716+
->willReturn(false);
717+
718+
$operationMock->expects($this->never())
719+
->method('validateOperation');
720+
721+
$entityMock->expects($this->any())
722+
->method('getEvents')
723+
->willReturn([$eventEntityMock]);
724+
725+
$eventEntityMock->expects($this->any())
726+
->method('getEventName')
727+
->willReturn('MyEvent');
728+
729+
$checkMock->expects($this->any())
730+
->method('supportedEntities')
731+
->willReturn([IEntity::class]);
732+
$checkMock->expects($this->never())
733+
->method('validateCheck');
734+
735+
$this->container->expects($this->any())
736+
->method('query')
737+
->willReturnCallback(function ($className) use ($operationMock, $entityMock, $eventEntityMock, $checkMock) {
738+
switch ($className) {
739+
case IOperation::class:
740+
return $operationMock;
741+
case IEntity::class:
742+
return $entityMock;
743+
case IEntityEvent::class:
744+
return $eventEntityMock;
745+
case ICheck::class:
746+
return $checkMock;
747+
default:
748+
return $this->createMock($className);
749+
}
750+
});
751+
752+
try {
753+
$this->manager->validateOperation(IOperation::class, 'test', [$check], $operationData, $scopeMock, IEntity::class, ['MyEvent']);
754+
} catch (\UnexpectedValueException $e) {
755+
$this->assertSame('Operation OCP\WorkflowEngine\IOperation is invalid', $e->getMessage());
756+
}
757+
}
602758
}

0 commit comments

Comments
 (0)