From e6a0239436b1d0b3204ab2c562ef31f3584b3cc8 Mon Sep 17 00:00:00 2001
From: cesnimda <connor.babbington@cesnimda.co.uk>
Date: Wed, 1 Jul 2026 00:20:56 +0200
Subject: [PATCH] fix(security): systemic IDOR safeguard via EF global query
 filters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Defense-in-depth tenant isolation: every user-owned entity (Email, Sender,
MailThread, MailDomain, Attachment, Label, SyncState, AnalyticsAggregate,
WidgetLayout, UnsubscribeItem) gets a global query filter restricting reads to the
authenticated user. AppDbContext takes an optional ICurrentUser; CurrentUserId is
Guid.Empty for background workers / design-time, which DISABLES the filter so sync
and tooling (which already scope by an explicit userId) are unaffected. On the HTTP
attack surface a forgotten manual `WHERE UserId ==` can no longer leak another
tenant's rows.

Phase 1 confirmed no active IDOR; this is preventive, and prioritised now because the
upcoming automation engine will add many new queries.

Also: moved the Npgsql-only tsvector FTS mapping out of EmailConfiguration into
AppDbContext.OnModelCreating, guarded by Database.IsRelational() (Ignored otherwise),
so non-relational test providers work — honouring the existing Email.SearchVector
comment. Production (Npgsql) model is unchanged; no migration needed.

Adds 3 cross-user tenant-isolation integration tests. All 38 tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../Persistence/AppDbContext.cs               | 52 ++++++++++++-
 .../Configurations/EmailConfiguration.cs      | 11 +--
 .../TenantIsolationTests.cs                   | 78 +++++++++++++++++++
 3 files changed, 132 insertions(+), 9 deletions(-)
 create mode 100644 tests/InboxIntel.IntegrationTests/TenantIsolationTests.cs

diff --git a/src/InboxIntel.Infrastructure/Persistence/AppDbContext.cs b/src/InboxIntel.Infrastructure/Persistence/AppDbContext.cs
index e4d32ec..47d390a 100644
--- a/src/InboxIntel.Infrastructure/Persistence/AppDbContext.cs
+++ b/src/InboxIntel.Infrastructure/Persistence/AppDbContext.cs
@@ -8,7 +8,22 @@ namespace InboxIntel.Infrastructure.Persistence;
 
 public class AppDbContext : DbContext, IAppDbContext
 {
-    public AppDbContext(DbContextOptions<AppDbContext> options) : base(options) { }
+    private readonly ICurrentUser? _currentUser;
+
+    public AppDbContext(DbContextOptions<AppDbContext> options, ICurrentUser? currentUser = null) : base(options)
+        => _currentUser = currentUser;
+
+    /// <summary>
+    /// Tenant id used by the global query filters below. Resolves to the
+    /// authenticated user during an HTTP request. It is <see cref="Guid.Empty"/>
+    /// when there is no current user (background workers, design-time tooling,
+    /// startup migration) — in which case filtering is DISABLED, because those
+    /// paths are trusted server code that already scope their own queries by a
+    /// userId passed in explicitly. The security value is on the HTTP attack
+    /// surface: a forgotten manual <c>WHERE UserId ==</c> can no longer leak
+    /// another tenant's rows, since the filter restricts to the caller.
+    /// </summary>
+    public Guid CurrentUserId => _currentUser?.UserId ?? Guid.Empty;
 
     public DbSet<User> Users => Set<User>();
     public DbSet<Email> Emails => Set<Email>();
@@ -26,6 +41,41 @@ public class AppDbContext : DbContext, IAppDbContext
     protected override void OnModelCreating(ModelBuilder modelBuilder)
     {
         modelBuilder.ApplyConfigurationsFromAssembly(Assembly.GetExecutingAssembly());
+
+        // Defense-in-depth tenant isolation (systemic IDOR safeguard). Every
+        // user-owned entity is filtered to the current user so a query that forgets
+        // its manual `WHERE UserId ==` clause cannot leak across tenants. Applied
+        // uniformly to all user-scoped entities so EF sees no filtered/unfiltered
+        // navigation mismatch. Bypassed when CurrentUserId is Guid.Empty (workers).
+        modelBuilder.Entity<Email>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<Sender>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<MailThread>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<MailDomain>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<Attachment>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<Label>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<SyncState>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<AnalyticsAggregate>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<WidgetLayout>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+        modelBuilder.Entity<UnsubscribeItem>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
+
+        // PostgreSQL full-text search: generated tsvector over subject + body with a
+        // GIN index, maintained by the DB and read-only in code. The tsvector type is
+        // Npgsql-only, so map it only for relational providers and ignore it otherwise
+        // (e.g. the InMemory provider used by tests). Production behaviour is unchanged.
+        if (Database.IsRelational())
+        {
+            modelBuilder.Entity<Email>().Property(e => e.SearchVector)
+                .HasColumnType("tsvector")
+                .HasComputedColumnSql(
+                    "to_tsvector('english', coalesce(\"Subject\",'') || ' ' || coalesce(\"BodyText\",''))",
+                    stored: true);
+            modelBuilder.Entity<Email>().HasIndex(e => e.SearchVector).HasMethod("GIN");
+        }
+        else
+        {
+            modelBuilder.Entity<Email>().Ignore(e => e.SearchVector);
+        }
+
         base.OnModelCreating(modelBuilder);
     }
 
diff --git a/src/InboxIntel.Infrastructure/Persistence/Configurations/EmailConfiguration.cs b/src/InboxIntel.Infrastructure/Persistence/Configurations/EmailConfiguration.cs
index 7fa0155..590b8e5 100644
--- a/src/InboxIntel.Infrastructure/Persistence/Configurations/EmailConfiguration.cs
+++ b/src/InboxIntel.Infrastructure/Persistence/Configurations/EmailConfiguration.cs
@@ -36,13 +36,8 @@ public class EmailConfiguration : IEntityTypeConfiguration<Email>
             .HasForeignKey(e => e.SenderId)
             .OnDelete(DeleteBehavior.Restrict);
 
-        // PostgreSQL full-text search: generated tsvector over subject + body,
-        // with a GIN index. Maintained by the database, read-only in code.
-        b.Property(e => e.SearchVector)
-            .HasColumnType("tsvector")
-            .HasComputedColumnSql(
-                "to_tsvector('english', coalesce(\"Subject\",'') || ' ' || coalesce(\"BodyText\",''))",
-                stored: true);
-        b.HasIndex(e => e.SearchVector).HasMethod("GIN");
+        // NOTE: the PostgreSQL full-text `tsvector` mapping is applied in
+        // AppDbContext.OnModelCreating, guarded by Database.IsRelational(), so that
+        // non-relational test providers (InMemory) can ignore the Npgsql-only type.
     }
 }
