forked from fa/breadcrumb-the-shire
fs
01f5265eff
Cleanup follow-up to commit 9ec10f5, which removed the unused
actionFragmentContext aggregator and its building block. Two
architecture tests still mentioned the removed aggregator in their
allowlists and recognizer regexes — patterns that now match an empty
set, harmless but misleading.
* ActionContextCsrfPairingContractTest: drop 'actionFragmentContext'
from the AGGREGATORS constant, update the docblock to list only the
two remaining aggregators, and rewrite the GET-only-allowlist
comment to no longer reference the fragment-specific case (the
guard itself stays — any future GET-only aggregator caller would
still hit it).
* DetailDrawerFragmentContractTest: drop the third alternative from
the aggregator-recognizer regex inside extractTopLevelAbility, and
trim two comments accordingly.
The historical documentation in ActionContextHelperContractTest is
deliberately kept — those comments explain to future readers why the
test only freezes 5 building blocks and 2 aggregators (instead of
the original 6/3) and why the CSRF-warning expectation is 2 instead
of 3. That is contextual documentation, not stale references.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
302 lines
12 KiB
PHP
302 lines
12 KiB
PHP
<?php
|
|
|
|
namespace MintyPHP\Tests\Architecture;
|
|
|
|
use PHPUnit\Framework\TestCase;
|
|
|
|
/**
|
|
* Every `initDetailDrawer({ fetchUrl })` consumer must expose a matching
|
|
* fragment endpoint following the convention:
|
|
* <route>-fragment($id).php ← action
|
|
* <route>-fragment(none).phtml ← view (no-layout)
|
|
*
|
|
* This test scans JS files for `initDetailDrawer` calls, extracts the fragment
|
|
* path from the fetchUrl template literal, and verifies both files exist.
|
|
*/
|
|
class DetailDrawerFragmentContractTest extends TestCase
|
|
{
|
|
use ProjectFileAssertionSupport;
|
|
|
|
public function testEveryDrawerConsumerHasMatchingFragmentEndpoint(): void
|
|
{
|
|
$root = $this->projectRootPath();
|
|
$scanRoots = ['web/js', 'modules'];
|
|
|
|
$consumers = [];
|
|
|
|
foreach ($scanRoots as $scanRoot) {
|
|
$basePath = $root . '/' . $scanRoot;
|
|
if (!is_dir($basePath)) {
|
|
continue;
|
|
}
|
|
|
|
$iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($basePath, \FilesystemIterator::SKIP_DOTS));
|
|
/** @var \SplFileInfo $file */
|
|
foreach ($iterator as $file) {
|
|
if (!$file->isFile() || $file->getExtension() !== 'js') {
|
|
continue;
|
|
}
|
|
// Exclude the drawer implementation itself.
|
|
$relativePath = str_replace($root . '/', '', $file->getPathname());
|
|
if ($relativePath === 'web/js/components/app-detail-drawer.js') {
|
|
continue;
|
|
}
|
|
|
|
$content = file_get_contents($file->getPathname());
|
|
if ($content === false || !str_contains($content, 'initDetailDrawer(')) {
|
|
continue;
|
|
}
|
|
|
|
// Find `fetchUrl: (uuid) => new URL(`<path>/${uuid}`, ...)` — extract <path>.
|
|
// The path must contain "-fragment" per the convention.
|
|
if (preg_match_all(
|
|
'/fetchUrl\s*:\s*\([^)]*\)\s*=>\s*new\s+URL\s*\(\s*`([^`${]+)\$\{[^}]+\}[^`]*`/',
|
|
$content,
|
|
$matches
|
|
)) {
|
|
foreach ($matches[1] as $prefix) {
|
|
$prefix = rtrim($prefix, '/');
|
|
$consumers[$prefix] = $relativePath;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
$this->assertNotEmpty(
|
|
$consumers,
|
|
'No initDetailDrawer fetchUrl patterns found. Either the convention changed or the regex is stale.'
|
|
);
|
|
|
|
$violations = [];
|
|
|
|
foreach ($consumers as $fragmentPath => $consumerFile) {
|
|
if (!str_contains($fragmentPath, '-fragment')) {
|
|
$violations[] = sprintf(
|
|
"Fragment path does not follow *-fragment convention: %s (in %s)",
|
|
$fragmentPath,
|
|
$consumerFile
|
|
);
|
|
continue;
|
|
}
|
|
|
|
[$actionFound, $viewFound, $searchedDirs] = $this->locateFragmentFiles($fragmentPath);
|
|
|
|
if (!$actionFound) {
|
|
$violations[] = sprintf(
|
|
"Missing action file for fragment path '%s'. Expected somewhere like:\n pages/%s(\$id).php\n modules/*/pages/%s(\$id).php\nSearched in: %s\n(Consumer: %s)",
|
|
$fragmentPath,
|
|
$fragmentPath,
|
|
$fragmentPath,
|
|
implode(', ', $searchedDirs),
|
|
$consumerFile
|
|
);
|
|
}
|
|
if (!$viewFound) {
|
|
$violations[] = sprintf(
|
|
"Missing (none) view file for fragment path '%s'. Expected:\n %s(none).phtml\nSearched in: %s\n(Consumer: %s)",
|
|
$fragmentPath,
|
|
$fragmentPath,
|
|
implode(', ', $searchedDirs),
|
|
$consumerFile
|
|
);
|
|
}
|
|
}
|
|
|
|
$this->assertSame(
|
|
[],
|
|
$violations,
|
|
"Detail-drawer fragment-contract violations:\n" . implode("\n", $violations)
|
|
);
|
|
}
|
|
|
|
/**
|
|
* AuthZ-Parity: a drawer fragment must enforce the same authorization as
|
|
* its full-page counterpart (CLAUDE.md: "Must enforce auth + scope exactly
|
|
* like the full-page view"). This test extracts authorize / requireAbility
|
|
* arguments from each fragment and compares them to the matched counterpart.
|
|
*
|
|
* Allowlist of semantic equivalences (verified manually against codebase):
|
|
* - admin/users/view-fragment ↔ admin/users/edit
|
|
* Reason: there is no view($id).php under pages/admin/users/, so the
|
|
* full-page detail action is edit. The fragment uses ABILITY_VIEW;
|
|
* edit uses ABILITY_EDIT_CONTEXT — semantically the fragment is the
|
|
* read-only subset of the edit context. Documented anomaly.
|
|
* - addressbook/view-fragment ↔ addressbook/view
|
|
* Reason: both call requireAbilityOrForbidden(ABILITY_VIEW).
|
|
* - helpdesk/ticket-fragment ↔ helpdesk/ticket
|
|
* Reason: both call requireAbilityOrForbidden(ABILITY_ACCESS).
|
|
*
|
|
* The test fails explicitly if an authorize/requireAbility call is nested
|
|
* inside an if/else block (not statically extractable) — a parity check
|
|
* cannot be made automatically in that case.
|
|
*/
|
|
public function testFragmentAuthzMatchesFullPage(): void
|
|
{
|
|
$root = $this->projectRootPath();
|
|
|
|
// counterpart map: fragment-relative-path => full-page-relative-path
|
|
$pairs = [
|
|
'pages/admin/users/view-fragment($id).php' => 'pages/admin/users/edit($id).php',
|
|
'modules/addressbook/pages/address-book/view-fragment($id).php' => 'modules/addressbook/pages/address-book/view($id).php',
|
|
'modules/helpdesk/pages/helpdesk/ticket-fragment($id).php' => 'modules/helpdesk/pages/helpdesk/ticket($id).php',
|
|
];
|
|
|
|
// semantic-equivalence allowlist — full-page ability => fragment ability accepted as parity
|
|
$equivalents = [
|
|
'UserAuthorizationPolicy::ABILITY_ADMIN_USERS_EDIT_CONTEXT' => ['UserAuthorizationPolicy::ABILITY_ADMIN_USERS_VIEW'],
|
|
];
|
|
|
|
$violations = [];
|
|
|
|
foreach ($pairs as $fragmentRel => $fullPageRel) {
|
|
$fragmentPath = $root . '/' . $fragmentRel;
|
|
$fullPagePath = $root . '/' . $fullPageRel;
|
|
|
|
$this->assertFileExists($fragmentPath, "Fragment file missing: {$fragmentRel}");
|
|
$this->assertFileExists($fullPagePath, "Full-page file missing: {$fullPageRel}");
|
|
|
|
$fragmentAbility = $this->extractTopLevelAbility((string) file_get_contents($fragmentPath), $fragmentRel, $violations);
|
|
$fullPageAbility = $this->extractTopLevelAbility((string) file_get_contents($fullPagePath), $fullPageRel, $violations);
|
|
|
|
if ($fragmentAbility === null || $fullPageAbility === null) {
|
|
continue; // already recorded as violation
|
|
}
|
|
|
|
// Strip leading namespace separators for comparison resilience.
|
|
$fragNorm = ltrim($fragmentAbility, '\\');
|
|
$fullNorm = ltrim($fullPageAbility, '\\');
|
|
|
|
if ($fragNorm === $fullNorm) {
|
|
continue;
|
|
}
|
|
|
|
$accepted = $equivalents[$fullNorm] ?? [];
|
|
if (in_array($fragNorm, $accepted, true)) {
|
|
continue;
|
|
}
|
|
|
|
$violations[] = sprintf(
|
|
"AuthZ parity mismatch: %s requires '%s' but full-page %s requires '%s' (no documented allowlist entry).",
|
|
$fragmentRel,
|
|
$fragNorm,
|
|
$fullPageRel,
|
|
$fullNorm
|
|
);
|
|
}
|
|
|
|
$this->assertSame([], $violations, "Drawer-fragment AuthZ-parity violations:\n" . implode("\n", $violations));
|
|
}
|
|
|
|
/**
|
|
* Extract the first top-level (non-nested) authorize / requireAbility ability
|
|
* argument from a PHP source string. Returns null and records a violation
|
|
* when no statically extractable call is found.
|
|
*
|
|
* Also recognizes the actionEditContext / actionCreateContext aggregator
|
|
* pattern: if the file calls one of those aggregators at top level, we
|
|
* look for an 'abilityKey' => SomePolicy::CONST entry inside the args
|
|
* array and treat it as the top-level ability.
|
|
*/
|
|
private function extractTopLevelAbility(string $source, string $fileRel, array &$violations): ?string
|
|
{
|
|
// Strip block comments and line comments to avoid false matches.
|
|
$stripped = preg_replace('/\/\*.*?\*\//s', '', $source) ?? $source;
|
|
$stripped = preg_replace('/\/\/[^\n]*/', '', $stripped) ?? $stripped;
|
|
|
|
$lines = explode("\n", $stripped);
|
|
$depth = 0;
|
|
$hasTopLevelMatch = false;
|
|
$hasNestedMatch = false;
|
|
$extracted = null;
|
|
$insideTopLevelAggregator = false;
|
|
|
|
foreach ($lines as $line) {
|
|
// Update brace depth AFTER matching this line so a `{` on the same
|
|
// line as an authorize call (rare) still counts as top-level.
|
|
$matchPattern = '/(?:Guard::requireAbility(?:OrForbidden|DecisionOrForbidden)?|->authorize|::authorize)\s*\(\s*([A-Za-z_\\\\][A-Za-z0-9_\\\\:]*)/';
|
|
if (preg_match($matchPattern, $line, $m)) {
|
|
if ($depth === 0) {
|
|
if (!$hasTopLevelMatch) {
|
|
$extracted = $m[1];
|
|
$hasTopLevelMatch = true;
|
|
}
|
|
} else {
|
|
$hasNestedMatch = true;
|
|
}
|
|
}
|
|
// actionEditContext / actionCreateContext aggregator pattern.
|
|
// The aggregator args use a [ ... ] array literal (not a { ... } block),
|
|
// so brace-depth tracking does not bracket them; we track the array
|
|
// opening with a separate "inside aggregator args" sticky flag instead.
|
|
if (!$insideTopLevelAggregator && $depth === 0 && preg_match('/actionEditContext\s*\(|actionCreateContext\s*\(/', $line)) {
|
|
$insideTopLevelAggregator = true;
|
|
}
|
|
if ($insideTopLevelAggregator && !$hasTopLevelMatch && preg_match('/[\'"]abilityKey[\'"]\s*=>\s*([A-Za-z_\\\\][A-Za-z0-9_\\\\:]*)/', $line, $am)) {
|
|
$extracted = $am[1];
|
|
$hasTopLevelMatch = true;
|
|
$insideTopLevelAggregator = false;
|
|
}
|
|
$opens = substr_count($line, '{');
|
|
$closes = substr_count($line, '}');
|
|
$depth += $opens - $closes;
|
|
if ($depth < 0) {
|
|
$depth = 0;
|
|
}
|
|
}
|
|
|
|
if (!$hasTopLevelMatch) {
|
|
if ($hasNestedMatch) {
|
|
$violations[] = sprintf(
|
|
"Cannot verify parity when authorization is conditional — review manually: %s",
|
|
$fileRel
|
|
);
|
|
} else {
|
|
$violations[] = sprintf(
|
|
"No authorize/requireAbility call found in %s — fragment must enforce auth.",
|
|
$fileRel
|
|
);
|
|
}
|
|
return null;
|
|
}
|
|
|
|
return $extracted;
|
|
}
|
|
|
|
/**
|
|
* @return array{0: bool, 1: bool, 2: list<string>} [actionFound, viewFound, searchedDirs]
|
|
*/
|
|
private function locateFragmentFiles(string $fragmentPath): array
|
|
{
|
|
$root = $this->projectRootPath();
|
|
$searchBases = [$root . '/pages'];
|
|
$modulesDir = $root . '/modules';
|
|
if (is_dir($modulesDir)) {
|
|
foreach (scandir($modulesDir) ?: [] as $moduleEntry) {
|
|
if ($moduleEntry === '.' || $moduleEntry === '..') {
|
|
continue;
|
|
}
|
|
$modulePages = $modulesDir . '/' . $moduleEntry . '/pages';
|
|
if (is_dir($modulePages)) {
|
|
$searchBases[] = $modulePages;
|
|
}
|
|
}
|
|
}
|
|
|
|
$actionFound = false;
|
|
$viewFound = false;
|
|
|
|
foreach ($searchBases as $base) {
|
|
$actionCandidate = $base . '/' . $fragmentPath . '($id).php';
|
|
$viewCandidate = $base . '/' . $fragmentPath . '(none).phtml';
|
|
if (is_file($actionCandidate)) {
|
|
$actionFound = true;
|
|
}
|
|
if (is_file($viewCandidate)) {
|
|
$viewFound = true;
|
|
}
|
|
}
|
|
|
|
return [$actionFound, $viewFound, $searchBases];
|
|
}
|
|
}
|