diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/AttributeAdmission.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/AttributeAdmission.php new file mode 100644 index 00000000..32c9841c --- /dev/null +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/AttributeAdmission.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission; + +use FreeDSx\Ldap\Server\AccessControl\Read\ReadVerdict; + +/** + * What one search request does with an attribute description wherever an entry holds it. + * + * @internal + */ +final readonly class AttributeAdmission +{ + /** + * @param string $name The lowercased base type. + * @param bool $returned Whether the request selects it. + * @param bool $concernsFilter Whether withholding it can change whether the entry matches. + */ + public function __construct( + public string $name, + public bool $returned, + public ReadVerdict $read, + public bool $concernsFilter, + ) {} +} diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/SearchResultGate.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/SearchResultGate.php new file mode 100644 index 00000000..ed924c10 --- /dev/null +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/SearchResultGate.php @@ -0,0 +1,182 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AttributeProjection; +use FreeDSx\Ldap\Search\Filter\FilterInterface; +use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadVerdict; +use FreeDSx\Ldap\Server\Backend\Storage\Filter\StrippedEntryMatcher; +use FreeDSx\Ldap\Server\Token\TokenInterface; + +use function count; + +/** + * Decides in one pass what a search request returns of each entry it matched, remembering per attribute description. + * + * @internal + */ +final class SearchResultGate +{ + /** + * Stored descriptions are arbitrary, and only these many decisions are remembered. + */ + private const MAX_REMEMBERED_DESCRIPTIONS = 1024; + + /** + * @var array + */ + private array $admissions = []; + + /** + * Decisions are only kept from the second entry, since a request returning one gains nothing from them. + */ + private bool $remembering = false; + + private ReadScopeInterface $scope; + + private function __construct( + private readonly AccessControlInterface $accessControl, + private readonly StrippedEntryMatcher $strippedMatcher, + private readonly AttributeProjection $projection, + private readonly TokenInterface $token, + private readonly FilterInterface $filter, + ) { + $this->scope = $this->accessControl->readScope($this->token); + } + + public static function forRequest( + AccessControlInterface $accessControl, + StrippedEntryMatcher $strippedMatcher, + AttributeProjection $projection, + TokenInterface $token, + FilterInterface $filter, + ): self { + return new self( + $accessControl, + $strippedMatcher, + $projection, + $token, + $filter, + ); + } + + /** + * The entry as the request returns it, or null when the identity may not see it or it stops matching once stripped. + */ + public function admit(Entry $entry): ?Entry + { + $this->followReload(); + + if (!$this->scope->isEntryVisible($entry)) { + return null; + } + + $dn = $entry->getDn(); + $all = $entry->getAttributes(); + $returned = []; + $withheld = []; + + foreach ($all as $index => $attribute) { + $admission = $this->admissionFor($attribute); + // Kept inline for performance reasons. + $readable = match ($admission->read) { + ReadVerdict::Allow => true, + ReadVerdict::Deny => false, + ReadVerdict::PerEntry => $this->scope->isReadableOn( + $admission->name, + $dn, + ), + }; + + if ($readable && $admission->returned) { + $returned[] = $attribute; + } elseif (!$readable && $admission->concernsFilter) { + $withheld[$index] = true; + } + } + $this->remembering = true; + + if ($withheld !== [] && !$this->strippedMatcher->stillMatches($entry, $withheld, $this->filter)) { + return null; + } + + return $this->projection->withAttributes( + $entry, + $returned, + count($returned) === count($all), + ); + } + + /** + * A reload replaces the policy for the entries still to come, as it does for every other decision. + */ + private function followReload(): void + { + if ($this->scope->isCurrent()) { + return; + } + + $this->scope = $this->accessControl->readScope($this->token); + $this->admissions = []; + } + + private function admissionFor(Attribute $attribute): AttributeAdmission + { + $description = $attribute->getDescription(); + + if (isset($this->admissions[$description])) { + return $this->admissions[$description]; + } + $admission = $this->decideAdmission($attribute); + + if ($this->remembering && count($this->admissions) < self::MAX_REMEMBERED_DESCRIPTIONS) { + $this->admissions[$description] = $admission; + } + + return $admission; + } + + /** + * An attribute neither returned nor read by the filter is dropped without a read decision ever being made. + */ + private function decideAdmission(Attribute $attribute): AttributeAdmission + { + $name = Attribute::normalizeName($attribute->getName()); + $returned = $this->projection->includes($attribute); + + if (!$returned && !$this->strippedMatcher->concerns($this->filter, $name)) { + return new AttributeAdmission( + $name, + false, + ReadVerdict::Deny, + false, + ); + } + $read = $this->scope->verdict($name); + + return new AttributeAdmission( + $name, + $returned, + $read, + $read !== ReadVerdict::Allow && $this->strippedMatcher->concerns( + $this->filter, + $name, + ), + ); + } +} diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php index 1bee1b94..2902d7ba 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AssertionEvaluator.php @@ -89,13 +89,12 @@ public function assertSatisfiedBy( return; } - $readable = $this->accessControl->stripUnreadableAttributes( - $token, - $this->linkedLeaves->witness( + $readable = $this->accessControl + ->readScope($token) + ->readable($this->linkedLeaves->witness( $entry, $control->getFilter(), - ), - ); + )); if ($this->filterEvaluator->evaluate($readable, $control->getFilter())) { return; diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php index 099372b1..6619d70e 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php @@ -16,7 +16,6 @@ use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\Request\SearchRequest; -use FreeDSx\Ldap\Schema\Definition\AttributeUsage; use FreeDSx\Ldap\Schema\Schema; /** @@ -34,11 +33,13 @@ final class AttributeProjection { /** - * Per-instance memo of operational classification keyed on lowercased attribute name. - * - * @var array + * Selectors that name no attribute type, which leave a list of only these to be decided by usage alone. */ - private array $operationalByName = []; + private const SELECTORS = [ + SearchRequest::ATTRIBUTES_ALL_USER, + SearchRequest::ATTRIBUTES_ALL_OPERATIONAL, + SearchRequest::ATTRIBUTES_NONE, + ]; /** * Per-instance memo of the selection decision keyed on attribute description. @@ -49,6 +50,7 @@ final class AttributeProjection /** * @param string[] $names + * @param array $rangeSliceTypes Lowercased base types a requested range slice asks for. */ private function __construct( private readonly array $names, @@ -56,6 +58,8 @@ private function __construct( private readonly bool $wantsOperational, private readonly bool $returnNone, private readonly bool $typesOnly, + private readonly bool $namesNoType, + private readonly array $rangeSliceTypes, private readonly Schema $schema, ) {} @@ -78,6 +82,11 @@ public static function forRequest( in_array(SearchRequest::ATTRIBUTES_ALL_OPERATIONAL, $names, true), count($names) === 1 && $names[0] === SearchRequest::ATTRIBUTES_NONE, $typesOnly, + array_diff( + $names, + self::SELECTORS, + ) === [], + self::rangeSliceTypes($requestedAttrs), $schema, ); } @@ -92,31 +101,71 @@ public function project(Entry $entry): Entry } $attributes = $entry->getAttributes(); - $selected = array_filter( + $selected = array_values(array_filter( $attributes, - $this->shouldInclude(...), + $this->includes(...), + )); + + return $this->withAttributes( + $entry, + $selected, + count($selected) === count($attributes), ); + } - // Nothing was withheld, so the entry already is its own projection. - if (!$this->typesOnly && count($selected) === count($attributes)) { + /** + * Whether the request selects the attribute. + */ + public function includes(Attribute $attribute): bool + { + return $this->includeByDescription[$attribute->getDescription()] + ??= $this->decideInclude($attribute); + } + + /** + * The entry as returned with just the attributes kept. + * + * @param list $kept + */ + public function withAttributes( + Entry $entry, + array $kept, + bool $keptAll, + ): Entry { + if (!$this->typesOnly && $keptAll) { return $entry; } return Entry::raw( $entry->getDn(), - array_values( - $this->typesOnly - ? $this->withoutValues($selected) - : $selected, - ), + $this->typesOnly + ? $this->withoutValues($kept) + : $kept, ); } + /** + * @param Attribute[] $requestedAttrs + * @return array + */ + private static function rangeSliceTypes(array $requestedAttrs): array + { + $types = []; + + foreach ($requestedAttrs as $requested) { + if (self::namesRange($requested)) { + $types[Attribute::normalizeName($requested->getName())] = true; + } + } + + return $types; + } + /** * Options are part of the description a client asked for, so they survive a types-only request. * - * @param array $attributes - * @return array + * @param list $attributes + * @return list */ private function withoutValues(array $attributes): array { @@ -126,15 +175,17 @@ private function withoutValues(array $attributes): array ); } - private function shouldInclude(Attribute $attribute): bool - { - return $this->includeByDescription[$attribute->getDescription()] - ??= $this->decideInclude($attribute); - } - private function decideInclude(Attribute $attribute): bool { - if ($this->names !== [] && in_array(strtolower($attribute->getDescription()), $this->names, true)) { + if ($this->returnNone) { + return false; + } + + if ($this->namesNoType) { + return $this->wantsByUsage($attribute); + } + + if (in_array(strtolower($attribute->getDescription()), $this->names, true)) { return true; } @@ -146,7 +197,12 @@ private function decideInclude(Attribute $attribute): bool return true; } - return $this->isOperational($attribute) + return $this->wantsByUsage($attribute); + } + + private function wantsByUsage(Attribute $attribute): bool + { + return $this->schema->isOperational($attribute->getDescription()) ? $this->wantsOperational : $this->wantsUser; } @@ -175,17 +231,7 @@ private function isNamedType(Attribute $attribute): bool */ private function isNamedSlice(Attribute $attribute): bool { - $held = Attribute::normalizeName($attribute->getName()); - - foreach ($this->names as $name) { - $wanted = new Attribute($name); - - if (self::namesRange($wanted) && Attribute::normalizeName($wanted->getName()) === $held) { - return true; - } - } - - return false; + return isset($this->rangeSliceTypes[Attribute::normalizeName($attribute->getName())]); } private static function namesRange(Attribute $attribute): bool @@ -198,18 +244,4 @@ private static function namesRange(Attribute $attribute): bool return false; } - - private function isOperational(Attribute $attribute): bool - { - $key = strtolower($attribute->getName()); - - if (isset($this->operationalByName[$key])) { - return $this->operationalByName[$key]; - } - - $type = $this->schema->getAttributeType($attribute->getName()); - - return $this->operationalByName[$key] = $type !== null - && $type->usage !== AttributeUsage::UserApplications; - } } diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php index c83249e5..22d41275 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php @@ -52,10 +52,9 @@ public function readable( Entry $entry, TokenInterface $token, ): Entry { - return $this->accessControl->stripUnreadableAttributes( - $token, - $entry, - ); + return $this->accessControl + ->readScope($token) + ->readable($entry); } /** diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/MatchedDnAccessFilterTrait.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/MatchedDnAccessFilterTrait.php index 45a906fb..4c0101ea 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/MatchedDnAccessFilterTrait.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/MatchedDnAccessFilterTrait.php @@ -47,8 +47,13 @@ private function filterMatchedDn( return new Dn(''); } - return $accessControl->filterEntry($token, $entry) === null - ? new Dn('') - : $matchedDn; + $visible = $accessControl->isEntryVisible( + $token, + $entry, + ); + + return $visible + ? $matchedDn + : new Dn(''); } } diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php index 328c9155..a9204371 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ReadEntryControlHandler.php @@ -96,14 +96,11 @@ private function readEntry( return null; } - $readable = $this->accessControl->filterEntry( - $token, - $entry, - ); - - if ($readable === null) { + $scope = $this->accessControl->readScope($token); + if (!$scope->isEntryVisible($entry)) { return null; } + $readable = $scope->readable($entry); $projection = AttributeProjection::forRequest( array_map( diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php index 6429bbc4..8320acdd 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php @@ -18,6 +18,7 @@ use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Protocol\LdapMessageRequest; use FreeDSx\Ldap\Protocol\Queue\Response\Cancellation; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission\SearchResultGate; use FreeDSx\Ldap\Protocol\Queue\Response\QueueWriterConfig; use FreeDSx\Ldap\Protocol\Queue\Response\ResponseStream; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; @@ -140,20 +141,19 @@ private function filteredEntryStream( $request->getSizeLimit(), $this->limits->maxSearchSize(), ); - $filter = $request->getFilter(); $emitted = 0; + $gate = SearchResultGate::forRequest( + $this->accessControl, + $this->strippedMatcher, + $projection, + $token, + $request->getFilter(), + ); foreach ($backend->entries() as $entry) { - $filtered = $this->accessControl->filterEntry( - $token, - $entry, - ); - - if ($filtered === null) { - continue; - } + $returned = $gate->admit($entry); - if (!$this->strippedMatcher->stillMatches($entry, $filtered, $filter)) { + if ($returned === null) { continue; } @@ -164,7 +164,7 @@ private function filteredEntryStream( return; } - yield $projection->project($filtered); + yield $returned; $emitted++; $state->entriesReturned++; } diff --git a/src/FreeDSx/Ldap/Schema/Schema.php b/src/FreeDSx/Ldap/Schema/Schema.php index 6034d545..dd003c86 100644 --- a/src/FreeDSx/Ldap/Schema/Schema.php +++ b/src/FreeDSx/Ldap/Schema/Schema.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Schema\Definition\AttributeType; +use FreeDSx\Ldap\Schema\Definition\AttributeUsage; use FreeDSx\Ldap\Schema\Definition\LdapSyntax; use FreeDSx\Ldap\Schema\Definition\MatchingRule; use FreeDSx\Ldap\Schema\Definition\MatchingRuleOid; @@ -41,6 +42,11 @@ final class Schema private const MAX_REMEMBERED_DESCRIPTIONS = 1024; + /** + * Pairs grow with the stored descriptions times the names filters and selections ask for, and get more room. + */ + private const MAX_REMEMBERED_SUBTYPE_PAIRS = 8192; + /** * Rules RFC 4517 and RFC 4530 define as ordering rules. */ @@ -81,6 +87,22 @@ final class Schema */ private array $confidentialDescriptions = []; + /** + * Whether each description seen names a type with operational usage, dropped whenever the type set changes. + * + * @var array + */ + private array $operationalDescriptions = []; + + /** + * Whether each pair of descriptions seen is a subtype pair, by description then by the one it was tested against. + * + * @var array> + */ + private array $subtypeDescriptions = []; + + private int $subtypePairs = 0; + /** * Bound to this schema, so it is built once rather than per lookup. */ @@ -110,6 +132,9 @@ public function addAttributeType(AttributeType $type): static $this->superTypeOids = null; $this->orderingRuleOids = null; $this->confidentialDescriptions = []; + $this->operationalDescriptions = []; + $this->subtypeDescriptions = []; + $this->subtypePairs = 0; return $this; } @@ -169,6 +194,26 @@ public function isConfidential(string $description): bool return $confidential; } + /** + * Whether a description names a type with operational usage, which an undefined type never has. + * + * @internal + */ + public function isOperational(string $description): bool + { + if (isset($this->operationalDescriptions[$description])) { + return $this->operationalDescriptions[$description]; + } + $usage = $this->getAttributeType(Attribute::normalizeName($description))?->usage; + $operational = $usage !== null && $usage !== AttributeUsage::UserApplications; + + if (count($this->operationalDescriptions) < self::MAX_REMEMBERED_DESCRIPTIONS) { + $this->operationalDescriptions[$description] = $operational; + } + + return $operational; + } + /** * Whether the attribute orders numerically. */ @@ -269,17 +314,20 @@ public function isDescriptionSubtypeOf( string $description, string $ofDescription, ): bool { - $type = Attribute::normalizeName($description); - $ofType = Attribute::normalizeName($ofDescription); - - if ($type !== $ofType && !$this->isTypeOrSubtypeOf($type, $ofType)) { - return false; + if (isset($this->subtypeDescriptions[$description][$ofDescription])) { + return $this->subtypeDescriptions[$description][$ofDescription]; } - - return self::optionsCover( + $isSubtype = $this->decideDescriptionSubtypeOf( $description, $ofDescription, ); + + if ($this->subtypePairs < self::MAX_REMEMBERED_SUBTYPE_PAIRS) { + $this->subtypeDescriptions[$description][$ofDescription] = $isSubtype; + $this->subtypePairs++; + } + + return $isSubtype; } /** @@ -531,6 +579,23 @@ private static function primarySpelling( : $primary; } + private function decideDescriptionSubtypeOf( + string $description, + string $ofDescription, + ): bool { + $type = Attribute::normalizeName($description); + $ofType = Attribute::normalizeName($ofDescription); + + if ($type !== $ofType && !$this->isTypeOrSubtypeOf($type, $ofType)) { + return false; + } + + return self::optionsCover( + $description, + $ofDescription, + ); + } + /** * Whether every option the wanted description carries is also carried by the one being tested. */ diff --git a/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php b/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php index 3eade7a6..174ccdaf 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php @@ -17,6 +17,7 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\OperationType; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; use FreeDSx\Ldap\Server\AccessControl\Rule\RelocationAccess; use FreeDSx\Ldap\Server\Token\TokenInterface; @@ -121,20 +122,9 @@ public function mayFilterOnAttribute( ): bool; /** - * Return $entry with unreadable attributes removed, or null to suppress the entry entirely. + * The read decisions $token holds, to be kept for one request and asked again once it is no longer current. */ - public function filterEntry( - TokenInterface $token, - Entry $entry, - ): ?Entry; - - /** - * Return $entry with unreadable attributes removed, for callers that have already settled visibility. - */ - public function stripUnreadableAttributes( - TokenInterface $token, - Entry $entry, - ): Entry; + public function readScope(TokenInterface $token): ReadScopeInterface; /** * Whether $token may see $entry at all, without stripping any attributes (the entry-level gate replication uses). diff --git a/src/FreeDSx/Ldap/Server/AccessControl/AclRuleIndex.php b/src/FreeDSx/Ldap/Server/AccessControl/AclRuleIndex.php index b75a8235..d7a042dc 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/AclRuleIndex.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/AclRuleIndex.php @@ -86,12 +86,6 @@ public function attributeRules( ?? $this->anyAttributeRules[$access->name]; } - public function hasAttributeRules(AttributeAccess $access): bool - { - return $this->namedAttributeRules[$access->name] !== [] - || $this->anyAttributeRules[$access->name] !== []; - } - /** * @return list */ diff --git a/src/FreeDSx/Ldap/Server/AccessControl/ConfidentialAttributeAccessControl.php b/src/FreeDSx/Ldap/Server/AccessControl/ConfidentialAttributeAccessControl.php index 041516fa..4a05d4d6 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/ConfidentialAttributeAccessControl.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/ConfidentialAttributeAccessControl.php @@ -16,13 +16,13 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\OperationType; +use FreeDSx\Ldap\Server\AccessControl\Read\ConfidentialReadScope; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; use FreeDSx\Ldap\Server\AccessControl\Rule\RelocationAccess; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Token\TokenInterface; -use function count; - /** * Wraps a policy so withheld confidential attributes are stripped from results whatever its own rules allow. * @@ -42,23 +42,11 @@ public function setBackend(ReadBackendInterface $backend): void } } - public function filterEntry( - TokenInterface $token, - Entry $entry, - ): ?Entry { - $filtered = $this->inner->filterEntry($token, $entry); - - return $filtered === null - ? null - : $this->stripWithheld($filtered, $token); - } - - public function stripUnreadableAttributes( - TokenInterface $token, - Entry $entry, - ): Entry { - return $this->stripWithheld( - $this->inner->stripUnreadableAttributes($token, $entry), + public function readScope(TokenInterface $token): ReadScopeInterface + { + return new ConfidentialReadScope( + $this->inner->readScope($token), + $this->policy, $token, ); } @@ -159,30 +147,4 @@ public function isEntryVisible( ): bool { return $this->inner->isEntryVisible($token, $entry); } - - /** - * The entry itself is returned when nothing was withheld, so a caller can still tell it went untouched. - */ - private function stripWithheld( - Entry $entry, - TokenInterface $token, - ): Entry { - $all = $entry->getAttributes(); - $kept = []; - - foreach ($all as $attribute) { - if (!$this->policy->isWithheldFromResult($attribute->getName(), $token)) { - $kept[] = $attribute; - } - } - - if (count($kept) === count($all)) { - return $entry; - } - - return Entry::raw( - $entry->getDn(), - $kept, - ); - } } diff --git a/src/FreeDSx/Ldap/Server/AccessControl/PrivilegedBypassAccessControl.php b/src/FreeDSx/Ldap/Server/AccessControl/PrivilegedBypassAccessControl.php index b4a1daad..bfe47abe 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/PrivilegedBypassAccessControl.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/PrivilegedBypassAccessControl.php @@ -16,6 +16,8 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\OperationType; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\UnrestrictedReadScope; use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; use FreeDSx\Ldap\Server\AccessControl\Rule\RelocationAccess; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; @@ -151,32 +153,13 @@ public function mayFilterOnAttribute( ); } - public function filterEntry( - TokenInterface $token, - Entry $entry, - ): ?Entry { - if ($token instanceof PrivilegedTokenInterface) { - return $entry; - } - - return $this->inner->filterEntry( - $token, - $entry, - ); - } - - public function stripUnreadableAttributes( - TokenInterface $token, - Entry $entry, - ): Entry { + public function readScope(TokenInterface $token): ReadScopeInterface + { if ($token instanceof PrivilegedTokenInterface) { - return $entry; + return new UnrestrictedReadScope(); } - return $this->inner->stripUnreadableAttributes( - $token, - $entry, - ); + return $this->inner->readScope($token); } public function isEntryVisible( diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/ConfidentialReadScope.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/ConfidentialReadScope.php new file mode 100644 index 00000000..6b64bb1b --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/ConfidentialReadScope.php @@ -0,0 +1,72 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Server\AccessControl\WithheldAttributePolicy; +use FreeDSx\Ldap\Server\Token\TokenInterface; + +/** + * Withholds confidential attributes the identity holds no grant over. + * + * @internal + * + * @author Chad Sikorra + */ +final class ConfidentialReadScope implements ReadScopeInterface +{ + use ReadsEntryAttributesTrait; + + public function __construct( + private readonly ReadScopeInterface $inner, + private readonly WithheldAttributePolicy $policy, + private readonly TokenInterface $token, + ) {} + + public function isCurrent(): bool + { + return $this->inner->isCurrent(); + } + + public function isEntryVisible(Entry $entry): bool + { + return $this->inner->isEntryVisible($entry); + } + + public function verdict(string $name): ReadVerdict + { + $verdict = $this->inner->verdict($name); + + if ($verdict === ReadVerdict::Deny || !$this->policy->isConfidential($name)) { + return $verdict; + } + + return ReadVerdict::PerEntry; + } + + public function isReadableOn( + string $name, + Dn $dn, + ): bool { + if (!$this->inner->isReadableOn($name, $dn)) { + return false; + } + + return !$this->policy->isWithheldFromResult( + $name, + $this->token, + ); + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadScopeInterface.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadScopeInterface.php new file mode 100644 index 00000000..6593f8da --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadScopeInterface.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; + +/** + * The read decisions one identity holds for one request. + * + * @internal + * + * @author Chad Sikorra + */ +interface ReadScopeInterface +{ + /** + * Whether the policy these decisions came from still decides. + */ + public function isCurrent(): bool; + + public function isEntryVisible(Entry $entry): bool; + + /** + * The read decision for an attribute wherever it is held, or PerEntry when the entry holding it decides. + */ + public function verdict(string $name): ReadVerdict; + + /** + * Whether the attribute may be read on the entry at this DN. + */ + public function isReadableOn( + string $name, + Dn $dn, + ): bool; + + /** + * The entry without the attributes that may not be read, itself when nothing was removed. + */ + public function readable(Entry $entry): Entry; +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadVerdict.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadVerdict.php new file mode 100644 index 00000000..f1c0dbe3 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadVerdict.php @@ -0,0 +1,39 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Server\AccessControl\Rule\Effect; + +/** + * Whether an attribute may be read wherever it is held, or only the entry holding it can say. + * + * @internal + * + * @author Chad Sikorra + */ +enum ReadVerdict +{ + case Allow; + + case Deny; + + case PerEntry; + + public static function fromEffect(Effect $effect): self + { + return $effect === Effect::Allow + ? self::Allow + : self::Deny; + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadsEntryAttributesTrait.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadsEntryAttributesTrait.php new file mode 100644 index 00000000..c3cbbafa --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReadsEntryAttributesTrait.php @@ -0,0 +1,57 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; + +use function count; + +/** + * Strips an entry to what a read scope's per-attribute decisions allow. + * + * @internal + * + * @author Chad Sikorra + */ +trait ReadsEntryAttributesTrait +{ + abstract public function isReadableOn( + string $name, + Dn $dn, + ): bool; + + public function readable(Entry $entry): Entry + { + $dn = $entry->getDn(); + $all = $entry->getAttributes(); + $kept = []; + + foreach ($all as $attribute) { + if ($this->isReadableOn(Attribute::normalizeName($attribute->getName()), $dn)) { + $kept[] = $attribute; + } + } + + if (count($kept) === count($all)) { + return $entry; + } + + return Entry::raw( + $dn, + $kept, + ); + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/ReloadableReadScope.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReloadableReadScope.php new file mode 100644 index 00000000..3a15df44 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/ReloadableReadScope.php @@ -0,0 +1,65 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Server\AccessControl\ReloadableAccessControl; + +/** + * The decisions of the policy installed when the request began, which stop being current once a reload replaces it. + * + * @internal + * + * @author Chad Sikorra + */ +final readonly class ReloadableReadScope implements ReadScopeInterface +{ + public function __construct( + private ReloadableAccessControl $owner, + private int $generation, + private ReadScopeInterface $inner, + ) {} + + public function isCurrent(): bool + { + return $this->owner->generation() === $this->generation + && $this->inner->isCurrent(); + } + + public function isEntryVisible(Entry $entry): bool + { + return $this->inner->isEntryVisible($entry); + } + + public function verdict(string $name): ReadVerdict + { + return $this->inner->verdict($name); + } + + public function isReadableOn( + string $name, + Dn $dn, + ): bool { + return $this->inner->isReadableOn( + $name, + $dn, + ); + } + + public function readable(Entry $entry): Entry + { + return $this->inner->readable($entry); + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/RuleReadScope.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/RuleReadScope.php new file mode 100644 index 00000000..7d40c4f3 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/RuleReadScope.php @@ -0,0 +1,101 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Exception\SubjectEvaluationException; +use FreeDSx\Ldap\Server\AccessControl\AclRuleIndex; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeRule; +use FreeDSx\Ldap\Server\AccessControl\Rule\Effect; +use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; +use FreeDSx\Ldap\Server\AccessControl\Subject\TokenDecidedSubjectInterface; +use FreeDSx\Ldap\Server\AccessControl\Target\AnyTargetMatcher; +use FreeDSx\Ldap\Server\Token\TokenInterface; + +/** + * Decides once per request what the rules make of an attribute anywhere, leaving the rest to each entry's DN. + * + * @internal + * + * @author Chad Sikorra + */ +final class RuleReadScope implements ReadScopeInterface +{ + use ReadsEntryAttributesTrait; + + public function __construct( + private readonly RuleBasedAccessControl $policy, + private readonly AclRuleIndex $index, + private readonly TokenInterface $token, + ) {} + + public function isCurrent(): bool + { + return true; + } + + public function isEntryVisible(Entry $entry): bool + { + return $this->policy->isEntryVisible( + $this->token, + $entry, + ); + } + + /** + * The first rule that matches anywhere decides, as it would on every entry; any rule that needs the entry defers. + */ + public function verdict(string $name): ReadVerdict + { + foreach ($this->index->attributeRules(AttributeAccess::Read, $name) as $rule) { + if (!$rule->target instanceof AnyTargetMatcher || !$rule->subject instanceof TokenDecidedSubjectInterface) { + return ReadVerdict::PerEntry; + } + + if ($this->subjectMatches($rule)) { + return ReadVerdict::fromEffect($rule->effect); + } + } + + return ReadVerdict::Allow; + } + + public function isReadableOn( + string $name, + Dn $dn, + ): bool { + return $this->policy->isAttributeReadable( + $this->token, + $dn, + $name, + ); + } + + /** + * A subject that cannot be decided counts as matching a deny, as it does when each entry is checked. + */ + private function subjectMatches(AttributeRule $rule): bool + { + try { + return $rule->subject->matches( + $this->token, + null, + ); + } catch (SubjectEvaluationException) { + return $rule->effect === Effect::Deny; + } + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Read/UnrestrictedReadScope.php b/src/FreeDSx/Ldap/Server/AccessControl/Read/UnrestrictedReadScope.php new file mode 100644 index 00000000..29165800 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Read/UnrestrictedReadScope.php @@ -0,0 +1,54 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; + +/** + * Reads everything, for the privileged identity that bypasses access control. + * + * @internal + * + * @author Chad Sikorra + */ +final class UnrestrictedReadScope implements ReadScopeInterface +{ + public function isCurrent(): bool + { + return true; + } + + public function isEntryVisible(Entry $entry): bool + { + return true; + } + + public function verdict(string $name): ReadVerdict + { + return ReadVerdict::Allow; + } + + public function isReadableOn( + string $name, + Dn $dn, + ): bool { + return true; + } + + public function readable(Entry $entry): Entry + { + return $entry; + } +} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/ReloadableAccessControl.php b/src/FreeDSx/Ldap/Server/AccessControl/ReloadableAccessControl.php index 5b14f864..a0f4685b 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/ReloadableAccessControl.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/ReloadableAccessControl.php @@ -16,6 +16,8 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\OperationType; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\ReloadableReadScope; use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; use FreeDSx\Ldap\Server\AccessControl\Rule\RelocationAccess; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; @@ -30,6 +32,8 @@ final class ReloadableAccessControl implements AccessControlInterface, BackendAw { private ?ReadBackendInterface $backend = null; + private int $generation = 0; + public function __construct(private AccessControlInterface $current) {} /** @@ -42,6 +46,17 @@ public function replace(AccessControlInterface $policy): void } $this->current = $policy; + $this->generation++; + } + + /** + * How many times the policy has been replaced. This tells a held read scope it is out of date. + * + * @internal + */ + public function generation(): int + { + return $this->generation; } public function setBackend(ReadBackendInterface $backend): void @@ -143,23 +158,12 @@ public function mayFilterOnAttribute( ); } - public function filterEntry( - TokenInterface $token, - Entry $entry, - ): ?Entry { - return $this->current->filterEntry( - $token, - $entry, - ); - } - - public function stripUnreadableAttributes( - TokenInterface $token, - Entry $entry, - ): Entry { - return $this->current->stripUnreadableAttributes( - $token, - $entry, + public function readScope(TokenInterface $token): ReadScopeInterface + { + return new ReloadableReadScope( + $this, + $this->generation, + $this->current->readScope($token), ); } diff --git a/src/FreeDSx/Ldap/Server/AccessControl/RuleBasedAccessControl.php b/src/FreeDSx/Ldap/Server/AccessControl/RuleBasedAccessControl.php index a89184b6..06f7e570 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/RuleBasedAccessControl.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/RuleBasedAccessControl.php @@ -20,6 +20,8 @@ use FreeDSx\Ldap\Exception\SubjectEvaluationException; use FreeDSx\Ldap\Operation\OperationType; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\RuleReadScope; use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeAccess; use FreeDSx\Ldap\Server\AccessControl\Rule\ConfidentialAccessRule; use FreeDSx\Ldap\Server\AccessControl\Rule\ControlRule; @@ -190,55 +192,32 @@ public function mayFilterOnAttribute( return true; } - public function filterEntry( - TokenInterface $token, - Entry $entry, - ): ?Entry { - if (!$this->isEntryVisible($token, $entry)) { - return null; - } - - return $this->stripUnreadableAttributes( + public function readScope(TokenInterface $token): ReadScopeInterface + { + return new RuleReadScope( + $this, + $this->index, $token, - $entry, ); } - public function stripUnreadableAttributes( + /** + * Whether the rules let $token read an attribute, by its lowercased base name, on the entry at $dn. + * + * @internal + */ + public function isAttributeReadable( TokenInterface $token, - Entry $entry, - ): Entry { - $dn = $entry->getDn(); - - if (!$this->index->hasAttributeRules(AttributeAccess::Read)) { - return $entry; - } - - $allAttributes = $entry->getAttributes(); - $kept = []; - - foreach ($allAttributes as $attribute) { - $effect = $this->resolveAttributeEffect( - $token, - $dn, - strtolower($attribute->getName()), - AttributeAccess::Read, - Effect::Allow, - ); - - if ($effect !== Effect::Deny) { - $kept[] = $attribute; - } - } - - if (count($kept) === count($allAttributes)) { - return $entry; - } - - return Entry::raw( + Dn $dn, + string $name, + ): bool { + return $this->resolveAttributeEffect( + $token, $dn, - $kept, - ); + $name, + AttributeAccess::Read, + Effect::Allow, + ) !== Effect::Deny; } public function isEntryVisible( diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnonymousSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnonymousSubjectMatcher.php index 86522c8d..10be4e3f 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnonymousSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnonymousSubjectMatcher.php @@ -22,7 +22,7 @@ * * @author Chad Sikorra */ -final class AnonymousSubjectMatcher implements SubjectMatcherInterface +final class AnonymousSubjectMatcher implements TokenDecidedSubjectInterface { public function matches( TokenInterface $token, diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnySubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnySubjectMatcher.php index 164f92df..367178e1 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnySubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AnySubjectMatcher.php @@ -21,7 +21,7 @@ * * @author Chad Sikorra */ -final class AnySubjectMatcher implements SubjectMatcherInterface +final class AnySubjectMatcher implements TokenDecidedSubjectInterface { public function matches( TokenInterface $token, diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AuthenticatedSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AuthenticatedSubjectMatcher.php index 43e5c438..5db3046a 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/AuthenticatedSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/AuthenticatedSubjectMatcher.php @@ -22,7 +22,7 @@ * * @author Chad Sikorra */ -final class AuthenticatedSubjectMatcher implements SubjectMatcherInterface +final class AuthenticatedSubjectMatcher implements TokenDecidedSubjectInterface { public function matches( TokenInterface $token, diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php index bfb2a534..19bba113 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php @@ -22,9 +22,9 @@ * * @author Chad Sikorra */ -final class DnSubjectMatcher implements SubjectMatcherInterface +final readonly class DnSubjectMatcher implements TokenDecidedSubjectInterface { - private readonly string $normalizedDn; + private string $normalizedDn; public function __construct(string $dn) { diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubtreeSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubtreeSubjectMatcher.php index 4fc9c1f5..f8610b81 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubtreeSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubtreeSubjectMatcher.php @@ -23,9 +23,9 @@ * * @author Chad Sikorra */ -final class DnSubtreeSubjectMatcher implements SubjectMatcherInterface +final readonly class DnSubtreeSubjectMatcher implements TokenDecidedSubjectInterface { - private readonly Dn $subtreeDn; + private Dn $subtreeDn; public function __construct(string $dn) { diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/TokenDecidedSubjectInterface.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/TokenDecidedSubjectInterface.php new file mode 100644 index 00000000..33781a0b --- /dev/null +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/TokenDecidedSubjectInterface.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\AccessControl\Subject; + +/** + * Marks a subject the bound identity alone decides, never the target and never by throwing. + * + * @internal + * + * @author Chad Sikorra + */ +interface TokenDecidedSubjectInterface extends SubjectMatcherInterface {} diff --git a/src/FreeDSx/Ldap/Server/AccessControl/WithheldAttributePolicy.php b/src/FreeDSx/Ldap/Server/AccessControl/WithheldAttributePolicy.php index 7e8508fa..2a47443f 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/WithheldAttributePolicy.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/WithheldAttributePolicy.php @@ -31,6 +31,14 @@ public function __construct( private ?Schema $schema = null, ) {} + /** + * Whether the schema marks the attribute confidential, whoever is asking. + */ + public function isConfidential(string $attribute): bool + { + return $this->schema?->isConfidential($attribute) === true; + } + /** * Whether the schema marks the attribute confidential, and the token holds no grant over it. */ @@ -38,7 +46,7 @@ public function isWithheldFromResult( string $attribute, TokenInterface $token, ): bool { - if ($this->schema?->isConfidential($attribute) !== true) { + if (!$this->isConfidential($attribute)) { return false; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Filter/StrippedEntryMatcher.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Filter/StrippedEntryMatcher.php index 3811fc16..3096b38d 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Filter/StrippedEntryMatcher.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Filter/StrippedEntryMatcher.php @@ -22,12 +22,12 @@ use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; use WeakMap; +use function array_diff_key; use function array_values; use function count; -use function strtolower; /** - * Judges an entry access control stripped against the filter again whenever an attribute it lost is one the filter reads. + * Tells whether withholding an attribute can change a filter's answer, and judges an entry again when it did. * * @internal */ @@ -49,49 +49,51 @@ public function __construct( } /** - * Whether the stripped entry still matches a filter the entry it was stripped from already matched. + * Whether withholding a lowercased base type can change what the filter answers. */ - public function stillMatches( - Entry $entry, - Entry $stripped, + public function concerns( FilterInterface $filter, + string $type, ): bool { - if ($stripped === $entry) { + $readSet = $this->readSets[$filter] ??= $this->readSetOf($filter); + if ($readSet->types === null) { return true; } - if (!$this->lostAnyRead($entry, $stripped, $filter)) { - return true; + if (isset($readSet->concerns[$type])) { + return $readSet->concerns[$type]; } - - return $this->evaluator->evaluate( - $stripped, - $filter, + $concerns = $this->isRead( + $readSet, + $type, ); + if (count($readSet->concerns) < FilterReadSet::MAX_REMEMBERED) { + $readSet->concerns[$type] = $concerns; + } + + return $concerns; } - private function lostAnyRead( + /** + * Whether an entry that matched the filter still does without the attributes access control withheld from it. + * + * @param array $withheld The keys the entry holds its withheld attributes at. + */ + public function stillMatches( Entry $entry, - Entry $stripped, + array $withheld, FilterInterface $filter, ): bool { - $readSet = $this->readSets[$filter] ??= $this->readSetOf($filter); - $removed = self::removed( - $entry, - $stripped, + return $this->evaluator->evaluate( + Entry::raw( + $entry->getDn(), + array_values(array_diff_key( + $entry->getAttributes(), + $withheld, + )), + ), + $filter, ); - - if ($readSet->types === null || $removed === null) { - return true; - } - - foreach ($removed as $attribute) { - if ($this->concerns($readSet, strtolower($attribute->getName()))) { - return true; - } - } - - return false; } private function readSetOf(FilterInterface $filter): FilterReadSet @@ -109,57 +111,18 @@ private function readSetOf(FilterInterface $filter): FilterReadSet } /** - * Whether a removed type is one the filter reads, directly or as a subtype of one it names. + * Whether the filter reads the type, directly or as a subtype of one it names. */ - private function concerns( + private function isRead( FilterReadSet $readSet, - string $removedType, + string $type, ): bool { - if (isset($readSet->concerns[$removedType])) { - return $readSet->concerns[$removedType]; - } - $concerns = false; - - foreach ($readSet->types ?? [] as $type) { - if ($removedType === $type || $this->schema->isTypeOrSubtypeOf($removedType, $type)) { - $concerns = true; - - break; - } - } - - if (count($readSet->concerns) < FilterReadSet::MAX_REMEMBERED) { - $readSet->concerns[$removedType] = $concerns; - } - - return $concerns; - } - - /** - * The attributes stripping removed, or null when the stripped entry is not an ordered subset of the original. - * - * @return list|null - */ - private static function removed( - Entry $entry, - Entry $stripped, - ): ?array { - $kept = array_values($stripped->getAttributes()); - $keptCount = count($kept); - $removed = []; - $next = 0; - - foreach ($entry->getAttributes() as $attribute) { - if ($next < $keptCount && $kept[$next] === $attribute) { - $next++; - - continue; + foreach ($readSet->types ?? [] as $readType) { + if ($type === $readType || $this->schema->isTypeOrSubtypeOf($type, $readType)) { + return true; } - $removed[] = $attribute; } - return $next === $keptCount - ? $removed - : null; + return false; } } diff --git a/src/FreeDSx/Ldap/Server/Paging/PageFiller.php b/src/FreeDSx/Ldap/Server/Paging/PageFiller.php index 43d31083..e4b020db 100644 --- a/src/FreeDSx/Ldap/Server/Paging/PageFiller.php +++ b/src/FreeDSx/Ldap/Server/Paging/PageFiller.php @@ -13,14 +13,13 @@ namespace FreeDSx\Ldap\Server\Paging; -use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\Request\SearchRequest; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission\SearchResultGate; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AttributeProjection; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerSearchTrait; use FreeDSx\Ldap\Schema\Schema; -use FreeDSx\Ldap\Search\Filter\FilterInterface; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Storage\EntryStream; @@ -64,7 +63,13 @@ public function fill( $request = $pagingRequest->getSearchRequest(); $page = $this->startPage($pagingRequest, $resumeFrom); $budget = $this->budgetFor($request); - $projection = $this->projectionFor($request); + $gate = SearchResultGate::forRequest( + $this->accessControl, + $this->strippedMatcher, + $this->projectionFor($request), + $token, + $request->getFilter(), + ); $exhausted = false; $hasFurtherMatch = false; @@ -86,9 +91,7 @@ public function fill( $entries, $page, $isFilling, - $token, - $request->getFilter(), - $projection, + $gate, ); // The stream was abandoned mid-slice and can say nothing about where it stopped. @@ -126,17 +129,11 @@ private function collectSlice( Generator $entries, FillingPage $page, bool $isFilling, - TokenInterface $token, - FilterInterface $filter, - AttributeProjection $projection, + SearchResultGate $gate, ): SliceEnd { try { foreach ($entries as $fetched) { - $kept = $this->keepForPage( - $fetched->entry, - $token, - $filter, - ); + $kept = $gate->admit($fetched->entry); if ($kept === null) { continue; @@ -155,7 +152,7 @@ private function collectSlice( } $page->add( - $projection->project($kept), + $kept, $fetched->cursor, ); } @@ -234,31 +231,6 @@ private function readSlice( ); } - /** - * The entry as this identity may see it, or null when it may not see it at all. - * - * @throws OperationException - */ - private function keepForPage( - Entry $entry, - TokenInterface $token, - FilterInterface $filter, - ): ?Entry { - $filtered = $this->accessControl->filterEntry( - $token, - $entry, - ); - - if ($filtered === null) { - return null; - } - - // Stripping an attribute can cost the entry its match. - return $this->strippedMatcher->stillMatches($entry, $filtered, $filter) - ? $filtered - : null; - } - private function projectionFor(SearchRequest $request): AttributeProjection { return AttributeProjection::forRequest( diff --git a/tests/integration/Security/DefaultAccessControlEnforcementTest.php b/tests/integration/Security/DefaultAccessControlEnforcementTest.php index 126a89b5..4a37f702 100644 --- a/tests/integration/Security/DefaultAccessControlEnforcementTest.php +++ b/tests/integration/Security/DefaultAccessControlEnforcementTest.php @@ -159,10 +159,10 @@ public function test_manager_read_keeps_userPassword_that_a_normal_user_loses(): ); self::assertNotNull( - $this->accessControl->filterEntry($manager, $entry)?->get('userPassword'), + $this->accessControl->readScope($manager)->readable($entry)->get('userPassword'), ); self::assertNull( - $this->accessControl->filterEntry($user, $entry)?->get('userPassword'), + $this->accessControl->readScope($user)->readable($entry)->get('userPassword'), ); } @@ -188,7 +188,7 @@ public function test_userPassword_is_write_only_for_self(): void // But self cannot read it, on search or via Compare. self::assertNull( - $this->accessControl->filterEntry($user, $ownEntry)?->get('userPassword'), + $this->accessControl->readScope($user)->readable($ownEntry)->get('userPassword'), ); $this->expectException(OperationException::class); diff --git a/tests/performance/Compare/BenchCompareCommand.php b/tests/performance/Compare/BenchCompareCommand.php index ef92177c..7cb67a1d 100644 --- a/tests/performance/Compare/BenchCompareCommand.php +++ b/tests/performance/Compare/BenchCompareCommand.php @@ -158,6 +158,18 @@ protected function configure(): void 'Password paired with --source-bind-dn', self::DEFAULT_SOURCE_BIND_PASSWORD, ) + ->addOption( + 'source-run-bind-dn', + null, + InputOption::VALUE_REQUIRED, + 'DN the workload binds to the source as, subject to its access control (defaults to --source-bind-dn)', + ) + ->addOption( + 'source-run-bind-password', + null, + InputOption::VALUE_REQUIRED, + 'Password paired with --source-run-bind-dn', + ) ->addOption( 'source-base-dn', null, @@ -200,6 +212,18 @@ protected function configure(): void 'Password paired with --target-bind-dn', self::DEFAULT_TARGET_BIND_PASSWORD, ) + ->addOption( + 'target-run-bind-dn', + null, + InputOption::VALUE_REQUIRED, + 'DN the workload binds to the target as, subject to its access control (defaults to --target-bind-dn)', + ) + ->addOption( + 'target-run-bind-password', + null, + InputOption::VALUE_REQUIRED, + 'Password paired with --target-run-bind-dn', + ) ->addOption( 'target-base-dn', null, @@ -294,8 +318,8 @@ protected function execute( $targetSide = $this->sideFromOptions($input, 'target'); $sourceSide = $this->sideFromOptions($input, 'source'); - $targetSnapshot = null; - $sourceSnapshot = null; + $targetRun = null; + $sourceRun = null; $benches = []; // Both sides are contacted first: seeding one side is wasted work if the other turns out to be unreachable. @@ -319,10 +343,22 @@ protected function execute( try { if (!$skipTarget) { - $targetSnapshot = $this->runSide($output, $progress, $targetSide, $params, $benches); + $targetRun = $this->runSide( + $output, + $progress, + $targetSide, + $params, + $benches, + ); } if (!$skipSource) { - $sourceSnapshot = $this->runSide($output, $progress, $sourceSide, $params, $benches); + $sourceRun = $this->runSide( + $output, + $progress, + $sourceSide, + $params, + $benches, + ); } } catch (Throwable $e) { $output->writeln('Benchmark failed: ' . $e->getMessage() . ''); @@ -333,15 +369,20 @@ protected function execute( } $format = $this->requireString($input, 'output'); - (new ComparisonReport( - target: $targetSnapshot, - source: $sourceSnapshot, + $report = new ComparisonReport( + target: $targetRun?->snapshot, + source: $sourceRun?->snapshot, format: $format, targetLabel: $targetSide->label, sourceLabel: $sourceSide->label, - ))->render($output); + targetAccessProblems: $targetRun->accessProblems ?? [], + sourceAccessProblems: $sourceRun->accessProblems ?? [], + ); + $report->render($output); - return Command::SUCCESS; + return $report->invalidReasons() === [] + ? Command::SUCCESS + : Command::FAILURE; } /** @@ -357,6 +398,10 @@ private function unreachableSides(array $sides): array try { $bench->preflight(); + $bench->assertBindable( + $side->runBindDn, + $side->runBindPassword, + ); } catch (Throwable $e) { $reasons[] = $e->getMessage(); } finally { @@ -388,10 +433,16 @@ private function runSide( BenchSide $side, array $params, array &$benches, - ): StatsSnapshot { + ): SideRun { $bench = $this->makeBench($side); $benches[] = $bench; + $bench->ensureSubtree(); + $bench->assertWritableBy( + $side->runBindDn, + $side->runBindPassword, + ); + $this->seedSide( $progress, $side, @@ -401,6 +452,13 @@ private function runSide( $params['seedGroupSize'], ); + $accessProblems = $params['seedEntries'] > 0 + ? $bench->accessProblemsFor( + $side->runBindDn, + $side->runBindPassword, + ) + : ['had nothing seeded to check access control against']; + $config = $this->buildConfig( $side, $bench, @@ -428,7 +486,10 @@ private function runSide( true, ); - return $snapshot; + return new SideRun( + $snapshot, + $accessProblems, + ); } private function seedSide( @@ -495,8 +556,8 @@ private function buildConfig( groupSize: $params['seedGroupSize'], ), directory: new DirectorySettings( - bindDn: $side->bindDn, - bindPassword: $side->bindPassword, + bindDn: $side->runBindDn, + bindPassword: $side->runBindPassword, baseDn: $bench->benchBaseDn, writeBase: $bench->writeBaseDn, ), @@ -535,13 +596,24 @@ private function sideFromOptions( InputInterface $input, string $prefix, ): BenchSide { + $bindDn = $this->requireString($input, "{$prefix}-bind-dn"); + $bindPassword = $this->requireString($input, "{$prefix}-bind-password"); + $runBindDn = $input->getOption("{$prefix}-run-bind-dn"); + $runBindDn = is_string($runBindDn) && $runBindDn !== '' + ? $runBindDn + : null; + return new BenchSide( label: $this->requireString($input, "{$prefix}-label"), host: $this->requireString($input, "{$prefix}-host"), port: $this->requireInt($input, "{$prefix}-port"), - bindDn: $this->requireString($input, "{$prefix}-bind-dn"), - bindPassword: $this->requireString($input, "{$prefix}-bind-password"), + bindDn: $bindDn, + bindPassword: $bindPassword, baseDn: $this->requireString($input, "{$prefix}-base-dn"), + runBindDn: $runBindDn ?? $bindDn, + runBindPassword: $runBindDn === null + ? $bindPassword + : $this->requireString($input, "{$prefix}-run-bind-password"), ); } diff --git a/tests/performance/Compare/BenchSide.php b/tests/performance/Compare/BenchSide.php index 5671a657..a22a20a8 100644 --- a/tests/performance/Compare/BenchSide.php +++ b/tests/performance/Compare/BenchSide.php @@ -14,7 +14,7 @@ namespace Tests\Performance\FreeDSx\Ldap\Compare; /** - * One server participating in a comparison run. + * One server participating in a comparison run, seeded as one identity and measured as another bound by its ACLs. */ final readonly class BenchSide { @@ -25,5 +25,7 @@ public function __construct( public string $bindDn, public string $bindPassword, public string $baseDn, + public string $runBindDn, + public string $runBindPassword, ) {} } diff --git a/tests/performance/Compare/ComparisonReport.php b/tests/performance/Compare/ComparisonReport.php index c5bf133d..c123b7eb 100644 --- a/tests/performance/Compare/ComparisonReport.php +++ b/tests/performance/Compare/ComparisonReport.php @@ -23,12 +23,18 @@ */ final class ComparisonReport { + /** + * @param list $targetAccessProblems What the target's access control got wrong for its run identity. + * @param list $sourceAccessProblems What the source's access control got wrong for its run identity. + */ public function __construct( private readonly ?StatsSnapshot $target, private readonly ?StatsSnapshot $source, private readonly string $format, private readonly string $targetLabel = 'Target', private readonly string $sourceLabel = 'FreeDSx', + private readonly array $targetAccessProblems = [], + private readonly array $sourceAccessProblems = [], ) {} public function render(OutputInterface $output): void @@ -40,6 +46,50 @@ public function render(OutputInterface $output): void } $this->renderText($output); + + foreach ($this->invalidReasons() as $reason) { + $output->writeln(sprintf( + 'INVALID: %s', + $reason, + )); + } + } + + /** + * @return list + */ + public function invalidReasons(): array + { + $reasons = []; + $sides = [ + [$this->targetLabel, $this->target, $this->targetAccessProblems], + [$this->sourceLabel, $this->source, $this->sourceAccessProblems], + ]; + + foreach ($sides as [$label, $snapshot, $accessProblems]) { + $barren = $snapshot?->opsReturningNothing(WorkloadMix::ENTRY_OPS) ?? []; + + if ($barren !== []) { + $reasons[] = sprintf( + '%s returned no entries for %s.', + $label, + implode( + ', ', + $barren, + ), + ); + } + + foreach ($accessProblems as $problem) { + $reasons[] = sprintf( + '%s %s.', + $label, + $problem, + ); + } + } + + return $reasons; } /** @@ -111,25 +161,6 @@ private function renderText(OutputInterface $output): void $this->sourceLabel, $this->source->elapsedSeconds, )); - - $this->warnBarrenSides($output); - } - - private function warnBarrenSides(OutputInterface $output): void - { - foreach ([[$this->targetLabel, $this->target], [$this->sourceLabel, $this->source]] as [$label, $snapshot]) { - $barren = $snapshot?->opsReturningNothing(WorkloadMix::ENTRY_OPS) ?? []; - - if ($barren === []) { - continue; - } - - $output->writeln(sprintf( - 'INVALID: %s returned no entries for %s; the comparison above is meaningless.', - $label, - implode(', ', $barren), - )); - } } /** @@ -174,6 +205,7 @@ private function renderJson(): string [ 'target' => $this->snapshotToArray($this->target), 'source' => $this->snapshotToArray($this->source), + 'invalid' => $this->invalidReasons(), ], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR, ); diff --git a/tests/performance/Compare/SideRun.php b/tests/performance/Compare/SideRun.php new file mode 100644 index 00000000..48dddb87 --- /dev/null +++ b/tests/performance/Compare/SideRun.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Performance\FreeDSx\Ldap\Compare; + +use Tests\Performance\FreeDSx\Ldap\Stats\StatsSnapshot; + +/** + * One side's measured run and anything its access control got wrong for the run identity. + */ +final readonly class SideRun +{ + /** + * @param list $accessProblems Empty when the run identity saw exactly what it should. + */ + public function __construct( + public StatsSnapshot $snapshot, + public array $accessProblems, + ) {} +} diff --git a/tests/performance/Compare/TargetBench.php b/tests/performance/Compare/TargetBench.php index 4784d12b..763f5f9e 100644 --- a/tests/performance/Compare/TargetBench.php +++ b/tests/performance/Compare/TargetBench.php @@ -22,6 +22,8 @@ use FreeDSx\Ldap\Operations; use FreeDSx\Ldap\Search\Filters; use RuntimeException; +use Tests\Support\FreeDSx\Ldap\Seed\PersonSeedEntries; +use Tests\Support\FreeDSx\Ldap\Seed\SeedDn; use Throwable; /** @@ -36,6 +38,11 @@ final class TargetBench */ private const CLEANUP_PAGE_SIZE = 500; + /** + * Seeded on every person but withheld from every run identity by each side's access control. + */ + private const WITHHELD_DESCRIPTION = 'userpassword'; + public readonly string $benchBaseDn; public readonly string $writeBaseDn; @@ -83,15 +90,10 @@ public function preflight(): void $this->bindIfNeeded(); } - public function seed( - int $seedEntries, - int $seedGroups = 0, - int $seedGroupSize = 0, - ): void { + public function ensureSubtree(): void + { $this->bindIfNeeded(); - $mailDomain = $this->mailDomain(); - $this->ensureOrganizationalUnit( $this->benchBaseDn, $this->benchOu, @@ -100,6 +102,16 @@ public function seed( $this->writeBaseDn, $this->peopleOu, ); + } + + public function seed( + int $seedEntries, + int $seedGroups = 0, + int $seedGroupSize = 0, + ): void { + $this->ensureSubtree(); + + $mailDomain = $this->mailDomain(); $this->ensurePerson( $this->compareDn(), @@ -109,19 +121,145 @@ public function seed( '1', ); - for ($i = 1; $i <= $seedEntries; $i++) { - $this->ensurePerson( - "cn=seed-{$i},{$this->writeBaseDn}", - "seed-{$i}", - 'Seed', - "seed-{$i}@{$mailDomain}", - (string) $i, - ); + foreach ($this->people()->people($seedEntries) as $person) { + $this->createOrIgnoreExists($person); } $this->ensureGroups($seedGroups, $seedGroupSize); } + /** + * @throws RuntimeException + */ + public function assertBindable( + string $bindDn, + string $bindPassword, + ): void { + $client = $this->buildClient(); + + try { + $client->bind( + $bindDn, + $bindPassword, + ); + $client->unbind(); + } catch (Throwable $e) { + throw new RuntimeException(sprintf( + 'Unable to bind to LDAP target at %s:%d as the run identity %s: %s', + $this->host, + $this->port, + $bindDn, + $e->getMessage(), + ), 0, $e); + } + } + + /** + * Fails when the identity the workload runs as may not add and delete an entry, which the write ops need. + * + * @throws RuntimeException + */ + public function assertWritableBy( + string $bindDn, + string $bindPassword, + ): void { + $probeDn = "cn=freedsx-bench-probe,{$this->writeBaseDn}"; + $this->removeIfPresent($probeDn); + $client = $this->buildClient(); + $probe = new Entry( + $probeDn, + new Attribute( + 'objectClass', + 'inetOrgPerson', + ), + new Attribute( + 'cn', + 'freedsx-bench-probe', + ), + new Attribute( + 'sn', + 'Probe', + ), + ); + + try { + $client->bind( + $bindDn, + $bindPassword, + ); + $client->create($probe); + $client->delete($probeDn); + $client->unbind(); + } catch (Throwable $e) { + throw new RuntimeException(sprintf( + 'The run identity %s may not write below %s on %s:%d: %s', + $bindDn, + $this->writeBaseDn, + $this->host, + $this->port, + $e->getMessage(), + ), 0, $e); + } + } + + /** + * What access control got wrong when an identity reads a seeded person's own attributes by name. + * + * @return list + * @throws OperationException + */ + public function accessProblemsFor( + string $bindDn, + string $bindPassword, + ): array { + $expected = $this->seededDescriptions(); + $client = $this->buildClient(); + $client->bind( + $bindDn, + $bindPassword, + ); + + try { + $person = $client->read( + "cn=seed-1,{$this->writeBaseDn}", + $expected, + ); + } finally { + $client->unbind(); + } + + $visible = array_map( + static fn(Attribute $attribute): string => strtolower($attribute->getDescription()), + $person?->getAttributes() ?? [], + ); + $problems = []; + + if (in_array(self::WITHHELD_DESCRIPTION, $visible, true)) { + $problems[] = sprintf( + 'returned %s to its run identity', + self::WITHHELD_DESCRIPTION, + ); + } + + $missing = array_diff( + $expected, + [self::WITHHELD_DESCRIPTION], + $visible, + ); + + if ($missing !== []) { + $problems[] = sprintf( + 'withheld %s from its run identity', + implode( + ', ', + $missing, + ), + ); + } + + return $problems; + } + public function cleanup(): void { // The seed connection may have gone idle and been dropped during the run, so reconnect for a reliable teardown. @@ -214,6 +352,49 @@ private function subtreeDns(): array return $dns; } + /** + * A probe left behind by an earlier run that kept its subtree would otherwise fail the next probe's add. + */ + private function removeIfPresent(string $dn): void + { + $this->bindIfNeeded(); + + try { + $this->client->delete($dn); + } catch (OperationException $e) { + if ($e->getCode() !== ResultCode::NO_SUCH_OBJECT) { + throw $e; + } + } + } + + private function people(): PersonSeedEntries + { + return new PersonSeedEntries( + dn: new SeedDn( + people: $this->writeBaseDn, + groups: "ou=groups,{$this->benchBaseDn}", + mailDomain: $this->mailDomain(), + ), + ); + } + + /** + * @return list + */ + private function seededDescriptions(): array + { + $descriptions = []; + + foreach ($this->people()->people(1) as $person) { + foreach ($person->getAttributes() as $attribute) { + $descriptions[] = strtolower($attribute->getDescription()); + } + } + + return $descriptions; + } + private function buildClient(): LdapClient { return new LdapClient( diff --git a/tests/performance/Workload/Worker.php b/tests/performance/Workload/Worker.php index a6e80972..16a6303c 100644 --- a/tests/performance/Workload/Worker.php +++ b/tests/performance/Workload/Worker.php @@ -14,6 +14,7 @@ namespace Tests\Performance\FreeDSx\Ldap\Workload; use FreeDSx\Ldap\ClientOptions; +use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; @@ -283,7 +284,10 @@ private function doSearchRead(LdapClient $client): int ->base($this->randomReadDn()) ->useBaseScope(); - return count($client->search($request)); + return $this->countEntries( + $client, + $request, + ); } private function doSearchEq(LdapClient $client): int @@ -292,7 +296,10 @@ private function doSearchEq(LdapClient $client): int ->base($this->config->directory->baseDn) ->useSubtreeScope(); - return count($client->search($request)); + return $this->countEntries( + $client, + $request, + ); } private function doSearchSub(LdapClient $client): int @@ -302,7 +309,10 @@ private function doSearchSub(LdapClient $client): int ->useSubtreeScope(); $this->applySearchSizeLimit($request); - return count($client->search($request)); + return $this->countEntries( + $client, + $request, + ); } private function doSearchList(LdapClient $client): int @@ -312,7 +322,37 @@ private function doSearchList(LdapClient $client): int ->useSingleLevelScope(); $this->applySearchSizeLimit($request); - return count($client->search($request)); + return $this->countEntries( + $client, + $request, + ); + } + + /** + * @throws OperationException + */ + private function countEntries( + LdapClient $client, + SearchRequest $request, + Control ...$controls, + ): int { + $delivered = 0; + $request->useEntryHandler(static function () use (&$delivered): void { + $delivered++; + }); + + try { + $client->search( + $request, + ...$controls, + ); + } catch (OperationException $e) { + if ($e->getCode() !== ResultCode::SIZE_LIMIT_EXCEEDED) { + throw $e; + } + } + + return $delivered; } /** @@ -338,11 +378,12 @@ private function doSearchSubstr(LdapClient $client): int ? Filters::contains('cn', (string) mt_rand(100, $this->config->seed->entries)) : Filters::contains('cn', 'eed'); - return count($client->search( + return $this->countEntries( + $client, $this->newSearch($filter) ->base($this->config->directory->baseDn) ->useSubtreeScope(), - )); + ); } private function doSearchSuffix(LdapClient $client): int @@ -351,11 +392,12 @@ private function doSearchSuffix(LdapClient $client): int ? Filters::endsWith('cn', "d-{$this->randomSeedIdx()}") : Filters::endsWith('cn', 'e'); - return count($client->search( + return $this->countEntries( + $client, $this->newSearch($filter) ->base($this->config->directory->baseDn) ->useSubtreeScope(), - )); + ); } private function doSearchRange(LdapClient $client): int @@ -364,11 +406,12 @@ private function doSearchRange(LdapClient $client): int ? 1000 + max(1, $this->config->seed->entries - 99) : 1000; - return count($client->search( + return $this->countEntries( + $client, $this->newSearch(Filters::greaterThanOrEqual('uidNumber', (string) $threshold)) ->base($this->config->directory->baseDn) ->useSubtreeScope(), - )); + ); } /** @@ -381,11 +424,12 @@ private function doSearchAnd(LdapClient $client): int $this->randomEqualityFilter(), ); - return count($client->search( + return $this->countEntries( + $client, $this->newSearch($filter) ->base($this->config->directory->baseDn) ->useSubtreeScope(), - )); + ); } /** @@ -398,11 +442,12 @@ private function doSearchOr(LdapClient $client): int $this->randomEqualityFilter(), ); - return count($client->search( + return $this->countEntries( + $client, $this->newSearch($filter) ->base($this->config->directory->baseDn) ->useSubtreeScope(), - )); + ); } /** @@ -418,10 +463,11 @@ private function doSearchSort(LdapClient $client): int $request->sizeLimit($this->config->search->sortSizeLimit); } - return count($client->search( + return $this->countEntries( + $client, $request, Controls::sort('sn'), - )); + ); } /** diff --git a/tests/profile/compare-ldap.sh b/tests/profile/compare-ldap.sh index 7b971836..7b637fc8 100755 --- a/tests/profile/compare-ldap.sh +++ b/tests/profile/compare-ldap.sh @@ -14,6 +14,9 @@ # --jit (function, tracing, or off) is the same for every runner, so a runner comparison is not also a JIT one. # --search-value is the cn prefix the subtree searches filter on (--search-value=seed- matches all). # +# Every side seeds as its root identity but runs the workload as a non-root bench administrator held to its own access +# control: a group with full access and userPassword withheld from everyone, mirroring FreeDSx's secure default. +# # Examples: # # # FreeDSx (sqlite) vs OpenDJ at 25k, with search-sub kept selective on OpenDJ's substring index @@ -82,32 +85,115 @@ case "$JIT" in *) echo "unknown --jit value: $JIT (expected off, function, or tracing)" >&2; exit 2 ;; esac -# Resolve a service key to its compose service, container, internal LDAP port, admin bind, base DN, display -# label, and per-service setup hook. Sets ${prefix}_SVC / _CONT / _PORT / _BIND / _PW / _BASE / _LABEL / _SETUP. +# The non-root identity a reference side runs the workload as, created by its setup hook. +BENCH_RUN_PW="P@ssword12345" +# Pre-encoded {SSHA} of BENCH_RUN_PW: every side verifies binds with a hash as cheap as FreeDSx's {SHA} admin. +BENCH_RUN_PW_HASH="{SSHA}sSdiTIT0MbdlzPesoNF+lJtOcJRmZHhiZW5jaA==" +# Setup hook output, kept for when a run identity check fails. +SETUP_LOG="tests/profile/out/compare-setup.log" + +# Resolve a service key to its compose service, container, internal LDAP port, root bind, run bind, base DN, display +# label, and per-service setup hook. Sets ${prefix}_SVC / _CONT / _PORT / _BIND / _PW / _RUN_BIND / _RUN_PW / _BASE / +# _LABEL / _SETUP. resolve_svc() { local key="$1" p="$2" case "$key" in freedsx) - # Seeding and the write ops need an administrator, matching the root identity the other two sides bind as. + # The admin is a member of cn=admins and held to the rules, unlike the privileged manager. declare -g "${p}_SVC=freedsx-server" "${p}_CONT=freedsx-profile-server" "${p}_PORT=10389" \ "${p}_BIND=cn=admin,dc=foo,dc=bar" "${p}_PW=12345" "${p}_BASE=dc=foo,dc=bar" \ + "${p}_RUN_BIND=cn=admin,dc=foo,dc=bar" "${p}_RUN_PW=12345" \ "${p}_LABEL=FreeDSx/$STORAGE" "${p}_SETUP=none" ;; openldap) declare -g "${p}_SVC=openldap" "${p}_CONT=freedsx-profile-openldap" "${p}_PORT=389" \ "${p}_BIND=cn=admin,dc=example,dc=com" "${p}_PW=P@ssword12345" "${p}_BASE=dc=example,dc=com" \ - "${p}_LABEL=OpenLDAP" "${p}_SETUP=none" ;; + "${p}_RUN_BIND=cn=bench-admin,dc=example,dc=com" "${p}_RUN_PW=$BENCH_RUN_PW" \ + "${p}_LABEL=OpenLDAP" "${p}_SETUP=openldap" ;; opendj) declare -g "${p}_SVC=opendj" "${p}_CONT=freedsx-profile-opendj" "${p}_PORT=1389" \ "${p}_BIND=cn=Directory Manager" "${p}_PW=P@ssword12345" "${p}_BASE=dc=example,dc=com" \ + "${p}_RUN_BIND=cn=bench-admin,dc=example,dc=com" "${p}_RUN_PW=$BENCH_RUN_PW" \ "${p}_LABEL=OpenDJ" "${p}_SETUP=opendj" ;; 389ds) declare -g "${p}_SVC=389ds" "${p}_CONT=freedsx-profile-389ds" "${p}_PORT=3389" \ "${p}_BIND=cn=Directory Manager" "${p}_PW=P@ssword12345" "${p}_BASE=dc=example,dc=com" \ + "${p}_RUN_BIND=cn=bench-admin,dc=example,dc=com" "${p}_RUN_PW=$BENCH_RUN_PW" \ "${p}_LABEL=389DS" "${p}_SETUP=389ds" ;; *) echo "unknown service: $key (expected freedsx, openldap, opendj, or 389ds)" >&2; exit 2 ;; esac } +# The bench administrator and its group, its password and limits replaced on every replay of a reused container. +bench_identity_ldif() { + local base="$1" + cat <>"$SETUP_LOG" + "$@" >>"$SETUP_LOG" 2>&1 || true +} + +# slapd.conf already grants the group write and lifts its limits, while withholding userPassword. +setup_openldap() { + local bind="$1" pw="$2" base="$3" + echo "==> adding the bench administrator to openldap" + bench_identity_ldif "$base" \ + | load_ldif openldap docker exec -i freedsx-profile-openldap ldapmodify -x -c -H ldap://localhost -D "$bind" -w "$pw" +} + if [[ "$SOURCE" == "$TARGET" ]]; then echo "--source and --target must differ (got '$SOURCE' for both)" >&2 exit 2 @@ -148,6 +234,17 @@ setup_opendj() { --backend-name userRoot --index-name objectClass --set index-entry-limit:200000 \ -h localhost -p 4444 -D "$bind" -w "$pw" -X -n >/dev/null 2>&1 \ || echo " (index-limit tweak skipped)" + + # The seeded people carry {SSHA} values, which the default policy refuses as pre-encoded. + docker exec freedsx-profile-opendj /opt/opendj/bin/dsconfig set-password-policy-prop \ + --policy-name "Default Password Policy" --set allow-pre-encoded-passwords:true \ + -h localhost -p 4444 -D "$bind" -w "$pw" -X -n >/dev/null 2>&1 \ + || echo " (pre-encoded password tweak skipped)" + + echo "==> adding the bench administrator and its ACIs to opendj" + { bench_identity_ldif "$base"; bench_unlimited_ldif "$base" opendj; bench_aci_ldif "$base"; } \ + | load_ldif opendj docker exec -i freedsx-profile-opendj /opt/opendj/bin/ldapmodify -c \ + -h localhost -p 1389 -D "$bind" -w "$pw" } # The image creates the instance but not the suffix, so create the backend, silence the per-op access log, and raise @@ -182,6 +279,10 @@ setup_389ds() { docker exec freedsx-profile-389ds dsconf -D "$bind" -w "$pw" "$uri" backend config set \ --idlistscanlimit 200000 --lookthroughlimit 200000 >/dev/null 2>&1 \ || echo " (scan-limit tweak skipped)" + # A bind otherwise rewrites the bench administrator's {SSHA} as PBKDF2, which every later bind then pays for. + docker exec freedsx-profile-389ds dsconf -D "$bind" -w "$pw" "$uri" \ + config replace nsslapd-enable-upgrade-hash=off >/dev/null 2>&1 \ + || echo " (hash-upgrade tweak skipped)" # A plugin only loads at startup, and the bench seeds after this returns... if [[ "$needs_restart" -eq 1 ]]; then @@ -189,6 +290,10 @@ setup_389ds() { docker restart freedsx-profile-389ds >/dev/null await_389ds "$bind" "$pw" "$uri" fi + + echo "==> adding the bench administrator and its ACIs to 389ds" + { bench_identity_ldif "$base"; bench_unlimited_ldif "$base" 389ds; bench_aci_ldif "$base"; } \ + | load_ldif 389ds docker exec -i freedsx-profile-389ds ldapmodify -x -c -H "$uri" -D "$bind" -w "$pw" } await_389ds() { @@ -206,33 +311,49 @@ await_389ds() { run_setup() { case "$1" in - opendj) setup_opendj "${@:2}" ;; - 389ds) setup_389ds "${@:2}" ;; + openldap) setup_openldap "${@:2}" ;; + opendj) setup_opendj "${@:2}" ;; + 389ds) setup_389ds "${@:2}" ;; esac } +# The OpenLDAP image bakes in its bootstrap, whose access rules a stale image would not have. +if [[ "$SOURCE" == "openldap" || "$TARGET" == "openldap" ]]; then + docker compose -f "$COMPOSE" build openldap +fi + echo "==> up $SRC_LABEL ($SRC_SVC) + $TGT_LABEL ($TGT_SVC) (cpuset=$SERVER_CPUSET each, driver on $DRIVER_CPUSET, seed=$SEED)" # Sharing cores with the driver can reverse which server looks faster, so each side is pinned to its own. PROFILE_CPUS="$CPUS" FREEDSX_STORAGE="$STORAGE" FREEDSX_RUNNER="$RUNNER" SEED_ENTRIES=0 \ SERVER_CPUSET="$SERVER_CPUSET" SWOOLE_WORKERS="$WORKERS" FREEDSX_JIT="$JIT" \ docker compose -f "$COMPOSE" up -d --wait "$SRC_SVC" "$TGT_SVC" +mkdir -p "$(dirname "$SETUP_LOG")" +: >"$SETUP_LOG" run_setup "$SRC_SETUP" "$SRC_BIND" "$SRC_PW" "$SRC_BASE" run_setup "$TGT_SETUP" "$TGT_BIND" "$TGT_PW" "$TGT_BASE" +echo "==> setup loader output is in $SETUP_LOG" NET=$(docker inspect -f '{{range $k,$_ := .NetworkSettings.Networks}}{{$k}}{{end}}' "$SRC_CONT") echo "==> running comparison (driver container on '$NET')" +# An invalid or failed comparison still tears the stack down, then exits with the driver's status. +STATUS=0 docker run --rm --network "$NET" --cpuset-cpus="$DRIVER_CPUSET" -v "$PWD":/app -w /app --entrypoint php \ freedsx-profile:latest -d xdebug.mode=off -d opcache.enable_cli=1 -d opcache.jit=off \ tests/bin/ldap-bench-compare.php \ --source-host="$SRC_SVC" --source-port="$SRC_PORT" --source-bind-dn="$SRC_BIND" --source-bind-password="$SRC_PW" --source-base-dn="$SRC_BASE" --source-label="$SRC_LABEL" \ + --source-run-bind-dn="$SRC_RUN_BIND" --source-run-bind-password="$SRC_RUN_PW" \ --target-host="$TGT_SVC" --target-port="$TGT_PORT" --target-bind-dn="$TGT_BIND" --target-bind-password="$TGT_PW" --target-base-dn="$TGT_BASE" --target-label="$TGT_LABEL" \ + --target-run-bind-dn="$TGT_RUN_BIND" --target-run-bind-password="$TGT_RUN_PW" \ --seed-entries="$SEED" --seed-groups="$SEED_GROUPS" --seed-group-size="$SEED_GROUP_SIZE" \ --mix="$MIX" --search-value="$SEARCHVAL" --duration="$DURATION" --warmup="$WARMUP" \ - --clients="$CLIENTS" --driver-processes="$DRIVER_PROCS" + --clients="$CLIENTS" --driver-processes="$DRIVER_PROCS" \ +|| STATUS=$? if [[ "$KEEP_UP" -eq 0 ]]; then echo "==> tearing down (pass --keep-up to skip)" docker compose -f "$COMPOSE" down fi + +exit "$STATUS" diff --git a/tests/profile/openldap-bootstrap.sh b/tests/profile/openldap-bootstrap.sh index 5be50cb2..083f0c3c 100644 --- a/tests/profile/openldap-bootstrap.sh +++ b/tests/profile/openldap-bootstrap.sh @@ -36,6 +36,9 @@ rootpw ${ROOTPW} directory /var/lib/ldap maxsize 1073741824 +# The bench administrator runs as free of limits as the rootdn and the FreeDSx side. +limits group/groupOfNames/member="cn=bench-admins,dc=example,dc=com" size=unlimited time=unlimited + index objectClass eq index cn,sn,uid,mail eq,sub index ou eq @@ -49,11 +52,13 @@ memberof-memberof-ad memberOf memberof-dangling ignore memberof-refint TRUE +# The bench runs as a member of bench-admins rather than the rootdn and is held to these rules like the other sides. access to attrs=userPassword by self write by anonymous auth by * none access to * + by group.exact="cn=bench-admins,dc=example,dc=com" write by self write by * read EOF diff --git a/tests/support/LdapBackendStorageCommand.php b/tests/support/LdapBackendStorageCommand.php index df23be3f..e322de4c 100644 --- a/tests/support/LdapBackendStorageCommand.php +++ b/tests/support/LdapBackendStorageCommand.php @@ -553,9 +553,10 @@ private function loadGroups( ): array { $members = []; $groups = []; + $seedDn = new SeedDn(); for ($i = 1; $i <= $size; $i++) { - $members[] = SeedDn::person($i); + $members[] = $seedDn->person($i); } for ($g = 0; $g < $count; $g++) { diff --git a/tests/support/Seed/MinimalSeedEntries.php b/tests/support/Seed/MinimalSeedEntries.php index 00071e2e..19eb11b8 100644 --- a/tests/support/Seed/MinimalSeedEntries.php +++ b/tests/support/Seed/MinimalSeedEntries.php @@ -24,13 +24,16 @@ { use FillerAttributesTrait; - public function __construct(private int $fillerAttributes = 0) {} + public function __construct( + private int $fillerAttributes = 0, + private SeedDn $dn = new SeedDn(), + ) {} public function generate(int $count): iterable { for ($i = 1; $i <= $count; $i++) { yield new Entry( - new Dn(SeedDn::person($i)), + new Dn($this->dn->person($i)), new Attribute( 'cn', "seed-{$i}", @@ -46,7 +49,7 @@ public function generate(int $count): iterable ), new Attribute( 'mail', - "seed-{$i}@foo.bar", + "seed-{$i}@{$this->dn->mailDomain}", ), new Attribute( 'uidNumber', diff --git a/tests/support/Seed/PersonSeedEntries.php b/tests/support/Seed/PersonSeedEntries.php index ca6fafb0..59b0c5f7 100644 --- a/tests/support/Seed/PersonSeedEntries.php +++ b/tests/support/Seed/PersonSeedEntries.php @@ -119,20 +119,21 @@ ['Raleigh', 'NC', '27601'], ]; - public function __construct(private int $fillerAttributes = 0) {} + public function __construct( + private int $fillerAttributes = 0, + private SeedDn $dn = new SeedDn(), + ) {} public function generate(int $count): iterable { - for ($i = 1; $i <= $count; $i++) { - yield $this->person($i); - } + yield from $this->people($count); if ($count === 0) { return; } yield new Entry( - new Dn(SeedDn::GROUPS), + new Dn($this->dn->groups), new Attribute( 'objectClass', 'organizationalUnit', @@ -152,6 +153,18 @@ public function isSchemaValid(): bool return true; } + /** + * Just the people, each seeded after the manager it names, without the groups that list them. + * + * @return iterable + */ + public function people(int $count): iterable + { + for ($i = 1; $i <= $count; $i++) { + yield $this->person($i); + } + } + private function person(int $i): Entry { $values = [ @@ -170,7 +183,7 @@ private function person(int $i): Entry } return new Entry( - new Dn(SeedDn::person($i)), + new Dn($this->dn->person($i)), ...$attributes, ...$this->fillers( $i, @@ -231,8 +244,8 @@ private function contact(int $i): array return [ 'mail' => [ - "seed-{$i}@foo.bar", - "{$alias}.{$i}@foo.bar", + "seed-{$i}@{$this->dn->mailDomain}", + "{$alias}.{$i}@{$this->dn->mailDomain}", ], 'telephoneNumber' => [ $this->phone('010', $i), @@ -248,7 +261,7 @@ private function contact(int $i): array 'postalAddress' => ["$street\$$city, $state $postalCode"], 'physicalDeliveryOfficeName' => ['Building ' . ($i % 12 + 1)], 'postOfficeBox' => ['PO Box ' . (1000 + $i % 9000)], - 'labeledURI' => ["https://foo.bar/~seed-{$i} Home page"], + 'labeledURI' => ["https://{$this->dn->mailDomain}/~seed-{$i} Home page"], ]; } @@ -261,7 +274,7 @@ private function relations(int $i): array return []; } - return ['seeAlso' => [SeedDn::person($this->manager($i))]]; + return ['seeAlso' => [$this->dn->person($this->manager($i))]]; } /** @@ -273,7 +286,7 @@ private function teams(int $count): iterable $members = []; for ($i = $first; $i <= min($first + self::TEAM_SIZE - 1, $count); $i++) { - $members[] = SeedDn::person($i); + $members[] = $this->dn->person($i); } $team = intdiv($first - 1, self::TEAM_SIZE) + 1; @@ -296,7 +309,7 @@ private function departments(int $count): iterable $members = []; for ($i = 1; $i <= $count; $i++) { - $members[$this->departmentIndex($i)][] = SeedDn::person($i); + $members[$this->departmentIndex($i)][] = $this->dn->person($i); } foreach ($members as $index => $departmentMembers) { @@ -317,7 +330,7 @@ private function group( array $members, ): Entry { return new Entry( - new Dn(SeedDn::group($cn)), + new Dn($this->dn->group($cn)), new Attribute( 'objectClass', 'groupOfNames', diff --git a/tests/support/Seed/SeedDn.php b/tests/support/Seed/SeedDn.php index d2fe5679..0f49b6a9 100644 --- a/tests/support/Seed/SeedDn.php +++ b/tests/support/Seed/SeedDn.php @@ -14,21 +14,29 @@ namespace Tests\Support\FreeDSx\Ldap\Seed; /** - * Where generated seed entries live, matching the DNs the load-test workloads address. + * Where generated seed entries live and the mail domain they use, defaulting to what the load-test workloads address. */ -final class SeedDn +final readonly class SeedDn { public const PEOPLE = 'ou=people,dc=foo,dc=bar'; public const GROUPS = 'ou=groups,dc=foo,dc=bar'; - public static function person(int $index): string + public const MAIL_DOMAIN = 'foo.bar'; + + public function __construct( + public string $people = self::PEOPLE, + public string $groups = self::GROUPS, + public string $mailDomain = self::MAIL_DOMAIN, + ) {} + + public function person(int $index): string { - return 'cn=seed-' . $index . ',' . self::PEOPLE; + return 'cn=seed-' . $index . ',' . $this->people; } - public static function group(string $cn): string + public function group(string $cn): string { - return 'cn=' . $cn . ',' . self::GROUPS; + return 'cn=' . $cn . ',' . $this->groups; } } diff --git a/tests/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php b/tests/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php new file mode 100644 index 00000000..4cad0308 --- /dev/null +++ b/tests/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php @@ -0,0 +1,332 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Unit\FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\Admission\SearchResultGate; +use FreeDSx\Ldap\Protocol\ServerProtocolHandler\AttributeProjection; +use FreeDSx\Ldap\Schema\Schema; +use FreeDSx\Ldap\Schema\SchemaResource; +use FreeDSx\Ldap\Search\Filter\FilterInterface; +use FreeDSx\Ldap\Search\Filters; +use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\ReloadableAccessControl; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeRule; +use FreeDSx\Ldap\Server\AccessControl\Rule\OperationRule; +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\Storage\Filter\FilterEvaluatorInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Filter\StrippedEntryMatcher; +use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; +use FreeDSx\Ldap\Server\Token\BindToken; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SearchResultGateTest extends TestCase +{ + private Schema $schema; + + private FilterEvaluatorInterface&MockObject $mockEvaluator; + + private StrippedEntryMatcher $strippedMatcher; + + private BindToken $token; + + protected function setUp(): void + { + $this->schema = SchemaResource::Core->load(); + $this->mockEvaluator = $this->createMock(FilterEvaluatorInterface::class); + $this->strippedMatcher = new StrippedEntryMatcher( + $this->mockEvaluator, + $this->schema, + new LinkedAttributes($this->schema), + ); + $this->token = BindToken::fromDn('cn=user,dc=foo,dc=bar'); + } + + public function test_an_entry_returned_whole_is_the_entry_itself(): void + { + $entry = $this->person('cn=a,dc=foo,dc=bar'); + + self::assertSame( + $entry, + $this->gate($this->allowing())->admit($entry), + ); + } + + public function test_an_entry_the_identity_may_not_see_is_not_returned(): void + { + $gate = $this->gate(new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow( + Subject::anyone(), + Target::subtree('ou=shown,dc=foo,dc=bar'), + )], + ))); + + self::assertNull($gate->admit($this->person('cn=a,ou=hidden,dc=foo,dc=bar'))); + self::assertNotNull($gate->admit($this->person('cn=a,ou=shown,dc=foo,dc=bar'))); + } + + public function test_a_withheld_attribute_the_filter_does_not_read_is_left_out_without_judging_the_entry_again(): void + { + $this->mockEvaluator + ->expects(self::never()) + ->method('evaluate'); + + $returned = $this->gate($this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'userPassword', + )->forRead()))->admit($this->person('cn=a,dc=foo,dc=bar')); + + self::assertSame( + ['cn', 'mail'], + $this->descriptions($returned), + ); + } + + public function test_an_entry_that_stops_matching_once_stripped_is_not_returned(): void + { + $this->mockEvaluator + ->expects(self::once()) + ->method('evaluate') + ->willReturn(false); + + $gate = $this->gate( + $this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'userPassword', + )->forRead()), + filter: Filters::present('userPassword'), + ); + + self::assertNull($gate->admit($this->person('cn=a,dc=foo,dc=bar'))); + } + + public function test_an_entry_that_still_matches_once_stripped_is_returned_without_what_was_withheld(): void + { + $this->mockEvaluator + ->expects(self::once()) + ->method('evaluate') + ->with(self::callback(static fn(Entry $stripped): bool => $stripped->get('userPassword') === null)) + ->willReturn(true); + + $gate = $this->gate( + $this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'userPassword', + )->forRead()), + filter: Filters::or( + Filters::present('userPassword'), + Filters::present('cn'), + ), + ); + + self::assertSame( + ['cn', 'mail'], + $this->descriptions($gate->admit($this->person('cn=a,dc=foo,dc=bar'))), + ); + } + + public function test_an_attribute_neither_returned_nor_read_by_the_filter_is_never_put_to_the_rules(): void + { + $consulted = 0; + $gate = $this->gate( + $this->allowing(AttributeRule::deny( + Subject::callback(static function () use (&$consulted): bool { + $consulted++; + + return false; + }), + Target::any(), + 'mail', + )->forRead()), + selected: [new Attribute('cn')], + ); + + $gate->admit($this->person('cn=a,dc=foo,dc=bar')); + $gate->admit($this->person('cn=b,dc=foo,dc=bar')); + + self::assertSame( + 0, + $consulted, + ); + } + + public function test_a_decision_only_the_entry_can_make_is_made_for_each_entry(): void + { + $gate = $this->gate($this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::subtree('ou=hidden,dc=foo,dc=bar'), + 'mail', + )->forRead())); + + self::assertSame( + ['cn', 'mail', 'userPassword'], + $this->descriptions($gate->admit($this->person('cn=a,ou=shown,dc=foo,dc=bar'))), + ); + self::assertSame( + ['cn', 'userPassword'], + $this->descriptions($gate->admit($this->person('cn=a,ou=hidden,dc=foo,dc=bar'))), + ); + } + + public function test_an_option_bearing_form_is_decided_by_its_type(): void + { + $returned = $this->gate($this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'mail', + )->forRead()))->admit(new Entry( + new Dn('cn=a,dc=foo,dc=bar'), + new Attribute( + 'cn', + 'a', + ), + new Attribute( + 'Mail;lang-en', + 'a@foo.bar', + ), + )); + + self::assertSame( + ['cn'], + $this->descriptions($returned), + ); + } + + public function test_a_types_only_request_returns_descriptions_without_values(): void + { + $returned = $this->gate( + $this->allowing(), + typesOnly: true, + )->admit($this->person('cn=a,dc=foo,dc=bar')); + + self::assertNotNull($returned); + + foreach ($returned->getAttributes() as $attribute) { + self::assertSame( + [], + $attribute->getValues(), + ); + } + } + + public function test_a_request_for_no_attributes_returns_only_the_dn(): void + { + $returned = $this->gate( + $this->allowing(), + selected: [new Attribute('1.1')], + )->admit($this->person('cn=a,dc=foo,dc=bar')); + + self::assertSame( + [], + $this->descriptions($returned), + ); + self::assertSame( + 'cn=a,dc=foo,dc=bar', + $returned?->getDn()->toString(), + ); + } + + public function test_a_reload_decides_the_entries_still_to_come(): void + { + $accessControl = new ReloadableAccessControl($this->allowing()); + $gate = $this->gate($accessControl); + + foreach (['cn=a,dc=foo,dc=bar', 'cn=b,dc=foo,dc=bar'] as $dn) { + self::assertSame( + ['cn', 'mail', 'userPassword'], + $this->descriptions($gate->admit($this->person($dn))), + ); + } + + $accessControl->replace($this->allowing(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'mail', + )->forRead())); + + self::assertSame( + ['cn', 'userPassword'], + $this->descriptions($gate->admit($this->person('cn=c,dc=foo,dc=bar'))), + ); + } + + /** + * @param list $selected + */ + private function gate( + AccessControlInterface $accessControl, + ?FilterInterface $filter = null, + array $selected = [], + bool $typesOnly = false, + ): SearchResultGate { + return SearchResultGate::forRequest( + $accessControl, + $this->strippedMatcher, + AttributeProjection::forRequest( + $selected, + $typesOnly, + $this->schema, + ), + $this->token, + $filter ?? Filters::present('cn'), + ); + } + + private function allowing(AttributeRule ...$rules): RuleBasedAccessControl + { + return new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow(Subject::anyone())], + attributes: $rules, + )); + } + + private function person(string $dn): Entry + { + return new Entry( + new Dn($dn), + new Attribute( + 'cn', + 'a', + ), + new Attribute( + 'mail', + 'a@foo.bar', + ), + new Attribute( + 'userPassword', + 'secret', + ), + ); + } + + /** + * @return list + */ + private function descriptions(?Entry $entry): array + { + return array_values(array_map( + static fn(Attribute $attribute): string => $attribute->getDescription(), + $entry?->getAttributes() ?? [], + )); + } +} diff --git a/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php b/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php index a206f1e6..5c021787 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php @@ -256,6 +256,72 @@ public function test_types_only_drops_values_but_keeps_attribute_names(): void ); } + public function test_the_entry_itself_is_returned_when_every_attribute_is_selected(): void + { + $projection = AttributeProjection::forRequest( + [ + new Attribute('*'), + new Attribute('+'), + ], + false, + $this->schema, + ); + + self::assertSame( + $this->entry, + $projection->project($this->entry), + ); + } + + public function test_a_requested_range_slice_selects_the_values_held_under_its_type(): void + { + $entry = new Entry( + new Dn('cn=staff,dc=example,dc=com'), + new Attribute('cn', 'staff'), + new Attribute('member', 'cn=Alice,dc=example,dc=com'), + ); + $projection = AttributeProjection::forRequest( + [new Attribute('member;range=0-*')], + false, + $this->schema, + ); + + self::assertSame( + ['member'], + $this->attributeNames($projection->project($entry)), + ); + } + + public function test_none_selector_includes_no_attribute(): void + { + $projection = AttributeProjection::forRequest( + [new Attribute('1.1')], + false, + $this->schema, + ); + + foreach ($this->entry->getAttributes() as $attribute) { + self::assertFalse($projection->includes($attribute)); + } + } + + public function test_none_selector_named_with_a_wildcard_is_ignored(): void + { + $projection = AttributeProjection::forRequest( + [ + new Attribute('1.1'), + new Attribute('*'), + ], + false, + $this->schema, + ); + + self::assertEqualsCanonicalizing( + ['cn', 'sn', 'userPassword'], + $this->attributeNames($projection->project($this->entry)), + ); + } + public function test_repeated_projection_returns_consistent_results(): void { $projection = AttributeProjection::forRequest( diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerCompareHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerCompareHandlerTest.php index 50328722..4934c2b1 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerCompareHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerCompareHandlerTest.php @@ -28,6 +28,7 @@ use FreeDSx\Ldap\Search\Filter\EqualityFilter; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\UnrestrictedReadScope; use FreeDSx\Ldap\Server\AccessControl\WithheldAttributePolicy; use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\NoLinkedValues; @@ -152,8 +153,8 @@ public function test_a_failing_assertion_refuses_the_compare_before_it_is_evalua ['foo' => ['bar']], )); $this->mockAccessControl - ->method('stripUnreadableAttributes') - ->willReturnArgument(1); + ->method('readScope') + ->willReturn(new UnrestrictedReadScope()); $this->mockFilterEvaluator ->method('evaluate') ->willReturn(false); @@ -212,8 +213,8 @@ public function test_a_failing_assertion_refuses_a_compare_on_a_withheld_attribu ], )); $this->mockAccessControl - ->method('stripUnreadableAttributes') - ->willReturnArgument(1); + ->method('readScope') + ->willReturn(new UnrestrictedReadScope()); $this->mockFilterEvaluator ->method('evaluate') ->willReturn(false); diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php index 8e288148..b9f70931 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerDispatchHandlerTest.php @@ -39,6 +39,7 @@ use FreeDSx\Ldap\Server\Backend\ReadBackendInterface; use FreeDSx\Ldap\Server\Backend\Write\WriteHandlerInterface; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\UnrestrictedReadScope; use FreeDSx\Ldap\Server\Backend\Write\Routing\WriteRequestRouter; use FreeDSx\Ldap\Server\Backend\Write\WriteRequestInterface; use FreeDSx\Ldap\Server\Operation\OperationOutcome; @@ -94,8 +95,8 @@ public function test_a_post_read_is_built_from_the_entry_the_write_kept(): void ->expects(self::never()) ->method('get'); $this->mockAccessControl - ->method('filterEntry') - ->willReturnArgument(1); + ->method('readScope') + ->willReturn(new UnrestrictedReadScope()); $this->mockWriteHandler ->method('handle') ->willReturnCallback(static function (WriteRequestInterface $request, WriteContext $context): void { diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php index 797dfa72..16850490 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php @@ -37,6 +37,13 @@ use FreeDSx\Ldap\Schema\SchemaResource; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\Read\UnrestrictedReadScope; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeRule; +use FreeDSx\Ldap\Server\AccessControl\Rule\OperationRule; +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\Backend\Storage\EntryStream; use FreeDSx\Ldap\Server\Backend\Storage\FetchedBatch; @@ -97,8 +104,11 @@ protected function setUp(): void ->willReturn(true); $this->mockAccessControl - ->method('filterEntry') - ->willReturnArgument(1); + ->method('readScope') + ->willReturn(new UnrestrictedReadScope()); + $this->mockAccessControl + ->method('isEntryVisible') + ->willReturn(true); $this->mockQueue ->method('sendMessages') @@ -122,15 +132,6 @@ public function test_a_page_is_filled_past_entries_access_control_hides(): void $hidden = Entry::create('cn=1,dc=foo,dc=bar', ['cn' => '1']); $visible = Entry::create('cn=2,dc=foo,dc=bar', ['cn' => '2']); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback( - static fn(TokenInterface $token, Entry $entry): ?Entry => $entry === $hidden - ? null - : $entry, - ); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware( @@ -138,7 +139,7 @@ public function test_a_page_is_filled_past_entries_access_control_hides(): void $visible, )); - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($this->hiding($hidden)); // A page of one, whose first candidate is hidden, so filling it means reading on rather than answering short. $this->drive( @@ -159,26 +160,16 @@ public function test_a_page_filled_part_way_through_a_read_resumes_from_the_last $entries[] = Entry::create("cn=$i,dc=foo,dc=bar", ['cn' => (string) $i]); } - // Hiding the first two starves the opening read, so the next one is widened and overshoots the page. - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback( - static fn(TokenInterface $token, Entry $entry): ?Entry => in_array( - $entry, - [$entries[0], $entries[1]], - true, - ) - ? null - : $entry, - ); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware(...$entries)); + // Hiding the first two starves the opening read, so the next one is widened and overshoots the page. $subject = $this->makeHandler( - $mockAccessControl, + $this->hiding( + $entries[0], + $entries[1], + ), new SearchLimits(maxSearchPageSize: 10), ); @@ -221,22 +212,13 @@ public function test_a_page_filled_before_the_lookthrough_runs_out_is_answered_r $entries[] = Entry::create("cn=$i,dc=foo,dc=bar", ['cn' => (string) $i]); } - // Only the tenth candidate is visible, so filling the page spends the whole lookthrough. - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback( - static fn(TokenInterface $token, Entry $entry): ?Entry => $entry === $entries[9] - ? $entry - : null, - ); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware(...$entries)); + // Only the tenth candidate is visible, so filling the page spends the whole lookthrough. $subject = $this->makeHandler( - $mockAccessControl, + $this->showingOnly($entries[9]), new SearchLimits( maxSearchPageSize: 1000, maxSearchPagedLookthrough: 10, @@ -270,22 +252,13 @@ public function test_a_page_still_short_of_its_limit_is_refused_when_the_lookthr $entries[] = Entry::create("cn=$i,dc=foo,dc=bar", ['cn' => (string) $i]); } - // The page wants two, but the only visible entry is the tenth candidate. - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback( - static fn(TokenInterface $token, Entry $entry): ?Entry => $entry === $entries[9] - ? $entry - : null, - ); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware(...$entries)); + // The page wants two, but the only visible entry is the tenth candidate. $subject = $this->makeHandler( - $mockAccessControl, + $this->showingOnly($entries[9]), new SearchLimits( maxSearchPageSize: 1000, maxSearchPagedLookthrough: 10, @@ -319,26 +292,16 @@ public function test_a_size_limit_probe_that_runs_out_of_lookthrough_is_refused( $entries[] = Entry::create("cn=$i,dc=foo,dc=bar", ['cn' => (string) $i]); } - // The first two fill the page, so only the probe for a further match meets the hidden rest. - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback( - static fn(TokenInterface $token, Entry $entry): ?Entry => in_array( - $entry, - [$entries[0], $entries[1]], - true, - ) - ? $entry - : null, - ); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware(...$entries)); + // The first two fill the page, so only the probe for a further match meets the hidden rest. $subject = $this->makeHandler( - $mockAccessControl, + $this->showingOnly( + $entries[0], + $entries[1], + ), new SearchLimits(maxSearchPagedLookthrough: 3), ); @@ -655,13 +618,8 @@ public function test_every_slice_of_one_page_carries_the_same_deadline(): void }); // Access control hides every entry, so the page is filled from slice after slice until the result runs out. - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturn(null); - $subject = $this->makeHandler( - $mockAccessControl, + $this->showingOnly(), new SearchLimits(maxSearchPageSize: 2), ); $this->drive($subject, $this->makeSearchMessage(size: 2, searchRequest: $searchRequest)); @@ -943,7 +901,7 @@ public function test_client_page_size_is_honoured_when_below_server_max(): void self::assertNotSame('', $this->donePagingControl()->getCookie()); } - public function test_it_should_suppress_entry_when_filter_entry_returns_null(): void + public function test_it_should_suppress_an_entry_the_identity_may_not_see(): void { $entry1 = Entry::create( 'cn=1,dc=foo,dc=bar', @@ -955,13 +913,6 @@ public function test_it_should_suppress_entry_when_filter_entry_returns_null(): ); $message = $this->makeSearchMessage(size: 10); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback(static function (TokenInterface $token, Entry $entry) use ($entry1): ?Entry { - return $entry === $entry1 ? null : $entry; - }); - $this->mockBackend ->method('search') ->willReturnCallback($this->sliceAware( @@ -969,7 +920,7 @@ public function test_it_should_suppress_entry_when_filter_entry_returns_null(): $entry2, )); - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($this->hiding($entry1)); $this->drive($subject, $message); self::assertEquals( @@ -984,16 +935,10 @@ public function test_it_should_skip_entry_when_filter_no_longer_matches_after_ac 'cn=1,dc=foo,dc=bar', ['cn' => '1', 'secret' => 'val'], ); - $stripped = Entry::create( - 'cn=1,dc=foo,dc=bar', - ['cn' => '1'], + $message = $this->makeSearchMessage( + size: 10, + searchRequest: $this->makeSearchRequest('(secret=val)'), ); - $message = $this->makeSearchMessage(size: 10); - - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturn($stripped); $mockFilterEvaluator = $this->createMock(FilterEvaluatorInterface::class); $mockFilterEvaluator @@ -1005,7 +950,14 @@ public function test_it_should_skip_entry_when_filter_no_longer_matches_after_ac ->willReturnCallback($this->sliceAware($entry)); $subject = $this->makeHandler( - $mockAccessControl, + new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow(Subject::anyone())], + attributes: [AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'secret', + )->forRead()], + )), filterEvaluator: $mockFilterEvaluator, ); $this->drive($subject, $message); @@ -1252,12 +1204,7 @@ public function test_matched_dn_is_dropped_when_access_control_hides_ancestor_on ->method('get') ->willReturn($matchedEntry); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturn(null); - - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($this->showingOnly()); $this->drive( $subject, @@ -1295,6 +1242,41 @@ private function drive( ); } + /** + * Rules that let anyone search every entry but the given ones. + */ + private function hiding(Entry ...$hidden): AccessControlInterface + { + return new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [ + ...array_map( + static fn(Entry $entry): OperationRule => OperationRule::deny( + Subject::anyone(), + Target::dn($entry->getDn()->toString()), + ), + $hidden, + ), + OperationRule::allow(Subject::anyone()), + ], + )); + } + + /** + * Rules that let anyone search just the given entries, and no entry at all when none is given. + */ + private function showingOnly(Entry ...$shown): AccessControlInterface + { + return new RuleBasedAccessControl(AclRules::fromEmpty( + operations: array_map( + static fn(Entry $entry): OperationRule => OperationRule::allow( + Subject::anyone(), + Target::dn($entry->getDn()->toString()), + ), + $shown, + ), + )); + } + private function makeHandler( ?AccessControlInterface $accessControl = null, ?SearchLimits $limits = null, diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php index 7d071a40..599d850c 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php @@ -41,6 +41,13 @@ use FreeDSx\Ldap\Schema\SchemaResource; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\Read\UnrestrictedReadScope; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeRule; +use FreeDSx\Ldap\Server\AccessControl\Rule\OperationRule; +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\Backend\Storage\EntryStream; use FreeDSx\Ldap\Server\Backend\Storage\Filter\FilterEvaluatorInterface; @@ -88,8 +95,8 @@ protected function setUp(): void $this->sentMessages = []; $this->mockAccessControl - ->method('filterEntry') - ->willReturnArgument(1); + ->method('readScope') + ->willReturn(new UnrestrictedReadScope()); $this->mockQueue ->method('sendMessages') @@ -275,17 +282,13 @@ public function test_it_accepts_every_scope_it_supports(): void public function test_entry_stripped_by_acl_is_excluded_when_it_no_longer_matches_filter(): void { $entry = Entry::create('dc=foo,dc=bar', ['userPassword' => 'secret', 'cn' => 'foo']); - $stripped = Entry::create('dc=foo,dc=bar', ['cn' => 'foo']); $search = new LdapMessageRequest( 2, (new SearchRequest(Filters::present('userPassword')))->base('dc=foo,dc=bar'), ); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl->method('filterEntry')->willReturn($stripped); - - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($this->withholding('userPassword')); $this->mockBackend ->method('search') @@ -547,7 +550,7 @@ public function test_it_should_send_a_successful_SearchResultDone_when_no_entrie ]); } - public function test_it_should_suppress_entry_when_filter_entry_returns_null(): void + public function test_it_should_suppress_an_entry_the_identity_may_not_see(): void { $entry1 = Entry::create( 'dc=foo,dc=bar', @@ -563,12 +566,15 @@ public function test_it_should_suppress_entry_when_filter_entry_returns_null(): (new SearchRequest(Filters::equal('foo', 'bar')))->base('dc=foo,dc=bar'), ); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturnCallback(static function (TokenInterface $token, Entry $entry) use ($entry1): ?Entry { - return $entry === $entry1 ? null : $entry; - }); + $accessControl = new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [ + OperationRule::deny( + Subject::anyone(), + Target::dn('dc=foo,dc=bar'), + ), + OperationRule::allow(Subject::anyone()), + ], + )); $this->mockBackend ->method('search') @@ -581,7 +587,7 @@ public function test_it_should_suppress_entry_when_filter_entry_returns_null(): ->method('evaluate') ->willReturn(true); - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($accessControl); $this->drive($subject, $search); $this->assertSentMessages([ @@ -599,20 +605,13 @@ public function test_it_should_skip_entry_when_filter_no_longer_matches_after_ac 'dc=foo,dc=bar', ['cn' => 'foo', 'secret' => 'val'], ); - $stripped = Entry::create( - 'dc=foo,dc=bar', - ['cn' => 'foo'], - ); $search = new LdapMessageRequest( 2, (new SearchRequest(Filters::equal('secret', 'val')))->base('dc=foo,dc=bar'), ); - $mockAccessControl = $this->createMock(AccessControlInterface::class); - $mockAccessControl - ->method('filterEntry') - ->willReturn($stripped); + $accessControl = $this->withholding('secret'); $this->mockBackend ->method('search') @@ -622,7 +621,7 @@ public function test_it_should_skip_entry_when_filter_no_longer_matches_after_ac ->method('evaluate') ->willReturnCallback(static fn(Entry $e): bool => $e === $entry); - $subject = $this->makeHandler($mockAccessControl); + $subject = $this->makeHandler($accessControl); $this->drive($subject, $search); $this->assertSentMessages([ @@ -1191,6 +1190,21 @@ private static function searchRequest(): SearchRequest ->base('dc=foo,dc=bar'); } + /** + * Rules that let anyone search but withhold the named attribute from every result. + */ + private function withholding(string $attribute): AccessControlInterface + { + return new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow(Subject::anyone())], + attributes: [AttributeRule::deny( + Subject::anyone(), + Target::any(), + $attribute, + )->forRead()], + )); + } + private function makeHandler( AccessControlInterface $accessControl, ?SearchLimits $limits = null, diff --git a/tests/unit/Schema/SchemaTest.php b/tests/unit/Schema/SchemaTest.php index aaf9640f..40962111 100644 --- a/tests/unit/Schema/SchemaTest.php +++ b/tests/unit/Schema/SchemaTest.php @@ -14,6 +14,7 @@ namespace Tests\Unit\FreeDSx\Ldap\Schema; use FreeDSx\Ldap\Schema\Definition\AttributeType; +use FreeDSx\Ldap\Schema\Definition\AttributeUsage; use FreeDSx\Ldap\Schema\Definition\LdapSyntax; use FreeDSx\Ldap\Schema\Definition\MatchingRule; use FreeDSx\Ldap\Schema\Definition\MatchingRuleOid; @@ -750,6 +751,89 @@ public function test_description_subtyping( ); } + public function test_a_subtype_answer_follows_a_type_redefined_after_it_was_asked(): void + { + $this->subject->addAttributeType(new AttributeType( + oid: '2.5.4.41', + names: ['name'], + )); + $this->subject->addAttributeType(new AttributeType( + oid: '2.5.4.3', + names: ['cn'], + )); + + self::assertFalse($this->subject->isDescriptionSubtypeOf( + 'cn', + 'name', + )); + + $this->subject->addAttributeType(new AttributeType( + oid: '2.5.4.3', + names: ['cn'], + superTypeOid: '2.5.4.41', + )); + + self::assertTrue($this->subject->isDescriptionSubtypeOf( + 'cn', + 'name', + )); + } + + public function test_a_remembered_subtype_answer_is_never_given_to_a_different_pair(): void + { + $this->subject->addAttributeType($this->cn); + $this->subject->addAttributeType(new AttributeType( + oid: '2.5.4.4', + names: ['sn'], + )); + + self::assertTrue($this->subject->isDescriptionSubtypeOf( + "sn;p\0cn;q", + 'sn', + )); + self::assertFalse($this->subject->isDescriptionSubtypeOf( + 'sn;p', + "cn;q\0sn", + )); + } + + public function test_an_operational_type_is_recognized_however_its_description_is_spelled(): void + { + $this->subject->addAttributeType(new AttributeType( + oid: '1.3.6.1.1.16.4', + names: ['entryUUID'], + usage: AttributeUsage::DirectoryOperation, + )); + $this->subject->addAttributeType($this->cn); + + foreach (['entryUUID', 'ENTRYUUID', 'entryUUID;x-tag', '1.3.6.1.1.16.4'] as $description) { + self::assertTrue( + $this->subject->isOperational($description), + $description, + ); + } + self::assertFalse($this->subject->isOperational('cn')); + self::assertFalse($this->subject->isOperational('unknownType')); + } + + public function test_a_type_redefined_as_operational_is_recognized_once_it_is_added(): void + { + $this->subject->addAttributeType(new AttributeType( + oid: '1.2.3.4', + names: ['tracked'], + )); + + self::assertFalse($this->subject->isOperational('tracked')); + + $this->subject->addAttributeType(new AttributeType( + oid: '1.2.3.4', + names: ['tracked'], + usage: AttributeUsage::DsaOperation, + )); + + self::assertTrue($this->subject->isOperational('tracked')); + } + public function test_a_standard_ordering_rule_is_an_ordering_rule_when_no_type_names_it(): void { $this->subject->addMatchingRule(new MatchingRule( diff --git a/tests/unit/Server/AccessControl/AclRuleIndexTest.php b/tests/unit/Server/AccessControl/AclRuleIndexTest.php index ae4712f7..fb37ae59 100644 --- a/tests/unit/Server/AccessControl/AclRuleIndexTest.php +++ b/tests/unit/Server/AccessControl/AclRuleIndexTest.php @@ -174,13 +174,25 @@ public function test_an_operation_holds_its_own_rules_and_those_naming_none_in_c ); } - public function test_read_rules_are_absent_when_only_write_rules_exist(): void + public function test_a_name_with_only_write_rules_has_no_read_rules(): void { $subject = AclRuleIndex::of(AclRules::fromEmpty( attributes: [$this->denyCnWrite], )); - self::assertFalse($subject->hasAttributeRules(AttributeAccess::Read)); - self::assertTrue($subject->hasAttributeRules(AttributeAccess::Write)); + self::assertSame( + [], + $subject->attributeRules( + AttributeAccess::Read, + 'cn', + ), + ); + self::assertSame( + [$this->denyCnWrite], + $subject->attributeRules( + AttributeAccess::Write, + 'cn', + ), + ); } } diff --git a/tests/unit/Server/AccessControl/AclRulesTest.php b/tests/unit/Server/AccessControl/AclRulesTest.php index c27b3e40..760d7a34 100644 --- a/tests/unit/Server/AccessControl/AclRulesTest.php +++ b/tests/unit/Server/AccessControl/AclRulesTest.php @@ -395,25 +395,24 @@ public function test_secureDefault_restricts_privileged_extended_ops_to_admin(): public function test_secureDefault_strips_userPassword_from_another_users_read(): void { - $filtered = $this->subject->filterEntry( - $this->user(), - $this->entry(self::OTHER_DN), - ); + $scope = $this->subject->readScope($this->user()); + $entry = $this->entry(self::OTHER_DN); + + self::assertTrue($scope->isEntryVisible($entry)); - self::assertNotNull($filtered); - self::assertNull($filtered->get('userPassword')); - self::assertNotNull($filtered->get('cn')); + $readable = $scope->readable($entry); + + self::assertNull($readable->get('userPassword')); + self::assertNotNull($readable->get('cn')); } public function test_secureDefault_strips_userPassword_from_self_read(): void { - $filtered = $this->subject->filterEntry( - $this->user(), - $this->entry(self::USER_DN), - ); + $scope = $this->subject->readScope($this->user()); + $entry = $this->entry(self::USER_DN); - self::assertNotNull($filtered); - self::assertNull($filtered->get('userPassword')); + self::assertTrue($scope->isEntryVisible($entry)); + self::assertNull($scope->readable($entry)->get('userPassword')); } public function test_secureDefault_lets_self_write_but_not_read_its_userPassword(): void diff --git a/tests/unit/Server/AccessControl/PrivilegedBypassAccessControlTest.php b/tests/unit/Server/AccessControl/PrivilegedBypassAccessControlTest.php index 57fdd053..a0692788 100644 --- a/tests/unit/Server/AccessControl/PrivilegedBypassAccessControlTest.php +++ b/tests/unit/Server/AccessControl/PrivilegedBypassAccessControlTest.php @@ -98,7 +98,7 @@ public function test_privileged_token_may_use_any_control_and_keeps_the_whole_en ->method('mayUseControl'); $this->inner ->expects(self::never()) - ->method('filterEntry'); + ->method('readScope'); $this->inner ->expects(self::never()) ->method('isEntryVisible'); @@ -111,10 +111,9 @@ public function test_privileged_token_may_use_any_control_and_keeps_the_whole_en )); self::assertSame( $entry, - $this->subject->filterEntry( - $this->manager, - $entry, - ), + $this->subject + ->readScope($this->manager) + ->readable($entry), ); self::assertTrue($this->subject->isEntryVisible( $this->manager, diff --git a/tests/unit/Server/AccessControl/Read/ConfidentialReadScopeTest.php b/tests/unit/Server/AccessControl/Read/ConfidentialReadScopeTest.php new file mode 100644 index 00000000..5d299402 --- /dev/null +++ b/tests/unit/Server/AccessControl/Read/ConfidentialReadScopeTest.php @@ -0,0 +1,205 @@ + + * + * 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\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Schema\SchemaResource; +use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\ConfidentialReadScope; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadScopeInterface; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadVerdict; +use FreeDSx\Ldap\Server\AccessControl\WithheldAttributePolicy; +use FreeDSx\Ldap\Server\Token\BindToken; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class ConfidentialReadScopeTest extends TestCase +{ + private ReadScopeInterface&MockObject $inner; + + private AccessControlInterface&MockObject $accessControl; + + private BindToken $user; + + private Dn $dn; + + private ConfidentialReadScope $subject; + + protected function setUp(): void + { + $this->inner = $this->createMock(ReadScopeInterface::class); + $this->accessControl = $this->createMock(AccessControlInterface::class); + $this->user = BindToken::fromDn('cn=user,dc=foo,dc=bar'); + $this->dn = new Dn('cn=other,dc=foo,dc=bar'); + $this->subject = new ConfidentialReadScope( + $this->inner, + new WithheldAttributePolicy( + $this->accessControl, + SchemaResource::Core->load(), + ), + $this->user, + ); + } + + public function test_a_deny_from_the_inner_scope_stays_a_deny_for_a_confidential_attribute(): void + { + $this->inner + ->method('verdict') + ->with('userpassword') + ->willReturn(ReadVerdict::Deny); + + self::assertSame( + ReadVerdict::Deny, + $this->subject->verdict('userpassword'), + ); + } + + public function test_a_confidential_attribute_the_inner_scope_allows_is_decided_on_each_entry(): void + { + $this->inner + ->method('verdict') + ->with('userpassword') + ->willReturn(ReadVerdict::Allow); + + self::assertSame( + ReadVerdict::PerEntry, + $this->subject->verdict('userpassword'), + ); + } + + public function test_an_attribute_that_is_not_confidential_keeps_the_inner_verdict(): void + { + $this->inner + ->method('verdict') + ->with('cn') + ->willReturn(ReadVerdict::Allow); + + self::assertSame( + ReadVerdict::Allow, + $this->subject->verdict('cn'), + ); + } + + public function test_a_grant_is_not_looked_for_when_the_inner_scope_withholds_the_attribute(): void + { + $this->inner + ->method('isReadableOn') + ->willReturn(false); + $this->accessControl + ->expects(self::never()) + ->method('hasConfidentialAccess'); + + self::assertFalse($this->subject->isReadableOn( + 'userpassword', + $this->dn, + )); + } + + public function test_a_confidential_attribute_is_withheld_without_a_grant(): void + { + $this->inner + ->method('isReadableOn') + ->willReturn(true); + $this->accessControl + ->expects(self::once()) + ->method('hasConfidentialAccess') + ->with( + $this->user, + 'userpassword', + ) + ->willReturn(false); + + self::assertFalse($this->subject->isReadableOn( + 'userpassword', + $this->dn, + )); + } + + public function test_a_confidential_attribute_is_readable_with_a_grant(): void + { + $this->inner + ->method('isReadableOn') + ->willReturn(true); + $this->accessControl + ->method('hasConfidentialAccess') + ->willReturn(true); + + self::assertTrue($this->subject->isReadableOn( + 'userpassword', + $this->dn, + )); + } + + public function test_an_attribute_that_is_not_confidential_is_readable_without_a_grant(): void + { + $this->inner + ->method('isReadableOn') + ->willReturn(true); + $this->accessControl + ->expects(self::never()) + ->method('hasConfidentialAccess'); + + self::assertTrue($this->subject->isReadableOn( + 'cn', + $this->dn, + )); + } + + public function test_readable_withholds_a_confidential_attribute_without_a_grant(): void + { + $this->inner + ->method('isReadableOn') + ->willReturn(true); + $this->accessControl + ->method('hasConfidentialAccess') + ->willReturn(false); + + $readable = $this->subject->readable(new Entry( + $this->dn, + new Attribute( + 'cn', + 'other', + ), + new Attribute( + 'userPassword', + 'secret', + ), + )); + + self::assertNotNull($readable->get('cn')); + self::assertNull($readable->get('userPassword')); + } + + public function test_it_is_current_only_while_the_inner_scope_is(): void + { + $this->inner + ->method('isCurrent') + ->willReturn(false); + + self::assertFalse($this->subject->isCurrent()); + } + + public function test_entry_visibility_comes_from_the_inner_scope(): void + { + $entry = new Entry($this->dn); + $this->inner + ->method('isEntryVisible') + ->with($entry) + ->willReturn(false); + + self::assertFalse($this->subject->isEntryVisible($entry)); + } +} diff --git a/tests/unit/Server/AccessControl/Read/RuleReadScopeTest.php b/tests/unit/Server/AccessControl/Read/RuleReadScopeTest.php new file mode 100644 index 00000000..b2a5089a --- /dev/null +++ b/tests/unit/Server/AccessControl/Read/RuleReadScopeTest.php @@ -0,0 +1,306 @@ + + * + * 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\AccessControl\Read; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Server\AccessControl\AclRules; +use FreeDSx\Ldap\Server\AccessControl\Read\ReadVerdict; +use FreeDSx\Ldap\Server\AccessControl\Rule\AttributeRule; +use FreeDSx\Ldap\Server\AccessControl\Rule\OperationRule; +use FreeDSx\Ldap\Server\AccessControl\RuleBasedAccessControl; +use FreeDSx\Ldap\Server\AccessControl\Subject\Subject; +use FreeDSx\Ldap\Server\AccessControl\Target\Target; +use FreeDSx\Ldap\Server\Token\AnonToken; +use FreeDSx\Ldap\Server\Token\BindToken; +use PHPUnit\Framework\TestCase; +use RuntimeException; + +final class RuleReadScopeTest extends TestCase +{ + private BindToken $user; + + protected function setUp(): void + { + $this->user = BindToken::fromDn('cn=user,dc=foo,dc=bar'); + } + + public function test_a_name_no_rule_reads_is_allowed_everywhere(): void + { + $scope = $this->policy()->readScope($this->user); + + self::assertSame( + ReadVerdict::Allow, + $scope->verdict('cn'), + ); + } + + public function test_a_rule_for_anyone_anywhere_decides_the_name_everywhere(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'userPassword', + )->forRead())->readScope($this->user); + + self::assertSame( + ReadVerdict::Deny, + $scope->verdict('userpassword'), + ); + self::assertSame( + ReadVerdict::Allow, + $scope->verdict('cn'), + ); + } + + public function test_a_rule_whose_subject_the_token_is_not_is_passed_over(): void + { + $policy = $this->policy(AttributeRule::deny( + Subject::anonymous(), + Target::any(), + 'mail', + )->forRead()); + + self::assertSame( + ReadVerdict::Allow, + $policy->readScope($this->user)->verdict('mail'), + ); + self::assertSame( + ReadVerdict::Deny, + $policy->readScope(new AnonToken())->verdict('mail'), + ); + } + + public function test_an_allow_ahead_of_a_deny_decides_for_the_identity_it_names(): void + { + $policy = $this->policy( + AttributeRule::allow( + Subject::dn('cn=user,dc=foo,dc=bar'), + Target::any(), + 'mail', + )->forRead(), + AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'mail', + )->forRead(), + ); + + self::assertSame( + ReadVerdict::Allow, + $policy->readScope($this->user)->verdict('mail'), + ); + self::assertSame( + ReadVerdict::Deny, + $policy->readScope(BindToken::fromDn('cn=other,dc=foo,dc=bar'))->verdict('mail'), + ); + } + + public function test_a_write_rule_does_not_decide_a_read(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'cn', + )->forWrite())->readScope($this->user); + + self::assertSame( + ReadVerdict::Allow, + $scope->verdict('cn'), + ); + } + + public function test_a_dn_subtree_subject_is_decided_from_the_identity_alone(): void + { + $policy = $this->policy(AttributeRule::deny( + Subject::dnSubtree('ou=hidden,dc=foo,dc=bar'), + Target::any(), + 'mail', + )->forRead()); + + self::assertSame( + ReadVerdict::Deny, + $policy->readScope(BindToken::fromDn('cn=a,ou=hidden,dc=foo,dc=bar'))->verdict('mail'), + ); + self::assertSame( + ReadVerdict::Allow, + $policy->readScope($this->user)->verdict('mail'), + ); + } + + public function test_a_rule_naming_no_attribute_decides_every_name(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::anyone(), + Target::any(), + )->forRead())->readScope($this->user); + + self::assertSame( + ReadVerdict::Deny, + $scope->verdict('cn'), + ); + self::assertSame( + ReadVerdict::Deny, + $scope->verdict('anythingelse'), + ); + } + + public function test_a_subtree_scoped_rule_leaves_the_decision_to_each_entry(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::anyone(), + Target::subtree('ou=hidden,dc=foo,dc=bar'), + 'mail', + )->forRead())->readScope($this->user); + + self::assertSame( + ReadVerdict::PerEntry, + $scope->verdict('mail'), + ); + self::assertFalse($scope->isReadableOn( + 'mail', + new Dn('cn=a,ou=hidden,dc=foo,dc=bar'), + )); + self::assertTrue($scope->isReadableOn( + 'mail', + new Dn('cn=a,ou=shown,dc=foo,dc=bar'), + )); + } + + public function test_a_self_rule_leaves_the_decision_to_each_entry(): void + { + $scope = $this->policy( + AttributeRule::allow( + Subject::self(), + Target::any(), + 'mail', + )->forRead(), + AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'mail', + )->forRead(), + )->readScope($this->user); + + self::assertSame( + ReadVerdict::PerEntry, + $scope->verdict('mail'), + ); + self::assertTrue($scope->isReadableOn( + 'mail', + new Dn('cn=user,dc=foo,dc=bar'), + )); + self::assertFalse($scope->isReadableOn( + 'mail', + new Dn('cn=other,dc=foo,dc=bar'), + )); + } + + public function test_a_group_rule_leaves_the_decision_to_each_entry(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::group('cn=staff,dc=foo,dc=bar'), + Target::any(), + 'mail', + )->forRead())->readScope($this->user); + + self::assertSame( + ReadVerdict::PerEntry, + $scope->verdict('mail'), + ); + } + + public function test_a_subject_that_cannot_be_decided_on_a_deny_withholds_the_attribute(): void + { + $scope = $this->policy(AttributeRule::deny( + Subject::callback(static fn(): bool => throw new RuntimeException('undecidable')), + Target::any(), + 'mail', + )->forRead())->readScope($this->user); + + self::assertFalse($scope->isReadableOn( + 'mail', + new Dn('cn=a,dc=foo,dc=bar'), + )); + } + + public function test_readable_keeps_the_same_entry_when_nothing_is_withheld(): void + { + $entry = new Entry( + new Dn('cn=a,dc=foo,dc=bar'), + new Attribute( + 'cn', + 'a', + ), + ); + + self::assertSame( + $entry, + $this->policy()->readScope($this->user)->readable($entry), + ); + } + + public function test_readable_withholds_every_form_of_a_denied_type(): void + { + $readable = $this->policy(AttributeRule::deny( + Subject::anyone(), + Target::any(), + 'userPassword', + )->forRead())->readScope($this->user)->readable(new Entry( + new Dn('cn=a,dc=foo,dc=bar'), + new Attribute( + 'cn', + 'a', + ), + new Attribute( + 'userPassword', + 'secret', + ), + new Attribute( + 'USERPASSWORD;binary', + 'secret', + ), + )); + + self::assertSame( + ['cn'], + array_map( + static fn(Attribute $attribute): string => $attribute->getDescription(), + $readable->getAttributes(), + ), + ); + } + + public function test_an_entry_is_visible_by_the_search_rules(): void + { + $scope = (new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow( + Subject::anyone(), + Target::subtree('ou=shown,dc=foo,dc=bar'), + )], + )))->readScope($this->user); + + self::assertTrue($scope->isEntryVisible(new Entry(new Dn('cn=a,ou=shown,dc=foo,dc=bar')))); + self::assertFalse($scope->isEntryVisible(new Entry(new Dn('cn=a,ou=hidden,dc=foo,dc=bar')))); + } + + private function policy(AttributeRule ...$rules): RuleBasedAccessControl + { + return new RuleBasedAccessControl(AclRules::fromEmpty( + operations: [OperationRule::allow(Subject::anyone())], + attributes: $rules, + )); + } +} diff --git a/tests/unit/Server/AccessControl/ReloadableAccessControlTest.php b/tests/unit/Server/AccessControl/ReloadableAccessControlTest.php index 4918b84a..f32a9b1b 100644 --- a/tests/unit/Server/AccessControl/ReloadableAccessControlTest.php +++ b/tests/unit/Server/AccessControl/ReloadableAccessControlTest.php @@ -60,6 +60,18 @@ public function test_decisions_after_a_replace_come_from_the_new_policy(): void ); } + public function test_a_read_scope_stops_being_current_once_the_policy_is_replaced(): void + { + $scope = $this->subject->readScope(BindToken::fromDn('cn=user,dc=foo,dc=bar')); + + self::assertTrue($scope->isCurrent()); + + $this->subject->replace(new RuleBasedAccessControl(AclRules::fromEmpty())); + + self::assertFalse($scope->isCurrent()); + self::assertTrue($this->subject->readScope(BindToken::fromDn('cn=user,dc=foo,dc=bar'))->isCurrent()); + } + public function test_a_replacing_policy_receives_the_backend_set_earlier(): void { $backend = $this->createMock(ReadBackendInterface::class); diff --git a/tests/unit/Server/AccessControl/RuleBasedAccessControlTest.php b/tests/unit/Server/AccessControl/RuleBasedAccessControlTest.php index 84f101fe..1bd64b81 100644 --- a/tests/unit/Server/AccessControl/RuleBasedAccessControlTest.php +++ b/tests/unit/Server/AccessControl/RuleBasedAccessControlTest.php @@ -314,7 +314,7 @@ public function test_may_use_control_is_false_for_an_unauthenticated_token(): vo )); } - public function test_filter_entry_returns_same_instance_when_no_attribute_rules(): void + public function test_readable_keeps_the_same_entry_when_there_are_no_attribute_rules(): void { $entry = Entry::create('dc=foo,dc=bar', ['cn' => 'foo']); @@ -322,7 +322,8 @@ public function test_filter_entry_returns_same_instance_when_no_attribute_rules( operations: [OperationRule::allow(new AnySubjectMatcher())], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -330,7 +331,7 @@ public function test_filter_entry_returns_same_instance_when_no_attribute_rules( self::assertSame($entry, $result); } - public function test_filter_entry_removes_attribute_when_deny_rule_matches(): void + public function test_readable_drops_an_attribute_a_deny_rule_matches(): void { $entry = Entry::create( 'dc=foo,dc=bar', @@ -348,7 +349,8 @@ public function test_filter_entry_removes_attribute_when_deny_rule_matches(): vo ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -358,7 +360,7 @@ public function test_filter_entry_removes_attribute_when_deny_rule_matches(): vo self::assertNotNull($result->get('cn')); } - public function test_filter_entry_keeps_attribute_when_allow_rule_precedes_deny_rule(): void + public function test_readable_keeps_an_attribute_when_an_allow_rule_precedes_a_deny_rule(): void { $matchingSubject = $this->createMock(SubjectMatcherInterface::class); $matchingSubject->method('matches')->willReturn(true); @@ -384,7 +386,8 @@ public function test_filter_entry_keeps_attribute_when_allow_rule_precedes_deny_ ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -393,7 +396,7 @@ public function test_filter_entry_keeps_attribute_when_allow_rule_precedes_deny_ self::assertNotNull($result->get('userPassword')); } - public function test_filter_entry_keeps_attribute_when_no_rule_matches(): void + public function test_readable_keeps_an_attribute_no_rule_matches(): void { $nonMatchingSubject = $this->createMock(SubjectMatcherInterface::class); $nonMatchingSubject->method('matches')->willReturn(false); @@ -414,7 +417,8 @@ public function test_filter_entry_keeps_attribute_when_no_rule_matches(): void ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -423,7 +427,7 @@ public function test_filter_entry_keeps_attribute_when_no_rule_matches(): void self::assertNotNull($result->get('cn')); } - public function test_filter_entry_returns_same_instance_when_all_attributes_kept(): void + public function test_readable_keeps_the_same_entry_when_every_attribute_is_allowed(): void { $entry = Entry::create( 'dc=foo,dc=bar', @@ -441,7 +445,8 @@ public function test_filter_entry_returns_same_instance_when_all_attributes_kept ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -449,7 +454,7 @@ public function test_filter_entry_returns_same_instance_when_all_attributes_kept self::assertSame($entry, $result); } - public function test_filter_entry_empty_attributes_list_on_deny_rule_matches_all_attributes(): void + public function test_readable_drops_every_attribute_for_a_deny_rule_naming_none(): void { $entry = Entry::create( 'dc=foo,dc=bar', @@ -463,7 +468,8 @@ public function test_filter_entry_empty_attributes_list_on_deny_rule_matches_all ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -501,7 +507,8 @@ public function matches( ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -534,7 +541,8 @@ public function test_a_deny_naming_no_attribute_ahead_of_a_named_allow_withholds ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -560,7 +568,8 @@ public function test_write_rules_alone_leave_a_read_entry_untouched(): void ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -588,7 +597,8 @@ public function test_an_undecidable_subject_on_a_deny_naming_the_attribute_still ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -598,7 +608,7 @@ public function test_an_undecidable_subject_on_a_deny_naming_the_attribute_still self::assertNotNull($result->get('sn')); } - public function test_filter_entry_returns_null_when_search_operation_is_denied_on_entry_dn(): void + public function test_a_read_scope_hides_an_entry_when_search_is_denied_on_its_dn(): void { $entry = Entry::create( 'dc=foo,dc=bar', @@ -615,7 +625,8 @@ public function test_filter_entry_returns_null_when_search_operation_is_denied_o ], )); - $result = $subject->filterEntry( + $result = self::readableOrNull( + $subject, $this->bindToken, $entry, ); @@ -1237,6 +1248,18 @@ public function test_the_first_matching_relocation_rule_wins(): void ); } + private static function readableOrNull( + RuleBasedAccessControl $subject, + TokenInterface $token, + Entry $entry, + ): ?Entry { + $scope = $subject->readScope($token); + + return $scope->isEntryVisible($entry) + ? $scope->readable($entry) + : null; + } + private function rulesDenyingFilterOn(string ...$attributes): AclRules { return AclRules::fromEmpty( diff --git a/tests/unit/Server/Backend/Storage/Filter/StrippedEntryMatcherTest.php b/tests/unit/Server/Backend/Storage/Filter/StrippedEntryMatcherTest.php index cd579516..d12a379d 100644 --- a/tests/unit/Server/Backend/Storage/Filter/StrippedEntryMatcherTest.php +++ b/tests/unit/Server/Backend/Storage/Filter/StrippedEntryMatcherTest.php @@ -34,14 +34,6 @@ final class StrippedEntryMatcherTest extends TestCase private StrippedEntryMatcher $subject; - private Attribute $cn; - - private Attribute $sn; - - private Attribute $userPassword; - - private Entry $entry; - protected function setUp(): void { $schema = SchemaResource::Core->load(); @@ -51,199 +43,131 @@ protected function setUp(): void $schema, new LinkedAttributes($schema), ); - - $this->cn = new Attribute( - 'cn', - 'alice', - ); - $this->sn = new Attribute( - 'sn', - 'smith', - ); - $this->userPassword = new Attribute( - 'userPassword', - 'secret', - ); - $this->entry = new Entry( - new Dn('cn=alice,dc=foo,dc=bar'), - $this->cn, - $this->sn, - $this->userPassword, - ); - } - - public function test_an_entry_nothing_was_stripped_from_still_matches_without_being_judged(): void - { - $this->mockEvaluator - ->expects(self::never()) - ->method('evaluate'); - - self::assertTrue($this->subject->stillMatches( - $this->entry, - $this->entry, - Filters::equal( - 'cn', - 'alice', - ), - )); } - public function test_losing_an_attribute_the_filter_does_not_read_keeps_the_match_without_judging_it(): void + public function test_withholding_a_type_the_filter_does_not_read_does_not_concern_it(): void { - $this->mockEvaluator - ->expects(self::never()) - ->method('evaluate'); - - self::assertTrue($this->subject->stillMatches( - $this->entry, - $this->stripped( - $this->cn, - $this->sn, - ), + self::assertFalse($this->subject->concerns( Filters::equal( 'cn', 'alice', ), + 'userpassword', )); } - public function test_losing_an_attribute_the_filter_reads_judges_the_stripped_entry_again(): void + public function test_withholding_a_type_the_filter_reads_concerns_it(): void { - $stripped = $this->stripped( - $this->cn, - $this->sn, - ); - $filter = Filters::and( - Filters::equal( - 'cn', - 'alice', + self::assertTrue($this->subject->concerns( + Filters::and( + Filters::equal( + 'cn', + 'alice', + ), + Filters::present('userPassword'), ), - Filters::present('userPassword'), - ); - - $this->mockEvaluator - ->expects(self::once()) - ->method('evaluate') - ->with( - $stripped, - $filter, - ) - ->willReturn(false); - - self::assertFalse($this->subject->stillMatches( - $this->entry, - $stripped, - $filter, + 'userpassword', )); } - public function test_losing_an_option_bearing_form_of_a_type_the_filter_reads_judges_the_stripped_entry_again(): void + public function test_withholding_the_type_an_option_bearing_assertion_names_concerns_it(): void { - $entry = $this->entryWithTaggedGivenName(); - - $this->mockEvaluator - ->expects(self::once()) - ->method('evaluate') - ->willReturn(false); - - self::assertFalse($this->subject->stillMatches( - $entry, - Entry::raw( - $entry->getDn(), - [$this->cn], - ), + self::assertTrue($this->subject->concerns( Filters::equal( - 'givenName', + 'givenName;lang-en', 'Ali', ), + 'givenname', )); } - public function test_losing_an_option_bearing_form_of_another_type_keeps_the_match_without_judging_it(): void + public function test_withholding_another_type_than_the_filter_reads_does_not_concern_it(): void { - $entry = $this->entryWithTaggedGivenName(); - - $this->mockEvaluator - ->expects(self::never()) - ->method('evaluate'); - - self::assertTrue($this->subject->stillMatches( - $entry, - Entry::raw( - $entry->getDn(), - [$this->cn], - ), + self::assertFalse($this->subject->concerns( Filters::equal( 'cn', 'alice', ), + 'givenname', )); } - public function test_losing_a_subtype_of_a_type_the_filter_names_judges_the_stripped_entry_again(): void + public function test_withholding_a_subtype_of_a_type_the_filter_names_concerns_it(): void { - $this->mockEvaluator - ->expects(self::once()) - ->method('evaluate') - ->willReturn(false); - - self::assertFalse($this->subject->stillMatches( - $this->entry, - $this->stripped( - $this->sn, - $this->userPassword, - ), + self::assertTrue($this->subject->concerns( Filters::equal( 'name', 'alice', ), + 'cn', )); } - public function test_a_stripped_entry_rebuilt_from_other_attributes_is_judged_again(): void + public function test_an_answer_is_the_same_when_asked_again(): void { - $this->mockEvaluator - ->expects(self::once()) - ->method('evaluate') - ->willReturn(true); + $filter = Filters::equal( + 'name', + 'alice', + ); - self::assertTrue($this->subject->stillMatches( - $this->entry, - new Entry( - new Dn('cn=alice,dc=foo,dc=bar'), - new Attribute( - 'cn', - 'alice', - ), - new Attribute( - 'sn', - 'smith', - ), + self::assertSame( + $this->subject->concerns( + $filter, + 'cn', ), - Filters::equal( + $this->subject->concerns( + $filter, 'cn', - 'alice', ), - )); + ); } #[DataProvider('alwaysJudgedFilterProvider')] - public function test_a_filter_whose_values_are_not_read_off_the_entry_alone_is_always_judged_again( + public function test_withholding_anything_concerns_a_filter_whose_values_are_not_read_off_the_entry_alone( FilterInterface $filter, ): void { + self::assertTrue($this->subject->concerns( + $filter, + 'sn', + )); + } + + public function test_the_entry_is_judged_against_the_filter_without_what_was_withheld(): void + { + $entry = new Entry( + new Dn('cn=alice,dc=foo,dc=bar'), + new Attribute( + 'cn', + 'alice', + ), + new Attribute( + 'userPassword', + 'secret', + ), + ); + $filter = Filters::present('userPassword'); + $judged = null; + $this->mockEvaluator ->expects(self::once()) ->method('evaluate') - ->willReturn(true); + ->willReturnCallback(static function (Entry $stripped) use (&$judged): bool { + $judged = $stripped; - self::assertTrue($this->subject->stillMatches( - $this->entry, - $this->stripped( - $this->cn, - $this->sn, - ), + return false; + }); + + self::assertFalse($this->subject->stillMatches( + $entry, + [1 => true], $filter, )); + self::assertInstanceOf( + Entry::class, + $judged, + ); + self::assertTrue($judged->has('cn')); + self::assertFalse($judged->has('userPassword')); } public static function alwaysJudgedFilterProvider(): Generator @@ -268,24 +192,4 @@ public static function alwaysJudgedFilterProvider(): Generator ), ]; } - - private function entryWithTaggedGivenName(): Entry - { - return new Entry( - new Dn('cn=alice,dc=foo,dc=bar'), - $this->cn, - new Attribute( - 'givenName;lang-en', - 'Ali', - ), - ); - } - - private function stripped(Attribute ...$kept): Entry - { - return Entry::raw( - $this->entry->getDn(), - $kept, - ); - } } diff --git a/tests/unit/Server/Middleware/ResponseWriterMiddlewareTest.php b/tests/unit/Server/Middleware/ResponseWriterMiddlewareTest.php index cae89b8f..8744c265 100644 --- a/tests/unit/Server/Middleware/ResponseWriterMiddlewareTest.php +++ b/tests/unit/Server/Middleware/ResponseWriterMiddlewareTest.php @@ -195,8 +195,8 @@ public function test_it_keeps_a_matched_dn_the_token_may_see(): void ->method('get') ->willReturn(Entry::create('dc=bar')); $this->mockAccessControl - ->method('filterEntry') - ->willReturn(Entry::create('dc=bar')); + ->method('isEntryVisible') + ->willReturn(true); $this->subject->process( $this->contextFor(new DeleteRequest('cn=missing,dc=bar')), @@ -217,8 +217,8 @@ public function test_it_drops_a_matched_dn_the_access_control_hides(): void ->method('get') ->willReturn(Entry::create('dc=bar')); $this->mockAccessControl - ->method('filterEntry') - ->willReturn(null); + ->method('isEntryVisible') + ->willReturn(false); $this->subject->process( $this->contextFor(new DeleteRequest('cn=missing,dc=bar')),