diff --git a/README.md b/README.md index 1ab5509..55dd3bc 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ GET https://moogleapi.com/api/monsters?gameId=7 GET https://moogleapi.com/api/games ``` -No API key required. Pass `X-Api-Key: your-key` to get 10× the rate limit. +No API key required. Pass an issued `X-Api-Key: your-key` to get 10× the rate limit. --- @@ -153,10 +153,14 @@ Stages can be run individually with `--only=`: `games`, `characters`, `playable` | Tier | Limit | How | |------|-------|-----| | Anonymous | 60 req / min | Per IP, no setup needed | -| Premium | 600 req / min | Pass `X-Api-Key: your-key` header | +| Premium | 600 req / min | Pass an issued `X-Api-Key: your-key` header | Responses over the limit return `429 Too Many Requests`. +Premium keys have to be issued — an unrecognized key isn't rejected, it just falls back to the +anonymous limit, so the API stays usable if you send a stale one. Self-hosting? Set the +allowlist with `ApiKeys__Keys__0`, `ApiKeys__Keys__1`, … With none set, everything is anonymous. + --- ## 📜 Disclaimer diff --git a/src/MoogleAPI.Web/Features/Dashboard/Browse/MonstersEndpoint.cs b/src/MoogleAPI.Web/Features/Dashboard/Browse/MonstersEndpoint.cs index bd58820..cc7b1da 100644 --- a/src/MoogleAPI.Web/Features/Dashboard/Browse/MonstersEndpoint.cs +++ b/src/MoogleAPI.Web/Features/Dashboard/Browse/MonstersEndpoint.cs @@ -22,8 +22,16 @@ public override async Task HandleAsync(BrowseRequest req, CancellationToken ct) if (req.GameId.HasValue) query = query.Where(m => m.GameId == req.GameId.Value); + // Name *or* description, matching what /api/monsters/search already does. Curation + // needs the description: what marks a row as not-a-monster — "#REDIRECT ... enemy + // abilities", "may refer to", "is a genus of" — is only ever in the prose the scrape + // kept, never in the name. Searching names alone left those rows unreachable here. if (!string.IsNullOrWhiteSpace(req.Search)) - query = query.Where(m => EF.Functions.ILike(m.Name, $"%{req.Search.Trim()}%")); + { + var search = req.Search.Trim(); + query = query.Where(m => EF.Functions.ILike(m.Name, $"%{search}%") || + (m.Description != null && EF.Functions.ILike(m.Description, $"%{search}%"))); + } var total = await query.CountAsync(ct); var items = await query diff --git a/src/MoogleAPI.Web/Infrastructure/Middleware/RequestLoggingMiddleware.cs b/src/MoogleAPI.Web/Infrastructure/Middleware/RequestLoggingMiddleware.cs index ce3207e..562c8f1 100644 --- a/src/MoogleAPI.Web/Infrastructure/Middleware/RequestLoggingMiddleware.cs +++ b/src/MoogleAPI.Web/Infrastructure/Middleware/RequestLoggingMiddleware.cs @@ -2,10 +2,14 @@ using System.Text; using MoogleAPI.Web.Infrastructure.Data; using MoogleAPI.Web.Infrastructure.Models; +using MoogleAPI.Web.Infrastructure.RateLimiting; namespace MoogleAPI.Web.Infrastructure.Middleware; -public class RequestLoggingMiddleware(RequestDelegate next, IServiceScopeFactory scopeFactory) +public class RequestLoggingMiddleware( + RequestDelegate next, + IServiceScopeFactory scopeFactory, + ApiKeyValidator apiKeys) { public async Task InvokeAsync(HttpContext context) { @@ -32,11 +36,15 @@ public async Task InvokeAsync(HttpContext context) await next(context); var durationMs = (int)(Environment.TickCount64 - start); + // Read off the request before handing to the background write — by the time that runs + // the response is done and the context is no longer ours to read. + var isPremium = apiKeys.ResolveKey(context.Request) is not null; + // Fire-and-forget: never slow down the response for logging - _ = WriteLogAsync(context, path, durationMs); + _ = WriteLogAsync(context, path, durationMs, isPremium); } - private async Task WriteLogAsync(HttpContext context, string path, int durationMs) + private async Task WriteLogAsync(HttpContext context, string path, int durationMs, bool isPremium) { try { @@ -52,7 +60,9 @@ private async Task WriteLogAsync(HttpContext context, string path, int durationM DurationMs = durationMs, ResourceType = ExtractResourceType(path), SearchTerm = context.Request.Query["query"].FirstOrDefault(), - IsPremium = context.Request.Headers.ContainsKey("X-Api-Key"), + // Recognized key, not merely a present header — otherwise the premium share in + // /api/stats counts anyone who sent the header, valid or not. + IsPremium = isPremium, IpHash = HashIp(context.Connection.RemoteIpAddress?.ToString()), }); diff --git a/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiKeyValidator.cs b/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiKeyValidator.cs new file mode 100644 index 0000000..d65e560 --- /dev/null +++ b/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiKeyValidator.cs @@ -0,0 +1,35 @@ +using Microsoft.Extensions.Options; + +namespace MoogleAPI.Web.Infrastructure.RateLimiting; + +/// +/// Decides whether an X-Api-Key header actually entitles a caller to the premium rate +/// limit. The header used to be taken at face value — any non-empty value bought 10× the +/// anonymous limit, so the limit was opt-out rather than enforced. +/// +public class ApiKeyValidator +{ + public const string HeaderName = "X-Api-Key"; + + private readonly HashSet _keys; + + public ApiKeyValidator(IOptions options) => + _keys = options.Value.Keys + .Where(k => !string.IsNullOrWhiteSpace(k)) + .Select(k => k.Trim()) + .ToHashSet(StringComparer.Ordinal); + + public bool IsValid(string? apiKey) => + !string.IsNullOrWhiteSpace(apiKey) && _keys.Contains(apiKey.Trim()); + + /// + /// The recognized key on this request, or null for anonymous. An unrecognized key + /// returns null rather than throwing: the API is public and documented as needing no + /// key at all, so a bad one degrades to the anonymous limit instead of failing the request. + /// + public string? ResolveKey(HttpRequest request) + { + var apiKey = request.Headers[HeaderName].ToString(); + return IsValid(apiKey) ? apiKey.Trim() : null; + } +} diff --git a/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiRateLimiterPolicy.cs b/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiRateLimiterPolicy.cs index eae489a..56e42dc 100644 --- a/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiRateLimiterPolicy.cs +++ b/src/MoogleAPI.Web/Infrastructure/RateLimiting/ApiRateLimiterPolicy.cs @@ -1,12 +1,11 @@ using System.Threading.RateLimiting; -using Microsoft.AspNetCore.RateLimiting; namespace MoogleAPI.Web.Infrastructure.RateLimiting; public static class ApiRateLimiterPolicy { - public const string Anonymous = "anonymous"; - public const string Premium = "premium"; + public const int AnonymousPermitLimit = 60; + public const int PremiumPermitLimit = 600; public static IServiceCollection AddApiRateLimiting(this IServiceCollection services) { @@ -14,46 +13,34 @@ public static IServiceCollection AddApiRateLimiting(this IServiceCollection serv { options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; - options.AddPolicy(Anonymous, context => - RateLimitPartition.GetFixedWindowLimiter( - partitionKey: context.Connection.RemoteIpAddress?.ToString() ?? "unknown", - factory: _ => new FixedWindowRateLimiterOptions - { - PermitLimit = 60, - Window = TimeSpan.FromMinutes(1), - QueueProcessingOrder = QueueProcessingOrder.OldestFirst, - QueueLimit = 0 - })); - - // Premium users identified by X-Api-Key header get 10x the limit - options.AddPolicy(Premium, context => + // One global limiter rather than named policies. There were two named policies here + // that no endpoint ever attached with RequireRateLimiting, so they enforced nothing + // while appearing to — and the premium one carried the same unchecked-key flaw as + // this limiter did. Anything that needs a per-endpoint limit should be added back + // deliberately, and wired up. + options.GlobalLimiter = PartitionedRateLimiter.Create(context => { - var apiKey = context.Request.Headers["X-Api-Key"].ToString(); - return RateLimitPartition.GetFixedWindowLimiter( - partitionKey: string.IsNullOrEmpty(apiKey) ? $"ip:{context.Connection.RemoteIpAddress}" : $"key:{apiKey}", - factory: _ => new FixedWindowRateLimiterOptions + // The key is validated against the configured allowlist, so the partition is + // only ever keyed on a credential we issued. Partitioning on the raw header + // would otherwise let a caller mint unlimited fresh windows just by varying it. + var validator = context.RequestServices.GetRequiredService(); + var apiKey = validator.ResolveKey(context.Request); + + var partitionKey = apiKey is null + ? $"ip:{context.Connection.RemoteIpAddress}" + : $"key:{apiKey}"; + + var permitLimit = apiKey is null ? AnonymousPermitLimit : PremiumPermitLimit; + + return RateLimitPartition.GetFixedWindowLimiter(partitionKey, _ => + new FixedWindowRateLimiterOptions { - PermitLimit = string.IsNullOrEmpty(apiKey) ? 60 : 600, + PermitLimit = permitLimit, Window = TimeSpan.FromMinutes(1), QueueProcessingOrder = QueueProcessingOrder.OldestFirst, QueueLimit = 0 }); }); - - options.GlobalLimiter = PartitionedRateLimiter.Create(context => - { - var apiKey = context.Request.Headers["X-Api-Key"].ToString(); - var limit = string.IsNullOrEmpty(apiKey) ? 60 : 600; - var key = string.IsNullOrEmpty(apiKey) - ? $"ip:{context.Connection.RemoteIpAddress}" - : $"key:{apiKey}"; - - return RateLimitPartition.GetFixedWindowLimiter(key, _ => new FixedWindowRateLimiterOptions - { - PermitLimit = limit, - Window = TimeSpan.FromMinutes(1) - }); - }); }); return services; diff --git a/src/MoogleAPI.Web/Infrastructure/RateLimiting/PremiumKeyOptions.cs b/src/MoogleAPI.Web/Infrastructure/RateLimiting/PremiumKeyOptions.cs new file mode 100644 index 0000000..b57d4ad --- /dev/null +++ b/src/MoogleAPI.Web/Infrastructure/RateLimiting/PremiumKeyOptions.cs @@ -0,0 +1,22 @@ +namespace MoogleAPI.Web.Infrastructure.RateLimiting; + +/// +/// Named for what these keys buy rather than what they are: the API itself needs no key, and +/// ApiKeyOptions would collide with Scalar's type of that name in Program.cs. +/// +public class PremiumKeyOptions +{ + public const string SectionName = "ApiKeys"; + + /// + /// The keys entitled to the premium rate limit. Anything not on this list is treated as + /// anonymous, so an empty list simply means nobody has premium — which is the correct + /// default, and the reason this isn't validated at startup the way + /// is. + /// + /// + /// Supply via ApiKeys__Keys__0, ApiKeys__Keys__1, … (env vars) or user-secrets + /// in development. These are credentials: keep them out of appsettings.json. + /// + public List Keys { get; set; } = []; +} diff --git a/src/MoogleAPI.Web/Program.cs b/src/MoogleAPI.Web/Program.cs index a7f0e25..7e33883 100644 --- a/src/MoogleAPI.Web/Program.cs +++ b/src/MoogleAPI.Web/Program.cs @@ -44,7 +44,11 @@ }; }); -// Partitioned rate limiting: 60 req/min anonymous, 600 req/min with X-Api-Key +// Partitioned rate limiting: 60 req/min anonymous, 600 req/min with a recognized X-Api-Key. +// The allowlist is what makes the premium tier mean anything — without it the header was +// self-service. No startup validation: an empty list legitimately means nobody has premium. +builder.Services.Configure(builder.Configuration.GetSection(PremiumKeyOptions.SectionName)); +builder.Services.AddSingleton(); builder.Services.AddApiRateLimiting(); // Daily puzzle seeding. Validated at startup rather than on first request: an empty secret diff --git a/tests/MoogleAPI.Tests/ApiKeyTests.cs b/tests/MoogleAPI.Tests/ApiKeyTests.cs new file mode 100644 index 0000000..ec3e290 --- /dev/null +++ b/tests/MoogleAPI.Tests/ApiKeyTests.cs @@ -0,0 +1,102 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; +using MoogleAPI.Web.Infrastructure.RateLimiting; + +namespace MoogleAPI.Tests; + +public class ApiKeyTests +{ + private static ApiKeyValidator Validator(params string[] keys) => + new(Options.Create(new PremiumKeyOptions { Keys = [.. keys] })); + + private static HttpRequest RequestWith(string? apiKey) + { + var context = new DefaultHttpContext(); + if (apiKey is not null) + context.Request.Headers[ApiKeyValidator.HeaderName] = apiKey; + return context.Request; + } + + [Fact] + public void RecognizedKeyIsValid() + { + Assert.True(Validator("sponsor-key").IsValid("sponsor-key")); + } + + // The regression this whole class exists for: any non-empty header used to buy the + // premium limit, because the value was never compared against anything. + [Theory] + [InlineData("x")] + [InlineData("not-a-real-key")] + [InlineData("SPONSOR-KEY")] // keys are case-sensitive + [InlineData("sponsor-key-2")] // no prefix matching + public void UnrecognizedKeyIsNotValid(string apiKey) + { + Assert.False(Validator("sponsor-key").IsValid(apiKey)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void MissingOrBlankKeyIsNotValid(string? apiKey) + { + Assert.False(Validator("sponsor-key").IsValid(apiKey)); + } + + [Fact] + public void NoConfiguredKeysMeansNobodyIsPremium() + { + var validator = Validator(); + + Assert.False(validator.IsValid("sponsor-key")); + Assert.False(validator.IsValid("anything")); + } + + [Fact] + public void SurroundingWhitespaceIsToleratedOnBothSides() + { + // Config values and curl invocations both pick up stray whitespace; a key that is + // otherwise correct shouldn't be rejected for it. + Assert.True(Validator(" sponsor-key ").IsValid("sponsor-key")); + Assert.True(Validator("sponsor-key").IsValid(" sponsor-key ")); + } + + [Fact] + public void BlankConfiguredKeysAreDiscarded() + { + // An unset env var binds as an empty string. If that were kept as a key, sending an + // empty header would match it and premium would be self-service again. + var validator = Validator("", " ", "sponsor-key"); + + Assert.False(validator.IsValid("")); + Assert.False(validator.IsValid(" ")); + Assert.True(validator.IsValid("sponsor-key")); + } + + [Fact] + public void ResolveKeyReturnsTheKeyForARecognizedHeader() + { + Assert.Equal("sponsor-key", Validator("sponsor-key").ResolveKey(RequestWith("sponsor-key"))); + } + + [Fact] + public void ResolveKeyFallsBackToAnonymousRatherThanFailing() + { + var validator = Validator("sponsor-key"); + + Assert.Null(validator.ResolveKey(RequestWith("bogus"))); + Assert.Null(validator.ResolveKey(RequestWith(null))); + } + + [Fact] + public void ResolveKeyNormalizesSoOneKeyCannotHoldSeveralRateLimitWindows() + { + // The resolved value becomes the limiter's partition key. If padding survived here, + // " key" and "key " would be separate partitions and the limit would multiply. + var validator = Validator("sponsor-key"); + + Assert.Equal("sponsor-key", validator.ResolveKey(RequestWith(" sponsor-key"))); + Assert.Equal("sponsor-key", validator.ResolveKey(RequestWith("sponsor-key "))); + } +}