From 6b8c189eb0d5d549b5770675742d768d3f72fe6e Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 18:13:09 -0400 Subject: [PATCH 1/4] Refactor the load test / comparison to be more fair: * Seed the comparison with person entries. * Run every side of a comparison under its own access control. * Probe the run identity's writes before a comparison seeds. * Count the entries a size-limited search returned in the load test. --- .../Compare/BenchCompareCommand.php | 102 +++++++-- tests/performance/Compare/BenchSide.php | 4 +- .../performance/Compare/ComparisonReport.php | 70 ++++-- tests/performance/Compare/SideRun.php | 30 +++ tests/performance/Compare/TargetBench.php | 211 ++++++++++++++++-- tests/performance/Workload/Worker.php | 78 +++++-- tests/profile/compare-ldap.sh | 135 ++++++++++- tests/profile/openldap-bootstrap.sh | 5 + tests/support/LdapBackendStorageCommand.php | 3 +- tests/support/Seed/MinimalSeedEntries.php | 9 +- tests/support/Seed/PersonSeedEntries.php | 39 ++-- tests/support/Seed/SeedDn.php | 20 +- 12 files changed, 610 insertions(+), 96 deletions(-) create mode 100644 tests/performance/Compare/SideRun.php diff --git a/tests/performance/Compare/BenchCompareCommand.php b/tests/performance/Compare/BenchCompareCommand.php index ef92177cb..7cb67a1d1 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 5671a657f..a22a20a85 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 c5bf133d7..c123b7ebc 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 000000000..48dddb871 --- /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 4784d12b8..763f5f9ed 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 a6e80972a..16a6303c3 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 7b9718367..7b637fc88 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 5be50cb20..083f0c3ce 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 df23be3f9..e322de4ca 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 00071e2ef..19eb11b83 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 ca6fafb0e..59b0c5f71 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 d2fe5679c..0f49b6a9d 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; } } From 001cb1903de2c02c56f39b732ec0218bbfe54bbf Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 19:28:48 -0400 Subject: [PATCH 2/4] Memoize schema subtype descriptions. --- src/FreeDSx/Ldap/Schema/Schema.php | 75 +++++++++++++++++++++++++++--- tests/unit/Schema/SchemaTest.php | 66 ++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 6 deletions(-) diff --git a/src/FreeDSx/Ldap/Schema/Schema.php b/src/FreeDSx/Ldap/Schema/Schema.php index 6034d5452..0de16cef7 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, so they get more room. + */ + private const MAX_REMEMBERED_SUBTYPE_PAIRS = 8192; + /** * Rules RFC 4517 and RFC 4530 define as ordering rules. */ @@ -81,6 +87,20 @@ 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, keyed by both joined on a NUL and dropped with the types. + * + * @var array + */ + private array $subtypeDescriptions = []; + /** * Bound to this schema, so it is built once rather than per lookup. */ @@ -110,6 +130,8 @@ public function addAttributeType(AttributeType $type): static $this->superTypeOids = null; $this->orderingRuleOids = null; $this->confidentialDescriptions = []; + $this->operationalDescriptions = []; + $this->subtypeDescriptions = []; return $this; } @@ -169,6 +191,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 +311,21 @@ public function isDescriptionSubtypeOf( string $description, string $ofDescription, ): bool { - $type = Attribute::normalizeName($description); - $ofType = Attribute::normalizeName($ofDescription); + $key = $description . "\0" . $ofDescription; - if ($type !== $ofType && !$this->isTypeOrSubtypeOf($type, $ofType)) { - return false; + if (isset($this->subtypeDescriptions[$key])) { + return $this->subtypeDescriptions[$key]; } - - return self::optionsCover( + $isSubtype = $this->decideDescriptionSubtypeOf( $description, $ofDescription, ); + + if (count($this->subtypeDescriptions) < self::MAX_REMEMBERED_SUBTYPE_PAIRS) { + $this->subtypeDescriptions[$key] = $isSubtype; + } + + return $isSubtype; } /** @@ -534,6 +580,23 @@ private static function primarySpelling( /** * Whether every option the wanted description carries is also carried by the one being tested. */ + 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, + ); + } + private static function optionsCover( string $description, string $ofDescription, diff --git a/tests/unit/Schema/SchemaTest.php b/tests/unit/Schema/SchemaTest.php index aaf9640f5..42d544151 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,71 @@ 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_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( From 48217df46d6349ab6c360aa15009a9a6375963eb Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 19:28:48 -0400 Subject: [PATCH 3/4] Decide a wildcard selection by usage. --- .../AttributeProjection.php | 127 +++++++++++------- .../AttributeProjectionTest.php | 66 +++++++++ 2 files changed, 144 insertions(+), 49 deletions(-) diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php index 099372b15..bb1306795 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, so a list of only these is 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,8 @@ 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 +98,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 +172,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 +194,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 +228,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 +241,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/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php b/tests/unit/Protocol/ServerProtocolHandler/AttributeProjectionTest.php index a206f1e60..5c0217872 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( From 9640f3c86308b3f061abd24c47e9c6e48b9b864e Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 19:28:57 -0400 Subject: [PATCH 4/4] Evaluate search results for an entry once instead of multiple passes. An identity's read decisions are now taken once per request for each attribute. Only rules that depend on the entry's DN are asked per entry. A single gate applies them together with the selection so an entry is rebuilt at most once. --- .../Admission/AttributeAdmission.php | 36 ++ .../Admission/SearchResultGate.php | 182 ++++++++++ .../AssertionEvaluator.php | 9 +- .../AttributeProjection.php | 7 +- .../GeneratedEntryResponder.php | 7 +- .../MatchedDnAccessFilterTrait.php | 11 +- .../ReadEntryControlHandler.php | 9 +- .../ServerSearchHandler.php | 22 +- src/FreeDSx/Ldap/Schema/Schema.php | 26 +- .../AccessControl/AccessControlInterface.php | 16 +- .../Server/AccessControl/AclRuleIndex.php | 6 - .../ConfidentialAttributeAccessControl.php | 52 +-- .../PrivilegedBypassAccessControl.php | 29 +- .../Read/ConfidentialReadScope.php | 72 ++++ .../AccessControl/Read/ReadScopeInterface.php | 52 +++ .../Server/AccessControl/Read/ReadVerdict.php | 39 ++ .../Read/ReadsEntryAttributesTrait.php | 57 +++ .../Read/ReloadableReadScope.php | 65 ++++ .../AccessControl/Read/RuleReadScope.php | 101 ++++++ .../Read/UnrestrictedReadScope.php | 54 +++ .../AccessControl/ReloadableAccessControl.php | 38 +- .../AccessControl/RuleBasedAccessControl.php | 65 ++-- .../Subject/AnonymousSubjectMatcher.php | 2 +- .../Subject/AnySubjectMatcher.php | 2 +- .../Subject/AuthenticatedSubjectMatcher.php | 2 +- .../Subject/DnSubjectMatcher.php | 4 +- .../Subject/DnSubtreeSubjectMatcher.php | 4 +- .../Subject/TokenDecidedSubjectInterface.php | 23 ++ .../AccessControl/WithheldAttributePolicy.php | 10 +- .../Storage/Filter/StrippedEntryMatcher.php | 117 +++--- src/FreeDSx/Ldap/Server/Paging/PageFiller.php | 52 +-- .../DefaultAccessControlEnforcementTest.php | 6 +- .../Admission/SearchResultGateTest.php | 332 ++++++++++++++++++ .../ServerCompareHandlerTest.php | 9 +- .../ServerDispatchHandlerTest.php | 5 +- .../ServerPagingHandlerTest.php | 172 ++++----- .../ServerSearchHandlerTest.php | 62 ++-- tests/unit/Schema/SchemaTest.php | 18 + .../Server/AccessControl/AclRuleIndexTest.php | 18 +- .../Server/AccessControl/AclRulesTest.php | 25 +- .../PrivilegedBypassAccessControlTest.php | 9 +- .../Read/ConfidentialReadScopeTest.php | 205 +++++++++++ .../AccessControl/Read/RuleReadScopeTest.php | 306 ++++++++++++++++ .../ReloadableAccessControlTest.php | 12 + .../RuleBasedAccessControlTest.php | 59 +++- .../Filter/StrippedEntryMatcherTest.php | 238 ++++--------- .../ResponseWriterMiddlewareTest.php | 8 +- 47 files changed, 2002 insertions(+), 653 deletions(-) create mode 100644 src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/AttributeAdmission.php create mode 100644 src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/Admission/SearchResultGate.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/ConfidentialReadScope.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/ReadScopeInterface.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/ReadVerdict.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/ReadsEntryAttributesTrait.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/ReloadableReadScope.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/RuleReadScope.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Read/UnrestrictedReadScope.php create mode 100644 src/FreeDSx/Ldap/Server/AccessControl/Subject/TokenDecidedSubjectInterface.php create mode 100644 tests/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php create mode 100644 tests/unit/Server/AccessControl/Read/ConfidentialReadScopeTest.php create mode 100644 tests/unit/Server/AccessControl/Read/RuleReadScopeTest.php 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 000000000..32c9841ca --- /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 000000000..ed924c10a --- /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 1bee1b947..2902d7ba5 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 bb1306795..6619d70ef 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/AttributeProjection.php @@ -33,7 +33,7 @@ final class AttributeProjection { /** - * Selectors that name no attribute type, so a list of only these is decided by usage alone. + * Selectors that name no attribute type, which leave a list of only these to be decided by usage alone. */ private const SELECTORS = [ SearchRequest::ATTRIBUTES_ALL_USER, @@ -82,7 +82,10 @@ 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) === [], + array_diff( + $names, + self::SELECTORS, + ) === [], self::rangeSliceTypes($requestedAttrs), $schema, ); diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/GeneratedEntryResponder.php index c83249e50..22d412752 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 45a906fb4..4c0101ea2 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 328c91557..a9204371d 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 6429bbc4b..8320acdd8 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 0de16cef7..dd003c867 100644 --- a/src/FreeDSx/Ldap/Schema/Schema.php +++ b/src/FreeDSx/Ldap/Schema/Schema.php @@ -43,7 +43,7 @@ final class Schema private const MAX_REMEMBERED_DESCRIPTIONS = 1024; /** - * Pairs grow with the stored descriptions times the names filters and selections ask for, so they get more room. + * 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; @@ -95,12 +95,14 @@ final class Schema private array $operationalDescriptions = []; /** - * Whether each pair of descriptions seen is a subtype pair, keyed by both joined on a NUL and dropped with the types. + * Whether each pair of descriptions seen is a subtype pair, by description then by the one it was tested against. * - * @var array + * @var array> */ private array $subtypeDescriptions = []; + private int $subtypePairs = 0; + /** * Bound to this schema, so it is built once rather than per lookup. */ @@ -132,6 +134,7 @@ public function addAttributeType(AttributeType $type): static $this->confidentialDescriptions = []; $this->operationalDescriptions = []; $this->subtypeDescriptions = []; + $this->subtypePairs = 0; return $this; } @@ -311,18 +314,17 @@ public function isDescriptionSubtypeOf( string $description, string $ofDescription, ): bool { - $key = $description . "\0" . $ofDescription; - - if (isset($this->subtypeDescriptions[$key])) { - return $this->subtypeDescriptions[$key]; + if (isset($this->subtypeDescriptions[$description][$ofDescription])) { + return $this->subtypeDescriptions[$description][$ofDescription]; } $isSubtype = $this->decideDescriptionSubtypeOf( $description, $ofDescription, ); - if (count($this->subtypeDescriptions) < self::MAX_REMEMBERED_SUBTYPE_PAIRS) { - $this->subtypeDescriptions[$key] = $isSubtype; + if ($this->subtypePairs < self::MAX_REMEMBERED_SUBTYPE_PAIRS) { + $this->subtypeDescriptions[$description][$ofDescription] = $isSubtype; + $this->subtypePairs++; } return $isSubtype; @@ -577,9 +579,6 @@ private static function primarySpelling( : $primary; } - /** - * Whether every option the wanted description carries is also carried by the one being tested. - */ private function decideDescriptionSubtypeOf( string $description, string $ofDescription, @@ -597,6 +596,9 @@ private function decideDescriptionSubtypeOf( ); } + /** + * Whether every option the wanted description carries is also carried by the one being tested. + */ private static function optionsCover( string $description, string $ofDescription, diff --git a/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php b/src/FreeDSx/Ldap/Server/AccessControl/AccessControlInterface.php index 3eade7a6a..174ccdafc 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 b75a8235b..d7a042dc0 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 041516faa..4a05d4d68 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 b4a1daad9..bfe47abe5 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 000000000..6b64bb1b0 --- /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 000000000..6593f8da2 --- /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 000000000..f1c0dbe3c --- /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 000000000..c3cbbafad --- /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 000000000..3a15df447 --- /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 000000000..7d40c4f38 --- /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 000000000..291658000 --- /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 5b14f864e..a0f4685ba 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 a89184b6c..06f7e5709 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 86522c8d2..10be4e3f1 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 164f92df8..367178e1d 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 43e5c4385..5db3046a6 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 bfb2a5344..19bba1135 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 4fc9c1f50..f8610b819 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 000000000..33781a0b7 --- /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 7e8508fa9..2a47443fe 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 3811fc16a..3096b38dd 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 43d310833..e4b020db0 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 126a89b50..4a37f7029 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/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php b/tests/unit/Protocol/ServerProtocolHandler/Admission/SearchResultGateTest.php new file mode 100644 index 000000000..4cad0308e --- /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/ServerCompareHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerCompareHandlerTest.php index 50328722d..4934c2b18 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 8e2881489..b9f709318 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 797dfa72f..16850490c 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 7d071a409..599d850c0 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 42d544151..409621119 100644 --- a/tests/unit/Schema/SchemaTest.php +++ b/tests/unit/Schema/SchemaTest.php @@ -779,6 +779,24 @@ public function test_a_subtype_answer_follows_a_type_redefined_after_it_was_aske )); } + 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( diff --git a/tests/unit/Server/AccessControl/AclRuleIndexTest.php b/tests/unit/Server/AccessControl/AclRuleIndexTest.php index ae4712f7b..fb37ae599 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 c27b3e40b..760d7a341 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 57fdd053f..a06927885 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 000000000..5d2994021 --- /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 000000000..b2a5089ae --- /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 4918b84ac..f32a9b1b6 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 84f101fec..1bd64b811 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 cd5795165..d12a379d0 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 cae89b8f4..8744c265e 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')),