Development - #12
Merged
Merged
Conversation
Replace player-ID-based password operations with username-based lookups and have CheckPlayerPassword return the matched player directly. - Change CheckPlayerPassword to return database.Player instead of a bool, dropping the redundant GetPlayerByUsername call previously issued by LoginPlayer after the password check. - Update the Authentication port's CheckPlayerPassword signature accordingly. - Change UpdatePlayerPassword and UpdatePlayerPasswordForce to accept a username instead of an id, reusing CheckPlayerPassword for the player lookup and validation, and removing the id-based GetPlayer query. - Update the PlayerManagement port signatures to match. - Update the CLI player passwd / passwd-force commands to take a username, plus README usage docs. - Update tests to cover the new signatures and username-based flow.
Adds a self-contained execution plan under .docs/plan/ with a baseline README and ten atomic step files covering the HTTP API completion, access control enforcement, and spec alignment (TODOs 1, 3, 4).
Introduces constant.SessionLifetime (72h) and uses it for the JWT exp claim, replacing a hardcoded duration. This is the foundation for aligning the login cookie and JWT lifetimes (TODO 4).
Completes TODO 1 and 3 by exposing GET /games, GET /shared, GET /interactions, POST /save, POST /play, and POST /pause under the authenticated API group. Every endpoint extracts the actor from the JWT and enforces access via AccessManagement.CheckPlayerAccess, returning 404 when access is denied (per spec). State-machine violations map to 409. Aligns login with the API spec (TODO 4): decodes a JSON body, sets a 'session' cookie expiring after constant.SessionLifetime, and returns an empty 204. Extracts route registration into a testable routes() method and wires the new use cases in cmd/server.
Adds integration tests exercising the interaction state machine against the real migrated SQLite DB via a fixed clock: valid save/pause/resume sequence, save-while-paused rejected, pause-when-not-playing rejected, resume-when-not-paused rejected, invalid inputs, and nil dependencies. These lock down the domain invariants the HTTP 409 responses depend on.
echo-jwt defaults NewClaimsFunc to jwt.MapClaims, so claim parsing returned MapClaims instead of *model.JwtPlayerClaims. The playerIDFromContext cast always failed, causing every protected endpoint to return 401. Configure NewClaimsFunc to return the typed JwtPlayerClaims so the actor's UserID is correctly read from the context.
Adds hermetic unit tests using injected fakes and Echo test helpers. Covers login (JSON body, session cookie, empty 204), auth guard (no cookie -> 401 on all protected routes), read endpoints (200, 404 denied access, 400 bad params), and command endpoints (204 success, 404 denied access, 400 invalid body, 409 state-machine conflicts). Also exercises the real JWT generator + NewClaimsFunc so the actor extraction path is verified end-to-end through the middleware.
Verifies the JwtTokenGenerator produces a signed HMAC-SHA256 token carrying the player's UserID/Username claims, with an exp claim equal to now + constant.SessionLifetime (matching the login cookie). Also covers rejection of wrong secrets and error handling for empty secret and nil time provider.
Ensures no DB error is discarded or returned raw. Every query call in game_commands, data_fetching, game_management, access_management, player_management, and authentication now wraps the underlying error with fmt.Errorf("...: %w", err) so callers can unwrap and diagnose. The HTTP requireAccess helper was refactored to return (denied, writeErr) so a denied-access response (404/401/500) is distinguishable from a failure to write that response, preventing a nil-deref on the denied path.
cmd/server now returns 1 when SERVER_ADDRESS is missing instead of continuing with an empty bind address (matching the existing JWT_SECRET fail-fast). Debuggers no longer discard Close() errors in cmd/server and cmd/manager: the deferred sqlDB.Close() result is captured and logged. cmd/manager prints CLI errors to stderr instead of stdout. LoadEnv no longer discards I/O, scan, or Setenv errors — they are logged, while a missing .env file remains non-fatal for externally-injected environments.
parseGameID previously collapsed the strconv.ParseInt error into a bare bool(false) with no logging or wrapping, silently discarding why a gameId was rejected. It now returns a wrapped error (matching the CLI adapter's parseID convention) and every caller logs the underlying cause before returning 400. Also stops discarding the ParseInt error for the limit query param in handleListInteractions.
- accessChecked collapsed 'response already written' into a nil error, so handlers proceeded on denied access and dereferenced a nil Fetch/Commands dependency, panicking - Return (gameID, playerID, denied, writeErr) so callers can stop cleanly when denied - Validate limit before the access check in handleListInteractions, so bad params return 400 regardless of authorization
- Move endpoint handlers, parsing, and response helpers into handlers.go - Move DTOs, adapter constants, and nullableInt64 into models.go - Leave HTTPAdapter, dependencies, route wiring, and server lifecycle in http.go - Trim comments that only restated what the code does
- Rename UpdatePlayerName/DeletePlayer's ambiguous 'id' parameter to playerId in the PlayerManagement port - Mirror the rename in the use case implementation and the CLI adapter's rename/delete subcommands
- Add client-facing error message constants in models.go - Use them in handlers.go instead of inline strings
- Test DefaultTimeProvider returns a valid near-now time - Test game CRUD, player access lifecycle, and ListPlayerGames against the real migrated SQLite DB - Test the state-machine trigger invariants: valid save/pause/resume persists, save-while-paused, pause-when-not-playing, and resume-when-not-paused are rejected - Fixtures honor schema constraints (RFC3339 occurred_at, seeded FK rows)
- BuildSharedData now takes the current time and computes the time-dependent Valid flag itself, so the field is always consistent with the deadline - DataFetching.GetSharedData passes the time provider's time and no longer calls ComputeValid separately
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.