Finishes functional design for unit 0 (back-end changes before front-end work)

2026-06-18 21:40:45 +02:00
parent 9e49489f7e
commit 53a307cdbd
14 changed files with 1065 additions and 4 deletions
@@ -0,0 +1,86 @@
+# Functional Design Plan — Unit 0: Backend Prerequisites
+
+## Unit Context
+- **Unit**: Unit 0 — Backend Prerequisites
+- **Type**: .NET backend changes
+- **Stories**: US-02 (partial — cookie support), US-06 (partial), US-07 (partial)
+- **Deliverables**:
+  - CORS policy via `appsettings.json`
+  - `AuthController` updated for httpOnly cookie (login/refresh/revoke)
+  - `RefreshTokenRequest.cs` removed
+  - `TokenResponse` updated with `Name` property
+  - `appsettings.json` + `appsettings.Development.json` updated
+
+## Plan Checklist
+- [x] Unit context analyzed
+- [x] Questions generated
+- [x] Questions answered
+- [x] business-logic-model.md generated
+- [x] business-rules.md generated
+- [x] domain-entities.md generated
+
+---
+
+## Clarification Questions
+
+Please answer the following questions by filling in the letter choice after the `[Answer]:` tag.
+
+---
+
+### Question 1: Cookie SameSite policy
+The refresh token cookie will use `SameSite=Strict`. Is this correct, or should it be `SameSite=Lax`?
+
+- `SameSite=Strict` — the cookie is only sent when the request originates from the exact same site (most secure; may cause issues if the login page is linked from an external source)
+- `SameSite=Lax` — the cookie is sent on top-level navigations (e.g. clicking a link) but not on cross-site sub-requests (good balance of security and usability)
+
+A) SameSite=Strict (as currently designed)
+B) SameSite=Lax (slightly more permissive, still secure)
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: A
+
+---
+
+### Question 2: Cookie Secure flag in development
+In local development (HTTP, not HTTPS), the `Secure` flag on the cookie will cause the browser to reject the cookie. How should this be handled?
+
+A) Use `Secure = true` always — developer must use HTTPS locally (e.g. via `dotnet dev-certs`)
+B) Set `Secure` based on environment: `true` in Production, `false` in Development
+C) Set `Secure = request.IsHttps` — automatically adapts to the current request protocol
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: B
+
+---
+
+### Question 3: TokenResponse — Name field source
+The `TokenResponse` needs a `Name` field for the user's display name. `ApplicationUser` has `UserName` (from IdentityUser) but no dedicated display name field. What should `Name` return?
+
+A) `user.UserName` — the username is used as the display name
+B) `user.Email` — the email is used as the display name
+C) `user.UserName ?? user.Email` — use UserName if set, fall back to Email
+X) Other — add a dedicated `DisplayName` property to `ApplicationUser` (describe below)
+
+[Answer]: C
+
+---
+
+### Question 4: Revoke endpoint — authentication requirement
+The current `Revoke` endpoint requires a valid Bearer token (`[Authorize]`). With the cookie-based flow, should this still require authentication?
+
+A) Yes — keep `[Authorize]` requirement (user must have a valid access token to revoke; most secure)
+B) No — remove `[Authorize]` (allows revoking even after access token expires; better UX for logout after expiry)
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: A, but make the UX still seemless and make it refresh in the background while logging out. Maybe adding a text along the line of "Securely logging out..." gives some more time.
+
+---
+
+### Question 5: CORS — allowed HTTP methods
+Which HTTP methods should the CORS policy allow?
+
+A) Only the methods used by the API: GET, POST, PUT, DELETE, OPTIONS
+B) All methods: `AllowAnyMethod()` (simpler, allows future endpoints without CORS changes)
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: A
@@ -0,0 +1,50 @@
+# NFR Requirements Plan — Unit 0: Backend Prerequisites
+
+## Context
+Unit 0 modifies existing .NET 10 backend endpoints. Most NFRs are inherited from the existing system. Only security-specific NFRs for the cookie/CORS changes require clarification.
+
+## Plan Checklist
+- [x] Functional design artifacts analyzed
+- [x] Questions generated
+- [x] Questions answered
+- [x] nfr-requirements.md generated
+- [x] tech-stack-decisions.md generated
+
+---
+
+## Clarification Questions
+
+---
+
+### Question 1: Refresh token lifetime
+What should the refresh token lifetime be?
+
+A) 7 days (current default — check actual `RefreshToken` expiry in the codebase)
+B) 30 days
+C) 90 days
+X) Other (specify in days after [Answer]: tag)
+
+[Answer]: A
+
+---
+
+### Question 2: Rate limiting on auth endpoints
+Should rate limiting be applied to the auth endpoints (login, refresh) to prevent brute-force attacks?
+
+A) Yes — add rate limiting now as part of Unit 0 (e.g. via ASP.NET Core built-in rate limiter)
+B) No — rate limiting is out of scope for this unit; document as future work
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: A
+
+---
+
+### Question 3: Error response format on auth failures
+What should the error response body look like when auth fails (wrong password, invalid token, missing cookie)?
+
+A) RFC 9457 ProblemDetails (standard ASP.NET format: `type`, `title`, `status`, `detail`)
+B) Simple JSON `{ "message": "..." }` (consistent with current API error format)
+C) Use the existing GlobalExceptionHandler format from `SlpModularCms.Api/Infrastructure/`
+X) Other (please describe after [Answer]: tag below)
+
+[Answer]: A
@@ -0,0 +1,93 @@
+# Business Logic Model — Unit 0: Backend Prerequisites
+
+## Overview
+
+Unit 0 modifies the authentication flow to use httpOnly cookies for refresh token delivery and adds CORS support for the React SPA. No new business transactions are introduced — existing auth transactions are made more secure.
+
+---
+
+## Flow 1: Login
+
+```mermaid
+sequenceDiagram
+    participant FE as Frontend
+    participant AC as AuthController
+    participant AS as AuthService
+    participant DB as Database
+
+    FE->>AC: POST /auth/login
+    AC->>AS: AuthenticateAsync()
+    AS->>DB: FindUser + ValidatePassword
+    DB-->>AS: User + Role
+    AS-->>AC: TokenResponse
+    AC-->>FE: 200 accessToken + Set-Cookie refreshToken
+```
+
+**Changes from current behaviour**:
+- `RefreshToken` is no longer returned in the response body
+- httpOnly cookie is set on the response
+- `UserDto` with `Name` (DisplayName ?? Email) is added to response
+
+---
+
+## Flow 2: Refresh Token
+
+```mermaid
+sequenceDiagram
+    participant FE as Frontend
+    participant AC as AuthController
+    participant AS as AuthService
+    participant DB as Database
+
+    FE->>AC: POST /auth/refresh (cookie auto-sent)
+    AC->>AC: Read cookie refreshToken
+    AC->>AS: RefreshTokenAsync(cookieValue)
+    AS->>DB: Validate RefreshToken
+    DB-->>AS: Valid + User
+    AS-->>AC: new TokenResponse
+    AC-->>FE: 200 new accessToken + Set-Cookie new refreshToken
+```
+
+**Changes from current behaviour**:
+- Request body (`RefreshTokenRequest`) is **removed** — token read from cookie only
+- New refresh token set in cookie (rotation still applies)
+
+---
+
+## Flow 3: Revoke (Logout)
+
+```mermaid
+sequenceDiagram
+    participant FE as Frontend
+    participant AC as AuthController
+    participant AS as AuthService
+    participant DB as Database
+
+    FE->>AC: POST /auth/revoke (no Authorize)
+    AC->>AC: Read cookie refreshToken
+    AC->>AS: RevokeTokenAsync(cookieValue)
+    AS->>DB: Mark token revoked
+    DB-->>AS: OK
+    AC->>AC: Clear cookie Expires=epoch
+    AC-->>FE: 204 No Content
+```
+
+**Changes from current behaviour**:
+- `[Authorize]` attribute removed — logout works even if access token has expired
+- Token read from cookie, not request body
+- Cookie explicitly cleared in response
+
+---
+
+## Flow 4: CORS Preflight
+
+```mermaid
+sequenceDiagram
+    participant Browser
+    participant API as API
+
+    Browser->>API: OPTIONS /auth/login preflight
+    API-->>Browser: 204 Access-Control-Allow headers
+    Browser->>API: POST /auth/login with credentials
+    API-->>Browser: 200 + Set-Cookie
+```
@@ -0,0 +1,66 @@
+# Business Rules — Unit 0: Backend Prerequisites
+
+## BR-U0-01: CORS Origin Validation
+- The CORS policy MUST only allow origins listed in `Cors:AllowedOrigins` from configuration
+- An empty `AllowedOrigins` array in production means no SPA origin is allowed (safe default)
+- `AllowCredentials()` MUST be set — required for the browser to send the httpOnly cookie
+- `AllowAnyMethod()` and `AllowAnyHeader()` are used to avoid CORS issues for future endpoints
+- The CORS middleware MUST be registered BEFORE `UseAuthentication` and `UseAuthorization` in the pipeline
+
+## BR-U0-02: Refresh Token Cookie — Set Rules
+- Cookie name: `refreshToken`
+- Cookie path: `/api/v1/auth` — scoped to auth endpoints only; not sent with other API calls
+- `HttpOnly = true` — ALWAYS, no exceptions
+- `Secure = request.IsHttps` — adapts to the current request protocol (true in production, false in local HTTP)
+- `SameSite = Strict` — cookie only sent when request originates from the exact same site
+- Cookie is set on BOTH `Login` and `Refresh` responses (token rotation)
+- Cookie MUST NOT be set on failed auth attempts
+
+## BR-U0-03: Refresh Token Cookie — Clear Rules
+- On `Revoke`: Set-Cookie with the same name/path but `Expires = DateTime.UnixEpoch` (epoch = effectively deleted)
+- On failed refresh (invalid/expired token): Clear the cookie in the error response
+- Cookie clearing MUST use the exact same `Path` as cookie setting (`/api/v1/auth`)
+
+## BR-U0-04: HTTP Response Body — Login and Refresh
+The following fields MUST be returned in the response body:
+- `accessToken` (string) — JWT bearer token
+- `expiresAt` (ISO datetime) — access token expiry
+- `user.id` (Guid)
+- `user.email` (string)
+- `user.name` (string) — `ApplicationUser.DisplayName ?? ApplicationUser.Email`
+- `user.role` (string) — the user's primary role name (Owner / Admin / User)
+- `user.isActive` (bool)
+
+The `refreshToken` MUST NOT appear in the response body.
+
+## BR-U0-05: Revoke Endpoint Authorization
+- `[Authorize]` is REMOVED from the `Revoke` endpoint
+- If the refresh token cookie is present: revoke it and clear the cookie
+- If the refresh token cookie is absent: no-op, return `204 No Content` (idempotent logout)
+- This allows logout to succeed even when the access token has already expired
+
+## BR-U0-06: Refresh Endpoint — Input
+- The `RefreshTokenRequest` body parameter is REMOVED
+- The refresh token is read exclusively from `Request.Cookies["refreshToken"]`
+- If the cookie is absent: return `401 Unauthorized` with a generic error message
+- The existing `accessToken` validation logic in `IAuthService.RefreshTokenAsync` can be relaxed or the signature adapted (see code generation for details)
+
+## BR-U0-07: DisplayName Field
+- `ApplicationUser.DisplayName` is nullable (`string?`)
+- Maximum length: 100 characters
+- Default value: `NULL` for existing users
+- `name` in the response is computed as: `user.DisplayName ?? user.Email`
+- On new user creation via invitation (`POST /users/complete-setup`): the `DisplayName` is set from the form field
+- On owner creation (`POST /setup/owner`): `DisplayName` defaults to `NULL` (falls back to email)
+
+## BR-U0-08: Migration
+- A new EF Core Code-First migration is required named e.g. `AddDisplayNameToApplicationUser`
+- The migration adds `DisplayName nvarchar(100) NULL` to the `AspNetUsers` table
+- All existing users receive `NULL` as their `DisplayName` (they see their email as display name)
+
+## BR-U0-09: Security Baseline Compliance (SECURITY-12)
+- httpOnly cookie prevents XSS token theft ✅
+- `Secure = request.IsHttps` ensures the cookie is only sent over HTTPS in production ✅
+- `SameSite=Strict` prevents CSRF attacks on the refresh endpoint ✅
+- No credentials (tokens) in localStorage or response body ✅
+- Session invalidated on logout (token revoked + cookie cleared) ✅
@@ -0,0 +1,90 @@
+# Domain Entities — Unit 0: Backend Prerequisites
+
+## Modified Entity: ApplicationUser
+
+`ApplicationUser` is extended with a new `DisplayName` property.
+
+```csharp
+public class ApplicationUser : IdentityUser<Guid>
+{
+    // Existing properties (unchanged)
+    public bool IsActive { get; set; }
+    public DateTime CreatedAt { get; set; }
+
+    // NEW: Display name shown in the frontend UI
+    // Optional — if null/empty, the frontend falls back to the user's email
+    public string? DisplayName { get; set; }
+}
+```
+
+**Migration required**: A new EF Core migration must be added to `SlpModularCms.Core/Migrations/` to add the `DisplayName` column to the `AspNetUsers` table.
+
+| Property | Type | Nullable | Max Length | Default |
+|----------|------|----------|-----------|---------|
+| `DisplayName` | `nvarchar` | Yes | 100 | NULL |
+
+---
+
+## Modified DTO: TokenResponse
+
+`TokenResponse` is extended to carry the user's display name and role for the frontend auth context.
+
+```csharp
+public record TokenResponse(
+    string AccessToken,
+    string RefreshToken,   // Kept in record for internal use; NOT returned in HTTP response body
+    DateTime ExpiresAt,
+    UserDto User
+);
+
+public record UserDto(
+    Guid Id,
+    string Email,
+    string Name,           // DisplayName ?? Email (fallback)
+    string Role,
+    bool IsActive
+);
+```
+
+> **Note**: `RefreshToken` is no longer returned in the HTTP response body. The `AuthController` reads it from the `TokenResponse` record internally to set the httpOnly cookie, then returns only `AccessToken`, `ExpiresAt`, and `User` to the client.
+
+---
+
+## Removed: RefreshTokenRequest
+
+`RefreshTokenRequest.cs` is deleted. The refresh token is now read exclusively from the httpOnly cookie.
+
+```csharp
+// DELETED — no longer needed
+// public record RefreshTokenRequest(string AccessToken, string RefreshToken);
+```
+
+---
+
+## Configuration Entity: CorsSettings
+
+New configuration section read via `IConfiguration` — no database entity.
+
+```json
+// appsettings.json (production placeholder)
+{
+  "Cors": {
+    "AllowedOrigins": []
+  }
+}
+
+// appsettings.Development.json
+{
+  "Cors": {
+    "AllowedOrigins": ["http://localhost:5173"]
+  }
+}
+```
+
+```csharp
+// Bound via IConfiguration — not a domain entity
+public class CorsSettings
+{
+    public string[] AllowedOrigins { get; set; } = [];
+}
+```
@@ -0,0 +1,53 @@
+# NFR Requirements — Unit 0: Backend Prerequisites
+
+## NFR-U0-01: Security — httpOnly Cookie (SECURITY-12)
+- Refresh token MUST be stored in an httpOnly, Secure (request.IsHttps), SameSite=Strict cookie
+- Access token MUST NOT be returned in persistent storage; only in response body and in-memory on client
+- See `business-rules.md` BR-U0-02, BR-U0-03 for detailed cookie specification
+
+## NFR-U0-02: Security — CORS (SECURITY-08)
+- CORS MUST use explicit origin allowlist from configuration — no wildcard origins
+- `AllowCredentials()` MUST be set to allow the refresh token cookie to be sent cross-origin
+- CORS middleware MUST be registered before `UseAuthentication` in the pipeline
+- An empty `AllowedOrigins` array in production configuration is a safe default (no SPA access)
+
+## NFR-U0-03: Security — Rate Limiting (SECURITY-11)
+- The `POST /api/v1/auth/login` and `POST /api/v1/auth/refresh` endpoints MUST have rate limiting
+- **Implementation**: ASP.NET Core built-in `RateLimiter` middleware (available in .NET 7+)
+- **Policy**: Fixed window — max **5 requests per 1 minute per IP address** on login endpoint
+- **Policy**: Sliding window — max **20 requests per 1 minute per IP address** on refresh endpoint
+- Exceeded rate limit returns `429 Too Many Requests`
+- Rate limit headers (`Retry-After`) MUST be included in 429 responses
+- Configuration stored in `appsettings.json → RateLimiting` section
+
+## NFR-U0-04: Reliability — Refresh Token Lifetime
+- Refresh token lifetime: **7 days** (unchanged from current default)
+- After expiry, the user must re-authenticate via the login page
+- Refresh token rotation is already implemented — each refresh issues a new token
+
+## NFR-U0-05: Error Response Format (SECURITY-09, SECURITY-15)
+- ALL authentication error responses MUST use **RFC 9457 ProblemDetails** format:
+  ```json
+  {
+    "type": "https://tools.ietf.org/html/rfc7235#section-3.1",
+    "title": "Unauthorized",
+    "status": 401,
+    "detail": "Invalid credentials."
+  }
+  ```
+- Error messages MUST be generic — do NOT reveal whether email or password was wrong
+- Error messages MUST NOT expose internal details (stack traces, exception types, DB details)
+- `asp-problem-details` behavior is already partially handled by `GlobalExceptionHandler`; auth-specific errors may need explicit `ProblemDetails` returns in `AuthController`
+
+## NFR-U0-06: Maintainability
+- Rate limiting configuration (window size, request limits) stored in `appsettings.json` — configurable without code changes
+- CORS origins stored in `appsettings.json` — configurable per environment
+- No magic strings for cookie name or policy name — use constants
+
+## NFR-U0-07: Test Coverage
+- Unit tests for `AuthController` must be updated to cover:
+  - Cookie is set on successful login/refresh
+  - Cookie is cleared on revoke
+  - 401 returned when cookie is missing on refresh
+  - Rate limit behavior (mock rate limiter)
+- Existing `AuthService` unit tests remain valid (service signature unchanged)
@@ -0,0 +1,105 @@
+# Tech Stack Decisions — Unit 0: Backend Prerequisites
+
+## Existing Stack (no changes)
+All existing technology choices are retained:
+- **.NET 10 / ASP.NET Core 10** — Web API framework
+- **Entity Framework Core 10** — ORM + migrations
+- **ASP.NET Core Identity** — User/role management, password hashing
+- **SQL Server** — Primary database
+
+---
+
+## New/Updated: Rate Limiting
+
+| Decision | Choice | Rationale |
+|----------|--------|-----------|
+| Rate limiter | ASP.NET Core built-in `RateLimiter` (Microsoft.AspNetCore.RateLimiting) | No extra NuGet package needed — available in .NET 7+; production-ready |
+| Login policy | Fixed window (5 req / 1 min / IP) | Predictable; blocks brute-force login attempts |
+| Refresh policy | Sliding window (20 req / 1 min / IP) | More lenient for token rotation; prevents abuse |
+| Configuration | `appsettings.json → RateLimiting` | Configurable without code recompile |
+
+---
+
+## New/Updated: CORS
+
+| Decision | Choice | Rationale |
+|----------|--------|-----------|
+| CORS implementation | ASP.NET Core built-in CORS middleware | No extra NuGet package; part of framework |
+| Origins configuration | `appsettings.json → Cors:AllowedOrigins[]` | Environment-specific; follows dotnet-appsettings pattern |
+| Credential support | `AllowCredentials()` | Required for httpOnly cookie to be sent cross-origin |
+| Methods | `AllowAnyMethod()` | Avoids future CORS issues when new endpoints are added |
+| Headers | `AllowAnyHeader()` | Standard approach; avoids pre-flight failures for custom headers |
+
+---
+
+## New/Updated: httpOnly Cookie
+
+| Decision | Choice | Rationale |
+|----------|--------|-----------|
+| Cookie implementation | ASP.NET Core `Response.Cookies.Append()` | Built-in, no extra library |
+| Token read | `Request.Cookies["refreshToken"]` | Standard ASP.NET Core cookie reading |
+| Secure flag | `request.IsHttps` | Adapts to environment; safe in production, usable in local HTTP dev |
+| SameSite | `Strict` | Maximum CSRF protection |
+
+---
+
+## New/Updated: Error Responses
+
+| Decision | Choice | Rationale |
+|----------|--------|-----------|
+| Error format | RFC 9457 ProblemDetails | .NET standard; consistent with ASP.NET Core defaults; interoperable |
+| Implementation | `Microsoft.AspNetCore.Mvc.ProblemDetails` (built-in) | No extra NuGet package needed |
+| Global handler | Existing `GlobalExceptionHandler` extended | Avoids duplication; centralises error formatting |
+
+---
+
+## appsettings.json Additions
+
+Following the dotnet-appsettings skill pattern:
+
+```json
+// appsettings.json (production defaults — no real values)
+{
+  "Cors": {
+    "AllowedOrigins": []
+  },
+  "RateLimiting": {
+    "Login": {
+      "PermitLimit": 5,
+      "WindowSeconds": 60
+    },
+    "Refresh": {
+      "PermitLimit": 20,
+      "WindowSeconds": 60
+    }
+  }
+}
+```
+
+```json
+// appsettings.Development.json (complete reference for developers)
+{
+  "Cors": {
+    "AllowedOrigins": ["http://localhost:5173"]
+  },
+  "RateLimiting": {
+    "Login": {
+      "PermitLimit": 5,
+      "WindowSeconds": 60
+    },
+    "Refresh": {
+      "PermitLimit": 20,
+      "WindowSeconds": 60
+    }
+  }
+}
+```
+
+```json
+// appsettings.local.json (developer override — gitignored)
+{
+  "Cors": {
+    "AllowedOrigins": ["http://localhost:5173"]
+  }
+}
+```