Standardize API auth challenge responses as ProblemDetails
This commit is contained in:
@@ -213,6 +213,10 @@ public class StateTests
|
||||
var anon = factory.CreateClient();
|
||||
var unauthorized = await anon.GetAsync("/api/state");
|
||||
Assert.Equal(HttpStatusCode.Unauthorized, unauthorized.StatusCode);
|
||||
var unauthorizedJson = await unauthorized.Content.ReadFromJsonAsync<JsonElement>();
|
||||
Assert.Equal("Unauthorized", unauthorizedJson.GetProperty("title").GetString());
|
||||
Assert.Equal("Unauthorized", unauthorizedJson.GetProperty("detail").GetString());
|
||||
Assert.Equal("Unauthorized", unauthorizedJson.GetProperty("error").GetString());
|
||||
|
||||
var client = factory.CreateClientWithCookies();
|
||||
await client.RegisterAsync("counting");
|
||||
|
||||
33
Program.cs
33
Program.cs
@@ -2,6 +2,7 @@ using GameList.Data;
|
||||
using GameList.Endpoints;
|
||||
using GameList.Infrastructure;
|
||||
using Microsoft.AspNetCore.Authentication.Cookies;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.DataProtection;
|
||||
using Microsoft.AspNetCore.HttpOverrides;
|
||||
using Microsoft.Data.Sqlite;
|
||||
@@ -54,16 +55,8 @@ builder.Services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationSc
|
||||
options.ExpireTimeSpan = TimeSpan.FromDays(30);
|
||||
options.Events = new CookieAuthenticationEvents
|
||||
{
|
||||
OnRedirectToLogin = ctx =>
|
||||
{
|
||||
ctx.Response.StatusCode = StatusCodes.Status401Unauthorized;
|
||||
return Task.CompletedTask;
|
||||
},
|
||||
OnRedirectToAccessDenied = ctx =>
|
||||
{
|
||||
ctx.Response.StatusCode = StatusCodes.Status401Unauthorized;
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
OnRedirectToLogin = ctx => WriteUnauthorizedChallengeAsync(ctx.HttpContext),
|
||||
OnRedirectToAccessDenied = ctx => WriteUnauthorizedChallengeAsync(ctx.HttpContext)
|
||||
};
|
||||
});
|
||||
|
||||
@@ -139,6 +132,26 @@ static ForwardedHeadersOptions BuildForwardedHeadersOptions(IConfiguration confi
|
||||
return options;
|
||||
}
|
||||
|
||||
static Task WriteUnauthorizedChallengeAsync(HttpContext context)
|
||||
{
|
||||
context.Response.StatusCode = StatusCodes.Status401Unauthorized;
|
||||
if (!context.Request.Path.StartsWithSegments("/api"))
|
||||
return Task.CompletedTask;
|
||||
|
||||
if (context.Response.HasStarted)
|
||||
return Task.CompletedTask;
|
||||
|
||||
context.Response.ContentType = "application/problem+json";
|
||||
var problem = new ProblemDetails
|
||||
{
|
||||
Status = StatusCodes.Status401Unauthorized,
|
||||
Title = "Unauthorized",
|
||||
Detail = "Unauthorized"
|
||||
};
|
||||
problem.Extensions["error"] = "Unauthorized";
|
||||
return context.Response.WriteAsJsonAsync(problem);
|
||||
}
|
||||
|
||||
static void UpdateIndexMetaBase(IWebHostEnvironment env, string basePath)
|
||||
{
|
||||
try
|
||||
|
||||
19
REVIEW.md
19
REVIEW.md
@@ -11,11 +11,7 @@ Active maintainability risks (priority order):
|
||||
- Cross-feature coupling still exists via shared mutable state usage across UI/data modules (`wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/data.js:82`).
|
||||
- Impact: high regression surface and expensive refactors even after removing global `window` bridges.
|
||||
|
||||
2. Unauthenticated 401 response shape is still framework-driven (Medium)
|
||||
- Endpoint and filter unauthorized responses are standardized when app logic executes (`Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Endpoints/SuggestEndpoints.cs:18`), but anonymous challenge responses remain middleware-controlled (`GameList.Tests/StateTests.cs:214`).
|
||||
- Impact: clients must tolerate both app-produced problem payloads and framework challenge responses.
|
||||
|
||||
3. Static analysis and frontend lint guardrails are still missing (Medium)
|
||||
2. Static analysis and frontend lint guardrails are still missing (Medium)
|
||||
- CI currently gates restore/build/test only (`.github/workflows/ci.yml:23`-`.github/workflows/ci.yml:29`).
|
||||
- Impact: style drift and low-signal warnings can enter the codebase undetected.
|
||||
|
||||
@@ -29,14 +25,6 @@ Active maintainability risks (priority order):
|
||||
- Effort / Risk: `L / Med`.
|
||||
- Dependencies (if any): none.
|
||||
|
||||
[P1] Unify client handling of unauthenticated 401 challenge responses
|
||||
- Problem: Severity `Medium`, Category `API/Contracts`. Fully unauthenticated requests can still bypass application-level problem shaping.
|
||||
- Evidence: challenge behavior is exercised in `GameList.Tests/StateTests.cs:214` while app-level unauthorized shaping is exercised in `GameList.Tests/AuthTests.cs:164`.
|
||||
- Recommendation: either customize cookie-auth challenge response to RFC7807 or explicitly codify dual-shape handling contract in API docs and frontend error layer.
|
||||
- Acceptance criteria (testable): single documented 401 contract path, with tests validating response shape and frontend behavior.
|
||||
- Effort / Risk: `M / Med`.
|
||||
- Dependencies (if any): none.
|
||||
|
||||
[P2] Add static analysis and JS lint/format guardrails
|
||||
- Problem: Severity `Medium`, Category `Tooling`. CI does not enforce analyzers or JS lint/format checks.
|
||||
- Evidence: `.github/workflows/ci.yml:23`-`.github/workflows/ci.yml:29`.
|
||||
@@ -56,9 +44,8 @@ Active maintainability risks (priority order):
|
||||
## C) Suggested execution order
|
||||
|
||||
1. Decompose `ui.js` by feature and keep orchestration thin.
|
||||
2. Normalize/declare unauthenticated 401 contract behavior.
|
||||
3. Add analyzers + JS lint gates in CI.
|
||||
4. Externalize i18n/FAQ assets.
|
||||
2. Add analyzers + JS lint gates in CI.
|
||||
3. Externalize i18n/FAQ assets.
|
||||
|
||||
## D) Guardrails
|
||||
|
||||
|
||||
Reference in New Issue
Block a user