[alexlovelltroy]

Description

This pull request introduces strict JWT-based authentication for all /admin/ API routes, ensuring that administrative operations are protected and unavailable if JWT verification cannot be configured or fails. Node bootstrap and non-admin endpoints remain accessible without tokens. The PR also updates documentation and adds comprehensive tests to validate the new authentication boundary and failure modes.

Admin authentication and API protection:

• All /admin/ APIs now require a bearer JWT, enforced via a middleware (adminJWTMiddleware). If the JWT verifier is unavailable or misconfigured (e.g., missing or empty JWKS), admin endpoints return HTTP 503, but non-admin endpoints (e.g., node bootstrap) remain accessible. [1] [2] [3] [4]
• The JWT verifier requires specific claims (sub, iss, aud) and only accepts tokens from the Authorization header (not cookies or query parameters), preventing ambient credential use. [1] [2]

Testing and validation:

• Adds comprehensive tests for admin JWT authentication, including coverage for valid/invalid tokens, claim requirements, JWKS fetch failures, and ensuring that only admin endpoints are gated by JWT auth. Tests also verify that node bootstrap remains tokenless and that state is not mutated by rejected admin requests. [1] [2]

Documentation and usage guidance:

• Updates README.md to document the new requirement for bearer JWTs on all /admin/ endpoints, including example curl commands with the Authorization header and a pointer to detailed admin authentication and upgrade guidance. [1] [2] [3] [4] [5]
• Clarifies CLI flag descriptions and startup messages to reflect that admin APIs are unavailable without a JWKS URL. [1] [2]

Other improvements:

• Ensures HTTP clients used for JWKS fetching have a timeout and robust error handling for non-200 responses.

These changes collectively harden the security boundary for administrative operations, provide clear operator guidance, and ensure robust, test-driven enforcement of the new authentication requirements.

Checklist

• My code follows the style guidelines of this project
• I have added/updated comments where needed
• I have added tests that prove my fix is effective or my feature works
• I have run make test (or equivalent) locally and all tests pass
• I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
• DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
• REUSE Compliance:
  • Each new/modified source file has SPDX copyright and license headers
  • Any non-commentable files include a <filename>.license sidecar
  • All referenced licenses are present in the LICENSES/ directory

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Dependency update
• Build system/CI

---

• I agree to follow the Code of Conduct.

[synackd]

Reviewed the authentication boundary, failure behavior, documentation, and dependency changes. The middleware placement and WireGuard private-key log removal look correct. The comments below cover the remaining security policy question and client/documentation gaps.

Upstream CI passes, including race tests and vulnerability checks. Local tests passed for the packages that ran, including pkg/wgtunnel, but the full suite could not complete because the FlatBuffers dependency download failed, including after retrying with network access.

[synackd]

This middleware verifies the signature and requires claim presence, but does not check expected issuer/audience values, require an admin role or scope, or require expiration. When the configured JWKS is shared with ordinary users or other services, their tokens can therefore authorize all admin operations, including configuration writes and impersonation.

The new test explicitly expects success after setting aud = "another-service" and deleting exp. The documentation acknowledges this, but safe deployment remains dependent on keys/issuance policy dedicated to administrative authority.

Would it be appropriate to include an explicit admin token policy in this PR? My read is that this PR focuses first and foremost on protecting the endpoints (which is definitely the priority), but we should have a follow-up issue/PR to do this if not here. To address this, we would need to configure and enforce the expected issuer and audience, require expiration, and define the admin role/scope policy, with rejection tests for unrelated-service and non-admin tokens.

[synackd]

This example now sends a bearer token, but the bundled demo scripts and demo documentation still send anonymous admin requests. For example, demo/set_cluster_defaults.sh omits the Authorization header, as do the other admin scripts and examples in demo/Demo.md. Those requests now return 401 when verification is configured, or 503 when JWKS is unavailable. Plain curl can still exit successfully for either HTTP error, making a failed demo setup easy to miss.

Could we update those scripts and examples with bearer headers, document the JWKS/token setup, and use --fail-with-body so failed administration produces a failing command? For example:

curl --fail-with-body -X POST "$CLOUD_INIT_URL/admin/cluster-defaults/" \
  -H "Authorization: Bearer ${CLOUD_INIT_ADMIN_TOKEN}" \
  -H "Content-Type: application/json" \
  --data-binary @cluster-defaults.json

[synackd]

This line applies JWT authentication to all registered admin operations, but the admin handler annotations and generated Swagger still omit authentication and the new 401/503 responses. Consumers of /openapi.json, including generated clients, therefore have no machine-readable indication that these operations require a bearer token.

Could we define an Authorization-header security scheme, apply it to the admin operations, document 401/503, and regenerate the checked-in Swagger files, while keeping node bootstrap operations outside that security requirement?

[synackd]

This passage directs operators to "the release endpoint validator" and a jwt: required site policy, but identifies no tool, schema, command, or link. I could not find an implementation of that policy in this repository, so an operator cannot act on this guidance without context from the original plan.

The instructions about how "this patch must not be described", the statement about qualifying a released ochami binary, and the test comment beginning "This PR requires" also describe review scope instead of lasting behavior.

Could we replace that wording with concrete behavior and operator checks, and either remove the validator/policy reference or link and explain the external tooling if that guidance is intentional?