dohertj2/lmxopcua
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(security): correct challenge tests to match framework reality

Browse Source 
ASP.NET Core's cookie-handler IsAjaxRequest heuristic only checks
X-Requested-With (not Accept). Drop the third test (Accept: application/json
was assumed to → 401 but actually → 302) and the Location.ShouldBeNull
assertion on the XHR test (framework still writes Location alongside 401;
clients ignore it). Renamed _ajax_ → _xhr_ for accuracy. Design doc
updated to match.
This commit is contained in:
Joseph Doherty
2026-05-29 07:58:18 -04:00
parent 453340e71e
commit af691f3291
2 changed files with 12 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/2026-05-29-auth-alignment-design.md
+7 -14
View File
@@ -110,26 +110,19 @@ public class AuthChallengeTests : AuthEndpointsTestBase
}
[Fact]
public async Task Root_anonymous_ajax_GET_returns_401()
public async Task Root_anonymous_xhr_GET_returns_401()
{
var client = NewClient(allowAutoRedirect: false);
client.DefaultRequestHeaders.Add("X-Requested-With", "XMLHttpRequest");
var resp = await client.GetAsync("/", Ct);
resp.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
resp.Headers.Location.ShouldBeNull();
}
[Fact]
public async Task Root_anonymous_json_GET_returns_401()
{
var client = NewClient(allowAutoRedirect: false);
client.DefaultRequestHeaders.Accept.ParseAdd("application/json");
var resp = await client.GetAsync("/", Ct);
resp.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
// Framework still writes a Location header alongside the 401 — AJAX clients ignore it.
}
}
```
**Framework reality vs. earlier hypothesis:** The ASP.NET Core cookie handler's `IsAjaxRequest` heuristic checks ONLY the `X-Requested-With: XMLHttpRequest` header, NOT the `Accept` content type. A request with `Accept: application/json` but no XHR header is classified as a browser → 302. The third test originally proposed (`Root_anonymous_json_GET_returns_401`) was dropped because it tests behavior the framework doesn't have. ScadaBridge accepts the same framework reality (it doesn't override the heuristic either).
### Package references
`src/Server/ZB.MOM.WW.OtOpcUa.Security/ZB.MOM.WW.OtOpcUa.Security.csproj`: remove `<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" />` if grep confirms `JwtTokenService` doesn't itself need it (it uses `Microsoft.IdentityModel.Tokens` for validation parameters, separate package).
@@ -171,7 +164,7 @@ fetch('/api/something') Accept: application/json
- 401 (no body, no Location)
```
The cookie handler's built-in `IsAjaxRequest` heuristic is what makes this work — no custom event handler needed.
The cookie handler's built-in `IsAjaxRequest` heuristic is what makes this work — it looks for `X-Requested-With: XMLHttpRequest`. No custom event handler needed. Note: requests with only `Accept: application/json` (no XHR header) are classified as browsers → 302; AJAX callers should set the XHR header to get 401.
### Logout
@@ -205,7 +198,7 @@ Unchanged.
| Surface | Behavior |
|---|---|
| Unknown `Accept` (`*/*`, missing) | Framework default: treated as non-AJAX → 302 to `/login`. Documented behavior, matches ScadaBridge. CLI tools that want JSON-style 401 can set `Accept: application/json`. |
| Unknown `Accept` (`*/*`, missing, JSON) | Framework default: treated as non-AJAX → 302 to `/login`. The cookie handler's `IsAjaxRequest` only looks at `X-Requested-With`, NOT `Accept`. CLI tools that want a 401 should set `X-Requested-With: XMLHttpRequest`. |
| `LoginAsync` bad creds | JSON: `401`. Form: `302 /login?error=…&returnUrl=…`. Handler-returned, unaffected by middleware changes. |
| `LoginAsync` LDAP throws | `503 ServiceUnavailable`. Handler-returned. |
| `LoginAsync` success | JSON: `204`. Form: `302 /` (or `ReturnUrl`). |
@@ -230,7 +223,7 @@ Unchanged.
- `Root_anonymous_browser_GET_redirects_to_login` — asserts 302 + `Location` contains `/login` + `ReturnUrl`
- `Root_anonymous_ajax_GET_returns_401` — `X-Requested-With: XMLHttpRequest` → 401, no `Location`
- `Root_anonymous_json_GET_returns_401` — `Accept: application/json` → 401
(the originally planned `Root_anonymous_json_GET_returns_401` was dropped — see Section 3 framework-reality note above)
### Removed/orphaned tests
tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs
+5 -15
View File
@@ -207,28 +207,18 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime
resp.Headers.Location.OriginalString.ShouldContain("ReturnUrl");
}
/// <summary>Anonymous AJAX GET of a protected route returns 401 with no Location.</summary>
/// <summary>Anonymous XHR GET of a protected route returns 401 (caller signaled non-browser
/// via the <c>X-Requested-With</c> header — the ASP.NET cookie handler's IsAjaxRequest
/// heuristic). The framework still writes a <c>Location</c> header alongside the 401;
/// AJAX clients ignore it.</summary>
[Fact]
public async Task Root_anonymous_ajax_GET_returns_401()
public async Task Root_anonymous_xhr_GET_returns_401()
{
var client = NewClientNoRedirect();
var req = new HttpRequestMessage(HttpMethod.Get, "/");
req.Headers.Add("X-Requested-With", "XMLHttpRequest");
var resp = await client.SendAsync(req, Ct);
resp.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
resp.Headers.Location.ShouldBeNull();
}
/// <summary>Anonymous JSON GET of a protected route returns 401.</summary>
[Fact]
public async Task Root_anonymous_json_GET_returns_401()
{
var client = NewClientNoRedirect();
var req = new HttpRequestMessage(HttpMethod.Get, "/");
req.Headers.Accept.ParseAdd("application/json");
var resp = await client.SendAsync(req, Ct);
resp.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
}