Integrate authentication via UserAuth (player accounts + login)#45
Conversation
Replace the anonymous username-only session flow with real authentication delegated to the standalone UserAuth service (register/login/logout + JWT). Backend: - Add UserAuthClient proxying UserAuth /register, /login, /session/validate, /logout - Rewrite AuthController for register/login/logout (proxy to UserAuth) - Authenticate /api/session/* via Authorization: Bearer, validating the token on every request so missing/invalid/expired/revoked tokens are refused (401) - Key game state by the authenticated username; refresh session access time on reuse - Add userauth.url config (USERAUTH_URL env) Web client: - Proxy register/login/logout and forward the bearer token on game requests - Add password field + register link to login screen, add register screen - Store the JWT client-side; logout revokes it server-side docker-compose: add userauth + its Postgres (JWT_SECRET, USERAUTH_PATH, ALLOWED_ORIGINS) Docs: README API + Quick Start + auth flow, PLAYER_GUIDE account section, CHANGELOG, DOCS Tests: UserAuthClientTest, AuthControllerTest, GameControllerAuthTest, SessionServiceTest; first web-client tests (BackendServiceTest) Closes #44 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oxies Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dmccoystephenson
left a comment
dmccoystephenson
left a comment
There was a problem hiding this comment.
Self-review rubric (adversarial — evidence-grounded; CI green on head 78b11bb as the external anchor):
Universal:
- Scope: PASS — all 17 touched paths serve #44. The one judgment call:
GameControllerwas rewritten for Bearer auth and I extracted a sharedvalidateDecision()helper (previously inline-duplicated indecision/sessionDecision); de-dup confined to the file already being changed, not unrelated churn. - Tests-new: PASS — every new public method is exercised:
UserAuthClient(register/login/validate/logout),AuthController(register/login/logout incl. 400s),GameControllerauth gating (401 no-token, 401 invalid/revoked, 200 valid),SessionService.getOrCreateSession, andBackendService(register/login/logout/getSessionState/sessionTick/sessionCommand) after the follow-up test commit. - Tests-fix: PASS — the
SessionServicelastAccessed-refresh fix (regression from removinggetSession) is covered bygetOrCreateSessionRefreshesLastAccessedOnReuse, which fails without the refresh. - Sibling structure: PASS —
UserAuthClientmirrorsBackendService(constructor-injected RestTemplate);register.htmlmirrorslogin.html; tests follow the<Class>Testconvention. - Sibling renames: PASS —
sessionId→tokenrenamed together across game.html (checkSession/getSessionHeaders/logout) and BackendService session methods;X-Session-Id→Authorization: Bearerswapped on both the backend endpoints and the web-client callers. - Docs: PASS — README (API + Quick Start + auth flow), PLAYER_GUIDE (account section), CHANGELOG, DOCS updated;
MVP.mdgrep for auth/session/X-Session-Id returned nothing, so no drift there. - Issue resolution: PASS — register/login/logout + per-request token validation + per-user state all implemented; not a partial close.
- CI: PASS — Backend (157 tests) and Web Client (6 tests) jobs both green.
Repo-specific:
- Both-modules: PASS — separate CI jobs per module both pass; verified locally too.
- API-contract: PASS — backend endpoint changes (
/api/session/*→ Bearer; new/api/auth/register|logout) reflected in BackendService/WebController and the README API section. - Changelog: PASS — Unreleased > Authentication entry added.
- Constructor-injection: PASS — UserAuthClient + AuthController/GameController/WebController use constructor injection (
@RequiredArgsConstructor); no field@Autowiredintroduced (the two rewritten controllers previously used field injection). - Override-correct: PASS (n/a) — no new
@Override.
Flagged for human reviewer (judgment, not blocking): live end-to-end against a running UserAuth was not run in CI (external service; mocked at the client boundary). docker-compose is modified, so this PR is intentionally left for manual review rather than auto-merged.
There was a problem hiding this comment.
Pull request overview
Integrates real player authentication into Barony by proxying the external UserAuth service through the backend and switching per-player game APIs from anonymous sessions to Authorization: Bearer <token>.
Changes:
- Added backend-side UserAuth integration (register/login/logout + per-request token validation) and migrated
/api/session/*to be per-player and token-authenticated. - Updated the web-client to support register/login/logout UI + bearer token forwarding, and added initial web-client tests.
- Extended
docker-compose.ymland docs to run UserAuth (+ Postgres) alongside Barony and document the auth flow.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| web-client/src/test/java/com/barony/webclient/service/BackendServiceTest.java | Adds first web-client tests for auth proxying + bearer forwarding. |
| web-client/src/main/resources/templates/register.html | New registration page posting credentials to /api/auth/register. |
| web-client/src/main/resources/templates/login.html | Adds password-based login and stores JWT client-side. |
| web-client/src/main/resources/templates/game.html | Switches game requests from X-Session-Id to bearer token; adds logout revocation call. |
| web-client/src/main/java/com/barony/webclient/service/BackendService.java | Adds register/logout methods and bearer-token session API calls. |
| web-client/src/main/java/com/barony/webclient/controller/WebController.java | Adds register page + auth proxy endpoints; forwards bearer token on session endpoints. |
| README.md | Documents UserAuth dependency, docker-compose startup, endpoints, and auth flow examples. |
| PLAYER_GUIDE.md | Updates player instructions for registration/login/logout with UserAuth. |
| DOCS.md | Notes authentication flow documentation coverage. |
| docker-compose.yml | Adds userauth + userauth-db services and wires backend to UserAuth. |
| CHANGELOG.md | Records UserAuth integration in Unreleased section. |
| backend/src/test/java/com/barony/backend/service/UserAuthClientTest.java | Tests for UserAuth client proxy behavior. |
| backend/src/test/java/com/barony/backend/service/SessionServiceTest.java | Tests session keying by username + last-access refresh. |
| backend/src/test/java/com/barony/backend/controller/GameControllerAuthTest.java | MVC tests for 401 behavior and per-user scoping via mocked validation. |
| backend/src/test/java/com/barony/backend/controller/AuthControllerTest.java | MVC tests for register/login/logout proxy behavior. |
| backend/src/main/resources/application.properties | Adds userauth.url configuration via USERAUTH_URL. |
| backend/src/main/java/com/barony/backend/service/UserAuthClient.java | New RestTemplate-based UserAuth client (register/login/validate/logout). |
| backend/src/main/java/com/barony/backend/service/SessionService.java | Removes sessionId lookup and refreshes last-access on username reuse. |
| backend/src/main/java/com/barony/backend/controller/GameController.java | Migrates /api/session/* to authenticate via bearer token + UserAuth validation. |
| backend/src/main/java/com/barony/backend/controller/AuthController.java | Replaces sessionId login with UserAuth proxy register/login/logout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Authenticated, per-user proxy endpoints (forward the bearer token to the backend) | ||
| @GetMapping("/api/session/state") | ||
| @ResponseBody | ||
| public GameState getSessionState(@RequestHeader("X-Session-Id") String sessionId) { | ||
| return backendService.getSessionState(sessionId); | ||
| public GameState getSessionState(@RequestHeader(value = "Authorization", required = false) String authorization) { | ||
| return backendService.getSessionState(token(authorization)); | ||
| } |
| @PostMapping("/api/session/tick") | ||
| @ResponseBody | ||
| public GameState sessionTick(@RequestHeader("X-Session-Id") String sessionId) { | ||
| return backendService.sessionTick(sessionId); | ||
| public GameState sessionTick(@RequestHeader(value = "Authorization", required = false) String authorization) { | ||
| return backendService.sessionTick(token(authorization)); | ||
| } |
| @PostMapping("/api/session/command") | ||
| @ResponseBody | ||
| public GameState sessionCommand( | ||
| @RequestHeader("X-Session-Id") String sessionId, | ||
| @RequestHeader(value = "Authorization", required = false) String authorization, | ||
| @RequestBody Command command) { | ||
| return backendService.sessionCommand(sessionId, command); | ||
| return backendService.sessionCommand(token(authorization), command); | ||
| } |
| @PostMapping("/api/session/reset") | ||
| @ResponseBody | ||
| public GameState sessionReset(@RequestHeader("X-Session-Id") String sessionId) { | ||
| return backendService.sessionReset(sessionId); | ||
| public GameState sessionReset(@RequestHeader(value = "Authorization", required = false) String authorization) { | ||
| return backendService.sessionReset(token(authorization)); | ||
| } |
| @GetMapping("/api/session/ruler-stats") | ||
| @ResponseBody | ||
| public RulerStats getSessionRulerStats(@RequestHeader("X-Session-Id") String sessionId) { | ||
| return backendService.sessionRulerStats(sessionId); | ||
| public RulerStats getSessionRulerStats(@RequestHeader(value = "Authorization", required = false) String authorization) { | ||
| return backendService.sessionRulerStats(token(authorization)); | ||
| } |
| synchronized (session.getGameState()) { | ||
| gameService.setGameState(session.getGameState()); | ||
|
|
||
| try { | ||
| gameService.changePolicy(decision.getCategory(), decision.getChoice()); | ||
| return gameService.getState(); |
| synchronized (session.getGameState()) { | ||
| gameService.setGameState(session.getGameState()); | ||
| return gameService.getRulerStats(); | ||
| } |
| @PostMapping("/logout") | ||
| public Map<String, String> logout(@RequestHeader(value = "Authorization", required = false) String authorization) { | ||
| userAuthClient.logout(bearerToken(authorization)); | ||
| return Map.of("message", "logged out"); | ||
| } |
| // Store the UserAuth-issued JWT and username | ||
| localStorage.setItem('token', result.data.token); | ||
| localStorage.setItem('username', username); | ||
| if (result.data.expiresAt) { | ||
| localStorage.setItem('tokenExpiresAt', result.data.expiresAt); |
There was a problem hiding this comment.
Acknowledged. The bearer-token-in-localStorage approach was taken to match the integration contract described in #44 ("store the returned JWT, send it as Authorization: Bearer <token>"). A move to an HttpOnly cookie (or a CSP, which would first require refactoring the inline scripts/handlers out of the templates) is a larger change that was kept out of this PR to bound its scope; it has been filed as a follow-up in #46.
— drafted by Claude on behalf of Daniel Stephenson
| /** | ||
| * Proxy UserAuth {@code POST /register}. Returns the created user's public fields. | ||
| * Propagates UserAuth's status (e.g. 400 validation, 409 username taken). | ||
| */ |
…logout/atomicity - web-client: per-player session proxy endpoints now pass the backend's status through (401/4xx/5xx) instead of collapsing to 500, so the browser's re-login flow on a revoked/expired token works; factored the try/catch into a proxy() helper - backend GameController: synchronize the load-operate-read sequence on the shared GameService monitor (not the per-session state object) to prevent cross-user interleaving of the singleton's mutable game state - backend SessionService: make getOrCreateSession atomic (synchronized) so concurrent first requests for a username can't create split sessions - backend AuthController: logout now requires a bearer token (400 if absent) rather than reporting success while revoking nothing - UserAuthClient: correct register() javadoc (4xx propagated; 5xx/connection → 503) - tests: WebControllerTest (backend 401→401 passthrough, unreachable→503), AuthController logout-without-token → 400 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review comments addressed — commit 8743645Thanks for the thorough review. All 15 comments were triaged; 14 are fixed in code and 1 is deferred to a follow-up with rationale. Both modules are green (backend 158 tests, web-client 8). Proxy status passthrough — web-client Cross-user interleaving — backend
JWT in Drafted by Claude on behalf of Daniel Stephenson. |
Summary
Implements #44 — adds real player authentication to Barony by integrating the standalone UserAuth service, replacing the previous anonymous username-only session flow. This makes Barony the first real-world consumer of UserAuth.
Architecture:
browser → web-client (proxy) → backend → UserAuth. The backend owns the UserAuth integration so credentials/JWTs never need direct browser↔UserAuth CORS. The backend validates the bearer token against UserAuthGET /session/validateon every authenticated request — the issue's recommended correctness-first option, which honours logout/revocation immediately.Modules touched
Both (
backend+web-client), plusdocker-compose.ymland docs.Backend
UserAuthClient(new) — proxies UserAuth/register,/login,/session/validate,/logout; base URL viauserauth.url(defaulthttp://localhost:9998).AuthController— rewritten to/api/auth/register,/api/auth/login,/api/auth/logout(proxy to UserAuth; login returns the JWT, logout revokes it).GameController—/api/session/*now authenticate viaAuthorization: Bearer; missing/invalid/expired/revoked tokens →401. Game state is keyed by the authenticated username.SessionService— refreshes a session's access time on reuse (removed the now-deadgetSessionlookup path) so an active player's state isn't reclaimed mid-play.Web client
BackendService/WebController— proxy register/login/logout and forward the bearer token on game requests.login.html— adds a password field + register link; stores the JWT.register.html(new).game.html— token-based headers; logout revokes server-side.docker-compose
Adds
userauth+userauth-db(Postgres) alongside Barony, wired viaJWT_SECRET,USERAUTH_PATH(sibling checkout build context),ALLOWED_ORIGINS, andUSERAUTH_URLfor the backend.Acceptance criteria
401)docker-composewith Barony; CORS allows the web clientTest plan
backend:./mvnw test— 157 passing (new:UserAuthClientTest,AuthControllerTest,GameControllerAuthTest,SessionServiceTest)web-client:./mvnw test— 4 passing (first test source for this module:BackendServiceTest)Notes
docker-compose.yml, so per the dev-loop's do-not-auto-merge policy it is left for manual review rather than auto-merged./register//login//session/validate//logoutcontract; this integration targets that documented contract. Any rough edges found while wiring it up will be filed back to the UserAuth repo per the issue.Closes #44
🤖 Generated with Claude Code