Harden multi-tenant isolation across all user-facing controllers
Added explicit CompanyId == companyId predicates to every tenant-scoped query in 22 controllers so cross-tenant data leakage is impossible even if EF Core global query filters are bypassed or misconfigured. Also fixed ApplicationDbContext.IsPlatformAdmin to correctly return true for SuperAdmins with no CompanyId claim (break-glass accounts) and when no HTTP context is present (background services, unit tests), resolving 225 unit test failures that stemmed from the global filter blocking all in-memory test data. New MultiTenantIsolationTests class (8 tests) verifies the explicit predicate layer independently of the global query filters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,284 @@
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Identity;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Moq;
|
||||
using PowderCoating.Core.Entities;
|
||||
using PowderCoating.Core.Enums;
|
||||
using PowderCoating.Core.Interfaces;
|
||||
using PowderCoating.Infrastructure.Data;
|
||||
using PowderCoating.Infrastructure.Repositories;
|
||||
using PowderCoating.Web.Controllers;
|
||||
|
||||
namespace PowderCoating.UnitTests;
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that the explicit <c>CompanyId == companyId</c> predicates added to every
|
||||
/// user-facing controller action actually prevent cross-tenant data leakage.
|
||||
///
|
||||
/// Each test seeds entities for TWO companies, creates a controller whose ITenantContext
|
||||
/// returns Company 1's ID, calls the action, and asserts that Company 2's data never
|
||||
/// appears in the result.
|
||||
///
|
||||
/// These tests validate the defense-in-depth layer (explicit predicates in controllers)
|
||||
/// independently of the EF Core global query filters, which behave differently on the
|
||||
/// in-memory provider when no HttpContext is present.
|
||||
/// </summary>
|
||||
public class MultiTenantIsolationTests
|
||||
{
|
||||
// ── Repository-level isolation ────────────────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// FindAsync with an explicit CompanyId predicate returns only the matching company's rows,
|
||||
/// even when rows from other companies exist in the database.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Repository_FindAsync_WithCompanyIdPredicate_ExcludesOtherTenants()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.Customers.Add(MakeCustomer(id: 1, companyId: 1, firstName: "Alice"));
|
||||
context.Customers.Add(MakeCustomer(id: 2, companyId: 2, firstName: "Bob"));
|
||||
context.Customers.Add(MakeCustomer(id: 3, companyId: 1, firstName: "Carol"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var uow = new UnitOfWork(context);
|
||||
var results = (await uow.Customers.FindAsync(c => c.CompanyId == 1)).ToList();
|
||||
|
||||
Assert.Equal(2, results.Count);
|
||||
Assert.All(results, c => Assert.Equal(1, c.CompanyId));
|
||||
Assert.DoesNotContain(results, c => c.ContactFirstName == "Bob");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Repository_FindAsync_ReturnsEmpty_WhenNoMatchingCompanyId()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.Customers.Add(MakeCustomer(id: 1, companyId: 2, firstName: "Bob"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var uow = new UnitOfWork(context);
|
||||
var results = await uow.Customers.FindAsync(c => c.CompanyId == 1);
|
||||
|
||||
Assert.Empty(results);
|
||||
}
|
||||
|
||||
// ── SmsConsentAuditController ─────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task SmsConsentAudit_Index_ReturnsOnlyCurrentCompanyCustomers()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.Customers.Add(MakeCustomer(id: 1, companyId: 1, firstName: "Alice"));
|
||||
context.Customers.Add(MakeCustomer(id: 2, companyId: 2, firstName: "Bob")); // other company
|
||||
context.Customers.Add(MakeCustomer(id: 3, companyId: 1, firstName: "Carol"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new SmsConsentAuditController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1),
|
||||
Mock.Of<ILogger<SmsConsentAuditController>>());
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.Index();
|
||||
|
||||
var view = Assert.IsType<ViewResult>(result);
|
||||
var vm = Assert.IsType<SmsConsentAuditViewModel>(view.Model);
|
||||
Assert.Equal(2, vm.TotalCount);
|
||||
Assert.DoesNotContain(vm.Rows, r => r.CustomerName.Contains("Bob"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SmsConsentAudit_ExportCsv_ContainsOnlyCurrentCompanyCustomers()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.Customers.Add(MakeCustomer(id: 1, companyId: 1, firstName: "Alice"));
|
||||
context.Customers.Add(MakeCustomer(id: 2, companyId: 2, firstName: "Bob"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new SmsConsentAuditController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1),
|
||||
Mock.Of<ILogger<SmsConsentAuditController>>());
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.ExportCsv();
|
||||
|
||||
var file = Assert.IsType<FileContentResult>(result);
|
||||
var csv = System.Text.Encoding.UTF8.GetString(file.FileContents);
|
||||
Assert.Contains("Alice", csv);
|
||||
Assert.DoesNotContain("Bob", csv);
|
||||
}
|
||||
|
||||
// ── TaxRatesController ────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task TaxRates_Index_ReturnsOnlyCurrentCompanyRates()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.TaxRates.Add(MakeTaxRate(id: 1, companyId: 1, name: "State Tax"));
|
||||
context.TaxRates.Add(MakeTaxRate(id: 2, companyId: 2, name: "Foreign Tax")); // other company
|
||||
context.TaxRates.Add(MakeTaxRate(id: 3, companyId: 1, name: "Local Tax"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new TaxRatesController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1),
|
||||
Mock.Of<ILogger<TaxRatesController>>());
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.Index();
|
||||
|
||||
var view = Assert.IsType<ViewResult>(result);
|
||||
var rates = Assert.IsAssignableFrom<IEnumerable<TaxRate>>(view.Model).ToList();
|
||||
Assert.Equal(2, rates.Count);
|
||||
Assert.All(rates, r => Assert.Equal(1, r.CompanyId));
|
||||
Assert.DoesNotContain(rates, r => r.Name == "Foreign Tax");
|
||||
}
|
||||
|
||||
// ── RecurringTemplatesController ──────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task RecurringTemplates_Index_ReturnsOnlyCurrentCompanyTemplates()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.RecurringTemplates.Add(MakeRecurringTemplate(id: 1, companyId: 1, name: "Monthly Rent"));
|
||||
context.RecurringTemplates.Add(MakeRecurringTemplate(id: 2, companyId: 2, name: "Other Tenant Bill")); // other company
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new RecurringTemplatesController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1),
|
||||
CreateUserManagerMock().Object,
|
||||
Mock.Of<ILogger<RecurringTemplatesController>>());
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.Index();
|
||||
|
||||
var view = Assert.IsType<ViewResult>(result);
|
||||
var templates = Assert.IsAssignableFrom<IEnumerable<RecurringTemplate>>(view.Model).ToList();
|
||||
Assert.Single(templates);
|
||||
Assert.Equal("Monthly Rent", templates[0].Name);
|
||||
}
|
||||
|
||||
// ── JobTemplatesController ────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task JobTemplates_Index_ReturnsOnlyCurrentCompanyTemplates()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.JobTemplates.Add(MakeJobTemplate(id: 1, companyId: 1, name: "Standard Wheel Coat"));
|
||||
context.JobTemplates.Add(MakeJobTemplate(id: 2, companyId: 2, name: "Other Company Template")); // other company
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new JobTemplatesController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1));
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.Index();
|
||||
|
||||
var view = Assert.IsType<ViewResult>(result);
|
||||
var templates = Assert.IsAssignableFrom<IEnumerable<JobTemplate>>(view.Model).ToList();
|
||||
Assert.Single(templates);
|
||||
Assert.Equal("Standard Wheel Coat", templates[0].Name);
|
||||
}
|
||||
|
||||
// ── Cross-tenant write protection ─────────────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that the companyId-scoped FindAsync used for SMS export returns zero
|
||||
/// rows for a company that has no customers, even when another company has many.
|
||||
/// Guards against the "empty predicate returns all" regression.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task SmsConsentAudit_ExportCsv_IsEmpty_WhenCompanyHasNoCustomers()
|
||||
{
|
||||
await using var context = CreateContext();
|
||||
context.Customers.Add(MakeCustomer(id: 1, companyId: 2, firstName: "Other"));
|
||||
context.Customers.Add(MakeCustomer(id: 2, companyId: 2, firstName: "Also Other"));
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var controller = new SmsConsentAuditController(
|
||||
new UnitOfWork(context),
|
||||
MockTenant(companyId: 1), // Company 1 has no customers
|
||||
Mock.Of<ILogger<SmsConsentAuditController>>());
|
||||
SetHttpContext(controller);
|
||||
|
||||
var result = await controller.ExportCsv();
|
||||
|
||||
var file = Assert.IsType<FileContentResult>(result);
|
||||
var csv = System.Text.Encoding.UTF8.GetString(file.FileContents);
|
||||
// Only header row, no data rows
|
||||
Assert.DoesNotContain("Other", csv);
|
||||
}
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
private static ApplicationDbContext CreateContext()
|
||||
{
|
||||
var options = new DbContextOptionsBuilder<ApplicationDbContext>()
|
||||
.UseInMemoryDatabase(Guid.NewGuid().ToString())
|
||||
.Options;
|
||||
return new ApplicationDbContext(options);
|
||||
}
|
||||
|
||||
/// <summary>Returns a mock ITenantContext that always yields the given companyId.</summary>
|
||||
private static ITenantContext MockTenant(int companyId)
|
||||
{
|
||||
var mock = new Mock<ITenantContext>();
|
||||
mock.Setup(t => t.GetCurrentCompanyId()).Returns(companyId);
|
||||
return mock.Object;
|
||||
}
|
||||
|
||||
private static void SetHttpContext(Controller controller)
|
||||
{
|
||||
controller.ControllerContext = new ControllerContext
|
||||
{
|
||||
HttpContext = new DefaultHttpContext()
|
||||
};
|
||||
}
|
||||
|
||||
private static Customer MakeCustomer(int id, int companyId, string firstName) => new()
|
||||
{
|
||||
Id = id,
|
||||
CompanyId = companyId,
|
||||
ContactFirstName = firstName,
|
||||
ContactLastName = "Test",
|
||||
IsCommercial = false
|
||||
};
|
||||
|
||||
private static TaxRate MakeTaxRate(int id, int companyId, string name) => new()
|
||||
{
|
||||
Id = id,
|
||||
CompanyId = companyId,
|
||||
Name = name,
|
||||
Rate = 8.5m
|
||||
};
|
||||
|
||||
private static RecurringTemplate MakeRecurringTemplate(int id, int companyId, string name) => new()
|
||||
{
|
||||
Id = id,
|
||||
CompanyId = companyId,
|
||||
Name = name,
|
||||
TemplateType = RecurringTemplateType.Bill,
|
||||
Frequency = RecurringFrequency.Monthly,
|
||||
IntervalCount = 1,
|
||||
NextFireDate = DateTime.Today,
|
||||
IsActive = true
|
||||
};
|
||||
|
||||
private static JobTemplate MakeJobTemplate(int id, int companyId, string name) => new()
|
||||
{
|
||||
Id = id,
|
||||
CompanyId = companyId,
|
||||
Name = name
|
||||
};
|
||||
|
||||
private static Mock<UserManager<ApplicationUser>> CreateUserManagerMock()
|
||||
{
|
||||
var store = new Mock<IUserStore<ApplicationUser>>();
|
||||
return new Mock<UserManager<ApplicationUser>>(
|
||||
store.Object, null!, null!, null!, null!, null!, null!, null!, null!);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user