Apple browser OAuth internal review follow-up
Response to the internal review of the three local Apple browser OAuth commits.
All changes remain in the server silo except documentation and the mechanically generated web API schema.
Findings
- Bind URL reloads and prefetches: generate the immutable nonce when inserting the CLI bind request, before printing its URL.
GET and HEAD only read and redirect; repeated requests return the same state and nonce.
Completion and cancellation remain single-use.
Legacy pending rows without a nonce require a fresh CLI attempt, not a migration.
- Apple cancellation: recognize
user_cancelled_authorize for Apple, as documented in Apple’s authorization endpoint reference.
Unknown provider errors receive neutral retry guidance, not an outage diagnosis.
- Callback hardening: the HTML/no-store/no-referrer response layer now wraps the rate limiter as well as the handler.
Regression tests cover body-size rejection, per-state and per-IP throttling, and a stalled request body.
- Bearer warning amplification: one fixed-size, process-wide warning slot is shared by HTTP and WebSocket rejection paths.
It emits at most one warning per 30 seconds; additional events are debug-only.
A concurrent test covers contention and reopening the warning window.
- OpenAPI: document GET query parameters and POST form input separately, both with HTML failure responses.
Regenerate the spec and web schema; a dedicated test asserts both methods and media types.
ADR 0054 records the existing GET error-envelope change.
- Retry messaging: the CLI reports cancellation, failure, or expiry without guessing which occurred, and explicitly requests a new bind attempt.
Provider-error text also specifies a new attempt; an upstream exchange cannot safely be resumed after its code may have been consumed.
Smaller findings
- Key-file diagnostics: include
io::ErrorKind and a relative-path/working-directory hint.
Keep paths redacted deliberately: configuration can accidentally put sensitive material in a path, and paths are not needed to distinguish missing files from permission errors.
- Callback URL construction: use one pure URL helper shared by startup validation and
OAuthRuntime, without allocating a temporary runtime or limiters.
- Bind nonce invariant: carry the validated nonce in the
CallbackFlow::Bind variant; remove the empty-string fallback.
- Diagnostic tests: assert every constructed OIDC error category, not a subset.
- Cross-crate fixture: move the shared in-memory EC key generator to the dev-only
axon-test-support crate; remove the cross-crate test-file include.
- Transaction cleanup: explicitly roll back the missing-identity branch of unbind.
- Bootstrap separation: reject the reserved bootstrap client in ordinary login even if operator configuration registers it; add a regression test.
PR review follow-up
- Parse form Content-Type using the same MIME rules as the handler; mixed case and charset parameters cannot bypass per-key throttling.
Callback and token endpoint regression tests exercise the router and limiter together.
- Explicit callback router metadata selects the state field and callback body cap; URL suffixes no longer select security policy.
- Redact Google/Microsoft client secrets in direct and nested configuration Debug output, with sentinel tests.
- Share redirect-versus-handoff dispatch between success and failure, delegate cancellation vocabulary to
OidcProvider, and keep failure reason/error/description in one exhaustive table.
- Initialize independent Apple/Google/Microsoft providers concurrently and clone one shared HTTP client for their connection pools.
- Defer Windows key-file ACL validation and any genuinely shared secret-file primitives to issue 482.
init::write_secure writes and mutates permissions; it is not a bounded reader or an ACL validator and must not be called on operator-owned signing keys during startup.
The operator guide now states the Windows limitation explicitly.
Verification commands
Use the commands in the verification guide, plus:
cargo test -p axon-api --lib auth::tests
cargo test -p axon-api --lib routes::oauth
cargo test -p axon-server oauth_runtime_tests
pnpm --dir clients/web check:api
Live cancellation and bootstrap acceptance remain operator follow-ups; unit and mock-provider tests do not replace those checks.