feat: HTTP layer with strict-server handlers and JWT middleware

[koinsaari]

Summary

• server.go — NewServer wires the generated StrictServerInterface into a middleware chain: RequestID → Logging → gen.Handler(strict)
• middleware.go — RequestID and Logging middleware; typed context keys for request ID, logger, userID, jfUserID
• handlers_auth.go — full StrictServerInterface implementation mapping auth.Service errors to typed JSON error responses using generated constants (gen.InvalidCredentials, gen.AccountLocked, etc.)
• errors.go — writeError helper using gen.ErrorErrorCode constants
• middleware_test.go — JWT middleware unit tests + compile-time interface check (var _ gen.StrictServerInterface = (*Server)(nil))
• auth/jwt.go — IssueAccessTokenForTest test seam used by middleware tests

JWT auth runs as a StrictMiddlewareFunc applied selectively to protected operations (postAuthLogout, postAuthLogoutAll) — not a blanket route group.

Test plan

• CI passes (test, lint, codegen-check, docker-build, gitleaks)
• go test -race ./internal/http/... passes locally

🤖 Generated with Claude Code

[claude]

Claude finished @koinsaari's task in 2m 23s —— View job

---

Code Review

• Read REVIEW.md and CLAUDE.md
• Read changed files and full diff
• Analyze for Important findings
• Post review findings
• Set commit status (blocked by permissions — run manually below)

---

Summary

Two blocking issues found. No import cycle. JWT keyfunc correctly validates signing method before returning the key (if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok in internal/auth/jwt.go:42).

---

🔴 Important — Wrong error code for missing Authorization header

internal/http/server.go, line ~57

writeError(w, r, stdhttp.StatusUnauthorized, gen.TokenExpired, "missing bearer token")

When no Authorization: Bearer header is present at all, the response carries gen.TokenExpired. A client receiving TokenExpired will assume it needs to re-authenticate / refresh an existing token, not that it failed to send a token in the first place. The correct code here is gen.TokenInvalid — the token is absent/malformed, not expired.

Fix this →

---

🔴 Important — No handler-level tests for new routes

internal/http/handlers_auth.go adds seven handler implementations (GetHealthz, PostAuthLogin, PostAuthRefresh, PostAuthLogout, PostAuthLogoutAll, PostAuthQuickConnectStart, PostAuthQuickConnectPoll), but middleware_test.go only covers JWT middleware behaviour. The core handler logic — error mapping from auth.Service sentinel errors to typed JSON responses and HTTP status codes — is entirely untested.

Per the review checklist: New API routes have a corresponding handler test. Each handler should have at least one test covering the happy path and the main error branches (e.g., PostAuthLogin → 401 on ErrInvalidCredentials, 423 on ErrAccountLocked, 503 on ErrJellyfinUnavailable).

Fix this →

---

🟡 Nit — PostAuthLogoutAll silently uses uid="" if context value absent

internal/http/handlers_auth.go, line ~56

uid, _ := ctx.Value(ctxUserID).(string)

This is unreachable in practice because the JWT middleware always sets ctxUserID before the handler runs. However, if that protection were ever removed or the key mismatched, LogoutAll would be called with an empty UID and silently do nothing (or behave unexpectedly). A if uid == "" guard + internal error return would make the invariant explicit.

---

🟡 Nit — IssueAccessTokenForTest exported on production type

internal/auth/jwt.go, line 36

Exporting a test helper on the production type (*auth.Service) pollutes the public API surface of the auth package. A _test.go file in the internal/http package that declares an unexported helper, or a small authtest sub-package, keeps the test seam invisible to non-test callers.

---

🟡 Nit — jwtStrictMiddleware lives in server.go

The function's concerns (HTTP auth middleware) fit middleware.go better. Minor organisational point.

---

Note: Commit status could not be set automatically (permissions not granted). To set it manually:

gh api repos/Stoganet/api-proxy/statuses/2a953d6f10a3a20cf2e08f2f83c75d8eced1210c \
  -f state=failure \
  -f context="claude-code-review" \
  -f description="2 blocking issues: wrong error code for missing token; no handler tests for new routes"

[koinsaari]

🟡 Nit — jwtStrictMiddleware lives in server.go

The function's concerns (HTTP auth middleware) fit middleware.go better. Minor organisational point.

@claude this would couple middleware with the generated code. you think it's still a good idea?. server.go already imports gen

[claude]

Claude finished @koinsaari's task in 40s —— View job

---

You're right to push back on that nit — keeping jwtStrictMiddleware in server.go is the better call.

