Merge: PR #47 from sameboat-platform/feat/week4-backend-notfound-badrequest-openapi-docs

This pull request introduces several backend improvements focused on error handling, endpoint hardening, documentation updates, and test coverage. The main highlights are the standardization of 404 (NOT_FOUND) error semantics, improved BAD_REQUEST mapping, the addition of a gated public user profile endpoint, and synchronization of OpenAPI and documentation with the current backend state.

Author: ArchILLtect

README.md

@@ -8,7 +8,13 @@
 > git remote set-url origin git@github.com:sameboat-platform/sameboat-backend.git
 > ```
 
-> Quick Links: [Instructions (setup & migrations)](./docs/instructions.md) | [API Reference](./docs/api.md) | [Risks](./docs/RISKS.md) | [JWT vs Extended Sessions (Spike)](./docs/spikes/jwt_vs_extended_sessions.md) | [Week 3 Plan](./docs/weekly-plan/week-3/week-3-plan.md) | [Contributing](./CONTRIBUTING.md) | Guard Rails: [Copilot Instructions](.github/copilot-instructions.md) | Journals: [Index](./docs/journals/README.md) · [Week 1](./docs/journals/Week1-Journal.md) · [Week 2](./docs/journals/Week2-Journal.md)
+> Quick Links: [Instructions (setup & migrations)](./docs/instructions.md) | [API Reference](./docs/api.md) | [Risks](./docs/RISKS.md) | [JWT vs Extended Sessions (Spike)](./docs/spikes/jwt_vs_extended_sessions.md) | [Week 4 Plan (Backend)](./docs/weekly-plan/week-4/week-4-plan-backend.md) | [Week 4 Checkout (Backend)](./docs/weekly-plan/week-4/week-4-checkout-backend.md) | [Contributing](./CONTRIBUTING.md) | Guard Rails: [Copilot Instructions](.github/copilot-instructions.md) | Journals: [Index](./docs/journals/README.md)
+
+## Week 4 Highlights
+- Standardized NOT_FOUND: added `ResourceNotFoundException` and mapped to HTTP 404 with error `NOT_FOUND` in the global handler; service throws, controllers stay thin.
+- BAD_REQUEST mapping: focused WebMvc test covering `IllegalArgumentException` → `{ "error": "BAD_REQUEST" }`.
+- OpenAPI sync: spec now includes `/api/version` and a reusable `ErrorResponse` schema enumerating current error codes.
+- Future endpoint (gated): added a hardened `GET /users/{id}` returning `PublicUserDto` (no email). It’s disabled by default and only enabled when `sameboat.endpoints.user-read=true`; access allowed to self or `ADMIN`.
 
 ## Getting Started (Contributors)
 Before writing code or using AI assistance:
@@ -36,6 +42,11 @@ Alias Tokens (for AI prompts): `BACKEND_CI_GUARD`, `LAYER_RULE`, `SECURITY_BASEL
    - `GET /health` → 200 OK (custom simple health)
    - `GET /actuator/health` → 200 OK and `{ "status": "UP" }`
 
+Windows note: prefer `mvn` (not the wrapper) when running multiple CLI properties in one go; wrap all flags in double quotes in cmd.exe, for example:
+```cmd
+mvn "-Dskip.migration.test=false" verify
+```
+
 ### Container (Docker) Usage
 A multi-stage Dockerfile is provided for local testing and Render deployment.
 
@@ -203,7 +214,7 @@ All non-2xx errors:
 ```json
 { "error": "<CODE>", "message": "Human readable explanation" }
 ```
-Current codes: `UNAUTHENTICATED`, `BAD_CREDENTIALS`, `SESSION_EXPIRED`, `EMAIL_EXISTS`, `VALIDATION_ERROR`, `BAD_REQUEST`, `RATE_LIMITED`, `INTERNAL_ERROR`.
+Current codes: `UNAUTHENTICATED`, `BAD_CREDENTIALS`, `SESSION_EXPIRED`, `EMAIL_EXISTS`, `VALIDATION_ERROR`, `BAD_REQUEST`, `RATE_LIMITED`, `NOT_FOUND`, `INTERNAL_ERROR`.
 
 | Code | Typical Trigger |
 |------|-----------------|
