diff --git a/docs/en/framework/infrastructure/operation-rate-limiting.md b/docs/en/framework/infrastructure/operation-rate-limiting.md index 45139b1313..bdc55f1c7e 100644 --- a/docs/en/framework/infrastructure/operation-rate-limiting.md +++ b/docs/en/framework/infrastructure/operation-rate-limiting.md @@ -205,7 +205,9 @@ policy.WithFixedWindow(TimeSpan.FromHours(1), maxCount: 100) ## Multi-Tenancy -By default, rate limit counters are **shared across all tenants**. You can enable tenant isolation for a rule by calling `WithMultiTenancy()`: +By default, partition keys do not include tenant information — for partition types like `PartitionByParameter`, `PartitionByCurrentUser`, `PartitionByClientIp`, etc., counters are shared across tenants unless you call `WithMultiTenancy()`. Note that `PartitionByCurrentTenant()` is inherently per-tenant since the partition key is the tenant ID itself, and `PartitionByClientIp()` is typically kept global since the same IP should share a counter regardless of tenant. + +You can enable tenant isolation for a rule by calling `WithMultiTenancy()`: ````csharp policy.AddRule(rule => rule @@ -326,7 +328,7 @@ public override void ConfigureServices(ServiceConfigurationContext context) ### Ban Policy (maxCount: 0) -Setting `maxCount` to `0` creates a ban policy that blocks all requests for the specified duration: +Setting `maxCount` to `0` creates a ban policy that permanently denies all requests regardless of the window duration. The `RetryAfter` value will be `null` since there is no window to wait for: ````csharp options.AddPolicy("BlockedUser", policy => diff --git a/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Rules/FixedWindowOperationRateLimitingRule.cs b/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Rules/FixedWindowOperationRateLimitingRule.cs index 7df79a0052..cd2dd4db32 100644 --- a/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Rules/FixedWindowOperationRateLimitingRule.cs +++ b/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Rules/FixedWindowOperationRateLimitingRule.cs @@ -99,12 +99,24 @@ public class FixedWindowOperationRateLimitingRule : IOperationRateLimitingRule $"Phone number is required for policy '{PolicyName}' (PartitionByPhoneNumber). Provide it via context.Parameter or ensure the user has a phone number."), OperationRateLimitingPartitionType.Custom => - await Definition.CustomPartitionKeyResolver!(context), + await ResolveCustomPartitionKeyAsync(context), _ => throw new AbpException($"Unknown partition type: {Definition.PartitionType}") }; } + protected virtual async Task ResolveCustomPartitionKeyAsync(OperationRateLimitingContext context) + { + var key = await Definition.CustomPartitionKeyResolver!(context); + if (string.IsNullOrEmpty(key)) + { + throw new AbpException( + $"Custom partition key resolver returned null or empty for policy '{PolicyName}'. " + + "The resolver must return a non-empty string."); + } + return key; + } + protected virtual string BuildStoreKey(string partitionKey) { // Stable rule descriptor based on content so reordering rules does not change the key. diff --git a/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Store/DistributedCacheOperationRateLimitingStore.cs b/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Store/DistributedCacheOperationRateLimitingStore.cs index ca64981e3b..d9f13b41d1 100644 --- a/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Store/DistributedCacheOperationRateLimitingStore.cs +++ b/framework/src/Volo.Abp.OperationRateLimiting/Volo/Abp/OperationRateLimiting/Store/DistributedCacheOperationRateLimitingStore.cs @@ -38,7 +38,7 @@ public class DistributedCacheOperationRateLimitingStore : IOperationRateLimiting IsAllowed = false, CurrentCount = 0, MaxCount = maxCount, - RetryAfter = duration + RetryAfter = null }; } @@ -111,7 +111,7 @@ public class DistributedCacheOperationRateLimitingStore : IOperationRateLimiting IsAllowed = false, CurrentCount = 0, MaxCount = maxCount, - RetryAfter = duration + RetryAfter = null }; } diff --git a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/AbpOperationRateLimitingTestModule.cs b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/AbpOperationRateLimitingTestModule.cs index 7a43716927..cd436546f1 100644 --- a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/AbpOperationRateLimitingTestModule.cs +++ b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/AbpOperationRateLimitingTestModule.cs @@ -161,6 +161,13 @@ public class AbpOperationRateLimitingTestModule : AbpModule .PartitionBy(ctx => Task.FromResult($"action:{ctx.Parameter}")); }); + // Custom resolver returning null - should throw + options.AddPolicy("TestCustomResolverNull", policy => + { + policy.WithFixedWindow(TimeSpan.FromHours(1), maxCount: 2) + .PartitionBy(ctx => Task.FromResult(null!)); + }); + // Multi-tenant: ByParameter with tenant isolation - same param, different tenants = different counters options.AddPolicy("TestMultiTenantByParameter", policy => { diff --git a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/DistributedCacheOperationRateLimitingStore_Tests.cs b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/DistributedCacheOperationRateLimitingStore_Tests.cs index aa21159613..b612419e48 100644 --- a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/DistributedCacheOperationRateLimitingStore_Tests.cs +++ b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/DistributedCacheOperationRateLimitingStore_Tests.cs @@ -118,7 +118,7 @@ public class DistributedCacheOperationRateLimitingStore_Tests : OperationRateLim result.IsAllowed.ShouldBeFalse(); result.CurrentCount.ShouldBe(0); result.MaxCount.ShouldBe(0); - result.RetryAfter.ShouldNotBeNull(); + result.RetryAfter.ShouldBeNull(); } [Fact] @@ -130,6 +130,6 @@ public class DistributedCacheOperationRateLimitingStore_Tests : OperationRateLim result.IsAllowed.ShouldBeFalse(); result.CurrentCount.ShouldBe(0); result.MaxCount.ShouldBe(0); - result.RetryAfter.ShouldNotBeNull(); + result.RetryAfter.ShouldBeNull(); } } diff --git a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingChecker_Tests.cs b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingChecker_Tests.cs index 23dcfb6799..8a967077c5 100644 --- a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingChecker_Tests.cs +++ b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingChecker_Tests.cs @@ -653,6 +653,7 @@ public class OperationRateLimitingChecker_Tests : OperationRateLimitingTestBase exception.Result.IsAllowed.ShouldBeFalse(); exception.Result.MaxCount.ShouldBe(0); + exception.Result.RetryAfter.ShouldBeNull(); exception.HttpStatusCode.ShouldBe(429); } @@ -674,6 +675,7 @@ public class OperationRateLimitingChecker_Tests : OperationRateLimitingTestBase status.IsAllowed.ShouldBeFalse(); status.MaxCount.ShouldBe(0); status.RemainingCount.ShouldBe(0); + status.RetryAfter.ShouldBeNull(); } [Fact] @@ -701,6 +703,19 @@ public class OperationRateLimitingChecker_Tests : OperationRateLimitingTestBase (await _checker.IsAllowedAsync("TestCustomResolver", ctx2)).ShouldBeTrue(); } + [Fact] + public async Task Should_Throw_When_Custom_Resolver_Returns_Null() + { + var context = new OperationRateLimitingContext { Parameter = "test" }; + + var exception = await Assert.ThrowsAsync(async () => + { + await _checker.CheckAsync("TestCustomResolverNull", context); + }); + + exception.Message.ShouldContain("Custom partition key resolver returned null or empty"); + } + [Fact] public void Should_Throw_When_Policy_Has_Duplicate_Rules() { diff --git a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingMultiTenant_Tests.cs b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingMultiTenant_Tests.cs index fbd1a17059..b55ffbc966 100644 --- a/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingMultiTenant_Tests.cs +++ b/framework/test/Volo.Abp.OperationRateLimiting.Tests/Volo/Abp/OperationRateLimiting/OperationRateLimitingMultiTenant_Tests.cs @@ -59,7 +59,7 @@ public class OperationRateLimitingMultiTenant_Tests : OperationRateLimitingTestB { // ClientIp counters are global: requests from the same IP are counted together // regardless of which tenant context is active. - // The NullClientIpAddressProvider returns null, which resolves to "unknown" in the rule. + // The mock IWebClientInfoProvider returns "127.0.0.1" for all requests. using (_currentTenant.Change(TenantA)) { diff --git a/nupkg/common.ps1 b/nupkg/common.ps1 index edc5374c28..cb356faee4 100644 --- a/nupkg/common.ps1 +++ b/nupkg/common.ps1 @@ -237,7 +237,7 @@ $projects = ( "framework/src/Volo.Abp.Minify", "framework/src/Volo.Abp.ObjectExtending", "framework/src/Volo.Abp.ObjectMapping", - "framework/src/Volo.Abp.OperationRateLimit", + "framework/src/Volo.Abp.OperationRateLimiting", "framework/src/Volo.Abp.Quartz", "framework/src/Volo.Abp.RabbitMQ", "framework/src/Volo.Abp.RemoteServices",