diff --git a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php index 09c932b5..ec9a2ae6 100644 --- a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php @@ -91,7 +91,7 @@ use FreeDSx\Ldap\Server\Logging\EventLogger; use FreeDSx\Ldap\Server\Metrics\MetricsRecorderInterface; use FreeDSx\Ldap\Server\Logging\OperationAuditor; -use FreeDSx\Ldap\Server\Middleware\AssertionMiddleware; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Server\Middleware\CriticalControlMiddleware; use FreeDSx\Ldap\Server\Middleware\OperationAuditMiddleware; use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareChain; @@ -306,11 +306,11 @@ private function makeWriteRequestReplayer(Container $container): WriteRequestRep ? [new ReadOnlyMiddleware($consumerConfig)] : []), $container->get(CriticalControlMiddleware::class), - $container->get(AssertionMiddleware::class), ], - new ReplayWriteHandler(new WriteRequestRouter( - $container->get(WriteOperationDispatcher::class), - )), + new ReplayWriteHandler( + new WriteRequestRouter($container->get(WriteOperationDispatcher::class)), + $container->get(AssertionEvaluator::class), + ), )); } diff --git a/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php index 6effbf8a..a0f5b482 100644 --- a/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php @@ -25,6 +25,7 @@ use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerPasswordModifyHandler; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerPasswordPolicyForwardHandler; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerProtocolHandlerInterface; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\GeneratedEntryResponder; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerRootDseHandler; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerSearchHandler; @@ -231,6 +232,7 @@ private function makeDispatchHandler( ), ), accessControl: $container->get(AccessControlInterface::class), + assertions: $container->get(AssertionEvaluator::class), schema: $container->get(ServerOptions::class)->getSchema(), ); } diff --git a/src/FreeDSx/Ldap/LdapServer.php b/src/FreeDSx/Ldap/LdapServer.php index 0e2bf01a..a1a8cc65 100644 --- a/src/FreeDSx/Ldap/LdapServer.php +++ b/src/FreeDSx/Ldap/LdapServer.php @@ -18,6 +18,7 @@ use FreeDSx\Ldap\Exception\LdifParseException; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Exception\RuntimeException; +use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageBusyException; use FreeDSx\Ldap\Ldif\LdifChangeRecord; use FreeDSx\Ldap\Ldif\LdifParser; use FreeDSx\Ldap\Ldif\Url\LdifUrlResolverInterface; @@ -84,6 +85,7 @@ public function getOptions(): ServerOptions * @throws RuntimeException when the LDIF contains a non-add change record * @throws InvalidArgumentException when the creator DN is malformed * @throws OperationException when an entry is refused, such as one violating the schema or missing its parent + * @throws StorageBusyException when a write keeps conflicting with concurrent writes */ public function seed( LdifLoaderInterface $loader, @@ -102,6 +104,7 @@ public function seed( * * @throws InvalidArgumentException when the creator DN is malformed * @throws OperationException when an entry is refused, such as one violating the schema or missing its parent + * @throws StorageBusyException when a write keeps conflicting with concurrent writes */ public function seedEntries( iterable $entries, @@ -126,6 +129,7 @@ public function seedEntries( * * @throws LdifParseException when the LDIF cannot be parsed * @throws OperationException when a write fails (no such entry, schema violation, etc.) + * @throws StorageBusyException when a write keeps conflicting with concurrent writes */ public function applyChanges( LdifLoaderInterface $loader, diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php index ff854239..8dbd3347 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php @@ -17,6 +17,7 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; @@ -55,18 +56,37 @@ public function assertSatisfied( ControlBag $controls, TokenInterface $token, ): void { - $control = $controls->get(Control::OID_ASSERTION); - - if (!$control instanceof AssertionControl) { + if ($this->assertionIn($controls) === null) { return; } $entry = $this->backend->get($targetDn); - if ($entry === null) { return; } + $this->assertSatisfiedBy( + $entry, + $controls, + $token, + ); + } + + /** + * Throws ASSERTION_FAILED when an assertion control is present and its filter does not match the given entry. + * + * @throws OperationException + */ + public function assertSatisfiedBy( + Entry $entry, + ControlBag $controls, + TokenInterface $token, + ): void { + $control = $this->assertionIn($controls); + if ($control === null) { + return; + } + $readable = $this->accessControl->stripUnreadableAttributes( $token, $entry, @@ -81,4 +101,18 @@ public function assertSatisfied( ResultCode::ASSERTION_FAILED, ); } + + /** + * A replayed change record carries its controls undecoded, so the assertion filter is decoded here when needed. + */ + private function assertionIn(ControlBag $controls): ?AssertionControl + { + $control = $controls->get(Control::OID_ASSERTION); + + return match (true) { + $control === null => null, + $control instanceof AssertionControl => $control, + default => AssertionControl::fromAsn1($control->toAsn1()), + }; + } } diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php index a58583b1..328c9155 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php @@ -19,17 +19,13 @@ use FreeDSx\Ldap\Control\ReadEntry\PreReadResponseControl; use FreeDSx\Ldap\Control\ReadEntry\ReadEntryControl; use FreeDSx\Ldap\Entry\Attribute; -use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; -use FreeDSx\Ldap\Operation\Request; use FreeDSx\Ldap\Schema\Schema; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; -use FreeDSx\Ldap\Server\AccessControl\OperationTargetDn; -use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Token\TokenInterface; /** - * Builds RFC 4527 Pre-Read / Post-Read response controls from the entry state around a write. + * Builds RFC 4527 Pre-Read / Post-Read response controls from the entries a write kept under its lock. * * The entry goes back to the client, so read policy applies to it as it would to a search result: a write privilege * over an entry must not disclose what the identity may not read. @@ -44,90 +40,59 @@ final readonly class ReadEntryControlHandler { public function __construct( - private ReadBackendInterface $backend, private Schema $schema, private AccessControlInterface $accessControl, ) {} /** - * Build the pre-read control for a write, reading the target before the change (not applicable to Add). + * The pre-read control for the entry as the write found it, or null when there is none to return. */ - public function preReadFor( - Request\RequestInterface $request, - ControlBag $controls, - TokenInterface $token, - ): ?PreReadResponseControl { - $dn = $this->preReadDn($request); - - return $dn !== null - ? $this->preRead($dn, $controls, $token) - : null; - } - - /** - * Build the post-read control for a write, reading the target after the change (not applicable to Delete). - */ - public function postReadFor( - Request\RequestInterface $request, - ControlBag $controls, - TokenInterface $token, - ): ?PostReadResponseControl { - $dn = $this->postReadDn($request); - - return $dn !== null - ? $this->postRead($dn, $controls, $token) - : null; - } - public function preRead( - Dn $dn, + ?Entry $entry, ControlBag $controls, TokenInterface $token, ): ?PreReadResponseControl { - $entry = $this->readEntry( + $readable = $this->readEntry( Control::OID_PRE_READ, - $dn, + $entry, $controls, $token, ); - return $entry !== null - ? new PreReadResponseControl($entry) + return $readable !== null + ? new PreReadResponseControl($readable) : null; } + /** + * The post-read control for the entry as the write stored it, or null when there is none to return. + */ public function postRead( - Dn $dn, + ?Entry $entry, ControlBag $controls, TokenInterface $token, ): ?PostReadResponseControl { - $entry = $this->readEntry( + $readable = $this->readEntry( Control::OID_POST_READ, - $dn, + $entry, $controls, $token, ); - return $entry !== null - ? new PostReadResponseControl($entry) + return $readable !== null + ? new PostReadResponseControl($readable) : null; } private function readEntry( string $oid, - Dn $dn, + ?Entry $entry, ControlBag $controls, TokenInterface $token, ): ?Entry { $control = $controls->get($oid); - if (!$control instanceof ReadEntryControl) { - return null; - } - - $entry = $this->backend->get($dn); - - if ($entry === null) { + if (!$control instanceof ReadEntryControl || $entry === null) { return null; } @@ -152,24 +117,4 @@ private function readEntry( // Make a copy so live references don't leak. return $projection->project($readable)->makeCopy(); } - - private function preReadDn(Request\RequestInterface $request): ?Dn - { - return match (true) { - $request instanceof Request\DeleteRequest, - $request instanceof Request\ModifyRequest, - $request instanceof Request\ModifyDnRequest => $request->getDn(), - default => null, - }; - } - - private function postReadDn(Request\RequestInterface $request): ?Dn - { - return match (true) { - $request instanceof Request\AddRequest => $request->getEntry()->getDn(), - $request instanceof Request\ModifyRequest => $request->getDn(), - $request instanceof Request\ModifyDnRequest => OperationTargetDn::resultOf($request), - default => null, - }; - } } diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerDispatchHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerDispatchHandler.php index e528deb4..b66387cc 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerDispatchHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerDispatchHandler.php @@ -15,7 +15,6 @@ use FreeDSx\Asn1\Exception\EncoderException; use FreeDSx\Ldap\Control\Control; -use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\Request; use FreeDSx\Ldap\Operation\ResultCode; @@ -27,6 +26,7 @@ use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolations; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; +use FreeDSx\Ldap\Server\Backend\Write\WriteControlEvaluator; use FreeDSx\Ldap\Server\Backend\Write\Routing\WriteRequestRouter; use FreeDSx\Ldap\Server\Operation\CompareOperationResult; use FreeDSx\Ldap\Server\Operation\WriteOperationResult; @@ -45,11 +45,11 @@ public function __construct( private ReadBackendInterface $backend, private WriteRequestRouter $router, private AccessControlInterface $accessControl, + private AssertionEvaluator $assertions, Schema $schema, private ResponseFactory $responseFactory = new ResponseFactory(), ) { $this->readEntryControlHandler = new ReadEntryControlHandler( - $this->backend, $schema, $this->accessControl, ); @@ -65,23 +65,19 @@ public function handleRequest( LdapMessageRequest $message, TokenInterface $token, ): ResponseStream { - $schemaViolations = new SchemaViolations(); $request = $message->getRequest(); - $controls = $message->controls(); if ($request instanceof Request\CompareRequest) { return $this->handleCompare( $message, $request, + $token, ); } return $this->handleWrite( $message, - $request, - $controls, $token, - $schemaViolations, ); } @@ -92,9 +88,17 @@ public function handleRequest( private function handleCompare( LdapMessageRequest $message, Request\CompareRequest $request, + TokenInterface $token, ): ResponseStream { + // The assertion and the comparison are answered from one read of the entry (RFC 4528 §3). + $entry = $this->backend->getOrFail($request->getDn()); + $this->assertions->assertSatisfiedBy( + $entry, + $message->controls(), + $token, + ); $match = $this->backend->compare( - $request->getDn(), + $entry, $request->getFilter(), ); @@ -118,26 +122,33 @@ private function handleCompare( */ private function handleWrite( LdapMessageRequest $message, - Request\RequestInterface $request, - ControlBag $controls, TokenInterface $token, - SchemaViolations $schemaViolations, ): ResponseStream { - $preRead = $this->readEntryControlHandler->preReadFor( - $request, - $controls, + $controls = $message->controls(); + $schemaViolations = new SchemaViolations(); + $controlEvaluator = new WriteControlEvaluator( + $this->assertions, $token, + $controls, ); - $this->dispatchWrite( - $request, + $this->router->route( + $message->getRequest(), + new WriteContext( + $token, + $controls, + schemaViolations: $schemaViolations, + controlEvaluator: $controlEvaluator, + ), + ); + + $preRead = $this->readEntryControlHandler->preRead( + $controlEvaluator->preReadEntry(), $controls, $token, - $schemaViolations, ); - - $postRead = $this->readEntryControlHandler->postReadFor( - $request, + $postRead = $this->readEntryControlHandler->postRead( + $controlEvaluator->postReadEntry(), $controls, $token, ); @@ -158,25 +169,6 @@ private function handleWrite( ); } - /** - * @throws OperationException - */ - private function dispatchWrite( - Request\RequestInterface $request, - ControlBag $controls, - TokenInterface $token, - SchemaViolations $schemaViolations, - ): void { - $this->router->route( - $request, - new WriteContext( - $token, - $controls, - schemaViolations: $schemaViolations, - ), - ); - } - /** * @return Control[] */ diff --git a/src/FreeDSx/Ldap/Server/Backend/ReadBackendInterface.php b/src/FreeDSx/Ldap/Server/Backend/ReadBackendInterface.php index 9ff39284..836cf075 100644 --- a/src/FreeDSx/Ldap/Server/Backend/ReadBackendInterface.php +++ b/src/FreeDSx/Ldap/Server/Backend/ReadBackendInterface.php @@ -50,12 +50,19 @@ public function search( public function get(Dn $dn): ?Entry; /** - * Evaluate a compare assertion; throws OperationException(NO_SUCH_OBJECT) when the entry is missing. + * Fetch a single entry by DN, or answer NO_SUCH_OBJECT carrying the deepest ancestor that exists. + * + * @throws OperationException + */ + public function getOrFail(Dn $dn): Entry; + + /** + * Evaluate a compare assertion against an entry already read. * * @throws OperationException */ public function compare( - Dn $dn, + Entry $entry, EqualityFilter $filter, ): bool; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactor.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactor.php index 136662dd..8361a8b9 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactor.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactor.php @@ -14,6 +14,7 @@ namespace FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\PdoDialectInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageBusyException; use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; use FreeDSx\Ldap\Server\Clock\Sleeper\BlockingSleeper; use FreeDSx\Ldap\Server\Clock\Sleeper\SleeperInterface; @@ -65,6 +66,9 @@ public function joinAtomic(callable $operation): void * Runs the operation in a transaction, reissuing it when the database rejects it as a transient conflict. * * @param callable(): void $operation + * + * @throws StorageBusyException when the conflict outlasts the retry budget + * @throws PDOException when the failure is not a transient conflict, or the transaction is nested */ public function atomic(callable $operation): void { @@ -76,10 +80,17 @@ public function atomic(callable $operation): void return; } catch (PDOException $e) { + if (!$this->isReissuable($e)) { + throw $e; + } + $attempt++; - if (!$this->canRetry($e, $attempt)) { - throw $e; + if ($attempt > $this->maxRetries) { + throw new StorageBusyException( + 'The transaction kept conflicting with concurrent writes.', + $e, + ); } $this->sleeper->sleep($this->backoff->delayFor($attempt)); @@ -90,12 +101,9 @@ public function atomic(callable $operation): void /** * Only the outermost transaction can be reissued, since a savepoint cannot be replayed on its own. */ - private function canRetry( - PDOException $exception, - int $attempt, - ): bool { - return $attempt <= $this->maxRetries - && $this->provider->txState()->depth === 0 + private function isReissuable(PDOException $exception): bool + { + return $this->provider->txState()->depth === 0 && $this->dialect->isRetryableConflict($exception); } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Exception/StorageBusyException.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Exception/StorageBusyException.php new file mode 100644 index 00000000..e7902d44 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Exception/StorageBusyException.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Storage\Exception; + +use FreeDSx\Ldap\Exception\AnswerableExceptionInterface; +use FreeDSx\Ldap\Exception\RuntimeException; +use FreeDSx\Ldap\Operation\ResultCode; +use Throwable; + +/** + * Thrown when a write keeps conflicting with concurrent writes after every retry the storage allows. + * + * @author Chad Sikorra + */ +final class StorageBusyException extends RuntimeException implements AnswerableExceptionInterface +{ + private const DIAGNOSTIC = 'The backend storage is too busy to complete the operation.'; + + public function __construct( + string $message, + ?Throwable $previous = null, + ) { + parent::__construct( + $message, + ResultCode::BUSY, + $previous, + ); + } + + public function getDiagnostic(): string + { + return self::DIAGNOSTIC; + } +} diff --git a/src/FreeDSx/Ldap/Server/Backend/StorageReadBackend.php b/src/FreeDSx/Ldap/Server/Backend/StorageReadBackend.php index 4ebdd00f..baeeef2c 100644 --- a/src/FreeDSx/Ldap/Server/Backend/StorageReadBackend.php +++ b/src/FreeDSx/Ldap/Server/Backend/StorageReadBackend.php @@ -60,18 +60,21 @@ public function get(Dn $dn): ?Entry return $this->storage->find($dn->normalize()); } + /** + * @throws OperationException + */ + public function getOrFail(Dn $dn): Entry + { + return $this->get($dn) ?? $this->locator->throwNoSuchObject($dn); + } + /** * @throws OperationException */ public function compare( - Dn $dn, + Entry $entry, EqualityFilter $filter, ): bool { - $entry = $this->get($dn); - - if ($entry === null) { - $this->locator->throwNoSuchObject($dn); - } // RFC 4511 4.10: only compareTrue and compareFalse may report a match, so Undefined needs its own code. $this->assertComparable($filter); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AddEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AddEntryHandler.php index 37b3d847..3f123612 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AddEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AddEntryHandler.php @@ -83,6 +83,7 @@ public function handle( $entry, $command->systemChanges, ); + $context->controlEvaluator()?->evaluateAddition($entry); if ($bulkLoad !== null && $bulkLoad->replaceExisting) { $this->storage->store( diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php index bc6095f9..32c7cf33 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php @@ -13,6 +13,7 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; @@ -36,10 +37,11 @@ trait AppliesEntryUpdate private function applyUpdate( UpdateCommand $command, WriteContext $context, + Entry $current, ): void { $dn = $command->dn->normalize(); $updated = $this->mutation->forUpdate( - $this->locator->findOrFail($dn), + $current, $command, $context, ); @@ -52,6 +54,7 @@ private function applyUpdate( $updated, $command->systemChanges, ); + $context->controlEvaluator()?->captureResult($updated); $this->storage->store($updated); $this->changeRecorder?->recordModify( diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ComputeUpdateHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ComputeUpdateHandler.php index 378d4e57..55e38f10 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ComputeUpdateHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ComputeUpdateHandler.php @@ -68,6 +68,7 @@ function () use ($command, $context, $dn): void { $changes, ), $context, + $entry, ); }, ); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteEntryHandler.php index fe720482..93aa4720 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteEntryHandler.php @@ -13,6 +13,7 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Directory\EntryLocator; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; @@ -45,10 +46,10 @@ public function handle( ): void { $dn = $command->dn->normalize(); - $this->writeLocked( + $this->writeLockedEntry( $dn, - function () use ($command, $context, $dn): void { - $entry = $this->locator->findOrFail($dn); + $context, + function (Entry $entry) use ($command, $context, $dn): void { $this->placement->assertDeletePlacement($command->dn); $this->storage->remove($dn); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/MoveEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/MoveEntryHandler.php index 12c2861b..5a93f252 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/MoveEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/MoveEntryHandler.php @@ -13,6 +13,7 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Directory\EntryLocator; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; @@ -49,11 +50,12 @@ public function handle( $normOld = $command->dn->normalize(); // Only the moved entry is locked; the destination is held by the unique key the rename and store land on. - $this->writeLocked( + $this->writeLockedEntry( $normOld, - function () use ($command, $context, $normOld): void { + $context, + function (Entry $current) use ($command, $context, $normOld): void { $newEntry = $this->mutation->forMove( - $this->locator->findOrFail($normOld), + $current, $command, $context, ); @@ -63,6 +65,7 @@ function () use ($command, $context, $normOld): void { $normOld, $context->isSystem(), ); + $context->controlEvaluator()?->captureResult($newEntry); $normNew = $newEntry->getDn()->normalize(); // Re-keyed before the base is stored, so the upsert lands on the moved row rather than inserting a second. diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php index 3705aa72..178e9635 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php @@ -13,6 +13,7 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Directory\EntryLocator; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; @@ -45,12 +46,14 @@ public function handle( UpdateCommand $command, WriteContext $context, ): void { - $this->writeLocked( + $this->writeLockedEntry( $command->dn->normalize(), - function () use ($command, $context): void { + $context, + function (Entry $current) use ($command, $context): void { $this->applyUpdate( $command, $context, + $current, ); }, ); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/WritesLockedEntry.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/WritesLockedEntry.php index 2e64afd0..9b7a5161 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/WritesLockedEntry.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/WritesLockedEntry.php @@ -15,8 +15,10 @@ use Closure; use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Capability\RowLockableInterface; +use FreeDSx\Ldap\Server\Backend\Write\WriteContext; /** * Opens the atomic write and takes the entry's row lock ahead of the body. @@ -39,6 +41,27 @@ private function writeLocked( }); } + /** + * Locates the entry under the lock, so a missing target answers before any control the write carries is evaluated. + * + * @param Closure(Entry): void $body + * @throws OperationException + */ + private function writeLockedEntry( + Dn $dn, + WriteContext $context, + Closure $body, + ): void { + $this->writeLocked( + $dn, + function () use ($dn, $context, $body): void { + $current = $this->locator->findOrFail($dn); + $context->controlEvaluator()?->evaluateTarget($current); + $body($current); + }, + ); + } + private function lockForWrite(Dn $dn): void { if (!$this->storage instanceof RowLockableInterface) { diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Replay/ReplayWriteHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Replay/ReplayWriteHandler.php index 1b7637b8..486f10a2 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Replay/ReplayWriteHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Replay/ReplayWriteHandler.php @@ -18,9 +18,11 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Protocol\Queue\Response\ResponseStream; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Server\Backend\Write\Routing\WriteRequestRouter; use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolations; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; +use FreeDSx\Ldap\Server\Backend\Write\WriteControlEvaluator; use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareHandlerInterface; use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext; use FreeDSx\Ldap\Server\Operation\WriteOperationResult; @@ -44,13 +46,16 @@ Control::OID_SUBTREE_DELETE, // Read off the write context by the storage backend. Control::OID_RELAX_RULES, - // Evaluated ahead of this handler by AssertionMiddleware. + // Evaluated by the write under the lock it takes on the entry. Control::OID_ASSERTION, // Recognized server-wide and inert, since there are no referral entries to reinterpret. Control::OID_MANAGE_DSA_IT, ]; - public function __construct(private WriteRequestRouter $router) {} + public function __construct( + private WriteRequestRouter $router, + private AssertionEvaluator $assertions, + ) {} /** * @throws OperationException @@ -60,11 +65,18 @@ public function handle(ServerRequestContext $context): ResponseStream $controls = $context->message->controls(); $this->assertAnswerable($controls); + $token = $context->tokenOrFail(); $this->router->route( $context->message->getRequest(), - WriteContext::system( - $context->tokenOrFail(), + new WriteContext( + $token, $controls, + isSystem: true, + controlEvaluator: new WriteControlEvaluator( + $this->assertions, + $token, + $controls, + ), ), ); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/WriteContext.php b/src/FreeDSx/Ldap/Server/Backend/Write/WriteContext.php index 0794754e..00bf9c28 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/WriteContext.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/WriteContext.php @@ -33,6 +33,7 @@ public function __construct( private bool $isSystem = false, private SchemaViolations $schemaViolations = new SchemaViolations(), private ?BulkLoadOptions $bulkLoad = null, + private ?WriteControlEvaluator $controlEvaluator = null, ) {} /** @@ -108,4 +109,12 @@ public function bulkLoadOptions(): ?BulkLoadOptions { return $this->bulkLoad; } + + /** + * Present when the write's controls are evaluated under its lock, as for a client request or a replayed record. + */ + public function controlEvaluator(): ?WriteControlEvaluator + { + return $this->controlEvaluator; + } } diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/WriteControlEvaluator.php b/src/FreeDSx/Ldap/Server/Backend/Write/WriteControlEvaluator.php new file mode 100644 index 00000000..486fd98e --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Write/WriteControlEvaluator.php @@ -0,0 +1,107 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Write; + +use FreeDSx\Ldap\Control\Control; +use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; +use FreeDSx\Ldap\Server\Token\TokenInterface; + +/** + * Evaluates a write's controls against the entries it holds under its lock, so they are atomic with the write. + * + * @internal + * + * @author Chad Sikorra + */ +class WriteControlEvaluator +{ + private ?Entry $preReadEntry = null; + + private ?Entry $postReadEntry = null; + + public function __construct( + private readonly AssertionEvaluator $assertions, + private readonly TokenInterface $token, + private readonly ControlBag $controls, + ) {} + + /** + * Holds the located entry to the assertion (RFC 4528 §3), then keeps it for a Pre-Read (RFC 4527 §3.1). + * + * @throws OperationException + */ + public function evaluateTarget(Entry $current): void + { + $this->assertions->assertSatisfiedBy( + $current, + $this->controls, + $this->token, + ); + $this->preReadEntry = $this->keptFor( + Control::OID_PRE_READ, + $current, + ); + } + + /** + * Holds the entry being added to the assertion, as it is the Add's target (RFC 4528 §3), then keeps it for a Post-Read. + * + * @throws OperationException + */ + public function evaluateAddition(Entry $entry): void + { + $this->assertions->assertSatisfiedBy( + $entry, + $this->controls, + $this->token, + ); + $this->captureResult($entry); + } + + /** + * Keeps the entry the write is about to store, for a Post-Read (RFC 4527 §3.2). + */ + public function captureResult(Entry $result): void + { + $this->postReadEntry = $this->keptFor( + Control::OID_POST_READ, + $result, + ); + } + + public function preReadEntry(): ?Entry + { + return $this->preReadEntry; + } + + public function postReadEntry(): ?Entry + { + return $this->postReadEntry; + } + + /** + * A copy, since storage may keep the very object the write goes on to change, and nothing without the control. + */ + private function keptFor( + string $oid, + Entry $entry, + ): ?Entry { + return $this->controls->has($oid) + ? $entry->makeCopy() + : null; + } +} diff --git a/src/FreeDSx/Ldap/Server/Middleware/AssertionMiddleware.php b/src/FreeDSx/Ldap/Server/Middleware/AssertionMiddleware.php index 80005005..fe7e8130 100644 --- a/src/FreeDSx/Ldap/Server/Middleware/AssertionMiddleware.php +++ b/src/FreeDSx/Ldap/Server/Middleware/AssertionMiddleware.php @@ -19,13 +19,14 @@ use FreeDSx\Ldap\Operation\Request\SearchRequest; use FreeDSx\Ldap\Protocol\Queue\Response\ResponseStream; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; -use FreeDSx\Ldap\Server\AccessControl\OperationTargetDn; use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareHandlerInterface; use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareInterface; use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext; /** - * Rejects an operation whose RFC 4528 assertion control does not match its target entry, before dispatch. + * Rejects a search whose RFC 4528 assertion control does not match its base entry, before the search runs. + * + * A compare or a write evaluates its assertion against the entry it reads itself, so the two are one atomic action. * * @internal * @author Chad Sikorra @@ -53,7 +54,7 @@ public function process( $target = $request instanceof SearchRequest ? $request->getBaseDn() - : OperationTargetDn::of($request); + : null; if ($target !== null) { $this->evaluator->assertSatisfied( diff --git a/src/FreeDSx/Ldap/Server/Middleware/CriticalControlValidator.php b/src/FreeDSx/Ldap/Server/Middleware/CriticalControlValidator.php index c0d25bed..05808afa 100644 --- a/src/FreeDSx/Ldap/Server/Middleware/CriticalControlValidator.php +++ b/src/FreeDSx/Ldap/Server/Middleware/CriticalControlValidator.php @@ -49,6 +49,7 @@ public function assertSupportedForRequest( $this->controlRegistry->supportedControlsFor( $routeId, $request, + $controls, ), ); } diff --git a/src/FreeDSx/Ldap/Server/Middleware/OperationAuthorizationMiddleware.php b/src/FreeDSx/Ldap/Server/Middleware/OperationAuthorizationMiddleware.php index 0235f6f7..78ccd3a4 100644 --- a/src/FreeDSx/Ldap/Server/Middleware/OperationAuthorizationMiddleware.php +++ b/src/FreeDSx/Ldap/Server/Middleware/OperationAuthorizationMiddleware.php @@ -20,6 +20,7 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\Request\AddRequest; use FreeDSx\Ldap\Operation\Request\CompareRequest; +use FreeDSx\Ldap\Operation\Request\DeleteRequest; use FreeDSx\Ldap\Operation\Request\AbandonRequest; use FreeDSx\Ldap\Operation\Request\ExtendedRequest; use FreeDSx\Ldap\Operation\Request\UnbindRequest; @@ -189,9 +190,6 @@ private function targetDnFor(RequestInterface $request): ?Dn * Only controls the client marked critical are enforced here, per RFC 4511 section 4.1.11. A non-critical * read-entry control is left to be dropped from the response instead of failing an otherwise valid operation. * - * An assertion is always enforced, critical or not, since ignoring a precondition would let a write land that - * the client conditioned on state it cannot verify. - * * @see \FreeDSx\Ldap\Protocol\ServerProtocolHandler\ReadEntryControlHandler * * @throws OperationException @@ -221,7 +219,7 @@ private function enforcedReadTargetsFor( ): array { $targets = []; - if ($controls->has(Control::OID_ASSERTION)) { + if ($controls->has(Control::OID_ASSERTION) && !$this->isSubtreeDelete($request, $controls)) { $targets[] = $this->targetDnFor($request); } @@ -239,6 +237,14 @@ private function enforcedReadTargetsFor( return array_values(array_filter($targets)); } + private function isSubtreeDelete( + RequestInterface $request, + ControlBag $controls, + ): bool { + return $request instanceof DeleteRequest + && $controls->has(Control::OID_SUBTREE_DELETE); + } + /** * Whether the control is present and the client marked it critical. */ diff --git a/src/FreeDSx/Ldap/Server/Middleware/ServerControlRegistry.php b/src/FreeDSx/Ldap/Server/Middleware/ServerControlRegistry.php index 4879d28c..d1b59b3b 100644 --- a/src/FreeDSx/Ldap/Server/Middleware/ServerControlRegistry.php +++ b/src/FreeDSx/Ldap/Server/Middleware/ServerControlRegistry.php @@ -14,6 +14,7 @@ namespace FreeDSx\Ldap\Server\Middleware; use FreeDSx\Ldap\Control\Control; +use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Operation\Request\AddRequest; use FreeDSx\Ldap\Operation\Request\CompareRequest; use FreeDSx\Ldap\Operation\Request\DeleteRequest; @@ -108,6 +109,14 @@ final class ServerControlRegistry Control::OID_SUBTREE_DELETE, ]; + /** + * A subtree is removed across several transactions, so neither a precondition nor a before image can be atomic. + */ + private const SUBTREE_DELETE_CONTROLS = [ + Control::OID_RELAX_RULES, + Control::OID_SUBTREE_DELETE, + ]; + /** * A compare reads rather than updates, so it takes neither the read-entry pair nor relax. */ @@ -146,10 +155,11 @@ public function appliesTo(HandlerId $id): bool public function supportedControlsFor( HandlerId $id, RequestInterface $request, + ControlBag $controls, ): array { return [ ...self::GLOBAL_CONTROLS, - ...$this->handlerControlsFor($id, $request), + ...$this->handlerControlsFor($id, $request, $controls), ]; } @@ -159,11 +169,12 @@ public function supportedControlsFor( private function handlerControlsFor( HandlerId $id, RequestInterface $request, + ControlBag $controls, ): array { return match ($id) { HandlerId::Search => self::SEARCH_CONTROLS, HandlerId::Paging => self::PAGING_CONTROLS, - HandlerId::Dispatch => $this->dispatchControlsFor($request), + HandlerId::Dispatch => $this->dispatchControlsFor($request, $controls), HandlerId::Sync => self::SYNC_CONTROLS, default => [], }; @@ -175,12 +186,16 @@ private function handlerControlsFor( * * @return list */ - private function dispatchControlsFor(RequestInterface $request): array - { + private function dispatchControlsFor( + RequestInterface $request, + ControlBag $controls, + ): array { return match (true) { $request instanceof AddRequest => self::ADD_CONTROLS, $request instanceof ModifyRequest => self::MODIFY_CONTROLS, $request instanceof ModifyDnRequest => self::MODIFY_DN_CONTROLS, + $request instanceof DeleteRequest + && $controls->has(Control::OID_SUBTREE_DELETE) => self::SUBTREE_DELETE_CONTROLS, $request instanceof DeleteRequest => self::DELETE_CONTROLS, $request instanceof CompareRequest => self::COMPARE_CONTROLS, default => [], diff --git a/tests/integration/Controls/ServerControlsTest.php b/tests/integration/Controls/ServerControlsTest.php index d9e4d313..f47341ec 100644 --- a/tests/integration/Controls/ServerControlsTest.php +++ b/tests/integration/Controls/ServerControlsTest.php @@ -61,70 +61,6 @@ public function setUp(): void parent::setUp(); } - public function test_assertion_allows_a_modify_when_it_matches(): void - { - $this->authenticateAdmin(); - $dn = $this->createPerson('assert-ok'); - - $this->ldapClient()->send( - Operations::modify($dn, Change::replace('sn', 'Jones')), - Controls::assertion(Filters::equal('sn', 'Smith')), - ); - - self::assertSame('Jones', $this->readValue($dn, 'sn')); - } - - public function test_assertion_fails_a_modify_when_it_does_not_match(): void - { - $this->authenticateAdmin(); - $dn = $this->createPerson('assert-no'); - - try { - $this->ldapClient()->send( - Operations::modify($dn, Change::replace('sn', 'Jones')), - Controls::assertion(Filters::equal('sn', 'Nope')), - ); - self::fail('Expected an OperationException was not thrown.'); - } catch (OperationException $e) { - self::assertSame(ResultCode::ASSERTION_FAILED, $e->getCode()); - } - - self::assertSame('Smith', $this->readValue($dn, 'sn')); - } - - public function test_assertion_allows_a_search_when_it_matches(): void - { - $this->bind(); - - $entries = $this->ldapClient()->search( - Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), - Controls::assertion(Filters::equal('dc', 'foo')), - ); - - self::assertGreaterThan(0, $entries->count()); - } - - public function test_assertion_fails_a_search_when_it_does_not_match(): void - { - $this->bind(); - - try { - $this->ldapClient()->search( - Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), - Controls::assertion(Filters::equal('dc', 'nope')), - ); - self::fail('Expected an OperationException was not thrown.'); - } catch (OperationException $e) { - self::assertSame(ResultCode::ASSERTION_FAILED, $e->getCode()); - } - - # The connection survives the per-operation rejection: a follow-up search still succeeds. - $entries = $this->ldapClient()->search( - Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), - ); - self::assertGreaterThan(0, $entries->count()); - } - public function test_assertion_on_an_unreadable_attribute_cannot_match_its_value(): void { $this->authenticateUser(); @@ -218,6 +154,35 @@ public function test_assertion_on_an_unreadable_attribute_blocks_a_write_it_woul } } + public function test_assertion_on_an_add_cannot_see_an_attribute_the_identity_may_not_read(): void + { + $this->authenticateAdmin(); + $dn = 'cn=assert-add-secret,ou=people,dc=foo,dc=bar'; + $entry = $this->person( + $dn, + 'assert-add-secret', + ); + $entry->set( + 'userPassword', + self::SEEDED_PASSWORD_HASH, + ); + + try { + $this->ldapClient()->send( + Operations::add($entry), + Controls::assertion(Filters::present('userPassword')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNull($this->ldapClient()->read($dn)); + } + public function test_pre_read_and_post_read_capture_state_around_a_modify(): void { $this->authenticateAdmin(); @@ -484,11 +449,6 @@ private function nonCritical(Control $control): Control return $control->setCriticality(false); } - private function bind(): void - { - $this->ldapClient()->bind('cn=user,dc=foo,dc=bar', '12345'); - } - private function createPerson(string $cn): string { $dn = "cn={$cn},ou=people,dc=foo,dc=bar"; @@ -507,17 +467,4 @@ private function person( 'sn' => ['Smith'], ]); } - - private function readValue( - string $dn, - string $attribute, - ): ?string { - $entries = $this->ldapClient()->search( - Operations::search(Filters::present('objectClass')) - ->base($dn) - ->useBaseScope(), - ); - - return $entries->first()?->get($attribute)?->firstValue(); - } } diff --git a/tests/integration/Security/PasswordPolicyPlainModifyEnforcementTest.php b/tests/integration/Security/PasswordPolicyPlainModifyEnforcementTest.php index d4da7a58..5317295c 100644 --- a/tests/integration/Security/PasswordPolicyPlainModifyEnforcementTest.php +++ b/tests/integration/Security/PasswordPolicyPlainModifyEnforcementTest.php @@ -36,6 +36,7 @@ use FreeDSx\Ldap\Server\Backend\Write\PasswordPolicyWriteHandler; use FreeDSx\Ldap\Server\Backend\Write\WriteOperationDispatcher; use FreeDSx\Ldap\Server\Backend\Write\Routing\WriteRequestRouter; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerDispatchHandler; use FreeDSx\Ldap\Server\Logging\EventLogger; use FreeDSx\Ldap\Server\PasswordPolicy\Guard\PasswordPolicyChangeGuard; @@ -423,6 +424,7 @@ private function dispatchHandler( backend: $this->backend, router: new WriteRequestRouter($policyWriteHandler), accessControl: $this->createMock(AccessControlInterface::class), + assertions: $container->get(AssertionEvaluator::class), schema: new Schema(), ); } diff --git a/tests/integration/Storage/Concern/ControlTestsTrait.php b/tests/integration/Storage/Concern/ControlTestsTrait.php index 8000d814..40d653e1 100644 --- a/tests/integration/Storage/Concern/ControlTestsTrait.php +++ b/tests/integration/Storage/Concern/ControlTestsTrait.php @@ -15,10 +15,13 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PostReadResponseControl; use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PreReadResponseControl; use FreeDSx\Ldap\Control\Sorting\SortingControl; use FreeDSx\Ldap\Control\Sorting\SortingResponseControl; use FreeDSx\Ldap\Control\Sorting\SortKey; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; @@ -388,6 +391,356 @@ public function testACriticalControlTheOperationDoesNotTakeIsRefused( } } + /** + * @return iterable + */ + public static function controlsASubtreeDeleteCannotHonor(): iterable + { + yield 'assertion' => [Controls::assertion(Filters::present('objectClass'))]; + yield 'pre-read' => [new PreReadControl('cn')]; + } + + #[DataProvider('controlsASubtreeDeleteCannotHonor')] + public function testACriticalControlASubtreeDeleteCannotHonorIsRefused(Control $control): void + { + $this->authenticateAdmin(); + + try { + $this->ldapClient()->sendAndReceive( + Operations::delete('ou=no-such-subtree,dc=foo,dc=bar'), + Controls::subtreeDelete(), + $control->setCriticality(true), + ); + self::fail('The critical control should have failed the subtree delete.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::UNAVAILABLE_CRITICAL_EXTENSION, + $e->getCode(), + ); + } + } + + public function testANonCriticalControlASubtreeDeleteCannotHonorIsIgnored(): void + { + $this->authenticateAdmin(); + $this->ldapClient()->create(Entry::fromArray('ou=ignored-controls,dc=foo,dc=bar', [ + 'objectClass' => ['organizationalUnit'], + 'ou' => ['ignored-controls'], + ])); + $this->ldapClient()->create(Entry::fromArray('cn=child,ou=ignored-controls,dc=foo,dc=bar', [ + 'objectClass' => ['inetOrgPerson'], + 'cn' => ['child'], + 'sn' => ['Child'], + ])); + + $response = $this->ldapClient()->sendAndReceive( + Operations::delete('ou=ignored-controls,dc=foo,dc=bar'), + Controls::subtreeDelete(), + Controls::assertion(Filters::equal('ou', 'nomatch'))->setCriticality(false), + Controls::preRead('ou')->setCriticality(false), + ); + + self::assertNull($response->controls()->get(Control::OID_PRE_READ)); + self::assertNull($this->ldapClient()->read('ou=ignored-controls,dc=foo,dc=bar')); + } + + public function testAPostReadMatchesTheEntryAsStoredAfterTheModify(): void + { + $this->authenticateAdmin(); + $dn = 'cn=post-read-modify,dc=foo,dc=bar'; + $this->ldapClient()->create(Entry::fromArray($dn, [ + 'objectClass' => ['inetOrgPerson'], + 'cn' => ['post-read-modify'], + 'sn' => ['Before'], + ])); + + $response = $this->ldapClient()->sendAndReceive( + Operations::modify( + $dn, + Change::replace('sn', 'After'), + ), + Controls::postRead(), + ); + $stored = $this->ldapClient()->read($dn); + $this->ldapClient()->delete($dn); + + $postRead = $response->controls()->get(Control::OID_POST_READ); + self::assertInstanceOf( + PostReadResponseControl::class, + $postRead, + ); + self::assertEquals( + $stored?->toArray(), + $postRead->getEntry()->toArray(), + ); + } + + public function testThePreReadAndPostReadOfAModifyDnCarryTheOldAndNewNames(): void + { + $this->authenticateAdmin(); + $this->ldapClient()->create(Entry::fromArray('cn=before-rename,dc=foo,dc=bar', [ + 'objectClass' => ['inetOrgPerson'], + 'cn' => ['before-rename'], + 'sn' => ['Renamed'], + ])); + + $response = $this->ldapClient()->sendAndReceive( + Operations::rename( + 'cn=before-rename,dc=foo,dc=bar', + 'cn=after-rename', + ), + Controls::preRead('cn'), + Controls::postRead('cn'), + ); + $this->ldapClient()->delete('cn=after-rename,dc=foo,dc=bar'); + + $preRead = $response->controls()->get(Control::OID_PRE_READ); + $postRead = $response->controls()->get(Control::OID_POST_READ); + self::assertInstanceOf( + PreReadResponseControl::class, + $preRead, + ); + self::assertInstanceOf( + PostReadResponseControl::class, + $postRead, + ); + self::assertSame( + 'cn=before-rename,dc=foo,dc=bar', + $preRead->getEntry()->getDn()->toString(), + ); + self::assertSame( + 'cn=after-rename,dc=foo,dc=bar', + $postRead->getEntry()->getDn()->toString(), + ); + self::assertSame( + ['after-rename'], + $postRead->getEntry()->get('cn')?->getValues(), + ); + } + + public function testAnAssertionMatchingTheSearchBaseLetsTheSearchRun(): void + { + $this->authenticateUser(); + + $entries = $this->ldapClient()->search( + Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), + Controls::assertion(Filters::equal('dc', 'foo')), + ); + + self::assertGreaterThan( + 0, + $entries->count(), + ); + } + + public function testAnAssertionNotMatchingTheSearchBaseFailsTheSearch(): void + { + $this->authenticateUser(); + + try { + $this->ldapClient()->search( + Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), + Controls::assertion(Filters::equal('dc', 'nope')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + // The rejection ends only the operation, not the connection. + $entries = $this->ldapClient()->search( + Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'), + ); + self::assertGreaterThan( + 0, + $entries->count(), + ); + } + + public function testAnAssertionMatchingTheEntryLetsTheCompareAnswer(): void + { + $this->authenticateUser(); + + self::assertTrue($this->ldapClient()->compare( + 'cn=alice,ou=people,dc=foo,dc=bar', + 'sn', + 'Smith', + Controls::assertion(Filters::equal('cn', 'alice')), + )); + } + + public function testAnAssertionNotMatchingTheEntryFailsTheCompare(): void + { + $this->authenticateUser(); + + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->ldapClient()->compare( + 'cn=alice,ou=people,dc=foo,dc=bar', + 'sn', + 'Smith', + Controls::assertion(Filters::equal('cn', 'nobody')), + ); + } + + public function testACompareOfAMissingEntryAnswersBeforeItsAssertion(): void + { + $this->authenticateUser(); + + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::NO_SUCH_OBJECT); + + $this->ldapClient()->compare( + 'cn=nobody,ou=people,dc=foo,dc=bar', + 'sn', + 'Smith', + Controls::assertion(Filters::equal('cn', 'nobody')), + ); + } + + public function testAnAssertionMatchingTheAddedEntryLetsTheAddRun(): void + { + $this->authenticateAdmin(); + $entry = $this->assertionTarget('assert-add-ok'); + + $this->ldapClient()->send( + Operations::add($entry), + Controls::assertion(Filters::equal('sn', 'Smith')), + ); + $added = $this->ldapClient()->read($entry->getDn()->toString()); + $this->ldapClient()->delete($entry->getDn()->toString()); + + self::assertSame( + ['Smith'], + $added?->get('sn')?->getValues(), + ); + } + + public function testAnAssertionNotMatchingTheAddedEntryFailsTheAdd(): void + { + $this->authenticateAdmin(); + $entry = $this->assertionTarget('assert-add-no'); + + try { + $this->ldapClient()->send( + Operations::add($entry), + Controls::assertion(Filters::equal('sn', 'doesnotmatch')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNull($this->ldapClient()->read($entry->getDn()->toString())); + } + + public function testAnAssertionMatchingTheEntryLetsTheModifyRun(): void + { + $this->authenticateAdmin(); + $dn = $this->createAssertionTarget('assert-modify-ok'); + + $this->ldapClient()->send( + Operations::modify( + $dn, + Change::replace('sn', 'Jones'), + ), + Controls::assertion(Filters::equal('sn', 'Smith')), + ); + $modified = $this->ldapClient()->read($dn); + $this->ldapClient()->delete($dn); + + self::assertSame( + ['Jones'], + $modified?->get('sn')?->getValues(), + ); + } + + public function testAnAssertionNotMatchingTheEntryFailsTheModify(): void + { + $this->authenticateAdmin(); + $dn = $this->createAssertionTarget('assert-modify-no'); + + try { + $this->ldapClient()->send( + Operations::modify( + $dn, + Change::replace('sn', 'Jones'), + ), + Controls::assertion(Filters::equal('sn', 'Nope')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + $unchanged = $this->ldapClient()->read($dn); + $this->ldapClient()->delete($dn); + + self::assertSame( + ['Smith'], + $unchanged?->get('sn')?->getValues(), + ); + } + + public function testAnAssertionNotMatchingTheEntryFailsTheDelete(): void + { + $this->authenticateAdmin(); + $dn = $this->createAssertionTarget('assert-delete-no'); + + try { + $this->ldapClient()->send( + Operations::delete($dn), + Controls::assertion(Filters::equal('sn', 'Nope')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + $kept = $this->ldapClient()->read($dn); + $this->ldapClient()->delete($dn); + + self::assertNotNull($kept); + } + + public function testAnAssertionNotMatchingTheEntryFailsTheModifyDn(): void + { + $this->authenticateAdmin(); + $dn = $this->createAssertionTarget('assert-rename-no'); + + try { + $this->ldapClient()->send( + Operations::rename( + $dn, + 'cn=assert-renamed', + ), + Controls::assertion(Filters::equal('sn', 'Nope')), + ); + self::fail('Expected an OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + $kept = $this->ldapClient()->read($dn); + $this->ldapClient()->delete($dn); + + self::assertNotNull($kept); + self::assertNull($this->ldapClient()->read('cn=assert-renamed,ou=people,dc=foo,dc=bar')); + } + public function testACriticalSortThatCannotBePerformedFailsAPagedSearch(): void { $this->authenticateUser(); @@ -689,6 +1042,32 @@ public function testSortControlPlacesMissingAttributeFirstWhenDescending(): void self::assertNotNull($entries[count($entries) - 1]->get('sn')); } + /** + * An entry under ou=people for a write carrying an assertion to act on. + */ + private function assertionTarget(string $cn): Entry + { + return Entry::fromArray( + "cn={$cn},ou=people,dc=foo,dc=bar", + [ + 'objectClass' => ['inetOrgPerson'], + 'cn' => [$cn], + 'sn' => ['Smith'], + ], + ); + } + + /** + * Creates the entry a write carrying an assertion acts on, which the test removes again. + */ + private function createAssertionTarget(string $cn): string + { + $entry = $this->assertionTarget($cn); + $this->ldapClient()->create($entry); + + return $entry->getDn()->toString(); + } + /** * The description values the seed carries, in the order the sort key put them. * diff --git a/tests/support/Backend/Write/WriteHandlerTestTrait.php b/tests/support/Backend/Write/WriteHandlerTestTrait.php index 6a617b33..89982c97 100644 --- a/tests/support/Backend/Write/WriteHandlerTestTrait.php +++ b/tests/support/Backend/Write/WriteHandlerTestTrait.php @@ -14,17 +14,24 @@ namespace Tests\Support\FreeDSx\Ldap\Backend\Write; use FreeDSx\Ldap\Container; +use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; +use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Filter\FilterEvaluatorInterface; use FreeDSx\Ldap\Server\Backend\Write\Operation\AddEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\DeleteEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\MoveEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\UpdateEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; +use FreeDSx\Ldap\Server\Backend\Write\WriteControlEvaluator; use FreeDSx\Ldap\Server\Token\AnonToken; use FreeDSx\Ldap\ServerOptions; use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; @@ -127,6 +134,29 @@ private function context(): WriteContext ); } + /** + * A client write's context, whose controls the handler evaluates under its lock. + */ + private function controlledContext(Control ...$controls): WriteContext + { + $token = new AnonToken(); + $controlBag = new ControlBag(...$controls); + + return new WriteContext( + $token, + $controlBag, + controlEvaluator: new WriteControlEvaluator( + new AssertionEvaluator( + $this->graph->get(FilterEvaluatorInterface::class), + $this->graph->get(ReadBackendInterface::class), + new RuleBasedAccessControl(AclRules::fromEmpty()), + ), + $token, + $controlBag, + ), + ); + } + private function systemContext(): WriteContext { return WriteContext::system( diff --git a/tests/unit/LdapServerTest.php b/tests/unit/LdapServerTest.php index 709b4950..cdf0bcd0 100644 --- a/tests/unit/LdapServerTest.php +++ b/tests/unit/LdapServerTest.php @@ -361,6 +361,49 @@ public function test_it_should_apply_a_change_record_carrying_a_non_critical_con self::assertNull($this->storage()->find(new Dn('cn=foo,dc=example,dc=com'))); } + public function test_it_should_refuse_a_subtree_delete_record_carrying_a_critical_assertion(): void + { + $this->subject->seed(new StringLdifLoader(self::SEED_LDIF . "\n\n" . self::SUBTREE_LDIF)); + + try { + $this->subject->applyChanges(new StringLdifLoader( + "dn: ou=people,dc=example,dc=com\n" + . "control: 1.2.840.113556.1.4.805 true\n" + . "control: 1.3.6.1.1.12 true\n" + . "changetype: delete\n", + )); + self::fail('The critical assertion should have refused the subtree delete.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::UNAVAILABLE_CRITICAL_EXTENSION, + $e->getCode(), + ); + } + + self::assertNotNull($this->storage()->find(new Dn('cn=child,ou=people,dc=example,dc=com'))); + } + + public function test_it_should_refuse_a_change_record_whose_assertion_does_not_match(): void + { + $this->subject->seed(new StringLdifLoader(self::SEED_LDIF)); + + try { + $this->subject->applyChanges(new StringLdifLoader( + "dn: cn=foo,dc=example,dc=com\n" + . "control: 1.3.6.1.1.12 true:: owoEAnNuBAROb3Bl\n" + . "changetype: delete\n", + )); + self::fail('The assertion should have refused the change record.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNotNull($this->storage()->find(new Dn('cn=foo,dc=example,dc=com'))); + } + public function test_it_should_refuse_to_seed_a_read_only_replica(): void { $options = (new ServerOptions( diff --git a/tests/unit/Protocol/ServerProtocolHandler/AssertionEvaluatorTest.php b/tests/unit/Protocol/ServerProtocolHandler/AssertionEvaluatorTest.php index fcb91c44..85f8743e 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/AssertionEvaluatorTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/AssertionEvaluatorTest.php @@ -5,6 +5,7 @@ namespace Tests\Unit\FreeDSx\Ldap\Protocol\ServerProtocolHandler; use FreeDSx\Ldap\Control\AssertionControl; +use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; @@ -92,6 +93,22 @@ public function test_it_throws_assertion_failed_when_the_assertion_does_not_matc ); } + public function test_an_assertion_left_undecoded_is_still_evaluated(): void + { + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->subject->assertSatisfiedBy( + Entry::fromArray('cn=foo,dc=ex,dc=com', ['cn' => ['foo']]), + new ControlBag(new Control( + Control::OID_ASSERTION, + true, + Filters::equal('cn', 'bar')->toAsn1(), + )), + $this->token, + ); + } + public function test_it_does_not_throw_when_the_target_entry_is_absent(): void { $this->backend diff --git a/tests/unit/Protocol/ServerProtocolHandler/ReadEntryControlHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ReadEntryControlHandlerTest.php index 9f12fd59..a0a87539 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ReadEntryControlHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ReadEntryControlHandlerTest.php @@ -9,7 +9,6 @@ use FreeDSx\Ldap\Control\ReadEntry\PostReadResponseControl; use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; use FreeDSx\Ldap\Control\ReadEntry\PreReadResponseControl; -use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ReadEntryControlHandler; use FreeDSx\Ldap\Schema\Schema; @@ -20,29 +19,23 @@ use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; use FreeDSx\Ldap\Server\AccessControl\Subject\Subject; use FreeDSx\Ldap\Server\AccessControl\Target\Target; -use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Token\BindToken; use FreeDSx\Ldap\Server\Token\TokenInterface; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; final class ReadEntryControlHandlerTest extends TestCase { - private ReadBackendInterface&MockObject $backend; - private ReadEntryControlHandler $subject; - private Dn $dn; + private Entry $entry; private TokenInterface $token; protected function setUp(): void { - $this->backend = $this->createMock(ReadBackendInterface::class); - $this->dn = new Dn('cn=foo,dc=ex,dc=com'); + $this->entry = Entry::fromArray('cn=foo,dc=ex,dc=com', ['cn' => ['foo']]); $this->token = BindToken::fromDn('cn=foo,dc=ex,dc=com'); $this->subject = new ReadEntryControlHandler( - $this->backend, new Schema(), new RuleBasedAccessControl(AclRules::fromEmpty(operations: self::searchAllowed())), ); @@ -50,41 +43,29 @@ protected function setUp(): void public function test_pre_read_returns_null_without_the_control(): void { - $this->backend - ->expects(self::never()) - ->method('get'); - self::assertNull( - $this->subject->preRead($this->dn, new ControlBag(), $this->token), + $this->subject->preRead($this->entry, new ControlBag(), $this->token), ); } public function test_post_read_returns_null_without_the_control(): void { self::assertNull( - $this->subject->postRead($this->dn, new ControlBag(), $this->token), + $this->subject->postRead($this->entry, new ControlBag(), $this->token), ); } - public function test_pre_read_returns_null_when_the_entry_is_absent(): void + public function test_pre_read_returns_null_when_the_write_kept_no_entry(): void { - $this->backend - ->method('get') - ->willReturn(null); - self::assertNull( - $this->subject->preRead($this->dn, new ControlBag(new PreReadControl()), $this->token), + $this->subject->preRead(null, new ControlBag(new PreReadControl()), $this->token), ); } public function test_pre_read_returns_a_response_control_with_the_entry(): void { - $this->backend - ->method('get') - ->willReturn(Entry::fromArray('cn=foo,dc=ex,dc=com', ['cn' => ['foo']])); - $control = $this->subject->preRead( - $this->dn, + $this->entry, new ControlBag(new PreReadControl()), $this->token, ); @@ -98,15 +79,11 @@ public function test_pre_read_returns_a_response_control_with_the_entry(): void public function test_post_read_projects_only_the_requested_attributes(): void { - $this->backend - ->method('get') - ->willReturn(Entry::fromArray('cn=foo,dc=ex,dc=com', [ + $control = $this->subject->postRead( + Entry::fromArray('cn=foo,dc=ex,dc=com', [ 'cn' => ['foo'], 'sn' => ['bar'], - ])); - - $control = $this->subject->postRead( - $this->dn, + ]), new ControlBag(new PostReadControl('cn')), $this->token, ); @@ -121,21 +98,15 @@ public function test_post_read_projects_only_the_requested_attributes(): void ); } - public function test_the_snapshot_is_isolated_from_later_mutation_of_the_stored_entry(): void + public function test_the_control_is_isolated_from_later_mutation_of_the_entry(): void { - $stored = Entry::fromArray('cn=foo,dc=ex,dc=com', ['cn' => ['foo']]); - $this->backend - ->method('get') - ->willReturn($stored); - $control = $this->subject->preRead( - $this->dn, + $this->entry, new ControlBag(new PreReadControl()), $this->token, ); - // Mutate the stored entry in place after the snapshot was taken. - $stored->get('cn')?->add('changed'); + $this->entry->get('cn')?->add('changed'); self::assertInstanceOf(PreReadResponseControl::class, $control); self::assertSame( @@ -146,15 +117,11 @@ public function test_the_snapshot_is_isolated_from_later_mutation_of_the_stored_ public function test_pre_read_omits_an_attribute_the_token_may_not_read(): void { - $this->backend - ->method('get') - ->willReturn(Entry::fromArray('cn=foo,dc=ex,dc=com', [ + $control = $this->denyingUserPassword()->preRead( + Entry::fromArray('cn=foo,dc=ex,dc=com', [ 'cn' => ['foo'], 'userPassword' => ['{SSHA}secret'], - ])); - - $control = $this->denyingUserPassword()->preRead( - $this->dn, + ]), new ControlBag(new PreReadControl()), $this->token, ); @@ -169,15 +136,11 @@ public function test_pre_read_omits_an_attribute_the_token_may_not_read(): void public function test_post_read_cannot_name_an_attribute_the_token_may_not_read(): void { - $this->backend - ->method('get') - ->willReturn(Entry::fromArray('cn=foo,dc=ex,dc=com', [ + $control = $this->denyingUserPassword()->postRead( + Entry::fromArray('cn=foo,dc=ex,dc=com', [ 'cn' => ['foo'], 'userPassword' => ['{SSHA}secret'], - ])); - - $control = $this->denyingUserPassword()->postRead( - $this->dn, + ]), new ControlBag(new PostReadControl('userPassword')), $this->token, ); @@ -191,18 +154,13 @@ public function test_post_read_cannot_name_an_attribute_the_token_may_not_read() public function test_no_control_is_returned_when_the_token_may_not_read_the_target(): void { - $this->backend - ->method('get') - ->willReturn(Entry::fromArray('cn=foo,dc=ex,dc=com', ['cn' => ['foo']])); - $subject = new ReadEntryControlHandler( - $this->backend, new Schema(), new RuleBasedAccessControl(AclRules::fromEmpty()), ); self::assertNull($subject->preRead( - $this->dn, + $this->entry, new ControlBag(new PreReadControl()), $this->token, )); @@ -225,7 +183,6 @@ private static function searchAllowed(): array private function denyingUserPassword(): ReadEntryControlHandler { return new ReadEntryControlHandler( - $this->backend, new Schema(), new RuleBasedAccessControl(AclRules::fromEmpty( operations: self::searchAllowed(), diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php index 45966556..7f1aad09 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php @@ -14,6 +14,13 @@ namespace Tests\Unit\FreeDSx\Ldap\Protocol\ServerProtocolHandler; use FreeDSx\Ldap\Control\Control; +use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PostReadResponseControl; +use FreeDSx\Ldap\Controls; +use FreeDSx\Ldap\Operations; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; +use FreeDSx\Ldap\Server\Backend\Storage\Filter\FilterEvaluatorInterface; +use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; @@ -52,21 +59,66 @@ final class ServerDispatchHandlerTest extends TestCase private AccessControlInterface&MockObject $mockAccessControl; + private FilterEvaluatorInterface&MockObject $mockFilterEvaluator; + protected function setUp(): void { $this->mockToken = $this->createMock(TokenInterface::class); $this->mockBackend = $this->createMock(ReadBackendInterface::class); $this->mockWriteHandler = $this->createMock(WriteHandlerInterface::class); $this->mockAccessControl = $this->createMock(AccessControlInterface::class); + $this->mockFilterEvaluator = $this->createMock(FilterEvaluatorInterface::class); $this->subject = new ServerDispatchHandler( backend: $this->mockBackend, router: new WriteRequestRouter($this->mockWriteHandler), accessControl: $this->mockAccessControl, + assertions: new AssertionEvaluator( + $this->mockFilterEvaluator, + $this->mockBackend, + $this->mockAccessControl, + ), schema: new Schema(), ); } + public function test_a_post_read_is_built_from_the_entry_the_write_kept(): void + { + $this->mockBackend + ->expects(self::never()) + ->method('get'); + $this->mockAccessControl + ->method('filterEntry') + ->willReturnArgument(1); + $this->mockWriteHandler + ->method('handle') + ->willReturnCallback(static function (WriteRequestInterface $request, WriteContext $context): void { + $context->controlEvaluator()?->captureResult(Entry::fromArray( + 'cn=foo,dc=bar', + ['cn' => ['kept']], + )); + }); + + $stream = $this->subject->handleRequest( + new LdapMessageRequest( + 1, + Operations::modify('cn=foo,dc=bar'), + new PostReadControl('cn'), + ), + $this->mockToken, + ); + $postRead = ([...$stream->messages][0])->controls()->get(Control::OID_POST_READ); + + self::assertInstanceOf( + PostReadResponseControl::class, + $postRead, + ); + self::assertSame( + ['kept'], + $postRead->getEntry()->get('cn')?->getValues(), + ); + } + public function test_it_dispatches_write_requests_through_the_write_handler(): void { $add = new LdapMessageRequest(1, new AddRequest(Entry::create('cn=foo,dc=bar'))); @@ -102,20 +154,30 @@ public function test_it_lets_operation_exceptions_from_the_write_handler_bubble( $this->subject->handleRequest($add, $this->mockToken); } - public function test_it_delegates_compare_to_the_backend(): void + public function test_it_compares_the_entry_it_reads_once(): void { - $filter = Filters::equal('foo', 'bar'); - $compare = new LdapMessageRequest(1, new CompareRequest('cn=foo,dc=bar', $filter)); + $entry = Entry::fromArray( + 'cn=foo,dc=bar', + ['foo' => ['bar']], + ); + $compare = new LdapMessageRequest(1, new CompareRequest('cn=foo,dc=bar', Filters::equal('foo', 'bar'))); $this->mockWriteHandler ->expects(self::never()) ->method('handle'); - + $this->mockBackend + ->expects(self::never()) + ->method('get'); + $this->mockBackend + ->expects(self::once()) + ->method('getOrFail') + ->with(self::isInstanceOf(Dn::class)) + ->willReturn($entry); $this->mockBackend ->expects(self::once()) ->method('compare') ->with( - self::isInstanceOf(Dn::class), + $entry, self::isInstanceOf(EqualityFilter::class), ) ->willReturn(true); @@ -125,16 +187,23 @@ public function test_it_delegates_compare_to_the_backend(): void self::assertInstanceOf(CompareOperationResult::class, $outcome); } - public function test_it_lets_operation_exceptions_from_backend_compare_bubble(): void + public function test_a_missing_compare_target_answers_before_its_assertion(): void { - $compare = new LdapMessageRequest(1, new CompareRequest('cn=foo,dc=bar', Filters::equal('foo', 'bar'))); + $compare = new LdapMessageRequest( + 1, + new CompareRequest('cn=foo,dc=bar', Filters::equal('foo', 'bar')), + Controls::assertion(Filters::equal('foo', 'nope')), + ); $this->mockBackend - ->method('compare') + ->method('getOrFail') ->willThrowException(new OperationException( 'No such object: cn=foo,dc=bar', ResultCode::NO_SUCH_OBJECT, )); + $this->mockFilterEvaluator + ->expects(self::never()) + ->method('evaluate'); $this->expectException(OperationException::class); $this->expectExceptionCode(ResultCode::NO_SUCH_OBJECT); @@ -142,6 +211,36 @@ public function test_it_lets_operation_exceptions_from_backend_compare_bubble(): $this->subject->handleRequest($compare, $this->mockToken); } + public function test_a_failing_assertion_refuses_the_compare_before_it_is_evaluated(): void + { + $compare = new LdapMessageRequest( + 1, + new CompareRequest('cn=foo,dc=bar', Filters::equal('foo', 'bar')), + Controls::assertion(Filters::equal('foo', 'nope')), + ); + + $this->mockBackend + ->method('getOrFail') + ->willReturn(Entry::fromArray( + 'cn=foo,dc=bar', + ['foo' => ['bar']], + )); + $this->mockAccessControl + ->method('stripUnreadableAttributes') + ->willReturnArgument(1); + $this->mockFilterEvaluator + ->method('evaluate') + ->willReturn(false); + $this->mockBackend + ->expects(self::never()) + ->method('compare'); + + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->subject->handleRequest($compare, $this->mockToken); + } + public function test_a_write_the_handler_refuses_surfaces_its_result_code(): void { $this->mockWriteHandler diff --git a/tests/unit/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactorTest.php b/tests/unit/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactorTest.php index d9be5b84..cc0e0e5c 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactorTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/Pdo/Connection/PdoTransactorTest.php @@ -13,8 +13,10 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection; +use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\PdoDialectInterface; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection\PdoTransactor; +use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageBusyException; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection\SharedPdoConnectionProvider; use FreeDSx\Ldap\Server\Utility\ExponentialBackoff; use PDO; @@ -76,22 +78,30 @@ public function test_it_reissues_the_transaction_until_the_conflict_clears(): vo ); } - public function test_it_gives_up_once_the_retry_budget_is_spent(): void + public function test_it_answers_busy_once_the_retry_budget_is_spent(): void { $this->dialect->method('isRetryableConflict') ->willReturn(true); $attempts = 0; + $conflict = new PDOException('Deadlock found when trying to get lock'); try { - $this->subject->atomic(function () use (&$attempts): void { + $this->subject->atomic(function () use (&$attempts, $conflict): void { $attempts++; - throw new PDOException('Deadlock found when trying to get lock'); + throw $conflict; }); - self::fail('Expected the conflict to be rethrown.'); - } catch (PDOException) { - // Expected once the budget is spent. + self::fail('Expected the spent budget to answer busy.'); + } catch (StorageBusyException $e) { + self::assertSame( + ResultCode::BUSY, + $e->getCode(), + ); + self::assertSame( + $conflict, + $e->getPrevious(), + ); } self::assertSame( @@ -100,6 +110,29 @@ public function test_it_gives_up_once_the_retry_budget_is_spent(): void ); } + public function test_a_nested_transaction_hands_the_raw_conflict_to_the_outermost_one(): void + { + $this->dialect->method('isRetryableConflict') + ->willReturn(true); + + $caught = null; + + $this->subject->atomic(function () use (&$caught): void { + try { + $this->subject->atomic(static function (): void { + throw new PDOException('Deadlock found when trying to get lock'); + }); + } catch (PDOException $e) { + $caught = $e; + } + }); + + self::assertInstanceOf( + PDOException::class, + $caught, + ); + } + public function test_it_does_not_reissue_a_failure_the_dialect_does_not_own(): void { $this->dialect->method('isRetryableConflict') @@ -145,8 +178,8 @@ public function test_it_reissues_only_the_outermost_transaction(): void throw new PDOException('Deadlock found when trying to get lock'); }); }); - self::fail('Expected the conflict to be rethrown.'); - } catch (PDOException) { + self::fail('Expected the spent budget to answer busy.'); + } catch (StorageBusyException) { // Expected once the budget is spent. } @@ -235,8 +268,8 @@ public function test_it_reissues_a_swallowed_nested_conflict(): void } catch (PDOException) { } }); - self::fail('Expected the conflict to be rethrown.'); - } catch (PDOException) { + self::fail('Expected the spent budget to answer busy.'); + } catch (StorageBusyException) { // Expected once the budget is spent. } diff --git a/tests/unit/Server/Backend/StorageReadBackendTest.php b/tests/unit/Server/Backend/StorageReadBackendTest.php index 5714591b..81bea663 100644 --- a/tests/unit/Server/Backend/StorageReadBackendTest.php +++ b/tests/unit/Server/Backend/StorageReadBackendTest.php @@ -560,10 +560,35 @@ public function test_no_such_object_on_search_subtree_carries_matched_dn(): void } } + public function test_it_gets_an_entry_that_exists_or_fails(): void + { + self::assertSame( + 'cn=Alice,dc=example,dc=com', + $this->subject->getOrFail(new Dn('cn=Alice,dc=example,dc=com'))->getDn()->toString(), + ); + } + + public function test_getting_a_missing_entry_answers_no_such_object_carrying_the_matched_dn(): void + { + try { + $this->subject->getOrFail(new Dn('cn=Nobody,dc=example,dc=com')); + self::fail('Expected OperationException was not thrown.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::NO_SUCH_OBJECT, + $e->getCode(), + ); + self::assertSame( + 'dc=example,dc=com', + $e->getMatchedDn()?->toString(), + ); + } + } + public function test_compare_answers_true_for_a_matching_value(): void { self::assertTrue($this->subject->compare( - new Dn('cn=Alice,dc=example,dc=com'), + $this->alice, new EqualityFilter('cn', 'Alice'), )); } @@ -571,7 +596,7 @@ public function test_compare_answers_true_for_a_matching_value(): void public function test_compare_answers_false_for_a_differing_value(): void { self::assertFalse($this->subject->compare( - new Dn('cn=Alice,dc=example,dc=com'), + $this->alice, new EqualityFilter('cn', 'Nobody'), )); } @@ -582,7 +607,7 @@ public function test_compare_answers_false_for_a_differing_value(): void public function test_compare_answers_false_when_the_entry_lacks_the_attribute(): void { self::assertFalse($this->subject->compare( - new Dn('cn=Alice,dc=example,dc=com'), + $this->alice, new EqualityFilter('description', 'anything'), )); } @@ -593,7 +618,7 @@ public function test_compare_refuses_an_unrecognized_attribute_type(): void $this->expectExceptionCode(ResultCode::UNDEFINED_ATTRIBUTE_TYPE); $this->subject->compare( - new Dn('cn=Alice,dc=example,dc=com'), + $this->alice, new EqualityFilter('shoeSize', '9'), ); } @@ -607,41 +632,11 @@ public function test_compare_refuses_an_assertion_value_the_syntax_rejects(): vo $this->expectExceptionCode(ResultCode::INVALID_ATTRIBUTE_SYNTAX); $this->subject->compare( - new Dn('cn=Alice,dc=example,dc=com'), + $this->alice, new EqualityFilter('c', 'UnitedStates'), ); } - /** - * The entry is located first, so a missing entry outranks anything the assertion itself is wrong about. - */ - public function test_compare_reports_a_missing_entry_before_an_undefined_assertion(): void - { - $this->expectException(OperationException::class); - $this->expectExceptionCode(ResultCode::NO_SUCH_OBJECT); - - $this->subject->compare( - new Dn('cn=Nobody,dc=example,dc=com'), - new EqualityFilter('shoeSize', '9'), - ); - } - - public function test_no_such_object_on_compare_carries_matched_dn(): void - { - try { - $this->subject->compare( - new Dn('cn=Nobody,dc=example,dc=com'), - new EqualityFilter('cn', 'Nobody'), - ); - self::fail('Expected OperationException was not thrown.'); - } catch (OperationException $e) { - self::assertSame( - 'dc=example,dc=com', - $e->getMatchedDn()?->toString(), - ); - } - } - public function test_no_such_object_with_no_existing_ancestor_has_null_matched_dn(): void { $backend = $this->backendFor(new InMemoryStorage()); diff --git a/tests/unit/Server/Backend/Write/Operation/AddEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/AddEntryHandlerTest.php index 6f6f0a0d..f2a79eb8 100644 --- a/tests/unit/Server/Backend/Write/Operation/AddEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/AddEntryHandlerTest.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Dn; @@ -22,6 +23,7 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Schema\SchemaValidationMode; +use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; @@ -77,6 +79,89 @@ public function test_it_does_not_repeat_a_naming_value_held_under_an_equivalent_ ); } + public function test_the_added_entry_is_kept_for_a_post_read(): void + { + $context = $this->controlledContext(new PostReadControl('cn')); + + $this->adds()->handle( + new AddCommand(new Entry( + new Dn('cn=New,dc=example,dc=com'), + new Attribute('cn', 'New'), + )), + $context, + ); + + $postRead = $context->controlEvaluator()?->postReadEntry(); + self::assertNotNull($postRead); + self::assertSame( + ['New'], + $postRead->get('cn')?->getValues(), + ); + self::assertNotNull($postRead->get('entryUUID')); + } + + public function test_a_failing_assertion_refuses_the_add_and_stores_nothing(): void + { + try { + $this->adds()->handle( + new AddCommand(new Entry( + new Dn('cn=New,dc=example,dc=com'), + new Attribute('cn', 'New'), + )), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Other'))), + ); + self::fail('The assertion should have refused the add.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNull($this->find('cn=New,dc=example,dc=com')); + } + + public function test_an_assertion_is_evaluated_against_the_entry_as_it_will_be_stored(): void + { + $this->adds()->handle( + new AddCommand(new Entry( + new Dn('cn=New,dc=example,dc=com'), + new Attribute('cn', 'New'), + )), + $this->controlledContext(Controls::assertion(Filters::and( + Filters::equal('cn', 'New'), + Filters::present('entryUUID'), + ))), + ); + + self::assertNotNull($this->find('cn=New,dc=example,dc=com')); + } + + public function test_a_missing_parent_answers_before_the_assertion(): void + { + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::NO_SUCH_OBJECT); + + $this->adds()->handle( + new AddCommand(new Entry( + new Dn('cn=New,ou=Missing,dc=example,dc=com'), + new Attribute('cn', 'New'), + )), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Other'))), + ); + } + + public function test_an_existing_entry_answers_before_the_assertion(): void + { + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::ENTRY_ALREADY_EXISTS); + + $this->adds()->handle( + new AddCommand($this->alice), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Other'))), + ); + } + public function test_it_refuses_an_entry_that_already_exists(): void { self::expectException(OperationException::class); diff --git a/tests/unit/Server/Backend/Write/Operation/DeleteEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/DeleteEntryHandlerTest.php index d0322a48..1225c54a 100644 --- a/tests/unit/Server/Backend/Write/Operation/DeleteEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/DeleteEntryHandlerTest.php @@ -13,11 +13,14 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; @@ -79,6 +82,50 @@ public function test_a_missing_entry_carries_the_deepest_ancestor_as_the_matched } } + public function test_a_failing_assertion_refuses_the_delete_and_leaves_the_entry(): void + { + try { + $this->deletes()->handle( + new DeleteCommand(new Dn('cn=Alice,dc=example,dc=com')), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Nobody'))), + ); + self::fail('The assertion should have refused the delete.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNotNull($this->find('cn=Alice,dc=example,dc=com')); + } + + public function test_an_assertion_answers_before_the_subordinate_check(): void + { + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->deletes()->handle( + new DeleteCommand(new Dn('dc=example,dc=com')), + $this->controlledContext(Controls::assertion(Filters::equal('dc', 'nomatch'))), + ); + } + + public function test_the_deleted_entry_is_kept_for_a_pre_read(): void + { + $context = $this->controlledContext(new PreReadControl('cn')); + + $this->deletes()->handle( + new DeleteCommand(new Dn('cn=Alice,dc=example,dc=com')), + $context, + ); + + self::assertSame( + ['Alice'], + $context->controlEvaluator()?->preReadEntry()?->get('cn')?->getValues(), + ); + } + public function test_it_refuses_an_entry_that_holds_subordinates(): void { self::expectException(OperationException::class); diff --git a/tests/unit/Server/Backend/Write/Operation/MoveEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/MoveEntryHandlerTest.php index 4e784c00..e2d7f466 100644 --- a/tests/unit/Server/Backend/Write/Operation/MoveEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/MoveEntryHandlerTest.php @@ -13,12 +13,16 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Entry\Rdn; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; use FreeDSx\Ldap\Server\Backend\Storage\EntryStream; @@ -95,6 +99,65 @@ public function test_it_refuses_an_entry_that_does_not_exist(): void $this->rename('cn=Nobody,dc=example,dc=com', 'cn=Ghost'); } + public function test_a_failing_assertion_refuses_the_rename_and_leaves_the_entry(): void + { + try { + $this->moves()->handle( + new MoveCommand( + new Dn(self::ALICE), + Rdn::create('cn=Alicia'), + true, + null, + ), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Nobody'))), + ); + self::fail('The assertion should have refused the rename.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertNotNull($this->find(self::ALICE)); + self::assertNull($this->find('cn=Alicia,dc=example,dc=com')); + } + + public function test_the_entries_around_the_rename_are_kept_for_the_read_controls(): void + { + $context = $this->controlledContext( + new PreReadControl('cn'), + new PostReadControl('cn'), + ); + + $this->moves()->handle( + new MoveCommand( + new Dn(self::ALICE), + Rdn::create('cn=Alicia'), + true, + null, + ), + $context, + ); + + $kept = $context->controlEvaluator(); + self::assertNotNull($kept); + self::assertSame( + self::ALICE, + $kept->preReadEntry()?->getDn()->toString(), + ); + $postRead = $kept->postReadEntry(); + self::assertNotNull($postRead); + self::assertSame( + 'cn=Alicia,dc=example,dc=com', + $postRead->getDn()->toString(), + ); + self::assertSame( + ['Alicia'], + $postRead->get('cn')?->getValues(), + ); + } + public function test_it_relocates_an_entry_that_has_children(): void { $this->addPeopleOu(); diff --git a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php index eedc35cd..767fcfaf 100644 --- a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php @@ -14,6 +14,9 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; @@ -21,6 +24,7 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Schema\SchemaValidationMode; +use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Server\Backend\Storage\EntryStorageInterface; use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; @@ -101,6 +105,71 @@ public function test_a_replace_with_no_values_clears_the_attribute(): void self::assertNull($this->find(self::ALICE)?->get('userPassword')); } + public function test_a_failing_assertion_refuses_the_modify_and_leaves_the_entry(): void + { + try { + $this->updates()->handle( + new UpdateCommand( + new Dn(self::ALICE), + [Change::replace('userPassword', 'changed')], + ), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Nobody'))), + ); + self::fail('The assertion should have refused the modify.'); + } catch (OperationException $e) { + self::assertSame( + ResultCode::ASSERTION_FAILED, + $e->getCode(), + ); + } + + self::assertSame( + ['secret'], + $this->find(self::ALICE)?->get('userPassword')?->getValues(), + ); + } + + public function test_a_missing_entry_answers_before_its_assertion(): void + { + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::NO_SUCH_OBJECT); + + $this->updates()->handle( + new UpdateCommand( + new Dn('cn=Nobody,dc=example,dc=com'), + [Change::replace('sn', 'Nobody')], + ), + $this->controlledContext(Controls::assertion(Filters::equal('cn', 'Somebody'))), + ); + } + + public function test_the_entries_around_the_modify_are_kept_for_the_read_controls(): void + { + $context = $this->controlledContext( + new PreReadControl('userPassword'), + new PostReadControl('userPassword'), + ); + + $this->updates()->handle( + new UpdateCommand( + new Dn(self::ALICE), + [Change::replace('userPassword', 'changed')], + ), + $context, + ); + + $kept = $context->controlEvaluator(); + self::assertNotNull($kept); + self::assertSame( + ['secret'], + $kept->preReadEntry()?->get('userPassword')?->getValues(), + ); + self::assertSame( + ['changed'], + $kept->postReadEntry()?->get('userPassword')?->getValues(), + ); + } + public function test_it_refuses_an_entry_that_does_not_exist(): void { self::expectException(OperationException::class); diff --git a/tests/unit/Server/Backend/Write/Replay/ReplayWriteHandlerTest.php b/tests/unit/Server/Backend/Write/Replay/ReplayWriteHandlerTest.php index 0be890c3..18dfe540 100644 --- a/tests/unit/Server/Backend/Write/Replay/ReplayWriteHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Replay/ReplayWriteHandlerTest.php @@ -19,6 +19,7 @@ use FreeDSx\Ldap\Operation\Request\AddRequest; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Protocol\LdapMessageRequest; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Server\Backend\Write\Replay\ReplayWriteHandler; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use FreeDSx\Ldap\Server\Backend\Write\WriteHandlerInterface; @@ -29,9 +30,12 @@ use FreeDSx\Ldap\Server\Token\SystemToken; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; final class ReplayWriteHandlerTest extends TestCase { + use ServerContainerTrait; + private const DN = 'cn=foo,dc=example,dc=com'; private WriteHandlerInterface&MockObject $writeHandler; @@ -42,7 +46,27 @@ protected function setUp(): void { $this->writeHandler = $this->createMock(WriteHandlerInterface::class); - $this->subject = new ReplayWriteHandler(new WriteRequestRouter($this->writeHandler)); + $this->subject = new ReplayWriteHandler( + new WriteRequestRouter($this->writeHandler), + $this->fromContainer(AssertionEvaluator::class), + ); + } + + public function test_the_write_is_handed_its_controls_to_evaluate_under_the_lock(): void + { + $seen = null; + $this->writeHandler + ->method('handle') + ->willReturnCallback(static function ( + WriteRequestInterface $request, + WriteContext $context, + ) use (&$seen): void { + $seen = $context->controlEvaluator(); + }); + + $this->subject->handle($this->contextWith()); + + self::assertNotNull($seen); } public function test_it_applies_the_write_and_reports_success(): void diff --git a/tests/unit/Server/Backend/Write/WriteControlEvaluatorTest.php b/tests/unit/Server/Backend/Write/WriteControlEvaluatorTest.php new file mode 100644 index 00000000..7b529134 --- /dev/null +++ b/tests/unit/Server/Backend/Write/WriteControlEvaluatorTest.php @@ -0,0 +1,159 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write; + +use FreeDSx\Ldap\Control\Control; +use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Control\ReadEntry\PostReadControl; +use FreeDSx\Ldap\Control\ReadEntry\PreReadControl; +use FreeDSx\Ldap\Controls; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; +use FreeDSx\Ldap\Search\Filters; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; +use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Filter\FilterEvaluatorInterface; +use FreeDSx\Ldap\Server\Backend\Write\WriteControlEvaluator; +use FreeDSx\Ldap\Server\Token\BindToken; +use PHPUnit\Framework\TestCase; +use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; + +final class WriteControlEvaluatorTest extends TestCase +{ + use ServerContainerTrait; + + private AssertionEvaluator $assertions; + + private Entry $entry; + + protected function setUp(): void + { + $this->assertions = new AssertionEvaluator( + $this->fromContainer(FilterEvaluatorInterface::class), + $this->createMock(ReadBackendInterface::class), + new RuleBasedAccessControl(AclRules::fromEmpty()), + ); + $this->entry = Entry::fromArray( + 'cn=foo,dc=ex,dc=com', + ['cn' => ['foo'], 'sn' => ['Smith']], + ); + } + + public function test_a_target_satisfying_the_assertion_is_accepted(): void + { + $subject = $this->evaluatorFor(Controls::assertion(Filters::equal('sn', 'Smith'))); + + $subject->evaluateTarget($this->entry); + + self::assertNull($subject->preReadEntry()); + } + + public function test_a_target_failing_the_assertion_answers_assertion_failed(): void + { + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->evaluatorFor(Controls::assertion(Filters::equal('sn', 'Jones'))) + ->evaluateTarget($this->entry); + } + + public function test_a_pre_read_keeps_a_copy_of_the_target(): void + { + $subject = $this->evaluatorFor(new PreReadControl('sn')); + + $subject->evaluateTarget($this->entry); + $this->entry->get('sn')?->set('Jones'); + + self::assertSame( + ['Smith'], + $subject->preReadEntry()?->get('sn')?->getValues(), + ); + } + + public function test_a_post_read_keeps_a_copy_of_the_result(): void + { + $subject = $this->evaluatorFor(new PostReadControl('sn')); + + $subject->captureResult($this->entry); + $this->entry->get('sn')?->set('Jones'); + + self::assertSame( + ['Smith'], + $subject->postReadEntry()?->get('sn')?->getValues(), + ); + } + + public function test_an_addition_failing_the_assertion_answers_assertion_failed(): void + { + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ASSERTION_FAILED); + + $this->evaluatorFor(Controls::assertion(Filters::equal('sn', 'Jones'))) + ->evaluateAddition($this->entry); + } + + public function test_an_addition_satisfying_the_assertion_is_kept_for_a_post_read(): void + { + $subject = $this->evaluatorFor( + Controls::assertion(Filters::equal('sn', 'Smith')), + new PostReadControl('sn'), + ); + + $subject->evaluateAddition($this->entry); + + self::assertSame( + ['Smith'], + $subject->postReadEntry()?->get('sn')?->getValues(), + ); + } + + public function test_nothing_is_kept_without_a_read_control(): void + { + $subject = $this->evaluatorFor(); + + $subject->evaluateTarget($this->entry); + $subject->captureResult($this->entry); + + self::assertNull($subject->preReadEntry()); + self::assertNull($subject->postReadEntry()); + } + + public function test_a_later_attempt_replaces_what_an_earlier_one_kept(): void + { + $subject = $this->evaluatorFor(new PostReadControl('sn')); + + $subject->captureResult($this->entry); + $subject->captureResult(Entry::fromArray( + 'cn=foo,dc=ex,dc=com', + ['sn' => ['Jones']], + )); + + self::assertSame( + ['Jones'], + $subject->postReadEntry()?->get('sn')?->getValues(), + ); + } + + private function evaluatorFor(Control ...$controls): WriteControlEvaluator + { + return new WriteControlEvaluator( + $this->assertions, + BindToken::fromDn('cn=foo,dc=ex,dc=com'), + new ControlBag(...$controls), + ); + } +} diff --git a/tests/unit/Server/Middleware/AssertionMiddlewareTest.php b/tests/unit/Server/Middleware/AssertionMiddlewareTest.php index 2b08a4f9..31ec1377 100644 --- a/tests/unit/Server/Middleware/AssertionMiddlewareTest.php +++ b/tests/unit/Server/Middleware/AssertionMiddlewareTest.php @@ -22,6 +22,7 @@ use FreeDSx\Ldap\Operation\Request\RequestInterface; use FreeDSx\Ldap\Operation\Request\SearchRequest; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Operations; use FreeDSx\Ldap\Protocol\LdapMessageRequest; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AssertionEvaluator; use FreeDSx\Ldap\Search\Filters; @@ -29,7 +30,6 @@ use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Storage\Filter\FilterEvaluatorInterface; -use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; use FreeDSx\Ldap\Server\Middleware\AssertionMiddleware; use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext; use FreeDSx\Ldap\Server\Token\TokenInterface; @@ -37,6 +37,7 @@ use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\Middleware\CallLog; use Tests\Support\FreeDSx\Ldap\Middleware\RecordingMiddlewareHandler; +use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; final class AssertionMiddlewareTest extends TestCase { @@ -69,7 +70,7 @@ protected function setUp(): void public function test_it_delegates_when_no_assertion_control_is_present(): void { $this->subject->process( - $this->contextFor(new DeleteRequest('cn=foo,dc=bar')), + $this->contextFor($this->search()), $this->next, ); @@ -80,7 +81,7 @@ public function test_it_delegates_when_the_assertion_matches(): void { $this->subject->process( $this->contextFor( - new DeleteRequest('cn=foo,dc=bar'), + $this->search(), Controls::assertion(Filters::equal('cn', 'foo')), ), $this->next, @@ -89,12 +90,12 @@ public function test_it_delegates_when_the_assertion_matches(): void self::assertNotNull($this->next->received); } - public function test_it_throws_and_stops_the_chain_when_the_assertion_does_not_match(): void + public function test_it_throws_and_stops_the_chain_when_the_assertion_does_not_match_the_search_base(): void { try { $this->subject->process( $this->contextFor( - new DeleteRequest('cn=foo,dc=bar'), + $this->search(), Controls::assertion(Filters::equal('cn', 'nope')), ), $this->next, @@ -113,40 +114,57 @@ public function test_it_throws_and_stops_the_chain_when_the_assertion_does_not_m ); } - public function test_it_resolves_the_search_base_as_the_target(): void + public function test_it_skips_assertion_on_a_paging_continuation(): void { - $search = (new SearchRequest(Filters::equal('cn', 'foo'))) - ->base('cn=foo,dc=bar'); + $this->subject->process( + $this->contextFor( + $this->search(), + Controls::assertion(Filters::equal('cn', 'nope')), + new PagingControl(10, 'continuation-cookie'), + ), + $this->next, + ); - $this->expectException(OperationException::class); + self::assertNotNull( + $this->next->received, + 'A non-matching assertion on a continuation page is not re-evaluated, so the chain proceeds.', + ); + } + public function test_a_compare_is_passed_on_to_be_evaluated_against_the_entry_it_reads(): void + { $this->subject->process( $this->contextFor( - $search, + Operations::compare( + 'cn=foo,dc=bar', + 'cn', + 'foo', + ), Controls::assertion(Filters::equal('cn', 'nope')), ), $this->next, ); + + self::assertNotNull($this->next->received); } - public function test_it_skips_assertion_on_a_paging_continuation(): void + public function test_a_write_is_passed_on_for_its_handler_to_evaluate_under_the_lock(): void { - $search = (new SearchRequest(Filters::equal('cn', 'foo'))) - ->base('cn=foo,dc=bar'); - $this->subject->process( $this->contextFor( - $search, + new DeleteRequest('cn=foo,dc=bar'), Controls::assertion(Filters::equal('cn', 'nope')), - new PagingControl(10, 'continuation-cookie'), ), $this->next, ); - self::assertNotNull( - $this->next->received, - 'A non-matching assertion on a continuation page is not re-evaluated, so the chain proceeds.', - ); + self::assertNotNull($this->next->received); + } + + private function search(): SearchRequest + { + return (new SearchRequest(Filters::equal('cn', 'foo'))) + ->base('cn=foo,dc=bar'); } private function contextFor( diff --git a/tests/unit/Server/Middleware/OperationAuthorizationMiddlewareTest.php b/tests/unit/Server/Middleware/OperationAuthorizationMiddlewareTest.php index dbd0a54c..94ee88d8 100644 --- a/tests/unit/Server/Middleware/OperationAuthorizationMiddlewareTest.php +++ b/tests/unit/Server/Middleware/OperationAuthorizationMiddlewareTest.php @@ -722,6 +722,35 @@ public function test_a_non_critical_assertion_still_authorizes_a_read_of_the_tar ); } + public function test_an_assertion_on_a_subtree_delete_authorizes_no_read(): void + { + $this->routeResolvesTo(HandlerId::Dispatch); + $seen = []; + $this->accessControl + ->method('authorizeOperation') + ->willReturnCallback(function (OperationType $operation, TokenInterface $token, Dn $dn) use (&$seen): void { + if ($operation === OperationType::Search) { + $seen[] = $dn->toString(); + } + }); + $assertion = Controls::assertion(Filters::equal('cn', 'foo')); + $assertion->setCriticality(false); + + $this->subject->process( + $this->contextFor( + new DeleteRequest('cn=foo,dc=bar'), + $assertion, + Controls::subtreeDelete(), + ), + $this->next, + ); + + self::assertSame( + [], + $seen, + ); + } + public function test_a_request_without_a_read_bearing_control_authorizes_no_read(): void { $this->routeResolvesTo(HandlerId::Dispatch); diff --git a/tests/unit/Server/Middleware/ServerControlRegistryTest.php b/tests/unit/Server/Middleware/ServerControlRegistryTest.php index 155f187b..c9b0b9af 100644 --- a/tests/unit/Server/Middleware/ServerControlRegistryTest.php +++ b/tests/unit/Server/Middleware/ServerControlRegistryTest.php @@ -14,6 +14,8 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\Middleware; use FreeDSx\Ldap\Control\Control; +use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\Request\RequestInterface; use FreeDSx\Ldap\Operation\Request\SearchRequest; @@ -47,6 +49,7 @@ public function test_search_supports_expected_controls(): void $this->subject->supportedControlsFor( HandlerId::Search, self::searchRequest(), + new ControlBag(), ), ); } @@ -66,6 +69,7 @@ public function test_paging_supports_expected_controls(): void $this->subject->supportedControlsFor( HandlerId::Paging, self::searchRequest(), + new ControlBag(), ), ); } @@ -137,6 +141,25 @@ public function test_dispatch_supports_the_controls_its_operation_takes( $this->subject->supportedControlsFor( HandlerId::Dispatch, $request, + new ControlBag(), + ), + ); + } + + public function test_a_subtree_delete_supports_neither_the_assertion_nor_pre_read(): void + { + self::assertSame( + [ + Control::OID_PROXY_AUTHORIZATION, + Control::OID_MANAGE_DSA_IT, + Control::OID_PWD_POLICY, + Control::OID_RELAX_RULES, + Control::OID_SUBTREE_DELETE, + ], + $this->subject->supportedControlsFor( + HandlerId::Dispatch, + Operations::delete('cn=foo,dc=foo,dc=bar'), + new ControlBag(Controls::subtreeDelete()), ), ); } @@ -156,6 +179,7 @@ public function test_handlers_without_specific_controls_support_only_the_global_ $this->subject->supportedControlsFor( $id, self::searchRequest(), + new ControlBag(), ), ); } @@ -171,6 +195,7 @@ public function test_the_password_policy_control_is_supported_on_every_checked_h $this->subject->supportedControlsFor( $id, self::searchRequest(), + new ControlBag(), ), ); }