@@ -214,6 +225,7 @@ Current codes: `UNAUTHENTICATED`, `BAD_CREDENTIALS`, `SESSION_EXPIRED`, `EMAIL_E
 | VALIDATION_ERROR | Bean validation (fields, sizes, empty patch body) |
 | BAD_REQUEST | Explicit IllegalArgument / future semantics |
 | RATE_LIMITED | Too many requests (e.g., repeated failed logins) |
+| NOT_FOUND | Requested resource doesn’t exist (service throws `ResourceNotFoundException`) |
 | INTERNAL_ERROR | Uncaught exception (trace id logged) |
 
 ## Quality Gates

docs/Architecture.md

@@ -48,3 +48,9 @@ SAMEBOAT_CORS_ALLOWED_ORIGINS=https://app.sameboatplatform.org
 - Potential WAF / distributed rate limiting (e.g., Redis) for multi-instance auth endpoints.
 
 ---
+
+## Week 4 Backend Summary
+- Introduced `ResourceNotFoundException` mapped to HTTP 404 with error code `NOT_FOUND` via `GlobalExceptionHandler`.
+- Standardized controller 404 behavior by adding service method `UserService.getByIdOrThrow(UUID)` and using it in `GET /users/{id}`.
+- Added focused tests: service throws `ResourceNotFoundException`; WebMvc test asserts 404 NOT_FOUND envelope; BAD_REQUEST mapping asserted for `IllegalArgumentException`.
+- Synced OpenAPI (`openapi/sameboat.yaml`) to include `/api/version` and the error envelope schema with current error codes.

docs/weekly-plan/week-4/week-4-checkout-backend.md

@@ -0,0 +1,73 @@
+# Week 4 Checkout Summary — Backend
+
+## Overview (Week 4 — late October)
+1. ### Standardized NOT_FOUND semantics
+   - Introduced `ResourceNotFoundException` in the common layer and mapped it in `GlobalExceptionHandler` to HTTP 404 with error code `NOT_FOUND`.
+   - Services throw the exception; controllers remain thin and rely on the centralized handler.
+   - Added tests: service unit test for missing user and WebMvc test asserting the 404 envelope.
+
+2. ### BAD_REQUEST mapping coverage
+   - Added a focused WebMvc test to assert `IllegalArgumentException` is translated to HTTP 400 with `{ "error": "BAD_REQUEST" }`.
+   - Keeps error envelope consistent and prevents controller-specific ad hoc responses.
+
+3. ### Endpoint hardening for future admin/public profiles
+   - Added a gated user read endpoint `GET /users/{id}` returning `PublicUserDto` (no email or sensitive fields).
+   - Endpoint is disabled by default; enable with `sameboat.endpoints.user-read=true`.
+   - Access control: only the authenticated user (self) or `ADMIN` may fetch. Tests are present but disabled until the feature is enabled for real environments.
+
+4. ### OpenAPI & Docs
+   - Updated `openapi/sameboat.yaml` to include `/api/version` and a reusable `ErrorResponse` schema enumerating current error codes (`UNAUTHENTICATED`, `BAD_CREDENTIALS`, `SESSION_EXPIRED`, `VALIDATION_ERROR`, `BAD_REQUEST`, `RATE_LIMITED`, `NOT_FOUND`, `INTERNAL_ERROR`).
+   - `docs/Architecture.md`: added a Week 4 Backend Summary with the above items.
+   - README: added Week 4 highlights, Windows `mvn` quoting note, and included `NOT_FOUND` in error catalog table.
+
+5. ### Tests & Quality Gates
+   - All tests pass locally (migration Testcontainers profile skipped without Docker).
+   - JaCoCo instruction coverage: ~84% total (gate ≥ 70% remains green).
+
+## Key Decisions & Rationale
+- Centralize 404 behavior via a domain exception to reduce controller boilerplate and ensure consistent envelopes.
+- Keep cross-user reads gated and least-privilege by default; only self or `ADMIN` can fetch when feature is enabled.
+- Public profile DTO excludes email to reduce PII exposure; future public profile endpoint can reuse the same DTO or an even slimmer one.
+
+## What Went Well
+- Layering preserved (Controller → Service → Repository). Controllers remain thin and secure.
+- Focused tests captured envelope behavior for both NOT_FOUND and BAD_REQUEST without bloating integration suites.
+- OpenAPI is back in sync, making frontend consumption clearer.
+
+## What I Struggled With
+- Security slices in WebMvc tests required explicit authentication setup; solved with `@WithMockUser` or session cookie when appropriate.
+- Property-gated endpoint meant careful test enable/disable to keep the suite green by default.
+
+## Suggested Next Steps (Backend)
+- When bringing up the admin console:
+  - Enable `sameboat.endpoints.user-read=true` in admin/staging environments.
+  - Add role-based method security (`@EnableMethodSecurity`) and migrate inline checks to `@PreAuthorize`.
+  - Add integration tests for `403` (non-self, non-admin) and `200` for admin fetching other users.
+- Consider adding a dedicated public profile route (e.g., `/profiles/{id}`) with no auth but constrained DTO.
+- Continue ratcheting coverage toward 75% by adding small unit tests (e.g., more `UserService` edge cases, filter behaviors).
+
+## Verification (Local)
+- Run fast tests (Windows cmd):
+```cmd
+mvn "-Dskip.migration.test=false" test
+```
+- Full verify (tests + coverage gate):
+```cmd
+mvn "-Dskip.migration.test=false" verify
+```
+- Optional migration schema test (requires Docker/Testcontainers):
+```cmd
+mvn -Pwith-migration-test test
+```
+
+## Checklist (Backend Focus)
+- [x] ResourceNotFoundException + 404 mapping in GlobalExceptionHandler
+- [x] Service method throws on missing resource and unit test
+- [x] WebMvc test for NOT_FOUND envelope
+- [x] Focused BAD_REQUEST mapping test (`IllegalArgumentException`)
+- [x] OpenAPI sync with `/api/version` and error schema
+- [x] Architecture.md Week 4 summary
+- [x] README updated (Week 4 highlights, Windows mvn note, error catalog includes NOT_FOUND)
+- [x] Coverage gate ≥ 70% (current ~84%)
+- [x] Gated `GET /users/{id}` returning `PublicUserDto`; access: self or ADMIN; disabled by default
+

docs/weekly-plan/week-4/week-4-plan-backend.md

@@ -0,0 +1,21 @@
+# Week 4 – Backend Plan (Checklist)
+
+Context: Backend-only items extracted from the Week 4 plan. Status as of 2025-10-29.
+
+## Done
+- [x] Environment onboarding: `.env.example` present at repo root and referenced in README (copy to `.env` for local runs).
+- [x] CI pipeline (BACKEND_CI_GUARD): `.github/workflows/backend-ci.yml` runs migration immutability check, `mvn verify` (unit + integration tests), JaCoCo coverage gate (≥ 70%), and a Flyway schema test profile step.
+- [x] Auth hardening: password complexity validation, in-memory login rate limiting returning `RATE_LIMITED` on abuse, and scheduled session pruning job.
+- [x] Public endpoints: `GET /health`, `GET /actuator/health`, and `GET /api/version` implemented and covered by integration tests.
+- [x] Profiles & CORS: `dev` vs `prod` profiles with secure cookie/domains in prod; CORS allowlist configured (credentials enabled, no wildcards).
+- [x] Flyway migrations: schema aligned with `UserEntity` (e.g., timezone column) via a new migration; historical migrations protected by immutability scripts.
+- [x] NOT_FOUND mapping: Added `ResourceNotFoundException`, mapped to 404 in `GlobalExceptionHandler`, service method `UserService.getByIdOrThrow(UUID)`, gated `GET /users/{id}` uses it when enabled; tests added (service + WebMvc 404 envelope).
+- [x] BAD_REQUEST mapping test: Focused WebMvc test asserting `IllegalArgumentException` → `{ "error": "BAD_REQUEST" }`.
+- [x] OpenAPI sync: `openapi/sameboat.yaml` updated to include `/api/version` and `ErrorResponse` schema with current error codes; brief Week 4 backend summary added to `docs/Architecture.md`.
+- [x] Coverage gate: verified JaCoCo ≥ 70% (current ~84%).
+
+## Left (Backend-focused)
+- [ ] None for Week 4. Carryovers: raise coverage target to 75% in a future cycle; enable gated `/users/{id}` when admin/public profile work lands.
+
+## Scope Notes
+- Excluded (frontend-only): health polling backoff/pause, reduced-motion accessibility controls, debug panel utilities, identicon fallback, and visibility-driven session refresh.

docs/weekly-plan/week-4/week-4-plan-draft.md

@@ -16,6 +16,27 @@ Theme (Proposed): Quality hardening, accessibility polish, developer ergonomics,
 | Visual polish (avatars)               | Clarity in demos                  | Deterministic identicon fallback implemented                 |
 | Session freshness                     | Better long-lived tab UX          | Visibility change triggers conditional refresh               |
 
+## Week 4 Status Checklist (as of 2025-10-29)
+
+Notes:
+- This repository is the backend (Spring Boot). Several Week 4 items target the frontend app (Vite/React) and its CI; those remain pending here and should be tracked/completed in the frontend repo.
+- BACKEND_CI_GUARD respected: main backend CI workflow exists at `.github/workflows/backend-ci.yml`.
+
+Done
+- [x] Environment onboarding (backend scope): `.env.example` is present at repo root and referenced in README “Run locally” section (copy `.env.example` → `.env`).
+- [x] CI (backend scope): Backend CI already runs tests (`mvn verify`) and fails on red; coverage gate enforced (JaCoCo ≥ 70%).
+
+Left (to be completed, mostly frontend scope unless noted)
+- [ ] Testing & CI (frontend): Add `npm test` step to the frontend CI (`frontend-ci.yml`) before build; add routing/auth UI tests listed below.
+- [ ] Health polling resilience (frontend): pause-on-error, exponential backoff, debug surface for failure streak/last success.
+- [ ] Accessibility / Motion control (frontend): central reduced‑motion preference, gate animations, document approach in `Architecture.md` (Week 4 section).
+- [ ] Developer control utilities (frontend): debug panel interval override, failure/backoff display, “Force full auth refresh” button.
+- [ ] Visual / UX polish (frontend): deterministic identicon fallback, copy‑to‑clipboard user ID, spacing sweep.
+- [ ] Session freshness (frontend): `visibilitychange`-based conditional refresh with overlap safeguards.
+- [ ] Documentation: Add Week 4 summary in `docs/Architecture.md`; update any week‑3 links if metrics scripts change; consider short CHANGELOG segment if scope grows.
+
+---
+
 ## Detailed Tasks (Initial Backlog)
 
 ### 1. Testing & CI

docs/weekly-plan/week-5-plan.md renamed to docs/weekly-plan/week-5/week-5-plan.md

@@ -8,4 +8,49 @@ paths:
       summary: Liveness check
       responses:
         '200':
-          description: OK
+          description: OK
+  /api/version:
+    get:
+      summary: API version
+      description: Returns the deployed backend version.
+      responses:
+        '200':
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/VersionResponse'
+        '500':
+          description: Internal error
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorResponse'
+components:
+  schemas:
+    ErrorResponse:
+      type: object
+      properties:
+        error:
+          type: string
+          description: Stable error code
+          enum:
+            - UNAUTHENTICATED
+            - BAD_CREDENTIALS
+            - SESSION_EXPIRED
+            - VALIDATION_ERROR
+            - BAD_REQUEST
+            - RATE_LIMITED
+            - INTERNAL_ERROR
+            - NOT_FOUND
+        message:
+          type: string
+          description: Human-readable error message
+      required: [error, message]
+    VersionResponse:
+      type: object
+      properties:
+        version:
+          type: string
+          description: Semantic version of the deployed backend
+      required: [version]

openapi/sameboat.yaml

@@ -36,6 +36,14 @@ public class AuthController {
     private final SameboatProperties props;
     private final RateLimiterService rateLimiter;
 
+    /**
+     * Constructor with dependencies injected.
+     * @param userService             the service managing user accounts
+     * @param sessionService        the service managing user sessions
+     * @param passwordEncoder   the password encoder for hashing and verifying passwords
+     * @param props                      application configuration properties
+     * @param rateLimiter              the rate limiter service for login attempts
+     */
     public AuthController(UserService userService, SessionService sessionService, PasswordEncoder passwordEncoder, SameboatProperties props, RateLimiterService rateLimiter) {
         this.userService = userService;
         this.sessionService = sessionService;
@@ -44,7 +52,11 @@ public AuthController(UserService userService, SessionService sessionService, Pa
         this.rateLimiter = rateLimiter;
     }
 
-    /** Builds a configured session cookie with security attributes. */
+    /**
+     * Builds the session cookie with appropriate attributes.
+     * @param token the session token value
+     * @return  the constructed Cookie object
+     */
     private Cookie buildSessionCookie(String token) {
         var sessionCfg = props.getSession();
         var cookieCfg = props.getCookie();
@@ -58,20 +70,34 @@ private Cookie buildSessionCookie(String token) {
         return cookie;
     }
 
-    /** Convenience builder for a uniform bad credentials response with logging. */
+    /**
+     * Convenience builder for a uniform bad credentials response.
+     * @param email the email used in the failed login attempt
+     * @return  the ResponseEntity with error details
+     */
     private ResponseEntity<ErrorResponse> badCredentials(String email) {
         log.info("Login failed email={} (bad credentials)", email);
         return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
                 .body(new ErrorResponse("BAD_CREDENTIALS", "Email or password is incorrect"));
     }
 
-    /** Convenience builder for a uniform rate limited response. */
+    /**
+     * Convenience builder for a rate limited response.
+     * @param key the rate limit key that was exceeded
+     * @return  the ResponseEntity with error details
+     */
     private ResponseEntity<ErrorResponse> rateLimited(String key) {
+        log.info("Rate limited response for key={}", key);
         return ResponseEntity.status(429)
                 .body(new ErrorResponse("RATE_LIMITED", "Too many attempts; try again later"));
     }
 
-    /** Build a rate key using normalized email + client ip if available. */
+    /**
+     * Builds a rate limiting key based on normalized email and request IP.
+     * @param emailNorm the normalized email address
+     * @param request   the HTTP servlet request
+     * @return  the constructed rate limit key
+     */
     private String rateKey(String emailNorm, jakarta.servlet.http.HttpServletRequest request) {
         String ip = request != null ? request.getRemoteAddr() : "";
         return emailNorm + "|" + ip;
@@ -80,6 +106,9 @@ private String rateKey(String emailNorm, jakarta.servlet.http.HttpServletRequest
     /**
      * Registers a new user account (unless email already exists) and immediately
      * creates a session, returning the new user id plus issuing the cookie.
+     * @param request   the registration request payload
+     * @param response  the HTTP servlet response to add the session cookie to
+     * @return          the ResponseEntity with new user id or error details
      */
     @PostMapping("/register")
     public ResponseEntity<?> register(@RequestBody @Valid RegisterRequest request, HttpServletResponse response) {
@@ -101,6 +130,10 @@ public ResponseEntity<?> register(@RequestBody @Valid RegisterRequest request, H
     /**
      * Authenticates a user by email + password, optionally auto-creating a dev
      * user if configured. Issues a fresh session cookie on success.
+     * @param request       the login request payload
+     * @param httpRequest   the HTTP servlet request for rate limiting info
+     * @param response      the HTTP servlet response to add the session cookie to
+     * @return              the ResponseEntity with user details or error info
      */
     @PostMapping("/login")
     public ResponseEntity<?> login(@RequestBody @Valid com.sameboat.backend.auth.dto.LoginRequest request,
@@ -109,7 +142,6 @@ public ResponseEntity<?> login(@RequestBody @Valid com.sameboat.backend.auth.dto
         String emailNorm = userService.normalizeEmail(request.email());
         String key = rateKey(emailNorm, httpRequest);
         if (rateLimiter.isLimited(key)) {
-            log.info("Login attempt blocked by rate limiter for key={}", key);
             return rateLimited(key);
         }
         var userOpt = userService.getByEmailNormalized(emailNorm);
@@ -144,6 +176,9 @@ public ResponseEntity<?> login(@RequestBody @Valid com.sameboat.backend.auth.dto
 
     /**
      * Logs out the current session (if present) and expires the cookie.
+     * @param token     the session token from the SBSESSION cookie
+     * @param response  the HTTP servlet response to add the expired cookie to
+     * @return          the ResponseEntity indicating no content
      */
     @PostMapping("/logout")
     public ResponseEntity<Void> logout(@CookieValue(value = "SBSESSION", required = false) String token, HttpServletResponse response) {