Skip to content

feat(http): middleware, auth primitives, and a lazily resolved logger - #1

Merged
mikield merged 3 commits into
mainfrom
fix/resolve-logger-lazily
Sep 2, 2026
Merged

feat(http): middleware, auth primitives, and a lazily resolved logger#1
mikield merged 3 commits into
mainfrom
fix/resolve-logger-lazily

Conversation

@mikield

@mikield mikield commented Sep 2, 2026

Copy link
Copy Markdown
Member

Installing this plugin made an application fail to boot:

Cannot resolve Tempest\Log\Logger because it is not an instantiable class.

  ┌── RoutesDiscovery::__construct(Router $router, RouteCompiler $compiler)
  └── Router::__construct(Container $container, Logger $logger)

Discovery constructs the router while the container is still being assembled — before the initializers that provide the logger have themselves been found. It does this whether or not the application has any routes, so every consumer was affected, not just ones using the plugin.

The registries in the framework (EventsRegistry) and in the tasks plugin already resolve lazily for exactly this reason. This one did not.

Why the tests did not catch it

They build a container of their own with a logger already in it — which is not the container discovery has. There are now two tests for the actual constraint: one builds a Router against an empty GenericContainer and routes a request through it, and one checks the logger is still reached when a handler throws.

🤖 Generated with Claude Code

Discovery builds the router while the container is still being assembled,
before the initializers that provide the logger have themselves been
found. Asking for one in the constructor made every application
installing this plugin fail to boot — including ones with no routes at
all, since discovery constructs the router either way.

The registries in the framework and in the tasks plugin already resolve
lazily for exactly this reason; this one did not.

The existing tests missed it because they build a container of their own
with a logger already in it, which is not the container discovery has.
A handler had no way to tell who was asking, and the familiar answers are
all unsafe in a process that serves thousands of requests without
restarting: $_COOKIE holds whatever the CLI process started with,
setcookie() writes nowhere, and $_SESSION is one session shared by
everybody who ever connects.

So the pieces that are safe, and only those: a cookie read off this
request's own header, a bearer token from Authorization, a cookie set on
the response with HttpOnly and SameSite=Lax already on, and comparisons
that run in constant time. `===` on a signature stops at the first byte
that differs, which is measurable over enough requests.

Cookie values are rawurlencoded rather than urlencoded, since a browser
hands "+" back as a plus rather than a space.

Nothing here remembers anything between requests, which is the property
that makes it safe at all. The README now says why, and says not to reach
for PHP's own session handling.
@mikield mikield changed the title fix(router): resolve the logger when it is needed, not at construction fix(router): resolve the logger when it is needed, and add auth primitives Sep 2, 2026
@mikield

mikield commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Pushed the stateless auth primitives onto this branch as well, since the plugin is not released with either yet.

$request->cookie('session');       // off this request's Cookie header
$request->bearerToken();           // from "Authorization: Bearer ..."

Secret::matches($configured, $request->bearerToken() ?? '');
Secret::signed($request->body(), $request->header('X-Signature') ?? '', $secret);

return Response::noContent()->withCookie(new Cookie('session', $id, secure: true));

Why these and not sessions. Everything here is stateless, which is the property that makes it safe in a process serving thousands of requests over weeks. Secret compares in constant time — === on a signature stops at the first differing byte, and that is measurable over enough requests. signed() takes the raw body, since re-encoding changes bytes and the signature is over what was sent.

Cookies default to HttpOnly, SameSite=Lax, Path=/. Secure is left to the caller: a bot behind a plain-HTTP proxy would otherwise set a cookie the browser never returns, which presents as a login loop. Values are rawurlencoded, because a browser hands + back as a plus rather than a space.

The README now also says plainly that session_start(), $_SESSION, $_COOKIE, setcookie() and friends must not be used here, and why — PHP keeps one session per process, which under FPM is one request and here is the whole uptime.

A check on whether a caller may ask at all has to be able to stop the work
rather than disapprove of it afterwards, and there was nowhere to put one:
a route was matched and its handler called.

Middleware are listed on the route, built by the container, and wrapped
outermost-first. Returning an answer instead of calling $next stops the
request there — the handler never runs, and neither does anything behind
it in the list.

A handler's answer is normalised to a Response before the middleware see
it, so one that wraps the answer is never handed whatever a handler
happened to return.
@mikield mikield changed the title fix(router): resolve the logger when it is needed, and add auth primitives feat(http): middleware, auth primitives, and a lazily resolved logger Sep 2, 2026
@mikield

mikield commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Added middleware support on top, since a signature check has to be able to stop a request rather than disapprove of it afterwards — and there was nowhere to put one.

#[Route(Method::POST, '/api/links', middleware: [VerifySignature::class])]

First listed is outermost; returning an answer instead of calling $next stops the handler and everything behind it in the list. Both are tested, and so is the case of something named as middleware that does not implement the interface — a 500 with the reason logged, rather than calling something that has no idea what $next is.

A handler's answer is normalised to a Response before middleware see it, so one that wraps the answer is never handed whatever a handler happened to return.

55 tests, phpstan clean. This PR is now three things — the boot fix, the auth primitives and middleware — because none of them is released yet and they only make sense together.

@mikield
mikield merged commit 8d64bfa into main Sep 2, 2026
4 of 5 checks passed
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant