diff --git a/dotnet/EcencyApi.Tests/NotificationsAuthorizationTests.cs b/dotnet/EcencyApi.Tests/NotificationsAuthorizationTests.cs index f5118e46..bb87e7ce 100644 --- a/dotnet/EcencyApi.Tests/NotificationsAuthorizationTests.cs +++ b/dotnet/EcencyApi.Tests/NotificationsAuthorizationTests.cs @@ -10,6 +10,7 @@ namespace EcencyApi.Tests; /// /// Kept separate from path construction because these are the rules that decide whose /// data is served, and a regression here is silent rather than a visible break. +/// The account is the validated code only, including when the body names someone else. /// public class NotificationsAuthorizationTests { @@ -40,38 +41,131 @@ public void AValidCodeAloneServesThatAccountsCompleteFeed() [Theory] [InlineData("good-karma")] - // Hive names are lowercase, but the comparison must not hinge on that. + // The body's spelling of the same account must not replace the code's account. [InlineData("Good-Karma")] [InlineData("GOOD-KARMA")] - public void NamingYourOwnAccountIsStillASelfView(string requested) + public void NamingYourOwnAccountStillServesTheCode(string requested) { var (username, fullScope) = PrivateApi.ResolveNotificationsTarget("good-karma", requested); - Assert.Equal(requested, username); + Assert.Equal("good-karma", username); Assert.True(fullScope); } [Fact] - public void NamingAnotherAccountIsServedTheRestrictedFeed() + public void ABodyNamingAnotherAccountDoesNotOverrideTheCode() { - // Still permitted: Decks builds notification columns for arbitrary accounts and - // notifications are largely public. It just does not unlock the complete feed. + // Previously this returned someone-else (a restricted feed). The body does not + // choose the account: the code's feed is served, not the one named here. var (username, fullScope) = PrivateApi.ResolveNotificationsTarget("good-karma", "someone-else"); - Assert.Equal("someone-else", username); - Assert.False(fullScope); + Assert.Equal("good-karma", username); + Assert.NotEqual("someone-else", username); + Assert.True(fullScope); } [Fact] - public void OnlyASelfViewEverSetsFullScope() + public void ANearMissNameDoesNotSwitchTheAccount() { - // The property that matters: for any requested account other than the validated - // one, fullScope is false. A near-miss must not slip through. foreach (var other in new[] { "good-karm", "good-karma2", "ood-karma", " good-karma", "good_karma" }) { - Assert.False( - PrivateApi.ResolveNotificationsTarget("good-karma", other).FullScope, - other); + var resolved = PrivateApi.ResolveNotificationsTarget("good-karma", other); + Assert.Equal("good-karma", resolved.Username); + Assert.True(resolved.FullScope); } } } + +/// +/// The notifications handler itself: a code for one account cannot be pointed at +/// another by the body's user field, and nothing is fetched when the code +/// does not validate. +/// +[Collection("notifications-auth")] +public class NotificationsHandlerTests : IDisposable +{ + private readonly CurationDeskTestSupport.Recorder _upstream = new(); + + public NotificationsHandlerTests() + { + // `code` "as:alice" validates as alice; anything else is invalid. + PrivateApi.NotificationsValidateCode = body => + { + var code = body["code"]?.GetValue(); + return Task.FromResult(code != null && code.StartsWith("as:", StringComparison.Ordinal) ? code[3..] : null); + }; + PrivateApi.NotificationsUpstream = (endpoint, method, headers) => + _upstream.Handle(endpoint, method, headers ?? Array.Empty>(), null); + } + + public void Dispose() + { + PrivateApi.NotificationsValidateCode = PrivateApi.ValidateCode; + PrivateApi.NotificationsUpstream = + (endpoint, method, extraHeaders) => + EcencyApi.Infrastructure.ApiClient.ApiRequest(endpoint, method, extraHeaders); + } + + [Fact] + public async Task ACodeAloneFetchesThatAccountsCompleteFeed() + { + var ctx = CurationDeskTestSupport.Post( + "/private-api/notifications", + """{"code":"as:alice"}"""); + await PrivateApi.Notifications(ctx); + + Assert.Equal(200, ctx.Response.StatusCode); + var call = Assert.Single(_upstream.Calls); + Assert.Equal("activities/alice?scope=full", call.Endpoint); + Assert.Equal(HttpMethod.Get, call.Method); + } + + [Theory] + [InlineData("alice")] + [InlineData("Alice")] + public async Task NamingTheCodesOwnAccountStillFetchesTheCode(string user) + { + var ctx = CurationDeskTestSupport.Post( + "/private-api/notifications", + $$"""{"code":"as:alice","user":"{{user}}"}"""); + await PrivateApi.Notifications(ctx); + + Assert.Equal(200, ctx.Response.StatusCode); + var call = Assert.Single(_upstream.Calls); + Assert.Equal("activities/alice?scope=full", call.Endpoint); + } + + [Theory] + [InlineData("victim")] + [InlineData("victim/unread-count")] + [InlineData("victim?x=1")] + public async Task ABodyNamingAnotherAccountServesTheCodesAccount(string user) + { + var ctx = CurationDeskTestSupport.Post( + "/private-api/notifications", + $$"""{"code":"as:alice","user":"{{user}}","filter":"follows","limit":20}"""); + await PrivateApi.Notifications(ctx); + + Assert.Equal(200, ctx.Response.StatusCode); + var call = Assert.Single(_upstream.Calls); + Assert.Equal(HttpMethod.Get, call.Method); + Assert.Equal("follows/alice?limit=20&scope=full", call.Endpoint); + Assert.DoesNotContain("victim", call.Endpoint); + } + + [Fact] + public async Task NamingAnAccountWithoutAValidCodeIsUnauthorized() + { + var ctx = CurationDeskTestSupport.Post( + "/private-api/notifications", + """{"code":"bogus","user":"victim"}"""); + await PrivateApi.Notifications(ctx); + + Assert.Equal(401, ctx.Response.StatusCode); + Assert.Equal("Unauthorized", CurationDeskTestSupport.Body(ctx)); + Assert.Empty(_upstream.Calls); + } +} + +[CollectionDefinition("notifications-auth", DisableParallelization = true)] +public class NotificationsAuthCollection { } diff --git a/dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs b/dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs index 5d8e0c14..15e4c439 100644 --- a/dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs +++ b/dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs @@ -13,15 +13,17 @@ public static partial class PrivateApi /// /// Who a notifications request is for, and whether it may see the complete feed. /// - /// A null Username means unauthorized. `requestedUser` is the body's `user` field - /// after JS-truthiness, or null when it was absent or falsy. + /// A null Username means unauthorized. The account is only the one + /// ValidateCode resolved — the same rule as . + /// is the body's user field after + /// JS-truthiness, or null when it was absent or falsy. It does not select the + /// account. It used to: an unauthenticated caller could pass the guard by naming + /// any account, and a valid code was then overwritten by that field with no + /// comparison (ecency/vision-api#90). /// - /// Two rules, both of which were wrong before: - /// - a validated code is REQUIRED. `requestedUser` used to satisfy the guard on its - /// own, so an unauthenticated caller could name any account. - /// - only a SELF view sees the complete feed. Naming another account is still - /// supported, because Decks builds notification columns for arbitrary accounts and - /// notifications are largely public, but it is served enotify's restricted feed. + /// An authorized request is therefore always a self-view, so it may see the + /// complete feed. A body that names another account still reads the code's + /// account; it is not a way to ask for someone else's activity. /// public static (string? Username, bool FullScope) ResolveNotificationsTarget( string? validatedUsername, string? requestedUser) @@ -31,20 +33,24 @@ public static (string? Username, bool FullScope) ResolveNotificationsTarget( return (null, false); } - if (requestedUser == null) - { - return (validatedUsername, true); - } - - return ( - requestedUser, - string.Equals(requestedUser, validatedUsername, StringComparison.OrdinalIgnoreCase)); + // Kept on the signature so a caller can show a body name was supplied, and so + // a test can fail if that name starts being returned. + _ = requestedUser; + return (validatedUsername, true); } + /// Signed-code validation for the notifications feed, replaceable for tests (no chain RPC). + internal static Func> NotificationsValidateCode = ValidateCode; + + /// The notifications feed's upstream call, replaceable so tests can observe the path. + internal static Func>?, Task> + NotificationsUpstream = + (endpoint, method, extraHeaders) => ApiClient.ApiRequest(endpoint, method, extraHeaders); + /// /// Whether a device request may act on `requestedUsername`. A push registration /// belongs to the account the code was issued for, so the device endpoints only - /// accept that account. Same case rule as ResolveNotificationsTarget. + /// accept that account. Comparison is case-insensitive, matching Hive names. /// public static bool IsOwnDeviceRequest(string? validatedUsername, string? requestedUsername) => !string.IsNullOrEmpty(validatedUsername) @@ -98,8 +104,9 @@ public static bool IsOwnDeviceRequest(string? validatedUsername, string? request } // Opts in to the complete feed. enotify defaults to chain-derived activity only, - // so omitting this is the safe direction: a cross-account view, or any request - // that never reaches this handler, gets the restricted feed. + // so omitting this is the safe direction: any request that does not ask, + // including one that never reaches this handler, gets the restricted feed. + // This handler asks only after a code has validated, and only for that account. if (fullScope) { query.Add("scope=full"); @@ -116,8 +123,10 @@ public static async Task Notifications(HttpContext ctx) // IsTruthy here rather than in the resolver, to keep the JS truthiness parity // this port is built on while the decision itself stays pure and testable. + // The body's user is passed in so the resolver can ignore it on purpose: + // it must not become the account that is queried. var (username, fullScope) = ResolveNotificationsTarget( - await ValidateCode(body), + await NotificationsValidateCode(body), JsJson.IsTruthy(user) ? UserData1Helpers.Template(user) : null); if (username == null) @@ -150,7 +159,7 @@ await ValidateCode(body), ? new[] { new KeyValuePair(EnotifyInternalTokenHeader, Config.EnotifyInternalToken) } : null; - await Upstream.Pipe(ApiClient.ApiRequest(u, HttpMethod.Get, extraHeaders), ctx); + await Upstream.Pipe(NotificationsUpstream(u, HttpMethod.Get, extraHeaders), ctx); } // GET ^/private-api/pub-notifications/:username