mirror of
https://github.com/azaion/missions.git
synced 2026-06-21 09:41:08 +00:00
refactor: enhance JWT authentication and CORS configuration
Updated JWT authentication to use configuration values instead of hardcoded secrets, improving security and flexibility. Enhanced CORS policy to conditionally allow origins based on configuration settings, with logging for permissive defaults. Updated README to reflect project renaming and clarify service context.
This commit is contained in:
@@ -0,0 +1,117 @@
|
||||
# 06 — HTTP Conventions (Suite-Standard Wire Layer)
|
||||
|
||||
**Spec source**: `../../../suite/_docs/00_top_level_architecture.md` § "Error Response Format" + § "Pagination". These are **suite-wide** — all .NET services (`missions`, `annotations`, `admin`, `satellite-provider`) are expected to emit the same shapes.
|
||||
|
||||
**Implementation status**: ⚠ **DIVERGES from suite spec on entity bodies (PascalCase) and on the error envelope's missing `errors` field**. The error envelope IS already camelCase on case (an accidental match — the anonymous object literal uses lowercase property names). See Caveats.
|
||||
|
||||
**Files**: `Middleware/ErrorHandlingMiddleware.cs`, `DTOs/ErrorResponse.cs`, `DTOs/PaginatedResponse.cs`
|
||||
|
||||
## 1. High-Level Overview
|
||||
|
||||
**Purpose**: Implement the suite's two cross-cutting wire conventions:
|
||||
1. **Error envelope** — `{ statusCode, message, errors? }` (camelCase, `errors` is `object?` keyed by field name) — emitted by the global exception → JSON middleware.
|
||||
2. **Paginated response envelope** — `{ items, totalCount, page, pageSize }` (camelCase) — wrapped around list endpoints.
|
||||
|
||||
**Architectural pattern**: ASP.NET Core middleware (pipeline interceptor) + plain DTO types.
|
||||
|
||||
**Upstream dependencies**: None internally.
|
||||
|
||||
**Downstream consumers**: `07_host` (registers middleware first in pipeline); `02_mission_planning` (consumes `PaginatedResponse<Mission>` from `MissionService.GetMissions`); every component benefits indirectly from the global error handler.
|
||||
|
||||
## 2. Internal Interface
|
||||
|
||||
```csharp
|
||||
public class ErrorHandlingMiddleware(RequestDelegate next, ILogger<ErrorHandlingMiddleware> logger) {
|
||||
Task Invoke(HttpContext context);
|
||||
}
|
||||
|
||||
public class ErrorResponse { // currently unused on the wire (Caveats)
|
||||
int StatusCode;
|
||||
string Message;
|
||||
List<string>?Errors; // wrong shape per spec — see Caveats
|
||||
}
|
||||
|
||||
public class PaginatedResponse<T> {
|
||||
List<T> Items;
|
||||
int TotalCount;
|
||||
int Page;
|
||||
int PageSize;
|
||||
}
|
||||
```
|
||||
|
||||
## 3. External API
|
||||
|
||||
Not an endpoint owner — defines the **error response wire shape** (which deviates from spec today).
|
||||
|
||||
### Spec-mandated shape (`../../../suite/_docs/00_top_level_architecture.md` § Error Response Format)
|
||||
|
||||
```json
|
||||
{
|
||||
"statusCode": 400,
|
||||
"message": "Missing required fields",
|
||||
"errors": {
|
||||
"Name": ["Name is required"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Code's actual shape (anonymous object via `JsonSerializer.Serialize(new { statusCode, message })` with no naming policy override)
|
||||
|
||||
```json
|
||||
{
|
||||
"statusCode": 404,
|
||||
"message": "Vehicle <guid> not found"
|
||||
}
|
||||
```
|
||||
|
||||
Property names are camelCase (the anonymous-type property names `statusCode` / `message` are written lowercase-first in code, and `System.Text.Json` preserves them as-is when no `JsonNamingPolicy` is configured). Two divergences from spec remain: no `errors` field, and the `ErrorResponse` DTO is unused (middleware writes the anonymous object instead, and the DTO's `Errors: List<string>?` is the wrong shape per spec — should be `object?` keyed by field name).
|
||||
|
||||
### Status code mapping (consistent with spec where there's overlap)
|
||||
|
||||
| Exception type | HTTP status |
|
||||
|----------------|-------------|
|
||||
| `KeyNotFoundException` | 404 Not Found |
|
||||
| `ArgumentException` (base — covers `ArgumentNullException`, etc.) | 400 Bad Request |
|
||||
| `InvalidOperationException` | 409 Conflict |
|
||||
| anything else | 500 Internal Server Error (message generic, exception logged) |
|
||||
|
||||
## 4. Data Access Patterns
|
||||
|
||||
None.
|
||||
|
||||
## 5. Implementation Details
|
||||
|
||||
**State Management**: Stateless.
|
||||
|
||||
**Key Dependencies**: `System.Text.Json` (response serialization), `Microsoft.Extensions.Logging.ILogger<T>`.
|
||||
|
||||
**Error Handling Strategy**: This component IS the error handler. Recoverable domain failures (`KeyNotFound`, `Argument`, `InvalidOperation`) are mapped to specific status codes with the exception's message; everything else is a 500 with a sanitized message and a logged stack trace.
|
||||
|
||||
## 6. Extensions and Helpers
|
||||
|
||||
`ErrorResponse` and `PaginatedResponse<T>` could move to a shared helpers folder if a future component spawns more wire-shape concerns; today only `PaginatedResponse<T>` is consumed (by `MissionService.GetMissions`).
|
||||
|
||||
## 7. Caveats & Edge Cases
|
||||
|
||||
1. **PascalCase wire shape for entity bodies** vs. suite-spec camelCase. Controller responses that return entities (`Ok(vehicle)`, `Ok(mission)`, `PaginatedResponse<Mission>`) serialize PascalCase property names because the entity / DTO types are declared PascalCase and no `JsonSerializerOptions.PropertyNamingPolicy = JsonNamingPolicy.CamelCase` is configured. **Exception**: the global error envelope IS already camelCase (the anonymous object literal uses lowercase property names directly).
|
||||
2. **`ErrorResponse` DTO is dead on the wire** — middleware writes an anonymous object instead. AND the DTO's `Errors` is `List<string>?` while spec says `object?` (per-field validation arrays keyed by field name). Two divergences in one DTO. Note: even if the DTO were used, its property names would serialize as PascalCase (`StatusCode`, `Message`, `Errors`) and would diverge from spec on case — additional reason the anonymous-object workaround happens to align with spec on case.
|
||||
3. **No `errors` field emitted** — even when it would be relevant (validation failures). Today the codebase has no validation attributes anyway, so 400s come from `ArgumentException` with a single `Message`. When validation is added, the spec's per-field shape will need to be implemented.
|
||||
4. **`InvalidOperationException → 409`** is non-standard; any third-party library throwing `InvalidOperationException` for an unrelated reason becomes a 409, masking the real cause. In this codebase the only intentional use is `VehicleService.DeleteVehicle` ("vehicle is referenced by missions" — a true 409).
|
||||
5. **No correlation ID / request ID** in the error body — production support has to grep logs by timestamp.
|
||||
6. **`PaginatedResponse<T>` is used by exactly one endpoint** (missions list). `vehicles` and `waypoints` listings are unpaginated by spec, so this is correct.
|
||||
|
||||
## 8. Dependency Graph
|
||||
|
||||
**Must be implemented after**: nothing.
|
||||
|
||||
**Can be implemented in parallel with**: `04_persistence`, `05_identity`.
|
||||
|
||||
**Blocks**: `07_host` (pipeline order matters — must be in DI by the time `app.UseMiddleware<ErrorHandlingMiddleware>` runs); `02_mission_planning` (uses `PaginatedResponse<T>`).
|
||||
|
||||
## 9. Logging Strategy
|
||||
|
||||
| Log Level | When | Example |
|
||||
|-----------|------|---------|
|
||||
| ERROR | Unhandled exception caught by the catch-all branch | `Unhandled exception` (stack trace attached via `LogError(ex, ...)`) |
|
||||
|
||||
(No INFO/DEBUG/WARN emitted by this component.)
|
||||
Reference in New Issue
Block a user