diff --git a/docs/Server/Configuration.md b/docs/Server/Configuration.md index fbf47abe..0c37eba7 100644 --- a/docs/Server/Configuration.md +++ b/docs/Server/Configuration.md @@ -47,6 +47,7 @@ LDAP Server Configuration * [ServerOptions:setMaxSearchPageSize](#setmaxsearchpagesize) * [ServerOptions:setMaxSearchLookthrough](#setmaxsearchlookthrough) * [ServerOptions:setMaxSearchPagedLookthrough](#setmaxsearchpagedlookthrough) + * [ServerOptions:setMaxPagingSessions](#setmaxpagingsessions) * [ServerOptions:setSearchLimitRules](#setsearchlimitrules) * [Directory Synchronization](#directory-synchronization) * [ServerOptions:setReplicationConfig](#setreplicationconfig) @@ -717,6 +718,15 @@ for ordinary searches. A value of `0` falls back to the regular lookthrough limi **Default**: `0` (use the regular lookthrough limit) +------------------ +#### setMaxPagingSessions + +Cap how many paged searches one connection may leave unfinished, since each holds its result state until the connection +closes. Starting one past the cap discards the least recently started session, logged as `paging.session_evicted`. A +client resuming a discarded session is refused with an invalid cookie. A value of `0` removes the cap. + +**Default**: `25` + ------------------ #### setSearchLimitRules diff --git a/docs/Server/Logging.md b/docs/Server/Logging.md index 3f65bc54..f696c4c4 100644 --- a/docs/Server/Logging.md +++ b/docs/Server/Logging.md @@ -45,6 +45,7 @@ $server = new LdapServer((new ServerOptions())->setLogger($logger)); | `control.critical.rejected` | on | notice | Client sent a critical control the server doesn't support | | `schema.violation` | on | notice | Add/Modify violates the schema (rejected, or allowed under Lenient mode / the Relax control) | | `session.disconnect_notice` | on | notice | Server sends an unsolicited Notice of Disconnect | +| `paging.session_evicted` | on | notice | A connection hit `setMaxPagingSessions`, so its least recently started paged search was discarded | | `entry.added` | off | info | Add succeeds (audit-trail) | | `entry.modified` | off | info | Modify succeeds (audit-trail) | | `entry.deleted` | off | info | Delete succeeds (audit-trail) | diff --git a/src/FreeDSx/Ldap/Container/Provider/ConnectionGraphContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/ConnectionGraphContainerProvider.php index 6fe47dc4..bdd4ea95 100644 --- a/src/FreeDSx/Ldap/Container/Provider/ConnectionGraphContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/ConnectionGraphContainerProvider.php @@ -76,6 +76,7 @@ private function makeBindStrategy(Container $container): PasswordPolicyBindStrat return new ReplicaBindStrategy( $engine, $container->get(ReplicaPasswordStateStoreInterface::class), + $container->get(WritableStorageBackend::class), ); } diff --git a/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php index 354caf11..19e99255 100644 --- a/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php @@ -262,6 +262,7 @@ private function makePagingHandler( requestHistory: $context->requestHistory, schema: $options->getSchema(), limits: $searchLimits ?? $options->makeSearchLimits(), + eventLogger: $context->eventLogger, ); } diff --git a/src/FreeDSx/Ldap/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilder.php b/src/FreeDSx/Ldap/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilder.php index a8e42e5c..6197ecf8 100644 --- a/src/FreeDSx/Ldap/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilder.php +++ b/src/FreeDSx/Ldap/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilder.php @@ -47,7 +47,14 @@ function (?string $authzId, string $authcId, string $password): bool { MechanismName::PLAIN, ); - if ($identity === null || !$this->hashService->verify($password, $identity->password)) { + // Refuses a name that reached no stored value at the cost of one that did. + if ($identity === null) { + $this->hashService->verifyDummy($password); + + return false; + } + + if (!$this->hashService->verify($password, $identity->password)) { return false; } diff --git a/src/FreeDSx/Ldap/Protocol/Bind/Sasl/SaslExchange.php b/src/FreeDSx/Ldap/Protocol/Bind/Sasl/SaslExchange.php index e57d46c1..c82ac78b 100644 --- a/src/FreeDSx/Ldap/Protocol/Bind/Sasl/SaslExchange.php +++ b/src/FreeDSx/Ldap/Protocol/Bind/Sasl/SaslExchange.php @@ -17,6 +17,7 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Exception\InvalidArgumentException; use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Exception\RequestValidationException; use FreeDSx\Ldap\Operation\LdapResult; use FreeDSx\Ldap\Operation\Request\SaslBindRequest; use FreeDSx\Ldap\Operation\Response\BindResponse; @@ -303,9 +304,15 @@ private function sendBindInProgress(LdapMessageRequest $message, ?string $respon * Validates that the message received mid-exchange is a SASL bind continuation. * * @throws OperationException if the client sends a non-SASL request. + * @throws RequestValidationException if the continuation carries an unusable message ID. */ private function requireSaslContinuation(LdapMessageRequest $message): SaslBindRequest { + // Continuations skip the validation middleware, and the challenge echoes back whatever ID arrives. + if ($message->getMessageId() === 0) { + throw new RequestValidationException('The message ID 0 cannot be used in a client request.'); + } + $request = $message->getRequest(); if ($request instanceof SaslBindRequest) { diff --git a/src/FreeDSx/Ldap/Protocol/Queue/ServerQueue.php b/src/FreeDSx/Ldap/Protocol/Queue/ServerQueue.php index b420c044..b21b5552 100644 --- a/src/FreeDSx/Ldap/Protocol/Queue/ServerQueue.php +++ b/src/FreeDSx/Ldap/Protocol/Queue/ServerQueue.php @@ -18,6 +18,7 @@ use FreeDSx\Asn1\Exception\PartialPduException; use FreeDSx\Asn1\Type\AbstractType; use FreeDSx\Ldap\Exception\ProtocolException; +use FreeDSx\Ldap\Exception\RequestValidationException; use FreeDSx\Ldap\Exception\RuntimeException; use FreeDSx\Ldap\Exception\UnsolicitedNotificationException; use FreeDSx\Ldap\Operation\Request\AbandonRequest; @@ -35,6 +36,7 @@ use FreeDSx\Socket\Socket; use Generator; +use function count; use function strlen; /** @@ -44,6 +46,11 @@ */ class ServerQueue extends LdapQueue implements ConnectionControl { + /** + * Well above what a pipelining client sends mid-stream, but low enough to bound what one connection can buffer. + */ + private const MAX_PENDING_MESSAGES = 128; + /** * @var LdapMessageRequest[] */ @@ -83,30 +90,35 @@ public function getMessage(?int $id = null): LdapMessageRequest return array_shift($this->pendingMessages); } - $message = $this->getAndValidateMessage($id); - - if (!$message instanceof LdapMessageRequest) { - throw new ProtocolException(sprintf( - 'Expected an instance of LdapMessageResponse but got: %s', - get_class($message), - )); - } - - return $message; + return $this->readRequest($id); } /** * Checks whether an Abandon or Cancel targeting a message ID has arrived. * * Other messages received while peeking are buffered. + * + * @throws RequestValidationException if a peeked message carries an unusable message ID. */ public function peekForCancelSignal(int $inFlightMessageId): ?LdapMessageRequest { + // Nothing drains the buffer until the stream ends, so reading pauses here, and the socket holds the backlog. + if (count($this->pendingMessages) >= self::MAX_PENDING_MESSAGES) { + return null; + } + if (!$this->hasPendingData()) { return null; } - $message = $this->getMessage(); + // Reads past anything already buffered; draining that here would re-inspect the same message forever. + $message = $this->readRequest(null); + + // Peeking skips the validation middleware, and buffering defers a refusal a persist stream never reaches. + if ($message->getMessageId() === 0) { + throw new RequestValidationException('The message ID 0 cannot be used in a client request.'); + } + $request = $message->getRequest(); if ($this->isAbandonOrCancelRequest($request, $inFlightMessageId)) { @@ -204,6 +216,24 @@ private function applyInterceptors(LdapMessageResponse $response): LdapMessageRe return $response; } + /** + * @throws ProtocolException + * @throws UnsolicitedNotificationException + * @throws ConnectionException + */ + private function readRequest(?int $id): LdapMessageRequest + { + $message = $this->getAndValidateMessage($id); + if (!$message instanceof LdapMessageRequest) { + throw new ProtocolException(sprintf( + 'Expected an instance of LdapMessageResponse but got: %s', + get_class($message), + )); + } + + return $message; + } + /** * @param RequestInterface $request * @param int $inFlightMessageId diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php index a114000a..96f568bc 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php @@ -32,6 +32,9 @@ use FreeDSx\Ldap\Server\RequestHistory; use FreeDSx\Ldap\Server\Backend\Storage\FilterEvaluatorInterface; use FreeDSx\Ldap\Server\Operation\SearchOperationResult; +use FreeDSx\Ldap\Server\Logging\EventContext; +use FreeDSx\Ldap\Server\Logging\EventLogger; +use FreeDSx\Ldap\Server\Logging\ServerEvent; use FreeDSx\Ldap\Server\SearchLimits; use FreeDSx\Ldap\Server\Token\TokenInterface; use Generator; @@ -55,6 +58,7 @@ public function __construct( private readonly Schema $schema, private readonly PagingRequestComparator $requestComparator = new PagingRequestComparator(), private readonly SearchLimits $limits = new SearchLimits(), + private readonly ?EventLogger $eventLogger = null, ) {} /** @@ -392,12 +396,39 @@ private function findOrMakePagingRequest(LdapMessageRequest $message): PagingReq return $this->findPagingRequestOrThrow($pagingControl->getCookie()); } + $this->evictOldestSessionAtLimit(); + $pagingRequest = $this->makePagingRequest($message); $this->requestHistory->pagingRequest()->add($pagingRequest); return $pagingRequest; } + /** + * Unfinished sessions would otherwise retain a generator apiece for the life of the connection. + */ + private function evictOldestSessionAtLimit(): void + { + $limit = $this->limits->maxPagingSessions ?? 0; + $requests = $this->requestHistory->pagingRequest(); + + if ($limit <= 0 || $requests->count() < $limit) { + return; + } + + $oldest = $requests->oldest(); + if ($oldest === null) { + return; + } + + $requests->remove($oldest); + $this->requestHistory->removePagingGenerator($oldest->getNextCookie()); + $this->eventLogger?->record( + ServerEvent::PagingSessionEvicted, + [EventContext::LIMIT => $limit], + ); + } + /** * @throws OperationException */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Auth/PasswordHashService.php b/src/FreeDSx/Ldap/Server/Backend/Auth/PasswordHashService.php index 5834048c..0f1bc671 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Auth/PasswordHashService.php +++ b/src/FreeDSx/Ldap/Server/Backend/Auth/PasswordHashService.php @@ -21,7 +21,7 @@ * * @author Chad Sikorra */ -final readonly class PasswordHashService +readonly class PasswordHashService { /** * Read-only legacy prefixes recognized in addition to the writable {@see PasswordHashScheme} set. @@ -35,7 +35,7 @@ ]; /** - * @param int|null $hashCost bcrypt cost forwarded to password_hash(); null uses the PHP default (10). Does not apply to argon2. + * @param int|null $hashCost bcrypt cost forwarded to password_hash(); null uses the PHP default. Does not apply to argon2. */ public function __construct( private PasswordHashScheme $scheme = PasswordHashScheme::Bcrypt, diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/SqlFilter/SqlFilterTranslatorTrait.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/SqlFilter/SqlFilterTranslatorTrait.php index da756bad..d4b1f475 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/SqlFilter/SqlFilterTranslatorTrait.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/SqlFilter/SqlFilterTranslatorTrait.php @@ -617,7 +617,8 @@ private function validateAttribute(string $attribute): string { $lower = strtolower($attribute); - if (preg_match('/^([a-z][a-z0-9-]*|\d+(\.\d+)+)(;[a-z0-9-]+)*$/', $lower) !== 1) { + // The D modifier matters here: without it a trailing newline slips past the whitelist and into an identifier. + if (preg_match('/^([a-z][a-z0-9-]*|\d+(\.\d+)+)(;[a-z0-9-]+)*$/D', $lower) !== 1) { throw new InvalidAttributeException(sprintf( 'Attribute description "%s" is not a valid RFC 4512 attribute description.', $attribute, diff --git a/src/FreeDSx/Ldap/Server/Logging/EventContext.php b/src/FreeDSx/Ldap/Server/Logging/EventContext.php index c0466f6e..ec78607e 100644 --- a/src/FreeDSx/Ldap/Server/Logging/EventContext.php +++ b/src/FreeDSx/Ldap/Server/Logging/EventContext.php @@ -76,6 +76,8 @@ final class EventContext public const REMOVED = 'removed'; + public const LIMIT = 'limit'; + public const DURATION_SECONDS = 'duration_seconds'; public const NEW_RDN = 'new_rdn'; diff --git a/src/FreeDSx/Ldap/Server/Logging/EventLogPolicy.php b/src/FreeDSx/Ldap/Server/Logging/EventLogPolicy.php index 226a87d6..f0a61d8f 100644 --- a/src/FreeDSx/Ldap/Server/Logging/EventLogPolicy.php +++ b/src/FreeDSx/Ldap/Server/Logging/EventLogPolicy.php @@ -60,6 +60,7 @@ public static function default(): self ServerEvent::CriticalControlRejected, ServerEvent::SchemaViolation, ServerEvent::SyncEntrySkipped, + ServerEvent::PagingSessionEvicted, ServerEvent::JournalPruned, ServerEvent::JournalPruneFailed, ServerEvent::NoticeOfDisconnectSent, diff --git a/src/FreeDSx/Ldap/Server/Logging/ServerEvent.php b/src/FreeDSx/Ldap/Server/Logging/ServerEvent.php index 62823419..a1226654 100644 --- a/src/FreeDSx/Ldap/Server/Logging/ServerEvent.php +++ b/src/FreeDSx/Ldap/Server/Logging/ServerEvent.php @@ -45,6 +45,7 @@ enum ServerEvent: string case OperationRefused = 'operation.refused'; case SchemaViolation = 'schema.violation'; case SyncEntrySkipped = 'sync.entry_skipped'; + case PagingSessionEvicted = 'paging.session_evicted'; case JournalPruned = 'journal.pruned'; case JournalPruneFailed = 'journal.prune_failed'; case NoticeOfDisconnectSent = 'session.disconnect_notice'; @@ -75,6 +76,7 @@ public function level(): string self::NoticeOfDisconnectSent, self::WriteTimeout, self::IdleTimeout, + self::PagingSessionEvicted, self::PasswordPolicyExpired, self::PasswordPolicyChangeRejected => LogLevel::NOTICE, default => LogLevel::INFO, diff --git a/src/FreeDSx/Ldap/Server/Middleware/RequestValidationMiddleware.php b/src/FreeDSx/Ldap/Server/Middleware/RequestValidationMiddleware.php index 7abe6dcb..28ee6bfa 100644 --- a/src/FreeDSx/Ldap/Server/Middleware/RequestValidationMiddleware.php +++ b/src/FreeDSx/Ldap/Server/Middleware/RequestValidationMiddleware.php @@ -19,11 +19,10 @@ use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareInterface; use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext; -use function in_array; use function sprintf; /** - * Rejects a message whose ID is zero or already used on this connection (RFC 4511 §4.1.1.1). + * Rejects a message whose ID is zero or in use by an operation still being served (RFC 4511 §4.1.1.1). * * @internal * @author Chad Sikorra @@ -31,9 +30,11 @@ final class RequestValidationMiddleware implements MiddlewareInterface { /** - * @var int[] + * Operations still being served, keyed by ID so a lookup does not scan. + * + * @var array */ - private array $messageIds = []; + private array $outstanding = []; /** * @throws RequestValidationException @@ -48,14 +49,18 @@ public function process( throw new RequestValidationException('The message ID 0 cannot be used in a client request.'); } - // Stricter than RFC 4511 §4.1.1.1, which only forbids reusing an ID that is still outstanding. Since the - // server processes a connection's messages serially, rejecting any reused ID is safe and simpler. - if (in_array($messageId, $this->messageIds, true)) { + // RFC 4511 §4.1.1.1 forbids only an ID in use by an operation still being served. + if (isset($this->outstanding[$messageId])) { throw new RequestValidationException(sprintf('The message ID %s is not valid.', $messageId)); } - $this->messageIds[] = $messageId; + $this->outstanding[$messageId] = true; - return $next->handle($context); + try { + // The response writer sits below this, so the operation is answered by the time the call returns. + return $next->handle($context); + } finally { + unset($this->outstanding[$messageId]); + } } } diff --git a/src/FreeDSx/Ldap/Server/Paging/PagingRequests.php b/src/FreeDSx/Ldap/Server/Paging/PagingRequests.php index 0f16eb3e..e7d1975d 100644 --- a/src/FreeDSx/Ldap/Server/Paging/PagingRequests.php +++ b/src/FreeDSx/Ldap/Server/Paging/PagingRequests.php @@ -36,6 +36,21 @@ public function add(PagingRequest $request): void $this->requests[] = $request; } + public function count(): int + { + return count($this->requests); + } + + /** + * The least recently started request, since they are held in the order they were added. + */ + public function oldest(): ?PagingRequest + { + $requests = array_values($this->requests); + + return $requests[0] ?? null; + } + public function remove(PagingRequest $toRemove): void { foreach ($this->requests as $i => $pagingRequest) { diff --git a/src/FreeDSx/Ldap/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategy.php b/src/FreeDSx/Ldap/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategy.php index 29f1aed1..13aeda4f 100644 --- a/src/FreeDSx/Ldap/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategy.php +++ b/src/FreeDSx/Ldap/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategy.php @@ -14,6 +14,7 @@ namespace FreeDSx\Ldap\Server\PasswordPolicy\Guard\BindStrategy; use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Server\Backend\LdapBackendInterface; use FreeDSx\Ldap\Server\PasswordPolicy\Attempt\PasswordBindAttempt; use FreeDSx\Ldap\Server\PasswordPolicy\Decision\OperationalChanges; use FreeDSx\Ldap\Server\PasswordPolicy\Decision\PasswordPolicyOutcome; @@ -34,6 +35,7 @@ public function __construct( private PasswordPolicyEngine $engine, private ReplicaPasswordStateStoreInterface $store, + private LdapBackendInterface $backend, ) {} /** @@ -50,10 +52,16 @@ public function preBindOutcome(PasswordBindAttempt $attempt): PasswordPolicyOutc return $entryOutcome; } - return $this->engine->evaluateLocalLockout( + $localOutcome = $this->engine->evaluateLocalLockout( $this->store->load($attempt->dn)->toUserPasswordState($attempt->dn), $attempt->policy, ); + + if ($localOutcome->denied) { + return $localOutcome; + } + + return $this->reReadEntryOutcome($attempt); } public function record( @@ -81,6 +89,24 @@ function (ReplicaPasswordState $local) use ($attempt, $decide, &$recorded): Oper ); } + /** + * The caller's entry state was read before the local one, and applying a replicated entry drops the local state it + * supersedes. Both reading clear therefore has to be confirmed against the entry as it stands now. + */ + private function reReadEntryOutcome(PasswordBindAttempt $attempt): PasswordPolicyOutcome + { + $entry = $this->backend->get($attempt->dn); + + if ($entry === null) { + return PasswordPolicyOutcome::allow(); + } + + return $this->engine->evaluatePreBind( + UserPasswordState::fromEntry($entry), + $attempt->policy, + ); + } + /** * Combine primary-authored entry fields (expiry / validity / must-change) with the replica-local volatile state so * the engine decides against the worst of both. diff --git a/src/FreeDSx/Ldap/Server/RequestHistory.php b/src/FreeDSx/Ldap/Server/RequestHistory.php index e98d9a5c..905039fc 100644 --- a/src/FreeDSx/Ldap/Server/RequestHistory.php +++ b/src/FreeDSx/Ldap/Server/RequestHistory.php @@ -13,7 +13,6 @@ namespace FreeDSx\Ldap\Server; -use FreeDSx\Ldap\Exception\ProtocolException; use FreeDSx\Ldap\Server\Paging\PagingRequests; use Generator; @@ -24,11 +23,6 @@ */ final class RequestHistory { - /** - * @var int[] - */ - private array $ids = []; - private PagingRequests $pagingRequests; /** @@ -44,23 +38,6 @@ public function __construct(?PagingRequests $pagingRequests = null) $this->pagingRequests = $pagingRequests ?? new PagingRequests(); } - /** - * Add a specific message ID that the client has used. - * - * @throws ProtocolException - */ - public function addId(int $id): void - { - if ($id === 0 || in_array($id, $this->ids, true)) { - throw new ProtocolException(sprintf( - 'The message ID %s is not valid.', - $id, - )); - } - - $this->ids[] = $id; - } - /** * The currently active paging requests from the client. */ @@ -69,14 +46,6 @@ public function pagingRequest(): PagingRequests return $this->pagingRequests; } - /** - * @return int[] - */ - public function getIds(): array - { - return $this->ids; - } - /** * Store a generator for the given paging cookie (the cookie that will be * sent to the client and returned on the next page request). diff --git a/src/FreeDSx/Ldap/Server/SearchLimit/SearchLimitResolver.php b/src/FreeDSx/Ldap/Server/SearchLimit/SearchLimitResolver.php index 7ae6c8f0..cf72e8c3 100644 --- a/src/FreeDSx/Ldap/Server/SearchLimit/SearchLimitResolver.php +++ b/src/FreeDSx/Ldap/Server/SearchLimit/SearchLimitResolver.php @@ -41,11 +41,25 @@ public function resolve(TokenInterface $token): SearchLimits { foreach ($this->rules->rules as $rule) { // Limit rules carry no target entry, so a subject needing one cannot match. - if ($rule->subject->matches($token, null)) { - return $rule->limits; + if (!$rule->subject->matches($token, null)) { + continue; } + + return $this->withServerWidePagingCap($rule->limits); } return $this->default; } + + /** + * Paging sessions cap per-connection memory, so a rule saying nothing about them inherits rather than lifts. + */ + private function withServerWidePagingCap(SearchLimits $limits): SearchLimits + { + if ($limits->maxPagingSessions !== null || $this->default->maxPagingSessions === null) { + return $limits; + } + + return $limits->withMaxPagingSessions($this->default->maxPagingSessions); + } } diff --git a/src/FreeDSx/Ldap/Server/SearchLimits.php b/src/FreeDSx/Ldap/Server/SearchLimits.php index 368257c0..6fe44f6f 100644 --- a/src/FreeDSx/Ldap/Server/SearchLimits.php +++ b/src/FreeDSx/Ldap/Server/SearchLimits.php @@ -24,8 +24,24 @@ public function __construct( public int $maxSearchPageSize = 0, public int $maxSearchLookthrough = 0, public int $maxSearchPagedLookthrough = 0, + public ?int $maxPagingSessions = null, ) {} + /** + * A per-identity rule that says nothing about paging sessions keeps the server-wide cap rather than lifting it. + */ + public function withMaxPagingSessions(?int $maxPagingSessions): self + { + return new self( + maxSearchSize: $this->maxSearchSize, + maxSearchTimeLimit: $this->maxSearchTimeLimit, + maxSearchPageSize: $this->maxSearchPageSize, + maxSearchLookthrough: $this->maxSearchLookthrough, + maxSearchPagedLookthrough: $this->maxSearchPagedLookthrough, + maxPagingSessions: $maxPagingSessions, + ); + } + /** * Effective lookthrough for paged searches: the paged limit when set, otherwise the regular lookthrough. */ diff --git a/src/FreeDSx/Ldap/ServerOptions.php b/src/FreeDSx/Ldap/ServerOptions.php index a089951f..b0f069ac 100644 --- a/src/FreeDSx/Ldap/ServerOptions.php +++ b/src/FreeDSx/Ldap/ServerOptions.php @@ -154,6 +154,8 @@ final class ServerOptions implements ServerListenerOptionsInterface private int $maxSearchPagedLookthrough = 0; + private int $maxPagingSessions = 25; + private ?SearchLimitRules $searchLimitRules = null; private ?ConfigReloaderInterface $configReloader = null; @@ -593,6 +595,21 @@ public function setMaxSearchPagedLookthrough(int $maxSearchPagedLookthrough): se return $this; } + /** + * Paged searches a connection may leave unfinished before the least recent is discarded. Zero means no cap. + */ + public function getMaxPagingSessions(): int + { + return $this->maxPagingSessions; + } + + public function setMaxPagingSessions(int $maxPagingSessions): self + { + $this->maxPagingSessions = $maxPagingSessions; + + return $this; + } + public function setSearchLimitRules(SearchLimitRules $searchLimitRules): self { $this->searchLimitRules = $searchLimitRules; @@ -613,6 +630,7 @@ public function makeSearchLimits(): SearchLimits maxSearchPageSize: $this->maxSearchPageSize, maxSearchLookthrough: $this->maxSearchLookthrough, maxSearchPagedLookthrough: $this->maxSearchPagedLookthrough, + maxPagingSessions: $this->maxPagingSessions, ); } diff --git a/src/FreeDSx/Ldap/Sync/Consumer/ReconcilingChangeApplier.php b/src/FreeDSx/Ldap/Sync/Consumer/ReconcilingChangeApplier.php index 96a43551..2c448b33 100644 --- a/src/FreeDSx/Ldap/Sync/Consumer/ReconcilingChangeApplier.php +++ b/src/FreeDSx/Ldap/Sync/Consumer/ReconcilingChangeApplier.php @@ -37,6 +37,10 @@ public function beginRefresh(): void $this->baseApplier->beginRefresh(); } + /** + * The entry is written before any local state is dropped, and that order must hold: a bind reading no local state + * concludes the entry it superseded is already durable, and re-reads the entry rather than allowing. + */ public function apply( SyncEntryResult $result, Session $session, diff --git a/tests/integration/LdapServerTest.php b/tests/integration/LdapServerTest.php index a30475bd..8d5891c0 100644 --- a/tests/integration/LdapServerTest.php +++ b/tests/integration/LdapServerTest.php @@ -719,6 +719,27 @@ public function testItCanEndPagingEarly(): void $this->assertFalse($paging->hasEntries()); } + public function testAnUnfinishedPagingSessionIsDiscardedOnceTheCapIsReached(): void + { + $this->createServerProcess( + 'tcp', + ['--max-paging-sessions=2'], + ); + $this->authenticateUser(); + + $search = Operations::search(Filters::present('objectClass'))->base('dc=foo,dc=bar'); + + // Each takes a page and stops, so none of them releases its slot. + $first = $this->ldapClient()->paging($search, 1); + $first->getEntries(); + $this->ldapClient()->paging($search, 1)->getEntries(); + $this->ldapClient()->paging($search, 1)->getEntries(); + + $this->expectException(OperationException::class); + + $first->getEntries(); + } + public function testSighupDoesNotShutdownTheServer(): void { if (!extension_loaded('posix')) { diff --git a/tests/integration/Security/SaslIntegrationTest.php b/tests/integration/Security/SaslIntegrationTest.php index b01ff064..8fc7ae73 100644 --- a/tests/integration/Security/SaslIntegrationTest.php +++ b/tests/integration/Security/SaslIntegrationTest.php @@ -14,6 +14,7 @@ namespace Tests\Integration\FreeDSx\Ldap\Security; use FreeDSx\Ldap\Exception\BindException; +use FreeDSx\Ldap\Exception\UnsolicitedNotificationException; use FreeDSx\Ldap\Operation\Request\SaslBindRequest; use FreeDSx\Ldap\Operation\Response\BindResponse; use FreeDSx\Ldap\Operation\Response\ResponseInterface; @@ -155,18 +156,46 @@ public function testSaslScramRefusesAnEmptyStoredPassword(): void ); } + /** + * Credentials that would otherwise authenticate, so only the message ID can account for the refusal. + */ + public function testSaslContinuationCarryingAZeroMessageIdIsRefused(): void + { + $queue = $this->rawQueue(); + $queue->sendMessage(new LdapMessageRequest( + 1, + new SaslBindRequest('CRAM-MD5'), + )); + $challenge = $queue->getMessage(1)->getResponse(); + self::assertInstanceOf( + BindResponse::class, + $challenge, + ); + + $queue->sendMessage(new LdapMessageRequest( + 0, + new SaslBindRequest( + 'CRAM-MD5', + 'user ' . hash_hmac('md5', (string) $challenge->getSaslCredentials(), '12345'), + ), + )); + + try { + $queue->getMessage(); + self::fail('The continuation was accepted.'); + } catch (UnsolicitedNotificationException $e) { + self::assertTrue($e->isNoticeOfDisconnection()); + } finally { + $queue->close(); + } + } + /** * The stock client never puts these credentials in an initial bind, so the request is sent raw. */ private function sendRawBind(SaslBindRequest $request): ResponseInterface { - $queue = new ClientQueue(new SocketPool( - (new SocketPoolOptions( - (new SocketOptions()) - ->setPort(TestWorker::port()) - ->setTimeoutConnect(1), - ))->setServers(['127.0.0.1']), - )); + $queue = $this->rawQueue(); $queue->sendMessage(new LdapMessageRequest( 1, @@ -177,4 +206,15 @@ private function sendRawBind(SaslBindRequest $request): ResponseInterface return $response; } + + private function rawQueue(): ClientQueue + { + return new ClientQueue(new SocketPool( + (new SocketPoolOptions( + (new SocketOptions()) + ->setPort(TestWorker::port()) + ->setTimeoutConnect(1), + ))->setServers(['127.0.0.1']), + )); + } } diff --git a/tests/integration/ServerTestCase.php b/tests/integration/ServerTestCase.php index b9c5455a..944a6082 100644 --- a/tests/integration/ServerTestCase.php +++ b/tests/integration/ServerTestCase.php @@ -136,12 +136,16 @@ protected static function tearDownSharedServer(): void /** * Per-test server override. For tests that require a different config. * + * Anything already listening is stopped first, since only one server can hold the port. + * * @param list $extraArgs */ protected function createServerProcess( string $transport, array $extraArgs = [], ): void { + $this->stopServer(); + $processArgs = [ 'php', '-dpcov.enabled=0', diff --git a/tests/integration/Sync/SyncReplForwardTestCase.php b/tests/integration/Sync/SyncReplForwardTestCase.php index d1436b25..94124b7f 100644 --- a/tests/integration/Sync/SyncReplForwardTestCase.php +++ b/tests/integration/Sync/SyncReplForwardTestCase.php @@ -62,9 +62,6 @@ public function test_a_password_reset_on_the_provider_retires_the_replica_local_ $this->tryReplicaBind($dn, 'wrong'); $this->tryReplicaBind($dn, 'wrong'); - // The replica enforces its own lock across connections before anything replicates back. - self::assertFalse($this->replicaBindSucceeds($dn, '12345')); - // Let the forward fully apply so the reset below does not race an in-flight forward. self::assertTrue( $this->pollUntil(fn(): bool => $this->providerHasLock($dn)), diff --git a/tests/support/LdapServerCommand.php b/tests/support/LdapServerCommand.php index d2216d02..72772de1 100644 --- a/tests/support/LdapServerCommand.php +++ b/tests/support/LdapServerCommand.php @@ -131,6 +131,13 @@ protected function configure(): void 'Lookthrough cap for paged searches (0 = fall back to the regular lookthrough)', '0', ) + ->addOption( + 'max-paging-sessions', + null, + InputOption::VALUE_REQUIRED, + 'Unfinished paged searches a connection may hold before the least recent is discarded (0 = no cap)', + '25', + ) ->addOption( 'authenticated-lookthrough', null, @@ -302,6 +309,7 @@ protected function execute( ->setAdministrators(Subject::dn(self::ADMIN_DN)) ->setMaxSearchLookthrough((int) $this->getStringOption($input, 'max-search-lookthrough')) ->setMaxSearchPagedLookthrough((int) $this->getStringOption($input, 'max-search-paged-lookthrough')) + ->setMaxPagingSessions((int) $this->getStringOption($input, 'max-paging-sessions')) ->setReplicationConfig(ReplicationConfig::forProvider( (new ProviderConfig())->setPollInterval(self::SYNC_POLL_INTERVAL), )) diff --git a/tests/unit/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilderTest.php b/tests/unit/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilderTest.php index 3f81b637..d7e7e0ac 100644 --- a/tests/unit/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilderTest.php +++ b/tests/unit/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilderTest.php @@ -16,6 +16,7 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Protocol\Bind\Sasl\OptionsBuilder\PlainMechanismOptionsBuilder; use FreeDSx\Ldap\Server\Backend\Auth\PasswordAuthenticatableInterface; +use FreeDSx\Ldap\Server\Backend\Auth\PasswordHashService; use FreeDSx\Ldap\Server\Backend\Auth\SaslIdentity; use FreeDSx\Sasl\Mechanism\MechanismName; use FreeDSx\Sasl\Options\PlainOptions; @@ -121,4 +122,62 @@ public function test_get_resolved_dn_is_populated_after_successful_validation(): $this->subject->getResolvedDn()?->toString(), ); } + + public function test_a_name_resolving_to_nothing_is_refused_at_the_cost_of_a_comparison(): void + { + $hashService = $this->createMock(PasswordHashService::class); + $hashService->expects(self::once()) + ->method('verifyDummy') + ->with('wrong'); + + $this->mockAuthenticator + ->method('getSaslIdentity') + ->willReturn(null); + + $subject = new PlainMechanismOptionsBuilder( + $this->mockAuthenticator, + $hashService, + ); + $options = $subject->buildOptions(null, MechanismName::PLAIN); + assert($options instanceof PlainOptions); + $validate = $options->getValidate(); + assert(is_callable($validate)); + + self::assertFalse($validate( + null, + 'unknown', + 'wrong', + )); + } + + public function test_a_resolved_name_is_not_charged_a_dummy_comparison(): void + { + $hashService = $this->createMock(PasswordHashService::class); + $hashService->expects(self::never()) + ->method('verifyDummy'); + $hashService->method('verify') + ->willReturn(false); + + $this->mockAuthenticator + ->method('getSaslIdentity') + ->willReturn(new SaslIdentity( + 'correct', + new Dn('cn=user,dc=foo,dc=bar'), + )); + + $subject = new PlainMechanismOptionsBuilder( + $this->mockAuthenticator, + $hashService, + ); + $options = $subject->buildOptions(null, MechanismName::PLAIN); + assert($options instanceof PlainOptions); + $validate = $options->getValidate(); + assert(is_callable($validate)); + + self::assertFalse($validate( + null, + 'cn=user,dc=foo,dc=bar', + 'wrong', + )); + } } diff --git a/tests/unit/Protocol/Bind/Sasl/SaslExchangeTest.php b/tests/unit/Protocol/Bind/Sasl/SaslExchangeTest.php index 7bde4b98..860520a1 100644 --- a/tests/unit/Protocol/Bind/Sasl/SaslExchangeTest.php +++ b/tests/unit/Protocol/Bind/Sasl/SaslExchangeTest.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Exception\RequestValidationException; use FreeDSx\Ldap\Operation\Request\SaslBindRequest; use FreeDSx\Ldap\Operation\Request\SimpleBindRequest; use FreeDSx\Ldap\Operation\ResultCode; @@ -227,6 +228,54 @@ public function test_it_throws_protocol_error_when_non_sasl_request_received_mid $this->subject->run($this->makeInput()); } + public function test_it_rejects_a_continuation_carrying_a_zero_message_id(): void + { + $this->mockChallenge + ->method('challenge') + ->willReturn($this->makeContext(isComplete: false, response: 'server-challenge')); + + $this->mockQueue + ->expects(self::once()) + ->method('sendMessage'); // SASL_BIND_IN_PROGRESS only; a zero ID cannot frame a response + + $this->mockQueue + ->expects(self::once()) + ->method('getMessage') + ->willReturn(new LdapMessageRequest( + 0, + new SaslBindRequest('PLAIN'), + )); + + self::expectException(RequestValidationException::class); + + $this->subject->run($this->makeInput()); + } + + public function test_it_accepts_a_continuation_reusing_the_initial_message_id(): void + { + $this->mockChallenge + ->method('challenge') + ->willReturnOnConsecutiveCalls( + $this->makeContext(isComplete: false, response: 'server-challenge'), + $this->makeContext(isComplete: true, response: 'server-final'), + ); + + $this->mockQueue->method('sendMessage'); + $this->mockQueue + ->method('getMessage') + ->willReturn(new LdapMessageRequest( + 1, + new SaslBindRequest('PLAIN', 'creds'), + )); + + $result = $this->subject->run($this->makeInput()); + + self::assertSame( + 1, + $result->getLastMessage()->getMessageId(), + ); + } + public function test_cram_md5_credentials_in_the_initial_bind_do_not_authenticate(): void { $password = '12345'; diff --git a/tests/unit/Protocol/Queue/ServerQueueTest.php b/tests/unit/Protocol/Queue/ServerQueueTest.php index 5cedc280..4035c7ed 100644 --- a/tests/unit/Protocol/Queue/ServerQueueTest.php +++ b/tests/unit/Protocol/Queue/ServerQueueTest.php @@ -29,6 +29,7 @@ use FreeDSx\Ldap\Protocol\LdapMessageRequest; use FreeDSx\Ldap\Protocol\LdapMessageResponse; use FreeDSx\Ldap\Exception\RequestSizeExceededException; +use FreeDSx\Ldap\Exception\RequestValidationException; use FreeDSx\Ldap\Protocol\Queue\MessageWrapperInterface; use FreeDSx\Ldap\Protocol\Queue\ServerQueue; use FreeDSx\Ldap\Server\Metrics\Recorder\InMemoryMetricsRecorder; @@ -41,6 +42,11 @@ final class ServerQueueTest extends TestCase { + /** + * Comfortably past the buffer cap, so an uncapped peek would read on every attempt. + */ + private const FLOOD_ATTEMPTS = 500; + private ServerQueue $subject; private Socket&MockObject $mockSocket; @@ -276,6 +282,79 @@ public function test_peek_buffers_non_cancel_message_and_returns_null(): void ); } + public function test_peek_refuses_a_cancel_carrying_a_zero_message_id(): void + { + $queue = $this->makeQueueWithEncodedRequest( + new LdapMessageRequest(0, new CancelRequest(2)), + ); + + self::expectException(RequestValidationException::class); + self::expectExceptionMessage('The message ID 0 cannot be used in a client request.'); + + $queue->peekForCancelSignal(2); + } + + public function test_peek_refuses_an_unrelated_message_carrying_a_zero_message_id(): void + { + $queue = $this->makeQueueWithEncodedRequest( + new LdapMessageRequest(0, new DeleteRequest('dc=foo,dc=bar')), + ); + + self::expectException(RequestValidationException::class); + + $queue->peekForCancelSignal(2); + } + + public function test_peek_finds_a_cancel_queued_behind_an_unrelated_message(): void + { + $encoder = new LdapEncoder(); + $socket = $this->createMock(Socket::class); + $socket->method('read')->willReturn( + $encoder->encode((new LdapMessageRequest(5, new DeleteRequest('dc=foo,dc=bar')))->toAsn1()) + . $encoder->encode((new LdapMessageRequest(6, new CancelRequest(2)))->toAsn1()), + false, + ); + + $queue = new ServerQueue($socket, $encoder); + + $buffered = $queue->peekForCancelSignal(2); + $signal = $queue->peekForCancelSignal(2); + + self::assertNull($buffered); + self::assertInstanceOf( + CancelRequest::class, + $signal?->getRequest(), + ); + } + + public function test_peek_stops_reading_once_the_pending_buffer_is_full(): void + { + $encoder = new LdapEncoder(); + $bytes = $encoder->encode( + (new LdapMessageRequest(5, new DeleteRequest('dc=foo,dc=bar')))->toAsn1(), + ); + $reads = 0; + + $socket = $this->createMock(Socket::class); + $socket->method('read') + ->willReturnCallback(function () use ($bytes, &$reads): string { + $reads++; + + return $bytes; + }); + + $queue = new ServerQueue($socket, $encoder); + + for ($attempt = 0; $attempt < self::FLOOD_ATTEMPTS; $attempt++) { + self::assertNull($queue->peekForCancelSignal(2)); + } + + self::assertLessThan( + self::FLOOD_ATTEMPTS, + $reads, + ); + } + public function test_peek_buffers_abandon_targeting_different_message_and_returns_null(): void { $queue = $this->makeQueueWithEncodedRequest( diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php index 370c0986..8e7f105a 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php @@ -423,6 +423,132 @@ public function test_server_max_search_size_applies_to_paged_search_when_client_ self::assertSame(ResultCode::SIZE_LIMIT_EXCEEDED, $done->getResultCode()); } + public function test_starting_a_session_at_the_limit_evicts_the_least_recent(): void + { + $this->mockBackend + ->method('search') + ->willReturnCallback(fn(): EntryStream => new EntryStream($this->makeGenerator( + Entry::create('cn=1,dc=foo,dc=bar', ['cn' => '1']), + Entry::create('cn=2,dc=foo,dc=bar', ['cn' => '2']), + ))); + + $subject = new ServerPagingHandler( + backend: $this->mockBackend, + filterEvaluator: $this->mockFilterEvaluator, + accessControl: $this->mockAccessControl, + requestHistory: $this->requestHistory, + schema: $this->schema, + limits: new SearchLimits(maxPagingSessions: 2), + ); + + // Each leaves a page outstanding, so none of them completes and releases its slot. + $this->drive($subject, $this->makeSearchMessage(size: 1)); + $firstCookie = $this->donePagingControl()->getCookie(); + $this->drive($subject, $this->makeSearchMessage(size: 1)); + $this->drive($subject, $this->makeSearchMessage(size: 1)); + + self::assertSame( + 2, + $this->requestHistory->pagingRequest()->count(), + ); + self::assertNull( + $this->requestHistory->getPagingGenerator($firstCookie), + 'The evicted session must not keep its generator alive.', + ); + self::assertFalse( + $this->requestHistory->pagingRequest()->has($firstCookie), + 'Resuming an evicted session must be refused.', + ); + } + + public function test_a_session_that_runs_to_completion_releases_its_slot(): void + { + $this->mockBackend + ->method('search') + ->willReturnCallback(fn(): EntryStream => new EntryStream($this->makeGenerator( + Entry::create('cn=1,dc=foo,dc=bar', ['cn' => '1']), + ))); + + $subject = new ServerPagingHandler( + backend: $this->mockBackend, + filterEvaluator: $this->mockFilterEvaluator, + accessControl: $this->mockAccessControl, + requestHistory: $this->requestHistory, + schema: $this->schema, + limits: new SearchLimits(maxPagingSessions: 2), + ); + + // A page larger than the result set finishes the search outright. + $this->drive($subject, $this->makeSearchMessage(size: 10)); + $this->drive($subject, $this->makeSearchMessage(size: 10)); + $this->drive($subject, $this->makeSearchMessage(size: 10)); + + self::assertSame( + 0, + $this->requestHistory->pagingRequest()->count(), + 'Completed sessions must be reclaimed without waiting for eviction.', + ); + } + + public function test_an_abandoned_session_releases_its_slot(): void + { + $this->mockBackend + ->method('search') + ->willReturnCallback(fn(): EntryStream => new EntryStream($this->makeGenerator( + Entry::create('cn=1,dc=foo,dc=bar', ['cn' => '1']), + Entry::create('cn=2,dc=foo,dc=bar', ['cn' => '2']), + ))); + + $subject = new ServerPagingHandler( + backend: $this->mockBackend, + filterEvaluator: $this->mockFilterEvaluator, + accessControl: $this->mockAccessControl, + requestHistory: $this->requestHistory, + schema: $this->schema, + limits: new SearchLimits(maxPagingSessions: 2), + ); + + $this->drive($subject, $this->makeSearchMessage(size: 1)); + $cookie = $this->donePagingControl()->getCookie(); + + // RFC 2696 section 3: a zero page size with the cookie abandons the search. + $this->drive($subject, $this->makeSearchMessage(size: 0, cookie: $cookie)); + + self::assertSame( + 0, + $this->requestHistory->pagingRequest()->count(), + ); + self::assertNull($this->requestHistory->getPagingGenerator($cookie)); + } + + public function test_sessions_are_not_evicted_when_no_limit_is_set(): void + { + $this->mockBackend + ->method('search') + ->willReturnCallback(fn(): EntryStream => new EntryStream($this->makeGenerator( + Entry::create('cn=1,dc=foo,dc=bar', ['cn' => '1']), + Entry::create('cn=2,dc=foo,dc=bar', ['cn' => '2']), + ))); + + $subject = new ServerPagingHandler( + backend: $this->mockBackend, + filterEvaluator: $this->mockFilterEvaluator, + accessControl: $this->mockAccessControl, + requestHistory: $this->requestHistory, + schema: $this->schema, + limits: new SearchLimits(maxPagingSessions: 0), + ); + + $this->drive($subject, $this->makeSearchMessage(size: 1)); + $this->drive($subject, $this->makeSearchMessage(size: 1)); + $this->drive($subject, $this->makeSearchMessage(size: 1)); + + self::assertSame( + 3, + $this->requestHistory->pagingRequest()->count(), + ); + } + public function test_server_max_page_size_caps_client_page_size(): void { $message = $this->makeSearchMessage(size: 10); diff --git a/tests/unit/Server/Backend/Storage/Adapter/MysqlFilterTranslatorTest.php b/tests/unit/Server/Backend/Storage/Adapter/MysqlFilterTranslatorTest.php index bde23414..e79f4811 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/MysqlFilterTranslatorTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/MysqlFilterTranslatorTest.php @@ -392,6 +392,9 @@ public static function provideInvalidAttributeDescriptions(): array 'contains single quote' => ["cn'"], 'sql injection' => ["cn'; DROP TABLE entries--"], 'null byte' => ["cn\0bad"], + 'trailing newline' => ["cn\n"], + 'newline after an option' => ["cn;binary\n"], + 'newline after a numericoid' => ["2.5.4.3\n"], 'non-ascii unicode' => ['ñame'], 'trailing semicolon' => ['cn;'], 'double semicolon' => ['cn;;lang'], diff --git a/tests/unit/Server/Backend/Storage/Adapter/SqliteFilterTranslatorTest.php b/tests/unit/Server/Backend/Storage/Adapter/SqliteFilterTranslatorTest.php index bb216a17..393123d9 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/SqliteFilterTranslatorTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/SqliteFilterTranslatorTest.php @@ -453,6 +453,9 @@ public static function provideInvalidAttributeDescriptions(): array 'contains single quote' => ["cn'"], 'sql injection' => ["cn'; DROP TABLE entries--"], 'null byte' => ["cn\0bad"], + 'trailing newline' => ["cn\n"], + 'newline after an option' => ["cn;binary\n"], + 'newline after a numericoid' => ["2.5.4.3\n"], 'non-ascii unicode' => ['ñame'], 'trailing semicolon' => ['cn;'], 'double semicolon' => ['cn;;lang'], diff --git a/tests/unit/Server/Middleware/RequestValidationMiddlewareTest.php b/tests/unit/Server/Middleware/RequestValidationMiddlewareTest.php index 4e205c56..a0022db1 100644 --- a/tests/unit/Server/Middleware/RequestValidationMiddlewareTest.php +++ b/tests/unit/Server/Middleware/RequestValidationMiddlewareTest.php @@ -18,20 +18,28 @@ use FreeDSx\Ldap\Protocol\LdapMessageRequest; use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext; use FreeDSx\Ldap\Server\Middleware\RequestValidationMiddleware; +use FreeDSx\Ldap\Server\Operation\OperationOutcomeResult; +use FreeDSx\Ldap\Server\Operation\OperationResult; use PHPUnit\Framework\TestCase; +use RuntimeException; +use Tests\Support\FreeDSx\Ldap\Middleware\CallbackMiddlewareHandler; use Tests\Support\FreeDSx\Ldap\Middleware\CallLog; use Tests\Support\FreeDSx\Ldap\Middleware\RecordingMiddlewareHandler; +use Tests\Support\FreeDSx\Ldap\Middleware\ThrowingMiddlewareHandler; final class RequestValidationMiddlewareTest extends TestCase { private RequestValidationMiddleware $subject; + private CallLog $log; + private RecordingMiddlewareHandler $next; protected function setUp(): void { $this->subject = new RequestValidationMiddleware(); - $this->next = new RecordingMiddlewareHandler(new CallLog()); + $this->log = new CallLog(); + $this->next = new RecordingMiddlewareHandler($this->log); } public function test_a_message_id_of_zero_is_rejected(): void @@ -45,20 +53,59 @@ public function test_a_message_id_of_zero_is_rejected(): void ); } - public function test_a_reused_message_id_is_rejected(): void + public function test_a_message_id_still_being_served_is_rejected(): void + { + $reuseWhileInFlight = new CallbackMiddlewareHandler(function (): OperationResult { + $this->subject->process( + $this->contextFor(1), + $this->next, + ); + + return OperationOutcomeResult::succeeded(); + }); + + $this->expectException(RequestValidationException::class); + $this->expectExceptionMessage('The message ID 1 is not valid.'); + + $this->subject->process( + $this->contextFor(1), + $reuseWhileInFlight, + ); + } + + public function test_a_message_id_is_reusable_once_its_operation_completes(): void { $this->subject->process( $this->contextFor(1), $this->next, ); + $this->subject->process( + $this->contextFor(1), + $this->next, + ); - $this->expectException(RequestValidationException::class); - $this->expectExceptionMessage('The message ID 1 is not valid.'); + self::assertSame( + ['terminal', 'terminal'], + $this->log->entries, + ); + } + + public function test_an_id_is_released_when_the_operation_fails(): void + { + try { + $this->subject->process( + $this->contextFor(1), + new ThrowingMiddlewareHandler(new RuntimeException('Operation failed.')), + ); + } catch (RuntimeException) { + } $this->subject->process( $this->contextFor(1), $this->next, ); + + self::assertNotNull($this->next->received); } public function test_a_valid_message_id_is_delegated(): void diff --git a/tests/unit/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategyTest.php b/tests/unit/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategyTest.php index 8ce740b3..9e465747 100644 --- a/tests/unit/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategyTest.php +++ b/tests/unit/Server/PasswordPolicy/Guard/BindStrategy/ReplicaBindStrategyTest.php @@ -14,7 +14,9 @@ namespace Tests\Unit\FreeDSx\Ldap\Server\PasswordPolicy\Guard\BindStrategy; use FreeDSx\Ldap\Control\PwdPolicyError; +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\Server\Logging\EventLogger; @@ -30,6 +32,8 @@ use FreeDSx\Ldap\Server\PasswordPolicy\Replica\ReplicaPasswordStateStoreInterface; use FreeDSx\Ldap\Server\PasswordPolicy\Rules\PasswordLockoutRules; use FreeDSx\Ldap\Server\PasswordPolicy\UserPasswordState; +use FreeDSx\Ldap\Server\Backend\LdapBackendInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\Clock\FrozenClock; use Tests\Support\FreeDSx\Ldap\Logging\RecordingLogger; @@ -43,6 +47,8 @@ final class ReplicaBindStrategyTest extends TestCase private ReplicaPasswordStateStoreInterface $store; + private LdapBackendInterface&MockObject $backend; + private PasswordPolicyContext $context; private RecordingSleeper $sleeper; @@ -52,6 +58,7 @@ final class ReplicaBindStrategyTest extends TestCase protected function setUp(): void { $this->store = SqliteReplicaPasswordStateStoreFactory::inMemory(); + $this->backend = $this->createMock(LdapBackendInterface::class); $this->context = new PasswordPolicyContext(); $this->sleeper = new RecordingSleeper(); @@ -64,6 +71,7 @@ protected function setUp(): void new ReplicaBindStrategy( $engine, $this->store, + $this->backend, ), $this->context, new EventLogger( @@ -109,6 +117,31 @@ public function test_preBind_denies_a_replicated_entry_lock_with_no_local_state( $this->subject->preBind($this->attempt(new UserPasswordState(permanentlyLocked: true))); } + public function test_preBind_denies_when_the_entry_locked_after_the_caller_read_it(): void + { + $this->backend + ->method('get') + ->willReturn(new Entry( + new Dn(self::DN), + new Attribute('pwdAccountLockedTime', '20260520120000Z'), + )); + + $this->expectException(OperationException::class); + + $this->subject->preBind($this->attempt(new UserPasswordState())); + } + + public function test_preBind_allows_when_neither_the_local_state_nor_the_entry_locks(): void + { + $this->backend + ->method('get') + ->willReturn(new Entry(new Dn(self::DN))); + + $this->expectNotToPerformAssertions(); + + $this->subject->preBind($this->attempt(new UserPasswordState())); + } + public function test_failure_is_persisted_to_the_local_store(): void { $this->subject->recordFailure($this->attempt( diff --git a/tests/unit/Server/RequestHistoryTest.php b/tests/unit/Server/RequestHistoryTest.php index ccad4e94..d015b475 100644 --- a/tests/unit/Server/RequestHistoryTest.php +++ b/tests/unit/Server/RequestHistoryTest.php @@ -13,7 +13,6 @@ namespace Tests\Unit\FreeDSx\Ldap\Server; -use FreeDSx\Ldap\Exception\ProtocolException; use FreeDSx\Ldap\Server\RequestHistory; use PHPUnit\Framework\TestCase; @@ -26,31 +25,6 @@ protected function setUp(): void $this->subject = new RequestHistory(); } - public function test_it_should_add_a_valid_id(): void - { - $this->subject->addId(1); - - self::assertSame( - [1], - $this->subject->getIds(), - ); - } - - public function test_it_should_throw_when_adding_an_existing_id(): void - { - self::expectException(ProtocolException::class); - - $this->subject->addId(1); - $this->subject->addId(1); - } - - public function test_it_should_throw_when_adding_an_invalid_id(): void - { - self::expectException(ProtocolException::class); - - $this->subject->addId(0); - } - public function test_it_should_get_the_paging_requests(): void { diff --git a/tests/unit/Server/SearchLimit/SearchLimitResolverTest.php b/tests/unit/Server/SearchLimit/SearchLimitResolverTest.php index 6c35cb7e..f98d317c 100644 --- a/tests/unit/Server/SearchLimit/SearchLimitResolverTest.php +++ b/tests/unit/Server/SearchLimit/SearchLimitResolverTest.php @@ -60,6 +60,51 @@ public function test_it_returns_the_first_matching_rule_limits(): void ); } + public function test_a_rule_silent_on_paging_sessions_keeps_the_server_wide_cap(): void + { + $resolver = new SearchLimitResolver( + (new SearchLimitRules())->withRules( + SearchLimitRule::for(Subject::authenticated(), new SearchLimits(maxSearchSize: 50)), + ), + new SearchLimits(maxPagingSessions: 25), + ); + + self::assertSame( + 25, + $resolver->resolve($this->authenticatedToken())->maxPagingSessions, + ); + } + + public function test_a_rule_can_raise_the_paging_session_cap_for_an_identity(): void + { + $resolver = new SearchLimitResolver( + (new SearchLimitRules())->withRules( + SearchLimitRule::for(Subject::authenticated(), new SearchLimits(maxPagingSessions: 100)), + ), + new SearchLimits(maxPagingSessions: 25), + ); + + self::assertSame( + 100, + $resolver->resolve($this->authenticatedToken())->maxPagingSessions, + ); + } + + public function test_a_rule_can_lift_the_paging_session_cap_entirely(): void + { + $resolver = new SearchLimitResolver( + (new SearchLimitRules())->withRules( + SearchLimitRule::for(Subject::authenticated(), new SearchLimits(maxPagingSessions: 0)), + ), + new SearchLimits(maxPagingSessions: 25), + ); + + self::assertSame( + 0, + $resolver->resolve($this->authenticatedToken())->maxPagingSessions, + ); + } + public function test_anonymous_falls_through_authenticated_rule_to_the_default(): void { $resolver = new SearchLimitResolver( diff --git a/tests/unit/ServerOptionsTest.php b/tests/unit/ServerOptionsTest.php index 8c022653..63a547af 100644 --- a/tests/unit/ServerOptionsTest.php +++ b/tests/unit/ServerOptionsTest.php @@ -578,7 +578,8 @@ public function test_make_search_limits_reflects_current_options(): void ->setMaxSearchTimeLimit(60) ->setMaxSearchPageSize(250) ->setMaxSearchLookthrough(5000) - ->setMaxSearchPagedLookthrough(100000); + ->setMaxSearchPagedLookthrough(100000) + ->setMaxPagingSessions(5); self::assertEquals( new SearchLimits( @@ -587,6 +588,7 @@ public function test_make_search_limits_reflects_current_options(): void maxSearchPageSize: 250, maxSearchLookthrough: 5000, maxSearchPagedLookthrough: 100000, + maxPagingSessions: 5, ), $this->subject->makeSearchLimits(), );