Gate page reads through a single PageReadAuthorizer

[JeroenDeDauw]

Follows-up to #1058

The design half of an AI review of #1058. #1058 is correct — this is about the shape it leaves behind, so it is entirely advisory and behaviour-preserving. Not a bug fix, and independent of #1071 (which carries that review's two blocking fixes; the two branches touch DatabaseSchemaNameLookupTest and will want a trivial conflict resolution whichever lands second).

The problem

The read gate is one decision — may this Authority read this page? — written five times. All five call authority->authorizeRead( 'read', $title ); four follow it with the same five-line denial log:

Site	Log copy?
AuthorityBasedSubjectAuthorizer	yes
CachingSchemaLookup	yes
CachingMappingLookup	yes
WikiPageLayoutLookup	yes
DatabaseSchemaNameLookup	no (deliberately diverged)

Three of those classes grew a LoggerInterface used only for that block. Four test classes each hold their own testGateUsesBindingAuthorizeRead, and three carry the comment // Mirrors AuthorityBasedSubjectAuthorizerTest::testDeniedReadIsLogged.

This is not hypothetical drift: it already happened once. Both caching lookups used probablyCan — an advisory hint verb — for a binding gate, and #1058 has to fix both by hand. The PR repairs the drift and re-creates the conditions for it, at five sites instead of two.

Why the name was the blocker

SubjectReadAuthorizer contains no Subject semantics — it is newTitle() plus authorizeRead — and two of its five callers gate an RDF export (ExportPageRdfApi) and a revision's page (GetSubjectApi), not a Subject. Schemas, Layouts and Mappings all live on pages and need the identical check, but a thing called SubjectReadAuthorizer cannot plausibly gate a Schema page — so four sites hand-rolled it instead.

The interface's existence is fine (3 Application callers, mirroring SubjectWriteAuthorizer). The name and the host class were the defects.

What this does

• SubjectReadAuthorizer → PageReadAuthorizer, moved off AuthorityBasedSubjectAuthorizer into its own AuthorityBasedPageReadAuthorizer, which owns the denial logging.
• Two entry points, because callers genuinely hold different keys: authorizeReadByPageId for the Application queries (which resolve a Subject to a page via the graph) and authorizeReadByPageTitle for the Persistence lookups (which already resolved a Title for their own content fetch, so keying by id would cost a pointless second page lookup). SubjectContentRepository sets the precedent for keying one page two ways.
• All five sites route through it. AuthorityBasedSubjectAuthorizer goes back to two interfaces and drops its logger; the three lookups drop theirs; two drop their Authority.
• The verb and the denial log are pinned once, in the authorizer's own test. The four lookup tests keep a delegation test each.
• Second commit: GetPageSubjectsQuery::pageIsReadable is inlined (single call site; its own docblock says the case it guards cannot happen today), and the survivor in GetSubjectQuery becomes pageIsReadableOrUnresolved — there null does not mean "readable" but "no page to gate". Two same-named helpers with opposite meanings, defended by cross-referencing docblocks, are gone.

Net: 5 gate implementations → 1, 4 log copies → 1, 4 LoggerInterface dependencies → 0, 5 verb tests → 1.

One intentional behaviour change

DatabaseSchemaNameLookup now logs denials like every other site; it previously stayed silent on the grounds that enumeration would log proportionally to list size. That reasoning was already inconsistent — the expand=relations loops in both queries log per referenced subject via the authorizer — and the log exists so a denial shaped as absence stays diagnosable, which applies just as much to "why is my Schema missing from the list?". Easy to revert if you disagree; it is the only non-cosmetic delta.

Verification

Behaviour-preserving, checked rather than assumed. make cs clean; full suite green (1690 tests). Re-ran the live denial matrix on a dev wiki under a getUserPermissionsErrorsExpensive ACL (the hook probablyCan skips, so this also proves the RIGOR_FULL gate survived): page/{id}/subjects → empty map, subject/{id} → {"subject":null}, rdf → 404, validate → 404, schema/{name} → {"schema":null}, schema-names → [], expand=relations → restricted target omitted, readable control page unaffected. All byte-identical to #1058 before the refactor.

_{AI-authored — Claude Code, Opus 4.8 (max); design verdict from a /pr-review of #1058, implemented at @JeroenDeDauw's request, no revisions; diff not yet human-reviewed; make cs clean, full PHPUnit suite green locally, and the denial matrix re-verified live against a dev wiki under an ACL hook.}

[JeroenDeDauw]

LGTM