Skip to content

Commit 342d18b

Browse files
vbilopavvbilopav
committed
fix: SSE scope hints were not enforced — hint-scoped events were delivered to every subscriber
Events published with RAISE ... USING HINT = 'authorize ...' were delivered to ALL connected SSE subscribers, including subscribers without the named role and unauthenticated subscribers. The hint was parsed correctly, but the scope authorization checks were 'else if' branches chained to the hint-parsing 'if', so they were skipped whenever a hint was present. Endpoint-level sse_scope annotation scoping (no hint) was not affected. Fixed by decoupling enforcement from hint parsing so the Matching/Authorize checks always run on the effective scope. Regression test proves delivery by ordering across three differently-authenticated subscribers (role-scoped, authenticated-scoped, unscoped).
1 parent b7cfe2e commit 342d18b

3 files changed

Lines changed: 140 additions & 2 deletions

File tree

‎NpgsqlRest/NpgsqlRestSseEventSource.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,11 @@ public async Task InvokeAsync(HttpContext context)
113113
words?[0], string.Join(", ", Enum.GetNames<SseEventsScope>()), hint);
114114
}
115115
}
116-
117-
else if (scope == SseEventsScope.Matching)
116+
117+
// NOTE: deliberately NOT an `else if` - the scope checks below must run regardless of
118+
// whether the scope came from the endpoint annotation or was overridden by the RAISE hint.
119+
// (Chaining them as `else if` delivered hint-scoped events to EVERY subscriber.)
120+
if (scope == SseEventsScope.Matching)
118121
{
119122
if (context.User?.Identity?.IsAuthenticated is false &&
120123
(endpoint?.RequiresAuthorization is true || endpoint?.AuthorizeRoles is not null))

‎NpgsqlRestTests/SseTests/SseScopeTests.cs‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
namespace NpgsqlRestTests
2+
{
3+
public static partial class Database
4+
{
5+
public static void SseScopeTests()
6+
{
7+
script.Append("""
8+
9+
-- Anonymous-subscribable event stream; per-event filtering comes from publish-side hints.
10+
create function ssecope_events()
11+
returns void
12+
language plpgsql immutable
13+
as $$ begin perform 1; end $$;
14+
comment on function ssecope_events() is '
15+
HTTP GET
16+
sse_subscribe
17+
';
18+
19+
-- Publisher: emits the message with an optional scope hint (mirrors the documented
20+
-- RAISE ... USING HINT = ''authorize <role>'' production pattern).
21+
create procedure ssecope_publish(_msg text, _hint text)
22+
language plpgsql
23+
as $$
24+
begin
25+
if _hint is null then
26+
raise info '%', _msg;
27+
else
28+
raise info '%', _msg using hint = _hint;
29+
end if;
30+
end $$;
31+
comment on procedure ssecope_publish(text, text) is '
32+
HTTP POST
33+
sse_publish
34+
';
35+
36+
create function ssecope_login_a()
37+
returns table (name_identifier int, name text, role text[])
38+
language sql as $$
39+
select 9001, 'scope_user_a', array['role_a']
40+
$$;
41+
comment on function ssecope_login_a() is 'login';
42+
43+
create function ssecope_login_b()
44+
returns table (name_identifier int, name text, role text[])
45+
language sql as $$
46+
select 9002, 'scope_user_b', array['role_b']
47+
$$;
48+
comment on function ssecope_login_b() is 'login';
49+
""");
50+
}
51+
}
52+
}
53+
54+
namespace NpgsqlRestTests.SseTests
55+
{
56+
using NpgsqlRestTests.Setup;
57+
58+
[Collection("TestFixture")]
59+
public class SseScopeTests(TestFixture test)
60+
{
61+
private const string SubscribeUrl = "/api/ssecope-events/info";
62+
private const string PublishUrl = "/api/ssecope-publish";
63+
64+
private static StringContent Publish(string msg, string? hint) =>
65+
new(hint is null
66+
? $"{{\"msg\":\"{msg}\",\"hint\":null}}"
67+
: $"{{\"msg\":\"{msg}\",\"hint\":\"{hint}\"}}",
68+
Encoding.UTF8, "application/json");
69+
70+
/// <summary>
71+
/// Per-event scoping via RAISE ... USING HINT: 'authorize role_a' must deliver ONLY to
72+
/// subscribers whose claims include role_a; bare 'authorize' must deliver only to
73+
/// authenticated subscribers. Non-delivery is proven by ordering, not by waiting: each
74+
/// excluded subscriber's FIRST received event must be the later, broader one.
75+
/// </summary>
76+
[Fact]
77+
public async Task Hint_Scoped_Events_Are_Delivered_Only_To_Matching_Subscribers()
78+
{
79+
// Three subscribers: A (role_a), B (role_b), Anon (not authenticated).
80+
using var clientA = test.Application.CreateClient();
81+
using var loginA = await clientA.PostAsync("/api/ssecope-login-a/", null);
82+
loginA.StatusCode.Should().Be(HttpStatusCode.OK);
83+
84+
using var clientB = test.Application.CreateClient();
85+
using var loginB = await clientB.PostAsync("/api/ssecope-login-b/", null);
86+
loginB.StatusCode.Should().Be(HttpStatusCode.OK);
87+
88+
using var clientAnon = test.Application.CreateClient();
89+
90+
await using var sseA = await SseTestClient.OpenAsync(clientA, SubscribeUrl);
91+
await sseA.WaitForRegisteredAsync();
92+
await using var sseB = await SseTestClient.OpenAsync(clientB, SubscribeUrl);
93+
await sseB.WaitForRegisteredAsync();
94+
await using var sseAnon = await SseTestClient.OpenAsync(clientAnon, SubscribeUrl);
95+
await sseAnon.WaitForRegisteredAsync();
96+
97+
// 1. Scoped to role_a only.
98+
using (var p = await clientA.PostAsync(PublishUrl, Publish("only-role-a", "authorize role_a")))
99+
p.StatusCode.Should().BeOneOf(HttpStatusCode.OK, HttpStatusCode.NoContent);
100+
101+
// 2. Scoped to any authenticated subscriber.
102+
using (var p = await clientA.PostAsync(PublishUrl, Publish("any-authenticated", "authorize")))
103+
p.StatusCode.Should().BeOneOf(HttpStatusCode.OK, HttpStatusCode.NoContent);
104+
105+
// 3. Unscoped terminator: delivered to everyone (subscribe endpoint is anonymous, default scope).
106+
using (var p = await clientA.PostAsync(PublishUrl, Publish("everyone", null)))
107+
p.StatusCode.Should().BeOneOf(HttpStatusCode.OK, HttpStatusCode.NoContent);
108+
109+
// A (role_a) receives all three, in order.
110+
(await sseA.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("only-role-a");
111+
(await sseA.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("any-authenticated");
112+
(await sseA.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("everyone");
113+
114+
// B (role_b) must NOT get the role_a event: its first event is the authenticated one.
115+
(await sseB.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("any-authenticated",
116+
"an event hinted 'authorize role_a' must not be delivered to a subscriber without role_a");
117+
(await sseB.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("everyone");
118+
119+
// Anonymous must get NEITHER scoped event: its first event is the unscoped one.
120+
(await sseAnon.ReadDataLineAsync(TimeSpan.FromSeconds(10))).Should().Be("everyone",
121+
"events hinted 'authorize …' must never be delivered to unauthenticated subscribers");
122+
}
123+
}
124+
}

‎changelog/v3.17.0.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ The public properties **`RoutineEndpoint.OpenApiHide` and `RoutineEndpoint.OpenA
8888

8989
## Fixes
9090

91+
### 🔴 Security: SSE scope hints were not enforced — hint-scoped events were delivered to every subscriber
92+
93+
Events published with a per-event scope override — `RAISE INFO ... USING HINT = 'authorize'` or `USING HINT = 'authorize <role-or-user> ...'` — were delivered to **all** connected SSE subscribers, including subscribers without the named role **and unauthenticated subscribers**. The hint was parsed correctly, but a control-flow bug (`else if` chaining) skipped the authorization checks whenever a hint was present. Endpoint-level scoping via the `sse_scope` annotation (no hint) was NOT affected.
94+
95+
**Impact:** any deployment using the documented per-user/per-role `USING HINT` pattern (e.g. private user messages or role-targeted notifications over SSE) was broadcasting those events to every connected subscriber. **Upgrade is strongly recommended** for anyone using SSE with hint-based scoping.
96+
97+
Fixed by decoupling scope enforcement from hint parsing so the `Matching`/`Authorize` checks always run on the effective scope. Covered by new tests proving delivery-by-ordering: a role-scoped event reaches only matching subscribers, a bare `authorize` event reaches only authenticated subscribers, and anonymous subscribers receive neither.
98+
9199
### Malformed JSON request body now returns `400 Bad Request` (was `404 Not Found`)
92100

93101
When an endpoint expects a JSON body and the request body is present but not a parseable JSON **object** (truncated JSON, a bare array/string, …), the response is now **`400 Bad Request`**. Previously the failed parse fell through to parameter matching and surfaced as a misleading `404 Not Found`. Parse failures were and still are logged; valid requests and empty-body handling are unchanged.
@@ -141,3 +149,6 @@ This fixes a startup crash: previously a missing optional variable left an unres
141149
- A binding-contract test locking the PostgreSQL/Npgsql resolution the JSON-parameter fix relies on (`json`-only for the old binding; `json`/`jsonb`/`text` for the new one, including `NULL` and round-trip integrity), plus end-to-end CSV upload tests for a row-command metadata parameter declared as `json`, `jsonb`, and `text`.
142150
- Config tests for optional `{NAME}` (resolves when set; left untouched / defaults when not — including Serilog-template preservation) and required `{!NAME}` (throws when unset) across `GetConfigBool` / `GetConfigInt` / `GetConfigStr` and the `ResolveEnv` resolver.
143151
- Malformed-JSON body tests: truncated JSON and non-object JSON → `400`; valid body unchanged.
152+
- SSE hardening suite: hint-scope authorization (the security-fix regression test — role-scoped, authenticated-scoped, and unscoped delivery across three differently-authenticated subscribers), multi-subscriber exactly-once fan-out, per-stream publish ordering, and subscriber-disconnect resilience.
153+
- Cache concurrency races: TTL-expiry under concurrent bursts (exactly one execution per cache window) and concurrent invalidation + read storms (no errors, cache coherent after).
154+
- CRUD endpoint authorization parity: table-comment `authorize`/roles enforced across select/insert/update/delete variants (401 anonymous, 403 wrong role, full cycle with the right role).

0 commit comments

Comments
 (0)