diff --git a/tests/InboxIntel.IntegrationTests/TenantIsolationTests.cs b/tests/InboxIntel.IntegrationTests/TenantIsolationTests.cs
new file mode 100644
index 0000000..4f2ced5
--- /dev/null
+++ b/tests/InboxIntel.IntegrationTests/TenantIsolationTests.cs
@@ -0,0 +1,78 @@
+using FluentAssertions;
+using InboxIntel.Application.Abstractions;
+using InboxIntel.Domain.Entities;
+using InboxIntel.Infrastructure.Persistence;
+using Microsoft.EntityFrameworkCore;
+using Xunit;
+
+namespace InboxIntel.IntegrationTests;
+
+/// <summary>
+/// Verifies the global query-filter IDOR safeguard on AppDbContext: an HTTP-scoped
+/// context (with a current user) sees only that user's rows even if a query forgets
+/// its manual UserId filter; a worker-scoped context (Guid.Empty) sees everything.
+/// </summary>
+public class TenantIsolationTests
+{
+    private sealed class FakeCurrentUser : ICurrentUser
+    {
+        public Guid UserId { get; set; }
+        public bool IsAuthenticated => UserId != Guid.Empty;
+    }
+
+    private static DbContextOptions<AppDbContext> InMemory(string name)
+        => new DbContextOptionsBuilder<AppDbContext>().UseInMemoryDatabase(name).Options;
+
+    private static readonly Guid UserA = Guid.NewGuid();
+    private static readonly Guid UserB = Guid.NewGuid();
+
+    private static async Task SeedAsync(DbContextOptions<AppDbContext> opts)
+    {
+        // Seed with no current user (Guid.Empty) so the filter is bypassed for writes/reads here.
+        using var seed = new AppDbContext(opts, new FakeCurrentUser());
+        seed.Emails.Add(new Email { UserId = UserA, GmailMessageId = "a1", Subject = "A-one" });
+        seed.Emails.Add(new Email { UserId = UserA, GmailMessageId = "a2", Subject = "A-two" });
+        seed.Emails.Add(new Email { UserId = UserB, GmailMessageId = "b1", Subject = "B-one" });
+        await seed.SaveChangesAsync();
+    }
+
+    [Fact]
+    public async Task Authenticated_context_sees_only_its_own_rows_even_without_manual_filter()
+    {
+        var opts = InMemory(nameof(Authenticated_context_sees_only_its_own_rows_even_without_manual_filter));
+        await SeedAsync(opts);
+
+        // Note: NO manual .Where(e => e.UserId == ...) here — the global filter must enforce it.
+        using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = UserA });
+        var emails = await ctx.Emails.ToListAsync();
+
+        emails.Should().HaveCount(2);
+        emails.Should().OnlyContain(e => e.UserId == UserA);
+    }
+
+    [Fact]
+    public async Task Other_users_row_is_invisible_by_id()
+    {
+        var opts = InMemory(nameof(Other_users_row_is_invisible_by_id));
+        await SeedAsync(opts);
+
+        using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = UserA });
+        // Fetch B's row by its GmailMessageId — classic IDOR attempt; must return null.
+        var leaked = await ctx.Emails.FirstOrDefaultAsync(e => e.GmailMessageId == "b1");
+
+        leaked.Should().BeNull();
+    }
+
+    [Fact]
+    public async Task Empty_current_user_bypasses_the_filter_for_background_workers()
+    {
+        var opts = InMemory(nameof(Empty_current_user_bypasses_the_filter_for_background_workers));
+        await SeedAsync(opts);
+
+        // Guid.Empty == background/worker scope: must see all tenants' rows so sync/upsert works.
+        using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = Guid.Empty });
+        var all = await ctx.Emails.ToListAsync();
+
+        all.Should().HaveCount(3);
+    }
+}