From 51e335f090afca457bf12ec4df7b547baac2a2ff Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 11 Dec 2024 21:17:46 +0100 Subject: [PATCH] feat: use a FixedWindowRateLimiter keyed by IP to rate limit emails we don't talk about the sent_emails table :) --- .../Authentication/EmailAuthController.cs | 37 +++++++++++++++++++ Foxnouns.Backend/Database/DatabaseContext.cs | 2 - .../DatabaseContextModelSnapshot.cs | 27 -------------- Foxnouns.Backend/Database/Models/SentEmail.cs | 13 ------- .../Extensions/WebApplicationExtensions.cs | 1 + .../Services/DataCleanupService.cs | 11 ------ Foxnouns.Backend/Services/EmailRateLimiter.cs | 36 ++++++++++++++++++ Foxnouns.Backend/Services/MailService.cs | 35 +----------------- 8 files changed, 75 insertions(+), 87 deletions(-) delete mode 100644 Foxnouns.Backend/Database/Models/SentEmail.cs create mode 100644 Foxnouns.Backend/Services/EmailRateLimiter.cs diff --git a/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs b/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs index a3854a6..1587f87 100644 --- a/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs +++ b/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs @@ -36,6 +36,7 @@ public class EmailAuthController( DatabaseContext db, AuthService authService, MailService mailService, + EmailRateLimiter rateLimiter, KeyCacheService keyCacheService, UserRendererService userRenderer, IClock clock, @@ -68,6 +69,9 @@ public class EmailAuthController( return NoContent(); } + if (IsRateLimited()) + return NoContent(); + mailService.QueueAccountCreationEmail(req.Email, state); return NoContent(); } @@ -221,6 +225,9 @@ public class EmailAuthController( return NoContent(); } + if (IsRateLimited()) + return NoContent(); + mailService.QueueAddEmailAddressEmail(req.Email, state, CurrentUser.Username); return NoContent(); } @@ -274,4 +281,34 @@ public class EmailAuthController( if (!config.EmailAuth.Enabled) throw new ApiError.BadRequest("Email authentication is not enabled on this instance."); } + + /// + /// Checks whether the context's IP address is rate limited from dispatching emails. + /// + private bool IsRateLimited() + { + if (HttpContext.Connection.RemoteIpAddress == null) + { + _logger.Information( + "No remote IP address in HTTP context for email-related request, ignoring as we can't rate limit it" + ); + return true; + } + + if ( + !rateLimiter.IsLimited( + HttpContext.Connection.RemoteIpAddress.ToString(), + out Duration retryAfter + ) + ) + { + return false; + } + + _logger.Information( + "IP address cannot send email until {RetryAfter}, ignoring", + retryAfter + ); + return true; + } } diff --git a/Foxnouns.Backend/Database/DatabaseContext.cs b/Foxnouns.Backend/Database/DatabaseContext.cs index 7407c5b..ddf7853 100644 --- a/Foxnouns.Backend/Database/DatabaseContext.cs +++ b/Foxnouns.Backend/Database/DatabaseContext.cs @@ -66,7 +66,6 @@ public class DatabaseContext(DbContextOptions options) : DbContext(options) public DbSet Applications { get; init; } = null!; public DbSet TemporaryKeys { get; init; } = null!; public DbSet DataExports { get; init; } = null!; - public DbSet SentEmails { get; init; } = null!; public DbSet PrideFlags { get; init; } = null!; public DbSet UserFlags { get; init; } = null!; @@ -86,7 +85,6 @@ public class DatabaseContext(DbContextOptions options) : DbContext(options) modelBuilder.Entity().HasIndex(m => m.Sid).IsUnique(); modelBuilder.Entity().HasIndex(k => k.Key).IsUnique(); modelBuilder.Entity().HasIndex(d => d.Filename).IsUnique(); - modelBuilder.Entity().HasIndex(e => new { e.Email, e.SentAt }); // Two indexes on auth_methods, one for fediverse auth and one for all other types. modelBuilder diff --git a/Foxnouns.Backend/Database/Migrations/DatabaseContextModelSnapshot.cs b/Foxnouns.Backend/Database/Migrations/DatabaseContextModelSnapshot.cs index a9f59e6..cfe2513 100644 --- a/Foxnouns.Backend/Database/Migrations/DatabaseContextModelSnapshot.cs +++ b/Foxnouns.Backend/Database/Migrations/DatabaseContextModelSnapshot.cs @@ -302,33 +302,6 @@ namespace Foxnouns.Backend.Database.Migrations b.ToTable("pride_flags", (string)null); }); - modelBuilder.Entity("Foxnouns.Backend.Database.Models.SentEmail", b => - { - b.Property("Id") - .ValueGeneratedOnAdd() - .HasColumnType("integer") - .HasColumnName("id"); - - NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); - - b.Property("Email") - .IsRequired() - .HasColumnType("text") - .HasColumnName("email"); - - b.Property("SentAt") - .HasColumnType("timestamp with time zone") - .HasColumnName("sent_at"); - - b.HasKey("Id") - .HasName("pk_sent_emails"); - - b.HasIndex("Email", "SentAt") - .HasDatabaseName("ix_sent_emails_email_sent_at"); - - b.ToTable("sent_emails", (string)null); - }); - modelBuilder.Entity("Foxnouns.Backend.Database.Models.TemporaryKey", b => { b.Property("Id") diff --git a/Foxnouns.Backend/Database/Models/SentEmail.cs b/Foxnouns.Backend/Database/Models/SentEmail.cs deleted file mode 100644 index 09f03d9..0000000 --- a/Foxnouns.Backend/Database/Models/SentEmail.cs +++ /dev/null @@ -1,13 +0,0 @@ -using System.ComponentModel.DataAnnotations.Schema; -using NodaTime; - -namespace Foxnouns.Backend.Database.Models; - -public class SentEmail -{ - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - public int Id { get; init; } - - public required string Email { get; init; } - public required Instant SentAt { get; init; } -} diff --git a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs index 30d97d8..41c9712 100644 --- a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs +++ b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs @@ -110,6 +110,7 @@ public static class WebApplicationExtensions .AddSingleton(SystemClock.Instance) .AddSnowflakeGenerator() .AddSingleton() + .AddSingleton() .AddScoped() .AddScoped() .AddScoped() diff --git a/Foxnouns.Backend/Services/DataCleanupService.cs b/Foxnouns.Backend/Services/DataCleanupService.cs index 5e40c08..3d60462 100644 --- a/Foxnouns.Backend/Services/DataCleanupService.cs +++ b/Foxnouns.Backend/Services/DataCleanupService.cs @@ -33,9 +33,6 @@ public class DataCleanupService( public async Task InvokeAsync(CancellationToken ct = default) { - _logger.Debug("Cleaning up sent email cache"); - await CleanEmailsAsync(ct); - _logger.Debug("Cleaning up expired users"); await CleanUsersAsync(ct); @@ -43,14 +40,6 @@ public class DataCleanupService( await CleanExportsAsync(ct); } - private async Task CleanEmailsAsync(CancellationToken ct = default) - { - Instant expiry = clock.GetCurrentInstant() - Duration.FromHours(2); - int count = await db.SentEmails.Where(e => e.SentAt < expiry).ExecuteDeleteAsync(ct); - if (count != 0) - _logger.Information("Deleted {Count} entries from the sent email cache", expiry); - } - private async Task CleanUsersAsync(CancellationToken ct = default) { Instant selfDeleteExpires = clock.GetCurrentInstant() - User.DeleteAfter; diff --git a/Foxnouns.Backend/Services/EmailRateLimiter.cs b/Foxnouns.Backend/Services/EmailRateLimiter.cs new file mode 100644 index 0000000..9e73792 --- /dev/null +++ b/Foxnouns.Backend/Services/EmailRateLimiter.cs @@ -0,0 +1,36 @@ +using System.Collections.Concurrent; +using System.Threading.RateLimiting; +using NodaTime; +using NodaTime.Extensions; + +namespace Foxnouns.Backend.Services; + +public class EmailRateLimiter +{ + private readonly ConcurrentDictionary _limiters = new(); + + private readonly FixedWindowRateLimiterOptions _limiterOptions = + new() { Window = TimeSpan.FromHours(2), PermitLimit = 3 }; + + private RateLimiter GetLimiter(string bucket) => + _limiters.GetOrAdd(bucket, _ => new FixedWindowRateLimiter(_limiterOptions)); + + public bool IsLimited(string bucket, out Duration retryAfter) + { + RateLimiter limiter = GetLimiter(bucket); + RateLimitLease lease = limiter.AttemptAcquire(); + + if (!lease.IsAcquired) + { + retryAfter = lease.TryGetMetadata(MetadataName.RetryAfter, out TimeSpan timeSpan) + ? timeSpan.ToDuration() + : default; + } + else + { + retryAfter = Duration.Zero; + } + + return !lease.IsAcquired; + } +} diff --git a/Foxnouns.Backend/Services/MailService.cs b/Foxnouns.Backend/Services/MailService.cs index e162b33..a1444d9 100644 --- a/Foxnouns.Backend/Services/MailService.cs +++ b/Foxnouns.Backend/Services/MailService.cs @@ -15,22 +15,11 @@ using Coravel.Mailer.Mail; using Coravel.Mailer.Mail.Interfaces; using Coravel.Queuing.Interfaces; -using Foxnouns.Backend.Database; -using Foxnouns.Backend.Database.Models; using Foxnouns.Backend.Mailables; -using Microsoft.EntityFrameworkCore; -using NodaTime; namespace Foxnouns.Backend.Services; -public class MailService( - ILogger logger, - IMailer mailer, - IQueue queue, - IClock clock, - Config config, - IServiceProvider serviceProvider -) +public class MailService(ILogger logger, IMailer mailer, IQueue queue, Config config) { private readonly ILogger _logger = logger.ForContext(); @@ -78,29 +67,7 @@ public class MailService( { try { - // ReSharper disable SuggestVarOrType_SimpleTypes - await using var scope = serviceProvider.CreateAsyncScope(); - await using var db = scope.ServiceProvider.GetRequiredService(); - // ReSharper restore SuggestVarOrType_SimpleTypes - - Instant now = clock.GetCurrentInstant(); - - int count = await db.SentEmails.CountAsync(e => - e.Email == to && e.SentAt > (now - Duration.FromHours(1)) - ); - if (count >= 2) - { - _logger.Information( - "Have already sent 2 or more emails to {ToAddress} in the past hour, not sending new email", - to - ); - return; - } - await mailer.SendAsync(mailable); - - db.SentEmails.Add(new SentEmail { Email = to, SentAt = now }); - await db.SaveChangesAsync(); } catch (Exception exc) {