The key distinction is the type boundary. middleware.go holds standard func(http.Handler) http.Handler middleware — generic, net/http-only, zero dependency on generated code. jwtStrictMiddleware returns a gen.StrictMiddlewareFunc, a different interface entirely that only exists because of the generated layer. Moving it to middleware.go would break that clean separation for purely cosmetic "group auth things together" reasons, and would force a gen import into a file that's currently free of it.

server.go is the wiring layer that already assembles the strict handler, passes in the middleware slice, and deals with gen.StrictHTTPServerOptions. jwtStrictMiddleware belongs there — it's part of how the generated handler is configured, not a generic HTTP middleware concern.

The nit stands retracted. Leave it where it is.

---

[claude]

Claude finished @koinsaari's task in 2m 23s —— View job

---

Code Review — e352b01

• Read REVIEW.md and CLAUDE.md
• Read changed files and full diff
• Verify previous blocking issues are resolved
• Post updated review findings
• Set commit status (blocked by permissions — run manually below)

---

Summary

No blocking issues. Both previous 🔴 findings are resolved. No import cycle. JWT keyfunc correctly validates signing method before returning the key (internal/auth/jwt.go:42).

---

✅ Previous blockers resolved

Missing bearer token error code — server.go:65 now uses gen.TokenInvalid for absent/malformed Authorization header. Fixed.

No handler tests — handlers_auth_test.go (218 lines) covers all seven handlers with happy-path and error-branch cases, including the PostAuthLogoutAll context-guard path. Fixed.

PostAuthLogoutAll silent uid="" — handlers_auth.go:55-57 now has an explicit !ok || uid == "" guard that returns a named error. Fixed.

---

🟡 Nit — TokenExpired used for all VerifyJWT failures

internal/http/server.go, line 71

writeError(w, r, stdhttp.StatusUnauthorized, gen.TokenExpired, "invalid or expired token")

When the access token has a bad signature or is malformed, the client receives TokenExpired and will attempt a token refresh. The semantically correct code for a malformed token is TokenInvalid. Since VerifyJWT returns a plain error, distinguishing expiry from invalidity requires an errors.Is(err, jwt.ErrTokenExpired) check (the golang-jwt/jwt/v5 library exposes this sentinel). The practical consequence is minor — clients with a genuinely invalid token will refresh and get a valid one — but it misuses the code's meaning.

Fix this →

---

🟡 Nit — IssueAccessTokenForTest exported on production type (unchanged)

internal/auth/jwt.go, line 36

Still present from the original diff. Exporting a test helper on *auth.Service adds to the public API surface of the auth package. A small authtest sub-package or placing the helper in a _test.go file keeps it invisible to non-test callers.

---

Note: Commit status could not be set automatically (permissions not granted). To set it manually:

gh api repos/Stoganet/api-proxy/statuses/e352b01f70c8e5cf97ebc8373fba4029fa902072 \
  -f state=success \
  -f context="claude-code-review" \
  -f description="No blocking issues — both previous blockers resolved"

[claude]

Claude finished @koinsaari's task in 1m 50s —— View job

---

Code Review — 7cc820b

• Read REVIEW.md and CLAUDE.md
• Read changed files and diff
• Verify previous findings are resolved
• Post updated review findings
• Set commit status (blocked by permissions — run manually below)

---

Summary

No blocking issues. The previous 🟡 nit about TokenExpired used for all VerifyJWT failures is resolved. One prior nit remains open.

---

✅ Previous nit resolved — TokenExpired / TokenInvalid distinction

internal/http/server.go, lines 72–75

errors.Is(err, jwt.ErrTokenExpired) now correctly routes expired tokens to gen.TokenExpired and all other failures (bad signature, malformed) to gen.TokenInvalid. The error chain is intact since VerifyJWT returns the jwt library error unwrapped, so errors.Is works. Tests for both cases added in middleware_test.go (TestJWTMiddleware_RejectsMalformedToken, TestJWTMiddleware_RejectsExpiredToken). Fixed.

---

🟡 Nit — IssueAccessTokenForTest exported on production type (unchanged)

internal/auth/jwt.go, line 36

Still present across all three commits. IssueAccessTokenForTest on *auth.Service is visible to any package that imports internal/auth, not just tests. Moving it to a _test.go file in internal/http (where it's used) or a small internal/auth/authtest sub-package would keep the production API surface clean.

Fix this →

---

Note: Commit status could not be set automatically (permissions not granted). To set it manually:

gh api repos/Stoganet/api-proxy/statuses/7cc820b0813f95fb89d85069a78280008dc0f1dc \
  -f state=success \
  -f context="claude-code-review" \
  -f description="No blocking issues — TokenExpired/TokenInvalid distinction resolved"